Merge pull request #71 from plank/fix-serialization

Fix serialization breaking indexed meta cache
plank · Dec 26, 2021 · f0dba1e · f0dba1e
2 parents 26633aa + bb9362b
commit f0dba1e
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 40 deletions.
diff --git a/.github/workflows/automated-test.yml b/.github/workflows/automated-test.yml
@@ -5,7 +5,7 @@ jobs:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
-        php-versions: ['7.4', '8.0']
+        php-versions: ['7.4', '8.0', '8.1']
         prefer-lowest: ['','--prefer-lowest']
     name: PHP ${{ matrix.php-versions }} ${{ matrix.prefer-lowest }}
     steps:

diff --git a/composer.json b/composer.json
@@ -17,13 +17,15 @@
     "require": {
         "php": ">=7.3.0",
         "ext-json": "*",
-        "illuminate/support": "^6.0|^7.0|^8.0",
-        "illuminate/database": "^6.20.12|^7.30.3|^8.22.1"
+        "illuminate/support": "^6.20.42|^8.22.1",
+        "illuminate/database": "^6.20.42|^8.22.1",
+        "phpoption/phpoption": "1.8.1"
     },
     "require-dev": {
+        "symfony/symfony": "^5.4.1|^6.1",
         "laravel/legacy-factories": "^1.0.4",
-        "orchestra/testbench": "^5.9|^6.6",
-        "phpunit/phpunit": "^9.5",
+        "orchestra/testbench": "^5.20|^6.23",
+        "phpunit/phpunit": "^9.5.11",
         "guzzlehttp/guzzle": "^7.2",
         "guzzlehttp/promises": "^1.4",
         "mockery/mockery": "^1.4.2",

diff --git a/src/Metable.php b/src/Metable.php
@@ -503,10 +503,16 @@ private function joinMetaTable(Builder $q, string $key, $type = 'left'): string
      */
     private function getMetaCollection()
     {
+        // load meta relation if not loaded.
         if (!$this->relationLoaded('meta')) {
             $this->setRelation('meta', $this->meta()->get());
         }
 
+        // reindex by key for quicker lookups if necessary.
+        if ($this->indexedMetaCollection === null) {
+            $this->indexedMetaCollection = $this->meta->keyBy('key');
+        }
+
         return $this->indexedMetaCollection;
     }
 
@@ -515,12 +521,7 @@ private function getMetaCollection()
      */
     public function setRelation($relation, $value)
     {
-        if ($relation == 'meta') {
-            // keep the meta relation indexed by key.
-            /** @var Collection $value */
-            $this->indexedMetaCollection = $value->keyBy('key');
-        }
-
+        $this->indexedMetaCollection = null;
         return parent::setRelation($relation, $value);
     }
 
@@ -532,11 +533,9 @@ public function setRelation($relation, $value)
      */
     public function setRelations(array $relations)
     {
-        // keep the meta relation indexed by key.
         if (isset($relations['meta'])) {
-            $this->indexedMetaCollection = (new Collection($relations['meta']))->keyBy('key');
-        } else {
-            $this->indexedMetaCollection = $this->makeMeta()->newCollection();
+            // clear the indexed cache
+            $this->indexedMetaCollection = null;
         }
 
         return parent::setRelations($relations);

diff --git a/tests/Integration/MetableTest.php b/tests/Integration/MetableTest.php
@@ -39,10 +39,10 @@ public function test_it_can_set_many_meta_values_at_once()
         $this->assertFalse($metable->hasMeta('baz'));
 
         $metable->setManyMeta([
-            'foo' => 'bar',
-            'bar' => 'baz',
-            'baz' => ['foo', 'bar'],
-        ]);
+                                  'foo' => 'bar',
+                                  'bar' => 'baz',
+                                  'baz' => ['foo', 'bar'],
+                              ]);
 
         $this->assertTrue($metable->hasMeta('foo'));
         $this->assertTrue($metable->hasMeta('bar'));
@@ -96,10 +96,10 @@ public function test_it_can_get_meta_all_values()
         $collection = $metable->getAllMeta();
 
         $this->assertEquals([
-            'foo' => 123,
-            'bar' => 'hello',
-            'baz' => ['a', 'b', 'c'],
-        ], $collection->toArray());
+                                'foo' => 123,
+                                'bar' => 'hello',
+                                'baz' => ['a', 'b', 'c'],
+                            ], $collection->toArray());
     }
 
     public function test_it_updates_existing_meta_records()
@@ -434,21 +434,22 @@ public function test_set_relation_updates_index()
         $meta = $this->makeMeta(['key' => 'foo', 'value' => 'bar']);
         $emptyCollection = new Collection();
         $metaCollection = new Collection([$meta]);
+        $metable->setRelation('meta', $emptyCollection);
 
-        $property = (new ReflectionClass($metable))
-            ->getProperty('indexedMetaCollection');
-        $property->setAccessible(true);
+        $method = (new ReflectionClass($metable))
+            ->getMethod('getMetaCollection');
+        $method->setAccessible(true);
 
-        $this->assertNull($property->getValue($metable));
+        $this->assertEquals($emptyCollection, $method->invoke($metable));
 
         $metable->setRelation('meta', $metaCollection);
-        $this->assertEquals(new Collection(['foo' => $meta]), $property->getValue($metable));
+        $this->assertEquals(new Collection(['foo' => $meta]), $method->invoke($metable));
 
         $metable->setRelation('other', $emptyCollection);
-        $this->assertEquals(new Collection(['foo' => $meta]), $property->getValue($metable));
+        $this->assertEquals(new Collection(['foo' => $meta]), $method->invoke($metable));
 
         $metable->setRelation('meta', $emptyCollection);
-        $this->assertEquals($emptyCollection, $property->getValue($metable));
+        $this->assertEquals($emptyCollection, $method->invoke($metable));
     }
 
     public function test_set_relations_updates_index()
@@ -457,24 +458,34 @@ public function test_set_relations_updates_index()
         $meta = $this->makeMeta(['key' => 'foo', 'value' => 'bar']);
         $emptyCollection = new Collection();
         $metaCollection = new Collection([$meta]);
+        $indexedCollection = new Collection(['foo' => $meta]);
+        $metable->setRelation('meta', $emptyCollection);
 
-        $property = (new ReflectionClass($metable))
-            ->getProperty('indexedMetaCollection');
-        $property->setAccessible(true);
+        $method = (new ReflectionClass($metable))
+            ->getMethod('getMetaCollection');
+        $method->setAccessible(true);
 
-        $this->assertNull($property->getValue($metable));
+        $this->assertEquals($emptyCollection, $method->invoke($metable));
 
         $metable->setRelations(['meta' => $metaCollection]);
-        $this->assertEquals(new Collection(['foo' => $meta]), $property->getValue($metable));
+        $this->assertEquals($indexedCollection, $method->invoke($metable));
 
         $metable->setRelations(['other' => $emptyCollection, 'meta' => $metaCollection]);
-        $this->assertEquals(new Collection(['foo' => $meta]), $property->getValue($metable));
-
-        $metable->setRelations(['other' => $emptyCollection]);
-        $this->assertEquals($emptyCollection, $property->getValue($metable));
+        $this->assertEquals($indexedCollection, $method->invoke($metable));
 
         $metable->setRelations(['meta' => $emptyCollection]);
-        $this->assertEquals($emptyCollection, $property->getValue($metable));
+        $this->assertEquals($emptyCollection, $method->invoke($metable));
+    }
+
+    public function test_it_can_serialize_properly()
+    {
+        $metable = $this->makeMetable();
+        $meta = $this->makeMeta(['key' => 'foo', 'value' => 'baz']);
+        $metaCollection = new Collection([$meta]);
+        $metable->setRelation('meta', $metaCollection);
+        /** @var SampleMetable $result */
+        $result = unserialize(serialize($metable));
+        $this->assertEquals('baz', $result->getMeta('foo'));
     }
 
     private function makeMeta(array $attributes = []): Meta

diff --git a/tests/Mocks/SampleSerializable.php b/tests/Mocks/SampleSerializable.php
@@ -22,4 +22,14 @@ public function unserialize($serialized)
     {
         $this->data = unserialize($serialized);
     }
+
+    public function __serialize(): array
+    {
+        return $this->data;
+    }
+
+    public function __unserialize(array $data)
+    {
+        return $this->data = $data;
+    }
 }