Skip to content
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.

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

fix-openapi-subresource #3690

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions src/Operation/Factory/SubresourceOperationFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,20 +104,21 @@ private function computeSubresourceOperations(string $resourceClass, array &$tre
if (null === $parentOperation) {
$rootShortname = $rootResourceMetadata->getShortName();
$operation['identifiers'] = [['id', $rootResourceClass, true]];
$operation['operation_name'] = sprintf(

$suffix = sprintf(
'%s_%s%s',
RouteNameGenerator::inflector($operation['property'], $operation['collection'] ?? false),
$operationName,
self::SUBRESOURCE_SUFFIX
);

$subresourceOperation = $rootResourceMetadata->getSubresourceOperations()[$operation['operation_name']] ?? [];
$subresourceOperation = $rootResourceMetadata->getSubresourceOperations()[$suffix] ?? [];

$operation['route_name'] = sprintf(
$operation['operation_name'] = $operation['route_name'] = sprintf(
'%s%s_%s',
RouteNameGenerator::ROUTE_NAME_PREFIX,
RouteNameGenerator::inflector($rootShortname),
$operation['operation_name']
$suffix
);

$prefix = trim(trim($rootResourceMetadata->getAttribute('route_prefix', '')), '/');
Expand Down
2 changes: 1 addition & 1 deletion src/Swagger/Serializer/DocumentationNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ private function addPaginationParameters(bool $v3, ResourceMetadata $resourceMet
*/
private function addSubresourceOperation(bool $v3, array $subresourceOperation, \ArrayObject $definitions, string $operationId, ResourceMetadata $resourceMetadata): \ArrayObject
{
$operationName = 'get'; // TODO: we might want to extract that at some point to also support other subresource operations
$operationName = $subresourceOperation['operation_name']; // TODO: we might want to extract that at some point to also support other subresource operations
$collection = $subresourceOperation['collection'] ?? false;

$subResourceMetadata = $this->resourceMetadataFactory->create($subresourceOperation['resource_class']);
Expand Down
58 changes: 29 additions & 29 deletions tests/Operation/Factory/SubresourceOperationFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function testCreate()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresource.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
Copy link
Member

Choose a reason for hiding this comment

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

isn't this a BC break?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, IMO yes and no. Yes because, even if generated, it could have been used. But no because I think it's a bug and that it should always have been like that. And no because subresources are experimental and that part is internal.

Copy link
Member

Choose a reason for hiding this comment

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

It should at least be documented in the changelog as a BC break.

] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subresource_another_subresource_get_subresource' => [
'property' => 'anotherSubresource',
Expand All @@ -88,7 +88,7 @@ public function testCreate()
],
'route_name' => 'api_dummy_entities_subresource_another_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresource/another_subresource.{_format}',
'operation_name' => 'subresource_another_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_another_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subresource_another_subresource_subcollections_get_subresource' => [
'property' => 'subcollection',
Expand All @@ -102,7 +102,7 @@ public function testCreate()
],
'route_name' => 'api_dummy_entities_subresource_another_subresource_subcollections_get_subresource',
'path' => '/dummy_entities/{id}/subresource/another_subresource/subcollections.{_format}',
'operation_name' => 'subresource_another_subresource_subcollections_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_another_subresource_subcollections_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subcollections_get_subresource' => [
'property' => 'subcollection',
Expand All @@ -114,7 +114,7 @@ public function testCreate()
],
'route_name' => 'api_dummy_entities_subcollections_get_subresource',
'path' => '/dummy_entities/{id}/subcollections.{_format}',
'operation_name' => 'subcollections_get_subresource',
'operation_name' => 'api_dummy_entities_subcollections_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subcollections_another_subresource_get_subresource' => [
'property' => 'anotherSubresource',
Expand All @@ -127,7 +127,7 @@ public function testCreate()
],
'route_name' => 'api_dummy_entities_subcollections_another_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subcollections/{subcollection}/another_subresource.{_format}',
'operation_name' => 'subcollections_another_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subcollections_another_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subcollections_another_subresource_subresource_get_subresource' => [
'property' => 'subresource',
Expand All @@ -141,7 +141,7 @@ public function testCreate()
],
'route_name' => 'api_dummy_entities_subcollections_another_subresource_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subcollections/{subcollection}/another_subresource/subresource.{_format}',
'operation_name' => 'subcollections_another_subresource_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subcollections_another_subresource_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand All @@ -154,7 +154,7 @@ public function testCreateByOverriding()
'subcollections_get_subresource' => [
'path' => '/dummy_entities/{id}/foobars',
],
'subcollections_another_subresource_get_subresource' => [
'api_dummy_entities_subcollections_another_subresource_get_subresource' => [
'path' => '/dummy_entities/{id}/foobars/{subcollection}/another_foobar.{_format}',
],
]));
Expand Down Expand Up @@ -193,7 +193,7 @@ public function testCreateByOverriding()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresource.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subresource_another_subresource_get_subresource' => [
'property' => 'anotherSubresource',
Expand All @@ -206,7 +206,7 @@ public function testCreateByOverriding()
],
'route_name' => 'api_dummy_entities_subresource_another_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresource/another_subresource.{_format}',
'operation_name' => 'subresource_another_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_another_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subresource_another_subresource_subcollections_get_subresource' => [
'property' => 'subcollection',
Expand All @@ -220,7 +220,7 @@ public function testCreateByOverriding()
],
'route_name' => 'api_dummy_entities_subresource_another_subresource_subcollections_get_subresource',
'path' => '/dummy_entities/{id}/subresource/another_subresource/subcollections.{_format}',
'operation_name' => 'subresource_another_subresource_subcollections_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_another_subresource_subcollections_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subcollections_get_subresource' => [
'property' => 'subcollection',
Expand All @@ -232,7 +232,7 @@ public function testCreateByOverriding()
],
'route_name' => 'api_dummy_entities_subcollections_get_subresource',
'path' => '/dummy_entities/{id}/foobars',
'operation_name' => 'subcollections_get_subresource',
'operation_name' => 'api_dummy_entities_subcollections_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subcollections_another_subresource_get_subresource' => [
'property' => 'anotherSubresource',
Expand All @@ -245,7 +245,7 @@ public function testCreateByOverriding()
],
'route_name' => 'api_dummy_entities_subcollections_another_subresource_get_subresource',
'path' => '/dummy_entities/{id}/foobars/{subcollection}/another_foobar.{_format}',
'operation_name' => 'subcollections_another_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subcollections_another_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subcollections_another_subresource_subresource_get_subresource' => [
'property' => 'subresource',
Expand All @@ -259,7 +259,7 @@ public function testCreateByOverriding()
],
'route_name' => 'api_dummy_entities_subcollections_another_subresource_subresource_get_subresource',
'path' => '/dummy_entities/{id}/foobars/{subcollection}/another_foobar/subresource.{_format}',
'operation_name' => 'subcollections_another_subresource_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subcollections_another_subresource_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -304,7 +304,7 @@ public function testCreateWithMaxDepth()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresource.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -364,7 +364,7 @@ public function testCreateWithMaxDepthMultipleSubresources()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresources.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_second_subresource_get_subresource' => [
'property' => 'secondSubresource',
Expand All @@ -376,7 +376,7 @@ public function testCreateWithMaxDepthMultipleSubresources()
],
'route_name' => 'api_dummy_entities_second_subresource_get_subresource',
'path' => '/dummy_entities/{id}/second_subresources.{_format}',
'operation_name' => 'second_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_second_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_second_subresource_more_subresource_get_subresource' => [
'property' => 'moreSubresource',
Expand All @@ -389,7 +389,7 @@ public function testCreateWithMaxDepthMultipleSubresources()
],
'route_name' => 'api_dummy_entities_second_subresource_more_subresource_get_subresource',
'path' => '/dummy_entities/{id}/second_subresources/mode_subresources.{_format}',
'operation_name' => 'second_subresource_more_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_second_subresource_more_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -448,7 +448,7 @@ public function testCreateWithMaxDepthMultipleSubresourcesSameMaxDepth()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresources.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_second_subresource_get_subresource' => [
'property' => 'secondSubresource',
Expand All @@ -460,7 +460,7 @@ public function testCreateWithMaxDepthMultipleSubresourcesSameMaxDepth()
],
'route_name' => 'api_dummy_entities_second_subresource_get_subresource',
'path' => '/dummy_entities/{id}/second_subresources.{_format}',
'operation_name' => 'second_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_second_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -503,7 +503,7 @@ public function testCreateSelfReferencingSubresources()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresources.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -555,7 +555,7 @@ public function testCreateWithDifferentMaxDepthSelfReferencingSubresources()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresources.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subresource_second_subresource_get_subresource' => [
'property' => 'secondSubresource',
Expand All @@ -568,7 +568,7 @@ public function testCreateWithDifferentMaxDepthSelfReferencingSubresources()
],
'route_name' => 'api_dummy_entities_subresource_second_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresources/second_subresources.{_format}',
'operation_name' => 'subresource_second_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_second_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_second_subresource_get_subresource' => [
'property' => 'secondSubresource',
Expand All @@ -580,7 +580,7 @@ public function testCreateWithDifferentMaxDepthSelfReferencingSubresources()
],
'route_name' => 'api_dummy_entities_second_subresource_get_subresource',
'path' => '/dummy_entities/{id}/second_subresources.{_format}',
'operation_name' => 'second_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_second_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -625,7 +625,7 @@ public function testCreateWithEnd()
],
'route_name' => 'api_dummy_entities_subresources_get_subresource',
'path' => '/dummy_entities/{id}/subresource.{_format}',
'operation_name' => 'subresources_get_subresource',
'operation_name' => 'api_dummy_entities_subresources_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_subresources_item_get_subresource' => [
'property' => 'id',
Expand All @@ -638,7 +638,7 @@ public function testCreateWithEnd()
],
'route_name' => 'api_dummy_entities_subresources_item_get_subresource',
'path' => '/dummy_entities/{id}/subresource/{subresource}.{_format}',
'operation_name' => 'subresources_item_get_subresource',
'operation_name' => 'api_dummy_entities_subresources_item_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $result);
}
Expand Down Expand Up @@ -683,7 +683,7 @@ public function testCreateWithEndButNoCollection()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresource.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $result);
}
Expand Down Expand Up @@ -728,7 +728,7 @@ public function testCreateWithRootResourcePrefix()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/root_resource_prefix/dummy_entities/{id}/subresource.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down Expand Up @@ -777,7 +777,7 @@ public function testCreateSelfReferencingSubresourcesWithSubresources()
],
'route_name' => 'api_dummy_entities_subresource_get_subresource',
'path' => '/dummy_entities/{id}/subresources.{_format}',
'operation_name' => 'subresource_get_subresource',
'operation_name' => 'api_dummy_entities_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
'api_dummy_entities_other_subresource_get_subresource' => [
'property' => 'otherSubresource',
Expand All @@ -789,7 +789,7 @@ public function testCreateSelfReferencingSubresourcesWithSubresources()
],
'route_name' => 'api_dummy_entities_other_subresource_get_subresource',
'path' => '/dummy_entities/{id}/other_subresources.{_format}',
'operation_name' => 'other_subresource_get_subresource',
'operation_name' => 'api_dummy_entities_other_subresource_get_subresource',
] + SubresourceOperationFactory::ROUTE_OPTIONS,
], $subresourceOperationFactory->create(DummyEntity::class));
}
Expand Down
Loading