[BUG] No more inheritance for components generated by version 5.0.0

antonio-petricca commented 3 years ago

Description

No schema inheritance.

openapi-generator version

5.0.0

OpenAPI declaration file content or url

schemas:
  UserDTO:
      type: object
      properties:
        id:
          type: string
          minLength: 1
          maxLength: 20

  QueryUserResponseDTO:
      type: object
      allOf:
        - $ref: '#/components/schemas/UserDTO'

Generation Details

The above YAML produces:

public class QueryUserResponseDTO   {
   private @Valid String id;
}

Instead as for the 4.3.1:

public class QueryUserResponseDTO extends UserDTO   {
}

Suggest a fix

Does exist a configuration parameter to enforce the DTO inheritance generation? Or it is a bug?

Msurrow commented 3 years ago

Just to confirm. I updated from 4.3.1 -> 5.0.0 and I'm seeing the same issue (with dotnet-core generator) and AllOf inheritance. For us, this makes vers 5.0.0 unusable.

If any help, here is a git diff of 4.3.1 generated class and 5.0.0 generated class which shows the issue:

superclass gone form class signature
base construtor not called
superclass properties added to subclass

namespace FOOBAR.Model
 {
@@ -29,7 +29,7 @@ namespace FOOBAR.Model
     /// ProductBankLoan
     /// </summary>
     [DataContract(Name = "ProductBankLoan")]
-    public partial class ProductBankLoan : Product, IEquatable<ProductBankLoan>
+    public partial class ProductBankLoan : IEquatable<ProductBankLoan>, IValidatableObject
     {
         /// <summary>
         /// Initializes a new instance of the <see cref="ProductBankLoan" /> class.
@@ -38,9 +38,12 @@ namespace FOOBAR.Model
         /// <param name="productID">productID.</param>
         /// <param name="debtors">debtors.</param>
         /// <param name="collaterals">collaterals.</param>
-        public ProductBankLoan(double balance = default(double), int productID = default(int), List<Debtor> debtors = default(List<Debtor>), List<Collateral> collaterals = default(List<Collateral>)) : base(productID, debtors, collaterals)
+        public ProductBankLoan(double balance = default(double), int productID = default(int), List<Debtor> debtors = default(List<Debtor>), List<Collateral> collaterals = default(List<Collateral>))
         {
             this.Balance = balance;
+            this.ProductID = productID;
+            this.Debtors = debtors;
+            this.Collaterals = collaterals;
         }

antonio-petricca commented 3 years ago

I am going to revert to 4.3.1. :(

Msurrow commented 3 years ago

@antonio-petricca I'm getting this from the log when running the generator:

[main] INFO  o.o.codegen.utils.ModelUtils - [deprecated] inheritance without use of 'discriminator.propertyName' has been deprecated in the 5.x release. Composed schema name: null. Title: null
[main] INFO  o.o.codegen.utils.ModelUtils - [deprecated] inheritance without use of 'discriminator.propertyName' has been deprecated in the 5.x release. Composed schema name: null. Title: null

I havn't had time to dive into it fully yet (we've reverted to 4.3.1 for now) but from what I can gather it seems that my swagger.json doesnt have the discriminator field on the supertype. However I'm using asp.net swashbuckle to generate the swagger.json/.yaml which is setup to OAS3. So what I havn't figured out yet is if the problem really is the yaml not being generated correctly by swashbuckle (i.e. missing the discriminator field). I cant seem to get it to generate the discriminator field. (yet - working on it when I have time)

felschr commented 3 years ago

This happened to me after upgrading from 5.0.0-beta3 to 5.0.0. The generated code using the typescript-fetch & dotnet generators is missing the inheritance. Our openapi schema hasn't changed.

UPDATE: Like @Msurrow we're using Swasbuckle and we disabled the oneOf & discriminator generation because we were previously hitting some other issues with the resulting unions that openapi-generator is creating (#7367). Using allOf inheritance with a discriminator instead doesn't seem to be currently supported by Swashbuckle, see domaindrivendev/Swashbuckle.AspNetCore/issues/1973.

adsanche commented 3 years ago

I have the same problem from 5.x.x.

Actually, the following warning doesn't make sense :

[INFO] [deprecated] inheritance without use of 'discriminator.propertyName' has been deprecated in the 5.x release. Composed schema name: null. Title: null

As I have a propertyName placed on the discriminator on the parent object :

DeliveryDateApi:
      description: 'Delivery date information.'
      oneOf:
        - $ref: '#/components/schemas/RangeDeliveryDateApi'
        - $ref: '#/components/schemas/FixedDeliveryDateApi'
      discriminator:
        propertyName: type
        mapping:
          RANGE: '#/components/schemas/RangeDeliveryDateApi'
          FIXED: '#/components/schemas/FixedDeliveryDateApi'

Are there any update on this that could avoid rollbacking to 4.3.1?

antonio-petricca commented 3 years ago

I reverted to 4.3.1.

tamershahin commented 3 years ago

I have the same problems with kotlin-spring generator. is this independent from the generator?

Blackclaws commented 3 years ago

This might be a bug in how ambiguousness of inheritance is being determined. I'm on it

tamershahin commented 3 years ago

This might be a bug in how ambiguousness of inheritance is being determined. I'm on it

I don't know if it's related to this bug, but I documented what worked for me here: https://github.com/OpenAPITools/openapi-generator/issues/8687

Blackclaws commented 3 years ago

So right now the issue is the following:

The spec itself says that allOf by itself does not imply a hierarchic relationship between the components. Therefore even if there is only a single reference in allOf this means that generating an inheritance relationship between them is not per se wrong (as the spec does not forbid it) but it also goes further than the spec and infers something that some people might not want to have.

I've opened an issue on the OpenAPI Spec itself to maybe get some clarification into the spec as to whether treating a single allOf ref as inheritance might make sense.

For now the solution is to simply add a discriminator property to the base object in order to get inheritance.

Msurrow commented 3 years ago

For now the solution is to simply add a discriminator property to the base object in order to get inheritance.

@Blackclaws Its some time since I've looked at this, but for me part of the problem is that the spec generated by Swashbuckle doesn't include a discriminator on the base object.

You can see a code example in my post above, and a snippet of the generated spec is shown below. Perhaps this is the wrong place for this question (but you mentioned discriminators), but am I doing something wrong here?


      "Product": {
        "type": "object",
        "properties": {
          "productId": {
            "type": "integer",
            "format": "int32"
          },
          "debtors": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/Debtor"
            },
            "nullable": true
          },
          "collaterals": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/Collateral"
            },
            "nullable": true
          }
        },
        "additionalProperties": false
      },
      "ProductBankLoan": {
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/Product"
          }
        ],
        "properties": {
          "balance": {
            "type": "number",
            "format": "double"
          }
        },
        "additionalProperties": false
      }

This is using the configuraton:

Services.AddSwaggerGen(c =>
            {
                // Add discrimitators to swagger generator to handle inheritance types e.g. Product
                c.UseAllOfForInheritance();

                c.SwaggerDoc("v1", new OpenApiInfo { Title = "FOOBARApi", Version = "v1" });
            });

Blackclaws commented 3 years ago

You need to actually add a discriminator to your base class and then add the annotation [SwaggerDiscriminator("discriminatorPropertyName")] as well. That has worked for me.

marabunta62 commented 3 years ago

I am going to revert to 4.3.1. :(

I should do the same. Impossible to generate with 5.X.X "com.fasterxml.jackson.core.JsonParseException: Unrecognized token 'openapi': was expecting .... truncated XXX chars" Inspect with cli validate my spec bring me here

Blackclaws commented 3 years ago

The problem is that the behavior of adding inheritance to schema composition by allOf without a discriminator was actually non-compliant regarding the spec.

If you add a discriminator everything should work as expected. See also the linked issue in the spec repository. If you want this sort of behavior then please go ahead an lobby for general code generation hints as part of the spec.

valkum commented 3 years ago

We hit the same issue with model composition as described here: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

rgoers commented 3 years ago

The problem is that the base class can't always know all the classes that will extend it. In my case, #9756 it isn't even necessary as the base class isn't used for Serialization/Deserialization. But I need the base class for methods that operate on the fields it provides. In my environment we have a "BasePage" object for paged requests. All the services that need paging extend that. BasePage can't possibly know all the services that will support paging as it is in a common library that the services import. This change has made it impossible to support this.

wing328 commented 1 year ago

Hi all, I've filed https://github.com/OpenAPITools/openapi-generator/pull/14172 to allow using $ref as parent in allOf with a new option called "openapi-normalizer".

Please give it a try as follows:

git clone --depth 1 -b allof-parent-type https://github.com/OpenAPITools/openapi-generator/
cd openapi-generator
mvn clean package -DskipTests -Dmaven.javadoc.skip=true
java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i /Users/williamcheng/Code/openapi-generator7/modules/openapi-generator/src/test/resources/3_0/allOf_extension_parent.yaml -o /tmp/java-okhttp/ --additional-properties hideGenerationTimestamp="true" --openapi-normalizer REF_AS_PARENT_IN_ALLOF=true

endreszilagyi1 commented 1 year ago

Hi @wing328,

we tried it out with your suggestion, however didn't worked. We are getting the following error: Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.util.List.iterator()" because the return value of "io.swagger.v3.oas.models.media.Schema.getAllOf()" is null at org.openapitools.codegen.OpenAPINormalizer.normalizeOneOf(OpenAPINormalizer.java:328) at org.openapitools.codegen.OpenAPINormalizer.normalizeSchema(OpenAPINormalizer.java:276) at org.openapitools.codegen.OpenAPINormalizer.normalizeContent(OpenAPINormalizer.java:154) at org.openapitools.codegen.OpenAPINormalizer.normalizeRequestBody(OpenAPINormalizer.java:180) at org.openapitools.codegen.OpenAPINormalizer.normalizePaths(OpenAPINormalizer.java:130) at org.openapitools.codegen.OpenAPINormalizer.normalize(OpenAPINormalizer.java:101) at org.openapitools.codegen.DefaultGenerator.configureGeneratorProperties(DefaultGenerator.java:261) at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:907) at org.openapitools.codegen.cmd.Generate.execute(Generate.java:473) at org.openapitools.codegen.cmd.OpenApiGeneratorCommand.run(OpenApiGeneratorCommand.java:32) at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:66)

This is the command I'm using: java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i swagger.json -o .\generated --additional-properties hideGenerationTimestamp="true" --openapi-normalizer REF_AS_PARENT_IN_ALLOF=true

When executing the same command but without --openapi-normalizer REF_AS_PARENT_IN_ALLOF=true works fine. Could you please check if we are doing something wrong or it is a bug?

I'm using windws 10 with JDK 19 x64 and maven 3.8.7.

Thank in advance for you help.

wing328 commented 1 year ago

can you share the spec to repeat the issue? hard to troubleshoot without the spec.

wing328 commented 1 year ago

closing this as https://github.com/OpenAPITools/openapi-generator/pull/14172 has been merged.

please use https://github.com/OpenAPITools/openapi-generator/blob/master/docs/customization.md#openapi-normalizer moving forward.

wing328 commented 1 year ago

@endreszilagyi1 I've merged https://github.com/OpenAPITools/openapi-generator/pull/15224 with better null check. Hopefully that should address the issue you reported.

OpenAPITools / openapi-generator