From f7c7f10c416f14727dcc262e43a1370a4a5859bb Mon Sep 17 00:00:00 2001 From: Jack Date: Tue, 1 Jul 2025 22:06:43 +0100 Subject: [PATCH 1/5] fix & cs adjustments --- .../Database/Eloquent/Concerns/HasEvents.php | 6 + .../EloquentModelRelationAutoloadTest.php | 125 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php b/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php index fa654de966d4..17b05028e63a 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php +++ b/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php @@ -208,6 +208,12 @@ protected function fireModelEvent($event, $halt = true) return true; } + // If we have automatic eager loading enabled for the creating event unset any null relations + // as it can cause issues and set the relations to null even if they exist. + if (static::isAutomaticallyEagerLoadingRelationships() && $event == 'creating') { + $this->setRelations(array_filter($this->getRelations())); + } + // First, we will get the proper method to call on the event dispatcher, and then we // will attempt to fire a custom, object based event for the given event. If that // returns a result we can return that result, or we'll call the string events. diff --git a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php index 03f1d7ca611f..c2c35e170ffe 100644 --- a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php +++ b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php @@ -3,6 +3,8 @@ namespace Illuminate\Tests\Integration\Database\EloquentModelRelationAutoloadTest; use DB; +use Illuminate\Database\Eloquent\Factories\Factory; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; @@ -12,6 +14,14 @@ class EloquentModelRelationAutoloadTest extends DatabaseTestCase { protected function afterRefreshingDatabase() { + Schema::create('tags', function (Blueprint $table) { + $table->increments('id'); + $table->string('name')->nullable(); + $table->string('status')->nullable(); + $table->unsignedInteger('post_id')->nullable(); + $table->unsignedInteger('video_id')->nullable(); + }); + Schema::create('posts', function (Blueprint $table) { $table->increments('id'); }); @@ -214,6 +224,95 @@ public function testRelationAutoloadVariousNestedMorphRelations() DB::disableQueryLog(); } + + public function testDoesntCauseNQueryIssueUsingGet() + { + Model::automaticallyEagerLoadRelationships(); + + DB::enableQueryLog(); + + $post = Post::create(); + + $video = Video::create(); + $video2 = Video::create(); + $video3 = Video::create(); + + Tag::create(['post_id' => $post->id, 'video_id' => $video->id]); + Tag::create(['post_id' => $post->id, 'video_id' => $video2->id]); + Tag::create(['post_id' => $post->id, 'video_id' => $video3->id]); + + $videos = []; + foreach ($post->tags()->get() as $tag) { + $videos[] = $tag->video; + } + + $this->assertCount(12, DB::getQueryLog()); + $this->assertCount(3, $videos); + + Model::automaticallyEagerLoadRelationships(false); + } + + public function testRelationAutoloadWorksOnCreatingEvent() + { + Model::automaticallyEagerLoadRelationships(); + + DB::enableQueryLog(); + + $tags = Tag::factory()->times(3)->make(); + + $post = Post::factory()->create(); + + $post->tags()->saveMany($tags); + + $this->assertCount(7, DB::getQueryLog()); + + Model::automaticallyEagerLoadRelationships(false); + + DB::disableQueryLog(); + } +} + +class TagFactory extends Factory +{ + protected $model = Tag::class; + + public function definition() + { + return []; + } +} + +class Tag extends Model +{ + use HasFactory; + + public $timestamps = false; + + protected $guarded = []; + + protected static function booted() + { + static::creating(function ($model) { + if ($model->post->shouldApplyStatus()) { + $model->status = 'Todo'; + } + }); + } + + protected static function newFactory() + { + return TagFactory::new(); + } + + public function post() + { + return $this->belongsTo(Post::class); + } + + public function video() + { + return $this->belongsTo(Video::class); + } } class Comment extends Model @@ -238,10 +337,26 @@ public function commentable() } } +class PostFactory extends Factory +{ + protected $model = Post::class; + + public function definition() + { + return []; + } +} class Post extends Model { + use HasFactory; + public $timestamps = false; + public function shouldApplyStatus() + { + return false; + } + public function comments() { return $this->morphMany(Comment::class, 'commentable'); @@ -256,6 +371,16 @@ public function likes() { return $this->morphMany(Like::class, 'likeable'); } + + protected static function newFactory() + { + return PostFactory::new(); + } + + public function tags() + { + return $this->hasMany(Tag::class); + } } class Video extends Model From 6754c029f3f21a6e5cc6fc3ec6dec96d3a71b52f Mon Sep 17 00:00:00 2001 From: Jack Date: Thu, 3 Jul 2025 15:32:23 +0100 Subject: [PATCH 2/5] Update EloquentModelRelationAutoloadTest.php --- .../Integration/Database/EloquentModelRelationAutoloadTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php index c2c35e170ffe..ee6df16dbde4 100644 --- a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php +++ b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php @@ -225,7 +225,7 @@ public function testRelationAutoloadVariousNestedMorphRelations() DB::disableQueryLog(); } - public function testDoesntCauseNQueryIssueUsingGet() + public function testDoesntCauseNQueryIssueUsingGetMethod() { Model::automaticallyEagerLoadRelationships(); From 4d7bfd3487bc5fb6210b8014940dfbad1a83720d Mon Sep 17 00:00:00 2001 From: Jack Date: Thu, 3 Jul 2025 15:49:25 +0100 Subject: [PATCH 3/5] clean up test --- .../EloquentModelRelationAutoloadTest.php | 43 +------------------ 1 file changed, 1 insertion(+), 42 deletions(-) diff --git a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php index ee6df16dbde4..93df4ed51f69 100644 --- a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php +++ b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php @@ -225,33 +225,6 @@ public function testRelationAutoloadVariousNestedMorphRelations() DB::disableQueryLog(); } - public function testDoesntCauseNQueryIssueUsingGetMethod() - { - Model::automaticallyEagerLoadRelationships(); - - DB::enableQueryLog(); - - $post = Post::create(); - - $video = Video::create(); - $video2 = Video::create(); - $video3 = Video::create(); - - Tag::create(['post_id' => $post->id, 'video_id' => $video->id]); - Tag::create(['post_id' => $post->id, 'video_id' => $video2->id]); - Tag::create(['post_id' => $post->id, 'video_id' => $video3->id]); - - $videos = []; - foreach ($post->tags()->get() as $tag) { - $videos[] = $tag->video; - } - - $this->assertCount(12, DB::getQueryLog()); - $this->assertCount(3, $videos); - - Model::automaticallyEagerLoadRelationships(false); - } - public function testRelationAutoloadWorksOnCreatingEvent() { Model::automaticallyEagerLoadRelationships(); @@ -260,7 +233,7 @@ public function testRelationAutoloadWorksOnCreatingEvent() $tags = Tag::factory()->times(3)->make(); - $post = Post::factory()->create(); + $post = Post::create(); $post->tags()->saveMany($tags); @@ -337,15 +310,6 @@ public function commentable() } } -class PostFactory extends Factory -{ - protected $model = Post::class; - - public function definition() - { - return []; - } -} class Post extends Model { use HasFactory; @@ -372,11 +336,6 @@ public function likes() return $this->morphMany(Like::class, 'likeable'); } - protected static function newFactory() - { - return PostFactory::new(); - } - public function tags() { return $this->hasMany(Tag::class); From 8052500f387bed375f554e6c98056030a7d41b17 Mon Sep 17 00:00:00 2001 From: Jack Date: Thu, 3 Jul 2025 15:52:19 +0100 Subject: [PATCH 4/5] lil bit more cleanup --- .../Database/EloquentModelRelationAutoloadTest.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php index 93df4ed51f69..a0acef5802de 100644 --- a/tests/Integration/Database/EloquentModelRelationAutoloadTest.php +++ b/tests/Integration/Database/EloquentModelRelationAutoloadTest.php @@ -19,7 +19,6 @@ protected function afterRefreshingDatabase() $table->string('name')->nullable(); $table->string('status')->nullable(); $table->unsignedInteger('post_id')->nullable(); - $table->unsignedInteger('video_id')->nullable(); }); Schema::create('posts', function (Blueprint $table) { @@ -281,11 +280,6 @@ public function post() { return $this->belongsTo(Post::class); } - - public function video() - { - return $this->belongsTo(Video::class); - } } class Comment extends Model @@ -312,8 +306,6 @@ public function commentable() class Post extends Model { - use HasFactory; - public $timestamps = false; public function shouldApplyStatus() From aeb1980c96b18958c5cae3ce531048224f1a52f9 Mon Sep 17 00:00:00 2001 From: Jack Date: Thu, 3 Jul 2025 15:58:00 +0100 Subject: [PATCH 5/5] kill comment --- src/Illuminate/Database/Eloquent/Concerns/HasEvents.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php b/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php index 17b05028e63a..7f01cef02e4a 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php +++ b/src/Illuminate/Database/Eloquent/Concerns/HasEvents.php @@ -208,8 +208,6 @@ protected function fireModelEvent($event, $halt = true) return true; } - // If we have automatic eager loading enabled for the creating event unset any null relations - // as it can cause issues and set the relations to null even if they exist. if (static::isAutomaticallyEagerLoadingRelationships() && $event == 'creating') { $this->setRelations(array_filter($this->getRelations())); }