diff --git a/CHANGELOG.md b/CHANGELOG.md index fc5571d..843aef8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ Latest ------ +### Changes +* [#25](https://github.com/cleverage/cache-process-bundle/issues/25) Give the ids of both services in the error on duplicate adapter codes: the adapters are registered by a compiler pass of the bundle, `AdapterRegistry::addAdapter()` gets an optional `$serviceId` argument. Update documentation, add tests. + ### Fixes * [#22](https://github.com/cleverage/cache-process-bundle/issues/22) Fix GetTask and SetTask: throw an explicit `\UnexpectedValueException` on a non-array input (a `\TypeError` was triggered by `array_merge()`). Update documentation, add tests. * [#23](https://github.com/cleverage/cache-process-bundle/issues/23) Fix GetTask and SetTask: validate the option values given by the input with the options resolver (they were used as is). Update documentation, add tests. diff --git a/docs/reference/adapter.md b/docs/reference/adapter.md index 7ee7463..b022d13 100644 --- a/docs/reference/adapter.md +++ b/docs/reference/adapter.md @@ -86,8 +86,9 @@ services: Notes ----- -* Codes must be unique: registering two adapters with the same code throws an `\UnexpectedValueException` - (`Adapter is already defined`) when the registry is instantiated, i.e. the first time a cache task is used. +* Codes must be unique: registering two adapters with the same code throws an `\UnexpectedValueException` giving the + ids of both services (`Adapter is already defined by service "", cannot register service ""`) when + the registry is instantiated, i.e. the first time a cache task is used. * Using a code that is not registered throws a `CleverAge\CacheProcessBundle\Exception\MissingAdapterException` (`Adapter is missing`) when the task is executed. * The cache tasks do not handle any expiration: the lifetime of the items is the default lifetime of the decorated diff --git a/src/CleverAgeCacheProcessBundle.php b/src/CleverAgeCacheProcessBundle.php index a35c607..c88f0d7 100644 --- a/src/CleverAgeCacheProcessBundle.php +++ b/src/CleverAgeCacheProcessBundle.php @@ -13,7 +13,7 @@ namespace CleverAge\CacheProcessBundle; -use CleverAge\ProcessBundle\DependencyInjection\Compiler\RegistryCompilerPass; +use CleverAge\CacheProcessBundle\DependencyInjection\Compiler\RegisterAdaptersPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -24,13 +24,7 @@ class CleverAgeCacheProcessBundle extends Bundle */ public function build(ContainerBuilder $container): void { - $container->addCompilerPass( - new RegistryCompilerPass( - 'cleverage_cache_process.registry.adapter', - 'cleverage.cache.adapter', - 'addAdapter' - ) - ); + $container->addCompilerPass(new RegisterAdaptersPass()); } #[\Override] diff --git a/src/DependencyInjection/Compiler/RegisterAdaptersPass.php b/src/DependencyInjection/Compiler/RegisterAdaptersPass.php new file mode 100644 index 0000000..b478e96 --- /dev/null +++ b/src/DependencyInjection/Compiler/RegisterAdaptersPass.php @@ -0,0 +1,36 @@ +has('cleverage_cache_process.registry.adapter')) { + return; + } + + $definition = $container->findDefinition('cleverage_cache_process.registry.adapter'); + foreach (array_keys($container->findTaggedServiceIds('cleverage.cache.adapter')) as $id) { + $definition->addMethodCall('addAdapter', [new Reference($id), $id]); + } + } +} diff --git a/src/Registry/AdapterRegistry.php b/src/Registry/AdapterRegistry.php index c8e2008..3df66df 100644 --- a/src/Registry/AdapterRegistry.php +++ b/src/Registry/AdapterRegistry.php @@ -24,12 +24,25 @@ class AdapterRegistry /** @var AdapterInterface[] */ private array $adapters = []; - public function addAdapter(AdapterInterface $adapter): void + /** @var array Service ids of the adapters, indexed by code */ + private array $serviceIds = []; + + /** + * @param string|null $serviceId Id of the adapter service, used to identify the adapters with the same code + */ + public function addAdapter(AdapterInterface $adapter, ?string $serviceId = null): void { - if (\array_key_exists($adapter->getCode(), $this->adapters)) { - throw new \UnexpectedValueException("Adapter {$adapter->getCode()} is already defined"); + $code = $adapter->getCode(); + if (\array_key_exists($code, $this->adapters)) { + $message = "Adapter {$code} is already defined"; + if (null !== $this->serviceIds[$code] && null !== $serviceId) { + $message .= " by service \"{$this->serviceIds[$code]}\", cannot register service \"{$serviceId}\""; + } + + throw new \UnexpectedValueException($message); } - $this->adapters[$adapter->getCode()] = $adapter; + $this->adapters[$code] = $adapter; + $this->serviceIds[$code] = $serviceId; } /** diff --git a/tests/DependencyInjection/Compiler/RegisterAdaptersPassTest.php b/tests/DependencyInjection/Compiler/RegisterAdaptersPassTest.php new file mode 100644 index 0000000..af9aefd --- /dev/null +++ b/tests/DependencyInjection/Compiler/RegisterAdaptersPassTest.php @@ -0,0 +1,76 @@ +createContainer(['app.adapter.memory' => 'memory', 'app.adapter.other' => 'other']); + $container->compile(true); + + /** @var AdapterRegistry $registry */ + $registry = $container->get('cleverage_cache_process.registry.adapter'); + self::assertSame('memory', $registry->getAdapter('memory')->getCode()); + self::assertSame('other', $registry->getAdapter('other')->getCode()); + } + + public function testDuplicateCodeGivesTheServiceIds(): void + { + $container = $this->createContainer(['app.adapter.memory' => 'memory', 'app.adapter.memory_duplicate' => 'memory']); + $container->compile(true); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Adapter memory is already defined by service "app.adapter.memory", cannot register service "app.adapter.memory_duplicate"'); + $container->get('cleverage_cache_process.registry.adapter'); + } + + public function testWithoutRegistry(): void + { + $container = new ContainerBuilder(); + (new RegisterAdaptersPass())->process($container); + + self::assertFalse($container->has('cleverage_cache_process.registry.adapter')); + } + + /** + * @param array $adapters Codes of the adapters, indexed by service id + */ + private function createContainer(array $adapters): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->addCompilerPass(new RegisterAdaptersPass()); + $container->setDefinition('cleverage_cache_process.registry.adapter', new Definition(AdapterRegistry::class)) + ->setPublic(true); + foreach ($adapters as $id => $code) { + $container->setDefinition($id, new Definition(Adapter::class, [new Definition(ArrayAdapter::class), $code])) + ->addTag('cleverage.cache.adapter'); + } + + return $container; + } +} diff --git a/tests/Registry/AdapterRegistryTest.php b/tests/Registry/AdapterRegistryTest.php new file mode 100644 index 0000000..ea5b8be --- /dev/null +++ b/tests/Registry/AdapterRegistryTest.php @@ -0,0 +1,67 @@ +addAdapter($memory, 'app.adapter.memory'); + $registry->addAdapter($other); + + self::assertSame($memory, $registry->getAdapter('memory')); + self::assertSame($other, $registry->getAdapter('other')); + } + + public function testMissingAdapter(): void + { + $this->expectException(MissingAdapterException::class); + $this->expectExceptionMessage('Adapter missing is missing'); + (new AdapterRegistry())->getAdapter('missing'); + } + + public function testDuplicateCodeGivesTheServiceIds(): void + { + $registry = new AdapterRegistry(); + $registry->addAdapter(new Adapter(new ArrayAdapter(), 'memory'), 'app.adapter.memory'); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Adapter memory is already defined by service "app.adapter.memory", cannot register service "app.adapter.memory_duplicate"'); + $registry->addAdapter(new Adapter(new ArrayAdapter(), 'memory'), 'app.adapter.memory_duplicate'); + } + + public function testDuplicateCodeWithoutServiceIds(): void + { + $registry = new AdapterRegistry(); + $registry->addAdapter(new Adapter(new ArrayAdapter(), 'memory')); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessageMatches('/^Adapter memory is already defined$/'); + $registry->addAdapter(new Adapter(new ArrayAdapter(), 'memory'), 'app.adapter.memory_duplicate'); + } +}