[BUG] [Java] inheritance is generated without discriminator field

[vitaliysapounov]

Description

We see that inheritance (instead of composition) is generated where, as we think, it should not.

Namely, we have the following model file:

openapi: "3.0.2"

info:
  version: "1.0.0"
  title: Test

  # We only care about schemas, but this prevents codegen error: "attribute paths is missing"
paths:
  /dummy:
    get:
      responses:
        '200':
          description: OK

components:
  schemas:

    PreventInheritance:
      description: We declare dummy dependency on this empty class to prevent inheritance

    Base:
      properties:
        base1:
          type: integer
        base2:
          type: string

    Inheritance:
      allOf:
        - $ref: "#/components/schemas/Base"
        - properties:
            # doses
            Inheritance1:
              type: integer
            Inheritance2:
              type: string

    NoInheritance:
      allOf:
        - $ref: "#/components/schemas/Base"
        - $ref: "#/components/schemas/PreventInheritance"
        - properties:
            NoInheritance1:
              type: integer
            NoInheritance2:
              type: string

The following two classes are of interest:

1. Basically, Inheritance class is declared as having all the properties of Base and some additional. Note that there is no discriminator defined. Thus, since there is no discriminator, our understanding that composition should be used, not inheritance. In the generated Java class we see that inheritance is generated, however:

public class Inheritance extends Base implements Serializable {

OBSERVED: inheritance is generated
EXPECTED: composition should be used, as there is no discriminator field (?)

2. To prevent generating of inheritance, we used a hack: in NoInheritance class we add not only Base but also empty PreventInheritance. It seems that in this case, the generator chooses to use composition (why?):

public class NoInheritance implements Serializable {

OBSERVED: inheritance is not generated
EXPECTED: ? (we are confused what should be used, composition or inheritance, the spec is unclear in that regard)

openapi-generator version

Gradle plugin org.openapitools:openapi-generator-gradle-plugin:4.2.2 (latest as of this writing)

OpenAPI declaration file content or url

Please see above

Command line used for generation

./gradlew build

Steps to reproduce

1. Unpack the attached Gradle project
2. Run ./gradlew build

Related issues/PRs

Suggest a fix

We are not sure how/why inheritance vs composition should be selected, as per the spec. But, the current behavior described above seems to be very confusing (the generator seems to choose inheritance in one case and composition in another case, for no clear reason).
test_openapi_inheritance.zip

[jimschubert]

@vitaliysapounov it looks like the confusion may come from our early implementations of OAS3 allOf and our cross-compatibility with OAS2 spec documents.

Can you verify the existence of something like this output when you generate?

[main] WARN  o.o.codegen.utils.ModelUtils - [deprecated] inheritance without use of 'discriminator.propertyName' is deprecated and will be removed in a future release. Generating model for null

This comes from here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java#L981

See #3771 for some background and more details.

I'm not saying the current implementation is "correct", but you can see that a single $ref in allOf is currently treated as though it implies inheritance. I'm not sure when we intend for the removal, but I'll tag this as 4.3.0 since this is a "break change with fallback" release (fallback here being updating spec or pinning to 4.2.3).

cc @wing328 I saw your feedback in the linked PR, did you have a goal release for changing this functionality? If we're targeting 5.0, can we skip the confusing behavior with a configuration option in the interim?

[vitaliysapounov]

@jimschubert

Yes, we see the warning you mentioned ("inheritance without use of 'discriminator.propertyName' is deprecated and will be removed in a future release"). Here is the full Gradle build output:

17:45:44: Executing task 'build'...


> Task :openApiGenerate
'host' (OAS 2.0) or 'servers' (OAS 3.0) not defined in the spec. Default to [http://localhost] for server URL [http://localhost/]
'host' (OAS 2.0) or 'servers' (OAS 3.0) not defined in the spec. Default to [http://localhost] for server URL [http://localhost/]
[deprecated] inheritance without use of 'discriminator.propertyName' is deprecated and will be removed in a future release. Generating model for null
More than one inline schema specified in allOf:. Only the first one is recognized. All others are ignored.
'host' (OAS 2.0) or 'servers' (OAS 3.0) not defined in the spec. Default to [http://localhost] for server URL [http://localhost/]
Successfully generated code to task ':openApiGenerate' property 'outputDir'

> Task :compileJava
> Task :processResources UP-TO-DATE
> Task :classes
> Task :jar UP-TO-DATE
> Task :assemble UP-TO-DATE
> Task :compileTestJava NO-SOURCE
> Task :processTestResources NO-SOURCE
> Task :testClasses UP-TO-DATE
> Task :test NO-SOURCE
> Task :check UP-TO-DATE
> Task :build UP-TO-DATE

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 20s
4 actionable tasks: 2 executed, 2 up-to-date
17:46:07: Task execution finished 'build'.

[wing328]

I'll look into this issue this weekend. Stay tuned.

[wing328]

It's bug and here is the workaround by adding "type: object" to the last schema definition in allOf:

components:
  schemas:
    Base:
      properties:
        base1:
          type: integer
        base2:
          type: string

    Inheritance:
      allOf:
        - $ref: "#/components/schemas/Base"
        - type: object
          properties:
            # doses
            Inheritance1:
              type: integer
            Inheritance2:
              type: string

    NoInheritance:
      allOf:
        - $ref: "#/components/schemas/Base"
        - type: object
          properties:
            NoInheritance1:
              type: integer
            NoInheritance2:
              type: string

and the output models no longer have inheritance generated.

I'll see if I can hunt down the bug and file a PR this weekend. Please use the workaround before we come up with a permanent fix.

[pvdbosch]

I find inheritance without descriminator rather useful and would prefer it to be kept.
I stumbled on this issue because I didn't get inheritance in generated code (I was using "type: object").

I've encountered two use cases where it is useful:

• when the full list of subtypes isn't known in advance when declaring parent type
• when no polymorphism is required in OpenAPI document, but it is still useful in the generated code

An example:

responses:
  '400':
    schema:
      $ref: '#/definitions/InvalidParamProblem'
  '403':
    schema:
      $ref: '#/definitions/AuthorizationProblem'

InvalidParamProblem:
  allOf:
    - $ref: './problem.yaml#/definitions/Problem'
    - type: object
      properties:
        invalidParams:
          type: array
          items:
            $ref: '#/definitions/InvalidParam'
  
  AuthorizationProblem:
    allOf:
    - $ref: './problem.yaml#/definitions/Problem'
    - type: object
      properties:
        requiredScopes:
          type: array
          items:
            type: string

Referenced problem.yaml schema:

  Problem:
    description: A Problem Details object (RFC 7807)
    type: object
    properties:
      type:
        type: string
        format: uri
        default: about:blank
      title:
        type: string
      status:
        type: integer
        format: int32
        minimum: 400
        maximum: 600
        exclusiveMaximum: true
      detail:
        type: string
      instance:
        type: string
        format: uri

Only a single Problem subtype is specified iper response code, so I don't need polymorphism.
I also can't use discriminator here at the parent type declaration because it's in another file than the subtypes, and I'd still want to write Java code that uses the parent type Problem.

On another note, there's some talk on an OAS issue about deprecating the discriminator functionality entirely: OAI/OpenAPI-Specification#2143

[wolfdale]

@wing328 I tried the workaround that you mentioned by adding "type: object" but output models still have inheritance generated instead of composition. Currently $ref in allOf is treated as it implies inheritance even if there are no discriminator field in schema file.

[wing328]

@wolfdale there's a bug (regression) in 4.3.0 release. I've filed and merged a PR fix it in the latest master.

Can you give it a try with the latest master or snapshot version?

If it's still an issue, I will reopen this issue.

[nhoughto]

Being able to trigger inheritance instead of composition when polymorphism isn't needed seems like a useful feature, with this changed does that mean there is now no longer a way to achieve it?