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

OperationIds are no longer optional since the addition of validation in version 4.2.10 #1153

Closed
JorickPepin opened this issue Mar 6, 2022 · 4 comments · Fixed by #1154
Closed

Comments

@JorickPepin
Copy link

As noted here nelmio/NelmioApiDocBundle#1967, the optional aspect of operationIds indicated in the specifications is no longer respected since the addition of the operationId uniqueness check in version 4.2.10 (#1149).

As I understand it, these identifiers must be unique if they are specified which is not mandatory. If this is the case, then null values should not be taken into account in the validation.

@DerManoMann
Copy link
Collaborator

Well, but that is the thing - they are specified. Optional means they are not in the spec. Having operationId: null in the spec does explicity set it and if that is done more than once it is duplicate.

The cause for this is here: https://github.com/nelmio/NelmioApiDocBundle/blob/master/OpenApiPhp/DefaultOperationId.php#L32.

Generator::UNDEFINED is a 'magic' value in swagger-php to mark a property as not set. In that case the property is
ommitted from the output - null is not.

I think the reason for this 'hack' was that historically operation ids were not always unique when swagger-php generated it. However, this has changed (I think).

Furthermore, there is a simpler way of avoiding operation ids altogether and that is to drop the OperationId processor in swagger-php.

Having said all that, I appreciate that operationId is a somewhat special case. Also, the spec defines it as string, so null is an invalid value in the first place....

@JorickPepin
Copy link
Author

Thank you for the explanations and the modification 👍

@GuilhemN
Copy link
Contributor

I think the issue originates in the fact that the null value used to be a special value in zircote/swagger-php for operationId (here 70316e3#diff-95de59e4b4f717f2f8872b2209151efd4cfe46c688762478b229d88a10aaea22L166-L168), it allowed us to prevent the OperationId processor from defining an operationId.

We need this because we generate a lot of annotations in NelmioApiDocBundle, and we don't always have a file/method/line to provide to the context so it unfortunately doesn't generate unique operation ids in our case.

The issue was mitigated by nelmio/NelmioApiDocBundle#1907 and I left the complete fix for later. If you don't like having a special case for the null value we could indeed also remove the OperationId processor on our end, let me know and we can do the change in NelmioApiDocBundle :)

@GuilhemN
Copy link
Contributor

Oh actually I just checked #1154 and this won't do for our case as we used null to bypass the OperationId processor. I guess we should just remove it then, right?

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 a pull request may close this issue.

3 participants