From fd4b2fd2fbc9bf879e81e8fccec9822d30e4677c Mon Sep 17 00:00:00 2001 From: Daniel Badura Date: Tue, 6 Oct 2026 21:20:55 +0200 Subject: [PATCH] Fix cache keys of the metadata factories The PSR-6 and PSR-16 metadata factories used the class name as cache key. Class names contain a backslash, which is a reserved character in PSR-6 and PSR-16, so the factories failed with any compliant cache like symfony/cache. The class name is now hashed and prefixed, and a cache hit is only accepted if the cached metadata really belongs to the requested class. Unserialized metadata also missed some values, so metadata loaded from a cache failed as soon as they were accessed: the class name of the class metadata, and the property name, type and personal data fallback callable of the property metadata. --- src/Metadata/CacheKey.php | 22 ++++++++ src/Metadata/ClassMetadata.php | 1 + src/Metadata/PropertyMetadata.php | 9 ++- src/Metadata/Psr16MetadataFactory.php | 7 +-- src/Metadata/Psr6MetadataFactory.php | 7 ++- tests/Unit/Metadata/CacheKeyTest.php | 22 ++++++++ tests/Unit/Metadata/ClassMetadataTest.php | 29 ++++++++++ tests/Unit/Metadata/PropertyMetadataTest.php | 45 +++++++++++++++ .../Metadata/Psr16MetadataFactoryTest.php | 49 ++++++++++++++++- .../Unit/Metadata/Psr6MetadataFactoryTest.php | 55 ++++++++++++++++++- 10 files changed, 234 insertions(+), 12 deletions(-) create mode 100644 src/Metadata/CacheKey.php create mode 100644 tests/Unit/Metadata/CacheKeyTest.php create mode 100644 tests/Unit/Metadata/ClassMetadataTest.php create mode 100644 tests/Unit/Metadata/PropertyMetadataTest.php diff --git a/src/Metadata/CacheKey.php b/src/Metadata/CacheKey.php new file mode 100644 index 00000000..95f07492 --- /dev/null +++ b/src/Metadata/CacheKey.php @@ -0,0 +1,22 @@ +reflection = new ReflectionClass($data['className']); + $this->className = $data['className']; $this->properties = $data['properties']; $this->dataSubjectIdField = $data['dataSubjectIdField']; $this->postHydrateCallbacks = $data['postHydrateCallbacks']; diff --git a/src/Metadata/PropertyMetadata.php b/src/Metadata/PropertyMetadata.php index 5577271e..cca80931 100644 --- a/src/Metadata/PropertyMetadata.php +++ b/src/Metadata/PropertyMetadata.php @@ -20,7 +20,9 @@ * normalizer: Normalizer|null, * isPersonalData: bool, * personalDataFallback: mixed, - * extras: array + * personalDataFallbackCallable: (callable(string, mixed):mixed)|null, + * extras: array, + * type: Type|null * } */ final class PropertyMetadata @@ -115,7 +117,9 @@ public function __serialize(): array 'normalizer' => $this->normalizer, 'isPersonalData' => $this->isPersonalData, 'personalDataFallback' => $this->personalDataFallback, + 'personalDataFallbackCallable' => $this->personalDataFallbackCallable, 'extras' => $this->extras, + 'type' => $this->type, ]; } @@ -123,10 +127,13 @@ public function __serialize(): array public function __unserialize(array $data): void { $this->reflection = new ReflectionProperty($data['className'], $data['property']); + $this->propertyName = $data['property']; $this->fieldName = $data['fieldName']; $this->normalizer = $data['normalizer']; $this->isPersonalData = $data['isPersonalData']; $this->personalDataFallback = $data['personalDataFallback']; + $this->personalDataFallbackCallable = $data['personalDataFallbackCallable']; $this->extras = $data['extras']; + $this->type = $data['type']; } } diff --git a/src/Metadata/Psr16MetadataFactory.php b/src/Metadata/Psr16MetadataFactory.php index 603e674f..f8ec5901 100644 --- a/src/Metadata/Psr16MetadataFactory.php +++ b/src/Metadata/Psr16MetadataFactory.php @@ -23,16 +23,15 @@ public function __construct( */ public function metadata(string $class): ClassMetadata { - /** @var ?ClassMetadata $metadata */ - $metadata = $this->cache->get($class); + $metadata = $this->cache->get(CacheKey::forClass($class)); - if ($metadata !== null) { + if ($metadata instanceof ClassMetadata && $metadata->className === $class) { return $metadata; } $metadata = $this->metadataFactory->metadata($class); - $this->cache->set($class, $metadata); + $this->cache->set(CacheKey::forClass($class), $metadata); return $metadata; } diff --git a/src/Metadata/Psr6MetadataFactory.php b/src/Metadata/Psr6MetadataFactory.php index f2cf0b5a..907eca1e 100644 --- a/src/Metadata/Psr6MetadataFactory.php +++ b/src/Metadata/Psr6MetadataFactory.php @@ -23,13 +23,14 @@ public function __construct( */ public function metadata(string $class): ClassMetadata { - $item = $this->cache->getItem($class); + $item = $this->cache->getItem(CacheKey::forClass($class)); if ($item->isHit()) { - /** @var ClassMetadata $data */ $data = $item->get(); - return $data; + if ($data instanceof ClassMetadata && $data->className === $class) { + return $data; + } } $metadata = $this->metadataFactory->metadata($class); diff --git a/tests/Unit/Metadata/CacheKeyTest.php b/tests/Unit/Metadata/CacheKeyTest.php new file mode 100644 index 00000000..42d6476b --- /dev/null +++ b/tests/Unit/Metadata/CacheKeyTest.php @@ -0,0 +1,22 @@ +metadata(ProfileCreated::class); + + $unserialized = unserialize(serialize($classMetadata)); + + self::assertInstanceOf(ClassMetadata::class, $unserialized); + self::assertSame(ProfileCreated::class, $unserialized->className); + self::assertEquals($classMetadata, $unserialized); + } +} diff --git a/tests/Unit/Metadata/PropertyMetadataTest.php b/tests/Unit/Metadata/PropertyMetadataTest.php new file mode 100644 index 00000000..b95d458e --- /dev/null +++ b/tests/Unit/Metadata/PropertyMetadataTest.php @@ -0,0 +1,45 @@ + 'bar'], + Type::object(Email::class), + ); + + $unserialized = unserialize(serialize($propertyMetadata)); + + self::assertInstanceOf(PropertyMetadata::class, $unserialized); + self::assertSame('email', $unserialized->propertyName()); + self::assertEquals($propertyMetadata, $unserialized); + $callback = $unserialized->personalDataFallbackCallback(); + + self::assertNotNull($callback); + self::assertEquals(new Email('foo@example.com'), $callback('foo', null)); + } +} diff --git a/tests/Unit/Metadata/Psr16MetadataFactoryTest.php b/tests/Unit/Metadata/Psr16MetadataFactoryTest.php index 6f356bc8..4fd5740a 100644 --- a/tests/Unit/Metadata/Psr16MetadataFactoryTest.php +++ b/tests/Unit/Metadata/Psr16MetadataFactoryTest.php @@ -4,14 +4,19 @@ namespace Patchlevel\Hydrator\Tests\Unit\Metadata; +use Patchlevel\Hydrator\Metadata\AttributeMetadataFactory; +use Patchlevel\Hydrator\Metadata\CacheKey; use Patchlevel\Hydrator\Metadata\ClassMetadata; use Patchlevel\Hydrator\Metadata\MetadataFactory; use Patchlevel\Hydrator\Metadata\Psr16MetadataFactory; +use Patchlevel\Hydrator\Tests\Unit\Fixture\ProfileCreated; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Psr\SimpleCache\CacheInterface; use ReflectionClass; use stdClass; +use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Cache\Psr16Cache; #[CoversClass(Psr16MetadataFactory::class)] final class Psr16MetadataFactoryTest extends TestCase @@ -23,7 +28,7 @@ public function testMetadataWithHit(): void $cache = $this->createMock(CacheInterface::class); $cache->expects(self::once()) ->method('get') - ->with(stdClass::class) + ->with(CacheKey::forClass(stdClass::class)) ->willReturn($classMetadata); $innerFactory = $this->createMock(MetadataFactory::class); @@ -43,11 +48,37 @@ public function testMetadataWithMiss(): void $cache = $this->createMock(CacheInterface::class); $cache->expects(self::once()) ->method('get') - ->with(stdClass::class) + ->with(CacheKey::forClass(stdClass::class)) ->willReturn(null); $cache->expects(self::once()) ->method('set') - ->with(stdClass::class, $classMetadata); + ->with(CacheKey::forClass(stdClass::class), $classMetadata); + + $innerFactory = $this->createMock(MetadataFactory::class); + $innerFactory->expects(self::once()) + ->method('metadata') + ->with(stdClass::class) + ->willReturn($classMetadata); + + $factory = new Psr16MetadataFactory($innerFactory, $cache); + $result = $factory->metadata(stdClass::class); + + self::assertSame($classMetadata, $result); + } + + public function testMetadataIgnoresEntryOfOtherClass(): void + { + $otherMetadata = new ClassMetadata(new ReflectionClass(ProfileCreated::class)); + $classMetadata = new ClassMetadata(new ReflectionClass(stdClass::class)); + + $cache = $this->createMock(CacheInterface::class); + $cache->expects(self::once()) + ->method('get') + ->with(CacheKey::forClass(stdClass::class)) + ->willReturn($otherMetadata); + $cache->expects(self::once()) + ->method('set') + ->with(CacheKey::forClass(stdClass::class), $classMetadata); $innerFactory = $this->createMock(MetadataFactory::class); $innerFactory->expects(self::once()) @@ -60,4 +91,16 @@ public function testMetadataWithMiss(): void self::assertSame($classMetadata, $result); } + + public function testMetadataWithSymfonyCache(): void + { + $cache = new Psr16Cache(new ArrayAdapter()); + $factory = new Psr16MetadataFactory(new AttributeMetadataFactory(), $cache); + + $metadata = $factory->metadata(ProfileCreated::class); + + self::assertSame(ProfileCreated::class, $metadata->className); + self::assertTrue($cache->has(CacheKey::forClass(ProfileCreated::class))); + self::assertEquals($metadata, $factory->metadata(ProfileCreated::class)); + } } diff --git a/tests/Unit/Metadata/Psr6MetadataFactoryTest.php b/tests/Unit/Metadata/Psr6MetadataFactoryTest.php index 0d6d5f24..e72762fa 100644 --- a/tests/Unit/Metadata/Psr6MetadataFactoryTest.php +++ b/tests/Unit/Metadata/Psr6MetadataFactoryTest.php @@ -4,15 +4,19 @@ namespace Patchlevel\Hydrator\Tests\Unit\Metadata; +use Patchlevel\Hydrator\Metadata\AttributeMetadataFactory; +use Patchlevel\Hydrator\Metadata\CacheKey; use Patchlevel\Hydrator\Metadata\ClassMetadata; use Patchlevel\Hydrator\Metadata\MetadataFactory; use Patchlevel\Hydrator\Metadata\Psr6MetadataFactory; +use Patchlevel\Hydrator\Tests\Unit\Fixture\ProfileCreated; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; use ReflectionClass; use stdClass; +use Symfony\Component\Cache\Adapter\ArrayAdapter; #[CoversClass(Psr6MetadataFactory::class)] final class Psr6MetadataFactoryTest extends TestCase @@ -32,7 +36,7 @@ public function testMetadataWithHit(): void $cache = $this->createMock(CacheItemPoolInterface::class); $cache->expects(self::once()) ->method('getItem') - ->with(stdClass::class) + ->with(CacheKey::forClass(stdClass::class)) ->willReturn($item); $innerFactory = $this->createMock(MetadataFactory::class); @@ -60,7 +64,44 @@ public function testMetadataWithMiss(): void $cache = $this->createMock(CacheItemPoolInterface::class); $cache->expects(self::once()) ->method('getItem') + ->with(CacheKey::forClass(stdClass::class)) + ->willReturn($item); + $cache->expects(self::once()) + ->method('save') + ->with($item); + + $innerFactory = $this->createMock(MetadataFactory::class); + $innerFactory->expects(self::once()) + ->method('metadata') ->with(stdClass::class) + ->willReturn($classMetadata); + + $factory = new Psr6MetadataFactory($innerFactory, $cache); + $result = $factory->metadata(stdClass::class); + + self::assertSame($classMetadata, $result); + } + + public function testMetadataIgnoresEntryOfOtherClass(): void + { + $otherMetadata = new ClassMetadata(new ReflectionClass(ProfileCreated::class)); + $classMetadata = new ClassMetadata(new ReflectionClass(stdClass::class)); + + $item = $this->createMock(CacheItemInterface::class); + $item->expects(self::once()) + ->method('isHit') + ->willReturn(true); + $item->expects(self::once()) + ->method('get') + ->willReturn($otherMetadata); + $item->expects(self::once()) + ->method('set') + ->with($classMetadata); + + $cache = $this->createMock(CacheItemPoolInterface::class); + $cache->expects(self::once()) + ->method('getItem') + ->with(CacheKey::forClass(stdClass::class)) ->willReturn($item); $cache->expects(self::once()) ->method('save') @@ -77,4 +118,16 @@ public function testMetadataWithMiss(): void self::assertSame($classMetadata, $result); } + + public function testMetadataWithSymfonyCache(): void + { + $cache = new ArrayAdapter(); + $factory = new Psr6MetadataFactory(new AttributeMetadataFactory(), $cache); + + $metadata = $factory->metadata(ProfileCreated::class); + + self::assertSame(ProfileCreated::class, $metadata->className); + self::assertTrue($cache->hasItem(CacheKey::forClass(ProfileCreated::class))); + self::assertEquals($metadata, $factory->metadata(ProfileCreated::class)); + } }