OpenAPI: Make sure we do not override defined parameters #4137 (#4138)

soyuka · web-flow · commit 32c59986f4d7 · 2021-03-16T17:43:42.000+01:00
diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php
@@ -80,7 +80,7 @@ public function buildSchema(string $className, string $format = 'json', string $
         }
 
         $version = $schema->getVersion();
-        $definitionName = $this->buildDefinitionName($className, $format, $type, $operationType, $operationName, $serializerContext);
+        $definitionName = $this->buildDefinitionName($className, $format, $inputOrOutputClass, $resourceMetadata, $serializerContext);
 
         if (null === $operationType || null === $operationName) {
             $method = Schema::TYPE_INPUT === $type ? 'POST' : 'GET';
@@ -239,10 +239,8 @@ private function buildPropertySchema(Schema $schema, string $definitionName, str
         $schema->getDefinitions()[$definitionName]['properties'][$normalizedPropertyName] = $propertySchema;
     }
 
-    private function buildDefinitionName(string $className, string $format = 'json', string $type = Schema::TYPE_OUTPUT, ?string $operationType = null, ?string $operationName = null, ?array $serializerContext = null): string
+    private function buildDefinitionName(string $className, string $format = 'json', ?string $inputOrOutputClass = null, ?ResourceMetadata $resourceMetadata = null, ?array $serializerContext = null): string
     {
-        [$resourceMetadata, $serializerContext,, $inputOrOutputClass] = $this->getMetadata($className, $type, $operationType, $operationName, $serializerContext);
-
         $prefix = $resourceMetadata ? $resourceMetadata->getShortName() : (new \ReflectionClass($className))->getShortName();
         if (null !== $inputOrOutputClass && $className !== $inputOrOutputClass) {
             $parts = explode('\\', $inputOrOutputClass);
diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php
@@ -142,7 +142,7 @@ private function getClassType(?string $className, string $format, ?bool $readabl
             throw new \LogicException('The schema factory must be injected by calling the "setSchemaFactory" method.');
         }
 
-        $subSchema = $this->schemaFactory->buildSchema($className, $format, Schema::TYPE_OUTPUT, null, null, $subSchema, $serializerContext);
+        $subSchema = $this->schemaFactory->buildSchema($className, $format, Schema::TYPE_OUTPUT, null, null, $subSchema, $serializerContext, false);
 
         return ['$ref' => $subSchema['$ref']];
     }
diff --git a/src/OpenApi/Factory/OpenApiFactory.php b/src/OpenApi/Factory/OpenApiFactory.php
@@ -86,19 +86,19 @@ public function __invoke(array $context = []): OpenApi
         $servers = '/' === $baseUrl || '' === $baseUrl ? [new Model\Server('/')] : [new Model\Server($baseUrl)];
         $paths = new Model\Paths();
         $links = [];
-        $schemas = [];
+        $schemas = new \ArrayObject();
 
         foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {
             $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
             $resourceShortName = $resourceMetadata->getShortName();
 
             // Items needs to be parsed first to be able to reference the lines from the collection operation
             [$itemOperationLinks, $itemOperationSchemas] = $this->collectPaths($resourceMetadata, $resourceClass, OperationType::ITEM, $context, $paths, $links, $schemas);
-            $schemas += $itemOperationSchemas;
+            $this->appendSchemaDefinitions($schemas, $itemOperationSchemas);
             [$collectionOperationLinks, $collectionOperationSchemas] = $this->collectPaths($resourceMetadata, $resourceClass, OperationType::COLLECTION, $context, $paths, $links, $schemas);
 
             [$subresourceOperationLinks, $subresourceOperationSchemas] = $this->collectPaths($resourceMetadata, $resourceClass, OperationType::SUBRESOURCE, $context, $paths, $links, $schemas);
-            $schemas += $collectionOperationSchemas;
+            $this->appendSchemaDefinitions($schemas, $collectionOperationSchemas);
         }
 
         $securitySchemes = $this->getSecuritySchemes();
@@ -113,7 +113,7 @@ public function __invoke(array $context = []): OpenApi
             $servers,
             $paths,
             new Model\Components(
-                new \ArrayObject($schemas),
+                $schemas,
                 new \ArrayObject(),
                 new \ArrayObject(),
                 new \ArrayObject(),
@@ -128,7 +128,7 @@ public function __invoke(array $context = []): OpenApi
     /**
      * @return array | array
      */
-    private function collectPaths(ResourceMetadata $resourceMetadata, string $resourceClass, string $operationType, array $context, Model\Paths $paths, array &$links, array $schemas = []): array
+    private function collectPaths(ResourceMetadata $resourceMetadata, string $resourceClass, string $operationType, array $context, Model\Paths $paths, array &$links, \ArrayObject $schemas): array
     {
         $resourceShortName = $resourceMetadata->getShortName();
         $operations = OperationType::COLLECTION === $operationType ? $resourceMetadata->getCollectionOperations() : (OperationType::ITEM === $operationType ? $resourceMetadata->getItemOperations() : $this->subresourceOperationFactory->create($resourceClass));
@@ -155,12 +155,14 @@ private function collectPaths(ResourceMetadata $resourceMetadata, string $resour
             $linkedOperationId = 'get'.ucfirst($resourceShortName).ucfirst(OperationType::ITEM);
             $pathItem = $paths->getPath($path) ?: new Model\PathItem();
             $forceSchemaCollection = OperationType::SUBRESOURCE === $operationType ? ($operation['collection'] ?? false) : false;
+            $schema = new Schema('openapi');
+            $schema->setDefinitions($schemas);
 
             $operationOutputSchemas = [];
             foreach ($responseMimeTypes as $operationFormat) {
-                $operationOutputSchema = $this->jsonSchemaFactory->buildSchema($resourceClass, $operationFormat, Schema::TYPE_OUTPUT, $operationType, $operationName, new Schema('openapi'), null, $forceSchemaCollection);
-                $schemas += $operationOutputSchema->getDefinitions()->getArrayCopy();
+                $operationOutputSchema = $this->jsonSchemaFactory->buildSchema($resourceClass, $operationFormat, Schema::TYPE_OUTPUT, $operationType, $operationName, $schema, null, $forceSchemaCollection);
                 $operationOutputSchemas[$operationFormat] = $operationOutputSchema;
+                $this->appendSchemaDefinitions($schemas, $operationOutputSchema->getDefinitions());
             }
 
             $parameters = [];
@@ -175,19 +177,42 @@ private function collectPaths(ResourceMetadata $resourceMetadata, string $resour
             // Set up parameters
             if (OperationType::ITEM === $operationType) {
                 foreach ($identifiers as $parameterName => $identifier) {
-                    $parameters[] = new Model\Parameter(\is_string($parameterName) ? $parameterName : $identifier, 'path', 'Resource identifier', true, false, false, ['type' => 'string']);
+                    $parameterName = \is_string($parameterName) ? $parameterName : $identifier;
+                    $parameter = new Model\Parameter($parameterName, 'path', 'Resource identifier', true, false, false, ['type' => 'string']);
+                    if ($this->hasParameter($parameter, $parameters)) {
+                        continue;
+                    }
+
+                    $parameters[] = $parameter;
                 }
                 $links[$operationId] = $this->getLink($resourceClass, $operationId, $path);
             } elseif (OperationType::COLLECTION === $operationType && 'GET' === $method) {
-                $parameters = array_merge($parameters, $this->getPaginationParameters($resourceMetadata, $operationName), $this->getFiltersParameters($resourceMetadata, $operationName, $resourceClass));
+                foreach (array_merge($this->getPaginationParameters($resourceMetadata, $operationName), $this->getFiltersParameters($resourceMetadata, $operationName, $resourceClass)) as $parameter) {
+                    if ($this->hasParameter($parameter, $parameters)) {
+                        continue;
+                    }
+
+                    $parameters[] = $parameter;
+                }
             } elseif (OperationType::SUBRESOURCE === $operationType) {
                 foreach ($operation['identifiers'] as $parameterName => [$class, $property]) {
-                    $parameters[] = new Model\Parameter($parameterName, 'path', $this->resourceMetadataFactory->create($class)->getShortName().' identifier', true, false, false, ['type' => 'string']);
+                    $parameter = new Model\Parameter($parameterName, 'path', $this->resourceMetadataFactory->create($class)->getShortName().' identifier', true, false, false, ['type' => 'string']);
+                    if ($this->hasParameter($parameter, $parameters)) {
+                        continue;
+                    }
+
+                    $parameters[] = $parameter;
                 }
 
                 if ($operation['collection']) {
                     $subresourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
-                    $parameters = array_merge($parameters, $this->getPaginationParameters($resourceMetadata, $operationName), $this->getFiltersParameters($subresourceMetadata, $operationName, $resourceClass));
+                    foreach (array_merge($this->getPaginationParameters($resourceMetadata, $operationName), $this->getFiltersParameters($subresourceMetadata, $operationName, $resourceClass)) as $parameter) {
+                        if ($this->hasParameter($parameter, $parameters)) {
+                            continue;
+                        }
+
+                        $parameters[] = $parameter;
+                    }
                 }
             }
 
@@ -241,9 +266,9 @@ private function collectPaths(ResourceMetadata $resourceMetadata, string $resour
             } elseif ('PUT' === $method || 'POST' === $method || 'PATCH' === $method) {
                 $operationInputSchemas = [];
                 foreach ($requestMimeTypes as $operationFormat) {
-                    $operationInputSchema = $this->jsonSchemaFactory->buildSchema($resourceClass, $operationFormat, Schema::TYPE_INPUT, $operationType, $operationName, new Schema('openapi'), null, $forceSchemaCollection);
-                    $schemas += $operationInputSchema->getDefinitions()->getArrayCopy();
+                    $operationInputSchema = $this->jsonSchemaFactory->buildSchema($resourceClass, $operationFormat, Schema::TYPE_INPUT, $operationType, $operationName, $schema, null, $forceSchemaCollection);
                     $operationInputSchemas[$operationFormat] = $operationInputSchema;
+                    $this->appendSchemaDefinitions($schemas, $operationInputSchema->getDefinitions());
                 }
 
                 $requestBody = new Model\RequestBody(sprintf('The %s %s resource', 'POST' === $method ? 'new' : 'updated', $resourceShortName), $this->buildContent($requestMimeTypes, $operationInputSchemas), true);
@@ -396,7 +421,7 @@ private function getFiltersParameters(ResourceMetadata $resourceMetadata, string
                     $schema,
                     'array' === $schema['type'] && \in_array($data['type'],
                     [Type::BUILTIN_TYPE_ARRAY, Type::BUILTIN_TYPE_OBJECT], true) ? 'deepObject' : 'form',
-                    'array' === $schema['type'],
+                    $data['openapi']['explode'] ?? ('array' === $schema['type']),
                     $data['openapi']['allowReserved'] ?? false,
                     $data['openapi']['example'] ?? null,
                     isset($data['openapi']['examples']
@@ -486,4 +511,25 @@ private function getSecuritySchemes(): array
 
         return $securitySchemes;
     }
+
+    private function appendSchemaDefinitions(\ArrayObject $schemas, \ArrayObject $definitions): void
+    {
+        foreach ($definitions as $key => $value) {
+            $schemas[$key] = $value;
+        }
+    }
+
+    /**
+     * @param Model\Parameter[] $parameters
+     */
+    private function hasParameter(Model\Parameter $parameter, array $parameters): bool
+    {
+        foreach ($parameters as $existingParameter) {
+            if ($existingParameter->getName() === $parameter->getName() && $existingParameter->getIn() === $parameter->getIn()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
diff --git a/src/OpenApi/Model/Encoding.php b/src/OpenApi/Model/Encoding.php
@@ -52,11 +52,21 @@ public function canExplode(): bool
         return $this->explode;
     }
 
+    public function getExplode(): bool
+    {
+        return $this->explode;
+    }
+
     public function canAllowReserved(): bool
     {
         return $this->allowReserved;
     }
 
+    public function getAllowReserved(): bool
+    {
+        return $this->allowReserved;
+    }
+
     public function withContentType(string $contentType): self
     {
         $clone = clone $this;
diff --git a/src/OpenApi/Model/Parameter.php b/src/OpenApi/Model/Parameter.php
@@ -83,7 +83,7 @@ public function getDeprecated(): bool
         return $this->deprecated;
     }
 
-    public function canAllowEmptyValue(): bool
+    public function getAllowEmptyValue(): bool
     {
         return $this->allowEmptyValue;
     }
@@ -103,11 +103,21 @@ public function canExplode(): bool
         return $this->explode;
     }
 
+    public function getExplode(): bool
+    {
+        return $this->explode;
+    }
+
     public function canAllowReserved(): bool
     {
         return $this->allowReserved;
     }
 
+    public function getAllowReserved(): bool
+    {
+        return $this->allowReserved;
+    }
+
     public function getExample()
     {
         return $this->example;
diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php
@@ -372,7 +372,7 @@ public function testGetClassType(): void
     {
         $schemaFactoryProphecy = $this->prophesize(SchemaFactoryInterface::class);
 
-        $schemaFactoryProphecy->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, null, null, Argument::type(Schema::class), ['foo' => 'bar'])->will(function (array $args) {
+        $schemaFactoryProphecy->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, null, null, Argument::type(Schema::class), ['foo' => 'bar'], false)->will(function (array $args) {
             $args[5]['$ref'] = 'ref';
 
             return $args[5];
diff --git a/tests/OpenApi/Factory/OpenApiFactoryTest.php b/tests/OpenApi/Factory/OpenApiFactoryTest.php
@@ -77,7 +77,8 @@ public function testInvoke(): void
                 'custom' => ['method' => 'HEAD', 'path' => '/foo/{id}', 'openapi_context' => [
                     'description' => 'Custom description',
                     'parameters' => [
-                        ['description' => 'Test parameter', 'name' => 'param', 'in' => 'path', 'type' => 'string', 'required' => true, 'default' => 'BOTH'],
+                        ['description' => 'Test parameter', 'name' => 'param', 'in' => 'path', 'required' => true],
+                        ['description' => 'Replace parameter', 'name' => 'id', 'in' => 'path', 'required' => true, 'schema' => ['type' => 'string', 'format' => 'uuid']],
                     ],
                     'tags' => ['Dummy', 'Profile'],
                     'responses' => [
@@ -117,7 +118,11 @@ public function testInvoke(): void
                 'formats' => ['method' => 'PUT', 'path' => '/formatted/{id}', 'output_formats' => ['json' => ['application/json'], 'csv' => ['text/csv']], 'input_formats' => ['json' => ['application/json'], 'csv' => ['text/csv']]],
             ],
             [
-                'get' => ['method' => 'GET'] + self::OPERATION_FORMATS,
+                'get' => ['method' => 'GET', 'openapi_context' => [
+                    'parameters' => [
+                        ['description' => 'Test modified collection page number', 'name' => 'page', 'in' => 'query', 'required' => false, 'schema' => ['type' => 'integer', 'default' => 1], 'allowEmptyValue' => true],
+                    ],
+                ]] + self::OPERATION_FORMATS,
                 'post' => ['method' => 'POST'] + self::OPERATION_FORMATS,
                 'filtered' => ['method' => 'GET', 'filters' => ['f1', 'f2', 'f3', 'f4', 'f5'], 'path' => '/filtered'] + self::OPERATION_FORMATS,
                 'paginated' => ['method' => 'GET', 'pagination_client_enabled' => true, 'pagination_client_items_per_page' => true, 'pagination_items_per_page' => 20, 'pagination_maximum_items_per_page' => 80, 'path' => '/paginated'] + self::OPERATION_FORMATS,
@@ -153,7 +158,7 @@ public function testInvoke(): void
                 'type' => 'string',
                 'required' => true,
                 'strategy' => 'exact',
-                'openapi' => ['example' => 'bar', 'deprecated' => true, 'allowEmptyValue' => true, 'allowReserved' => true],
+                'openapi' => ['example' => 'bar', 'deprecated' => true, 'allowEmptyValue' => true, 'allowReserved' => true, 'explode' => true],
             ]]),
             'f2' => new DummyFilter(['ha' => [
                 'property' => 'foo',
@@ -306,7 +311,7 @@ public function testInvoke(): void
             'Retrieves the collection of Dummy resources.',
             null,
             [
-                new Model\Parameter('page', 'query', 'The collection page number', false, false, true, [
+                new Model\Parameter('page', 'query', 'Test modified collection page number', false, false, true, [
                     'type' => 'integer',
                     'default' => 1,
                 ]),
@@ -434,7 +439,7 @@ public function testInvoke(): void
             'Dummy',
             'Custom description',
             null,
-            [new Model\Parameter('param', 'path', 'Test parameter', true), new Model\Parameter('id', 'path', 'Resource identifier', true, false, false, ['type' => 'string'])],
+            [new Model\Parameter('param', 'path', 'Test parameter', true), new Model\Parameter('id', 'path', 'Replace parameter', true, false, false, ['type' => 'string', 'format' => 'uuid'])],
             new Model\RequestBody('Custom request body', new \ArrayObject([
                 'multipart/form-data' => [
                     'schema' => [
@@ -512,7 +517,7 @@ public function testInvoke(): void
                 ]),
                 new Model\Parameter('name', 'query', '', true, true, true, [
                     'type' => 'string',
-                ], 'form', false, true, 'bar'),
+                ], 'form', true, true, 'bar'),
                 new Model\Parameter('ha', 'query', '', false, false, true, [
                     'type' => 'integer',
                 ]),

Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,7 @@ public function buildSchema(string $className, string $format = 'json', string $`
`80`	`80`	`}`
`81`	`81`
`82`	`82`	`$version = $schema->getVersion();`
`83`		`- $definitionName = $this->buildDefinitionName($className, $format, $type, $operationType, $operationName, $serializerContext);`
	`83`	`+ $definitionName = $this->buildDefinitionName($className, $format, $inputOrOutputClass, $resourceMetadata, $serializerContext);`
`84`	`84`
`85`	`85`	`if (null === $operationType \|\| null === $operationName) {`
`86`	`86`	`$method = Schema::TYPE_INPUT === $type ? 'POST' : 'GET';`
`@@ -239,10 +239,8 @@ private function buildPropertySchema(Schema $schema, string $definitionName, str`
`239`	`239`	`$schema->getDefinitions()[$definitionName]['properties'][$normalizedPropertyName] = $propertySchema;`
`240`	`240`	`}`
`241`	`241`
`242`		`- private function buildDefinitionName(string $className, string $format = 'json', string $type = Schema::TYPE_OUTPUT, ?string $operationType = null, ?string $operationName = null, ?array $serializerContext = null): string`
	`242`	`+ private function buildDefinitionName(string $className, string $format = 'json', ?string $inputOrOutputClass = null, ?ResourceMetadata $resourceMetadata = null, ?array $serializerContext = null): string`
`243`	`243`	`{`
`244`		`- [$resourceMetadata, $serializerContext,, $inputOrOutputClass] = $this->getMetadata($className, $type, $operationType, $operationName, $serializerContext);`
`245`		`-`
`246`	`244`	`$prefix = $resourceMetadata ? $resourceMetadata->getShortName() : (new \ReflectionClass($className))->getShortName();`
`247`	`245`	`if (null !== $inputOrOutputClass && $className !== $inputOrOutputClass) {`
`248`	`246`	`$parts = explode('\\', $inputOrOutputClass);`
Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ private function getClassType(?string $className, string $format, ?bool $readabl`
`142`	`142`	`throw new \LogicException('The schema factory must be injected by calling the "setSchemaFactory" method.');`
`143`	`143`	`}`
`144`	`144`
`145`		`- $subSchema = $this->schemaFactory->buildSchema($className, $format, Schema::TYPE_OUTPUT, null, null, $subSchema, $serializerContext);`
	`145`	`+ $subSchema = $this->schemaFactory->buildSchema($className, $format, Schema::TYPE_OUTPUT, null, null, $subSchema, $serializerContext, false);`
`146`	`146`
`147`	`147`	`return ['$ref' => $subSchema['$ref']];`
`148`	`148`	`}`
Original file line number	Diff line number	Diff line change
`@@ -52,11 +52,21 @@ public function canExplode(): bool`
`52`	`52`	`return $this->explode;`
`53`	`53`	`}`
`54`	`54`
	`55`	`+ public function getExplode(): bool`
	`56`	`+ {`
	`57`	`+ return $this->explode;`
	`58`	`+ }`
	`59`	`+`
`55`	`60`	`public function canAllowReserved(): bool`
`56`	`61`	`{`
`57`	`62`	`return $this->allowReserved;`
`58`	`63`	`}`
`59`	`64`
	`65`	`+ public function getAllowReserved(): bool`
	`66`	`+ {`
	`67`	`+ return $this->allowReserved;`
	`68`	`+ }`
	`69`	`+`
`60`	`70`	`public function withContentType(string $contentType): self`
`61`	`71`	`{`
`62`	`72`	`$clone = clone $this;`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ public function getDeprecated(): bool`
`83`	`83`	`return $this->deprecated;`
`84`	`84`	`}`
`85`	`85`
`86`		`- public function canAllowEmptyValue(): bool`
	`86`	`+ public function getAllowEmptyValue(): bool`
`87`	`87`	`{`
`88`	`88`	`return $this->allowEmptyValue;`
`89`	`89`	`}`
`@@ -103,11 +103,21 @@ public function canExplode(): bool`
`103`	`103`	`return $this->explode;`
`104`	`104`	`}`
`105`	`105`
	`106`	`+ public function getExplode(): bool`
	`107`	`+ {`
	`108`	`+ return $this->explode;`
	`109`	`+ }`
	`110`	`+`
`106`	`111`	`public function canAllowReserved(): bool`
`107`	`112`	`{`
`108`	`113`	`return $this->allowReserved;`
`109`	`114`	`}`
`110`	`115`
	`116`	`+ public function getAllowReserved(): bool`
	`117`	`+ {`
	`118`	`+ return $this->allowReserved;`
	`119`	`+ }`
	`120`	`+`
`111`	`121`	`public function getExample()`
`112`	`122`	`{`
`113`	`123`	`return $this->example;`
Original file line number	Diff line number	Diff line change
`@@ -372,7 +372,7 @@ public function testGetClassType(): void`
`372`	`372`	`{`
`373`	`373`	`$schemaFactoryProphecy = $this->prophesize(SchemaFactoryInterface::class);`
`374`	`374`
`375`		`- $schemaFactoryProphecy->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, null, null, Argument::type(Schema::class), ['foo' => 'bar'])->will(function (array $args) {`
	`375`	`+ $schemaFactoryProphecy->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, null, null, Argument::type(Schema::class), ['foo' => 'bar'], false)->will(function (array $args) {`
`376`	`376`	`$args[5]['$ref'] = 'ref';`
`377`	`377`
`378`	`378`	`return $args[5];`