Support definition of Schema property order

[MikeEdgar]

There has been some discussion in smallrye/smallrye-open-api#87 around ways for a user of MP-OAI to define the order of properties read during annotation/class scanning. Although maybe not technically significant for formats like JSON and YAML, predicable order does help with readability and may be significant for formats like XML.

Several annotations are in common use today by the various serialization frameworks that could be looked at for an idea of how to implement the same option in MP-OAI

• javax.json.bind.annotation.JsonbPropertyOrder
• javax.xml.bind.annotation.XmlType#propOrder
• com.fasterxml.jackson.annotation.JsonPropertyOrder

One possible option is to define a new String[] propertyOrder attribute in @Schema that behaves similarly to the three format-specific annotations listed above.

Other things to consider:

1. Ordering when inheritance is involved, should align with common behavior of existing serialization frameworks (suggestions below)
   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
2. Clarity around precedence of ordering. The model reader's ordering and the static file would supersede any ordering defined in annotations
3. Potential mp.openapi configuration option to override ordering

[EricWittmann]

@MikeEdgar - would you consider joining the MP-OAI TC calls - they are normally held every other Monday at 10am your time (assuming GitHub isn't lying about your location). The next one is on the 15th.

[MikeEdgar]

@EricWittmann , I'll add it to my calendar.

[EricWittmann]

We discussed this at the most recent TC meeting this week, so I am updating this based on that discussion. That said, @MikeEdgar has already documented the options pretty well. To summarize, we think there are three basic options here:

1. Re-Use the @Schema annotation by adding a propertyOrder property to the annotation
2. Create a new MP-OpenAPI annotation to specify property ordering - something similar to one of the existing annotations listed in Mike's comment above
3. Do nothing - leave this up to implementations and don't pollute the spec

I think my preference is for (1) even though that further increases the @Schema annotation, which is already pretty complex.

That said, if (1) is the best option, then it might be best to come up with a solution that solves both this issue and issue #360

Perhaps the solution to #360 can be to introduce a new property called properties which would not only serve as the ordering of properties but also allows actually defining them. The semantics of such a thing would need to be worked out, but something like this:

    Schema[] properties() default {};

With an example application that might be something like this:

@Schema(properties={
    @Schema(name="creditCard", required=true),
    @Schema(name="departtureFlight", description="The departure flight information."),
    @Schema(name="returningFlight")
})
public class Booking {

    private Flight departtureFlight;
    private Flight returningFlight;
    private CreditCard creditCard;

This approach would allow developers to indicate the order (in this case the order would be creditCard, departtureFlight, and then returningFlight rather than the nature order of the fields in the bean) while also augmenting the schema properties with additional information like description and required. Of course, you could also put that information on the fields themselves, so some merging of information is possibly required.

As mentioned in the TC call this week, the annotations could easily become out of sync with the actual java bean code (there is nothing but convention tying them together). However, I don't really see a solution to this problem that doesn't have that flaw. The implementation could output some sort of warning, perhaps, when the array of annotations in properties doesn't match the list of fields.

There's also the issue of data type hierarchies - if Booking extends some other bean which itself has properties.

Thoughts?

[MikeEdgar]

I do like this, but like we mentioned in #360, it might need to be adjusted a bit to account for the restrictions on annotation cycles in Java.

Just to throw a few other options out there, these also popped into my head and I figured I would post here in case they have any value.

1. Add an int order to @Schema. Might help partially with the name synchronization issue, although it might be hard to maintain for classes with many properties.
2. Introduce an annotation such as the following to allow the application to sort the discovered properties as necessary. This is similar to what JsonB does with its JsonbVisibility annotation.

   
   @Target({ElementType.ANNOTATION_TYPE, ElementType.TYPE, ElementType.PACKAGE})
   public @interface SchemaPropertyOrderStrategy {
   Class<? extends PropertyOrderStrategy> value();
   }

interface PropertyOrderStrategy { List order(List properties); }

[EricWittmann]

Thanks for the additional suggestions. And yes, anyone reading this should follow the conversation going on in #360 as well.

Do you have a preferred solution to this? I'm on the fence, myself.

[MikeEdgar]

Do you have a preferred solution to this? I'm on the fence, myself.

I really do not at this point. I'm continuing to think it through. What are your top solutions?

I like the idea of @SchemaPropertyOrder (or similar) a little, and it allows for a couple of defaults to be specified (e.g. alphabetic, natural, etc.). We might also put the String[] propertyOrder() (or use value) method on it rather than directly in @Schema.

There may be a similar user response to using @SchemaProperty for order that we see with #363. I.e., it's too complex/verbose for the common cases. That is definitely the most DRY approach, however.

@Target({ElementType.ANNOTATION_TYPE, ElementType.TYPE, ElementType.PACKAGE})
public @interface SchemaPropertyOrder {
    private PropertyOrderStrategy NATURAL = ...
    private PropertyOrderStrategy ALPHA = ...
    /* PropertyOrderStrategy defined in earlier comment. */
    Class<? extends PropertyOrderStrategy> strategy() default NATURAL;
    String[] value() default {};
}

public @interface Schema {
    ...
    SchemaPropertyOrder propertyOrder() default ...; /* some reasonable default */
    ...
}

[EricWittmann]

I think your most recent proposal is a nice synthesis of the best options - combining the ability to specify the order of the properties and the ability to specify a strategy. So I'd be good with that. I don't like the option of adding order to @Schema. And upon further consideration I don't think I like the idea of re-using the solution to #360 for this.

[EricWittmann]

We can perhaps discuss this again at the next hangout and make a final decision. @arthurdm and @msavy might want to have a look at this before the meeting. :)

[MikeEdgar]

I'm in agreement about not re-using the solution to #360. It might take some time to determine a good approach there and I think whatever solution we come up with will probably be too verbose for here.

[arthurdm]

hey guys, joining the discussion late. Given the popularity / familiarity of Jackson within the Java community, my vote would be to use the propertyOrder solution, to mirror what JsonPropertyOrder does.

As it was mentioned, there is a drawback of having this list out of sync with the actual properties, but perhaps we can put words in the spec such as: serialize properties that match the list in the specified order and then afterwards serialize in an undefined order any properties that did not match the list.

[MikeEdgar]

What are your thoughts on having a @SchemaPropertyOrder annotation as shown above with a String[] and also a "strategy" class? The string is the most basic, but the addition of a strategy would give more run-time control. We probably need to discuss the method signature as well if that is included. My preference would be to omit that for now and leave it for a future enhancement discussion.

[arthurdm]

hi @MikeEdgar - I was thinking that if user can specify the exact order of the properties, would they need to specify a strategy?

[EricWittmann]

I think the idea is that a strategy doesn't have the problem of going out of sync.

[MikeEdgar]

That's right. Also, another scenario would be to define some built-in strategies like alphabetical order, i.e. like the values in PropertyOrderStrategy here.

[arthurdm]

My preference would be to add a new field to our existing @Schema annotation, called propertyOrder, which would be an array String values.

[arthurdm]

Thinking about this some more, could we have a solution that is a bit of a hybrid of the proposals: a new field to @Schema, called orderStrategy, which is a simple enum value - public enum SchemaOrderStrategy, with values like DEFAULT("natural"), ALPHA("alphabetical"), etc?

[MikeEdgar]

That sounds reasonable to me. To be more specific, the solution would be to add both String[] propertyOrder and SchemaOrderStrategy orderStrategy to @Schema, correct?

[arthurdm]

I was thinking about adding just the SchemaOrderStrategy orderStrategy property to @Schema.

[arthurdm]

We agreed to take this to the architecture board to see what's the solution across other spec (for example, adopting JsonB's annotation to have index for each property)

[phillip-kruger]

Hi all. Just to repeat here what I have said in the meeting. Another option could be to allow a user to annotation the field with an @Order(1) annotation. This way there is no change of getting out of sync.

This will be similar to @Priority on class ordering (or @Order in the Spring world)

[arthurdm]

we discussed this item in today's hangout and decided to go with the @Order(n) approach.

[MikeEdgar]

What are the group's thoughts on placing the new @Order annotation in the org.eclipse.microprofile.openapi.annotations package (rather than in media) to allow for re-use with other items (e.g. ordering of parameters or other things we haven't though of) in the future?

[arthurdm]

sounds good to me.

[MikeEdgar]

We'll wait on this one and (potentially) reconsider an approach that uses a common Jakarta annotation.