openapi-generator: [BUG] Inheritance is broken

Enough people have commented on this. The old behavior is what we expect. Not the fixed one.

Inheritance via allOf (what we want) isn’t the same as polymorphic serialization (what discriminator does). Could we at least have an option to restore the previous behavior that worked for a lot of people?

@wing328 The discriminator is intended to tell API consumers how they can polymorphically distinguish the type of an incoming object, it doesn’t imply that a code generator must flatten any relationship without a discriminator in order to represent the composition.

At the very least, this decision should be left up to the language-specific generator rather than being made prematurely in the default codegen. TypeScript for example has absolutely no problem representing this kind of discriminator-less composition via interfaces, which do not imply any kind of runtime inheritance hierarchy, but allow for types to represent an extension/composition of one or more parent types.

And, indeed, this is exactly how the current release of openapi-generator works!

Run the example I provided through the release available in homebrew, and you will get an interface ExtendedErrorModel that inherits from an interface BasicErrorModel, exactly as you would expect. It is only with the current snapshot that you get these three, unrelated interfaces:

BasicErrorModel ExtendedErrorModel ExtendedErrorModelAllOf

This is just not right, and is happening because DefaultCodegen is prematurely deciding to throw away the child-parent relationship expressed by the allOf, perhaps based on the incorrect assumption that Java-style class hierarchies are the only type of extension mechanism in existence?

See also this inheritance example, again taken directly from the OpenAPI docs:

openapi: "3.0.1"
info:
  version: 1.0.0
  title: Inheritance test
paths:
  /pets:
    patch:
      requestBody:
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/Cat'
                - $ref: '#/components/schemas/Dog'
              discriminator:
                propertyName: pet_type
      responses:
        '200':
          description: Updated
components:
  schemas:
    Pet:
      type: object
      required:
        - pet_type
      properties:
        pet_type:
          type: string
      discriminator:
        propertyName: pet_type
    Dog:     # "Dog" is a value for the pet_type property (the discriminator value)
      allOf: # Combines the main `Pet` schema with `Dog`-specific properties 
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Dog`
          properties:
            bark:
              type: boolean
            breed:
              type: string
              enum: [Dingo, Husky, Retriever, Shepherd]
    Cat:     # "Cat" is a value for the pet_type property (the discriminator value)
      allOf: # Combines the main `Pet` schema with `Cat`-specific properties 
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Cat`
          properties:
            hunts:
              type: boolean
            age:
              type: integer

The code generated for this spec is broken and unusable.

@loicdesguin @palufra90 I think some of the indentation is wrong. I got it to work with this:

ExtendedErrorModel:
      allOf:
        - $ref: '#/components/schemas/BasicErrorModel'
        - type: object
          required:
            - rootCause
          properties:
            rootCause:
              type: string

to

ExtendedErrorModel:
      allOf:
        - $ref: '#/components/schemas/BasicErrorModel'
      type: object
      required:
      - rootCause
      properties:
      rootCause:
      type: string

Notice how allOf and type: object have to be at the same level for this to work!

Tested on 4.3.1 with typescript-axios.

I am seeing the same issues for the test files in the project. These generate broken code:

• allOf.yaml
• allOfDuplicatedProperties.yaml
• allOfMappingDuplicatedProperties.yaml

Etc.

How on Earth is the project passing all its tests when the output is this state?

EDIT: I see now that the petstore.yaml that is being used to regression test every generator doesn’t contain any use of anyOf, allOf, oneOf, discriminators, etc. There’s no way you can catch issues with these features, even when they’re massively broken, like now.

@macjohnny thanks for bringing it to my attention. I definitely want to take another look at the issue as I thought discriminator should setup the model inheritance properly but @Patre suggested otherwise in https://github.com/OpenAPITools/openapi-generator/issues/2845#issuecomment-493863254.

However I’m very busy as usual and I’ll try to look into it again after the Open Summit Japan 2019 next week.

If anyone wants to help on the issue, please feel free to submit a PR to start with.

@wing328 You don’t appear to be even trying to read or understand what I’m writing, so I don’t see much point in continuing this.

It seems that there are quite a few issues open about this:

• https://github.com/OpenAPITools/openapi-generator/issues/9788
• https://github.com/OpenAPITools/openapi-generator/issues/5903
• https://github.com/OpenAPITools/openapi-generator/issues/2236
• https://github.com/OpenAPITools/openapi-generator/issues/5381
• https://github.com/OpenAPITools/openapi-generator/issues/9981
• https://github.com/OpenAPITools/openapi-generator/issues/8721

I solved the inheritance problem for the typescript/angular generator indenting the property object with the ref object. From

ExtendedErrorModel:
      allOf:
        - $ref: '#/components/schemas/BasicErrorModel'
        - type: object
          required:
            - rootCause
          properties:
            rootCause:
              type: string

to

ExtendedErrorModel:
      allOf:
        - $ref: '#/components/schemas/BasicErrorModel'
        type: object
        required:
          - rootCause
        properties:
          rootCause:
            type: string

In this way the generated typescript interface is

export interface ExtendedErrorModel extends BaseErrorModel { 
    rootCause: string;
}

So why is this closed?

In our project we had to temporarily switch to another Gradle plugin ‘org.hidetake.swagger.generator’ which works well with Gradle 5 and allows downgrading open-api generator to 3.3.4(org.openapitools:openapi-generator-cli:3.3.4). And 3.3.4 properly handles inheritance, so if you need it(inheritance i mean), then you definitely have to somehow use this particular version

Is there a previous version I can rollback to so I can get extends back?

@wing328 could you have a look at this one again? I think this is a regression since it used to work correctly

@wing328 This is not about enhancing generators, it’s an issue of fixing DefaultCodegen.java to create the models correctly in the first place. The changes made to support multiple inheritance and OA3 appear to have made the situation worse, not better. For example, basic Swagger 2.0 allOf aggregation works in the current release version, but is broken in the snapshot, because the code for resolving parents now only understands discriminator-based parent relationships. If 4.0 is released as-is, it is going to break with a lot of specs that previously worked.

There should have been a comprehensive regression test for what aggregation was supported before the attempts to implement OA3 began, to ensure those changes didn’t break anything. As things stand, the codebase is already broken WRT inheritance, so the task now is to both implement a comprehensive regression test while simultaneously fixing the default codegen. I’m happy to try to help with this, but it’s going to take me a while to understand everything that’s happening in DefaultCodegen.java.

Any update on this?

I cannot manage to get inheritance when generating client for dart

Same here, just upgraded to the latest version 4, and interfaces generated with typescript-angular no longer extend other interfaces when using allOf without discriminator. I understand that this seems to be correct according to the spec:

Still, it does not imply a hierarchy between the models. For that purpose, you should include the discriminator.

But: This is a breaking change. IMHO, at the very least it should be pointed out / documented, at best we’d have a fallback option as @lemoinem suggested.

I think I have the same error that allOf is not supprted correctly. I use a derived type as request body. I´m using maven generator “java” with different libraries - native, retrofit , … 4.3.0-SNAPSHOT

  /api/system/users:
    post:
      tags:
        - users
      description: CreateUser https://udemo.tixxt.com/docs/api/system/#api-Users-CreateUser
      operationId: CreateUser
      requestBody:
        required: true
        content:
          application/x-www-form-urlencoded:
            schema:
              $ref: '#/components/schemas/UserCreate'
      responses:
        default:
          description: default response
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/User'
[...]


    UserUpdate:
      type: object
      properties:
        external_id:
          type: string
[...]
    UserCreate:
      allOf:
        - $ref: '#/components/schemas/UserUpdate'
        - type: object
          properties:
            send_welcome_mail:
              type: boolean

The class UserCreate is created with all fields from UserUpdate and the send_welcome_email property - but the method has no argument.

public User createUser() throws ApiException {

So why is this issue marked as closed? Please reopen…

Hod do you guys set supportsMultipleInheritance/supportsInheritance for typescript-angular generator? I did try --additional-properties supportsMultipleInheritance/supportsInheritance=true with no success. There is also no mention about such a parametres in here: https://github.com/OpenAPITools/openapi-generator/blob/master/docs/generators/typescript-angular.md Thanks.

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.2.md#composition-and-inheritance-polymorphism

I’m afraid we’ve to stick with the definitions defined in the specification.

In your case, why not update the OpenAPI documents to use the discriminator field?

Expected output The following class/interface hierarchy: to be generated:

BasicErrorModel <- ExtendedErrorModel

@jonrimmer I read your issue report again and notice there may be an incorrect interpretation of allOf (composition vs inheritance).

In the example you provided, BasicErrorModel does not contain discriminator (example) so model composition is used instead

Ref:

• https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#discriminator-object
• https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#schemaComposition

@jonrimmer you are right, there should be some more tests. would you like to give a hand? cc @wing328

I have no issues with the spec other than that I fail to understand what should be a discriminator in this case:

{
"definitions": {
  "Category": {
    "title": "Category",
    "additionalProperties": false,
    "properties": {
      "id": {
        "type": "number"
      },
      "tags": {
        "type": "array",
        "items": {
          "type": "string",
          "enum": [
            "foo",
            "bar",
            "baz"
          ]
        }
      }
    }
  },
  "Product": {
    "title": "Product",
    "additionalProperties": false,
    "properties": {
      "id": {
        "type": "number"
      },
      "categoryId": {
        "type": "number"
      }
    }
  }
},
"title": "CategoryWithRelations",
"allOf": [
  {
    "$ref": "#/definitions/Category"
  },
  {
    "properties": {
      "products": {
        "type": "array",
        "items": {
          "$ref": "#/definitions/Product"
        }
      }
    }
  }
]
}

But that’s OK, I can live with composition in TypeScript case. The problem I have is that I see no way to distinguish own properties and those pulled from ref in templates. isInherited flag is not set and there is no isMixedIn flag. In the example above this makes the generator create 2 separate tags enums which leads to Category and CategoryWithRelations being not compatible TypeScript types.

typescript-fetch generator is also broken in 4.3.1 Inheritance without a discriminator works perfectly fine in 4.3.0. In 4.3.1 a subclass gets all the properties of a superclass instead of inheritance and an extra class named SubclassAllOf is generated. @wing328 Please reopen this

@lacksfish @jonrimmer Following the advice to leverage the incredible https://github.com/APIDevTools/json-schema-ref-parser tool, I threw together a little script that does a hybrid de-reference of the openapi schemas (models). This way, you can still keep the non-inheritance/polymorphism-related references (for example, properties of an object that just reference another defined object), but still collapse any allOf references in a model.

I hope it helps you guys.

'use strict';

const fs = require('fs');
const $RefParser = require('json-schema-ref-parser');

const input = 'openapi.json';
const output = 'openapi--dereferenced.json';

(async () => {
    try {
        // parse the full spec file
        let openapi = await $RefParser.parse(input);

        // resolve the schema to get the complete set of references
        let $refs = await $RefParser.resolve(openapi);

        // loop over each schema (model) in the schema
        for (const schema in openapi.components.schemas) {
            // resolve the individual openapi schema object
            let $schema = await $RefParser.resolve(openapi.components.schemas[schema]);
            // loop over the references in the openapi schema object
            for (const path in $schema.values()) {
                // we need to manually 'de-reference' any composed models
                if (!$schema.get(path).allOf) {
                    continue
                }
                // for each reference in the 'allOf' statement, make sure we actually have
                // a reference (as opposed to just an inline model)
                $schema.get(path).allOf.forEach(element => {
                    if (element.$ref) {
                        // if the reference element matches the reference in the schema,
                        // replace the reference object with the actual object, thereby
                        // 'de-referencing' the composed model
                        const predicate = i => i.$ref === element.$ref;

                        let index = 0
                        while (index <= openapi.components.schemas[schema].allOf.length - 1) {
                            if (predicate(openapi.components.schemas[schema].allOf[index])) {
                                // get the inherited model from the master collection of references
                                openapi.components.schemas[schema].allOf.splice(index, 1, $refs.get(path + element.$ref))
                            }
                            index++;
                        }
                    }
                });
            }
        }
        let bundle = await $RefParser.bundle(openapi);
        fs.writeFileSync(output, JSON.stringify(bundle, null, 2));
    }
    catch (err) {
        console.error(err);
    }
})();

This getting UNKNOWN_BASE_TYPE UNKNOWN_BASE_TYPE with spring-boot generator for the Cat Dog example above

@bricox I think we’ve only added better anyOf support (beta) to the Ruby client generator. We welcome contributions to make similar enhancements to other generators.

I retested with the current version and the generated files are working fine with discriminator inheritance (elm and java/springboot).

For both languages there are additional files generated with an AllOf suffix that weren’t before. (MessageAllOf.elm / MessageAllOf.java). Not an issue, but I noticed it as a difference.

Thanks to everybody who took a look at the issue 😃

We just started adding better support for allOf, anyOf, oneOf to different generators and I foresee it will take a while for all generators to support these new features in OAS v3.

If you or anyone wants to help with the implementation of these new features in any generators, let me know and I’ll show you some good starting points.