From 109c7c48d0aa96835ce9a1ed2cfc109e14433440 Mon Sep 17 00:00:00 2001 From: its Date: Thu, 11 Jul 2019 00:31:27 +0200 Subject: [PATCH 1/3] Fixed queue jobs using SerializesModels losing order of passed in collections. --- .../SerializesAndRestoresModelIdentifiers.php | 8 ++++-- .../Queue/ModelSerializationTest.php | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php index 4b9476e4c04b..d1d9be028f91 100644 --- a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php +++ b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php @@ -67,9 +67,13 @@ protected function restoreCollection($value) return new EloquentCollection; } - return $this->getQueryForModelRestoration( + $collection = $this->getQueryForModelRestoration( (new $value->class)->setConnection($value->connection), $value->id - )->useWritePdo()->get(); + )->useWritePdo()->get()->keyBy->getKey(); + + return new EloquentCollection( + collect($value->id)->map(function ($id) use ($collection) { return $collection[$id]; }) + ); } /** diff --git a/tests/Integration/Queue/ModelSerializationTest.php b/tests/Integration/Queue/ModelSerializationTest.php index 8b7f16530743..d704c09b095a 100644 --- a/tests/Integration/Queue/ModelSerializationTest.php +++ b/tests/Integration/Queue/ModelSerializationTest.php @@ -220,6 +220,21 @@ public function test_it_serializes_an_empty_collection() unserialize($serialized); } + + public function test_it_serializes_a_collection_in_correct_order() + { + ModelSerializationTestUser::create([ 'email' => 'mohamed@laravel.com' ]); + ModelSerializationTestUser::create([ 'email' => 'taylor@laravel.com' ]); + + $serialized = serialize(new CollectionSerializationTestClass( + ModelSerializationTestUser::orderByDesc('email')->get() + )); + + $unserialized = unserialize($serialized); + + $this->assertEquals($unserialized->users->first()->email, 'taylor@laravel.com'); + $this->assertEquals($unserialized->users->last()->email, 'mohamed@laravel.com'); + } } class ModelSerializationTestUser extends Model @@ -330,3 +345,15 @@ public function __construct($order) $this->order = $order; } } + +class CollectionSerializationTestClass +{ + use SerializesModels; + + public $users; + + public function __construct($users) + { + $this->users = $users; + } +} From ca6cedfc1c6bce2f312f6f94c5f0fb01b9696790 Mon Sep 17 00:00:00 2001 From: its Date: Thu, 11 Jul 2019 01:28:01 +0200 Subject: [PATCH 2/3] Skip the order recovery when recovering pivot records in SerializesModels. --- .../Queue/SerializesAndRestoresModelIdentifiers.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php index d1d9be028f91..0bb25a6cb07e 100644 --- a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php +++ b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php @@ -6,6 +6,8 @@ use Illuminate\Contracts\Database\ModelIdentifier; use Illuminate\Contracts\Queue\QueueableCollection; use Illuminate\Database\Eloquent\Collection as EloquentCollection; +use Illuminate\Database\Eloquent\Relations\Pivot; +use Illuminate\Database\Eloquent\Relations\Concerns\AsPivot; trait SerializesAndRestoresModelIdentifiers { @@ -69,7 +71,13 @@ protected function restoreCollection($value) $collection = $this->getQueryForModelRestoration( (new $value->class)->setConnection($value->connection), $value->id - )->useWritePdo()->get()->keyBy->getKey(); + )->useWritePdo()->get(); + + if (is_a($value->class, Pivot::class, true) || in_array(AsPivot::class, class_uses($value->class))) { + return $collection; + } + + $collection = $collection->keyBy->getKey(); return new EloquentCollection( collect($value->id)->map(function ($id) use ($collection) { return $collection[$id]; }) From d6b93c908bb8f2efc29f33c325db3f74dc24457f Mon Sep 17 00:00:00 2001 From: its Date: Mon, 15 Jul 2019 13:33:04 +0200 Subject: [PATCH 3/3] Code style fixes. --- .../Queue/SerializesAndRestoresModelIdentifiers.php | 8 +++++--- tests/Integration/Queue/ModelSerializationTest.php | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php index 0bb25a6cb07e..7924de411aca 100644 --- a/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php +++ b/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php @@ -3,11 +3,11 @@ namespace Illuminate\Queue; use Illuminate\Contracts\Queue\QueueableEntity; +use Illuminate\Database\Eloquent\Relations\Pivot; use Illuminate\Contracts\Database\ModelIdentifier; use Illuminate\Contracts\Queue\QueueableCollection; -use Illuminate\Database\Eloquent\Collection as EloquentCollection; -use Illuminate\Database\Eloquent\Relations\Pivot; use Illuminate\Database\Eloquent\Relations\Concerns\AsPivot; +use Illuminate\Database\Eloquent\Collection as EloquentCollection; trait SerializesAndRestoresModelIdentifiers { @@ -80,7 +80,9 @@ protected function restoreCollection($value) $collection = $collection->keyBy->getKey(); return new EloquentCollection( - collect($value->id)->map(function ($id) use ($collection) { return $collection[$id]; }) + collect($value->id)->map(function ($id) use ($collection) { + return $collection[$id]; + }) ); } diff --git a/tests/Integration/Queue/ModelSerializationTest.php b/tests/Integration/Queue/ModelSerializationTest.php index d704c09b095a..c56eb5f8c42f 100644 --- a/tests/Integration/Queue/ModelSerializationTest.php +++ b/tests/Integration/Queue/ModelSerializationTest.php @@ -223,8 +223,8 @@ public function test_it_serializes_an_empty_collection() public function test_it_serializes_a_collection_in_correct_order() { - ModelSerializationTestUser::create([ 'email' => 'mohamed@laravel.com' ]); - ModelSerializationTestUser::create([ 'email' => 'taylor@laravel.com' ]); + ModelSerializationTestUser::create(['email' => 'mohamed@laravel.com']); + ModelSerializationTestUser::create(['email' => 'taylor@laravel.com']); $serialized = serialize(new CollectionSerializationTestClass( ModelSerializationTestUser::orderByDesc('email')->get()