From 77da8e27964416ce596bf9e7355cc991531adb86 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 9 Apr 2024 11:31:34 +0200 Subject: [PATCH 1/2] fix: Stop using tags in operationIds Signed-off-by: provokateurin --- generate-spec | 19 +++++++++++-------- src/Route.php | 3 +-- tests/openapi-administration.json | 4 ++-- tests/openapi-full.json | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/generate-spec b/generate-spec index 9131bda..0be1461 100755 --- a/generate-spec +++ b/generate-spec @@ -523,13 +523,21 @@ foreach ($parsedRoutes as $key => $value) { continue; } + $operationId = [ + $tagName, + ...Helpers::splitOnUppercaseFollowedByNonUppercase($methodName) + ]; + if ($postfix !== null) { + $operationId[] = $postfix; + } + $operationId = strtolower(implode("-", $operationId)); + foreach ($scopes as $scope) { $routes[$scope] ??= []; $routes[$scope][] = new Route( $routeName, $routeTags[$scope] ?? [$tagName], - $methodName, - $postfix, + $operationId, $verb, $url, $requirements, @@ -714,11 +722,6 @@ foreach ($routes as $scope => $scopeRoutes) { $mergedResponses[$statusCode] = $response; } - $operationId = [...$route->tags, ...Helpers::splitOnUppercaseFollowedByNonUppercase($route->methodName)]; - if ($route->postfix != null) { - $operationId[] = $route->postfix; - } - $security = []; if ($route->isPublic) { // Add empty authentication, meaning that it's optional. We can't know if there is a difference in behaviour for authenticated vs. unauthenticated access on public pages (e.g. capabilities) @@ -734,7 +737,7 @@ foreach ($routes as $scope => $scopeRoutes) { } $operation = [ - "operationId" => strtolower(implode("-", $operationId)), + "operationId" => $route->operationId, ]; if ($route->controllerMethod->summary !== null) { $operation["summary"] = $route->controllerMethod->summary; diff --git a/src/Route.php b/src/Route.php index 2df871a..ea511ec 100644 --- a/src/Route.php +++ b/src/Route.php @@ -6,8 +6,7 @@ class Route { public function __construct( public string $name, public array $tags, - public string $methodName, - public ?string $postfix, + public string $operationId, public string $verb, public string $url, public array $requirements, diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index d1bcdf2..5fd4a69 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -154,7 +154,7 @@ }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/moved-with-tag": { "post": { - "operationId": "settings-admin-settings-moved-to-settings-tag", + "operationId": "admin_settings-moved-to-settings-tag", "summary": "Route in default scope with tags", "description": "This endpoint requires admin access", "tags": [ @@ -227,7 +227,7 @@ }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/moved-with-unnamed-tag": { "post": { - "operationId": "settings-admin-settings-moved-to-settings-tag-unnamed", + "operationId": "admin_settings-moved-to-settings-tag-unnamed", "summary": "Route in default scope with tags but without named parameters on the attribute", "description": "This endpoint requires admin access", "tags": [ diff --git a/tests/openapi-full.json b/tests/openapi-full.json index f4c88bc..19f203b 100644 --- a/tests/openapi-full.json +++ b/tests/openapi-full.json @@ -281,7 +281,7 @@ }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/moved-with-tag": { "post": { - "operationId": "settings-admin-settings-moved-to-settings-tag", + "operationId": "admin_settings-moved-to-settings-tag", "summary": "Route in default scope with tags", "description": "This endpoint requires admin access", "tags": [ @@ -354,7 +354,7 @@ }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/moved-with-unnamed-tag": { "post": { - "operationId": "settings-admin-settings-moved-to-settings-tag-unnamed", + "operationId": "admin_settings-moved-to-settings-tag-unnamed", "summary": "Route in default scope with tags but without named parameters on the attribute", "description": "This endpoint requires admin access", "tags": [ From 574fb44a57bc07f19ad5feacab4c7123a5811884 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 9 Apr 2024 11:36:09 +0200 Subject: [PATCH 2/2] fix: Ensure operationIds are unique Signed-off-by: provokateurin --- generate-spec | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/generate-spec b/generate-spec index 0be1461..8a0ed10 100755 --- a/generate-spec +++ b/generate-spec @@ -329,6 +329,8 @@ if (count($parsedRoutes) === 0) { Logger::warning("Routes", "No routes were loaded"); } +$operationIds = []; + foreach ($parsedRoutes as $key => $value) { $isOCS = $key === "ocs"; $isIndex = $key === "routes"; @@ -532,6 +534,11 @@ foreach ($parsedRoutes as $key => $value) { } $operationId = strtolower(implode("-", $operationId)); + if (in_array($operationId, $operationIds, true)) { + Logger::panic($routeName, 'Route is not unique! If you want to have two routes pointing to the same controller method you must specify a postfix on at least one of the routes.'); + } + $operationIds[] = $operationId; + foreach ($scopes as $scope) { $routes[$scope] ??= []; $routes[$scope][] = new Route(