From 230eeef6113bf7a5632578508c3316583ac22534 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 13 Sep 2017 15:20:02 -0500 Subject: [PATCH 1/4] make unserialized models reload relationships - add new `getQueueableRelations` method to queuable interfaces - accept the 'relations' into the `ModelIdentifier` - implement `getQueueableRelations` method on Eloquent Collection and Model. - extract a method on `SerializesAndRestoresModelIdentifiers` for `restoreModel`. Load the relationships in this method. - add test --- .../Contracts/Database/ModelIdentifier.php | 11 ++- .../Contracts/Queue/QueueableCollection.php | 7 ++ .../Contracts/Queue/QueueableEntity.php | 7 ++ .../Database/Eloquent/Collection.php | 10 ++ src/Illuminate/Database/Eloquent/Model.php | 32 ++++++ .../SerializesAndRestoresModelIdentifiers.php | 21 +++- .../Queue/ModelSerializationTest.php | 99 +++++++++++++++++++ tests/Queue/QueueSyncQueueTest.php | 5 + 8 files changed, 187 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Contracts/Database/ModelIdentifier.php b/src/Illuminate/Contracts/Database/ModelIdentifier.php index 587044a8dab3..cdd4b9cc91a2 100644 --- a/src/Illuminate/Contracts/Database/ModelIdentifier.php +++ b/src/Illuminate/Contracts/Database/ModelIdentifier.php @@ -27,18 +27,27 @@ class ModelIdentifier */ public $connection; + /** + * The relationships loaded on the model. + * + * @var array + */ + public $relations; + /** * Create a new model identifier. * * @param string $class * @param mixed $id * @param mixed $connection + * @param array $relations * @return void */ - public function __construct($class, $id, $connection) + public function __construct($class, $id, $connection, $relations) { $this->id = $id; $this->class = $class; $this->connection = $connection; + $this->relations = $relations; } } diff --git a/src/Illuminate/Contracts/Queue/QueueableCollection.php b/src/Illuminate/Contracts/Queue/QueueableCollection.php index 0331b814f04b..a26fa57ab4d3 100644 --- a/src/Illuminate/Contracts/Queue/QueueableCollection.php +++ b/src/Illuminate/Contracts/Queue/QueueableCollection.php @@ -24,4 +24,11 @@ public function getQueueableIds(); * @return string|null */ public function getQueueableConnection(); + + /** + * Get the relationships of the entities being queued. + * + * @return array + */ + public function getQueueableRelations(); } diff --git a/src/Illuminate/Contracts/Queue/QueueableEntity.php b/src/Illuminate/Contracts/Queue/QueueableEntity.php index 00e28f8a070f..2388e1e2183c 100644 --- a/src/Illuminate/Contracts/Queue/QueueableEntity.php +++ b/src/Illuminate/Contracts/Queue/QueueableEntity.php @@ -17,4 +17,11 @@ public function getQueueableId(); * @return string|null */ public function getQueueableConnection(); + + /** + * Get the relationships for the entity. + * + * @return array + */ + public function getQueueableRelations(); } diff --git a/src/Illuminate/Database/Eloquent/Collection.php b/src/Illuminate/Database/Eloquent/Collection.php index 62e25c6dabd9..914e480f49ae 100755 --- a/src/Illuminate/Database/Eloquent/Collection.php +++ b/src/Illuminate/Database/Eloquent/Collection.php @@ -419,4 +419,14 @@ public function getQueueableConnection() return $connection; } + + /** + * Get the relationships of the entities being queued. + * + * @return array + */ + public function getQueueableRelations() + { + return $this->isNotEmpty() ? $this->first()->getRelations() : []; + } } diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index bee9bf0e47fb..43a5b93c0b13 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -13,6 +13,7 @@ use Illuminate\Contracts\Routing\UrlRoutable; use Illuminate\Contracts\Queue\QueueableEntity; use Illuminate\Database\Eloquent\Relations\Pivot; +use Illuminate\Contracts\Queue\QueueableCollection; use Illuminate\Database\Query\Builder as QueryBuilder; use Illuminate\Database\ConnectionResolverInterface as Resolver; @@ -1250,6 +1251,37 @@ public function getQueueableConnection() return $this->getConnectionName(); } + /** + * Get the queueable relationships for the entity. + * + * @return array + */ + public function getQueueableRelations() + { + $relations = []; + + foreach ($this->getRelations() as $key => $relation){ + + $relations[] = $key; + + if ($relation instanceof QueueableCollection) + { + foreach($relation->getQueueableRelations() as $collectionKey => $collectionValue){ + $relations[] = $key . '.' . $collectionKey; + } + } + + if ($relation instanceof QueueableEntity) + { + foreach($relation->getQueueableRelations() as $entityKey => $entityValue){ + $relations[] = $key . '.' . $entityValue; + } + } + } + + return $relations; + } + /** * Get the value of the model's route key. * diff --git a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php index 6bb677aee8ac..e90a0b8ea3b3 100644 --- a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php +++ b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php @@ -21,7 +21,8 @@ protected function getSerializedPropertyValue($value) return new ModelIdentifier( $value->getQueueableClass(), $value->getQueueableIds(), - $value->getQueueableConnection() + $value->getQueueableConnection(), + $value->getQueueableRelations() ); } @@ -29,7 +30,8 @@ protected function getSerializedPropertyValue($value) return new ModelIdentifier( get_class($value), $value->getQueueableId(), - $value->getQueueableConnection() + $value->getQueueableConnection(), + $value->getQueueableRelations() ); } @@ -50,8 +52,7 @@ protected function getRestoredPropertyValue($value) return is_array($value->id) ? $this->restoreCollection($value) - : $this->getQueryForModelRestoration((new $value->class)->setConnection($value->connection)) - ->useWritePdo()->findOrFail($value->id); + : $this->restoreModel($value); } /** @@ -72,6 +73,18 @@ protected function restoreCollection($value) ->whereIn($model->getQualifiedKeyName(), $value->id)->get(); } + /** + * @param \Illuminate\Contracts\Database\ModelIdentifier $value + * @return \Illuminate\Database\Eloquent\Model + */ + public function restoreModel($value) + { + $model = $this->getQueryForModelRestoration((new $value->class)->setConnection($value->connection)) + ->useWritePdo()->findOrFail($value->id); + + return $model->load($value->relations); + } + /** * Get the query for restoration. * diff --git a/tests/Integration/Queue/ModelSerializationTest.php b/tests/Integration/Queue/ModelSerializationTest.php index 0949b683f78c..1cfe302bd7c3 100644 --- a/tests/Integration/Queue/ModelSerializationTest.php +++ b/tests/Integration/Queue/ModelSerializationTest.php @@ -43,6 +43,21 @@ public function setUp() $table->increments('id'); $table->string('email'); }); + + Schema::create('orders', function ($table) { + $table->increments('id'); + }); + + Schema::create('lines', function ($table) { + $table->increments('id'); + $table->unsignedInteger('order_id'); + $table->unsignedInteger('product_id'); + }); + + Schema::create('products', function ($table) { + $table->increments('id'); + + }); } /** @@ -126,6 +141,50 @@ public function it_fails_if_models_on_multi_connections() unserialize($serialized); } + + /** + * @test + */ + public function it_reloads_relationships() + { + $order = Order::create(); + + $product1 = Product::create(); + $product2 = Product::create(); + + Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); + Line::create(['order_id' => $order->id, 'product_id' => $product2->id]); + Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); + + $order->load('lines'); + + $serialized = serialize(new ModelRelationSerializationTestClass($order)); + $unSerialized = unserialize($serialized); + + $this->assertEquals($unSerialized->order->getRelations(), $order->getRelations()); + } + + /** + * @test + */ + public function it_reloads_nested_relationships() + { + $order = Order::create(); + + $product1 = Product::create(); + $product2 = Product::create(); + + Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); + Line::create(['order_id' => $order->id, 'product_id' => $product2->id]); + Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); + + $order->load('lines', 'lines.product'); + + $nestedSerialized = serialize(new ModelRelationSerializationTestClass($order)); + $nestedUnSerialized = unserialize($nestedSerialized); + + $this->assertEquals($nestedUnSerialized->order->getRelations(), $order->getRelations()); + } } class ModelSerializationTestUser extends Model @@ -135,6 +194,34 @@ class ModelSerializationTestUser extends Model public $timestamps = false; } +class Order extends Model +{ + public $guarded = ['id']; + public $timestamps = false; + + public function lines() + { + return $this->hasMany(Line::class); + } +} + +class Line extends Model +{ + public $guarded = ['id']; + public $timestamps = false; + + public function product() + { + return $this->belongsTo(Product::class); + } +} + +class Product extends Model +{ + public $guarded = ['id']; + public $timestamps = false; +} + class ModelSerializationTestClass { use \Illuminate\Queue\SerializesModels; @@ -146,3 +233,15 @@ public function __construct($user) $this->user = $user; } } + +class ModelRelationSerializationTestClass +{ + use \Illuminate\Queue\SerializesModels; + + public $order; + + public function __construct($order) + { + $this->order = $order; + } +} diff --git a/tests/Queue/QueueSyncQueueTest.php b/tests/Queue/QueueSyncQueueTest.php index 8d533c8b197f..cbbac59c9c4a 100755 --- a/tests/Queue/QueueSyncQueueTest.php +++ b/tests/Queue/QueueSyncQueueTest.php @@ -61,6 +61,11 @@ public function getQueueableConnection() { // } + + public function getQueueableRelations() + { + // + } } class SyncQueueTestHandler From 85c1081b26b7cd2411f5ab9b2b5c734e22931933 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 14 Sep 2017 08:06:52 -0500 Subject: [PATCH 2/4] remove unneeded order lines --- tests/Integration/Queue/ModelSerializationTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Integration/Queue/ModelSerializationTest.php b/tests/Integration/Queue/ModelSerializationTest.php index 1cfe302bd7c3..829f6cd1eb30 100644 --- a/tests/Integration/Queue/ModelSerializationTest.php +++ b/tests/Integration/Queue/ModelSerializationTest.php @@ -154,7 +154,6 @@ public function it_reloads_relationships() Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); Line::create(['order_id' => $order->id, 'product_id' => $product2->id]); - Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); $order->load('lines'); @@ -176,7 +175,6 @@ public function it_reloads_nested_relationships() Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); Line::create(['order_id' => $order->id, 'product_id' => $product2->id]); - Line::create(['order_id' => $order->id, 'product_id' => $product1->id]); $order->load('lines', 'lines.product'); From 3c3c877d0626dd5258cf64fd1967f08dcb722481 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 14 Sep 2017 08:47:45 -0500 Subject: [PATCH 3/4] styleCI fixes --- src/Illuminate/Database/Eloquent/Model.php | 31 +++++++++---------- .../Queue/ModelSerializationTest.php | 1 - 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index 43a5b93c0b13..11739533f9bf 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -2,20 +2,20 @@ namespace Illuminate\Database\Eloquent; -use Exception; use ArrayAccess; -use JsonSerializable; use BadMethodCallException; -use Illuminate\Support\Arr; -use Illuminate\Support\Str; -use Illuminate\Contracts\Support\Jsonable; -use Illuminate\Contracts\Support\Arrayable; -use Illuminate\Contracts\Routing\UrlRoutable; +use Exception; +use Illuminate\Contracts\Queue\QueueableCollection; use Illuminate\Contracts\Queue\QueueableEntity; +use Illuminate\Contracts\Routing\UrlRoutable; +use Illuminate\Contracts\Support\Arrayable; +use Illuminate\Contracts\Support\Jsonable; +use Illuminate\Database\ConnectionResolverInterface as Resolver; use Illuminate\Database\Eloquent\Relations\Pivot; -use Illuminate\Contracts\Queue\QueueableCollection; use Illuminate\Database\Query\Builder as QueryBuilder; -use Illuminate\Database\ConnectionResolverInterface as Resolver; +use Illuminate\Support\Arr; +use Illuminate\Support\Str; +use JsonSerializable; /** * @mixin \Illuminate\Database\Eloquent\Builder @@ -1260,20 +1260,17 @@ public function getQueueableRelations() { $relations = []; - foreach ($this->getRelations() as $key => $relation){ - + foreach ($this->getRelations() as $key => $relation) { $relations[] = $key; - if ($relation instanceof QueueableCollection) - { - foreach($relation->getQueueableRelations() as $collectionKey => $collectionValue){ + if ($relation instanceof QueueableCollection) { + foreach ($relation->getQueueableRelations() as $collectionKey => $collectionValue) { $relations[] = $key . '.' . $collectionKey; } } - if ($relation instanceof QueueableEntity) - { - foreach($relation->getQueueableRelations() as $entityKey => $entityValue){ + if ($relation instanceof QueueableEntity) { + foreach ($relation->getQueueableRelations() as $entityKey => $entityValue) { $relations[] = $key . '.' . $entityValue; } } diff --git a/tests/Integration/Queue/ModelSerializationTest.php b/tests/Integration/Queue/ModelSerializationTest.php index 829f6cd1eb30..89b3b89ffbc9 100644 --- a/tests/Integration/Queue/ModelSerializationTest.php +++ b/tests/Integration/Queue/ModelSerializationTest.php @@ -56,7 +56,6 @@ public function setUp() Schema::create('products', function ($table) { $table->increments('id'); - }); } From 98ee9bac3bb89b7afc897f16fd9abc2676337490 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 14 Sep 2017 08:53:31 -0500 Subject: [PATCH 4/4] one more try at styleci --- src/Illuminate/Database/Eloquent/Model.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Model.php b/src/Illuminate/Database/Eloquent/Model.php index 11739533f9bf..8aa938b5f77b 100644 --- a/src/Illuminate/Database/Eloquent/Model.php +++ b/src/Illuminate/Database/Eloquent/Model.php @@ -2,20 +2,20 @@ namespace Illuminate\Database\Eloquent; +use Exception; use ArrayAccess; +use JsonSerializable; use BadMethodCallException; -use Exception; -use Illuminate\Contracts\Queue\QueueableCollection; -use Illuminate\Contracts\Queue\QueueableEntity; -use Illuminate\Contracts\Routing\UrlRoutable; -use Illuminate\Contracts\Support\Arrayable; +use Illuminate\Support\Arr; +use Illuminate\Support\Str; use Illuminate\Contracts\Support\Jsonable; -use Illuminate\Database\ConnectionResolverInterface as Resolver; +use Illuminate\Contracts\Support\Arrayable; +use Illuminate\Contracts\Routing\UrlRoutable; +use Illuminate\Contracts\Queue\QueueableEntity; use Illuminate\Database\Eloquent\Relations\Pivot; +use Illuminate\Contracts\Queue\QueueableCollection; use Illuminate\Database\Query\Builder as QueryBuilder; -use Illuminate\Support\Arr; -use Illuminate\Support\Str; -use JsonSerializable; +use Illuminate\Database\ConnectionResolverInterface as Resolver; /** * @mixin \Illuminate\Database\Eloquent\Builder @@ -1265,13 +1265,13 @@ public function getQueueableRelations() if ($relation instanceof QueueableCollection) { foreach ($relation->getQueueableRelations() as $collectionKey => $collectionValue) { - $relations[] = $key . '.' . $collectionKey; + $relations[] = $key.'.'.$collectionKey; } } if ($relation instanceof QueueableEntity) { foreach ($relation->getQueueableRelations() as $entityKey => $entityValue) { - $relations[] = $key . '.' . $entityValue; + $relations[] = $key.'.'.$entityValue; } } }