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

[BUG] Property isEnum false for referenced ENUM schemas #2645

Open
5 tasks done
DonDi94 opened this issue Apr 11, 2019 · 6 comments
Open
5 tasks done

[BUG] Property isEnum false for referenced ENUM schemas #2645

DonDi94 opened this issue Apr 11, 2019 · 6 comments

Comments

@DonDi94
Copy link
Contributor

DonDi94 commented Apr 11, 2019

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
Description

When generating the code of the Erlang server, I have noticed that the ENUM check would not work on one parameter. By reducing the file I found out that the generator wasn't putting the enum values in the generated code, not because allowableValues was null but because isEnum was set on false for referenced ENUM parameters.

openapi-generator version

4.0.0, commit b426bab

OpenAPI declaration file content or url
openapi: '3.0.0'
info:
  version: 1.0.0
  title: Enums Issue
paths: 
  /test:    
    get:
      operationId: TestEnum
      parameters:
        - name: paramA
          in: query
          required: true
          schema:
            $ref: '#/components/schemas/EnumString'
        - name: paramB
          in: query
          required: true
          schema:
            type: string
            enum:
              - first
              - second
              - third
      responses:
        default:
          description: unexpected error
          content:       
            application/json:
              schema:
                type: object
                properties:
                  message:
                    type: string
components:
  schemas:
    EnumString:
      type: string
      enum:
        - first
        - second
        - third
Command line used for generation

java -jar openapi-generator-cli.jar -i enums.yaml -g erlang-server -o server/

Steps to reproduce
  • Create the file enums.yaml
  • Generate the code with the command
  • Open server/src/openapi_api.erl, look at function request_param_info(line 52)
  • Compare the param infos of paramA and paramB

Obtained result:

request_param_info('TestEnum', 'paramA') ->
    #{
        source => qs_val  ,
        rules => [
            required
        ]
    };

request_param_info('TestEnum', 'paramB') ->
    #{
        source => qs_val  ,
        rules => [
            {type, 'binary'},
            {enum, ['first', 'second', 'third'] },
            required
        ]
    };

Expected result:

request_param_info('TestEnum', 'paramA') ->
    #{
        source => qs_val  ,
        rules => [
            {type, 'binary'},
            {enum, ['first', 'second', 'third'] },
            required
        ]
    };

request_param_info('TestEnum', 'paramB') ->
    #{
        source => qs_val  ,
        rules => [
            {type, 'binary'},
            {enum, ['first', 'second', 'third'] },
            required
        ]
    };
Suggest a fix

In DefaultCodegen, fromProperty, adding property.isEnum= true; at line 2068 after assigning allowableValues seems to fix the issue of the missing enum values, but doesn't assign any type to the property. To fix that I would move the type checking block right over the inline enum case as a function protected void setPropertyType(CodegenProperty property, Schema p, String name) and add two calls, one for p (setPropertyType(property, p, name)) and one for referencedSchema (setPropertyType(property, referencedSchema, name).

        //Referenced enum case:
        if (referencedSchema.getEnum() != null && !referencedSchema.getEnum().isEmpty()) {
            List<Object> _enum = referencedSchema.getEnum();

            Map<String, Object> allowableValues = new HashMap<String, Object>();
            allowableValues.put("values", _enum);
            if (allowableValues.size() > 0) {
                property.allowableValues = allowableValues;
                property.isEnum = true;
            }
        }
@auto-labeler
Copy link

auto-labeler bot commented Apr 11, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@jmini
Copy link
Member

jmini commented Jul 9, 2019

The isEnum topic for referenced enum is not new...

I have found one of my comment in swagger-api/swagger-codegen#7763 (comment)


There is good input to the topic in PR #3186 (That was not merged, exactly it was merged and reverted).


From an OpenAPI semantic perspective:

(Spec 1)

components:
  schemas:
    SomeObject:
      type: object
      Properties:
        id:
          type: integer
        someProp:
          $ref: '#/components/schemas/SomeEnum'
    SomeEnum:
      type: string
      enum: [a, b, c]

And (Spec 2):

components:
  schemas:
    SomeObject:
      type: object
      Properties:
        id:
          type: string
        someProp:
          type: string
          enum: [a, b, c]

are exactly the same, but the code generated by OpenAPI-Generator in both cases is not the same.


The semantic of isEnum in CodegenProperty is confusing.

For example Java generators are using this to decide if a new enum type must be generated (when isEnum is true) or if a shared enum object must be used.

Example:

Without any change to the Java-Generators:

components:
  schemas:
    MyObject:
      type: object
      Properties:
        id:
          type: integer
        someProp:
          $ref: '#/components/schemas/SomeEnum'
    SomeOtherObject:
      type: object
      Properties:
        firstName:
          type: string
        lastName:
          type: string
        initState:
          $ref: '#/components/schemas/SomeEnum'
    SomeEnum:
      type: string
      enum: [a, b, c]

If isEnum is true for MyObject.someProp and SomeOtherObject.initState then 2 enum objects will be generated:

  • MyObject.SomePropEnum
  • SomeOtherObject.InitStateEnum

Instead of using the shared model class SomeEnum.


Of course everything can be changed/improved to be more consistent. But we need to keep the current use-cases still working.


For your use case:

{type, 'datetime'},{{/isDateTime}}{{#isEnum}}
{enum, [{{#allowableValues}}{{#values}}'{{.}}'{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] },{{/isEnum}}{{#maximum}}
{max, {{maximum}} }, {{/maximum}}{{#exclusiveMaximum}}

Line 68 is inside a {#isEnum}..{/isEnum} check, IMO you can try to replace it with a
{{^allowableValues.empty}}...{{/allowableValues.empty}}.

In mustache java, this works (source).

 {type, 'datetime'},{{/isDateTime}}{{^allowableValues.empty}} 
 {enum, [{{#allowableValues}}{{#values}}'{{.}}'{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] },{{/allowableValues.empty}}{{#maximum}} 

@jmini
Copy link
Member

jmini commented Aug 15, 2019

Mentioned by @jaumard on Slack, following works for him to know if a field is an enum:

    {{#vars}}
        {{#allowableValues}}
            {{^enumVars.empty}}
                '{{name}}': const {{{datatype}}}FieldProcessor(),
            {{/enumVars.empty}}
        {{/allowableValues}}
    {{/vars}}

@erik-sab
Copy link

erik-sab commented Aug 21, 2019

This workaround works for single enum field
"rightOrig" : { "$ref" : "#/definitions/RightType", "originalRef" : "RightType" },
but does not work for array of enums
"rightsOrig" : { "type" : "array", "items" : { "$ref" : "#/definitions/RightType", "originalRef" : "RightType" } },
ah, made it work with {{{isListContainer}}} + {{{items.allowableValues}}}
thanks!

@adamscybot
Copy link

None of these tricks work if your enum is referenced.

@benmeiri
Copy link

@erik-telesoftas The suggested workaround would work only assuming that the enum is a String enum. The values in allowedValues is allowableValues={values=[REJECTED, RECYCLED]}.

If you will have a number enum, this would break, or you would get a false positive.

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

5 participants