Skip to content

Commit

Permalink
Merge pull request #43 from nextcloud/fix-ocs-exception-handling
Browse files Browse the repository at this point in the history 
Add correct response for OCS*Exceptions
  • Loading branch information
@nickvergessen
nickvergessen authored Apr 8, 2024
2 parents 5379789 + 9e44738 commit ae84659
Show file tree
Hide file tree
Showing 6 changed files with 400 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/ControllerMethod.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ public static function parse(string $context, array $definitions, ClassMethod $m
} else {
$responseDescriptions[$statusCode] = $docNode->value->description;
}
$responses[] = new ControllerMethodResponse($docNode->value->type, $statusCode, "text/plain", new OpenApiType(type: "string"), null);

if (str_starts_with($type->name, 'OCS') && str_ends_with($type->name, 'Exception')) {
$responses[] = new ControllerMethodResponse($docNode->value->type, $statusCode, "application/json", new OpenApiType(type: "array", maxLength: 0), null);
} else {
$responses[] = new ControllerMethodResponse($docNode->value->type, $statusCode, "text/plain", new OpenApiType(type: "string"), null);
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/Helpers.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ public static function mergeSchemas(array $schemas): mixed {
}

public static function wrapOCSResponse(Route $route, ControllerMethodResponse $response, array|stdClass $schema): array|stdClass {
if ($route->isOCS && $response->className == "DataResponse") {
if ($route->isOCS
&& ($response->className === 'DataResponse'
|| (str_starts_with($response->className, 'OCS') && str_ends_with($response->className, 'Exception')))) {
return [
"type" => "object",
"required" => [
Expand Down
2 changes: 2 additions & 0 deletions tests/appinfo/routes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,7 @@
['name' => 'Settings#numericParameter', 'url' => '/api/{apiVersion}/numeric', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
['name' => 'Settings#arrayListParameter', 'url' => '/api/{apiVersion}/array-list', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
['name' => 'Settings#arrayKeyedParameter', 'url' => '/api/{apiVersion}/array-keyed', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
['name' => 'Settings#throwingOCS', 'url' => '/api/{apiVersion}/throwing/ocs', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
['name' => 'Settings#throwingOther', 'url' => '/api/{apiVersion}/throwing/other', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
],
];
29 changes: 29 additions & 0 deletions tests/lib/Controller/SettingsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCSController;

/**
Expand Down Expand Up @@ -410,4 +411,32 @@ public function arrayListParameter(array $value = ['test']): DataResponse {
public function arrayKeyedParameter(array $value = ['test' => 'abc']): DataResponse {
return new DataResponse();
}

/**
* @NoAdminRequired
*
* Route throws an OCS exception
*
* @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
* @throws OCSNotFoundException Description of 404 because we throw all the time
*
* 200: Admin settings updated
*/
public function throwingOCS(): DataResponse {
throw new OCSNotFoundException();
}

/**
* @NoAdminRequired
*
* Route throws an OCS exception
*
* @return DataResponse<Http::STATUS_OK, array<empty>, array{}>
* @throws NotFoundException Description of 404 because we throw all the time
*
* 200: Admin settings updated
*/
public function throwingOther(): DataResponse {
throw new NotFoundException();
}
}
180 changes: 180 additions & 0 deletions tests/openapi-full.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2963,6 +2963,186 @@
}
}
},
"/ocs/v2.php/apps/notifications/api/{apiVersion}/throwing/ocs": {
"post": {
"operationId": "settings-throwingocs",
"summary": "Route throws an OCS exception",
"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": {}
}
}
}
}
}
}
},
"404": {
"description": "Description of 404 because we throw all the time",
"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}/throwing/other": {
"post": {
"operationId": "settings-throwing-other",
"summary": "Route throws an OCS exception",
"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": {}
}
}
}
}
}
}
},
"500": {
"description": "Description of 404 because we throw all the time",
"content": {
"text/plain": {
"schema": {
"type": "string"
}
}
}
}
}
}
},
"/ocs/v2.php/apps/notifications/api/{apiVersion}/controller-scope": {
"post": {
"operationId": "federation-federation-by-controller",
Expand Down
Loading

0 comments on commit ae84659

Please sign in to comment.