Skip to content

feature: ensure stable order for sorted-parameters.enable=false - #2645

Closed
Postremus wants to merge 1 commit into
smallrye:mainfrom
Postremus:issues/2630-add-test-param-order-unsorted
Closed

feature: ensure stable order for sorted-parameters.enable=false#2645
Postremus wants to merge 1 commit into
smallrye:mainfrom
Postremus:issues/2630-add-test-param-order-unsorted

Conversation

@Postremus

@Postremus Postremus commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

sorted-parameters.enable=false allows to preserve definition order of parameters into the final openapi document.
Jandex first scans for annotations (our source of parameters) on fields, then on methods.
The output was therefore never in definition order.
Compare org.jboss.jandex.Indexer.indexWithSummary, which calls processFieldInfo before processMethodInfo.
(matchin jvms, which already sorts fields before methods https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html)

For openapi this means, we can add a test which makes sure this order stays stable with sorted-parameters.enable=false.

One hiccup was though, that the keys of ClassInfo.annotationsmap are not sorted in any particular way. The order changed mostly when adding a new kind of parameter type, e.g. adding header params on a resource where previously only query and path params where present

I therefore added an explicit sort on the In parameter in the sorted-parameters.enable=false case. This means though that we might break the currently existing order for existing apis.

Related to #2630. The next changeset basically requires that we agree upon a stable ordering.

@Postremus
Postremus force-pushed the issues/2630-add-test-param-order-unsorted branch from 06e82fc to 8ba1667 Compare August 29, 2026 18:40
@Postremus

Postremus commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Draft; I might want to expand the tests further.
Might be I interesting to explicitly add parameters sourced from method annotations to the test.

sorted-parameters.enable=false allows to preserve definition order of parameters into the final openapi document.
Jandex first scans for annotations (our source of parameters) on fields, then on methods.
The output was therefore never in definition order.
Compare org.jboss.jandex.Indexer.indexWithSummary, which calls processFieldInfo before processMethodInfo.
(matchin jvms, which already sorts fields before methods https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html)

For openapi this means, we can add a test which makes sure this order stays stable with sorted-parameters.enable=false.

One hiccup was though, that the keys of ClassInfo.annotationsmap are not sorted in any particular way. The order changed mostly when adding a new kind of parameter type, e.g. adding header params on a resource where previously only query and path params where present. Now the path parameters stay sorted first, then the newly added header params, and then the query params.
-> At least the order is now defined.

I therefore added an explicit sort on the In parameter in the sorted-parameters.enable=false case. This means though that we might break the currently existing order for existing apis.
@Postremus
Postremus force-pushed the issues/2630-add-test-param-order-unsorted branch from 8ba1667 to 7fc32ba Compare August 30, 2026 15:01
@Postremus

Copy link
Copy Markdown
Contributor Author

Ok I expanded the tests a bit more. Should be ready now.

@Postremus

Copy link
Copy Markdown
Contributor Author

Thinking some more. This approach might not be that great.

Before, at least the method parameters (of the resource method) where sorted in declaration order.
Only parameters from the class or from beanparams where grouped by in.

Now however everything is grouped. Within each group the parameters are in declaration order.

Not sure if that is a nicer behaviour.

I will try out if I can get this PR to emit every parameter in declaration order.

Ok I just checked. I will drop this PR. My next changeset ensures declaration order for free. I will pull the test changes into the next PR.

@Postremus Postremus closed this Aug 30, 2026
@Postremus
Postremus deleted the issues/2630-add-test-param-order-unsorted branch August 30, 2026 16:24
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.

1 participant