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

Enhancement Request: Provide a fixed order for responses #1802

Closed
dgf opened this issue Apr 17, 2024 · 6 comments
Closed

Enhancement Request: Provide a fixed order for responses #1802

dgf opened this issue Apr 17, 2024 · 6 comments

Comments

@dgf
Copy link

dgf commented Apr 17, 2024

We've encountered some challenges while using definitions generated by Quarkus in our CI workflow. Constantly/randomly it changes the order of the responses in the generated OpenAPI JSON and YML files. This has various side effects and requires manual reviews in our processes.

The cause is the generation of the responses section itself. The function io.smallrye.openapi.runtime.io.response.ResponseWriter#writeAPIResponses loops over the entry set of responses without any sorting.

We fixed it for now in our env by deploying a SNAPSHOT that sorts the set with Map.Entry.comparingByKey()

Its tested and running well already for some dozen builds and we would like to see the stabilization for the export of definitions in the standard.

To do that we/you need to decide what should be the fixed order. For now I'm seeing two different ones in the tests. Some JSON based expectation files like core/src/test/resources/io/smallrye/openapi/runtime/io/_everything.json containing the responses in alphabetical order "default", "200", "404" and the some YML assertions like in the test io.smallrye.openapi.runtime.io.OpenApiParserAndSerializerTest#testParsingBigStaticYamlFile are reversed: "204", "200" and "404", "200"

What is the expected order? Are there any notes about in the specs or a best practice?

We can easily create a PR, as we have a fix ready (just need to decide if forward or backwards sorted).

@dgf dgf changed the title Enhancement Request: Provide a fixed order for responses+++ Enhancement Request: Provide a fixed order for responses Apr 17, 2024
@phillip-kruger
Copy link
Member

Please provide that PR and we can discuss there. Thanks

@dgf
Copy link
Author

dgf commented Apr 18, 2024

oh dear, I've just discovered that the entire core package has been refactored. I created the fix based on 3.10.0 . So I'll have to start again, but first I'll have to familiarize myself with the new JSON handling.

@MikeEdgar
Copy link
Member

Hi @dgf , how large were your changes? Even if you can post a link to a git commit here, we can at least discuss the approach before re-basing on the latest main branch.

@dgf
Copy link
Author

dgf commented Apr 18, 2024

I have rebuilt the changes to the current version. Unfortunately, I can't test this with Quarkus at the moment because the class references have changed. But that should be sufficient as a basis for further exchanges about that topic.

@dgf
Copy link
Author

dgf commented Apr 24, 2024

Solved on Quarkus itself by using a sorted map, see quarkusio/quarkus#40222

In general, I still think it's a good idea to have a predefined order for responses, but no longer necessary, at least, for our specific case.

@phillip-kruger
Copy link
Member

Closing here. Thanks

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

No branches or pull requests

3 participants