OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.42k stars 6.48k forks source link

[JAVA] Cannot use single discriminator field for two-level inheritance #1113

Open wheezil opened 6 years ago

wheezil commented 6 years ago
Description

I would like to use "inheritance" via allOf in the spec, and I'd like more than one level for my request schemas. This will allow me to have models all share a common base, but then further split them into categories. It seems that openapi-generator supports this, and validates such a spec, but the Java codegen does not produce valid code if the same discriminator field is used at both levels.

openapi-generator version

3.3.0-SNAPSHOT

OpenAPI declaration file content or url
{
   "openapi": "3.0.0",
   "info": {
      "description": "blah",
      "version": "1.0.0",
      "title": "blah"
   },
   "paths": {
      "/test1": {
         "post": {
            "tags": [
              "test"
            ],
            "operationId": "testOp1",
            "responses": {
               "200": {
                  "description": "Success",
                  "content": {
                     "application/json": {
                        "schema": {
                           "oneOf": [
                              { "$ref": "#/components/schemas/Response1" },
                              { "$ref": "#/components/schemas/Response2" }
                           ]
                        }
                     }
                  }
               },
               "405": {
                  "description": "Invalid input"
               }
            },
            "requestBody": {
               "content": {
                  "application/json": {
                     "schema": {
                        "$ref": "#/components/schemas/Request"
                     }
                  }
               }
            }
         }
      }
   },
   "components": {
      "schemas": {
         "Base": {
            "properties": {
               "type":{
                  "type":"string"
               }
            },
            "discriminator":{
               "propertyName": "type"
            }
         },
         "Request": {
            "allOf":[
               { "$ref": "#/components/schemas/Base" },
               {
                  "type":"object",
                  "properties": {
                     "resource": {
                        "type": "string"
                     }
                  }
               }
            ]
         },
         "ResponseBase": {
            "allOf":[
               { "$ref": "#/components/schemas/Base" }
            ],
            "discriminator":{
               "propertyName": "type"
            }
         },
         "Response1": {
            "allOf":[
               { "$ref": "#/components/schemas/ResponseBase" },
               {
                  "type":"object",
                  "properties": {
                     "status": {
                        "type": "string"
                     }
                  }
               }
            ]
         },
         "Response2": {
            "allOf":[
               { "$ref": "#/components/schemas/ResponseBase" },
               {
                  "type":"object",
                  "properties": {
                     "name": {
                        "type": "string"
                     }
                  }
               }
            ]
         }
      }
   }
}
Command line used for generation

java -jar OPENAPIJAR.jar generate -o genjava -g java -i two_level_inheritance.json -c rpdmcpp.config

Steps to reproduce

Run above command and load generated maven project

Related issues/PRs

1068 (CSharp generator)

Suggest a fix/enhancement

The problem is that the discriminator field is private in the base class, yet the intermediate class attempts to set it in the constructor:

  public ResponseBase() {
    this.type = this.getClass().getSimpleName();
  }

There is no reason to when there is a parent using the same discriminator, because the parent (which has access to the field) will do it anyway. I believe this happens around line 81 of pojo.mustache:

  {{^parcelableModel}}
  {{#gson}}
  {{#discriminator}}
  public {{classname}}() {
    this.{{{discriminatorName}}} = this.getClass().getSimpleName();
  }
  {{/discriminator}}
  {{/gson}}
  {{/parcelableModel}}

It may be impossible to condition mustache template on "if parent has the same discriminator field". However the setter is public. Can we know what is the setter for the discriminator? Can the fields be made protected instead of private?

pcoltau commented 5 years ago

I have the same problem.

And in addition, I'm not sure setting the value of the discriminatorName field to the class name is a good idea, as it will override the actual value, which might be something other than the class name (if a discriminator mapping has been used).

wheezil commented 5 years ago

In my custom C++ generator, I have added a "discriminator" argument to the constructor of every model class, which defaults to the class name if unspecified. (Cause in C++ you can't just ask for the class name). This allows for derived classes to hand THEIR class name down to superclass, at that produces the right default during construction. You don't need to worry about this overriding the actual value, because this is set during deserialization which happens after construction. The point is to get the correct default matching the actual class instance. In Java, it would be sufficient to make the discriminator field name protected instead of private.