Merge pull request #114 from nextcloud/fix/non-unique-operation-ids

nextcloud · Apr 9, 2024 · aa4c24f · aa4c24f
2 parents 570cc3e + 574fb44
commit aa4c24f
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 14 deletions.
diff --git 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";
@@ -523,13 +525,26 @@ foreach ($parsedRoutes as $key => $value) {
 			continue;
 		}
 
+		$operationId = [
+			$tagName,
+			...Helpers::splitOnUppercaseFollowedByNonUppercase($methodName)
+		];
+		if ($postfix !== null) {
+			$operationId[] = $postfix;
+		}
+		$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(
 				$routeName,
 				$routeTags[$scope] ?? [$tagName],
-				$methodName,
-				$postfix,
+				$operationId,
 				$verb,
 				$url,
 				$requirements,
@@ -714,11 +729,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 +744,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
@@ -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
@@ -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
@@ -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": [