[BUG] [JAVA] REF_AS_PARENT_IN_ALLOF no longer effective with multiple `$ref`

[kzander91]

Bug Report Checklist

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

Description

The REF_AS_PARENT_IN_ALLOF normalizer doesn't work anymore in a case like this:

TestResponse:
  allOf:
    - $ref: '#/components/schemas/ParentType1'
    - $ref: '#/components/schemas/ParentType2'

With 7.15.0, this generated:

public class ParentType1 {
  // ...
  protected String parentProp1;
  // ...
}

public class TestResponse extends ParentType1 {
  // ...
  private String parentProp2;
  // ...
}

With 7.16.0, this now generates:

public class TestResponse { // <-- no longer inherits from ParentType1
  // ...
  private String parentProp1;
  // ...
  private String parentProp2;
  // ...
}

openapi-generator version

7.16.0. It is a regression introduced with this version, it worked with 7.15.0.

OpenAPI declaration file content or url

openapi: 3.0.3
info:
  title: Test
  version: 1.0.0
servers:
  - url: /test

paths:
  /test:
    get:
      responses:
        200:
          description: test
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TestResponse'

components:
  schemas:
    ParentType1:
      type: object
      properties:
        parent_prop1:
          type: string
      required:
        - parent_prop1
    ParentType2:
      type: object
      properties:
        parent_prop2:
          type: string
      required:
        - parent_prop2

    TestResponse:
      allOf:
        - $ref: '#/components/schemas/ParentType1'
        - $ref: '#/components/schemas/ParentType2'

Generation Details

I'm using the Maven plugin:

<build>
    <plugins>
        <plugin>
            <groupId>org.openapitools</groupId>
            <artifactId>openapi-generator-maven-plugin</artifactId>
            <version>7.16.0</version>
            <configuration>
                <generatorName>java</generatorName>
                <library>webclient</library>
                <openapiNormalizer>REF_AS_PARENT_IN_ALLOF=true</openapiNormalizer>
            </configuration>
            <executions>
                <execution>
                    <id>generate-api</id>
                    <goals>
                        <goal>generate</goal>
                    </goals>
                    <configuration>
                        <inputSpec>${project.basedir}/src/main/resources/test-openapi.yaml</inputSpec>
                    </configuration>
                </execution>
            </executions>
        </plugin>
    </plugins>
</build>

Steps to reproduce

Related issues/PRs

Haven't looked too deep into it, but I'm suspecting #21799 as the cause of this regression.

Suggest a fix

[wing328]

thanks for reporting the issue

no clue off the top of my head

we do have tests in place: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java#L34-L100

so not sure why it becomes a regression in v7.16.0.

i'll try to take a look later this week

please fall back to v7.15.0 for the time being

[kzander91]

The spec in those tests (https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/allOf_extension_parent.yaml) doesn't use two $ref like I do in my reproducer.

Something like

allOf:
  - $ref: 'foo'
  - type: object
    # props

wasn't broken by this release.

[wing328]

OK. I see.

cc @fkellner - the author of #21799

[fkellner]

Ah, I am pretty sure that this is caused by my PR. But fixing this for your use case probably requires more extensive work than making sure a helper function does what it is documented to do.

In your example, the TestResponse has two parents: ParentType1 and ParentType2. The behaviour of version 7.15 was that the helper function returns the first parent it finds - an arbitrary decision, which broke other logic that relied on it returning null when there is no single parent and feels a little bit "hacky" tbh.

It even seems like a weird implementation decision, taking one as the parent and just collecting the properties of the others - wouldn't it be more true to the spec to generate two interfaces ParentType1 and ParentType2 and have TestResponse implement both, keeping the full set of properties?

Maybe we could fix the helper function to return an array of parent types, and then it is the decision of the generator you are using to just take the (lexically?) first as the parent type to be extend (or - if we have time - to implement it "cleanly" with interfaces), and the generator I am using to ignore it? I will try to make time in the next weeks to find a fix, but I can't promise anything.

[kzander91]

wouldn't it be more true to the spec to generate two interfaces

Absolutely, but this doesn't work in Java, unfortunately. Or are there some Jackson shenanigans we can pull with annotations to make this work? 🤔

[fkellner]

I don't understand, what exactly does not work in Java?

Classes can implement more than one Interface, and all the Interfaces would need to define are Getters and Setters. Like:

public interface ParentType1 {
  // ...
  public String getParentProp1();
  public void setParentProp1();
  // ...
}

public interface ParentType2 {
  // ...
  public String getParentProp2();
  public void setParentProp2();
  // ...
}

public class TestResponse implements ParentType1, ParentType2 {
  // ...
  private String parentProp1;
  private String parentProp2;
  // setter&getter implementations
  // ...
}

The problem I see is that then you could not instantiate the ParentTypes, which may be necessary if they are returned or expected somewhere else. So you would need to have a ParentType1Impl or rename the interface to IParentType1 and have it be implemented by ParentType1 and TestResponse. Which probably would be a headache to keep track of during code generation.

Or is this related to how Serialization/Deserialization works?

[kzander91]

Ah I see now, thanks. I thought you were referring to the property declarations coming from the interfaces instead of just the getters/setters.

[Mattias-Sehlstedt]

Hi @kzander91,

Initial note: I am only a contributor to this project, so these thoughts are only my own, and does not represent the project in any way.

Could we elaborate on how helpful the previous behavior was for your code generation. I am interested in knowing to what extent the inheritance was helpful and what potential issues you might have with adjusting for the new behavior. I.e., how is the extension used, and does the new composition not work at all due to the extensions being vital to the design?

To me it seems like the previous behavior was completely unintentional, and also that it is not something that is meaningful/helpful to retain/reintroduce. @fkellner have provided some better ideas for generation that might be helpful, but I believe that they might be a bit complex to implement for what they might provide.

We can see both in the method's JavaDoc that this might not have been the intentional behavior. If we look back at the initial introduction, and the test shared by wing and its specification, we also see that it is mainly intended for the specific case where there is a single allOf-ref, and that the other allOf:s are inline models (i.e., we have a single known ref that is a parent that we can extend).

So I believe the way forward is to aid you in adjusting for it on your end, rather than keeping an accidental bug as a supported behavior (the main reason for wanting it removed is because of the arbitrary selection, as mentioned also by fkellner).

Sidenote, might even be that this change would fix it out of the box? (adding x-parent manually)

components:
  schemas:
    ParentType1:
      x-parent: true
      type: object
      properties:
        parent_prop1:
          type: string
      required:
        - parent_prop1
    ParentType2:
      type: object
      properties:
        parent_prop2:
          type: string
      required:
        - parent_prop2

    TestResponse:
      allOf:
        - $ref: '#/components/schemas/ParentType1'
        - $ref: '#/components/schemas/ParentType2'

[kzander91]

x-parent didn't have any effect, unfortunately.

My use case is this:

I have a common base type for all API response body types, that all my concrete response body types are inheriting from (this is the first item in the allOf list). This is used to transport a few meta attributes like transaction ids and transaction timestamps that all responses contain:

{
  "transactionId": "deadbeef",
  "transactionTimestamp": "1970-01-01T00:00:00Z",
  // ...
  "responseAttributeSpecificToConcreteOperation1": "foo",
  "responseAttributeSpecificToConcreteOperation2": "bar",
}

My base type defines the schema for transactionId and transactionTimestamp, and I have a number of types extending that one, in the example above one would then additionally define responseAttributeSpecificToConcreteOperation1 and responseAttributeSpecificToConcreteOperation2.

Within those concrete response types, there's some overlap as well. For example, I have a response that looks like this:

{
  "transactionId": "deadbeef",
  "transactionTimestamp": "1970-01-01T00:00:00Z",
  // ...
  "detailedUserInfos": [{
    "userId": "1234",
    "userName": "Mr. Foo",
    "dateOfBirth": "1970-01-01",
    "address": "..."
    // etc.
  }, {
    "userId": "4321",
    "userName": "Ms. Bar",
    "dateOfBirth": "1970-01-01",
    "address": "..."
    // etc.
  }]
}

And I have a response that looks like this:

{
  "transactionId": "deadbeef",
  "transactionTimestamp": "1970-01-01T00:00:00Z",
  // ...
  "userId": "1234",
  "userName": "Mr. Foo"
}

Note how the elements of detailedUserInfos extend the userId and userName attributes and add more details.
This is modelled by defining a BasicUser base type that defines userId and userName, and a DetailedUser that extends that and adds dateOfBirth, address, etc.
The first response is defined as allOf with a $ref referencing the common response type, and an inline schema adding the detailedUserInfos array.
The second response is defined as allOf with two $ref, one referencing the common response type, and the second one referencing BasicUser.

---

The API itself is out of my control. I'm just documenting its shape and behaviour after the fact with an OpenAPI spec, to get code generation and validation features for the API client that I am developing.
If supporting multiple $ref isn't as straightforward, I will have to work around this in some way, likely by duplicating the property definitions of BasicUser in the schema definition for the second response 🤷‍♂️.

[Mattias-Sehlstedt]

Thanks for the extensive details, it makes it easier to mimic your current scenario when attempting to investigate possible adjustments.

I can see if I can figure out if it would be easy to support your code structure with some vendor-extension decoration. That would also make sense from a REF_AS_PARENT_IN_ALLOF perspective. The behavior of the setting would be that no parent is set if there isn't only and only one clear candidate, but that you can still mimic the behavior by adding a x-something to the allOf-ref that you believe is the ref that should be a parent (and thus used with an extends in Java).

[Mattias-Sehlstedt]

For about how many classes are you using the inheritance that is introduced by REF_AS_PARENT_IN_ALLOF? If it is not too many then I believe a possible solution could be to disable setting REF_AS_PARENT_IN_ALLOF and instead manually setting x-parent: true on the schemas that were generated as parents (being used as Child extends Parent) with 7.15.

The reason for why this should work is because what REF_AS_PARENT_IN_ALLOF does internally is add x-parent: true to schemas if they are present in an allOf as a ref. This annotation later leads to them being considered as parents for schemas. There then is a special condition that if there is a single parent then that is used to generate an extends relation between the child and the parent. The issue was that previously "single parent" was simply defined as "take the first parent in the list", which was incorrect. This of course then leads to REF_AS_PARENT_IN_ALLOF working differently for the cases where there are several parents.

I can create a PR later this week that contains an test case that verifies and illustrates how such an approach works. I will reference this ticket within the PR.

[kzander91]

Ah thanks, yes that did the trick!

x-parent didn't have any effect, unfortunately.

because in my earlier comment REF_AS_PARENT_IN_ALLOF was still enabled. I didn't know those two clashed.

[Mattias-Sehlstedt]

Great that that solved the issue! Is there more issue on your end after the change to parent calculation or could we consider the ticket closable with the suggested adjustment with adding the vendor extension yourself? (I will still make sure to add some test for it still).