From 734eec65fd168d7335487b7c408500aefff0c8b5 Mon Sep 17 00:00:00 2001 From: Dev Date: Wed, 25 Feb 2026 22:01:04 +0000 Subject: [PATCH 1/2] Fix bugs and add product improvements Bug fixes: - Remove declaring-class filter in supportsByPropertyAttribute() that silently skipped inherited properties, breaking MappedSuperclass detection - Assert on preg_replace return in sanitize() to prevent silent cache key corruption - Remove dead MetadataSubscriber manual registration from services.php Improvements: - Replace spl_object_id arrays with WeakMap in MetadataReader and AbstractExtensionMetadataFactory to prevent stale metadata in long-running processes (Messenger, RoadRunner) - Add exception hierarchy: DuplicateMappingException and InvalidMappingAttributeException extending MappingException - Add hasConfiguration() and getConfigurations() to ExtensionMetadataInterface - Add driver priority support via getPriority() on MappingDriverInterface and AutowireIterator defaultPriorityMethod - Add getEntityChangeSet() helper to AbstractDoctrineListener - Add ValidatableConfigurationInterface for opt-in post-load validation - Add WakeupAwareConfigurationInterface for opt-in wakeup hooks - Add trigger_error(E_USER_NOTICE) for shadowed field mappings instead of silent array_diff_key - Add MetadataDumpCommand (chamber-orchestra:metadata:dump) for debugging - Add MetadataDataCollector for Symfony profiler integration Co-Authored-By: Claude Opus 4.6 --- src/Command/MetadataDumpCommand.php | 96 +++++++++++++++++++ src/DataCollector/MetadataDataCollector.php | 63 ++++++++++++ .../ChamberOrchestraMetadataExtension.php | 5 + .../AbstractDoctrineListener.php | 8 ++ src/Exception/DuplicateMappingException.php | 16 ++++ .../InvalidMappingAttributeException.php | 16 ++++ src/Exception/MappingException.php | 8 +- src/Mapping/AbstractExtensionMetadata.php | 23 ++++- .../AbstractExtensionMetadataFactory.php | 37 +++++-- src/Mapping/Driver/AbstractMappingDriver.php | 8 +- src/Mapping/Driver/MappingDriverInterface.php | 5 + src/Mapping/ExtensionMetadataInterface.php | 9 +- src/Mapping/MetadataReader.php | 31 +++--- src/Mapping/ORM/ExtensionMetadataFactory.php | 14 ++- .../ORM/ValidatableConfigurationInterface.php | 19 ++++ .../ORM/WakeupAwareConfigurationInterface.php | 17 ++++ src/Resources/config/services.php | 3 - tests/Fixtures/Mapping/DummyMappingDriver.php | 5 + .../Mapping/TrackingMappingDriver.php | 5 + .../Unit/Command/MetadataDumpCommandTest.php | 83 ++++++++++++++++ .../MetadataDataCollectorTest.php | 66 +++++++++++++ .../AbstractDoctrineListenerTest.php | 31 +++++- .../DuplicateMappingExceptionTest.php | 33 +++++++ .../InvalidMappingAttributeExceptionTest.php | 33 +++++++ tests/Unit/Exception/MappingExceptionTest.php | 4 + .../Mapping/AbstractExtensionMetadataTest.php | 3 + .../ORM/ExtensionMetadataFactoryTest.php | 36 +++++++ 27 files changed, 638 insertions(+), 39 deletions(-) create mode 100644 src/Command/MetadataDumpCommand.php create mode 100644 src/DataCollector/MetadataDataCollector.php create mode 100644 src/Exception/DuplicateMappingException.php create mode 100644 src/Exception/InvalidMappingAttributeException.php create mode 100644 src/Mapping/ORM/ValidatableConfigurationInterface.php create mode 100644 src/Mapping/ORM/WakeupAwareConfigurationInterface.php create mode 100644 tests/Unit/Command/MetadataDumpCommandTest.php create mode 100644 tests/Unit/DataCollector/MetadataDataCollectorTest.php create mode 100644 tests/Unit/Exception/DuplicateMappingExceptionTest.php create mode 100644 tests/Unit/Exception/InvalidMappingAttributeExceptionTest.php diff --git a/src/Command/MetadataDumpCommand.php b/src/Command/MetadataDumpCommand.php new file mode 100644 index 0000000..cd22bd6 --- /dev/null +++ b/src/Command/MetadataDumpCommand.php @@ -0,0 +1,96 @@ +addArgument('entity-class', InputArgument::REQUIRED, 'The fully-qualified entity class name'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + + /** @var string $entityClass */ + $entityClass = $input->getArgument('entity-class'); + + if (!\class_exists($entityClass)) { + $io->error(\sprintf('Class "%s" does not exist.', $entityClass)); + + return Command::FAILURE; + } + + $metadata = $this->metadataReader->getExtensionMetadata($this->entityManager, $entityClass); + + $io->title(\sprintf('Extension Metadata for %s', $entityClass)); + + $configurations = $metadata->getConfigurations(); + + if ([] === $configurations) { + $io->info('No metadata configurations found for this entity.'); + + return Command::SUCCESS; + } + + foreach ($configurations as $configClass => $configuration) { + $io->section($configClass); + + $mappings = $configuration->getMappings(); + if ([] === $mappings) { + $io->text('No field mappings.'); + continue; + } + + $rows = []; + foreach ($mappings as $fieldName => $mapping) { + $rows[] = [$fieldName, \json_encode($mapping, \JSON_THROW_ON_ERROR | \JSON_PRETTY_PRINT)]; + } + + $io->table(['Field', 'Mapping'], $rows); + } + + $embeddedMetadata = $metadata->getEmbeddedMetadata(); + if ([] !== $embeddedMetadata) { + $io->section('Embedded Metadata'); + foreach ($embeddedMetadata as $fieldName => $embeddedMeta) { + $io->text(\sprintf(' %s -> %s', $fieldName, $embeddedMeta->getName())); + foreach ($embeddedMeta->getConfigurations() as $configClass => $configuration) { + $io->text(\sprintf(' Configuration: %s (%d fields)', $configClass, \count($configuration->getFieldNames()))); + } + } + } + + return Command::SUCCESS; + } +} diff --git a/src/DataCollector/MetadataDataCollector.php b/src/DataCollector/MetadataDataCollector.php new file mode 100644 index 0000000..ce671cc --- /dev/null +++ b/src/DataCollector/MetadataDataCollector.php @@ -0,0 +1,63 @@ + + */ + private array $invocations = []; + + public function recordDriverInvocation(string $driverClass, string $entityClass, bool $supported, bool $loaded): void + { + $this->invocations[] = [ + 'driver' => $driverClass, + 'entity' => $entityClass, + 'supported' => $supported, + 'loaded' => $loaded, + ]; + } + + public function collect(Request $request, Response $response, ?\Throwable $exception = null): void + { + $this->data = [ + 'invocations' => $this->invocations, + ]; + } + + /** + * @return list + */ + public function getDriverStats(): array + { + /** @var list $stats */ + $stats = $this->data['invocations'] ?? []; + + return $stats; + } + + public function getName(): string + { + return 'chamber_orchestra_metadata'; + } + + public function reset(): void + { + $this->data = []; + $this->invocations = []; + } +} diff --git a/src/DependencyInjection/ChamberOrchestraMetadataExtension.php b/src/DependencyInjection/ChamberOrchestraMetadataExtension.php index 7c6ad7c..8898178 100644 --- a/src/DependencyInjection/ChamberOrchestraMetadataExtension.php +++ b/src/DependencyInjection/ChamberOrchestraMetadataExtension.php @@ -11,6 +11,7 @@ namespace ChamberOrchestra\MetadataBundle\DependencyInjection; +use ChamberOrchestra\MetadataBundle\DataCollector\MetadataDataCollector; use ChamberOrchestra\MetadataBundle\Mapping\Driver\MappingDriverInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -26,5 +27,9 @@ public function load(array $configs, ContainerBuilder $container): void $container->registerForAutoconfiguration(MappingDriverInterface::class) ->addTag('chamber_orchestra_metadata.mapping.driver'); + + if ($container->hasParameter('kernel.debug') && !$container->getParameter('kernel.debug')) { + $container->removeDefinition(MetadataDataCollector::class); + } } } diff --git a/src/EventSubscriber/AbstractDoctrineListener.php b/src/EventSubscriber/AbstractDoctrineListener.php index a3e11aa..d983199 100644 --- a/src/EventSubscriber/AbstractDoctrineListener.php +++ b/src/EventSubscriber/AbstractDoctrineListener.php @@ -43,6 +43,14 @@ protected function getScheduledEntityDeletions(EntityManagerInterface $em, strin return $this->collectArgs($em->getUnitOfWork()->getScheduledEntityDeletions(), $em, $class); } + /** + * @return array + */ + protected function getEntityChangeSet(EntityManagerInterface $em, MetadataArgs $args): array + { + return $em->getUnitOfWork()->getEntityChangeSet($args->entity); + } + /** * @param array $entities * diff --git a/src/Exception/DuplicateMappingException.php b/src/Exception/DuplicateMappingException.php new file mode 100644 index 0000000..f14430c --- /dev/null +++ b/src/Exception/DuplicateMappingException.php @@ -0,0 +1,16 @@ +configurations[\get_class($configuration)] = $configuration; + $this->configurations[$configuration::class] = $configuration; } public function getConfiguration(string $class): ?MetadataConfigurationInterface @@ -62,6 +63,16 @@ public function getConfiguration(string $class): ?MetadataConfigurationInterface return $this->configurations[$class] ?? null; } + public function hasConfiguration(string $class): bool + { + return isset($this->configurations[$class]); + } + + public function getConfigurations(): array + { + return $this->configurations; + } + /** * @param ClassMetadata $metadata */ @@ -86,6 +97,10 @@ public function wakeup(ClassMetadata $metadata, ReflectionService $reflectionSer } } if ($metadata->fieldMappings) { + $shadowed = \array_intersect_key($mappings, $metadata->fieldMappings); + foreach ($shadowed as $field => $mapping) { + @\trigger_error(\sprintf('Extension metadata field "%s" in class "%s" is shadowed by a Doctrine field mapping and will be skipped.', $field, $this->name), \E_USER_NOTICE); + } $mappings = \array_diff_key($mappings, $metadata->fieldMappings); } @@ -121,6 +136,12 @@ public function wakeup(ClassMetadata $metadata, ReflectionService $reflectionSer $this->reflectionFields[$field] = $reflectionService->getAccessibleProperty($this->name, $field) ?? throw new LogicException(\sprintf('Unable to resolve reflection property "%s" on class "%s".', $field, $this->name)); } + + foreach ($this->configurations as $configuration) { + if ($configuration instanceof WakeupAwareConfigurationInterface) { + $configuration->wakeup(); + } + } } public function getOriginMetadata(): ClassMetadata diff --git a/src/Mapping/AbstractExtensionMetadataFactory.php b/src/Mapping/AbstractExtensionMetadataFactory.php index 50bdd53..291d922 100644 --- a/src/Mapping/AbstractExtensionMetadataFactory.php +++ b/src/Mapping/AbstractExtensionMetadataFactory.php @@ -27,9 +27,9 @@ abstract class AbstractExtensionMetadataFactory */ private static string $cacheSalt = '$EXTENSIONMETADATA'; /** - * @var ExtensionMetadataInterface[] + * @var \WeakMap> */ - private array $loadedMetadata = []; + private \WeakMap $loadedMetadata; private ?ReflectionService $reflectionService = null; /** @@ -39,15 +39,16 @@ abstract class AbstractExtensionMetadataFactory */ public function getMetadataFor(EntityManagerInterface $em, ClassMetadata $metadata): ExtensionMetadataInterface { - $key = \spl_object_id($em).'#'.$metadata->getName(); - if (isset($this->loadedMetadata[$key])) { - return $this->loadedMetadata[$key]; + $emMetadata = $this->getLoadedMetadata()[$em] ?? []; + $className = $metadata->getName(); + if (isset($emMetadata[$className])) { + return $emMetadata[$className]; } if ($cache = $em->getConfiguration()->getMetadataCache()) { // Note: cache key does not include EM identity. // If using multiple EntityManagers, ensure each has a separate metadata cache pool. - $item = $cache->getItem($this->sanitize($metadata->getName())); + $item = $cache->getItem($this->sanitize($className)); if ($item->isHit()) { $extensionMetadata = $item->get(); @@ -62,7 +63,12 @@ public function getMetadataFor(EntityManagerInterface $em, ClassMetadata $metada $extensionMetadata = $this->loadMetadata($em, $metadata); } - return $this->loadedMetadata[$key] = $extensionMetadata; + $loadedMetadata = $this->getLoadedMetadata(); + $emMetadata = $loadedMetadata[$em] ?? []; + $emMetadata[$className] = $extensionMetadata; + $loadedMetadata[$em] = $emMetadata; + + return $extensionMetadata; } /** @@ -70,9 +76,9 @@ public function getMetadataFor(EntityManagerInterface $em, ClassMetadata $metada */ public function hasMetadataFor(EntityManagerInterface $em, string $className): bool { - $key = \spl_object_id($em).'#'.$className; + $emMetadata = $this->getLoadedMetadata()[$em] ?? []; - return isset($this->loadedMetadata[$key]); + return isset($emMetadata[$className]); } /** @@ -123,6 +129,14 @@ private function loadEmbeddedMetadata(EntityManagerInterface $em, ClassMetadata } } + /** + * @return \WeakMap> + */ + private function getLoadedMetadata(): \WeakMap + { + return $this->loadedMetadata ??= new \WeakMap(); + } + private function getReflectionService(): ReflectionService { return $this->reflectionService ??= new RuntimeReflectionService(); @@ -135,6 +149,9 @@ protected function getCacheNamespace(): string private function sanitize(string $string): string { - return \preg_replace('/[^a-zA-Z0-9._-]/', '_', $string.'.'.$this->getCacheNamespace()).self::$cacheSalt; + $result = \preg_replace('/[^a-zA-Z0-9._-]/', '_', $string.'.'.$this->getCacheNamespace()); + \assert(\is_string($result)); + + return $result.self::$cacheSalt; } } diff --git a/src/Mapping/Driver/AbstractMappingDriver.php b/src/Mapping/Driver/AbstractMappingDriver.php index 9e242ae..254951a 100644 --- a/src/Mapping/Driver/AbstractMappingDriver.php +++ b/src/Mapping/Driver/AbstractMappingDriver.php @@ -20,6 +20,11 @@ public function __construct(protected readonly AttributeReader $reader) { } + public static function getPriority(): int + { + return 0; + } + /** * Returns true if this driver should process the given metadata. * @@ -91,9 +96,6 @@ private function supportsByPropertyAttribute(ExtensionMetadataInterface $metadat $reflection = $metadata->getOriginMetadata()->getReflectionClass(); foreach ($reflection->getProperties() as $property) { - if ($property->getDeclaringClass()->getName() !== $reflection->getName()) { - continue; - } if (null !== $this->reader->getPropertyAttribute($property, $attribute)) { return true; } diff --git a/src/Mapping/Driver/MappingDriverInterface.php b/src/Mapping/Driver/MappingDriverInterface.php index 412a8fb..54cfc2c 100644 --- a/src/Mapping/Driver/MappingDriverInterface.php +++ b/src/Mapping/Driver/MappingDriverInterface.php @@ -25,4 +25,9 @@ public function loadMetadataForClass(ExtensionMetadataInterface $extensionMetada * This is only the case if it is either mapped as an Entity or a MappedSuperclass. */ public function supports(ExtensionMetadataInterface $metadata): bool; + + /** + * Returns the priority of this driver. Higher values mean earlier execution. + */ + public static function getPriority(): int; } diff --git a/src/Mapping/ExtensionMetadataInterface.php b/src/Mapping/ExtensionMetadataInterface.php index 438ebd9..dd77e5d 100644 --- a/src/Mapping/ExtensionMetadataInterface.php +++ b/src/Mapping/ExtensionMetadataInterface.php @@ -32,10 +32,17 @@ public function getEmbeddedMetadata(): array; */ public function getEmbeddedMetadataWithConfiguration(string $class): iterable; - public function addEmbeddedMetadata(string $fieldName, ExtensionMetadataInterface $metadata): void; + public function addEmbeddedMetadata(string $fieldName, self $metadata): void; public function getConfiguration(string $class): ?MetadataConfigurationInterface; + public function hasConfiguration(string $class): bool; + + /** + * @return array + */ + public function getConfigurations(): array; + public function addConfiguration(MetadataConfigurationInterface $configuration): void; /** diff --git a/src/Mapping/MetadataReader.php b/src/Mapping/MetadataReader.php index 46a528a..6e36375 100644 --- a/src/Mapping/MetadataReader.php +++ b/src/Mapping/MetadataReader.php @@ -20,12 +20,13 @@ class MetadataReader { /** - * @var ExtensionMetadataInterface[] + * @var \WeakMap> */ - private array $configurations = []; + private \WeakMap $configurations; public function __construct(private readonly ExtensionMetadataFactory $factory) { + $this->configurations = new \WeakMap(); } /** @@ -33,13 +34,15 @@ public function __construct(private readonly ExtensionMetadataFactory $factory) */ public function loadExtensionMetadata(EntityManagerInterface $em, ClassMetadata $metadata): void { - $key = \spl_object_id($em).'#'.$metadata->getName(); - if (isset($this->configurations[$key])) { + $emConfigs = $this->configurations[$em] ??= []; + $className = $metadata->getName(); + if (isset($emConfigs[$className])) { return; } $config = $this->factory->getMetadataFor($em, $metadata); - $this->configurations[$key] = $config; + $emConfigs[$className] = $config; + $this->configurations[$em] = $emConfigs; } /** @@ -49,9 +52,9 @@ public function loadExtensionMetadata(EntityManagerInterface $em, ClassMetadata */ public function getExtensionMetadata(EntityManagerInterface $em, string $class): ExtensionMetadataInterface { - $key = \spl_object_id($em).'#'.$class; - if (isset($this->configurations[$key])) { - return $this->configurations[$key]; + $emConfigs = $this->configurations[$em] ??= []; + if (isset($emConfigs[$class])) { + return $emConfigs[$class]; } // Resolve canonical class name (handles proxies, aliases, etc.) @@ -62,16 +65,20 @@ public function getExtensionMetadata(EntityManagerInterface $em, string $class): if ($class === $canonicalClass) { $this->loadExtensionMetadata($em, $metadata); - return $this->configurations[$key]; + return $this->configurations[$em][$class]; } // For aliases/proxies, ensure canonical key exists, then alias it - $resolvedKey = \spl_object_id($em).'#'.$canonicalClass; - if (!isset($this->configurations[$resolvedKey])) { + $emConfigs = $this->configurations[$em] ?? []; + if (!isset($emConfigs[$canonicalClass])) { $this->loadExtensionMetadata($em, $metadata); + $emConfigs = $this->configurations[$em]; } - return $this->configurations[$key] = $this->configurations[$resolvedKey]; + $emConfigs[$class] = $emConfigs[$canonicalClass]; + $this->configurations[$em] = $emConfigs; + + return $emConfigs[$class]; } /** diff --git a/src/Mapping/ORM/ExtensionMetadataFactory.php b/src/Mapping/ORM/ExtensionMetadataFactory.php index a7db98d..6886f7d 100644 --- a/src/Mapping/ORM/ExtensionMetadataFactory.php +++ b/src/Mapping/ORM/ExtensionMetadataFactory.php @@ -11,6 +11,7 @@ namespace ChamberOrchestra\MetadataBundle\Mapping\ORM; +use ChamberOrchestra\MetadataBundle\DataCollector\MetadataDataCollector; use ChamberOrchestra\MetadataBundle\Mapping\AbstractExtensionMetadataFactory; use ChamberOrchestra\MetadataBundle\Mapping\Driver\MappingDriverInterface; use ChamberOrchestra\MetadataBundle\Mapping\ExtensionMetadataInterface; @@ -23,17 +24,26 @@ class ExtensionMetadataFactory extends AbstractExtensionMetadataFactory * @param iterable $drivers */ public function __construct( - #[AutowireIterator('chamber_orchestra_metadata.mapping.driver')] + #[AutowireIterator('chamber_orchestra_metadata.mapping.driver', defaultPriorityMethod: 'getPriority')] private readonly iterable $drivers, + private readonly ?MetadataDataCollector $dataCollector = null, ) { } protected function doLoadMetadata(ExtensionMetadataInterface $class): void { foreach ($this->drivers as $driver) { - if ($driver->supports($class)) { + $supported = $driver->supports($class); + if ($supported) { $driver->loadMetadataForClass($class); } + $this->dataCollector?->recordDriverInvocation($driver::class, $class->getName(), $supported, $supported); + } + + foreach ($class->getConfigurations() as $configuration) { + if ($configuration instanceof ValidatableConfigurationInterface) { + $configuration->validate($class); + } } } diff --git a/src/Mapping/ORM/ValidatableConfigurationInterface.php b/src/Mapping/ORM/ValidatableConfigurationInterface.php new file mode 100644 index 0000000..0043627 --- /dev/null +++ b/src/Mapping/ORM/ValidatableConfigurationInterface.php @@ -0,0 +1,19 @@ +set(MetadataReader::class) ->lazy(); - - $services->set(MetadataSubscriber::class); }; diff --git a/tests/Fixtures/Mapping/DummyMappingDriver.php b/tests/Fixtures/Mapping/DummyMappingDriver.php index 976a675..ad059c9 100644 --- a/tests/Fixtures/Mapping/DummyMappingDriver.php +++ b/tests/Fixtures/Mapping/DummyMappingDriver.php @@ -24,4 +24,9 @@ public function supports(ExtensionMetadataInterface $metadata): bool { return false; } + + public static function getPriority(): int + { + return 0; + } } diff --git a/tests/Fixtures/Mapping/TrackingMappingDriver.php b/tests/Fixtures/Mapping/TrackingMappingDriver.php index 60f0154..7677b75 100644 --- a/tests/Fixtures/Mapping/TrackingMappingDriver.php +++ b/tests/Fixtures/Mapping/TrackingMappingDriver.php @@ -30,4 +30,9 @@ public function supports(ExtensionMetadataInterface $metadata): bool return true; } + + public static function getPriority(): int + { + return 0; + } } diff --git a/tests/Unit/Command/MetadataDumpCommandTest.php b/tests/Unit/Command/MetadataDumpCommandTest.php new file mode 100644 index 0000000..1b483a6 --- /dev/null +++ b/tests/Unit/Command/MetadataDumpCommandTest.php @@ -0,0 +1,83 @@ +mapField('name'); + + $extensionMetadata = new ExtensionMetadata(new ClassMetadata(SimpleEntity::class)); + $extensionMetadata->addConfiguration($configuration); + + $reader = $this->createStub(MetadataReader::class); + $reader->method('getExtensionMetadata')->willReturn($extensionMetadata); + + $em = $this->createStub(EntityManagerInterface::class); + + $command = new MetadataDumpCommand($reader, $em); + $tester = new CommandTester($command); + + $tester->execute(['entity-class' => SimpleEntity::class]); + + $output = $tester->getDisplay(); + self::assertStringContainsString(SimpleEntity::class, $output); + self::assertStringContainsString(TestMetadataConfiguration::class, $output); + self::assertStringContainsString('name', $output); + self::assertSame(0, $tester->getStatusCode()); + } + + public function testReportsNoConfigurations(): void + { + $extensionMetadata = new ExtensionMetadata(new ClassMetadata(SimpleEntity::class)); + + $reader = $this->createStub(MetadataReader::class); + $reader->method('getExtensionMetadata')->willReturn($extensionMetadata); + + $em = $this->createStub(EntityManagerInterface::class); + + $command = new MetadataDumpCommand($reader, $em); + $tester = new CommandTester($command); + + $tester->execute(['entity-class' => SimpleEntity::class]); + + $output = $tester->getDisplay(); + self::assertStringContainsString('No metadata configurations found', $output); + self::assertSame(0, $tester->getStatusCode()); + } + + public function testFailsForNonexistentClass(): void + { + $reader = $this->createStub(MetadataReader::class); + $em = $this->createStub(EntityManagerInterface::class); + + $command = new MetadataDumpCommand($reader, $em); + $tester = new CommandTester($command); + + $tester->execute(['entity-class' => 'NonExistent\\Class\\Name']); + + self::assertSame(1, $tester->getStatusCode()); + self::assertStringContainsString('does not exist', $tester->getDisplay()); + } +} diff --git a/tests/Unit/DataCollector/MetadataDataCollectorTest.php b/tests/Unit/DataCollector/MetadataDataCollectorTest.php new file mode 100644 index 0000000..1e7508d --- /dev/null +++ b/tests/Unit/DataCollector/MetadataDataCollectorTest.php @@ -0,0 +1,66 @@ +recordDriverInvocation('App\\Driver\\FooDriver', 'App\\Entity\\Bar', true, true); + $collector->recordDriverInvocation('App\\Driver\\BazDriver', 'App\\Entity\\Bar', false, false); + + $collector->collect(new Request(), new Response()); + + $stats = $collector->getDriverStats(); + + self::assertCount(2, $stats); + self::assertSame('App\\Driver\\FooDriver', $stats[0]['driver']); + self::assertTrue($stats[0]['supported']); + self::assertSame('App\\Driver\\BazDriver', $stats[1]['driver']); + self::assertFalse($stats[1]['supported']); + } + + public function testResetClearsData(): void + { + $collector = new MetadataDataCollector(); + + $collector->recordDriverInvocation('App\\Driver\\FooDriver', 'App\\Entity\\Bar', true, true); + $collector->collect(new Request(), new Response()); + + self::assertCount(1, $collector->getDriverStats()); + + $collector->reset(); + + self::assertSame([], $collector->getDriverStats()); + } + + public function testGetName(): void + { + $collector = new MetadataDataCollector(); + + self::assertSame('chamber_orchestra_metadata', $collector->getName()); + } + + public function testReturnsEmptyStatsBeforeCollect(): void + { + $collector = new MetadataDataCollector(); + + self::assertSame([], $collector->getDriverStats()); + } +} diff --git a/tests/Unit/EventSubscriber/AbstractDoctrineListenerTest.php b/tests/Unit/EventSubscriber/AbstractDoctrineListenerTest.php index 8a1607b..2226290 100644 --- a/tests/Unit/EventSubscriber/AbstractDoctrineListenerTest.php +++ b/tests/Unit/EventSubscriber/AbstractDoctrineListenerTest.php @@ -12,6 +12,7 @@ namespace Tests\Unit\EventSubscriber; use ChamberOrchestra\MetadataBundle\EventSubscriber\AbstractDoctrineListener; +use ChamberOrchestra\MetadataBundle\Helper\MetadataArgs; use ChamberOrchestra\MetadataBundle\Mapping\MetadataReader; use ChamberOrchestra\MetadataBundle\Mapping\ORM\ExtensionMetadata; use Doctrine\ORM\EntityManagerInterface; @@ -42,9 +43,7 @@ public function testFiltersScheduledEntitiesByConfiguration(): void $reader = $this->createStub(MetadataReader::class); $reader ->method('getExtensionMetadata') - ->willReturnCallback(static function (EntityManagerInterface $em, string $class) use ($metadataByClass): ExtensionMetadata { - return $metadataByClass[$class]; - }); + ->willReturnCallback(static fn (EntityManagerInterface $em, string $class): ExtensionMetadata => $metadataByClass[$class]); $unitOfWork = $this->createStub(UnitOfWork::class); $unitOfWork->method('getScheduledEntityInsertions')->willReturn([$configuredEntity, $unconfiguredEntity]); @@ -86,4 +85,30 @@ public function deletions(EntityManagerInterface $em, string $class): array self::assertCount(0, $deletions); } + + public function testGetEntityChangeSetDelegatesToUnitOfWork(): void + { + $entity = new SimpleEntity(); + $changeSet = ['name' => ['old', 'new']]; + + $unitOfWork = $this->createStub(UnitOfWork::class); + $unitOfWork->method('getEntityChangeSet')->willReturn($changeSet); + + $entityManager = $this->createStub(EntityManagerInterface::class); + $entityManager->method('getUnitOfWork')->willReturn($unitOfWork); + + $extensionMetadata = new ExtensionMetadata(new ClassMetadata(SimpleEntity::class)); + $configuration = new TestMetadataConfiguration(); + + $args = new MetadataArgs($entityManager, $extensionMetadata, $configuration, $entity); + + $listener = new class extends AbstractDoctrineListener { + public function changeSet(EntityManagerInterface $em, MetadataArgs $args): array + { + return $this->getEntityChangeSet($em, $args); + } + }; + + self::assertSame($changeSet, $listener->changeSet($entityManager, $args)); + } } diff --git a/tests/Unit/Exception/DuplicateMappingExceptionTest.php b/tests/Unit/Exception/DuplicateMappingExceptionTest.php new file mode 100644 index 0000000..bd7455e --- /dev/null +++ b/tests/Unit/Exception/DuplicateMappingExceptionTest.php @@ -0,0 +1,33 @@ +getMessage()); } @@ -34,6 +37,7 @@ public function testMissingAttributeMessage(): void { $exception = MappingException::missingAttribute('Foo', 'bar', 'Baz'); + self::assertInstanceOf(InvalidMappingAttributeException::class, $exception); self::assertSame('Class "Foo" has no required attribute "Baz" at field "bar".', $exception->getMessage()); } } diff --git a/tests/Unit/Mapping/AbstractExtensionMetadataTest.php b/tests/Unit/Mapping/AbstractExtensionMetadataTest.php index 4a5c9ca..0a4b927 100644 --- a/tests/Unit/Mapping/AbstractExtensionMetadataTest.php +++ b/tests/Unit/Mapping/AbstractExtensionMetadataTest.php @@ -28,8 +28,11 @@ public function testConfigurationRegistrationAndLookup(): void $metadata = new ExtensionMetadata(new ClassMetadata(SimpleEntity::class)); $configuration = new TestMetadataConfiguration(); + self::assertFalse($metadata->hasConfiguration(TestMetadataConfiguration::class)); + $metadata->addConfiguration($configuration); + self::assertTrue($metadata->hasConfiguration(TestMetadataConfiguration::class)); self::assertSame($configuration, $metadata->getConfiguration(TestMetadataConfiguration::class)); } diff --git a/tests/Unit/Mapping/ORM/ExtensionMetadataFactoryTest.php b/tests/Unit/Mapping/ORM/ExtensionMetadataFactoryTest.php index c851535..ab898fb 100644 --- a/tests/Unit/Mapping/ORM/ExtensionMetadataFactoryTest.php +++ b/tests/Unit/Mapping/ORM/ExtensionMetadataFactoryTest.php @@ -12,12 +12,15 @@ namespace Tests\Unit\Mapping\ORM; use ChamberOrchestra\MetadataBundle\Mapping\Driver\MappingDriverInterface; +use ChamberOrchestra\MetadataBundle\Mapping\ExtensionMetadataInterface; use ChamberOrchestra\MetadataBundle\Mapping\ORM\ExtensionMetadataFactory; +use ChamberOrchestra\MetadataBundle\Mapping\ORM\ValidatableConfigurationInterface; use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; use PHPUnit\Framework\TestCase; use Tests\Fixtures\Entity\SimpleEntity; +use Tests\Fixtures\Mapping\TestMetadataConfiguration; final class ExtensionMetadataFactoryTest extends TestCase { @@ -46,4 +49,37 @@ public function testLoadsMetadataFromSupportedDrivers(): void $factory->getMetadataFor($entityManager, $metadata); } + + public function testCallsValidateOnValidatableConfigurations(): void + { + $validateCalled = false; + + $driver = $this->createStub(MappingDriverInterface::class); + $driver->method('supports')->willReturn(true); + $driver->method('loadMetadataForClass') + ->willReturnCallback(static function (ExtensionMetadataInterface $meta) use (&$validateCalled): void { + $config = new class($validateCalled) extends TestMetadataConfiguration implements ValidatableConfigurationInterface { + public function __construct(private bool &$called) + { + } + + public function validate(ExtensionMetadataInterface $metadata): void + { + $this->called = true; + } + }; + $meta->addConfiguration($config); + }); + + $factory = new ExtensionMetadataFactory([$driver]); + + $entityManager = $this->createStub(EntityManagerInterface::class); + $entityManager->method('getConfiguration')->willReturn(new Configuration()); + + $metadata = new ClassMetadata(SimpleEntity::class); + + $factory->getMetadataFor($entityManager, $metadata); + + self::assertTrue($validateCalled, 'validate() should have been called on the validatable configuration'); + } } From 296f029379af25d110f79f40c6128f552634e18b Mon Sep 17 00:00:00 2001 From: Dev Date: Fri, 27 Feb 2026 13:54:15 +0000 Subject: [PATCH 2/2] Add Claude Code settings Co-Authored-By: Claude Opus 4.6 --- .claude/settings.json | 12 ++++++++++++ src/EventSubscriber/AbstractDoctrineListener.php | 2 +- src/Mapping/MetadataReader.php | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..432db3f --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,12 @@ +{ + "enabledPlugins": { + "code-review@claude-plugins-official": true, + "github@claude-plugins-official": true, + "feature-dev@claude-plugins-official": true, + "code-simplifier@claude-plugins-official": true, + "ralph-loop@claude-plugins-official": true, + "pr-review-toolkit@claude-plugins-official": true, + "claude-md-management@claude-plugins-official": true, + "php-lsp@claude-plugins-official": true + } +} diff --git a/src/EventSubscriber/AbstractDoctrineListener.php b/src/EventSubscriber/AbstractDoctrineListener.php index d983199..2006a07 100644 --- a/src/EventSubscriber/AbstractDoctrineListener.php +++ b/src/EventSubscriber/AbstractDoctrineListener.php @@ -44,7 +44,7 @@ protected function getScheduledEntityDeletions(EntityManagerInterface $em, strin } /** - * @return array + * @return array> */ protected function getEntityChangeSet(EntityManagerInterface $em, MetadataArgs $args): array { diff --git a/src/Mapping/MetadataReader.php b/src/Mapping/MetadataReader.php index 6e36375..6b26c44 100644 --- a/src/Mapping/MetadataReader.php +++ b/src/Mapping/MetadataReader.php @@ -69,7 +69,7 @@ public function getExtensionMetadata(EntityManagerInterface $em, string $class): } // For aliases/proxies, ensure canonical key exists, then alias it - $emConfigs = $this->configurations[$em] ?? []; + $emConfigs = $this->configurations[$em]; if (!isset($emConfigs[$canonicalClass])) { $this->loadExtensionMetadata($em, $metadata); $emConfigs = $this->configurations[$em];