From cd01946f575ac058f9c51f80a12cdc26e1872f91 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 28 Oct 2023 17:33:51 +0200 Subject: [PATCH 1/8] feat(scopes): Allow apps to define different API scopes for different target clients Signed-off-by: Joas Schilling --- .editorconfig | 14 ++ generate-spec | 427 ++++++++++++++++++++++++++---------------------- src/Helpers.php | 37 +++++ 3 files changed, 287 insertions(+), 191 deletions(-) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..35630aa --- /dev/null +++ b/.editorconfig @@ -0,0 +1,14 @@ +# https://editorconfig.org + +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 4 +indent_style = tab +insert_final_newline = true +trim_trailing_whitespace = true + +[*.md] +trim_trailing_whitespace = false diff --git a/generate-spec b/generate-spec index fa80019..7e345bd 100755 --- a/generate-spec +++ b/generate-spec @@ -277,6 +277,12 @@ foreach ($parsedRoutes as $key => $value) { continue; } + $controllerScope = Helpers::getAttributeScope($controllerClass, 'OpenAPI', $routeName); + if ($controllerScope === 'ignore') { + Logger::info($routeName, "Controller '" . $controllerName . "' ignored because of OpenAPI attribute"); + continue; + } + $tagName = implode("_", array_map(fn(string $s) => strtolower($s), Helpers::splitOnUppercaseFollowedByNonUppercase($controllerName))); $doc = $controllerClass->getDocComment()?->getText(); if ($doc != null && count(array_filter($tags, fn(array $tag) => $tag["name"] == $tagName)) == 0) { @@ -331,6 +337,20 @@ foreach ($parsedRoutes as $key => $value) { continue; } + $scope = Helpers::getAttributeScope($classMethod, 'OpenAPI', $routeName); + if ($scope === 'ignore') { + Logger::info($routeName, "Route ignored because of OpenAPI attribute"); + continue; + } + + if ($scope === null) { + if ($controllerScope !== null) { + $scope = $controllerScope; + } else { + $scope = 'default'; + } + } + if ($isOCS && !array_key_exists("OCSMeta", $schemas)) { $schemas["OCSMeta"] = [ "type" => "object", @@ -378,7 +398,8 @@ foreach ($parsedRoutes as $key => $value) { continue; } - $routes[] = new Route( + $routes[$scope] ??= []; + $routes[$scope][] = new Route( $routeName, $tagName, $controllerName, @@ -400,220 +421,225 @@ foreach ($parsedRoutes as $key => $value) { $tagNames = []; if ($useTags) { - foreach ($routes as $route) { - if (!in_array($route->tag, $tagNames)) { - $tagNames[] = $route->tag; + foreach ($routes as $scope => $scopeRoutes) { + foreach ($scopeRoutes as $route) { + if (!in_array($route->tag, $tagNames)) { + $tagNames[] = $route->tag; + } } } } -foreach ($routes as $route) { - $pathParameters = []; - $urlParameters = []; - - preg_match_all("/{[^}]*}/", $route->url, $urlParameters); - $urlParameters = array_map(fn(string $name) => substr($name, 1, -1), $urlParameters[0]); - - foreach ($urlParameters as $urlParameter) { - $matchingParameters = array_filter($route->controllerMethod->parameters, function (ControllerMethodParameter $param) use ($urlParameter) { - return $param->name == $urlParameter; - }); - $requirement = array_key_exists($urlParameter, $route->requirements) ? $route->requirements[$urlParameter] : null; - if (count($matchingParameters) == 1) { - $parameter = $matchingParameters[array_keys($matchingParameters)[0]]; - if ($parameter?->methodParameter == null && ($route->requirements == null || !array_key_exists($urlParameter, $route->requirements))) { - Logger::error($route->name, "Unable to find parameter for '" . $urlParameter . "'"); - continue; - } - - $schema = $parameter->type->toArray($openapiVersion, true); - $description = $parameter?->docParameter != null && $parameter->docParameter->description != "" ? Helpers::cleanDocComment($parameter->docParameter->description) : null; - } else { - $schema = [ - "type" => "string", - ]; - $description = null; - } +$scopePaths = []; + +foreach ($routes as $scope => $scopeRoutes) { + foreach ($scopeRoutes as $route) { + $pathParameters = []; + $urlParameters = []; + + preg_match_all("/{[^}]*}/", $route->url, $urlParameters); + $urlParameters = array_map(fn(string $name) => substr($name, 1, -1), $urlParameters[0]); + + foreach ($urlParameters as $urlParameter) { + $matchingParameters = array_filter($route->controllerMethod->parameters, function (ControllerMethodParameter $param) use ($urlParameter) { + return $param->name == $urlParameter; + }); + $requirement = array_key_exists($urlParameter, $route->requirements) ? $route->requirements[$urlParameter] : null; + if (count($matchingParameters) == 1) { + $parameter = $matchingParameters[array_keys($matchingParameters)[0]]; + if ($parameter?->methodParameter == null && ($route->requirements == null || !array_key_exists($urlParameter, $route->requirements))) { + Logger::error($route->name, "Unable to find parameter for '" . $urlParameter . "'"); + continue; + } - if ($requirement != null) { - if (!str_starts_with($requirement, "^")) { - $requirement = "^" . $requirement; - } - if (!str_ends_with($requirement, "$")) { - $requirement = $requirement . "$"; + $schema = $parameter->type->toArray($openapiVersion, true); + $description = $parameter?->docParameter != null && $parameter->docParameter->description != "" ? Helpers::cleanDocComment($parameter->docParameter->description) : null; + } else { + $schema = [ + "type" => "string", + ]; + $description = null; } - } - if ($schema["type"] == "string") { - if ($urlParameter == "apiVersion") { - if ($requirement == null) { - Logger::error($route->name, "Missing requirement for apiVersion"); - continue; + if ($requirement != null) { + if (!str_starts_with($requirement, "^")) { + $requirement = "^" . $requirement; } - preg_match("/^\^\(([v0-9-.|]*)\)\\$$/m", $requirement, $matches); - if (count($matches) == 2) { - $enum = explode("|", $matches[1]); - } else { - Logger::error($route->name, "Invalid requirement for apiVersion"); - continue; + if (!str_ends_with($requirement, "$")) { + $requirement = $requirement . "$"; } - $schema["enum"] = $enum; - $schema["default"] = end($enum); - } else if ($requirement != null) { - $schema["pattern"] = $requirement; } - } - $pathParameters[] = array_merge( - [ - "name" => $urlParameter, - "in" => "path", - ], - $description != null ? ["description" => $description] : [], - [ - "required" => true, - "schema" => $schema, - ], - ); - } - - $queryParameters = []; - foreach ($route->controllerMethod->parameters as $parameter) { - $alreadyInPath = false; - foreach ($pathParameters as $pathParameter) { - if ($pathParameter["name"] == $parameter->name) { - $alreadyInPath = true; - break; + if ($schema["type"] == "string") { + if ($urlParameter == "apiVersion") { + if ($requirement == null) { + Logger::error($route->name, "Missing requirement for apiVersion"); + continue; + } + preg_match("/^\^\(([v0-9-.|]*)\)\\$$/m", $requirement, $matches); + if (count($matches) == 2) { + $enum = explode("|", $matches[1]); + } else { + Logger::error($route->name, "Invalid requirement for apiVersion"); + continue; + } + $schema["enum"] = $enum; + $schema["default"] = end($enum); + } else if ($requirement != null) { + $schema["pattern"] = $requirement; + } } - } - if (!$alreadyInPath) { - $queryParameters[] = $parameter; - } - } - $mergedResponses = []; - foreach (array_unique(array_map(fn(ControllerMethodResponse $response) => $response->statusCode, array_filter($route->controllerMethod->responses, fn(?ControllerMethodResponse $response) => $response != null))) as $statusCode) { - if ($firstStatusCode && count($mergedResponses) > 0) { - break; + $pathParameters[] = array_merge( + [ + "name" => $urlParameter, + "in" => "path", + ], + $description != null ? ["description" => $description] : [], + [ + "required" => true, + "schema" => $schema, + ], + ); } - $statusCodeResponses = array_filter($route->controllerMethod->responses, fn(?ControllerMethodResponse $response) => $response != null && $response->statusCode == $statusCode); - $headers = array_merge(...array_map(fn(ControllerMethodResponse $response) => $response->headers ?? [], $statusCodeResponses)); + $queryParameters = []; + foreach ($route->controllerMethod->parameters as $parameter) { + $alreadyInPath = false; + foreach ($pathParameters as $pathParameter) { + if ($pathParameter["name"] == $parameter->name) { + $alreadyInPath = true; + break; + } + } + if (!$alreadyInPath) { + $queryParameters[] = $parameter; + } + } - $mergedContentTypeResponses = []; - foreach (array_unique(array_map(fn(ControllerMethodResponse $response) => $response->contentType, array_filter($statusCodeResponses, fn(ControllerMethodResponse $response) => $response->contentType != null))) as $contentType) { - if ($firstContentType && count($mergedContentTypeResponses) > 0) { + $mergedResponses = []; + foreach (array_unique(array_map(fn(ControllerMethodResponse $response) => $response->statusCode, array_filter($route->controllerMethod->responses, fn(?ControllerMethodResponse $response) => $response != null))) as $statusCode) { + if ($firstStatusCode && count($mergedResponses) > 0) { break; } - /** @var ControllerMethodResponse[] $contentTypeResponses */ - $contentTypeResponses = array_values(array_filter($statusCodeResponses, fn(ControllerMethodResponse $response) => $response->contentType == $contentType)); + $statusCodeResponses = array_filter($route->controllerMethod->responses, fn(?ControllerMethodResponse $response) => $response != null && $response->statusCode == $statusCode); + $headers = array_merge(...array_map(fn(ControllerMethodResponse $response) => $response->headers ?? [], $statusCodeResponses)); - $hasEmpty = count(array_filter($contentTypeResponses, fn(ControllerMethodResponse $response) => $response->type == null)) > 0; - $uniqueResponses = array_values(array_intersect_key($contentTypeResponses, array_unique(array_map(fn(ControllerMethodResponse $response) => $response->type->toArray($openapiVersion), array_filter($contentTypeResponses, fn(ControllerMethodResponse $response) => $response->type != null)), SORT_REGULAR))); - if (count($uniqueResponses) == 1) { - if ($hasEmpty) { - $mergedContentTypeResponses[$contentType] = []; + $mergedContentTypeResponses = []; + foreach (array_unique(array_map(fn(ControllerMethodResponse $response) => $response->contentType, array_filter($statusCodeResponses, fn(ControllerMethodResponse $response) => $response->contentType != null))) as $contentType) { + if ($firstContentType && count($mergedContentTypeResponses) > 0) { + break; + } + + /** @var ControllerMethodResponse[] $contentTypeResponses */ + $contentTypeResponses = array_values(array_filter($statusCodeResponses, fn(ControllerMethodResponse $response) => $response->contentType == $contentType)); + + $hasEmpty = count(array_filter($contentTypeResponses, fn(ControllerMethodResponse $response) => $response->type == null)) > 0; + $uniqueResponses = array_values(array_intersect_key($contentTypeResponses, array_unique(array_map(fn(ControllerMethodResponse $response) => $response->type->toArray($openapiVersion), array_filter($contentTypeResponses, fn(ControllerMethodResponse $response) => $response->type != null)), SORT_REGULAR))); + if (count($uniqueResponses) == 1) { + if ($hasEmpty) { + $mergedContentTypeResponses[$contentType] = []; + } else { + $schema = Helpers::cleanEmptyResponseArray($contentTypeResponses[0]->type->toArray($openapiVersion)); + $mergedContentTypeResponses[$contentType] = ["schema" => Helpers::wrapOCSResponse($route, $contentTypeResponses[0], $schema)]; + } } else { - $schema = Helpers::cleanEmptyResponseArray($contentTypeResponses[0]->type->toArray($openapiVersion)); - $mergedContentTypeResponses[$contentType] = ["schema" => Helpers::wrapOCSResponse($route, $contentTypeResponses[0], $schema)]; + $mergedContentTypeResponses[$contentType] = [ + "schema" => [ + [$hasEmpty ? "anyOf" : "oneOf" => array_map(function (ControllerMethodResponse $response) use ($route, $openapiVersion) { + $schema = Helpers::cleanEmptyResponseArray($response->type->toArray($openapiVersion)); + return Helpers::wrapOCSResponse($route, $response, $schema); + }, $uniqueResponses)], + ], + ]; } - } else { - $mergedContentTypeResponses[$contentType] = [ - "schema" => [ - [$hasEmpty ? "anyOf" : "oneOf" => array_map(function (ControllerMethodResponse $response) use ($route, $openapiVersion) { - $schema = Helpers::cleanEmptyResponseArray($response->type->toArray($openapiVersion)); - return Helpers::wrapOCSResponse($route, $response, $schema); - }, $uniqueResponses)], - ], - ]; } - } - $mergedResponses[$statusCode] = array_merge( - [ - "description" => array_key_exists($statusCode, $route->controllerMethod->responseDescription) ? $route->controllerMethod->responseDescription[$statusCode] : "", - ], - count($headers) > 0 ? [ - "headers" => array_combine( - array_keys($headers), - array_map( - fn(OpenApiType $type) => [ - "schema" => $type->toArray($openapiVersion), - ], - array_values($headers), + $mergedResponses[$statusCode] = array_merge( + [ + "description" => array_key_exists($statusCode, $route->controllerMethod->responseDescription) ? $route->controllerMethod->responseDescription[$statusCode] : "", + ], + count($headers) > 0 ? [ + "headers" => array_combine( + array_keys($headers), + array_map( + fn(OpenApiType $type) => [ + "schema" => $type->toArray($openapiVersion), + ], + array_values($headers), + ), ), + ] : [], + count($mergedContentTypeResponses) > 0 ? [ + "content" => $mergedContentTypeResponses, + ] : [], + ); + } + + $operationId = [$route->tag]; + $operationId = array_merge($operationId, array_map(fn(string $s) => Helpers::mapVerb(strtolower($s)), 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) + $security[] = new stdClass(); + } + if (!$route->isCORS) { + // Bearer auth is not allowed on CORS routes + $security[] = ["bearer_auth" => []]; + } + if (!$route->isCSRFRequired || $route->isOCS) { + // Add basic auth last, so it's only fallback if bearer is available + $security[] = ["basic_auth" => []]; + } + + $operation = array_merge( + ["operationId" => implode("-", $operationId)], + $route->controllerMethod->summary != null ? ["summary" => $route->controllerMethod->summary] : [], + count($route->controllerMethod->description) > 0 ? ["description" => implode("\n", $route->controllerMethod->description)] : [], + $route->controllerMethod->isDeprecated ? ["deprecated" => true] : [], + $useTags ? ["tags" => [$route->tag]] : [], + count($security) > 0 ? ["security" => $security] : [], + count($queryParameters) > 0 || count($pathParameters) > 0 || $route->isOCS ? [ + "parameters" => array_merge( + array_map(fn(ControllerMethodParameter $parameter) => array_merge( + [ + "name" => $parameter->name . ($parameter->type->type == "array" ? "[]" : ""), + "in" => "query", + ], + $parameter->docParameter != null && $parameter->docParameter->description != "" ? ["description" => Helpers::cleanDocComment($parameter->docParameter->description)] : [], + !$parameter->type->nullable && !$parameter->type->hasDefaultValue ? ["required" => true] : [], + ["schema" => $parameter->type->toArray($openapiVersion, true),], + ), $queryParameters), + $pathParameters, + $route->isOCS ? [[ + "name" => "OCS-APIRequest", + "in" => "header", + "description" => "Required to be true for the API request to pass", + "required" => true, + "schema" => [ + "type" => "boolean", + "default" => true, + ], + ]] : [], ), ] : [], - count($mergedContentTypeResponses) > 0 ? [ - "content" => $mergedContentTypeResponses, - ] : [], + ["responses" => $mergedResponses], ); - } - $operationId = [$route->tag]; - $operationId = array_merge($operationId, array_map(fn(string $s) => Helpers::mapVerb(strtolower($s)), Helpers::splitOnUppercaseFollowedByNonUppercase($route->methodName))); - if ($route->postfix != null) { - $operationId[] = $route->postfix; - } + $scopePaths[$scope] ??= []; + $scopePaths[$scope][$route->url] ??= []; - $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) - $security[] = new stdClass(); - } - if (!$route->isCORS) { - // Bearer auth is not allowed on CORS routes - $security[] = ["bearer_auth" => []]; - } - if (!$route->isCSRFRequired || $route->isOCS) { - // Add basic auth last, so it's only fallback if bearer is available - $security[] = ["basic_auth" => []]; - } - - $operation = array_merge( - ["operationId" => implode("-", $operationId)], - $route->controllerMethod->summary != null ? ["summary" => $route->controllerMethod->summary] : [], - count($route->controllerMethod->description) > 0 ? ["description" => implode("\n", $route->controllerMethod->description)] : [], - $route->controllerMethod->isDeprecated ? ["deprecated" => true] : [], - $useTags ? ["tags" => [$route->tag]] : [], - count($security) > 0 ? ["security" => $security] : [], - count($queryParameters) > 0 || count($pathParameters) > 0 || $route->isOCS ? [ - "parameters" => array_merge( - array_map(fn(ControllerMethodParameter $parameter) => array_merge( - [ - "name" => $parameter->name . ($parameter->type->type == "array" ? "[]" : ""), - "in" => "query", - ], - $parameter->docParameter != null && $parameter->docParameter->description != "" ? ["description" => Helpers::cleanDocComment($parameter->docParameter->description)] : [], - !$parameter->type->nullable && !$parameter->type->hasDefaultValue ? ["required" => true] : [], - ["schema" => $parameter->type->toArray($openapiVersion, true),], - ), $queryParameters), - $pathParameters, - $route->isOCS ? [[ - "name" => "OCS-APIRequest", - "in" => "header", - "description" => "Required to be true for the API request to pass", - "required" => true, - "schema" => [ - "type" => "boolean", - "default" => true, - ], - ]] : [], - ), - ] : [], - ["responses" => $mergedResponses], - ); - if (!array_key_exists($route->url, $openapi["paths"])) { - $openapi["paths"][$route->url] = []; - } - $path = &$openapi["paths"][$route->url]; - - $verb = strtolower($route->verb); - if (!array_key_exists($verb, $path)) { - $path[$verb] = $operation; - } else { - Logger::error($route->name, "Operation '" . $route->verb . "' already set for path '" . $route->url . "'"); + $verb = strtolower($route->verb); + if (!array_key_exists($verb, $scopePaths[$scope][$route->url])) { + $scopePaths[$scope][$route->url][$verb] = $operation; + } else { + Logger::error($route->name, "Operation '" . $route->verb . "' already set for path '" . $route->url . "'"); + } } } @@ -657,7 +683,7 @@ if ($appIsCore) { ], ], ]; - $openapi["paths"]["/status.php"] = [ + $scopePaths['default']['/status.php'] = [ "get" => [ "operationId" => "get-status", "responses" => [ @@ -680,10 +706,6 @@ if (count($schemas) == 0 && count($routes) == 0) { Logger::error("app", "No spec generated"); } -if (count($openapi["paths"]) == 0) { - $openapi["paths"] = new stdClass(); -} - ksort($schemas); $openapi["components"]["schemas"] = count($schemas) == 0 ? new stdClass() : $schemas; @@ -691,7 +713,30 @@ if ($useTags) { $openapi["tags"] = $tags; } -file_put_contents($out, json_encode($openapi, Helpers::jsonFlags())); +foreach ($scopePaths as $scope => $paths) { + $openapiScope = $openapi; + + if (count($paths) == 0) { + $paths = new stdClass(); + } + + $scopeSuffix = $scope === 'default' ? '' : '-' . $scope; + $openapiScope['info']['title'] .= $scopeSuffix; + $openapiScope['paths'] = $paths; + + $startExtension = strrpos($out, '.'); + if ($startExtension !== false) { + // Path + filename (without extension) + $path = substr($out, 0, $startExtension); + // Extension + $extension = substr($out, $startExtension); + $scopeOut = $path . $scopeSuffix . $extension; + } else { + $scopeOut = $out . $scopeSuffix; + } + + file_put_contents($scopeOut, json_encode($openapiScope, Helpers::jsonFlags())); +} function cleanSchemaName(string $name): string { global $readableAppID; diff --git a/src/Helpers.php b/src/Helpers.php index a29d044..3513669 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -4,6 +4,8 @@ use Exception; use PhpParser\Node; +use PhpParser\Node\Expr\ClassConstFetch; +use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Class_; use stdClass; @@ -148,4 +150,39 @@ static function classMethodHasAnnotationOrAttribute(ClassMethod|Class_|Node $nod return false; } + + static function getAttributeScope(ClassMethod|Class_|Node $node, string $annotation, string $routeName): ?string { + /** @var Node\AttributeGroup $attrGroup */ + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($attr->name->getLast() === $annotation) { + if (empty($attr->args)) { + return 'default'; + } + + foreach ($attr->args as $arg) { + if ($arg->name->name === 'scope') { + if ($arg->value instanceof ClassConstFetch) { + if ($arg->value->class->getLast() === 'OpenAPI') { + return match ($arg->value->name->name) { + 'SCOPE_DEFAULT' => 'default', + 'SCOPE_ADMINISTRATION' => 'administration', + 'SCOPE_FEDERATION' => 'federation', + 'SCOPE_IGNORE' => 'ignore', + // Fall back for future scopes assuming we follow the pattern (cut of 'SCOPE_' and lower case) + default => strtolower(substr($arg->value->name->name, 6)), + }; + } + } elseif ($arg->value instanceof String_) { + return $arg->value->value; + } + Logger::panic($routeName, 'Can not interpret value of scope provided in OpenAPI(scope: …) attribute. Please use string or OpenAPI::SCOPE_* constants'); + } + } + } + } + } + + return null; + } } From 7f6cefbc6f2bde770719f4548dc4b3f706454c79 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Nov 2023 15:55:03 +0100 Subject: [PATCH 2/8] Allow multiple scopes Signed-off-by: Joas Schilling --- generate-spec | 50 +-- src/Helpers.php | 16 +- tests/appinfo/info.xml | 52 +++ tests/appinfo/routes.php | 36 ++ tests/lib/Controller/SettingsController.php | 118 ++++++ tests/lib/ResponseDefinitions.php | 63 ++++ tests/openapi-administration.json | 311 ++++++++++++++++ tests/openapi-federation.json | 240 ++++++++++++ tests/openapi.json | 383 ++++++++++++++++++++ 9 files changed, 1239 insertions(+), 30 deletions(-) create mode 100644 tests/appinfo/info.xml create mode 100644 tests/appinfo/routes.php create mode 100644 tests/lib/Controller/SettingsController.php create mode 100644 tests/lib/ResponseDefinitions.php create mode 100644 tests/openapi-administration.json create mode 100644 tests/openapi-federation.json create mode 100644 tests/openapi.json diff --git a/generate-spec b/generate-spec index 7e345bd..227eece 100755 --- a/generate-spec +++ b/generate-spec @@ -277,8 +277,8 @@ foreach ($parsedRoutes as $key => $value) { continue; } - $controllerScope = Helpers::getAttributeScope($controllerClass, 'OpenAPI', $routeName); - if ($controllerScope === 'ignore') { + $controllerScopes = Helpers::getAttributeScopes($controllerClass, 'OpenAPI', $routeName); + if (in_array('ignore', $controllerScopes, true)) { Logger::info($routeName, "Controller '" . $controllerName . "' ignored because of OpenAPI attribute"); continue; } @@ -337,17 +337,17 @@ foreach ($parsedRoutes as $key => $value) { continue; } - $scope = Helpers::getAttributeScope($classMethod, 'OpenAPI', $routeName); - if ($scope === 'ignore') { + $scopes = Helpers::getAttributeScopes($classMethod, 'OpenAPI', $routeName); + if (in_array('ignore', $scopes, true)) { Logger::info($routeName, "Route ignored because of OpenAPI attribute"); continue; } - if ($scope === null) { - if ($controllerScope !== null) { - $scope = $controllerScope; + if (empty($scopes)) { + if (!empty($controllerScopes)) { + $scopes = $controllerScopes; } else { - $scope = 'default'; + $scopes = ['default']; } } @@ -398,22 +398,24 @@ foreach ($parsedRoutes as $key => $value) { continue; } - $routes[$scope] ??= []; - $routes[$scope][] = new Route( - $routeName, - $tagName, - $controllerName, - $methodName, - $postfix, - $verb, - $url, - $requirements, - $classMethodInfo, - $isOCS, - $isCORS, - $isCSRFRequired, - $isPublic, - ); + foreach ($scopes as $scope) { + $routes[$scope] ??= []; + $routes[$scope][] = new Route( + $routeName, + $tagName, + $controllerName, + $methodName, + $postfix, + $verb, + $url, + $requirements, + $classMethodInfo, + $isOCS, + $isCORS, + $isCSRFRequired, + $isPublic, + ); + } Logger::info($routeName, "Route generated"); } diff --git a/src/Helpers.php b/src/Helpers.php index 3513669..ff8738e 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -151,20 +151,23 @@ static function classMethodHasAnnotationOrAttribute(ClassMethod|Class_|Node $nod return false; } - static function getAttributeScope(ClassMethod|Class_|Node $node, string $annotation, string $routeName): ?string { + static function getAttributeScopes(ClassMethod|Class_|Node $node, string $annotation, string $routeName): array { + $scopes = []; + + /** @var Node\AttributeGroup $attrGroup */ foreach ($node->attrGroups as $attrGroup) { foreach ($attrGroup->attrs as $attr) { if ($attr->name->getLast() === $annotation) { if (empty($attr->args)) { - return 'default'; + $scopes[] = 'default'; } foreach ($attr->args as $arg) { if ($arg->name->name === 'scope') { if ($arg->value instanceof ClassConstFetch) { if ($arg->value->class->getLast() === 'OpenAPI') { - return match ($arg->value->name->name) { + $scopes[] = match ($arg->value->name->name) { 'SCOPE_DEFAULT' => 'default', 'SCOPE_ADMINISTRATION' => 'administration', 'SCOPE_FEDERATION' => 'federation', @@ -174,15 +177,16 @@ static function getAttributeScope(ClassMethod|Class_|Node $node, string $annotat }; } } elseif ($arg->value instanceof String_) { - return $arg->value->value; + $scopes[] = $arg->value->value; + } else { + Logger::panic($routeName, 'Can not interpret value of scope provided in OpenAPI(scope: …) attribute. Please use string or OpenAPI::SCOPE_* constants'); } - Logger::panic($routeName, 'Can not interpret value of scope provided in OpenAPI(scope: …) attribute. Please use string or OpenAPI::SCOPE_* constants'); } } } } } - return null; + return $scopes; } } diff --git a/tests/appinfo/info.xml b/tests/appinfo/info.xml new file mode 100644 index 0000000..7147b0e --- /dev/null +++ b/tests/appinfo/info.xml @@ -0,0 +1,52 @@ + + + notifications + Notifications + + + + 2.16.0 + agpl + Joas Schilling + + + + + + tools + + https://github.com/nextcloud/notifications + https://github.com/nextcloud/notifications/issues + https://github.com/nextcloud/notifications.git + + + + + + + OCA\Notifications\BackgroundJob\GenerateUserSettings + OCA\Notifications\BackgroundJob\SendNotificationMails + + + + OCA\Notifications\Command\Generate + OCA\Notifications\Command\TestPush + + + + OCA\Notifications\Settings\Admin + OCA\Notifications\Settings\AdminSection + OCA\Notifications\Settings\Personal + OCA\Notifications\Settings\PersonalSection + + diff --git a/tests/appinfo/routes.php b/tests/appinfo/routes.php new file mode 100644 index 0000000..764dccb --- /dev/null +++ b/tests/appinfo/routes.php @@ -0,0 +1,36 @@ + + * @copyright Copyright (c) 2016, ownCloud, Inc. + * + * @author Joas Schilling + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +return [ + 'ocs' => [ + ['name' => 'Settings#federationByController', 'url' => '/api/{apiVersion}/controller-scope', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#ignoreByMethod', 'url' => '/api/{apiVersion}/ignore-method', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#defaultScope', 'url' => '/api/{apiVersion}/settings', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#defaultAdminScope', 'url' => '/api/{apiVersion}/default-admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#adminScope', 'url' => '/api/{apiVersion}/admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#doubleScope', 'url' => '/api/{apiVersion}/double', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ], +]; diff --git a/tests/lib/Controller/SettingsController.php b/tests/lib/Controller/SettingsController.php new file mode 100644 index 0000000..fdd42a1 --- /dev/null +++ b/tests/lib/Controller/SettingsController.php @@ -0,0 +1,118 @@ + + * + * @author Julien Barnoin + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Notifications\Controller; + +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\OpenAPI; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCSController; + +#[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)] +class SettingsController extends OCSController { + + /** + * @NoAdminRequired + * + * Route is ignored because of scope on the controller + * + * @return DataResponse, array{}> + * + * 200: OK + */ + public function federationByController(): DataResponse { + return new DataResponse(); + } + + /** + * @NoAdminRequired + * + * Route is ignored because of scope on the method + * + * @return DataResponse, array{}> + * + * 200: OK + */ + #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] + public function ignoreByMethod(): DataResponse { + return new DataResponse(); + } + + /** + * @NoAdminRequired + * + * Route is only in the default scope + * + * @return DataResponse, array{}> + * + * 200: Personal settings updated + */ + #[OpenAPI] + public function defaultScope(): DataResponse { + return new DataResponse(); + } + + /** + * Route is only in the admin scope because there is no "NoAdminRequired" annotation or attribute + * + * @return DataResponse, array{}> + * + * 200: Personal settings updated + */ + #[OpenAPI] + public function defaultAdminScope(): DataResponse { + return new DataResponse(); + } + + /** + * @NoAdminRequired + * + * Route is only in the admin scope due to defined scope + * + * @return DataResponse, array{}> + * + * 200: Admin settings updated + */ + #[OpenAPI(scope: OpenAPI::SCOPE_ADMINISTRATION)] + public function adminScope(): DataResponse { + return new DataResponse(); + } + + /** + * @NoAdminRequired + * + * Route is in admin and default scope + * + * @return DataResponse, array{}> + * + * 200: Admin settings updated + */ + #[OpenAPI] + #[OpenAPI(scope: OpenAPI::SCOPE_ADMINISTRATION)] + public function doubleScope(): DataResponse { + return new DataResponse(); + } +} diff --git a/tests/lib/ResponseDefinitions.php b/tests/lib/ResponseDefinitions.php new file mode 100644 index 0000000..39df467 --- /dev/null +++ b/tests/lib/ResponseDefinitions.php @@ -0,0 +1,63 @@ + + * + * @author Kate Döen + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Notifications; + +/** + * @psalm-type NotificationsNotificationAction = array{ + * label: string, + * link: string, + * type: string, + * primary: bool, + * } + * + * @psalm-type NotificationsNotification = array{ + * notification_id: int, + * app: string, + * user: string, + * datetime: string, + * object_type: string, + * object_id: string, + * subject: string, + * message: string, + * link: string, + * actions: NotificationsNotificationAction[], + * subjectRich?: string, + * subjectRichParameters?: array, + * messageRich?: string, + * messageRichParameters?: array, + * icon?: string, + * shouldNotify?: bool, + * } + * + * @psalm-type NotificationsPushDevice = array{ + * publicKey: string, + * deviceIdentifier: string, + * signature: string, + * } + */ +class ResponseDefinitions { +} diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json new file mode 100644 index 0000000..502151a --- /dev/null +++ b/tests/openapi-administration.json @@ -0,0 +1,311 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "notifications-administration", + "version": "0.0.1", + "description": "This app provides a backend and frontend for the notification API available in Nextcloud.", + "license": { + "name": "agpl" + } + }, + "components": { + "securitySchemes": { + "basic_auth": { + "type": "http", + "scheme": "basic" + }, + "bearer_auth": { + "type": "http", + "scheme": "bearer" + } + }, + "schemas": { + "Notification": { + "type": "object", + "required": [ + "notification_id", + "app", + "user", + "datetime", + "object_type", + "object_id", + "subject", + "message", + "link", + "actions" + ], + "properties": { + "notification_id": { + "type": "integer", + "format": "int64" + }, + "app": { + "type": "string" + }, + "user": { + "type": "string" + }, + "datetime": { + "type": "string" + }, + "object_type": { + "type": "string" + }, + "object_id": { + "type": "string" + }, + "subject": { + "type": "string" + }, + "message": { + "type": "string" + }, + "link": { + "type": "string" + }, + "actions": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NotificationAction" + } + }, + "subjectRich": { + "type": "string" + }, + "subjectRichParameters": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "messageRich": { + "type": "string" + }, + "messageRichParameters": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "icon": { + "type": "string" + }, + "shouldNotify": { + "type": "boolean" + } + } + }, + "NotificationAction": { + "type": "object", + "required": [ + "label", + "link", + "type", + "primary" + ], + "properties": { + "label": { + "type": "string" + }, + "link": { + "type": "string" + }, + "type": { + "type": "string" + }, + "primary": { + "type": "boolean" + } + } + }, + "OCSMeta": { + "type": "object", + "required": [ + "status", + "statuscode" + ], + "properties": { + "status": { + "type": "string" + }, + "statuscode": { + "type": "integer" + }, + "message": { + "type": "string" + }, + "totalitems": { + "type": "string" + }, + "itemsperpage": { + "type": "string" + } + } + }, + "PushDevice": { + "type": "object", + "required": [ + "publicKey", + "deviceIdentifier", + "signature" + ], + "properties": { + "publicKey": { + "type": "string" + }, + "deviceIdentifier": { + "type": "string" + }, + "signature": { + "type": "string" + } + } + } + } + }, + "paths": { + "/ocs/v2.php/apps/notifications/api/{apiVersion}/admin": { + "post": { + "operationId": "settings-admin-scope", + "summary": "Route is only in the admin scope due to defined scope", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/double": { + "post": { + "operationId": "settings-double-scope", + "summary": "Route is in admin and default scope", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + } + }, + "tags": [] +} \ No newline at end of file diff --git a/tests/openapi-federation.json b/tests/openapi-federation.json new file mode 100644 index 0000000..2a492e3 --- /dev/null +++ b/tests/openapi-federation.json @@ -0,0 +1,240 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "notifications-federation", + "version": "0.0.1", + "description": "This app provides a backend and frontend for the notification API available in Nextcloud.", + "license": { + "name": "agpl" + } + }, + "components": { + "securitySchemes": { + "basic_auth": { + "type": "http", + "scheme": "basic" + }, + "bearer_auth": { + "type": "http", + "scheme": "bearer" + } + }, + "schemas": { + "Notification": { + "type": "object", + "required": [ + "notification_id", + "app", + "user", + "datetime", + "object_type", + "object_id", + "subject", + "message", + "link", + "actions" + ], + "properties": { + "notification_id": { + "type": "integer", + "format": "int64" + }, + "app": { + "type": "string" + }, + "user": { + "type": "string" + }, + "datetime": { + "type": "string" + }, + "object_type": { + "type": "string" + }, + "object_id": { + "type": "string" + }, + "subject": { + "type": "string" + }, + "message": { + "type": "string" + }, + "link": { + "type": "string" + }, + "actions": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NotificationAction" + } + }, + "subjectRich": { + "type": "string" + }, + "subjectRichParameters": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "messageRich": { + "type": "string" + }, + "messageRichParameters": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "icon": { + "type": "string" + }, + "shouldNotify": { + "type": "boolean" + } + } + }, + "NotificationAction": { + "type": "object", + "required": [ + "label", + "link", + "type", + "primary" + ], + "properties": { + "label": { + "type": "string" + }, + "link": { + "type": "string" + }, + "type": { + "type": "string" + }, + "primary": { + "type": "boolean" + } + } + }, + "OCSMeta": { + "type": "object", + "required": [ + "status", + "statuscode" + ], + "properties": { + "status": { + "type": "string" + }, + "statuscode": { + "type": "integer" + }, + "message": { + "type": "string" + }, + "totalitems": { + "type": "string" + }, + "itemsperpage": { + "type": "string" + } + } + }, + "PushDevice": { + "type": "object", + "required": [ + "publicKey", + "deviceIdentifier", + "signature" + ], + "properties": { + "publicKey": { + "type": "string" + }, + "deviceIdentifier": { + "type": "string" + }, + "signature": { + "type": "string" + } + } + } + } + }, + "paths": { + "/ocs/v2.php/apps/notifications/api/{apiVersion}/controller-scope": { + "post": { + "operationId": "settings-federation-by-controller", + "summary": "Route is ignored because of scope on the controller", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + } + }, + "tags": [] +} \ No newline at end of file diff --git a/tests/openapi.json b/tests/openapi.json new file mode 100644 index 0000000..de51e19 --- /dev/null +++ b/tests/openapi.json @@ -0,0 +1,383 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "notifications", + "version": "0.0.1", + "description": "This app provides a backend and frontend for the notification API available in Nextcloud.", + "license": { + "name": "agpl" + } + }, + "components": { + "securitySchemes": { + "basic_auth": { + "type": "http", + "scheme": "basic" + }, + "bearer_auth": { + "type": "http", + "scheme": "bearer" + } + }, + "schemas": { + "Notification": { + "type": "object", + "required": [ + "notification_id", + "app", + "user", + "datetime", + "object_type", + "object_id", + "subject", + "message", + "link", + "actions" + ], + "properties": { + "notification_id": { + "type": "integer", + "format": "int64" + }, + "app": { + "type": "string" + }, + "user": { + "type": "string" + }, + "datetime": { + "type": "string" + }, + "object_type": { + "type": "string" + }, + "object_id": { + "type": "string" + }, + "subject": { + "type": "string" + }, + "message": { + "type": "string" + }, + "link": { + "type": "string" + }, + "actions": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NotificationAction" + } + }, + "subjectRich": { + "type": "string" + }, + "subjectRichParameters": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "messageRich": { + "type": "string" + }, + "messageRichParameters": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "icon": { + "type": "string" + }, + "shouldNotify": { + "type": "boolean" + } + } + }, + "NotificationAction": { + "type": "object", + "required": [ + "label", + "link", + "type", + "primary" + ], + "properties": { + "label": { + "type": "string" + }, + "link": { + "type": "string" + }, + "type": { + "type": "string" + }, + "primary": { + "type": "boolean" + } + } + }, + "OCSMeta": { + "type": "object", + "required": [ + "status", + "statuscode" + ], + "properties": { + "status": { + "type": "string" + }, + "statuscode": { + "type": "integer" + }, + "message": { + "type": "string" + }, + "totalitems": { + "type": "string" + }, + "itemsperpage": { + "type": "string" + } + } + }, + "PushDevice": { + "type": "object", + "required": [ + "publicKey", + "deviceIdentifier", + "signature" + ], + "properties": { + "publicKey": { + "type": "string" + }, + "deviceIdentifier": { + "type": "string" + }, + "signature": { + "type": "string" + } + } + } + } + }, + "paths": { + "/ocs/v2.php/apps/notifications/api/{apiVersion}/settings": { + "post": { + "operationId": "settings-default-scope", + "summary": "Route is only in the default scope", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Personal settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/default-admin": { + "post": { + "operationId": "settings-default-admin-scope", + "summary": "Route is only in the admin scope because there is no \"NoAdminRequired\" annotation or attribute", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Personal settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/double": { + "post": { + "operationId": "settings-double-scope", + "summary": "Route is in admin and default scope", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + } + }, + "tags": [] +} \ No newline at end of file From 52198a2db1e29e640134d0ee1d510a07456ccfe8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Nov 2023 16:43:53 +0100 Subject: [PATCH 3/8] Only list schemas that are used in this Scope Signed-off-by: Joas Schilling --- generate-spec | 38 ++++++- src/Helpers.php | 15 +++ tests/lib/Controller/SettingsController.php | 19 +++- tests/openapi-administration.json | 102 +---------------- tests/openapi-federation.json | 117 -------------------- tests/openapi.json | 117 -------------------- 6 files changed, 70 insertions(+), 338 deletions(-) diff --git a/generate-spec b/generate-spec index 227eece..df3e7b6 100755 --- a/generate-spec +++ b/generate-spec @@ -214,10 +214,10 @@ foreach ($capabilitiesFiles as $path) { } } if ($capabilities != null) { - $schemas["Capabilities"] = $capabilities; + $schemas['Capabilities'] = $capabilities; } if ($publicCapabilities != null) { - $schemas["PublicCapabilities"] = $publicCapabilities; + $schemas['PublicCapabilities'] = $publicCapabilities; } if ($capabilities == null && $publicCapabilities == null) { Logger::warning("Capabilities", "No capabilities were loaded"); @@ -709,7 +709,6 @@ if (count($schemas) == 0 && count($routes) == 0) { } ksort($schemas); -$openapi["components"]["schemas"] = count($schemas) == 0 ? new stdClass() : $schemas; if ($useTags) { $openapi["tags"] = $tags; @@ -726,6 +725,39 @@ foreach ($scopePaths as $scope => $paths) { $openapiScope['info']['title'] .= $scopeSuffix; $openapiScope['paths'] = $paths; + $usedSchemas = []; + foreach ($paths as $url => $urlRoutes) { + foreach ($urlRoutes as $httpMethod => $routeData) { + foreach ($routeData['responses'] as $statusCode => $responseData) { + $usedSchemas = array_merge($usedSchemas, Helpers::collectUsedRefs($responseData['content']['application/json']['schema'])); + } + } + } + + $scopedSchemas = []; + foreach ($usedSchemas as $usedSchema) { + if (!str_starts_with($usedSchema, '#/components/schemas/')) { + continue; + } + + $schemaName = substr($usedSchema, strlen('#/components/schemas/')); + + if (!isset($schemas[$schemaName])) { + Logger::error("app", "Schema $schemaName used by scope $scope is not defined"); + } + + $scopedSchemas[$schemaName] = $schemas[$schemaName]; + } + + if (isset($schemas['Capabilities'])) { + $scopedSchemas['Capabilities'] = $schemas['Capabilities']; + } + if (isset($schemas['PublicCapabilities'])) { + $scopedSchemas['PublicCapabilities'] = $schemas['PublicCapabilities']; + } + + $openapiScope['components']['schemas'] = $scopedSchemas; + $startExtension = strrpos($out, '.'); if ($startExtension !== false) { // Path + filename (without extension) diff --git a/src/Helpers.php b/src/Helpers.php index ff8738e..b464b00 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -189,4 +189,19 @@ static function getAttributeScopes(ClassMethod|Class_|Node $node, string $annota return $scopes; } + + static function collectUsedRefs(array $data): array { + $refs = []; + if (isset($data['$ref'])) { + $refs[] = [$data['$ref']]; + } + if (isset($data['properties'])) { + foreach ($data['properties'] as $property) { + if (is_array($property)) { + $refs[] = self::collectUsedRefs($property); + } + } + } + return array_merge(...$refs); + } } diff --git a/tests/lib/Controller/SettingsController.php b/tests/lib/Controller/SettingsController.php index fdd42a1..e237f3f 100644 --- a/tests/lib/Controller/SettingsController.php +++ b/tests/lib/Controller/SettingsController.php @@ -26,11 +26,15 @@ namespace OCA\Notifications\Controller; +use OCA\Notifications\ResponseDefinitions; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; +/** + * @psalm-import-type NotificationsPushDevice from ResponseDefinitions + */ #[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)] class SettingsController extends OCSController { @@ -92,13 +96,24 @@ public function defaultAdminScope(): DataResponse { * * Route is only in the admin scope due to defined scope * - * @return DataResponse, array{}> + * @return DataResponse * * 200: Admin settings updated */ #[OpenAPI(scope: OpenAPI::SCOPE_ADMINISTRATION)] public function adminScope(): DataResponse { - return new DataResponse(); + return new DataResponse($this->createNotificationsPushDevice()); + } + + /** + * @return NotificationsPushDevice + */ + protected function createNotificationsPushDevice(): array { + return [ + 'publicKey' => 'publicKey', + 'deviceIdentifier' => 'deviceIdentifier', + 'signature' => 'signature', + ]; } /** diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index 502151a..65f90fb 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -20,104 +20,6 @@ } }, "schemas": { - "Notification": { - "type": "object", - "required": [ - "notification_id", - "app", - "user", - "datetime", - "object_type", - "object_id", - "subject", - "message", - "link", - "actions" - ], - "properties": { - "notification_id": { - "type": "integer", - "format": "int64" - }, - "app": { - "type": "string" - }, - "user": { - "type": "string" - }, - "datetime": { - "type": "string" - }, - "object_type": { - "type": "string" - }, - "object_id": { - "type": "string" - }, - "subject": { - "type": "string" - }, - "message": { - "type": "string" - }, - "link": { - "type": "string" - }, - "actions": { - "type": "array", - "items": { - "$ref": "#/components/schemas/NotificationAction" - } - }, - "subjectRich": { - "type": "string" - }, - "subjectRichParameters": { - "type": "object", - "additionalProperties": { - "type": "object" - } - }, - "messageRich": { - "type": "string" - }, - "messageRichParameters": { - "type": "object", - "additionalProperties": { - "type": "object" - } - }, - "icon": { - "type": "string" - }, - "shouldNotify": { - "type": "boolean" - } - } - }, - "NotificationAction": { - "type": "object", - "required": [ - "label", - "link", - "type", - "primary" - ], - "properties": { - "label": { - "type": "string" - }, - "link": { - "type": "string" - }, - "type": { - "type": "string" - }, - "primary": { - "type": "boolean" - } - } - }, "OCSMeta": { "type": "object", "required": [ @@ -224,7 +126,9 @@ "meta": { "$ref": "#/components/schemas/OCSMeta" }, - "data": {} + "data": { + "$ref": "#/components/schemas/PushDevice" + } } } } diff --git a/tests/openapi-federation.json b/tests/openapi-federation.json index 2a492e3..fc38cea 100644 --- a/tests/openapi-federation.json +++ b/tests/openapi-federation.json @@ -20,104 +20,6 @@ } }, "schemas": { - "Notification": { - "type": "object", - "required": [ - "notification_id", - "app", - "user", - "datetime", - "object_type", - "object_id", - "subject", - "message", - "link", - "actions" - ], - "properties": { - "notification_id": { - "type": "integer", - "format": "int64" - }, - "app": { - "type": "string" - }, - "user": { - "type": "string" - }, - "datetime": { - "type": "string" - }, - "object_type": { - "type": "string" - }, - "object_id": { - "type": "string" - }, - "subject": { - "type": "string" - }, - "message": { - "type": "string" - }, - "link": { - "type": "string" - }, - "actions": { - "type": "array", - "items": { - "$ref": "#/components/schemas/NotificationAction" - } - }, - "subjectRich": { - "type": "string" - }, - "subjectRichParameters": { - "type": "object", - "additionalProperties": { - "type": "object" - } - }, - "messageRich": { - "type": "string" - }, - "messageRichParameters": { - "type": "object", - "additionalProperties": { - "type": "object" - } - }, - "icon": { - "type": "string" - }, - "shouldNotify": { - "type": "boolean" - } - } - }, - "NotificationAction": { - "type": "object", - "required": [ - "label", - "link", - "type", - "primary" - ], - "properties": { - "label": { - "type": "string" - }, - "link": { - "type": "string" - }, - "type": { - "type": "string" - }, - "primary": { - "type": "boolean" - } - } - }, "OCSMeta": { "type": "object", "required": [ @@ -141,25 +43,6 @@ "type": "string" } } - }, - "PushDevice": { - "type": "object", - "required": [ - "publicKey", - "deviceIdentifier", - "signature" - ], - "properties": { - "publicKey": { - "type": "string" - }, - "deviceIdentifier": { - "type": "string" - }, - "signature": { - "type": "string" - } - } } } }, diff --git a/tests/openapi.json b/tests/openapi.json index de51e19..c349739 100644 --- a/tests/openapi.json +++ b/tests/openapi.json @@ -20,104 +20,6 @@ } }, "schemas": { - "Notification": { - "type": "object", - "required": [ - "notification_id", - "app", - "user", - "datetime", - "object_type", - "object_id", - "subject", - "message", - "link", - "actions" - ], - "properties": { - "notification_id": { - "type": "integer", - "format": "int64" - }, - "app": { - "type": "string" - }, - "user": { - "type": "string" - }, - "datetime": { - "type": "string" - }, - "object_type": { - "type": "string" - }, - "object_id": { - "type": "string" - }, - "subject": { - "type": "string" - }, - "message": { - "type": "string" - }, - "link": { - "type": "string" - }, - "actions": { - "type": "array", - "items": { - "$ref": "#/components/schemas/NotificationAction" - } - }, - "subjectRich": { - "type": "string" - }, - "subjectRichParameters": { - "type": "object", - "additionalProperties": { - "type": "object" - } - }, - "messageRich": { - "type": "string" - }, - "messageRichParameters": { - "type": "object", - "additionalProperties": { - "type": "object" - } - }, - "icon": { - "type": "string" - }, - "shouldNotify": { - "type": "boolean" - } - } - }, - "NotificationAction": { - "type": "object", - "required": [ - "label", - "link", - "type", - "primary" - ], - "properties": { - "label": { - "type": "string" - }, - "link": { - "type": "string" - }, - "type": { - "type": "string" - }, - "primary": { - "type": "boolean" - } - } - }, "OCSMeta": { "type": "object", "required": [ @@ -141,25 +43,6 @@ "type": "string" } } - }, - "PushDevice": { - "type": "object", - "required": [ - "publicKey", - "deviceIdentifier", - "signature" - ], - "properties": { - "publicKey": { - "type": "string" - }, - "deviceIdentifier": { - "type": "string" - }, - "signature": { - "type": "string" - } - } } } }, From b5b1e4ebfdc440c8b8887e1bac0a2f38324c777d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Nov 2023 16:51:01 +0100 Subject: [PATCH 4/8] Move admin-only routes to administration scope when default only Signed-off-by: Joas Schilling --- generate-spec | 2 + tests/appinfo/routes.php | 4 +- tests/lib/Controller/Settings2Controller.php | 58 ++++++++++++++++ tests/lib/Controller/SettingsController.php | 12 ---- tests/openapi-administration.json | 72 ++++++++++++++++++++ tests/openapi.json | 20 +++--- 6 files changed, 145 insertions(+), 23 deletions(-) create mode 100644 tests/lib/Controller/Settings2Controller.php diff --git a/generate-spec b/generate-spec index df3e7b6..b4c5a65 100755 --- a/generate-spec +++ b/generate-spec @@ -346,6 +346,8 @@ foreach ($parsedRoutes as $key => $value) { if (empty($scopes)) { if (!empty($controllerScopes)) { $scopes = $controllerScopes; + } else if ($isAdmin) { + $scopes = ['administration']; } else { $scopes = ['default']; } diff --git a/tests/appinfo/routes.php b/tests/appinfo/routes.php index 764dccb..c739c82 100644 --- a/tests/appinfo/routes.php +++ b/tests/appinfo/routes.php @@ -29,8 +29,10 @@ ['name' => 'Settings#federationByController', 'url' => '/api/{apiVersion}/controller-scope', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#ignoreByMethod', 'url' => '/api/{apiVersion}/ignore-method', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#defaultScope', 'url' => '/api/{apiVersion}/settings', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], - ['name' => 'Settings#defaultAdminScope', 'url' => '/api/{apiVersion}/default-admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#adminScope', 'url' => '/api/{apiVersion}/admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#doubleScope', 'url' => '/api/{apiVersion}/double', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + + ['name' => 'Settings2#defaultAdminScopeOverwritten', 'url' => '/api/{apiVersion}/default-admin-overwritten', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings2#defaultAdminScope', 'url' => '/api/{apiVersion}/default-admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ], ]; diff --git a/tests/lib/Controller/Settings2Controller.php b/tests/lib/Controller/Settings2Controller.php new file mode 100644 index 0000000..e2bd2e5 --- /dev/null +++ b/tests/lib/Controller/Settings2Controller.php @@ -0,0 +1,58 @@ + + * + * @author Julien Barnoin + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Notifications\Controller; + +use OCA\Notifications\ResponseDefinitions; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\OpenAPI; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCSController; + +class Settings2Controller extends OCSController { + /** + * Route is only in the admin scope because there is no "NoAdminRequired" annotation or attribute + * + * @return DataResponse, array{}> + * + * 200: Personal settings updated + */ + public function defaultAdminScope(): DataResponse { + return new DataResponse(); + } + + /** + * Route is only in the admin scope because there is no "NoAdminRequired" annotation or attribute + * + * @return DataResponse, array{}> + * + * 200: Personal settings updated + */ + #[OpenAPI] + public function defaultAdminScopeOverwritten(): DataResponse { + return new DataResponse(); + } +} diff --git a/tests/lib/Controller/SettingsController.php b/tests/lib/Controller/SettingsController.php index e237f3f..0d5faa2 100644 --- a/tests/lib/Controller/SettingsController.php +++ b/tests/lib/Controller/SettingsController.php @@ -79,18 +79,6 @@ public function defaultScope(): DataResponse { return new DataResponse(); } - /** - * Route is only in the admin scope because there is no "NoAdminRequired" annotation or attribute - * - * @return DataResponse, array{}> - * - * 200: Personal settings updated - */ - #[OpenAPI] - public function defaultAdminScope(): DataResponse { - return new DataResponse(); - } - /** * @NoAdminRequired * diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index 65f90fb..07cb371 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -209,6 +209,78 @@ } } } + }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/default-admin": { + "post": { + "operationId": "settings2-default-admin-scope", + "summary": "Route is only in the admin scope because there is no \"NoAdminRequired\" annotation or attribute", + "description": "This endpoint requires admin access", + "tags": [ + "settings2" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Personal settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } } }, "tags": [] diff --git a/tests/openapi.json b/tests/openapi.json index c349739..4eb5bb0 100644 --- a/tests/openapi.json +++ b/tests/openapi.json @@ -118,11 +118,10 @@ } } }, - "/ocs/v2.php/apps/notifications/api/{apiVersion}/default-admin": { + "/ocs/v2.php/apps/notifications/api/{apiVersion}/double": { "post": { - "operationId": "settings-default-admin-scope", - "summary": "Route is only in the admin scope because there is no \"NoAdminRequired\" annotation or attribute", - "description": "This endpoint requires admin access", + "operationId": "settings-double-scope", + "summary": "Route is in admin and default scope", "tags": [ "settings" ], @@ -160,7 +159,7 @@ ], "responses": { "200": { - "description": "Personal settings updated", + "description": "Admin settings updated", "content": { "application/json": { "schema": { @@ -190,12 +189,13 @@ } } }, - "/ocs/v2.php/apps/notifications/api/{apiVersion}/double": { + "/ocs/v2.php/apps/notifications/api/{apiVersion}/default-admin-overwritten": { "post": { - "operationId": "settings-double-scope", - "summary": "Route is in admin and default scope", + "operationId": "settings2-default-admin-scope-overwritten", + "summary": "Route is only in the admin scope because there is no \"NoAdminRequired\" annotation or attribute", + "description": "This endpoint requires admin access", "tags": [ - "settings" + "settings2" ], "security": [ { @@ -231,7 +231,7 @@ ], "responses": { "200": { - "description": "Admin settings updated", + "description": "Personal settings updated", "content": { "application/json": { "schema": { From 905ce33b10af52f1ba406b2ee82ed9860385db04 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Nov 2023 17:14:02 +0100 Subject: [PATCH 5/8] Add basic CI Signed-off-by: Joas Schilling --- .github/workflows/lint-php.yml | 60 +++++++++++++++++++++++++ .github/workflows/pr-feedback.yml | 34 ++++++++++++++ .github/workflows/test.yml | 75 +++++++++++++++++++++++++++++++ composer.json | 38 +++++++++------- 4 files changed, 190 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/lint-php.yml create mode 100644 .github/workflows/pr-feedback.yml create mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/lint-php.yml b/.github/workflows/lint-php.yml new file mode 100644 index 0000000..c624cc1 --- /dev/null +++ b/.github/workflows/lint-php.yml @@ -0,0 +1,60 @@ +# This workflow is provided via the organization template repository +# +# https://github.com/nextcloud/.github +# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization + +name: Lint php + +on: + pull_request: + push: + branches: + - main + - master + - stable* + +permissions: + contents: read + +concurrency: + group: lint-php-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + php-lint: + runs-on: ubuntu-latest + strategy: + matrix: + php-versions: [ "8.1" ] + + name: php-lint + + steps: + - name: Checkout + uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0 + + - name: Set up php ${{ matrix.php-versions }} + uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b # v2 + with: + php-version: ${{ matrix.php-versions }} + coverage: none + ini-file: development + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Lint + run: composer run lint + + summary: + permissions: + contents: none + runs-on: ubuntu-latest + needs: php-lint + + if: always() + + name: php-lint-summary + + steps: + - name: Summary status + run: if ${{ needs.php-lint.result != 'success' && needs.php-lint.result != 'skipped' }}; then exit 1; fi diff --git a/.github/workflows/pr-feedback.yml b/.github/workflows/pr-feedback.yml new file mode 100644 index 0000000..46eaff9 --- /dev/null +++ b/.github/workflows/pr-feedback.yml @@ -0,0 +1,34 @@ +# This workflow is provided via the organization template repository +# +# https://github.com/nextcloud/.github +# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization + +name: 'Ask for feedback on PRs' +on: + schedule: + - cron: '30 1 * * *' + +jobs: + pr-feedback: + runs-on: ubuntu-latest + steps: + - name: The get-github-handles-from-website action + uses: marcelklehr/get-github-handles-from-website-action@a739600f6b91da4957f51db0792697afbb2f143c # v1.0.0 + id: scrape + with: + website: 'https://nextcloud.com/team/' + - uses: marcelklehr/pr-feedback-action@601109aa729eb4c8d6d0ece7567b9d4901db4aef + with: + feedback-message: | + Hello there, + Thank you so much for taking the time and effort to create a pull request to our Nextcloud project. + + We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. + + Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 + + Thank you for contributing to Nextcloud and we hope to hear from you soon! + days-before-feedback: 14 + start-date: "2023-11-08" + exempt-authors: "${{ steps.scrape.outputs.users }},nextcloud-command,nextcloud-android-bot" + exempt-bots: true diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..a8984e9 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,75 @@ +# This workflow is provided via the organization template repository +# +# https://github.com/nextcloud/.github +# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization + +name: Generate OpenAPI + +on: + pull_request: + push: + branches: + - main + - master + - stable* + +permissions: + contents: read + +concurrency: + group: openapi-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + openapi: + runs-on: ubuntu-latest + strategy: + matrix: + php-versions: [ "8.1" ] + + name: openapi + + steps: + - name: Checkout + uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0 + + - name: Set up php ${{ matrix.php-versions }} + uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b # v2 + with: + php-version: ${{ matrix.php-versions }} + coverage: none + ini-file: development + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Install dependencies + run: composer i + + - name: Generate OpenAPI + working-directory: tests/ + run: ../generate-spec + + - name: Check openapi changes + run: | + bash -c "[[ ! \"`git status --porcelain `\" ]] || (echo 'Please recompile and commit the assets, see the section \"Show changes on failure\" for details' && exit 1)" + + - name: Show changes on failure + if: failure() + run: | + git status + git --no-pager diff + exit 1 # make it red to grab attention + + summary: + permissions: + contents: none + runs-on: ubuntu-latest + needs: openapi + + if: always() + + name: openapi-summary + + steps: + - name: Summary status + run: if ${{ needs.openapi.result != 'success' && needs.openapi.result != 'skipped' }}; then exit 1; fi diff --git a/composer.json b/composer.json index 18d15f1..0ea1e64 100644 --- a/composer.json +++ b/composer.json @@ -1,19 +1,23 @@ { - "name": "nextcloud/openapi-extractor", - "require": { - "php": "^8.1", - "ext-simplexml": "*", - "nikic/php-parser": "^4.16", - "adhocore/cli": "^v1.6", - "phpstan/phpdoc-parser": "^1.23" - }, - "bin": [ - "generate-spec", - "merge-specs" - ], - "autoload": { - "psr-4": { - "OpenAPIExtractor\\": "src" - } - } + "name": "nextcloud/openapi-extractor", + "require": { + "php": "^8.1", + "ext-simplexml": "*", + "nikic/php-parser": "^4.16", + "adhocore/cli": "^v1.6", + "phpstan/phpdoc-parser": "^1.23" + }, + "bin": [ + "generate-spec", + "merge-specs" + ], + "autoload": { + "psr-4": { + "OpenAPIExtractor\\": "src" + } + }, + "scripts": { + "lint": "find . -name \\*.php -not -path './tests/*' -not -path './vendor/*' -not -path './build/*' -print0 | xargs -0 -n1 php -l && php -l generate-spec && php -l merge-specs", + "test:unit": "cd tests && ../generate-spec" + } } From 23ca851970de5be3992d67b67075f5c06cea2cad Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 9 Nov 2023 09:34:10 +0100 Subject: [PATCH 6/8] Handle tags Signed-off-by: Joas Schilling --- generate-spec | 14 ++-- src/Helpers.php | 59 ++++++++++++++++ src/Route.php | 2 +- tests/appinfo/routes.php | 1 + tests/lib/Controller/Settings2Controller.php | 12 ++++ tests/openapi-administration.json | 73 ++++++++++++++++++++ 6 files changed, 155 insertions(+), 6 deletions(-) diff --git a/generate-spec b/generate-spec index b4c5a65..f8027a1 100755 --- a/generate-spec +++ b/generate-spec @@ -353,6 +353,8 @@ foreach ($parsedRoutes as $key => $value) { } } + $routeTags = Helpers::getAttributeTagsByScope($classMethod, 'OpenAPI', $routeName, $tagName, reset($scopes)); + if ($isOCS && !array_key_exists("OCSMeta", $schemas)) { $schemas["OCSMeta"] = [ "type" => "object", @@ -404,7 +406,7 @@ foreach ($parsedRoutes as $key => $value) { $routes[$scope] ??= []; $routes[$scope][] = new Route( $routeName, - $tagName, + $routeTags[$scope] ?? [$tagName], $controllerName, $methodName, $postfix, @@ -427,8 +429,10 @@ $tagNames = []; if ($useTags) { foreach ($routes as $scope => $scopeRoutes) { foreach ($scopeRoutes as $route) { - if (!in_array($route->tag, $tagNames)) { - $tagNames[] = $route->tag; + foreach ($route->tags as $tag) { + if (!in_array($tag, $tagNames)) { + $tagNames[] = $tag; + } } } } @@ -581,7 +585,7 @@ foreach ($routes as $scope => $scopeRoutes) { ); } - $operationId = [$route->tag]; + $operationId = $route->tags; $operationId = array_merge($operationId, array_map(fn(string $s) => Helpers::mapVerb(strtolower($s)), Helpers::splitOnUppercaseFollowedByNonUppercase($route->methodName))); if ($route->postfix != null) { $operationId[] = $route->postfix; @@ -606,7 +610,7 @@ foreach ($routes as $scope => $scopeRoutes) { $route->controllerMethod->summary != null ? ["summary" => $route->controllerMethod->summary] : [], count($route->controllerMethod->description) > 0 ? ["description" => implode("\n", $route->controllerMethod->description)] : [], $route->controllerMethod->isDeprecated ? ["deprecated" => true] : [], - $useTags ? ["tags" => [$route->tag]] : [], + $useTags ? ["tags" => $route->tags] : [], count($security) > 0 ? ["security" => $security] : [], count($queryParameters) > 0 || count($pathParameters) > 0 || $route->isOCS ? [ "parameters" => array_merge( diff --git a/src/Helpers.php b/src/Helpers.php index b464b00..3c3b2e4 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -4,6 +4,8 @@ use Exception; use PhpParser\Node; +use PhpParser\Node\Expr\Array_; +use PhpParser\Node\Expr\ArrayItem; use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\ClassMethod; @@ -190,6 +192,63 @@ static function getAttributeScopes(ClassMethod|Class_|Node $node, string $annota return $scopes; } + static function getAttributeTagsByScope(ClassMethod|Class_|Node $node, string $annotation, string $routeName, string $defaultTag, string $defaultScope): array { + $tags = []; + + /** @var Node\AttributeGroup $attrGroup */ + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($attr->name->getLast() === $annotation) { + if (empty($attr->args)) { + $tags[$defaultScope] = [$defaultTag]; + continue; + } + + $foundsTags = []; + $foundScopeName = null; + foreach ($attr->args as $arg) { + if ($arg->name->name === 'scope') { + if ($arg->value instanceof ClassConstFetch) { + if ($arg->value->class->getLast() === 'OpenAPI') { + $foundScopeName = match ($arg->value->name->name) { + 'SCOPE_DEFAULT' => 'default', + 'SCOPE_ADMINISTRATION' => 'administration', + 'SCOPE_FEDERATION' => 'federation', + 'SCOPE_IGNORE' => 'ignore', + // Fall back for future scopes assuming we follow the pattern (cut of 'SCOPE_' and lower case) + default => strtolower(substr($arg->value->name->name, 6)), + }; + } + } elseif ($arg->value instanceof String_) { + $foundScopeName = $arg->value->value; + } else { + Logger::panic($routeName, 'Can not interpret value of scope provided in OpenAPI(scope: …) attribute. Please use string or OpenAPI::SCOPE_* constants'); + } + } + + if ($arg->name->name === 'tags') { + if ($arg->value instanceof Array_) { + foreach ($arg->value->items as $item) { + if ($item instanceof ArrayItem) { + if ($item->value instanceof String_) { + $foundsTags[] = $item->value->value; + } + } + } + } + } + } + + if (!empty($foundsTags)) { + $tags[$foundScopeName ?: $defaultScope] = $foundsTags; + } + } + } + } + + return $tags; + } + static function collectUsedRefs(array $data): array { $refs = []; if (isset($data['$ref'])) { diff --git a/src/Route.php b/src/Route.php index 3c7117f..20b9f96 100644 --- a/src/Route.php +++ b/src/Route.php @@ -5,7 +5,7 @@ class Route { public function __construct( public string $name, - public string $tag, + public array $tags, public string $controllerName, public string $methodName, public ?string $postfix, diff --git a/tests/appinfo/routes.php b/tests/appinfo/routes.php index c739c82..b157fd4 100644 --- a/tests/appinfo/routes.php +++ b/tests/appinfo/routes.php @@ -34,5 +34,6 @@ ['name' => 'Settings2#defaultAdminScopeOverwritten', 'url' => '/api/{apiVersion}/default-admin-overwritten', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings2#defaultAdminScope', 'url' => '/api/{apiVersion}/default-admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings2#movedToSettingsTag', 'url' => '/api/{apiVersion}/moved-with-tag', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ], ]; diff --git a/tests/lib/Controller/Settings2Controller.php b/tests/lib/Controller/Settings2Controller.php index e2bd2e5..a5a324f 100644 --- a/tests/lib/Controller/Settings2Controller.php +++ b/tests/lib/Controller/Settings2Controller.php @@ -55,4 +55,16 @@ public function defaultAdminScope(): DataResponse { public function defaultAdminScopeOverwritten(): DataResponse { return new DataResponse(); } + + /** + * Route is only in the admin scope because there is no "NoAdminRequired" annotation or attribute + * + * @return DataResponse, array{}> + * + * 200: Personal settings updated + */ + #[OpenAPI(tags: ['settings', 'admin-settings'])] + public function movedToSettingsTag(): DataResponse { + return new DataResponse(); + } } diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index 07cb371..c55dec4 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -281,6 +281,79 @@ } } } + }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/moved-with-tag": { + "post": { + "operationId": "settings-admin-settings-moved-to-settings-tag", + "summary": "Route is only in the admin scope because there is no \"NoAdminRequired\" annotation or attribute", + "description": "This endpoint requires admin access", + "tags": [ + "settings", + "admin-settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Personal settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } } }, "tags": [] From 0358899eb9c27adb479fef9b8b95bc4f0091db2a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 10 Nov 2023 11:20:17 +0100 Subject: [PATCH 7/8] Don't break on files responses Signed-off-by: Joas Schilling --- generate-spec | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/generate-spec b/generate-spec index f8027a1..f1c8176 100755 --- a/generate-spec +++ b/generate-spec @@ -735,7 +735,11 @@ foreach ($scopePaths as $scope => $paths) { foreach ($paths as $url => $urlRoutes) { foreach ($urlRoutes as $httpMethod => $routeData) { foreach ($routeData['responses'] as $statusCode => $responseData) { - $usedSchemas = array_merge($usedSchemas, Helpers::collectUsedRefs($responseData['content']['application/json']['schema'])); + if (isset($responseData['content']['application/json'])) { + $usedSchemas = array_merge($usedSchemas, Helpers::collectUsedRefs($responseData['content']['application/json']['schema'])); + } else { + Logger::warning("app", "Could not read used schemas for response to '$httpMethod $url' with status code $statusCode"); + } } } } From d2c2487732311e9683b0673d67b89c3aff1fd116 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 10 Nov 2023 12:51:01 +0100 Subject: [PATCH 8/8] fix(parameters): Support int numerals Signed-off-by: Joas Schilling --- src/OpenApiType.php | 22 +++++ tests/appinfo/routes.php | 1 + tests/lib/Controller/SettingsController.php | 13 +++ tests/openapi-federation.json | 94 +++++++++++++++++++++ 4 files changed, 130 insertions(+) diff --git a/src/OpenApiType.php b/src/OpenApiType.php index b86e85e..07f9b31 100644 --- a/src/OpenApiType.php +++ b/src/OpenApiType.php @@ -161,6 +161,27 @@ static function resolve(string $context, array $definitions, ParamTagValueNode|N return new OpenApiType(type: "string", enum: $values); } + if ($isUnion && count($node->types) == count(array_filter($node->types, fn($type) => $type instanceof ConstTypeNode && $type->constExpr instanceof ConstExprIntegerNode))) { + $values = []; + /** @var ConstTypeNode $type */ + foreach ($node->types as $type) { + $values[] = (int) $type->constExpr->value; + } + + if (count(array_filter($values, fn(string $value) => $value == '')) > 0) { + // Not a valid enum + return new OpenApiType( + type: "integer", + format: "int64", + ); + } + + return new OpenApiType( + type: "integer", + format: "int64", + enum: $values, + ); + } if ($isUnion || $isIntersection) { $nullable = false; @@ -205,6 +226,7 @@ enum: [$node->constExpr->value], return new OpenApiType( type: "integer", format: "int64", + enum: [(int) $node->constExpr->value], ); } diff --git a/tests/appinfo/routes.php b/tests/appinfo/routes.php index b157fd4..8e734e3 100644 --- a/tests/appinfo/routes.php +++ b/tests/appinfo/routes.php @@ -31,6 +31,7 @@ ['name' => 'Settings#defaultScope', 'url' => '/api/{apiVersion}/settings', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#adminScope', 'url' => '/api/{apiVersion}/admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#doubleScope', 'url' => '/api/{apiVersion}/double', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#listOfIntParameters', 'url' => '/api/{apiVersion}/list-of-int', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings2#defaultAdminScopeOverwritten', 'url' => '/api/{apiVersion}/default-admin-overwritten', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings2#defaultAdminScope', 'url' => '/api/{apiVersion}/default-admin', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], diff --git a/tests/lib/Controller/SettingsController.php b/tests/lib/Controller/SettingsController.php index 0d5faa2..d80641c 100644 --- a/tests/lib/Controller/SettingsController.php +++ b/tests/lib/Controller/SettingsController.php @@ -118,4 +118,17 @@ protected function createNotificationsPushDevice(): array { public function doubleScope(): DataResponse { return new DataResponse(); } + + /** + * A route with a limited set of possible integers + * + * @param 1|2|3|4|5|6|7|8|9|10 $limit Maximum number of objects + * @psalm-param int<1, 10> $limit + * @return DataResponse, array{}> + * + * 200: Admin settings updated + */ + public function listOfIntParameters(int $limit): DataResponse { + return new DataResponse(); + } } diff --git a/tests/openapi-federation.json b/tests/openapi-federation.json index fc38cea..f3a5449 100644 --- a/tests/openapi-federation.json +++ b/tests/openapi-federation.json @@ -117,6 +117,100 @@ } } } + }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/list-of-int": { + "post": { + "operationId": "settings-list-of-int-parameters", + "summary": "A route with a limited set of possible integers", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "limit", + "in": "query", + "description": "Maximum number of objects", + "required": true, + "schema": { + "type": "integer", + "format": "int64", + "enum": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ] + } + }, + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } } }, "tags": []