Skip to content

Conversation

@zhemant
Copy link
Contributor

@zhemant zhemant commented Jan 2, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

fix #1780 , Along with setting isModel to true, set isEnum to true when a model is generated for enum. This will allow better handling for enum variables when enum is of nonPrimitiveType.

@wing328 @OpenAPITools/generator-core-team

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhemant What about the following instead?

if (ref != null && ref.getEnum() != null) {
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is better. I will make these changes and push again.

@zhemant
Copy link
Contributor Author

zhemant commented Jan 7, 2019

For this spec: https://pastebin.com/f7RZsL4J, when pet model is generated it has following model structure:

      "baseName" : "status",
      "complexType" : "status",
      "getter" : "getStatus",
      "setter" : "setStatus",
      "dataType" : "status",
      "datatypeWithEnum" : "status",
      "name" : "status",
      "defaultValueWithParam" : " = data.status;",
      "baseType" : "status",
      "example" : "null",
      "jsonSchema" : "{\n  \"$ref\" : \"#/components/schemas/Status\"\n}",
      "exclusiveMinimum" : false,
      "exclusiveMaximum" : false,
      "hasMore" : false,
      "required" : false,
      "secondaryParam" : false,
      "hasMoreNonReadOnly" : false,
      "isPrimitiveType" : false,
      "isModel" : true,
      "isContainer" : false,
      "isString" : false,
      "isNumeric" : false,
      "isInteger" : false,
      "isLong" : false,
      "isNumber" : false,
      "isFloat" : false,
      "isDouble" : false,
      "isByteArray" : false,
      "isBinary" : false,
      "isFile" : false,
      "isBoolean" : false,
      "isDate" : false,
      "isDateTime" : false,
      "isUuid" : false,
      "isEmail" : false,
      "isFreeFormObject" : false,
      "isListContainer" : false,
      "isMapContainer" : false,
      "isEnum" : false,
      "isReadOnly" : false,
      "isWriteOnly" : false,
      "isNullable" : false,
      "isSelfReference" : false,
      "vendorExtensions" : { },
      "hasValidation" : false,
      "isInherited" : false,
      "nameInCamelCase" : "Status",
      "nameInSnakeCase" : "STATUS",
      "isXmlAttribute" : false,
      "isXmlWrapped" : false,
      "datatype" : "status",
      "iexclusiveMaximum" : false

isModel is set and isPrimitiveType is false, with this much info is not sufficient to handle enum in language like C, so if we can set isEnum flag then in mustache we can handle model enums in non primitive part

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using ModelUtils.unaliasSchema(globalSchemas, p) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already done for schema p in the same function before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have analyzed the same code: ModelUtils.unaliasSchema(globalSchemas, p) is returning the reference-schema to indicate that a outer-enum must be created (which is correct).

You could use ModelUtils.getReferencedSchema(this.openAPI, p) to get the referenced schema regardless of any other consideration.

@wing328
Copy link
Member

wing328 commented Jan 14, 2019

Looks like this will cause a lot of changes to other generators (e.g. C#) as shown in the CircleCI logs.

We need to consider other ways to address the problem you're facing.

@zhemant
Copy link
Contributor Author

zhemant commented Jan 24, 2019

Can we override the fromProperty method in CLibCurlClient.java and there we set isEnum tag? I have an idea of it, I am already sending openAPI for projectName so may be I can use it to set a globalvariable there and use it during fromProperty

@wing328
Copy link
Member

wing328 commented Jan 26, 2019

When you've time in the coming week, please ping me via Gitter and we can discuss other solutions in details.

@wing328
Copy link
Member

wing328 commented Jan 26, 2019

Closing this one for the time being.

@wing328 wing328 closed this Jan 26, 2019
@jmini
Copy link
Member

jmini commented Jan 29, 2019

Please have a look at PR #2001 I have filed to fix the default value case where an outer-enum is created.

components:
  schemas:
    TestObject:
      type: object
      properties:
        myEnum:
          $ref: '#/components/schemas/MyEnum'
    MyEnum:
      type: string
      default: FIRSTVALUE
      enum:
      - FIRSTVALUE
      - SECONDVALUE

For the java generators property.isEnum for myEnum can not be set to true, because this is the trigger to indicate that an inner enum needs to be generated. I think it is the case for other languages.

My PR sets allowableValues in case of outer enum (enum created as separated type).

I did not read the details behind your problem, but I have the felling that the topics are connected.

Feel free to continue the discussion.

@zhemant
Copy link
Contributor Author

zhemant commented Jan 29, 2019

I had a look at PR #2001, as I haven't dealt with the default value in an enum, but sure it will be helpful in case default value needs to be set.

I want to differentiate between enum and non-primitive datatype but we only have isModel flag set to true rest all flags are false. so it was creating an issue. hence I thought of setting isEnum flag but didn't know it would interfere with other generators.

By looking at your code, I got a few hints on how I can handle it in CLibCurl.java so it doesn't interfere with other generators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REQ][General] Set boolean flags in case of enum in non-primitive type

3 participants