Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model not generated with the right type (allOf) #582

Closed
JFCote opened this issue Jul 17, 2018 · 7 comments
Closed

Model not generated with the right type (allOf) #582

JFCote opened this issue Jul 17, 2018 · 7 comments

Comments

@JFCote
Copy link
Member

JFCote commented Jul 17, 2018

Description

A member of a model is not using the model type requested by the spec. Please check the example for a better explanation

openapi-generator version

Master (3.1.1-SNAPSHOT)

OpenAPI declaration file content or url

This is a self contained file that reproduce the issue

swagger: "2.0"
info:
  description: "dfsdfsdfs"
  version: "1.0.0"
  title: "Swagger Petstore"
  termsOfService: "http://swagger.io/terms/"
  contact:
    email: "apiteam@swagger.io"
  license:
    name: "Apache 2.0"
    url: "http://www.apache.org/licenses/LICENSE-2.0.html"
host: "petstore.swagger.io"
basePath: "/v2"
schemes:
- "https"
- "http"
paths:
  /lifecycles:
    put:
      tags:
        - Device
      summary: xxxxxxx
      operationId: editDeviceLifecycles
      parameters:
      - in: body
        name: device-lifecycle-update-info
        required: true
        schema:
          $ref: "#/definitions/DeviceLifecycleCore"
      responses:
        200:
          description: OK
          schema:
            $ref: "#/definitions/DeviceLifecycleCore"
definitions:
  DeviceLifecycleCore:
    type: object
    description: "DeviceLifecycleCore"
    properties:
      aaa:
        type: string
        description: "aaa"
      bbb:
        type: string
        description: "bbb"
      ccc:
        type: string
        description: "ccc"
      returnSoftwareStatus:
        $ref: "#/definitions/DeviceLifecycleStatus"
  DeviceLifecycleStatus:
    type: object
    description: "DeviceLifecycleStatus"
    allOf:
      - $ref: "#/definitions/DeviceLifecycleStatusCore"
      - type: object
        required:
          - id
        properties:
          id:
            type: integer
            format: int64
            description: "Id associated with the status"
            example: 123
  DeviceLifecycleStatusCore:
    type: object
    description: "DeviceLifecycleStatusCore"
    properties:
      name:
        type: string
        description: "Name"
      details:
        type: string
        description: "Any additional information needed for the status"
        example: "Something is wrong"

As you can see returnSoftwareStatus is supposed to be of type DeviceLifecycleStatus. If you look in the generated code, you will see that it is of type DeviceLifecycleStatusCore instead:

public DeviceLifecycleCore returnSoftwareStatus(DeviceLifecycleStatusCore returnSoftwareStatus) {
    this.returnSoftwareStatus = returnSoftwareStatus;
    return this;
  }

   /**
   * DeviceLifecycleStatus
   * @return returnSoftwareStatus
  **/
  @Valid
  public DeviceLifecycleStatusCore getReturnSoftwareStatus() {
    return returnSoftwareStatus;
  }

  public void setReturnSoftwareStatus(DeviceLifecycleStatusCore returnSoftwareStatus) {
    this.returnSoftwareStatus = returnSoftwareStatus;
  }
Command line used for generation

java -jar openapi-generator-cli.jar generate -i ./swagger.yaml --generator-name java-play-framework -o generatedServer -DhideGenerationTimestamp=true,booleanGetterPrefix=is

Steps to reproduce

Just run the command above

Related issues/PRs

None

Suggest a fix/enhancement

?

@macjohnny
Copy link
Member

@JFCote I had a similar issue with 3.0.0, which was solved by upgrading to 3.1.0. In my case it was the body parameter that had the wrong type.

@JFCote
Copy link
Member Author

JFCote commented Jul 17, 2018

@macjohnny Yeah I had this too. It's all fixed with 3.1.X but this is something else. I'm using 3.1.1-SNAPSHOT and still got this. I think it's a different use case.

@jmini jmini changed the title Model not generated with the right type Model not generated with the right type (allOf) Jul 17, 2018
@jmini
Copy link
Member

jmini commented Jul 17, 2018

Related issues/PRs: #340 (but not directly)

@rubms
Copy link
Contributor

rubms commented Jul 30, 2018

I am also affected by this bug.

The problem comes from the a897fee commit in the #360 pull request. More precisely, from this change a897fee#diff-7cb46fa53f89a458a7b7cb201d2214a8R1527.

Before this commit, when these properties where not un-aliased, a Schema instance was used to compose the model property of composed models (models containing allOf). This Schema instance contained the appropriate $ref field indicating the original datatype name, from which the data type name was correctly extracted via DefaultCodeGen::getSchemaType().

Now, with this un-aliasing, instead of a Schema with the proper $ref field, a ComposedSchema is used to compose the model properties of composed datatypes. In those ComposedSchemas the original data type name is lost and DefaultCodeGen::getSchemaType() returns the datatype of the first of the models involved in the allOff clause:

    public String getSchemaType(Schema schema) {
        if (schema instanceof ComposedSchema) {
            ComposedSchema cs = (ComposedSchema)schema;
            if (cs.getAllOf() != null) {
                Iterator var3 = cs.getAllOf().iterator();

                while(var3.hasNext()) {
                    Schema s = (Schema)var3.next();
                    if (s != null) {
                        schema = s;
                        break;
                    }
                }
            }
        }
...

Though the #360 pull request fixed the issues #255 and #191, it originated this #582 issue.

@wing328, do you think it would be possible to keep the fixes brought by #360 and at the same time have a proper generation of composed schemas?

Maybe something like this:

    public static Schema unaliasSchema(Map<String, Schema> allSchemas, Schema schema) {
        if (schema != null && StringUtils.isNotEmpty(schema.get$ref())) {
            Schema ref = allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref()));
            if (ref == null) {
                LOGGER.warn("{} is not defined", schema.get$ref());
                return schema;
            } else if (isObjectSchema(ref)) { // model
                return schema;
            } else if (isStringSchema(ref) && (ref.getEnum() != null && !ref.getEnum().isEmpty())) {
                // top-level enum class
                return schema;
            } else if (isMapSchema(ref) || isArraySchema(ref) || isComposedSchema(ref)) { // map/array def should be created as models
                return schema;
            } else {
                return unaliasSchema(allSchemas, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())));
            }
        }
        return schema;
    }

You can notice I added || isComposedSchema(ref) to the final else if. This way composed models would not get broken when creating properties for models.

@jmini
Copy link
Member

jmini commented Jul 30, 2018

This change looks good to me! Please create a PR for it.
In the ideal case, a unit test could help to ensure no regression.

rubms pushed a commit to rubms/openapi-generator that referenced this issue Aug 1, 2018
…type is a composed (allOf) schema. Before this fix, the data type name of the generated property was that of the first model participating in the allOf clause. After this fix the property data type is again as expected: the one of the composed schema and not one of its parents.
@rubms
Copy link
Contributor

rubms commented Aug 1, 2018

Just created the #704 pull request in order to get this issue fixed.

rubms pushed a commit to rubms/openapi-generator that referenced this issue Aug 1, 2018
…OpenAPITools#582 issue (references to composed schemas should not get unaliased for them to get a proper data type name in the generation of model properties).
JFCote pushed a commit that referenced this issue Aug 8, 2018
…d (allOf) schema (#704)

* #582 Fixed the generation of model properties whose data type is a composed (allOf) schema. Before this fix, the data type name of the generated property was that of the first model participating in the allOf clause. After this fix the property data type is again as expected: the one of the composed schema and not one of its parents.

* Added unit test in order to have regression testing in the fix for the #582 issue (references to composed schemas should not get unaliased for them to get a proper data type name in the generation of model properties).

* Run ./bin/utils/ensure-up-to-date to re-generate samples run in the CI.

* Removed tabs from ModelUtilsTest.java
@JFCote
Copy link
Member Author

JFCote commented Aug 8, 2018

Fixed by #704

@JFCote JFCote closed this as completed Aug 8, 2018
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this issue Feb 27, 2019
…d (allOf) schema (OpenAPITools#704)

* OpenAPITools#582 Fixed the generation of model properties whose data type is a composed (allOf) schema. Before this fix, the data type name of the generated property was that of the first model participating in the allOf clause. After this fix the property data type is again as expected: the one of the composed schema and not one of its parents.

* Added unit test in order to have regression testing in the fix for the OpenAPITools#582 issue (references to composed schemas should not get unaliased for them to get a proper data type name in the generation of model properties).

* Run ./bin/utils/ensure-up-to-date to re-generate samples run in the CI.

* Removed tabs from ModelUtilsTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants