Don't automatically sort object fields

[neon-dev]

I wondered why my OpenAPI file under Thorntail didn't look like I expected it to.
Classes under components.schemas always have sorted fields instead of their original order in the Java class.
I think it's because you are using ClassInfo from Jandex internally to get the field info. Unfortunately ClassInfo does automatic field sorting: https://github.com/wildfly/jandex/blob/61d1115e0a0f15dcfb2b0bbd082100f847963529/src/main/java/org/jboss/jandex/ClassInfo.java#L557

There seems to be no option to disable this behavior. Neither in this implementation, nor in Jandex ClassInfo.
I suggest not to alter the property order or at least make it optional (default: false).
Thanks :)

[EricWittmann]

This is a reasonable suggestion, for sure. Thanks for the report.

[neon-dev]

I created an issue over at Jandex: smallrye/jandex#58
If they are willing to implement an option to the Indexer, it should be relatively easy to solve. Otherwise I see no clean way.

[EricWittmann]

Agreed.

[neon-dev]

This one is really bugging me so much that I today tried to work around it via reflection, but without success.
Since it can not easily be solved during runtime (except maybe via bytecode manipulation, which would be an overkill), I can only hope for an official implementation. Unfortunately the Jandex repo seems deserted, so we may not get feedback from there anytime soon.
Please let me know if you have any idea how to tackle this without some ugly workaround. Given a direction I'd create a PR once I find the time.
Otherwise hints would be greatly appreciated how to maybe fix it quick and dirty during runtime. My main problem is that schema generation is run so early because it's @DeploymentScoped that it's virtually impossible to intervene on time.

[kenfinnigan]

https://github.com/wildfly/jandex is most definitely still developed and worked on.

If you need something fixed there, just raise an issue and it will be looked at

[neon-dev]

I did on April 8th (see comment above) and asked for a quick update on May 17th, but without an answer and the repo did not receive commits for three months now. That's why I thought it's temporarily/no longer maintained.

[kenfinnigan]

Apologies, missed that link the first time.

I know Jason has been traveling a lot of late, but can reach out about it

[kenfinnigan]

If you're able to propose a solution a PR it's likely to get evaluated quicker

[neon-dev]

Thought about that, but since the PR section looked similar in that regard I shied away from it.
Thanks for your advice and reasonable opinions in my last issues. Certainly appreciate it and I don't want to bug anyone too much with these things, since nobody other than me seems to mind 😄

[neon-dev]

As you proposed, I created a PR over at Jandex: smallrye/jandex#61

If that gets merged and you don't want to add annotation driven control for sorting, this issue could be solved by changing one or two lines:

1. smallrye-open-api/implementation/src/main/java/io/smallrye/openapi/api/util/ArchiveUtil.java

   Line 154 in cdfbdb6

   indexer.index(contentStream);

   to indexer.index(contentStream, true, false);
2. I don't know what CollectionStandin and MapStandin are for, but in case these need to be consistent with the indexed archive, you'd need to also change this line in the same way:

   smallrye-open-api/implementation/src/main/java/io/smallrye/openapi/api/util/ArchiveUtil.java

   Line 133 in cdfbdb6

   indexer.index(klazzStream);

Otherwise implementation could become a bit messy, because we'd need to read annotations before the indexer reads the class stream.

[kenfinnigan]

I think there's a good chance the PR will be merged, and we can update open-api to use it

[MikeEdgar]

A few ideas on how to address this without only relying on the order from Jandex. These should all put the developer in control and not rely on an implementation detail.

1. Request a new String[] propertyOrder attribute in @Schema. Requires an MP-OAI change - long term solution.
2. Support for commonly-used annotations intended for ordering
   • javax.json.bind.annotation.JsonbPropertyOrder
   • javax.xml.bind.annotation.XmlType#propOrder
   • com.fasterxml.jackson.annotation.JsonPropertyOrder
3. Support a new mp.openapi.extensions.* property

If this seems like an OK approach, I am of course happy to look at implementing it. For number 3, I am thinking about using a convention similar to what was mentioned in #147.
mp.openapi.extensions.<hyphenated extension name>.<fully qualified class name>=<value>

[EricWittmann]

These are all great suggestions. My opinion is that (2) is the best in the short term. We can discuss (1) in the MPOAI community. I'm less enthusiastic about (3) only because I suspect it wouldn't be used often. But it would solve the problem where the target code cannot be modified. We can, of course, support all of these options eventually - we just need to decide on order of preference. For that, I would probably vote for (3), (1), (2), (default/current behavior).

[EricWittmann]

I do also think we should continue to try to get the change merged in jandex. I'm guessing no one disagrees with that. :)

[msavy]

Bringing this one up at the spec level seems the ideal solution to me. We should figure out the specific morphology over there (perhaps you can join us @MikeEdgar ?), but this isn't a bad suggestion.

Side node: I'm generally against adding options unless we figure we really need them; it's just going to become a maintenance and documentation pain in the long term (and eventual fun of conflicting options, and such).

(2) also seems a great idea to me, and that could be done regardless.

[EricWittmann]

Normally I agree with you @msavy about config options. But what do you think about my comment regarding code that cannot be easily modified to include another annotation? Valid use-case?

[kenfinnigan]

I'm all for pushing ideas up to the MP spec level to resolve, but let's not wait on that ceremony to include changes into the implementation

[msavy]

@kenfinnigan Yes, I think option (2) can be done immediately.

@EricWittmann You make a good point on the unmodifiable source/transitive dependencies. Although, I do wonder how wise it is for users to go down that track; it could get fragile very quickly, as there would be the propensity for the config to drift from the source and there would be no easy way to spot that (especially for those libraries out of their realm of control, so to speak).

But, that does seem a valid use-case. One suggestion might just be that we speak in the next MP OAI meeting about what the option name should be, so we can 'pre-align' that for any future addition to the spec?

(Given that field order is not guaranteed to be deterministic according to the spec, I think this area is always going to be a bit messy, even with 'natural' order).

[neon-dev]

If I might add one thing regarding unmodifiable sources. Since we are (hopefully always) talking about interfaces over which the developer has full control, it should be questioned why they would directly read or write 3rd party entities over which they may not have control (de-/serializing wise). In my opinion endpoints and their input/output should be well-defined.
I think the suggested solution for this should be to read/write mapped intermediate classes instead of introducing configurations that help realize questionable API design.

[MikeEdgar]

Bringing this one up at the spec level seems the ideal solution to me. We should figure out the specific morphology over there (perhaps you can join us @MikeEdgar ?), but this isn't a bad suggestion.

I'm happy to join in or open an issue at the spec level.

(2) also seems a great idea to me, and that could be done regardless.

I've been thinking more about this one and think there needs to be a good definition around the behavior. Also, as you said @msavy , it should pre-align with any potential standard. The situation gets a little messy when inheritance is involved, in particular. I haven't done any experimentation to see how Jackson or Yasson output looks, but the order below seems like the most intuitive at the moment.

1. Parent fields with order specified (excluding any fields that the child explicitly provides order for)
2. Child fields with order specified
3. Parent fields without order specified
4. Child fields without order specified

Additionally, I think that if a field of the same name is defined in both a parent class and a child, the one in the child should be preferred. Something else I haven't experimented with, but I think the current implementation may be using the parent class's field.

[MikeEdgar]

This issue is pending a Jandex change [smallrye/jandex#61] to support "remembering" the original order of field and method declaration in the class files used to generate the index. It may become a bigger issue at some point depending on the direction of eclipse/microprofile-open-api#359.