Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 16, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1da4c1a
OpenAPI: Make sure we do not override defined parameters #4137
soyuka Mar 15, 2021
afdbdd1
Fix #4124
soyuka Mar 15, 2021
e5cf715
Improve openapi performances
soyuka Mar 16, 2021
d57626f
fix PHPDoc
alanpoulain Mar 16, 2021
441ffa6
remove ref
soyuka Mar 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/JsonSchema/SchemaFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/JsonSchema/TypeFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']];
}
Expand Down
74 changes: 60 additions & 14 deletions src/OpenApi/Factory/OpenApiFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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(),
Expand All @@ -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));
Expand All @@ -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 = [];
Expand All @@ -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;
}
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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']
Expand Down Expand Up @@ -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;
}
}
10 changes: 10 additions & 0 deletions src/OpenApi/Model/Encoding.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 11 additions & 1 deletion src/OpenApi/Model/Parameter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function getDeprecated(): bool
return $this->deprecated;
}

public function canAllowEmptyValue(): bool
public function getAllowEmptyValue(): bool
Copy link
Contributor

@guilliamxavier guilliamxavier Apr 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BC break? Did you intend to duplicate (like the other ones below) rather than rename the existing?

Update: #4203

{
return $this->allowEmptyValue;
}
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/JsonSchema/TypeFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
17 changes: 11 additions & 6 deletions tests/OpenApi/Factory/OpenApiFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' => [
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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,
]),
Expand Down Expand Up @@ -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' => [
Expand Down Expand Up @@ -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',
]),
Expand Down