[BUG] [Spring] [4.3.0] allOf forces inheritance

[ondrakucera]

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)?
• [x] What's the version of OpenAPI Generator used?
• [x] Have you search for related issues/PRs?
• [x] What's the actual output vs expected output?
• [ ] [Optional] Bounty to sponsor the fix (example)

Description

When trying to switch from 4.2.3 to 4.3.0 in our existing project, I've noticed different behavior when it comes to allOf. I'm not even sure it's a bug, maybe it's intentional but quickly going through release notes of 4.3.0, I didn't see it mentioned and for me it's even a breaking change (though the fix in our project shouldn't be too hard).

To be brief: when having a structure Foo with a few attributes and a structure Bar referencing allOf Foo and then having additional attributes of its own, with version 4.2.3, attributes of Foo would be just "copied" (or mixed-in) into the Bar class but otherwise Bar would have nothing to do with Foo. With version 4.3.0, Bar extends Foo.

Besides this being slightly unexpected (the specification of Swagger 2.0, which we're currently using, states that "[composition] does not imply a hierarchy between the models", it is a bit inconvenient when using fluent syntax to build objects because methods of Foo return Foo (of course).

If Foo has attributes foo1 and foo2 and Bar has attributes bar1 and bar2, previously it was possible to write

return new Bar().bar1("a").bar2("b").foo1("c").foo2("d");

Now something like this must be done:

Bar bar = new Bar().bar1("a").bar2("b");
bar.setFoo1("c");
bar.setFoo2("d");
return bar;

openapi-generator version

4.3.0

OpenAPI declaration file content or url

swagger: "2.0"
info:
    title: Regression test
    version: "1.0"
paths:
    /bars:
        get:
            responses:
                200:
                    description: Success
                    schema:
                        type: array
                        items:
                            $ref: '#/definitions/Bar'
definitions:
    Foo:
        type: object
        properties:
            foo1:
                type: string
            foo2:
                type: string
    Bar:
        allOf:
            -
                $ref: '#/definitions/Foo'
            -
                type: object
                properties:
                    bar1:
                        type: string
                    bar2:
                        type: string

With 4.3.0, three model classes are created: Bar, BarAllOf, and Foo. Interesting parts of the files are:

Bar.java:

public class Bar extends Foo  {
  @JsonProperty("bar1")
  private String bar1;
  @JsonProperty("bar2")
  private String bar2;

  public Bar bar1(String bar1) {
    this.bar1 = bar1;
    return this;
  }

  /**
   * Get bar1
   * @return bar1
  */
  @ApiModelProperty(value = "")
  public String getBar1() {
    return bar1;
  }
  public void setBar1(String bar1) {
    this.bar1 = bar1;
  }
  public Bar bar2(String bar2) {
    this.bar2 = bar2;
    return this;
  }
  /**
   * Get bar2
   * @return bar2
  */
  @ApiModelProperty(value = "")
  public String getBar2() {
    return bar2;
  }
  public void setBar2(String bar2) {
    this.bar2 = bar2;
  }
  ...
}

BarAllOf.java:

public class BarAllOf   {
  @JsonProperty("bar1")
  private String bar1;
  @JsonProperty("bar2")
  private String bar2;

  public BarAllOf bar1(String bar1) {
    this.bar1 = bar1;
    return this;
  }

  /**
   * Get bar1
   * @return bar1
  */
  @ApiModelProperty(value = "")
  public String getBar1() {
    return bar1;
  }
  public void setBar1(String bar1) {
    this.bar1 = bar1;
  }
  public BarAllOf bar2(String bar2) {
    this.bar2 = bar2;
    return this;
  }
  /**
   * Get bar2
   * @return bar2
  */
  @ApiModelProperty(value = "")
  public String getBar2() {
    return bar2;
  }
  public void setBar2(String bar2) {
    this.bar2 = bar2;
  }
  ...
}

Foo.java:

public class Foo   {
  @JsonProperty("foo1")
  private String foo1;

  @JsonProperty("foo2")
  private String foo2;

  public Foo foo1(String foo1) {
    this.foo1 = foo1;
    return this;
  }

  /**
   * Get foo1
   * @return foo1
  */
  @ApiModelProperty(value = "")
  public String getFoo1() {
    return foo1;
  }
  public void setFoo1(String foo1) {
    this.foo1 = foo1;
  }
  public Foo foo2(String foo2) {
    this.foo2 = foo2;
    return this;
  }
  /**
   * Get foo2
   * @return foo2
  */
  @ApiModelProperty(value = "")
  public String getFoo2() {
    return foo2;
  }
  public void setFoo2(String foo2) {
    this.foo2 = foo2;
  }
  ...
}

With version 4.2.3, the same three model classes are generated. Foo and BarAllOf have pretty much the same contents (as with version 4.3.0) but this is how Bar.java looks like:

public class Bar   {
  @JsonProperty("foo1")
  private String foo1;

  @JsonProperty("foo2")
  private String foo2;

  @JsonProperty("bar1")
  private String bar1;

  @JsonProperty("bar2")
  private String bar2;

  public Bar foo1(String foo1) {
    this.foo1 = foo1;
    return this;
  }

  /**
   * Get foo1
   * @return foo1
  */
  @ApiModelProperty(value = "")
  public String getFoo1() {
    return foo1;
  }
  public void setFoo1(String foo1) {
    this.foo1 = foo1;
  }
  public Bar foo2(String foo2) {
    this.foo2 = foo2;
    return this;
  }
  /**
   * Get foo2
   * @return foo2
  */
  @ApiModelProperty(value = "")
  public String getFoo2() {
    return foo2;
  }
  public void setFoo2(String foo2) {
    this.foo2 = foo2;
  }
  public Bar bar1(String bar1) {
    this.bar1 = bar1;
    return this;
  }
  /**
   * Get bar1
   * @return bar1
  */
  @ApiModelProperty(value = "")
  public String getBar1() {
    return bar1;
  }
  public void setBar1(String bar1) {
    this.bar1 = bar1;
  }
  public Bar bar2(String bar2) {
    this.bar2 = bar2;
    return this;
  }
  /**
   * Get bar2
   * @return bar2
  */
  @ApiModelProperty(value = "")
  public String getBar2() {
    return bar2;
  }
  public void setBar2(String bar2) {
    this.bar2 = bar2;
  }
  ...
}

Command line used for generation

java -jar openapi-generator-cli.jar generate -g spring

Steps to reproduce

Related issues/PRs

Suggest a fix

[wing328]

@ondrakucera thanks for reporting the issue. May I know if you've time to do a git bisect to help identify the commit that changes the behavior? Let me know if you need help on that.

[ondrakucera]

@wing328: I've only done this about the second time in my life but unless I made a mistake, the commit in question is 0a3272697de84afe29b0b54fea37d9517d073fb5.

[wing328]

@ondrakucera thanks. Can you please also share a spec (minimal) to reproduce the issue?

[ondrakucera]

@wing328: All right, I've updated the issue.

[wing328]

@ondrakucera I've also performed a git bisect and arrive at the same conclusion https://github.com/OpenAPITools/openapi-generator/commit/0a3272697de84afe29b0b54fea37d9517d073fb5 breaks allOf in OpenAPI/Swagger 2.0 spec.

I migrated your test spec to OpenAPI 3.0 and no longer experience the issue. Can you please give it a try with OpenAPI 3.0 spec and see if it works for you?

[ondrakucera]

@wing328: Unfortunately, it doesn't seem to be working for me. I'm still getting the unwanted inheritance with the following definition:

openapi: 3.0.1
info:
  title: Regression test
  version: "1.0"

paths:
  /bars:
    get:
      responses:
        "200":
          content:
            'application/json':
              schema:
                items:
                  $ref: '#/components/schemas/Bar'
                type: array
          description: Success

components:
  schemas:
    Foo:
      type: object
      description: Foo description
      properties:
        foo1:
          type: string
        foo2:
          type: string
    Bar:
      allOf:
        -
          $ref: '#/components/schemas/Foo'
        -
          type: object
          description: Bar description
          properties:
            bar1:
              type: string
            bar2:
              type: string

[wing328]

Can you please test https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/allOf_composition.yaml to see if you get model inheritances in the output?

[ondrakucera]

@wing328: I think we've found the problem. With your example, there's no inheritance. However, remove Human and suddenly SuperMan extends Hero. Which means that the generator apparently checks whether it is possible to use inheritance (i.e. there's exactly one ancestor candidate for that) and if that's so, it uses it. If there are multiple "ancestors", the inheritance cannot be used and so it isn't.

Now we're back to the original question whether this is intentional or whether it is a bug. It is a different behavior when compared to 4.2.3 and my feeling is that it is a bug but I don't consider myself an authority on the subject. :-)

[wing328]

Since there's a change in behavior (to me it's a bug), I consider this a regression. I'll try to file a PR to fix it in the next days when I can find the time

[jimschubert]

4.3.0 was a minor release with breaking changes w/fallback. Are we saying there's not fallback here?

In #4906 (not yet merged), there's been a lot of discussion around this behavior. The commit linked here was supposed to introduce only a test helper which I'd proposed in the above PR, and I must not have noticed the additional for loop logic should have remained in #4906.

While looking at #4096, we found the behavior you've experienced before 4.3.0 could be achieved by removing type: object from Foo. Would you be willing to evaluate that before reverting the change?

The logic explained above (automagically inferring inheritance) has existed since 3.x for some cases. I think there's a condition in your spec which has prevented it from working as designed (probably our composition improvements in 4.3.0).

[amakhrov]

The logic explained above (automagically inferring inheritance) has existed since 3.x for some cases

Yep. I believe this is the original PR that re-introduced inheritance in such a case: https://github.com/OpenAPITools/openapi-generator/pull/3771 (landed in 4.1.2) The description of issue fixed by that PR indicates that such behavior had been in place long before that.

Apparently it violates OAS2, however it seemed to be important enough to restore it in generator 4.x.

Perhaps it should be completely eliminated in 5.x?

[wing328]

Perhaps it should be completely eliminated in 5.x?

Definitely

[wing328]

While looking at #4096, we found the behavior you've experienced before 4.3.0 could be achieved by removing type: object from Foo. Would you be willing to evaluate that before reverting the change?

@jimschubert tried removing "type: object" one by one in the spec provided in https://github.com/OpenAPITools/openapi-generator/issues/5876#issuecomment-613314387 but no luck restoring the behavior (composition without inheritance)

[ondrakucera]

While looking at #4096, we found the behavior you've experienced before 4.3.0 could be achieved by removing type: object from Foo. Would you be willing to evaluate that before reverting the change?

@jimschubert: I've actually tried this before but as @wing328 said, it doesn't help.

BTW, I could do without type: object but what this minimal example doesn't show, in the real swagger I've got description for both Foo and the second allOf part of Bar. Interestingly enough, if you then "visualize" the swagger in https://editor.swagger.io/, the description is actually really shown for Foo and Bar. If you look at the generated classes, the description for Foo is generated as javadoc for Foo class but the description meant for Bar is only generated as javadoc for BarAllOf class and not for Bar class. Yet again, I'm not even sure what the correct behavior here should be but frankly, I don't care that much about javadoc of the generated classes at this moment. (To be clear: this javadoc behavior was present with 4.2.3, too. I'm mentioning it just as a side-note and as the reason why I have type: object there.)

[wing328]

@ondrakucera the fix has been merged into master. Please give it another try.

@jimschubert I'll try to submit a fix for missing "type: object" affecting the output (composition vs inheritance)

[jimschubert]

The mention in that last comment was meant for me.

[wing328]

oops.. updated.

[wing328]

@jimschubert I've filed https://github.com/OpenAPITools/openapi-generator/pull/5992 to the fix the incorrect behavior due to missing "type: object"

[ondrakucera]

@wing328 Yes, I can confirm that the current master's version restores the original behavior for me. Thank you all.

[wing328]

FYI. Filed https://github.com/OpenAPITools/openapi-generator/pull/6901 to completely remove the old (incorrect) behavior.