Merge pull request #16 from membrane-php/ensure-unique-operationids

Adds detection of duplicated operation Ids
membrane-php · Jun 8, 2023 · 85af736 · 85af736
2 parents ff8c1f1 + 905e2ae
commit 85af736
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 1 deletion.
diff --git a/src/Console/Commands/CacheOpenAPIRoutes.php b/src/Console/Commands/CacheOpenAPIRoutes.php
@@ -4,6 +4,7 @@
 
 namespace Membrane\OpenAPIRouter\Console\Commands;
 
+use Membrane\OpenAPIRouter\Console\Service\CacheOpenAPIRoutes as CacheOpenAPIRoutesService;
 use Symfony\Component\Console\Attribute\AsCommand;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
@@ -40,7 +41,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         assert(is_string($destination));
 
         $logger = new ConsoleLogger($output);
-        $service = new \Membrane\OpenAPIRouter\Console\Service\CacheOpenAPIRoutes($logger);
+        $service = new CacheOpenAPIRoutesService($logger);
 
         return $service->cache($openAPIFilePath, $destination) ? Command::SUCCESS : Command::FAILURE;
     }

diff --git a/src/Exception/CannotProcessOpenAPI.php b/src/Exception/CannotProcessOpenAPI.php
@@ -20,6 +20,7 @@ class CannotProcessOpenAPI extends RuntimeException
     public const PATH_MISMATCH = 2;
     public const TYPE_MISMATCH = 3;
     public const MISSING_OPERATION_ID = 4;
+    public const DUPLICATE_OPERATION_ID = 5;
 
 
     public static function invalidPath(string $path): self
@@ -55,4 +56,25 @@ public static function missingOperationId(string $path, string $method): self
         );
         return new self($message, self::MISSING_OPERATION_ID);
     }
+
+    /**
+     * @param array{operation: string, path: string} $operationIdUse1
+     * @param array{operation: string, path: string} $operationIdUse2
+     */
+    public static function duplicateOperationId(
+        string $operationId,
+        array $operationIdUse1,
+        array $operationIdUse2
+    ): self {
+        $message = sprintf(
+            'Duplicate operation id: %s. Used by %s %s and %s %s',
+            $operationId,
+            $operationIdUse1['operation'],
+            $operationIdUse1['path'],
+            $operationIdUse2['operation'],
+            $operationIdUse2['path']
+        );
+
+        return new self($message, self::DUPLICATE_OPERATION_ID);
+    }
 }
diff --git a/src/Router/Collector/RouteCollector.php b/src/Router/Collector/RouteCollector.php
@@ -29,6 +29,7 @@ public function collect(OpenApi $openApi): RouteCollection
     private function collectRoutes(OpenApi $openApi): array
     {
         $rootServers = $this->getServers($openApi);
+        $operationIds = [];
         foreach ($openApi->paths as $path => $pathObject) {
             $pathServers = $this->getServers($pathObject);
             foreach ($pathObject->getOperations() as $operation => $operationObject) {
@@ -39,6 +40,14 @@ private function collectRoutes(OpenApi $openApi): array
                     throw CannotProcessOpenAPI::missingOperationId($path, $operation);
                 }
 
+                if (isset($operationIds[$operationObject->operationId])) {
+                    throw CannotProcessOpenAPI::duplicateOperationId(
+                        $operationObject->operationId,
+                        $operationIds[$operationObject->operationId],
+                        ['path' => $path, 'operation' => $operation]
+                    );
+                }
+
                 if ($operationServers !== []) {
                     $servers = $operationServers;
                 } elseif ($pathServers !== []) {
@@ -53,6 +62,11 @@ private function collectRoutes(OpenApi $openApi): array
                     $operation,
                     $operationObject->operationId
                 );
+
+                $operationIds[$operationObject->operationId] = [
+                    'path' => $path,
+                    'operation' => $operation
+                ];
             }
         }
         return $collection ?? [];

diff --git a/tests/Router/Collector/RouteCollectorTest.php b/tests/Router/Collector/RouteCollectorTest.php
@@ -5,6 +5,7 @@
 namespace Membrane\OpenAPIRouter\Router\Collector;
 
 use cebe\openapi\Reader;
+use Membrane\OpenAPIRouter\Exception\CannotProcessOpenAPI;
 use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI;
 use Membrane\OpenAPIRouter\Router\ValueObject\Route;
 use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection;
@@ -33,6 +34,32 @@ public function throwExceptionIfThereAreNoRoutes(): void
         $sut->collect($openApi);
     }
 
+    #[Test]
+    public function throwsExceptionForMissingOperationId(): void
+    {
+        $sut = new RouteCollector();
+        $openApi = Reader::readFromYamlFile(self::FIXTURES . 'missingOperationId.yaml');
+
+        self::expectExceptionObject(CannotProcessOpenAPI::missingOperationId('/path', 'get'));
+
+        $sut->collect($openApi);
+    }
+
+    #[Test]
+    public function throwsExceptionForDuplicateOperationId(): void
+    {
+        $sut = new RouteCollector();
+        $openApi = Reader::readFromYamlFile(self::FIXTURES . 'duplicateOperationId.yaml');
+
+        self::expectExceptionObject(CannotProcessOpenAPI::duplicateOperationId(
+            'operation1',
+            ['path' => '/path', 'operation' => 'get'],
+            ['path' => '/path', 'operation' => 'delete'],
+        ));
+
+        $sut->collect($openApi);
+    }
+
     public static function collectTestProvider(): array
     {
         return [

diff --git a/tests/fixtures/duplicateOperationId.yaml b/tests/fixtures/duplicateOperationId.yaml
@@ -0,0 +1,39 @@
+---
+openapi: 3.0.0
+info:
+  title: Test API
+  version: 1.0.0
+servers:
+  - url: http://test.com
+paths:
+  "/path":
+    get:
+      operationId: operation1
+      parameters:
+        - name: id
+          in: header
+          required: true
+          schema:
+            type: integer
+      responses:
+        '200':
+          description: Successful Response
+          content:
+            application/json:
+              schema:
+                type: integer
+    delete:
+      operationId: operation1
+      parameters:
+        - name: id
+          in: header
+          required: true
+          schema:
+            type: integer
+      responses:
+        '204':
+          description: Successful Response
+          content:
+            application/json:
+              schema:
+                type: integer
diff --git a/tests/fixtures/missingOperationId.yaml b/tests/fixtures/missingOperationId.yaml
@@ -0,0 +1,23 @@
+---
+openapi: 3.0.0
+info:
+  title: Test API
+  version: 1.0.0
+servers:
+  - url: http://test.com
+paths:
+  "/path":
+    get:
+      parameters:
+        - name: id
+          in: header
+          required: true
+          schema:
+            type: integer
+      responses:
+        '200':
+          description: Successful Response
+          content:
+            application/json:
+              schema:
+                type: integer