-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Consider callbacks in unused schemas #1232
Consider callbacks in unused schemas #1232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. I'm not that familiar with callbacks, so I don't know how I'd go about testing this. I'll leave the evaluation from that perspective to someone else with experience in callback usages.
if (allOperations != null) { | ||
for (Operation operation : allOperations) { | ||
//Params: | ||
if (operation.getParameters() != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know most of the code here was just reorganized, but I just had a question. I'm wondering why we follow visitor pattern only for a few elements (visitSchema
) but not others which are commented here like "Params", "RequestBody", "Responses", and "Callbacks"? It seems like this would make for smaller surface area for unit testing edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jimschubert: I never forgot your comment about having a true visitor Pattern for an OpenAPI spec.
I have implemented it in OpenAPITools/empoa#14
Thank you for the review. This PR is mainly about taking into account the Schema that can be referenced in PathItems of Callbacks. I did not change anything else. Of course a proper/complete visitor pattern would be better. This might also be an overkill for this use case. In the List<String> getAllUsedSchemas(OpenAPI openAPI)
List<String> getUnusedSchemas(OpenAPI openAPI)
List<String> getSchemasUsedOnlyInFormParam(OpenAPI openAPI) |
It was not possible to add it in OpenAPITools#1232 because of issue swagger-api/swagger-parser#879
It was not possible to add it in #1232 because of issue swagger-api/swagger-parser#879
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.master
,3.4.x
,4.0.x
. Default:master
.Description of the PR
Callbacks can reference Schemas. The
ModelUtils
methods that compute used and unused schemas should visit the callbacks.Callbacks effort in general is tracked as #372.