From 9642aa525ff0c247810d6db10aee75b866e94008 Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 13 Mar 2026 08:37:54 +0100 Subject: [PATCH 1/3] fix cipher key store & change api --- .../Cryptography/BaseCryptographer.php | 2 +- .../Cryptography/Store/CipherKeyStore.php | 4 +- .../Store/InMemoryCipherKeyStore.php | 42 ++-- .../Cryptography/BaseCryptographerTest.php | 2 +- .../Store/InMemoryCipherKeyStoreTest.php | 203 ++++++++++++++++-- 5 files changed, 203 insertions(+), 50 deletions(-) diff --git a/src/Extension/Cryptography/BaseCryptographer.php b/src/Extension/Cryptography/BaseCryptographer.php index 51dd0b5..2d19a64 100644 --- a/src/Extension/Cryptography/BaseCryptographer.php +++ b/src/Extension/Cryptography/BaseCryptographer.php @@ -54,7 +54,7 @@ public function encrypt(string $subjectId, mixed $value): array $cipherKey = $this->cipherKeyStore->currentKeyFor($subjectId); } catch (CipherKeyNotExists) { $cipherKey = ($this->cipherKeyFactory)($subjectId); - $this->cipherKeyStore->store($cipherKey->id, $cipherKey); + $this->cipherKeyStore->store($cipherKey); } $parameter = $this->cipher->encrypt($cipherKey, $value); diff --git a/src/Extension/Cryptography/Store/CipherKeyStore.php b/src/Extension/Cryptography/Store/CipherKeyStore.php index 82541c9..c0eefdf 100644 --- a/src/Extension/Cryptography/Store/CipherKeyStore.php +++ b/src/Extension/Cryptography/Store/CipherKeyStore.php @@ -13,9 +13,9 @@ interface CipherKeyStore public function currentKeyFor(string $subjectId): CipherKey; /** @throws CipherKeyNotExists */ - public function get(string $keyId): CipherKey; + public function get(string $id): CipherKey; - public function store(string $id, CipherKey $key): void; + public function store(CipherKey $key): void; public function remove(string $id): void; diff --git a/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php b/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php index 20a0322..70c8f66 100644 --- a/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php +++ b/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php @@ -14,7 +14,7 @@ final class InMemoryCipherKeyStore implements CipherKeyStore /** @var array */ private array $indexById = []; - /** @var array> */ + /** @var array> */ private array $indexBySubjectId = []; public function currentKeyFor(string $subjectId): CipherKey @@ -32,43 +32,31 @@ public function currentKeyFor(string $subjectId): CipherKey return $this->indexBySubjectId[$subjectId][$lastKey]; } - public function get(string $keyId): CipherKey + public function get(string $id): CipherKey { - return $this->indexById[$keyId] ?? throw CipherKeyNotExists::forKeyId($keyId); + return $this->indexById[$id] ?? throw CipherKeyNotExists::forKeyId($id); } - public function store(string $id, CipherKey $key): void + public function store(CipherKey $key): void { - $this->indexById[$id] = $key; + $this->remove($key->id); - if (!isset($this->indexBySubjectId[$key->subjectId])) { - $this->indexBySubjectId[$key->subjectId] = []; - } - - $this->indexBySubjectId[$key->subjectId][] = $key; + $this->indexById[$key->id] = $key; + $this->indexBySubjectId[$key->subjectId][$key->id] = $key; } public function remove(string $id): void { - unset($this->indexById[$id]); - - foreach ($this->indexBySubjectId as $subjectId => $keys) { - $filtered = []; - - foreach ($keys as $key) { - if ($key->id === $id) { - continue; - } + $key = $this->indexById[$id] ?? null; - $filtered[] = $key; - } - - if ($filtered === []) { - unset($this->indexBySubjectId[$subjectId]); - } else { - $this->indexBySubjectId[$subjectId] = $filtered; - } + if (!$key) { + return; } + + unset( + $this->indexBySubjectId[$key->subjectId][$id], + $this->indexById[$id], + ); } public function clear(): void diff --git a/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php b/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php index 296026a..2ee8ef5 100644 --- a/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php +++ b/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php @@ -70,7 +70,7 @@ public function testEncryptWithoutKey(): void $cipherKeyStore ->expects($this->once()) ->method('store') - ->with('key-456', $cipherKey); + ->with($cipherKey); $cipher = $this->createMock(Cipher::class); $cipher->expects($this->once())->method('encrypt')->with($cipherKey, 'info@patchlevel.de') diff --git a/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php b/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php index 99dcb44..0f19d62 100644 --- a/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php +++ b/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php @@ -17,17 +17,17 @@ final class InMemoryCipherKeyStoreTest extends TestCase public function testStoreAndLoad(): void { $key = new CipherKey( - 'foo', - 'bar', - 'baz', + 'key-1', + 'subject-1', + 'secret', 'aes-256-gcm', new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); - $store->store('foo', $key); + $store->store($key); - self::assertSame($key, $store->get('foo')); + self::assertSame($key, $store->get('key-1')); } public function testLoadFailed(): void @@ -35,50 +35,215 @@ public function testLoadFailed(): void $this->expectException(CipherKeyNotExists::class); $store = new InMemoryCipherKeyStore(); - $store->get('foo'); + $store->get('non-existent'); } public function testRemove(): void { $key = new CipherKey( - 'foo', - 'bar', - 'baz', + 'key-1', + 'subject-1', + 'secret', 'aes-256-gcm', new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); - $store->store('foo', $key); + $store->store($key); - self::assertSame($key, $store->get('foo')); + self::assertSame($key, $store->get('key-1')); - $store->remove('foo'); + $store->remove('key-1'); $this->expectException(CipherKeyNotExists::class); - $store->get('foo'); + $store->get('key-1'); } public function testClear(): void { $key = new CipherKey( - 'foo', - 'bar', - 'baz', + 'key-1', + 'subject-1', + 'secret', 'aes-256-gcm', new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); - $store->store('foo', $key); + $store->store($key); - self::assertSame($key, $store->get('foo')); + self::assertSame($key, $store->get('key-1')); $store->clear(); $this->expectException(CipherKeyNotExists::class); - $store->get('foo'); + $store->get('key-1'); + } + + public function testCurrentKeyFor(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-1', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + self::assertSame($key2, $store->currentKeyFor('subject-1')); + } + + public function testCurrentKeyForNotExists(): void + { + $this->expectException(CipherKeyNotExists::class); + + $store = new InMemoryCipherKeyStore(); + $store->currentKeyFor('non-existent'); + } + + public function testRemoveWithSubjectId(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-1', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key3 = new CipherKey( + 'key-3', + 'subject-2', + 'secret-3', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + $store->store($key3); + + $store->removeWithSubjectId('subject-1'); + + $this->expectException(CipherKeyNotExists::class); + $store->get('key-1'); + } + + public function testRemoveWithSubjectIdDoesNotAffectOtherSubjects(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-2', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + $store->removeWithSubjectId('subject-1'); + + self::assertSame($key2, $store->get('key-2')); + } + + public function testRemoveWithSubjectIdNonExistent(): void + { + $store = new InMemoryCipherKeyStore(); + $store->removeWithSubjectId('non-existent'); + + $this->expectNotToPerformAssertions(); + } + + public function testRemoveNonExistent(): void + { + $store = new InMemoryCipherKeyStore(); + $store->remove('non-existent'); + + $this->expectNotToPerformAssertions(); + } + + public function testStoreOverwritesExistingKey(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + self::assertSame($key2, $store->get('key-1')); + self::assertSame($key2, $store->currentKeyFor('subject-1')); + } + + public function testMultipleSubjects(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-2', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + self::assertSame($key1, $store->currentKeyFor('subject-1')); + self::assertSame($key2, $store->currentKeyFor('subject-2')); } } From 0eb767063770d46b0760e8b2a7241a33a8e1987e Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 13 Mar 2026 17:43:16 +0100 Subject: [PATCH 2/3] add type in property metadata --- phpstan-baseline.neon | 8 +++---- src/Metadata/AttributeMetadataFactory.php | 11 +++++---- src/Metadata/PropertyMetadata.php | 2 ++ .../Metadata/AttributeMetadataFactoryTest.php | 23 +++++++++++++++++++ 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 73ed976..0aa5792 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -175,13 +175,13 @@ parameters: path: tests/Unit/Fixture/DtoWithHooks.php - - message: '#^Method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:454\:\:postHydrate\(\) is unused\.$#' + message: '#^Method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:477\:\:postHydrate\(\) is unused\.$#' identifier: method.unused count: 1 path: tests/Unit/Metadata/AttributeMetadataFactoryTest.php - - message: '#^Method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:454\:\:preExtract\(\) is unused\.$#' + message: '#^Method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:477\:\:preExtract\(\) is unused\.$#' identifier: method.unused count: 1 path: tests/Unit/Metadata/AttributeMetadataFactoryTest.php @@ -193,13 +193,13 @@ parameters: path: tests/Unit/Metadata/AttributeMetadataFactoryTest.php - - message: '#^Static method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:482\:\:postHydrate\(\) is unused\.$#' + message: '#^Static method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:505\:\:postHydrate\(\) is unused\.$#' identifier: method.unused count: 1 path: tests/Unit/Metadata/AttributeMetadataFactoryTest.php - - message: '#^Static method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:482\:\:preExtract\(\) is unused\.$#' + message: '#^Static method class@anonymous/tests/Unit/Metadata/AttributeMetadataFactoryTest\.php\:505\:\:preExtract\(\) is unused\.$#' identifier: method.unused count: 1 path: tests/Unit/Metadata/AttributeMetadataFactoryTest.php diff --git a/src/Metadata/AttributeMetadataFactory.php b/src/Metadata/AttributeMetadataFactory.php index f53d6af..37fbe9c 100644 --- a/src/Metadata/AttributeMetadataFactory.php +++ b/src/Metadata/AttributeMetadataFactory.php @@ -155,11 +155,14 @@ private function getPropertyMetadataList(ReflectionClass $reflectionClass): arra ); } + $type = $this->typeResolver->resolve($reflectionProperty); + $properties[$fieldName] = new PropertyMetadata( $reflectionProperty, $fieldName, - $this->getNormalizer($reflectionProperty), + $this->getNormalizer($reflectionProperty, $type), ...$this->getPersonalData($reflectionProperty), + type: $type, ); } @@ -352,18 +355,16 @@ private function validate(ClassMetadata $metadata): void } } - private function getNormalizer(ReflectionProperty $reflectionProperty): Normalizer|null + private function getNormalizer(ReflectionProperty $reflectionProperty, Type $type): Normalizer|null { $normalizer = $this->findNormalizerOnProperty($reflectionProperty); - $type = null; if (!$normalizer) { - $type = $this->typeResolver->resolve($reflectionProperty); $normalizer = $this->inferNormalizerByType($type); } if ($normalizer instanceof TypeAwareNormalizer) { - $normalizer->handleType($type ?? $this->typeResolver->resolve($reflectionProperty)); + $normalizer->handleType($this->typeResolver->resolve($reflectionProperty)); } if ($normalizer instanceof ReflectionTypeAwareNormalizer) { diff --git a/src/Metadata/PropertyMetadata.php b/src/Metadata/PropertyMetadata.php index 4736ff1..5577271 100644 --- a/src/Metadata/PropertyMetadata.php +++ b/src/Metadata/PropertyMetadata.php @@ -8,6 +8,7 @@ use InvalidArgumentException; use Patchlevel\Hydrator\Normalizer\Normalizer; use ReflectionProperty; +use Symfony\Component\TypeInfo\Type; use function str_starts_with; @@ -40,6 +41,7 @@ public function __construct( public readonly mixed $personalDataFallback = null, public readonly mixed $personalDataFallbackCallable = null, public array $extras = [], + public readonly Type|null $type = null, ) { $this->propertyName = $reflection->getName(); diff --git a/tests/Unit/Metadata/AttributeMetadataFactoryTest.php b/tests/Unit/Metadata/AttributeMetadataFactoryTest.php index 9ba032d..149a6b6 100644 --- a/tests/Unit/Metadata/AttributeMetadataFactoryTest.php +++ b/tests/Unit/Metadata/AttributeMetadataFactoryTest.php @@ -35,6 +35,7 @@ use Patchlevel\Hydrator\Tests\Unit\Fixture\Status; use Patchlevel\Hydrator\Tests\Unit\Fixture\Wrapper; use PHPUnit\Framework\TestCase; +use Symfony\Component\TypeInfo\Type; final class AttributeMetadataFactoryTest extends TestCase { @@ -91,6 +92,7 @@ public function testWithProperties(): void self::assertSame('name', $propertyMetadata->propertyName()); self::assertSame('name', $propertyMetadata->fieldName()); + self::assertEquals(Type::nullable(Type::string()), $propertyMetadata->type); self::assertNull($propertyMetadata->normalizer()); } @@ -136,6 +138,7 @@ public function __construct( self::assertSame('name', $propertyMetadata->propertyName()); self::assertSame('name', $propertyMetadata->fieldName()); + self::assertEquals(Type::string(), $propertyMetadata->type); self::assertNull($propertyMetadata->normalizer()); } @@ -184,6 +187,7 @@ public function __construct( self::assertSame('email', $propertyMetadata->propertyName()); self::assertSame('email', $propertyMetadata->fieldName()); + self::assertEquals(Type::object(Email::class), $propertyMetadata->type); self::assertInstanceOf(EmailNormalizer::class, $propertyMetadata->normalizer()); } @@ -208,6 +212,7 @@ public function __construct( self::assertSame('status', $propertyMetadata->propertyName()); self::assertSame('status', $propertyMetadata->fieldName()); + self::assertEquals(Type::enum(Status::class), $propertyMetadata->type); $normalizer = $propertyMetadata->normalizer(); @@ -245,6 +250,10 @@ public function testInferNormalizerWithGeneric(): void $propertyMetadata = $metadata->propertyForField('email'); self::assertEquals(new ObjectNormalizer(Wrapper::class), $propertyMetadata->normalizer()); + self::assertEquals( + Type::generic(Type::object(Wrapper::class), Type::object(Email::class)), + $propertyMetadata->type, + ); } public function testInferNormalizerWithTemplate(): void @@ -259,9 +268,23 @@ public function testInferNormalizerWithTemplate(): void $propertyMetadata = $metadata->propertyForField('object'); self::assertEquals(new ObjectNormalizer(Wrapper::class), $propertyMetadata->normalizer()); + self::assertEquals( + Type::generic(Type::object(Wrapper::class), Type::object(Email::class)), + $propertyMetadata->type, + ); $propertyMetadata = $metadata->propertyForField('scalar'); self::assertEquals(new ObjectNormalizer(Wrapper::class), $propertyMetadata->normalizer()); + + self::assertEquals( + Type::nullable( + Type::generic( + Type::object(Wrapper::class), + Type::string(), + ), + ), + $propertyMetadata->type, + ); } public function testExtends(): void From fb802134da1eb6294a4358b1f55dbe3621104d80 Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 13 Mar 2026 18:09:15 +0100 Subject: [PATCH 3/3] use previously defined type --- src/Metadata/AttributeMetadataFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Metadata/AttributeMetadataFactory.php b/src/Metadata/AttributeMetadataFactory.php index 37fbe9c..7a13d50 100644 --- a/src/Metadata/AttributeMetadataFactory.php +++ b/src/Metadata/AttributeMetadataFactory.php @@ -364,7 +364,7 @@ private function getNormalizer(ReflectionProperty $reflectionProperty, Type $typ } if ($normalizer instanceof TypeAwareNormalizer) { - $normalizer->handleType($this->typeResolver->resolve($reflectionProperty)); + $normalizer->handleType($type); } if ($normalizer instanceof ReflectionTypeAwareNormalizer) {