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

Refactor the management of operations by the swagger DocumentationNormalizer #648

Merged
merged 3 commits into from Aug 1, 2016
Merged

Conversation

GuilhemN
Copy link
Contributor

@GuilhemN GuilhemN commented Jul 29, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Initial post:

I found two issues in the swagger DocumentationNormalizer.
The first one, custom operations always made the normalizer creates a standard operation even if it didn't exist.
The second one, if two or more custom operations had the same path, they conflicted and only the last one was added to the doc.

In fact I do not even understand why some operations are treated as custom so this PR simplify the DocumentationNormalizer by removing this notion and threat all operations at once.

@GuilhemN GuilhemN changed the title [DocumentationNormalizer] Fix management of custom operations Simplify the management of operation by the swagger DocumentationNormalizer Jul 29, 2016
@GuilhemN GuilhemN changed the title Simplify the management of operation by the swagger DocumentationNormalizer Simplify the management of operations by the swagger DocumentationNormalizer Jul 29, 2016
@GuilhemN GuilhemN changed the title Simplify the management of operations by the swagger DocumentationNormalizer Refactor the management of operations by the swagger DocumentationNormalizer Jul 30, 2016
@GuilhemN
Copy link
Contributor Author

The latest commit made the IriConverter a soft dependency in case there are only custom operations.
I'm wondering if the path should not always be included in the metadata ? That way the IriConverter wouldn't be needed at all in the DocumentationNormalizer and its limitation would be fixed (it doesn't work if a resource has no collection operations).

@@ -44,7 +44,7 @@
private $operationMethodResolver;
private $iriConverter;

public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceClassResolverInterface $resourceClassResolver, OperationMethodResolverInterface $operationMethodResolver, IriConverterInterface $iriConverter)
public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceClassResolverInterface $resourceClassResolver, OperationMethodResolverInterface $operationMethodResolver, IriConverterInterface $iriConverter = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

why null ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not needed in case there are only custom operations (which should be the case when used as a component).

@dunglas
Copy link
Member

dunglas commented Jul 31, 2016

👍

Do you consider this PR ready to be merged @Ener-Getick?

@GuilhemN
Copy link
Contributor Author

@dunglas yes, it looks good to me :)

@dunglas dunglas merged commit fa190f9 into api-platform:master Aug 1, 2016
@dunglas
Copy link
Member

dunglas commented Aug 1, 2016

Thanks @Ener-Getick

@Jarablus
Copy link

Jarablus commented Aug 1, 2016

@Simperfit no more unwanted operation with this PR 👍

@GuilhemN GuilhemN deleted the DOCUMENTATION branch August 1, 2016 08:08
magarzon pushed a commit to magarzon/core that referenced this pull request Feb 12, 2017
Refactor the management of operations by the swagger DocumentationNormalizer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants