Skip to content

Commit 5a97989

Browse files
provokateurinprovokateurin
authored
Merge pull request #383 from nextcloud/fix/openapitype/resolve-refs-for-parameters
fix(OpenApiType): Resolve refs for parameters to check if they are serializable
2 parents 7549a2e + a133600 commit 5a97989

7 files changed

Lines changed: 1249 additions & 19 deletions

File tree

‎generate-spec.php‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@
170170
}
171171
}
172172
foreach (array_keys($definitions) as $name) {
173-
$schemas[Helpers::cleanSchemaName($name)] = OpenApiType::resolve('Response definitions: ' . $name, $definitions, $definitions[$name])->toArray();
173+
$schemas[Helpers::cleanSchemaName($name)] = OpenApiType::resolve('Response definitions: ' . $name, $definitions, $definitions[$name])->toArray($schemas);
174174
}
175175
} else {
176176
Logger::debug('Response definitions', 'No response definitions were loaded');
@@ -236,7 +236,7 @@
236236
continue;
237237
}
238238

239-
$schema = $type->toArray();
239+
$schema = $type->toArray($schemas);
240240

241241
if ($implementsPublicCapability) {
242242
$publicCapabilities = $publicCapabilities == null ? $schema : Helpers::mergeSchemas([$publicCapabilities, $schema]);
@@ -656,7 +656,7 @@
656656
if (count($matchingParameters) === 1) {
657657
$parameter = $matchingParameters[array_keys($matchingParameters)[0]];
658658

659-
$schema = $parameter->type->toArray(true);
659+
$schema = $parameter->type->toArray($schemas, true);
660660
$description = $parameter->type->description;
661661
} else {
662662
$schema = [
@@ -754,20 +754,20 @@
754754
$contentTypeResponses = array_values(array_filter($statusCodeResponses, fn (ControllerMethodResponse $response): bool => $response->contentType == $contentType));
755755

756756
$hasEmpty = array_filter($contentTypeResponses, fn (ControllerMethodResponse $response): bool => $response->type == null) !== [];
757-
$uniqueResponses = array_values(array_intersect_key($contentTypeResponses, array_unique(array_map(fn (ControllerMethodResponse $response): array|\stdClass => $response->type->toArray(), array_filter($contentTypeResponses, fn (ControllerMethodResponse $response): bool => $response->type != null)), SORT_REGULAR)));
757+
$uniqueResponses = array_values(array_intersect_key($contentTypeResponses, array_unique(array_map(fn (ControllerMethodResponse $response): array|\stdClass => $response->type->toArray($schemas), array_filter($contentTypeResponses, fn (ControllerMethodResponse $response): bool => $response->type != null)), SORT_REGULAR)));
758758
if (count($uniqueResponses) === 1) {
759759
if ($hasEmpty) {
760760
$mergedContentTypeResponses[$contentType] = [];
761761
} else {
762-
$schema = Helpers::cleanEmptyResponseArray($contentTypeResponses[0]->type->toArray());
762+
$schema = Helpers::cleanEmptyResponseArray($contentTypeResponses[0]->type->toArray($schemas));
763763
$mergedContentTypeResponses[$contentType] = ['schema' => Helpers::wrapOCSResponse($route, $contentTypeResponses[0], $schema)];
764764
}
765765
} else {
766766
$mergedContentTypeResponses[$contentType] = [
767767
'schema' => [
768768
// At least one should match, but it's possible that multiple match, so oneOf can't be used.
769-
'anyOf' => array_map(function (ControllerMethodResponse $response) use ($route): stdClass|array {
770-
$schema = Helpers::cleanEmptyResponseArray($response->type->toArray());
769+
'anyOf' => array_map(function (ControllerMethodResponse $response) use ($route, $schemas): stdClass|array {
770+
$schema = Helpers::cleanEmptyResponseArray($response->type->toArray($schemas));
771771
return Helpers::wrapOCSResponse($route, $response, $schema);
772772
}, $uniqueResponses),
773773
],
@@ -783,7 +783,7 @@
783783
array_keys($headers),
784784
array_map(
785785
fn (OpenApiType $type): array => [
786-
'schema' => $type->toArray(),
786+
'schema' => $type->toArray($schemas),
787787
],
788788
array_values($headers),
789789
),
@@ -844,7 +844,7 @@
844844
}
845845
$schema['properties'] = [];
846846
foreach ($bodyParameters as $bodyParameter) {
847-
$schema['properties'][$bodyParameter->name] = $bodyParameter->type->toArray();
847+
$schema['properties'][$bodyParameter->name] = $bodyParameter->type->toArray($schemas);
848848
}
849849

850850
$operation['requestBody'] = [
@@ -872,7 +872,7 @@
872872
if ($queryParameter->type->deprecated) {
873873
$parameter['deprecated'] = true;
874874
}
875-
$parameter['schema'] = $queryParameter->type->toArray(true);
875+
$parameter['schema'] = $queryParameter->type->toArray($schemas, true);
876876

877877
$parameters[] = $parameter;
878878
}
@@ -1049,6 +1049,11 @@
10491049
$usedRefs[] = Helpers::collectUsedRefs($responseData['content']);
10501050
}
10511051
}
1052+
foreach (($routeData['parameters'] ?? []) as $parameterData) {
1053+
if (isset($parameterData['schema'])) {
1054+
$usedRefs[] = Helpers::collectUsedRefs($parameterData['schema']);
1055+
}
1056+
}
10521057
if (isset($routeData['requestBody']['content']) && $routeData['requestBody']['content'] !== []) {
10531058
$usedRefs[] = Helpers::collectUsedRefs($routeData['requestBody']['content']);
10541059
}

‎src/OpenApiType.php‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,35 @@ public function __construct(
6464
) {
6565
}
6666

67-
public function toArray(bool $isParameter = false): array|stdClass {
68-
if ($isParameter && ($this->type === 'object' || $this->ref !== null || $this->anyOf !== null || $this->allOf !== null)) {
67+
/**
68+
* @param array<string, array<string, mixed>> $schemas
69+
*/
70+
private function isParameterSerializable(array $schemas, ?string $type, ?bool $nullable, ?string $ref, ?array $anyOf, ?array $allOf): bool {
71+
if ($ref !== null) {
72+
$prefix = '#/components/schemas/';
73+
if (str_starts_with($ref, $prefix) && ($schema = $schemas[substr($ref, strlen($prefix))] ?? null) !== null) {
74+
return $this->isParameterSerializable($schemas, $schema['type'] ?? null, $schema['nullable'] ?? null, $schema['ref'] ?? null, $schema['anyOf'] ?? null, $schema['allOf'] ?? null);
75+
}
76+
77+
return false;
78+
}
79+
80+
// https://github.com/OAI/OpenAPI-Specification/issues/1368#issuecomment-354037150
81+
return $type !== 'object' && $anyOf === null && ($allOf === null || (($nullable ?? false) && count($allOf) === 1));
82+
}
83+
84+
/**
85+
* @param array<string, array<string, mixed>> $schemas
86+
*/
87+
public function toArray(array $schemas, bool $isParameter = false): array|stdClass {
88+
if ($isParameter && !$this->isParameterSerializable($schemas, $this->type, $this->nullable, $this->ref, $this->anyOf, $this->allOf)) {
6989
Logger::warning($this->context, 'Complex types can not be part of query or URL parameters. Falling back to string due to undefined serialization!');
7090
return (new OpenApiType(
7191
context: $this->context,
7292
type: 'string',
7393
nullable: $this->nullable,
7494
description: $this->description,
75-
))->toArray($isParameter);
95+
))->toArray($schemas, $isParameter);
7696
}
7797

7898
$values = [];
@@ -101,7 +121,7 @@ public function toArray(bool $isParameter = false): array|stdClass {
101121
$values['description'] = Helpers::cleanDocComment($this->description);
102122
}
103123
if ($this->items instanceof \OpenAPIExtractor\OpenApiType) {
104-
$values['items'] = $this->items->toArray();
124+
$values['items'] = $this->items->toArray($schemas);
105125
}
106126
if ($this->minLength !== null) {
107127
$values['minLength'] = $this->minLength;
@@ -126,24 +146,24 @@ public function toArray(bool $isParameter = false): array|stdClass {
126146
}
127147
if ($this->properties !== null && $this->properties !== []) {
128148
$values['properties'] = array_combine(array_keys($this->properties),
129-
array_map(static fn (OpenApiType $property): array|\stdClass => $property->toArray(), array_values($this->properties)),
149+
array_map(static fn (OpenApiType $property): array|\stdClass => $property->toArray($schemas), array_values($this->properties)),
130150
);
131151
}
132152
if ($this->additionalProperties !== null) {
133153
if ($this->additionalProperties instanceof OpenApiType) {
134-
$values['additionalProperties'] = $this->additionalProperties->toArray();
154+
$values['additionalProperties'] = $this->additionalProperties->toArray($schemas);
135155
} else {
136156
$values['additionalProperties'] = $this->additionalProperties;
137157
}
138158
}
139159
if ($this->oneOf !== null) {
140-
$values['oneOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->oneOf);
160+
$values['oneOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray($schemas), $this->oneOf);
141161
}
142162
if ($this->anyOf !== null) {
143-
$values['anyOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->anyOf);
163+
$values['anyOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray($schemas), $this->anyOf);
144164
}
145165
if ($this->allOf !== null) {
146-
$values['allOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->allOf);
166+
$values['allOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray($schemas), $this->allOf);
147167
}
148168

149169
return $values !== [] ? $values : new stdClass();

‎tests/appinfo/routes.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@
9797
['name' => 'Settings#intBackedEnumParameter', 'url' => '/api/{apiVersion}/enums/int-backed', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
9898
['name' => 'Settings#sortDirectionParameter', 'url' => '/api/{apiVersion}/enums/sort-direction', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
9999
['name' => 'Settings#injectedServiceParameter', 'url' => '/api/{apiVersion}/injected-service', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']],
100+
['name' => 'Settings#intersectionTypeEnumParameter', 'url' => '/api/{apiVersion}/intersection-type-enum', 'verb' => 'GET', 'requirements' => ['apiVersion' => '(v2)']],
101+
['name' => 'Settings#nullableIntersectionTypeEnumParameter', 'url' => '/api/{apiVersion}/nullable-intersection-type-enum', 'verb' => 'GET', 'requirements' => ['apiVersion' => '(v2)']],
102+
['name' => 'Settings#intersectionTypeAliasEnumParameter', 'url' => '/api/{apiVersion}/intersection-type-alias-enum', 'verb' => 'GET', 'requirements' => ['apiVersion' => '(v2)']],
103+
['name' => 'Settings#nullableIntersectionTypeAliasEnumParameter', 'url' => '/api/{apiVersion}/nullable-intersection-type-alias-enum', 'verb' => 'GET', 'requirements' => ['apiVersion' => '(v2)']],
100104
['name' => 'V1\SubDir#subDirRoute', 'url' => '/sub-dir', 'verb' => 'GET'],
101105
],
102106
];

‎tests/lib/Controller/SettingsController.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
* @psalm-import-type NotificationsPushDevice from ResponseDefinitions
3131
* @psalm-import-type NotificationsNotification from ResponseDefinitions
3232
* @psalm-import-type NotificationsCollection from ResponseDefinitions
33+
* @psalm-import-type NotificationsEnum from ResponseDefinitions
3334
*/
3435
class SettingsController extends OCSController {
3536
/**
@@ -901,4 +902,52 @@ public function sortDirectionParameter(\SortDirection $direction): DataResponse
901902
public function injectedServiceParameter(IUser $user, string $path): DataResponse {
902903
return new DataResponse();
903904
}
905+
906+
/**
907+
* A route with a intersection type as enum parameter
908+
*
909+
* @param 'A'|'B' $enum The enum
910+
* @return DataResponse<Http::STATUS_OK, array{}, array{}>
911+
*
912+
* 200: OK
913+
*/
914+
public function intersectionTypeEnumParameter(string $enum): DataResponse {
915+
return new DataResponse();
916+
}
917+
918+
/**
919+
* A route with a nullable intersection type as enum parameter
920+
*
921+
* @param null|'A'|'B' $enum The enum
922+
* @return DataResponse<Http::STATUS_OK, array{}, array{}>
923+
*
924+
* 200: OK
925+
*/
926+
public function nullableIntersectionTypeEnumParameter(?string $enum): DataResponse {
927+
return new DataResponse();
928+
}
929+
930+
/**
931+
* A route with a intersection type alias as enum parameter
932+
*
933+
* @param NotificationsEnum $enum The enum
934+
* @return DataResponse<Http::STATUS_OK, array{}, array{}>
935+
*
936+
* 200: OK
937+
*/
938+
public function intersectionTypeAliasEnumParameter(string $enum): DataResponse {
939+
return new DataResponse();
940+
}
941+
942+
/**
943+
* A route with a nullable intersection type alias as enum parameter
944+
*
945+
* @param ?NotificationsEnum $enum The enum
946+
* @return DataResponse<Http::STATUS_OK, array{}, array{}>
947+
*
948+
* 200: OK
949+
*/
950+
public function nullableIntersectionTypeAliasEnumParameter(?string $enum): DataResponse {
951+
return new DataResponse();
952+
}
904953
}

‎tests/lib/ResponseDefinitions.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@
7575
* @psalm-type NotificationsSchemaOnlyInCapabilities = array{
7676
* key: string,
7777
* }
78+
*
79+
* @psalm-type NotificationsEnum = 'A'|'B'
7880
*/
7981
class ResponseDefinitions {
8082
}

0 commit comments

Comments
 (0)