Merge pull request #87 from chimeraphp/prevent-duplicated-route-regis…

…tration Prevent the registration of duplicated routes
chimeraphp · Feb 22, 2021 · ef09ad3 · ef09ad3
2 parents 7c2080a + f6424e0
commit ef09ad3
Show file tree

Hide file tree

Showing 5 changed files with 127 additions and 1 deletion.
diff --git a/infection.json.dist b/infection.json.dist
@@ -9,5 +9,5 @@
         "text": "infection.log"
     },
     "minMsi": 40,
-    "minCoveredMsi": 90
+    "minCoveredMsi": 80
 }
diff --git a/src/Routing/Expressive/RegisterServices.php b/src/Routing/Expressive/RegisterServices.php
@@ -49,6 +49,7 @@
 use Zend\Stratigility\MiddlewarePipe;
 
 use function array_combine;
+use function array_key_exists;
 use function array_map;
 use function assert;
 use function explode;
@@ -61,6 +62,9 @@ final class RegisterServices implements CompilerPassInterface
     private const MESSAGE_INVALID_ROUTE = 'You must specify the "route_name", "path", and "behavior" arguments in '
     . '"%s" (tag "%s").';
 
+    private const MESSAGE_DUPLICATED_ROUTE = 'The service "%s" is trying to declare a route with name "%s" which has '
+                                           . 'already been defined by the service "%s".';
+
     private const BEHAVIORS = [
         'fetch'         => ['methods' => ['GET'], 'callback' => 'fetchOnly'],
         'create'        => ['methods' => ['POST'], 'callback' => 'createOnly'],
@@ -104,6 +108,7 @@ public function process(ContainerBuilder $container): void
     private function extractRoutes(ContainerBuilder $container): array
     {
         $routes = [];
+        $names  = [];
 
         foreach ($container->findTaggedServiceIds(Tags::HTTP_ROUTE) as $serviceId => $tags) {
             foreach ($tags as $tag) {
@@ -113,6 +118,19 @@ private function extractRoutes(ContainerBuilder $container): array
                     );
                 }
 
+                assert(is_string($tag['route_name']));
+
+                if (array_key_exists($tag['route_name'], $names)) {
+                    throw new InvalidArgumentException(
+                        sprintf(
+                            self::MESSAGE_DUPLICATED_ROUTE,
+                            $serviceId,
+                            $tag['route_name'],
+                            $names[$tag['route_name']]
+                        )
+                    );
+                }
+
                 if (isset($tag['methods'])) {
                     $tag['methods'] = explode(',', $tag['methods']);
                 }
@@ -123,6 +141,8 @@ private function extractRoutes(ContainerBuilder $container): array
 
                 $routes[$tag['app']] ??= [];
                 $routes[$tag['app']][] = $tag;
+
+                $names[$tag['route_name']] = $serviceId;
             }
         }
 

diff --git a/src/Routing/Mezzio/RegisterServices.php b/src/Routing/Mezzio/RegisterServices.php
@@ -49,6 +49,7 @@
 use Symfony\Component\DependencyInjection\Reference;
 
 use function array_combine;
+use function array_key_exists;
 use function array_map;
 use function assert;
 use function explode;
@@ -61,6 +62,9 @@ final class RegisterServices implements CompilerPassInterface
     private const MESSAGE_INVALID_ROUTE = 'You must specify the "route_name", "path", and "behavior" arguments in '
                                           . '"%s" (tag "%s").';
 
+    private const MESSAGE_DUPLICATED_ROUTE = 'The service "%s" is trying to declare a route with name "%s" which has '
+                                           . 'already been defined by the service "%s".';
+
     private const BEHAVIORS = [
         'fetch'         => ['methods' => ['GET'], 'callback' => 'fetchOnly'],
         'create'        => ['methods' => ['POST'], 'callback' => 'createOnly'],
@@ -104,6 +108,7 @@ public function process(ContainerBuilder $container): void
     private function extractRoutes(ContainerBuilder $container): array
     {
         $routes = [];
+        $names  = [];
 
         foreach ($container->findTaggedServiceIds(Tags::HTTP_ROUTE) as $serviceId => $tags) {
             foreach ($tags as $tag) {
@@ -113,6 +118,19 @@ private function extractRoutes(ContainerBuilder $container): array
                     );
                 }
 
+                assert(is_string($tag['route_name']));
+
+                if (array_key_exists($tag['route_name'], $names)) {
+                    throw new InvalidArgumentException(
+                        sprintf(
+                            self::MESSAGE_DUPLICATED_ROUTE,
+                            $serviceId,
+                            $tag['route_name'],
+                            $names[$tag['route_name']]
+                        )
+                    );
+                }
+
                 if (isset($tag['methods'])) {
                     $tag['methods'] = explode(',', $tag['methods']);
                 }
@@ -123,6 +141,8 @@ private function extractRoutes(ContainerBuilder $container): array
 
                 $routes[$tag['app']] ??= [];
                 $routes[$tag['app']][] = $tag;
+
+                $names[$tag['route_name']] = $serviceId;
             }
         }
 

diff --git a/tests/Unit/Routing/Expressive/RegisterServicesTest.php b/tests/Unit/Routing/Expressive/RegisterServicesTest.php
@@ -0,0 +1,43 @@
+<?php
+declare(strict_types=1);
+
+namespace Chimera\DependencyInjection\Tests\Unit\Routing\Expressive;
+
+use Chimera\DependencyInjection\Routing\Expressive\RegisterServices;
+use Chimera\DependencyInjection\Tags;
+use InvalidArgumentException;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+/** @coversDefaultClass \Chimera\DependencyInjection\Routing\Expressive\RegisterServices */
+final class RegisterServicesTest extends TestCase
+{
+    /**
+     * @test
+     *
+     * @covers ::__construct
+     * @covers ::process
+     * @covers ::extractRoutes
+     */
+    public function exceptionShouldBeRaisedWhenTryingToRegisterDuplicatedRoutes(): void
+    {
+        $service1 = new Definition();
+        $service1->addTag(Tags::HTTP_ROUTE, ['route_name' => 'test', 'path' => '/one', 'behavior' => 'fetch']);
+
+        $service2 = new Definition();
+        $service2->addTag(Tags::HTTP_ROUTE, ['route_name' => 'test', 'path' => '/two', 'behavior' => 'fetch']);
+
+        $builder = new ContainerBuilder();
+        $builder->addDefinitions(['service1' => $service1, 'service2' => $service2]);
+
+        $registerServices = new RegisterServices('app', 'app.command-bus', 'app.query-bus');
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(
+            'The service "service2" is trying to declare a route with name "test" '
+            . 'which has already been defined by the service "service1".'
+        );
+        $registerServices->process($builder);
+    }
+}
diff --git a/tests/Unit/Routing/Mezzio/RegisterServicesTest.php b/tests/Unit/Routing/Mezzio/RegisterServicesTest.php
@@ -0,0 +1,43 @@
+<?php
+declare(strict_types=1);
+
+namespace Chimera\DependencyInjection\Tests\Unit\Routing\Mezzio;
+
+use Chimera\DependencyInjection\Routing\Mezzio\RegisterServices;
+use Chimera\DependencyInjection\Tags;
+use InvalidArgumentException;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+/** @coversDefaultClass \Chimera\DependencyInjection\Routing\Mezzio\RegisterServices */
+final class RegisterServicesTest extends TestCase
+{
+    /**
+     * @test
+     *
+     * @covers ::__construct
+     * @covers ::process
+     * @covers ::extractRoutes
+     */
+    public function exceptionShouldBeRaisedWhenTryingToRegisterDuplicatedRoutes(): void
+    {
+        $service1 = new Definition();
+        $service1->addTag(Tags::HTTP_ROUTE, ['route_name' => 'test', 'path' => '/one', 'behavior' => 'fetch']);
+
+        $service2 = new Definition();
+        $service2->addTag(Tags::HTTP_ROUTE, ['route_name' => 'test', 'path' => '/two', 'behavior' => 'fetch']);
+
+        $builder = new ContainerBuilder();
+        $builder->addDefinitions(['service1' => $service1, 'service2' => $service2]);
+
+        $registerServices = new RegisterServices('app', 'app.command-bus', 'app.query-bus');
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(
+            'The service "service2" is trying to declare a route with name "test" '
+            . 'which has already been defined by the service "service1".'
+        );
+        $registerServices->process($builder);
+    }
+}