From 7c0b981d0e70ffa2e29405a79cf0bccd5a02e0fa Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 20 Nov 2020 15:56:42 -0500 Subject: [PATCH 1/7] Add support for associating existing models using the 'for' factory method --- .../Database/Eloquent/Factories/Factory.php | 10 ++++++++-- tests/Database/DatabaseEloquentFactoryTest.php | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Factories/Factory.php b/src/Illuminate/Database/Eloquent/Factories/Factory.php index e719683b70e2..7cd8e2548e81 100644 --- a/src/Illuminate/Database/Eloquent/Factories/Factory.php +++ b/src/Illuminate/Database/Eloquent/Factories/Factory.php @@ -500,12 +500,18 @@ public function hasAttached(self $factory, $pivot = [], $relationship = null) /** * Define a parent relationship for the model. * - * @param \Illuminate\Database\Eloquent\Factories\Factory $factory + * @param \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Database\Eloquent\Model $factory * @param string|null $relationship * @return static */ - public function for(self $factory, $relationship = null) + public function for($factory, $relationship = null) { + if ($factory instanceof Model) { + return $this->afterMaking(function ($model) use ($factory, $relationship) { + $model->{$relationship ?: Str::camel(class_basename($factory))}()->associate($factory); + }); + } + return $this->newInstance(['for' => $this->for->concat([new BelongsToRelationship( $factory, $relationship ?: Str::camel(class_basename($factory->modelName())) diff --git a/tests/Database/DatabaseEloquentFactoryTest.php b/tests/Database/DatabaseEloquentFactoryTest.php index 3b87fca3ee58..5ae8dfc54793 100644 --- a/tests/Database/DatabaseEloquentFactoryTest.php +++ b/tests/Database/DatabaseEloquentFactoryTest.php @@ -250,6 +250,21 @@ public function test_belongs_to_relationship() $this->assertCount(3, FactoryTestPost::all()); } + public function test_belongs_to_relationship_with_existing_model_instance() + { + $user = FactoryTestUserFactory::new(['name' => 'Taylor Otwell'])->create(); + $posts = FactoryTestPostFactory::times(3) + ->for($user, 'user') + ->create(); + + $this->assertCount(3, $posts->filter(function ($post) use ($user) { + return $post->user->is($user); + })); + + $this->assertCount(1, FactoryTestUser::all()); + $this->assertCount(3, FactoryTestPost::all()); + } + public function test_morph_to_relationship() { $posts = FactoryTestCommentFactory::times(3) From 6afbe0d05fc13e27427aa6e006ebbab5e10c163a Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 20 Nov 2020 16:35:16 -0500 Subject: [PATCH 2/7] Add support for 'hasAttached' --- .../Database/Eloquent/Factories/Factory.php | 12 ++++++++-- .../Database/DatabaseEloquentFactoryTest.php | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Factories/Factory.php b/src/Illuminate/Database/Eloquent/Factories/Factory.php index 7cd8e2548e81..2c6726c43326 100644 --- a/src/Illuminate/Database/Eloquent/Factories/Factory.php +++ b/src/Illuminate/Database/Eloquent/Factories/Factory.php @@ -481,13 +481,21 @@ protected function guessRelationship(string $related) /** * Define an attached relationship for the model. * - * @param \Illuminate\Database\Eloquent\Factories\Factory $factory + * @param \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Support\Collection|\Illuminate\Database\Eloquent\Model $factory * @param callable|array $pivot * @param string|null $relationship * @return static */ - public function hasAttached(self $factory, $pivot = [], $relationship = null) + public function hasAttached($factory, $pivot = [], $relationship = null) { + if ($factory instanceof Collection || $factory instanceof Model) { + $factory = Collection::wrap($factory); + + return $this->afterCreating(function ($model) use ($factory, $pivot, $relationship) { + $model->{$relationship ?: Str::camel(Str::plural(class_basename($factory->first())))}()->attach($factory, $pivot); + }); + } + return $this->newInstance([ 'has' => $this->has->concat([new BelongsToManyRelationship( $factory, diff --git a/tests/Database/DatabaseEloquentFactoryTest.php b/tests/Database/DatabaseEloquentFactoryTest.php index 5ae8dfc54793..ea607a564861 100644 --- a/tests/Database/DatabaseEloquentFactoryTest.php +++ b/tests/Database/DatabaseEloquentFactoryTest.php @@ -305,6 +305,28 @@ public function test_belongs_to_many_relationship() unset($_SERVER['__test.role.creating-user']); } + public function test_belongs_to_many_relationship_with_existing_model_instances() + { + $roles = FactoryTestRoleFactory::times(3)->afterCreating(function ($role) { + $_SERVER['__test.role.creating-role'] = $role; + }) + ->create(); + $users = FactoryTestUserFactory::times(3) + ->hasAttached($roles, ['admin' => 'Y'], 'roles') + ->create(); + + $this->assertCount(3, FactoryTestRole::all()); + + $user = FactoryTestUser::latest()->first(); + + $this->assertCount(3, $user->roles); + $this->assertSame('Y', $user->roles->first()->pivot->admin); + + $this->assertInstanceOf(Eloquent::class, $_SERVER['__test.role.creating-role']); + + unset($_SERVER['__test.role.creating-role']); + } + public function test_sequences() { $users = FactoryTestUserFactory::times(2)->sequence( From a2e567b4e5a30378f41fa8cc5a38d2c30be97cb4 Mon Sep 17 00:00:00 2001 From: "John T. Bonaccorsi" Date: Sun, 22 Nov 2020 23:44:39 -0500 Subject: [PATCH 3/7] Fix support for BelongsTo with existing model --- .../Database/Eloquent/Factories/BelongsToRelationship.php | 8 ++++---- src/Illuminate/Database/Eloquent/Factories/Factory.php | 5 ----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php b/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php index 1a4ceb3c0d0b..b0e4863f1a58 100644 --- a/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php +++ b/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php @@ -31,11 +31,11 @@ class BelongsToRelationship /** * Create a new "belongs to" relationship definition. * - * @param \Illuminate\Database\Eloquent\Factories\Factory $factory + * @param \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Database\Eloquent\Model $factory * @param string $relationship * @return void */ - public function __construct(Factory $factory, $relationship) + public function __construct($factory, $relationship) { $this->factory = $factory; $this->relationship = $relationship; @@ -52,7 +52,7 @@ public function attributesFor(Model $model) $relationship = $model->{$this->relationship}(); return $relationship instanceof MorphTo ? [ - $relationship->getMorphType() => $this->factory->newModel()->getMorphClass(), + $relationship->getMorphType() => $this->factory instanceof Model ? $this->factory->getMorphClass() : $this->factory->newModel()->getMorphClass(), $relationship->getForeignKeyName() => $this->resolver($relationship->getOwnerKeyName()), ] : [ $relationship->getForeignKeyName() => $this->resolver($relationship->getOwnerKeyName()), @@ -69,7 +69,7 @@ protected function resolver($key) { return function () use ($key) { if (! $this->resolved) { - $instance = $this->factory->create(); + $instance = $this->factory instanceof Model ? $this->factory : $this->factory->create(); return $this->resolved = $key ? $instance->{$key} : $instance->getKey(); } diff --git a/src/Illuminate/Database/Eloquent/Factories/Factory.php b/src/Illuminate/Database/Eloquent/Factories/Factory.php index 2c6726c43326..b51a36412008 100644 --- a/src/Illuminate/Database/Eloquent/Factories/Factory.php +++ b/src/Illuminate/Database/Eloquent/Factories/Factory.php @@ -514,11 +514,6 @@ public function hasAttached($factory, $pivot = [], $relationship = null) */ public function for($factory, $relationship = null) { - if ($factory instanceof Model) { - return $this->afterMaking(function ($model) use ($factory, $relationship) { - $model->{$relationship ?: Str::camel(class_basename($factory))}()->associate($factory); - }); - } return $this->newInstance(['for' => $this->for->concat([new BelongsToRelationship( $factory, From c278a965511be32925f3d00903d5de370f4f667a Mon Sep 17 00:00:00 2001 From: "John T. Bonaccorsi" Date: Sun, 22 Nov 2020 23:59:24 -0500 Subject: [PATCH 4/7] Fix support for BelongsToMany with existing model --- .../Factories/BelongsToManyRelationship.php | 8 ++++---- .../Eloquent/Factories/BelongsToRelationship.php | 6 +++--- .../Database/Eloquent/Factories/Factory.php | 8 -------- tests/Database/DatabaseEloquentFactoryTest.php | 16 +++++++++++++++- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php b/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php index 6395b28e02a2..e124448ff5a2 100644 --- a/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php +++ b/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php @@ -10,7 +10,7 @@ class BelongsToManyRelationship /** * The related factory instance. * - * @var \Illuminate\Database\Eloquent\Factories\Factory + * @var \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Database\Eloquent\Model */ protected $factory; @@ -31,12 +31,12 @@ class BelongsToManyRelationship /** * Create a new attached relationship definition. * - * @param \Illuminate\Database\Eloquent\Factories\Factory $factory + * @param \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Support\Collection|\Illuminate\Database\Eloquent\Model $factory * @param callable|array $pivot * @param string $relationship * @return void */ - public function __construct(Factory $factory, $pivot, $relationship) + public function __construct($factory, $pivot, $relationship) { $this->factory = $factory; $this->pivot = $pivot; @@ -51,7 +51,7 @@ public function __construct(Factory $factory, $pivot, $relationship) */ public function createFor(Model $model) { - Collection::wrap($this->factory->create([], $model))->each(function ($attachable) use ($model) { + Collection::wrap($this->factory instanceof Factory ? $this->factory->create([], $model) : $this->factory)->each(function ($attachable) use ($model) { $model->{$this->relationship}()->attach( $attachable, is_callable($this->pivot) ? call_user_func($this->pivot, $model) : $this->pivot diff --git a/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php b/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php index b0e4863f1a58..55747fdc6488 100644 --- a/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php +++ b/src/Illuminate/Database/Eloquent/Factories/BelongsToRelationship.php @@ -10,7 +10,7 @@ class BelongsToRelationship /** * The related factory instance. * - * @var \Illuminate\Database\Eloquent\Factories\Factory + * @var \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Database\Eloquent\Model */ protected $factory; @@ -52,7 +52,7 @@ public function attributesFor(Model $model) $relationship = $model->{$this->relationship}(); return $relationship instanceof MorphTo ? [ - $relationship->getMorphType() => $this->factory instanceof Model ? $this->factory->getMorphClass() : $this->factory->newModel()->getMorphClass(), + $relationship->getMorphType() => $this->factory instanceof Factory ? $this->factory->newModel()->getMorphClass() : $this->factory->getMorphClass(), $relationship->getForeignKeyName() => $this->resolver($relationship->getOwnerKeyName()), ] : [ $relationship->getForeignKeyName() => $this->resolver($relationship->getOwnerKeyName()), @@ -69,7 +69,7 @@ protected function resolver($key) { return function () use ($key) { if (! $this->resolved) { - $instance = $this->factory instanceof Model ? $this->factory : $this->factory->create(); + $instance = $this->factory instanceof Factory ? $this->factory->create() : $this->factory; return $this->resolved = $key ? $instance->{$key} : $instance->getKey(); } diff --git a/src/Illuminate/Database/Eloquent/Factories/Factory.php b/src/Illuminate/Database/Eloquent/Factories/Factory.php index b51a36412008..6b6c315945a5 100644 --- a/src/Illuminate/Database/Eloquent/Factories/Factory.php +++ b/src/Illuminate/Database/Eloquent/Factories/Factory.php @@ -488,14 +488,6 @@ protected function guessRelationship(string $related) */ public function hasAttached($factory, $pivot = [], $relationship = null) { - if ($factory instanceof Collection || $factory instanceof Model) { - $factory = Collection::wrap($factory); - - return $this->afterCreating(function ($model) use ($factory, $pivot, $relationship) { - $model->{$relationship ?: Str::camel(Str::plural(class_basename($factory->first())))}()->attach($factory, $pivot); - }); - } - return $this->newInstance([ 'has' => $this->has->concat([new BelongsToManyRelationship( $factory, diff --git a/tests/Database/DatabaseEloquentFactoryTest.php b/tests/Database/DatabaseEloquentFactoryTest.php index ea607a564861..a7137116de26 100644 --- a/tests/Database/DatabaseEloquentFactoryTest.php +++ b/tests/Database/DatabaseEloquentFactoryTest.php @@ -278,6 +278,20 @@ public function test_morph_to_relationship() $this->assertCount(3, FactoryTestComment::all()); } + public function test_morph_to_relationship_with_existing_model_instance() + { + $post = FactoryTestPostFactory::new(['title' => 'Test Title'])->create(); + $posts = FactoryTestCommentFactory::times(3) + ->for($post, 'commentable') + ->create(); + + $this->assertSame('Test Title', FactoryTestPost::first()->title); + $this->assertCount(3, FactoryTestPost::first()->comments); + + $this->assertCount(1, FactoryTestPost::all()); + $this->assertCount(3, FactoryTestComment::all()); + } + public function test_belongs_to_many_relationship() { $users = FactoryTestUserFactory::times(3) @@ -311,7 +325,7 @@ public function test_belongs_to_many_relationship_with_existing_model_instances( $_SERVER['__test.role.creating-role'] = $role; }) ->create(); - $users = FactoryTestUserFactory::times(3) + FactoryTestUserFactory::times(3) ->hasAttached($roles, ['admin' => 'Y'], 'roles') ->create(); From f83d7dc3acdec4826cfc438ad4ab42ac55b2b2ad Mon Sep 17 00:00:00 2001 From: "John T. Bonaccorsi" Date: Mon, 23 Nov 2020 10:35:43 -0500 Subject: [PATCH 5/7] Update docblock --- .../Database/Eloquent/Factories/BelongsToManyRelationship.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php b/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php index e124448ff5a2..e0c42c4c642b 100644 --- a/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php +++ b/src/Illuminate/Database/Eloquent/Factories/BelongsToManyRelationship.php @@ -10,7 +10,7 @@ class BelongsToManyRelationship /** * The related factory instance. * - * @var \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Database\Eloquent\Model + * @var \Illuminate\Database\Eloquent\Factories\Factory|\Illuminate\Support\Collection|\Illuminate\Database\Eloquent\Model */ protected $factory; From 137f32b54ff3fd1cd7f164335ff78eef60cce69a Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 4 Dec 2020 11:14:58 -0500 Subject: [PATCH 6/7] Formatting --- src/Illuminate/Database/Eloquent/Factories/Factory.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Illuminate/Database/Eloquent/Factories/Factory.php b/src/Illuminate/Database/Eloquent/Factories/Factory.php index 6b6c315945a5..c335d9f301c8 100644 --- a/src/Illuminate/Database/Eloquent/Factories/Factory.php +++ b/src/Illuminate/Database/Eloquent/Factories/Factory.php @@ -506,7 +506,6 @@ public function hasAttached($factory, $pivot = [], $relationship = null) */ public function for($factory, $relationship = null) { - return $this->newInstance(['for' => $this->for->concat([new BelongsToRelationship( $factory, $relationship ?: Str::camel(class_basename($factory->modelName())) From fd2a2b997602a946d7124e0073366d4955fa9961 Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 4 Dec 2020 11:49:39 -0500 Subject: [PATCH 7/7] Formatting --- tests/Database/DatabaseEloquentFactoryTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Database/DatabaseEloquentFactoryTest.php b/tests/Database/DatabaseEloquentFactoryTest.php index a7137116de26..3c575e8ce989 100644 --- a/tests/Database/DatabaseEloquentFactoryTest.php +++ b/tests/Database/DatabaseEloquentFactoryTest.php @@ -321,7 +321,8 @@ public function test_belongs_to_many_relationship() public function test_belongs_to_many_relationship_with_existing_model_instances() { - $roles = FactoryTestRoleFactory::times(3)->afterCreating(function ($role) { + $roles = FactoryTestRoleFactory::times(3) + ->afterCreating(function ($role) { $_SERVER['__test.role.creating-role'] = $role; }) ->create();