fix(core): properly handle unserializable discovery items when caching discovery (#1503)

Author: brendt

packages/core/src/CouldNotStoreDiscoveryCache.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Tempest\Core;
+
+use Exception;
+use Tempest\Discovery\DiscoveryLocation;
+
+final class CouldNotStoreDiscoveryCache extends Exception
+{
+    public function __construct(DiscoveryLocation $location)
+    {
+        parent::__construct(sprintf(
+            'Could not store discovery cache for %s. This is likely because you\'re trying to store unserializable data like reflection classes or closures in discovery items.',
+            $location->path,
+        ));
+    }
+}

packages/core/src/DiscoveryCache.php

@@ -68,7 +68,11 @@ public function store(DiscoveryLocation $location, array $discoveries): void
             ->getItem($location->key)
             ->set($cachedForLocation);
 
-        $this->pool->save($item);
+        $saved = $this->pool->save($item);
+
+        if (! $saved) {
+            throw new CouldNotStoreDiscoveryCache($location);
+        }
     }
 
     public function clear(): void

packages/discovery/src/DiscoveryItems.php

@@ -31,6 +31,12 @@ public function getForLocation(DiscoveryLocation $location): array
         return $this->items[$location->path] ?? [];
     }
 
+    /**
+     * Add items to discover.
+     * Whatever input you add here should be serializable,
+     * otherwise that input cannot be cached,
+     * and you'll run into errors when discovery cache is enabled
+     */
     public function add(DiscoveryLocation $location, mixed $value): self
     {
         $this->items[$location->path] ??= [];

packages/reflection/src/ClassReflector.php

@@ -178,4 +178,14 @@ private function memoize(string $key, Closure $closure): mixed
 
         return $this->memoize[$key];
     }
+
+    public function __serialize(): array
+    {
+        return ['name' => $this->getName()];
+    }
+
+    public function __unserialize(array $data): void
+    {
+        $this->reflectionClass = new PHPReflectionClass($data['name']);
+    }
 }

packages/reflection/tests/ClassReflectorTest.php

@@ -60,4 +60,14 @@ public function test_recursive_attribute_from_parent(): void
         $this->assertNull($reflector->getAttribute(RecursiveAttribute::class));
         $this->assertNotNull($reflector->getAttribute(RecursiveAttribute::class, recursive: true));
     }
+
+    public function test_serialize(): void
+    {
+        $reflector = new ClassReflector(TestClassA::class);
+
+        $serialized = serialize($reflector);
+        $unserialized = unserialize($serialized);
+
+        $this->assertEquals($reflector, $unserialized);
+    }
 }

tests/Integration/Core/DiscoveryCacheTest.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace Tests\Tempest\Integration\Core;
+
+use Tempest\Core\CouldNotStoreDiscoveryCache;
+use Tempest\Core\DiscoveryCache;
+use Tempest\Discovery\DiscoveryLocation;
+use Tests\Tempest\Integration\Core\Fixtures\TestDiscovery;
+use Tests\Tempest\Integration\FrameworkIntegrationTestCase;
+
+use function Tempest\reflect;
+
+final class DiscoveryCacheTest extends FrameworkIntegrationTestCase
+{
+    public function test_exception_with_unserializable_discovery_items(): void
+    {
+        $this->assertException(CouldNotStoreDiscoveryCache::class, function () {
+            $discoveryCache = $this->container->get(DiscoveryCache::class);
+
+            $location = new DiscoveryLocation('Test\\', '.');
+            $discovery = new TestDiscovery();
+            $discovery->discover($location, reflect($this));
+
+            $discoveryCache->store($location, [
+                $discovery,
+            ]);
+        });
+    }
+}

tests/Integration/Core/Fixtures/TestDiscovery.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace Tests\Tempest\Integration\Core\Fixtures;
+
+use Exception;
+use Serializable;
+use Tempest\Discovery\Discovery;
+use Tempest\Discovery\DiscoveryItems;
+use Tempest\Discovery\DiscoveryLocation;
+use Tempest\Discovery\IsDiscovery;
+use Tempest\Reflection\ClassReflector;
+
+final class TestDiscovery implements Discovery
+{
+    use IsDiscovery;
+
+    public function __construct()
+    {
+        $this->discoveryItems = new DiscoveryItems();
+    }
+
+    public function discover(DiscoveryLocation $location, ClassReflector $class): void
+    {
+        $class = new class {
+            public function __serialize(): array
+            {
+                throw new Exception('Cannot serialize');
+            }
+        };
+
+        $this->discoveryItems->add($location, $class);
+    }
+
+    public function apply(): void
+    {
+        // Nothing
+    }
+}