From 0717ced5cf42b8ff16212a4a00ac11d8a4e98644 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Fri, 15 Mar 2024 10:45:29 +0100 Subject: [PATCH] refactor: Put non-atomic parameters into body Signed-off-by: provokateurin --- generate-spec | 90 +++++++--- src/OpenApiType.php | 57 ++---- tests/openapi-administration.json | 288 ++++++++++++++++++------------ tests/openapi-full.json | 288 ++++++++++++++++++------------ 4 files changed, 433 insertions(+), 290 deletions(-) diff --git a/generate-spec b/generate-spec index c14b98f..e8d880f 100755 --- a/generate-spec +++ b/generate-spec @@ -43,6 +43,7 @@ $command ->option('--no-tags', 'Use no tags') ->option('--continue-on-error', 'Continue on error') ->option('--openapi-version', 'OpenAPI version to use', null, '3.0.3') + ->option('--parameters-to-body', 'Put all parameters into the JSON body') ->option('--verbose', 'Verbose logging') ->parse($_SERVER["argv"]); @@ -55,6 +56,7 @@ $useTags = $command->tags ?? true; Logger::$exitOnError = !($command->continueOnError ?? false); Logger::$verbose = $command->verbose ?? false; $openapiVersion = $command->openapiVersion ?? '3.0.3'; +$parametersToBody = $command->parametersToBody ?? false; if ($dir == "") { $dir = "."; @@ -151,7 +153,7 @@ if (file_exists($definitionsPath)) { } } foreach (array_keys($definitions) as $name) { - $schemas[Helpers::cleanSchemaName($name)] = OpenApiType::resolve("Response definitions", $definitions, $definitions[$name])->toArray($openapiVersion); + $schemas[Helpers::cleanSchemaName($name)] = OpenApiType::resolve("Response definitions", $definitions, $definitions[$name])->toArray(); } } else { Logger::debug("Response definitions", "No response definitions were loaded"); @@ -212,7 +214,7 @@ foreach ($capabilitiesFiles as $path) { continue; } - $schema = $type->toArray($openapiVersion); + $schema = $type->toArray(); if ($implementsPublicCapability) { $publicCapabilities = $publicCapabilities == null ? $schema : Helpers::mergeSchemas([$publicCapabilities, $schema]); @@ -574,7 +576,7 @@ foreach ($routes as $scope => $scopeRoutes) { continue; } - $schema = $parameter->type->toArray($openapiVersion, true); + $schema = $parameter->type->toArray(true); $description = $parameter?->docType != null && $parameter->docType->description != "" ? Helpers::cleanDocComment($parameter->docType->description) : null; } else { $schema = [ @@ -667,19 +669,19 @@ foreach ($routes as $scope => $scopeRoutes) { $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))); + $uniqueResponses = array_values(array_intersect_key($contentTypeResponses, array_unique(array_map(fn (ControllerMethodResponse $response) => $response->type->toArray(), 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)); + $schema = Helpers::cleanEmptyResponseArray($contentTypeResponses[0]->type->toArray()); $mergedContentTypeResponses[$contentType] = ["schema" => Helpers::wrapOCSResponse($route, $contentTypeResponses[0], $schema)]; } } else { $mergedContentTypeResponses[$contentType] = [ "schema" => [ - [$hasEmpty ? "anyOf" : "oneOf" => array_map(function (ControllerMethodResponse $response) use ($route, $openapiVersion) { - $schema = Helpers::cleanEmptyResponseArray($response->type->toArray($openapiVersion)); + [$hasEmpty ? "anyOf" : "oneOf" => array_map(function (ControllerMethodResponse $response) use ($route) { + $schema = Helpers::cleanEmptyResponseArray($response->type->toArray()); return Helpers::wrapOCSResponse($route, $response, $schema); }, $uniqueResponses)], ], @@ -695,7 +697,7 @@ foreach ($routes as $scope => $scopeRoutes) { array_keys($headers), array_map( fn (OpenApiType $type) => [ - "schema" => $type->toArray($openapiVersion), + "schema" => $type->toArray(), ], array_values($headers), ), @@ -745,24 +747,65 @@ foreach ($routes as $scope => $scopeRoutes) { $operation["security"] = $security; } if (count($queryParameters) > 0 || count($pathParameters) > 0 || $route->isOCS) { - $parameters = [ - ...array_map(static function (ControllerMethodParameter $parameter) use ($openapiVersion) { - $out = [ - "name" => $parameter->name . ($parameter->type->type === "array" ? "[]" : ""), + $requiredBodyParameters = []; + $bodyParameters = []; + $parameters = []; + + foreach ($queryParameters as $queryParameter) { + // Non-atomic types have to be put into the body as the serialization is too complex for query parameters + // Booleans also have to be in the body because for query parameters 0/1 would have to be used which is not ergonomic. + $isBodyParameter = $parametersToBody || + $queryParameter->type->type === "boolean" || + $queryParameter->type->type === "object" || + $queryParameter->type->type === "array" || + $queryParameter->type->ref !== null || + $queryParameter->type->anyOf !== null || + $queryParameter->type->allOf !== null; + $required = !$queryParameter->type->nullable && !$queryParameter->type->hasDefaultValue; + + if ($isBodyParameter) { + $bodyParameters[$queryParameter->name] = $queryParameter->type->toArray(); + if ($required) { + $requiredBodyParameters[] = $queryParameter->name; + } + } else { + $parameter = [ + "name" => $queryParameter->name, "in" => "query", ]; - if ($parameter->docType !== null && $parameter->docType->description !== "") { - $out["description"] = Helpers::cleanDocComment($parameter->docType->description); + if ($queryParameter->docType !== null && $queryParameter->docType->description !== "") { + $parameter["description"] = Helpers::cleanDocComment($queryParameter->docType->description); } - if (!$parameter->type->nullable && !$parameter->type->hasDefaultValue) { - $out["required"] = true; + if ($required) { + $parameter["required"] = true; } - $out["schema"] = $parameter->type->toArray($openapiVersion, true); + $parameter["schema"] = $queryParameter->type->toArray(true); + $parameters[] = $parameter; + } + } - return $out; - }, $queryParameters), - ...$pathParameters, - ]; + if (count($bodyParameters) > 0) { + $required = count($requiredBodyParameters) > 0; + + $schema = [ + "type" => "object", + ]; + if ($required) { + $schema["required"] = $requiredBodyParameters; + } + $schema["properties"] = $bodyParameters; + + $operation["requestBody"] = [ + "required" => $required, + "content" => [ + "application/json" => [ + "schema" => $schema, + ], + ], + ]; + } + + $parameters = array_merge($parameters, $pathParameters); if ($route->isOCS) { $parameters[] = [ "name" => "OCS-APIRequest", @@ -775,7 +818,10 @@ foreach ($routes as $scope => $scopeRoutes) { ], ]; } - $operation["parameters"] = $parameters; + + if (count($parameters) > 0) { + $operation["parameters"] = $parameters; + } } $operation["responses"] = $mergedResponses; diff --git a/src/OpenApiType.php b/src/OpenApiType.php index f686cb6..5db04bc 100644 --- a/src/OpenApiType.php +++ b/src/OpenApiType.php @@ -51,44 +51,13 @@ public function __construct( ) { } - public function toArray(string $openapiVersion, bool $isParameter = false): array|stdClass { - $asContentString = $isParameter && ( - $this->type == "object" || - $this->ref !== null || - $this->anyOf !== null || - $this->allOf !== null); - if ($asContentString) { - $values = [ - "type" => "string", - ]; - if ($this->nullable) { - $values["nullable"] = true; - } - if (version_compare($openapiVersion, "3.1.0", ">=")) { - $values["contentMediaType"] = "application/json"; - $values["contentSchema"] = $this->toArray($openapiVersion); - } - - return $values; - } - - $type = $this->type; - $defaultValue = $this->defaultValue; - $enum = $this->enum; - if ($isParameter && $type == "boolean") { - $type = "integer"; - $enum = [0, 1]; - if ($this->hasDefaultValue) { - $defaultValue = $defaultValue === true ? 1 : 0; - } - } - + public function toArray(bool $isParameter = false): array|stdClass { $values = []; if ($this->ref !== null) { $values["\$ref"] = $this->ref; } - if ($type !== null) { - $values["type"] = $type; + if ($this->type !== null) { + $values["type"] = $this->type; } if ($this->format !== null) { $values["format"] = $this->format; @@ -96,17 +65,17 @@ public function toArray(string $openapiVersion, bool $isParameter = false): arra if ($this->nullable) { $values["nullable"] = true; } - if ($this->hasDefaultValue && $defaultValue !== null) { - $values["default"] = $defaultValue; + if ($this->hasDefaultValue && $this->defaultValue !== null) { + $values["default"] = $this->defaultValue; } - if ($enum !== null) { - $values["enum"] = $enum; + if ($this->enum !== null) { + $values["enum"] = $this->enum; } if ($this->description !== null && $this->description !== "" && !$isParameter) { $values["description"] = $this->description; } if ($this->items !== null) { - $values["items"] = $this->items->toArray($openapiVersion); + $values["items"] = $this->items->toArray(); } if ($this->minLength !== null) { $values["minLength"] = $this->minLength; @@ -125,24 +94,24 @@ public function toArray(string $openapiVersion, bool $isParameter = false): arra } if ($this->properties !== null && count($this->properties) > 0) { $values["properties"] = array_combine(array_keys($this->properties), - array_map(static fn (OpenApiType $property) => $property->toArray($openapiVersion), array_values($this->properties)), + array_map(static fn (OpenApiType $property) => $property->toArray(), array_values($this->properties)), ); } if ($this->additionalProperties !== null) { if ($this->additionalProperties instanceof OpenApiType) { - $values["additionalProperties"] = $this->additionalProperties->toArray($openapiVersion); + $values["additionalProperties"] = $this->additionalProperties->toArray(); } else { $values["additionalProperties"] = $this->additionalProperties; } } if ($this->oneOf !== null) { - $values["oneOf"] = array_map(fn (OpenApiType $type) => $type->toArray($openapiVersion), $this->oneOf); + $values["oneOf"] = array_map(fn (OpenApiType $type) => $type->toArray(), $this->oneOf); } if ($this->anyOf !== null) { - $values["anyOf"] = array_map(fn (OpenApiType $type) => $type->toArray($openapiVersion), $this->anyOf); + $values["anyOf"] = array_map(fn (OpenApiType $type) => $type->toArray(), $this->anyOf); } if ($this->allOf !== null) { - $values["allOf"] = array_map(fn (OpenApiType $type) => $type->toArray($openapiVersion), $this->allOf); + $values["allOf"] = array_map(fn (OpenApiType $type) => $type->toArray(), $this->allOf); } return count($values) > 0 ? $values : new stdClass(); diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index 7aa37bc..aee919d 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -1339,20 +1339,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "Boolean required", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "Boolean required" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1424,20 +1430,24 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "Boolean defaulting to false", - "schema": { - "type": "integer", - "default": 0, - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "yesOrNo": { + "type": "boolean", + "default": false, + "description": "Boolean defaulting to false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1509,20 +1519,24 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "Boolean defaulting to true", - "schema": { - "type": "integer", - "default": 1, - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "yesOrNo": { + "type": "boolean", + "default": true, + "description": "Boolean defaulting to true" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1594,20 +1608,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "boolean or true", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "boolean or true" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1679,20 +1699,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "boolean or false", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "boolean or false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1764,20 +1790,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "boolean or true or false", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "boolean or true or false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1849,20 +1881,30 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "true or false", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "enum": [ + true, + false + ], + "description": "true or false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -2178,21 +2220,29 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "value[]", - "in": "query", - "description": "Some array value", - "schema": { - "type": "array", - "default": [ - "test" - ], - "items": { - "type": "string" + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "value": { + "type": "array", + "default": [ + "test" + ], + "description": "Some array value", + "items": { + "type": "string" + } + } + } } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -2264,15 +2314,29 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "value", - "in": "query", - "description": "Some array value", - "schema": { - "type": "string" + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "value": { + "type": "object", + "default": { + "test": "abc" + }, + "description": "Some array value", + "additionalProperties": { + "type": "string" + } + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", diff --git a/tests/openapi-full.json b/tests/openapi-full.json index 9230ef2..65c8eda 100644 --- a/tests/openapi-full.json +++ b/tests/openapi-full.json @@ -1466,20 +1466,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "Boolean required", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "Boolean required" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1551,20 +1557,24 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "Boolean defaulting to false", - "schema": { - "type": "integer", - "default": 0, - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "yesOrNo": { + "type": "boolean", + "default": false, + "description": "Boolean defaulting to false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1636,20 +1646,24 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "Boolean defaulting to true", - "schema": { - "type": "integer", - "default": 1, - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "yesOrNo": { + "type": "boolean", + "default": true, + "description": "Boolean defaulting to true" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1721,20 +1735,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "boolean or true", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "boolean or true" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1806,20 +1826,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "boolean or false", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "boolean or false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1891,20 +1917,26 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "boolean or true or false", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "description": "boolean or true or false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -1976,20 +2008,30 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "yesOrNo", - "in": "query", - "description": "true or false", - "required": true, - "schema": { - "type": "integer", - "enum": [ - 0, - 1 - ] + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "yesOrNo" + ], + "properties": { + "yesOrNo": { + "type": "boolean", + "enum": [ + true, + false + ], + "description": "true or false" + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -2305,21 +2347,29 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "value[]", - "in": "query", - "description": "Some array value", - "schema": { - "type": "array", - "default": [ - "test" - ], - "items": { - "type": "string" + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "value": { + "type": "array", + "default": [ + "test" + ], + "description": "Some array value", + "items": { + "type": "string" + } + } + } } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path", @@ -2391,15 +2441,29 @@ "basic_auth": [] } ], - "parameters": [ - { - "name": "value", - "in": "query", - "description": "Some array value", - "schema": { - "type": "string" + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "value": { + "type": "object", + "default": { + "test": "abc" + }, + "description": "Some array value", + "additionalProperties": { + "type": "string" + } + } + } + } } - }, + } + }, + "parameters": [ { "name": "apiVersion", "in": "path",