[BUG][JAVA] Java generator puts parent properties from allOf last instead of first and skips properties entirely for "middle" references

[swpalmer-cl]

Bug Report Checklist

• [x] Have you provided a full/minimal spec to reproduce the issue?
• [x] Have you validated the input using an OpenAPI validator (example)?
• [ ] Have you tested with the latest master to confirm the issue still exists?
• [x] Have you searched for related issues/PRs?
• [x] What's the actual output vs expected output?
• [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Properties should be evaluated by the templates in the order they are declared. For objects that reference a parent with allOf the properties from the parent should appear first. Instead they are processed last. This is of course very wrong. You might argue that the order of fields as they are declared in a Java class doesn't matter, but this makes the order wrong for JSON serialization (if you care), and when modifying the pojo template to produce a Java record (which works well as the properties are flattened without a descriminator) instead of a regular class the parameter order will be non-sensical. The common parts should be first so they are always at the same position.

openapi-generator version

7.2.0

OpenAPI declaration file content or url

For object schemas like this"

components:
    schemas:
        BaseObject:
            type: object
            required: 
                -  id
            properties:
                id:
                    type: string
                    description: Object ID
                name:
                    type: string
                    description: Optional name

        BigObject:
            allOf: 
                -  $ref: "#/components/schemas/BaseObject"
            type: object
            required:
                - value
            properties:
                value:
                    type: string

        BiggerObject:
            allOf: 
                -  $ref: "#/components/schemas/BigObject"
            type: object
            properties:
            weight:
                type: integer
            children:
                type: array
                items: string

This produces fields in the following order:

  private Integer weight;
  private List<String> children;
  private String id;
  private String name;

Note that the value field is missing entirely - that's another bug

instead of the correct order (and including the value field):

  private String id;
  private String name;
  private String value;
  private Integer weight;
  private List<String> children;

Generation Details

I used the Gradle plugin to generate models.

Steps to reproduce

Run the Java code generation on a model that uses allOf, note the order that the vars are processed by the template.

Related issues/PRs

https://github.com/OAI/OpenAPI-Specification/issues/2424

Suggest a fix

I am not equipped to build and test modifications to the Java code generator at this time.

[wing328]

I think v7.1.0 didn't have this issue, right? If that's the case, please fall back to v7.1.0 for the time being.

I've filed https://github.com/OpenAPITools/openapi-generator/issues/17676 to fix the issue with allOf having a single item only.

[jpfinne]

@swpalmer-cl It works with a slightly different syntax

Tested with 7.3.0

    BaseObject:
      type: object
      required:
        -  id
      properties:
        id:
          type: string
          description: Object ID
        name:
          type: string
          description: Optional name
    BigObject:
      allOf:
        - $ref: "#/components/schemas/BaseObject"
        - type: object
          required:
            - value
          properties:
            value:
              type: string
    BiggerObject:
      allOf:
        - $ref: "#/components/schemas/BigObject"
        - type: object
          properties:
            weight:
              type: integer
            children:
              type: array
              items:
                type: string

It gives

public class BiggerObject {
  private String id;
  private String name;
  private String value;
  private Integer weight;
  @Valid
  private List<String> children;

Using your syntax, the values are still out of order. "value" is present in my test

[jpfinne]

I've looked at the code. The order is decided in DefaultCodeGen.updateModelForComposedSchema

It uses io.swagger.v3.oas.models.media.ComposedSchema to find the list of Properties and the list of AllOf. The issue is that ComposedSchema does not specify the order of the properties and the allOf in the contract. The code adds first the properties then the allOf.
Very complex code ensures that constructors with required fields are correct even when there is inheritance.

I think it is more common to have allOf first then properties. It is the assumption of swagger editor. I think the openapi genrator should use the same order.

BiggerObject{
    id*	[...]
    name	[...]
    value*	[...]
    weight	[...]
    children	[...]  
}

Nothing prevents the opposite, but swagger editor still put the allOf first !

    BiggerObject:
      type: object
      properties:
        weight:
          type: integer
        children:
          type: array
          items: 
            type: string
      allOf:
        -  $ref: "#/components/schemas/BigObject"

Having a deterministic order is important for the constructor in java (required args/all args). Changing the order would be a huge breaking change for required constructors and for lombok allArgsConstructor.

There could be an option to specify the order of fields and constructor arguments:

• propertiesFirst (default for backward compatibility)
• inheritanceFirst (this request)
• declarationOrder (would need to analyze the order in the swagger schema)

[wing328]

        BigObject:
            allOf: 
                -  $ref: "#/components/schemas/BaseObject"
            type: object
            required:
                - value
            properties:
                value:
                    type: string

Given that allOf and properties are in the same level, please enable REFACTOR_ALLOF_WITH_PROPERTIES_ONLY in openapi normalizer for the time being.

We'll enable this rule by default in v8.0.0 release.

[jpfinne]

Using REFACTOR_ALLOF_WITH_PROPERTIES_ONLY, the properties are last.

Nice.

Of course it breaks all the generated constructors (lombok allArgsConstructor, readonly fields constructor for java client, required fields constructor for spring generator...). IMHO it is ok.

Then use the normalizer REF_AS_PARENT_IN_ALLOF (needed if you want to force inheritance for allOf) It seems to disable REFACTOR_ALLOF_WITH_PROPERTIES_ONLY and the order of fields is again with properties first and inheritance last.

For example, using this contract:

components:
  schemas:
    Person:
      required:
        - $_type
        - lastName
        - firstName
      type: object
      properties:
        $_type:
          type: string
        lastName:
          type: string
        firstName:
          type: string
    Child:
      allOf:
      - type: object
        required:
          - age
        properties:
          age:
            type: integer
            format: int32
      - $ref: '#/components/schemas/Person'

With REF_AS_PARENT_IN_ALLOF this constructor is generated by SpringCodeGen:

 /**
   * Constructor with only required parameters
   */
  public Child(Integer age, String $type, String lastName, String firstName) {
    super($type, lastName, firstName);
    this.age = age;
  }

With REMOVE_ANYOF_ONEOF_AND_KEEP_PROPERTIES_ONLY:

/**
   * Constructor with only required parameters
   */
  public Child(String $type, String lastName, String firstName, Integer age) {
    this.$type = $type;
    this.lastName = lastName;
    this.firstName = firstName;
    this.age = age;
  }

@wing328 We need a deterministic order of fields.