feat(registry) #25 Give the service ids in the error on duplicate adapter codes

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.

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 <code> 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 <code> is already defined by service "<id>", cannot register service "<id>"`) 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 <code> 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

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]

src/DependencyInjection/Compiler/RegisterAdaptersPass.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/CacheProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\CacheProcessBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Reference;
+
+/**
+ * Adds the tagged cache adapters to the adapter registry, with their service id.
+ */
+class RegisterAdaptersPass implements CompilerPassInterface
+{
+    public function process(ContainerBuilder $container): void
+    {
+        if (!$container->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]);
+        }
+    }
+}

src/Registry/AdapterRegistry.php

@@ -24,12 +24,25 @@ class AdapterRegistry
     /** @var AdapterInterface[] */
     private array $adapters = [];
 
-    public function addAdapter(AdapterInterface $adapter): void
+    /** @var array<string, string|null> 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;
     }
 
     /**

tests/DependencyInjection/Compiler/RegisterAdaptersPassTest.php

@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/CacheProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\CacheProcessBundle\Tests\DependencyInjection\Compiler;
+
+use CleverAge\CacheProcessBundle\Adapter\Adapter;
+use CleverAge\CacheProcessBundle\DependencyInjection\Compiler\RegisterAdaptersPass;
+use CleverAge\CacheProcessBundle\Registry\AdapterRegistry;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+#[CoversClass(RegisterAdaptersPass::class)]
+#[UsesClass(AdapterRegistry::class)]
+#[UsesClass(Adapter::class)]
+class RegisterAdaptersPassTest extends TestCase
+{
+    public function testAdaptersAreRegistered(): void
+    {
+        $container = $this->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<string, string> $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;
+    }
+}

tests/Registry/AdapterRegistryTest.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/CacheProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\CacheProcessBundle\Tests\Registry;
+
+use CleverAge\CacheProcessBundle\Adapter\Adapter;
+use CleverAge\CacheProcessBundle\Exception\MissingAdapterException;
+use CleverAge\CacheProcessBundle\Registry\AdapterRegistry;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+
+#[CoversClass(AdapterRegistry::class)]
+#[UsesClass(Adapter::class)]
+#[UsesClass(MissingAdapterException::class)]
+class AdapterRegistryTest extends TestCase
+{
+    public function testGetAdapter(): void
+    {
+        $registry = new AdapterRegistry();
+        $memory = new Adapter(new ArrayAdapter(), 'memory');
+        $other = new Adapter(new ArrayAdapter(), 'other');
+        $registry->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');
+    }
+}