fix: Generate operationids properly (#2266)

| Q | A |

|---------------|---------------------------------------------------------------------------------------------------------------------------|
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no                                                  |
| Issues        | --- |


*Current behaviour*
- OperationId is not generated when only using symfony attributes, e.g.
#[Route]
- OperationId is not generated when only using one OA attributes, eg.
#[Get] in combination with routes
- However, some combination results in properly generated OperationIds,
e.g. using #[Security] in combination with
   #[Get] attributes.

*Expected behaviour*
- In all cases an operationId gets generated from the route information
(unless it is explicitly set).

*Implementation*
Test cases have been added for those scenarios. The existing
OpenApiPhpDescriber has been updated to always reach the part where the
operationId is generated.

---------

Co-authored-by: Simon Rothfahl <simon@rothfahl.ch>

src/Describer/OpenApiPhpDescriber.php

@@ -90,10 +90,6 @@ public function describe(OA\OpenApi $api): void
 
             $annotations = array_merge($annotations, $this->getAttributesAsAnnotation($method, $context));
 
-            if (0 === count($annotations) && 0 === count($classAnnotations[$declaringClass->getName()])) {
-                continue;
-            }
-
             $implicitAnnotations = [];
             $mergeProperties = new \stdClass();
 
@@ -154,14 +150,14 @@ public function describe(OA\OpenApi $api): void
                 $implicitAnnotations[] = $annotation;
             }
 
-            if ([] === $implicitAnnotations && [] === get_object_vars($mergeProperties)) {
-                continue;
-            }
-
             foreach ($httpMethods as $httpMethod) {
                 $operation = Util::getOperation($path, $httpMethod);
-                $operation->merge($implicitAnnotations);
-                $operation->mergeProperties($mergeProperties);
+                if ([] !== $implicitAnnotations) {
+                    $operation->merge($implicitAnnotations);
+                }
+                if ([] !== get_object_vars($mergeProperties)) {
+                    $operation->mergeProperties($mergeProperties);
+                }
 
                 if (Generator::UNDEFINED === $operation->operationId) {
                     $operation->operationId = $httpMethod.'_'.$routeName;

tests/Functional/Controller/OperationIdController.php

@@ -0,0 +1,69 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;
+
+use Nelmio\ApiDocBundle\Annotation\Security;
+use OpenApi\Attributes as OA;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\Routing\Annotation\Route;
+
+/*
+ * Not all operationIds were generated properly. This test case covers the following scenarios:
+ * - routes with unnamed symfony route annotations (fall back to SF naming strategy)
+ * - routes combined with OA\Get annotations
+ * - routes with OA\Get annotations and additional ApiDoc root annotations
+ * - routes with operationId explicitly set
+
+ */
+class OperationIdController
+{
+    // a route with only a symfony route annotation (generates GET Operation using available metadata as operationId)
+    #[Route(path: '/generate/operation_id_route_unnamed', methods: 'GET')]
+    public function getMustGenerateOperationIdByUnnamedRouteAnnotation(): JsonResponse
+    {
+        return new JsonResponse();
+    }
+
+    // a route with a named symfony route annotation (generates GET Operation using route name as operationId)
+    #[Route(path: '/generate/operation_id_route', name: 'named_route', methods: 'GET')]
+    public function getMustGenerateOperationIdByRouteAnnotation(): JsonResponse
+    {
+        return new JsonResponse();
+    }
+
+    // a route with an OA\Get annotation and a separate ApiDoc Annotation(extends GET operation with operationId)
+    #[Route(path: '/generate/operation_id_with_security/', name: 'with_security', methods: 'GET')]
+    #[OA\Get('OperationId must be generated automatically when additional OA/nelmio root annotations are present')]
+    #[Security(name: 'bearerAuth')] // additioanl root annotations
+    public function getWithAdditionalAnnotationsGeneratesOperationId(): JsonResponse
+    {
+        return new JsonResponse();
+    }
+
+    // a route with an OA\GET annotation (extends GET operation with operationId)
+    #[Route(path: '/generate/operation_id_get', name: 'get_annotation', methods: 'GET')]
+    #[OA\Get(summary: 'OperationId must be generated automatically if not provided')]
+    public function getMustGenerateOperationIByGetAnnotation(): JsonResponse
+    {
+        return new JsonResponse();
+    }
+
+    // custom operationId is used when set explicitly
+    #[Route(path: '/has/explicit/operationid', name: 'customOperationId', methods: 'GET')]
+    #[OA\Get(summary: 'Custom operation id must be used if provided', operationId: 'customOperationId')]
+    public function getWithCustomOperationId(): JsonResponse
+    {
+        return new JsonResponse();
+    }
+}

tests/Functional/ControllerTest.php

@@ -122,6 +122,13 @@ public static function provideAttributeTestCases(): \Generator
                 'MapQueryStringCleanupComponents',
                 [__DIR__.'/Configs/CleanUnusedComponentsProcessor.yaml'],
             ];
+
+            yield 'operationId must always be generated' => [
+                [
+                    'name' => 'OperationIdController',
+                    'type' => $type,
+                ],
+            ];
         }
     }