[BUG] Code is not generated correctly for allOf.

[jeff9finger]

Description

I am not able to get the following definition to generate java or type script correctly.

Have tried with 4.3.1.
In Java, RealCommand is generated as

public class RealCommand extends Command {
...
}

Notice that I did not specify a discriminator in Command. I expect this definition to generate a composition of Command and RealCommand.java and that Command.java would not be generated. Command.java file is not generated, but it is also expected as a base class in RealCommand.java, so this does not compile.

There should not be any inheritance here because there is no discriminator.

openapi-generator version

4.3.1

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• What's the version of OpenAPI Generator used? - 4.3.1
• Have you search for related issues/PRs? - yes
• What's the actual output vs expected output?
• [Optional] Bounty to sponsor the fix (example)

OpenAPI declaration file content or url

swagger: "2.0"
info:
  title: Test Command model generation
  description: Test Command model generation
  version: 1.0.0
host: localhost:8080
schemes:
  - https
definitions:
  Command:
    title: Command
    description: The base object for all command objects.
    x-swagger-router-model: CommandDto
    type: object
    properties: {}
  RealCommand:
    title: RealCommand
    description: The real command.
    x-swagger-router-model: RealCommandDto
    allOf:
      - $ref: '#/definitions/Command'
  ApiError:
    description: The base object for API errors.
    x-swagger-router-model: ApiGeneralException
    type: object
    required:
    - code
    - message
    properties:
      code:
        description: The error code. Usually, it is the HTTP error code.
        type: string
        readOnly: true
      message:
        description: The error message.
        type: string
        readOnly: true
    title: ApiError

parameters:
  b_real_command:
    name: real_command
    in: body
    description: A payload for executing a real command.
    required: true
    schema:
      $ref: '#/definitions/RealCommand'

paths:
  /execute:
    post:
      produces: []
      x-swagger-router-controller: FakeController
      operationId: executeRealCommand
      parameters:
      - name: real_command
        in: body
        description: A payload for executing a real command.
        required: true
        schema:
          $ref: '#/definitions/RealCommand'
      responses:
        '204':
          description: Successful request. No content returned.
        '400':
          description: Bad request.
          schema:
            $ref: '#/definitions/ApiError'
        '404':
          description: Not found.
          schema:
            $ref: '#/definitions/ApiError'
        default:
          description: Unknown error.
          schema:
            $ref: '#/definitions/ApiError'

Command line used for generation

openapi-generator generate -i test.yaml -g java --library jersey2 -o java --additional-properties legacyDiscriminatorBehavior=false

Steps to reproduce

Related issues/PRs

#2845

Suggest a fix

I see that maybe ModelUtils#isFreeFormObject() should also check to see if the object is used in an allOf, anyOf, or oneOf in any other schema in the definition.
But this still does not explain why RealCommand is using Command as a base class.

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[amakhrov]

But this still does not explain why RealCommand is using Command as a base class.

Looks like it's because of the (deprecated) support for this legacy case: if there is a single parent class, then it's treated as inheritance even though there is discriminator. Generator probably prints a "deprecated" message in the console when it happens.

This special behavior is supposed to be removed in v5: #5876 (comment)

[jeff9finger]

if there is a single parent class, then it's treated as inheritance even though there is discriminator.

There is no discriminator in this case. So I would expect that the standard allOf semantics of composition to be used instead of inheritance.

Generator probably prints a "deprecated" message in the console when it happens.
Yes, there was a message about deprecation. I can live with that as we are trying to migrate from swagger-codegen with our v2 definition. Then hopefully, we'll be able to move to use OAS v3.

---

I like the inheritance actually, but this behavior seems to deviate from my reading of the semantics of allOf.

I can live with the inheritance, but my point is that this seems to deviate from what allOf is supposed to do and from how swagger-codegen treats this situation. And from this perspective, how should this issue be resolved?

[jeff9finger]

Looking at CodegenModel.java, parent has the following comments:

    // The parent model name from the schemas. The parent is determined by inspecting the allOf, anyOf and
    // oneOf attributes in the OAS. First codegen inspects 'allOf', then 'anyOf', then 'oneOf'.
    // If there are multiple object references in the attribute ('allOf', 'anyOf', 'oneOf'), and one of the
    // object is a discriminator, that object is set as the parent. If no discriminator is specified,
    // codegen returns the first one in the list, i.e. there is no obvious parent in the OpenAPI specification.
    // When possible, the mustache templates should use 'allParents' to handle multiple parents.
    public String parent, parentSchema;

Notice

If no discriminator is specified, codegen returns the first one in the list, i.e. there is no obvious parent in the OpenAPI specification.

This does not seem complete to me. A parent seems to refer to an inheritance structure. An object should only have a parent if there is a $ref specified in allOf, oneOf, anyOf AND that $refed model contains a discriminator. (Though not sure exactly what the semantics are for oneOf, anyOf without a discriminator) But if there is no discriminator in the $refed model, that $refed model should not be a parent object. Correct?

I think that parent should not be set unless there is a discriminator in the $refed model.

This seems to be the issue with composition vs inheritance.

https://spec.openapis.org/oas/v3.0.3#composition-and-inheritance-polymorphism

[amakhrov]

I agree it doesn't comply with the spec. I don't know the reasons for this special case to exist in the first place. But apparently it's been there for years. Perhaps will be removed in the next major release (although I'm not sure if it's still on the radar)

[wing328]

I'll take a look in the coming week. Clearly, the old incorrect behavior is something we want to remove as part of the 5.x release.

@jeff9finger can you convert the spec to 3.x and see if you still encounter the same issue?

Currently, we do advise users to migrate their spec to 3.x to have better support for model composition and inheritance.

[m-mohr]

Is there a way to enable inheritance without adding a discriminator field? We rely quite a bit on the "old incorrect behavior" to enable inheritance in code generation so having an alternative (and may it just be an option, disabled by default) to use the old behavior, would be much appreciated. Adding a discriminator field is not always possible unfortunately.

[jeff9finger]

@wing328

Clearly, the old incorrect behavior is something we want to remove as part of the 5.x release.
What is the "correct" behavior? Is it documented somewhere?

@jeff9finger can you convert the spec to 3.x and see if you still encounter the same issue?
Do you mean, just the example? Or my company's definitions? The code already converts from v2 to v3 for processing.

My company's OAS definitions can not be converted to v3 at this time, as much as I would like to. It is on our roadmap, but not an option at the moment. I'm just trying to get us to use openapi-generator first :-)

Currently, we do advise users to migrate their spec to 3.x to have better support for model composition and inheritance.

As I said above, I am unable to do that at this time. Also, what is the "correct" behavior of composition and inheritance?

---

After more investigation, my example, looks to be an edge case - models with no properties. But IMO, these should be handled properly. So my question is what is the correct behavior?

My reading of the OAS v3 spec, states that when a referenced model from allOf, anyOf, oneOf does not have a discriminator, composition should be used (instead of inheritance). But in this case, inheritance is used.

Also, when the model containing allOf, anyOf, oneOf has no properties, the generator does not generate code correctly.

I have both of these cases in our OAS definitions and we must use v2 at this time. But please link me to the decisions in the inheritance and composition so I can push our management to put a higher priority on moving to v3.

Thanks

[jeff9finger]

There are 2 issues here.

• [core] Add x-is-free-form vendor extension #6849
• A definition/schema that contains an allOf with a reference to another definition/schema, but has no other properties itself, does not generate the code properly.

I have added a line in ModelUtils#getParentName() (4.3.x branch) that seems to address this issue, but have not done extensive testing either.

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

Line 1174 in 003165c

public static String getParentName(ComposedSchema composedSchema, Map<String, Schema> allSchemas) {

Added the following at

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

Line 1192 in 003165c

} else {

if (interfaces.size() > 1) {

This allows the desired generation of code from the definitions above. And seems to conform to the OAS v3 as well. But not sure what the current thinking on composition and inheritance is.

[wing328]

@jeff9finger I've filed #6901 to fix the issue. I tested with the spec you provided and no longer get the models with inheritance. (It turns out the same spec in OAS v3 also has the issue, which is addressed by the PR)

[wing328]

Is there a way to enable inheritance without adding a discriminator field?

@m-mohr I don't think that's a good idea. We've been trying hard to correct this since 4.x release. Better update the spec to use discriminator if you want model inheritances. (other tools that can import/read OpenAPI spec also rely on the discriminator field to determine model inheritances)

[ndup0nt]

@wing328 wee have a similar need than @m-mohr. Our specification is generated from classes in which we don't want to add a discriminator property. Our consumer would like to get inheritance in their generated classes from our spec, for reusability reasons. I wonder in which case one would want to NOT having the inheritance ?
Also other tool (swagger-codegen) do generate inheritance without the need of a discriminator, see swagger-api/swagger-codegen#9693

[wing328]

Our specification is generated from classes in which we don't want to add a discriminator property.

Don't think you can do that according to the spec

The OpenAPI Specification allows combining and extending model definitions using the allOf property of JSON Schema, in effect offering model composition. allOf takes an array of object definitions that are validated independently but together compose a single object.

While composition offers model extensibility, it does not imply a hierarchy between the models. To support polymorphism, the OpenAPI Specification adds the discriminator field

Ref: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#composition-and-inheritance-polymorphism

I wonder in which case one would want to NOT having the inheritance?

There are definitely use cases in which users simply want the model composition (which is correct according to the spec). You can search in the "issues" to find out those use cases.

The current (incorrect) behavior doesn't support multiple inheritances either, which is supported by C++, Perl and more so we think it's a good time to put an end to this incorrect behavior in a major release (5.0.0)

Also other tool (swagger-codegen) do generate inheritance without the need of a discriminator, see swagger-api/swagger-codegen#9693

Being the top contributor to Swagger Codegen, I can tell you the behavior is still incorrect according to the spec. Also worth mentioning other tools that support OpenAPI spec won't recognize this kinda hidden convention.

Many have made the switch to add discriminators for model inheritances. It's not hard so please give it a try.

[CamilYed]

When this issue will be resolved? We can't wait any longer.

[jeff9finger]

Many have made the switch to add discriminators for model inheritances. It's not hard so please give it a try.

Just a quick note on my company's use/ non-use of discriminator.
The issue we have is that adding a discriminator in a multi hierarchical model makes the payloads contain a bunch of unneeded discriminator properties. Only the discriminator for the outermost model is actually needed for serialization.
This seems to be a limitation/feature of the OAS/JSON schema, and one that we have tried to workaround by using composition instead of inheritance.

I don't see a way around this, except maybe some custom templates/vendor extensions that generate the desired behavior. Are there other ideas? I don't want to hijack the topic, in this issue, so please message me privately @jeff9finger or in the slack channel

[Kenjee]

i have more less same problem
i switched from 4.3.0 to 4.3.1 and since 4.3.1 a simple allOf definition does not generate X extends Y anymore

i put and example project into https://github.com/Kenjee/hello-world << simple maven code generation
Readme explains all in details

but using same swagger definition with
swagger.io and openapi-generator 4.3.0 generates SubscriptionResponse extends Response
but with 4.3.1 it does not anymore

i also was reading https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/ but i stay with "allOf" should be enough, i dont get the need "discriminator"!

would be nice to get help and how to integrate the "discriminator" to get simple response object generated in the old way.

Why i raise this question: We have a couple of generics in place, so each response is "Response" with 2 fields but some times there is an inhertitad class with more details.

UPDATE: solved by simply adding "discriminator" (feels not correct but works)

[gwre-ivan]

There is definitely a use-case for being able to express commonalities across multiple generated types, as per comments on this ticket (and other tickets as well).

We have some code that fills / transforms / processes generated data types, and we want to generalize this code in cases where multiple data types share some common structure. I understand there is a some subtlety around "inheritance" and what does it mean for data types. Especially, as it was mentioned above, when you have "multiple" inheritance. I do find "single parent inheritance" (as in this ticket to be quite convenient, but I do agree that it might fail short in more complex scenarios (like multiple allOf).

I mildly disagree with the suggestion that one can just simply add discriminators and be done with it. In our use-cases, we don't need to express any kind of "inheritance". My main issue with "just add the discriminators" is that it drives long-term, high cost decisions (structure of one's APIs / data types) via minutiae of a particular implementation. Clients of my API should not care that in my service implementation I have switched from version 4.x to 5.x of the generator.

Also, rewriting code to introduce a bunch of copy-paste just because new implementation of the generator does not allow us to generalize code in the language of our choice (Java) is also less than ideal. However, I think there is a solution that might work well here: it is possible to extend generator in a way such that it can express commonalities through "interfaces".

I created a proof-of-concept in my fork where I add support for x-trait: true vendor extension. This extension causes two things:

1. Java interface is generated for the data type x-trait is declared on (similarly to how POJOs are generated, but without function bodies).
2. For each data type which directly or indirectly "extends" data type with x-trait ("extends" via allOf references), it adds the interface generated for the x-trait data type to the list of interfaces POJO implements (via x-implements).

It works well for our case. Actually, for Java generator (but not Spring one) it seems like you can manually do those "traits" via x-implements vendor annotation. The main drawback, though, is that you'll have to manually declare every single common accessor you care about (whereas in my implementation, it is done automatically for x-trait).

What do you think?

[javadev-jef]

Any updates for this issue? Currently in version 6.3.0 I still can't generate classes with inheritance using only allOf as below:

public class Apple extends Fruit {
...
}

[AndreaScarselli]

Any news on this? Is it going to be picked up?