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
20.61k stars 6.29k forks source link

[BUG][protobuf-schema] Protobuf generator does not respect enableEnumPrefixRemoval #11484

Open JulianGmp opened 2 years ago

JulianGmp commented 2 years ago

Bug Report Checklist

Description

When setting enableEnumPrefixRemoval=false, the protobuf-schema generator does not respect the flag and removes the enum value prefixes anyway.

openapi-generator version
openapi-generator-cli 6.0.0-SNAPSHOT
  commit : 8a2131f
  built  : 2022-02-01T15:07:00+01:00
  source : https://github.com/openapitools/openapi-generator
  docs   : https://openapi-generator.tech/
OpenAPI declaration file content or url

test.yaml

openapi: 3.0.0
info:
  title: Sample API
  description: Optional multiline or single-line description in [CommonMark](http://commonmark.org/help/) or HTML.
  version: 0.1.9

servers:
  - url: http://api.example.com/v1
    description: Optional server description, e.g. Main (production) server

components:
  schemas:
    userinfo:
      properties:
        something:
          type: string
          format: enum
          enum:
          - MY_ENUM_a
          - MY_ENUM_b
        somethingElse:
          type: string
          format: enum
          enum:
          - MY_OTHER_ENUM_a
          - MY_OTHER_ENUM_bla

paths:
  /users:
    get:
      summary: Returns a list of users.
      description: Optional extended description in CommonMark or HTML.
      responses:
        '200':
          description: A JSON array of user names
          content:
            application/json:
              schema:
                $ref: '#components/schemas/userinfo'
Generation Details

Generated output:

syntax = "proto3";

package openapitools;

message Userinfo {

  enum SomethingEnum {
    A = 0;
    B = 1;
  }

  SomethingEnum something = 28014471;

  enum SomethingElseEnum {
    A = 0;
    BLA = 1;
  }

  SomethingElseEnum somethingElse = 396934355;

}

Expected:

syntax = "proto3";

package openapitools;

message Userinfo {

  enum SomethingEnum {
    MY_ENUM_A = 0;
    MY_ENUM_B = 1;
  }

  SomethingEnum something = 28014471;

  enum SomethingElseEnum {
    MY_OTHER_ENUM_A = 0;
    MY_OTHER_ENUM_BLA = 1;
  }

  SomethingElseEnum somethingElse = 396934355;

}

NOTE: This is especially a problem with protobuf, because enum values have to be globally unique (imagine C enums instead of Java enums). This is a general issue of translating openapi to protobuf, but that's out of scope for this bug report.

I added a prefix to the values in my project, specifically to prevent this, but the generator removes it despite the option set.

Note that I tried the java generator too, which respected the setting.

Steps to reproduce
java -jar ./openapi-generator-cli-master.jar generate -g protobuf-schema -i test.yaml -o testout/ --additional-properties 'enableEnumPrefixRemoval=false'
Related issues/PRs
Suggest a fix
Yurzel commented 2 years ago

Yea I don't know about this. So far there is an option to put Unknown = 0 to the enum which always starts with a prefix of the enum name.

With that in mind I made the prefix across all the enum values. It should fix the collision issue aswell. I'll link PR to this issue.

The result would be:

enum SomethingEnum {
   SomethingEnum.A = 0;
   SomethingEnum.B = 1;
}

enum SomethingElseEnum {
   SomethingElseEnum.A = 0;
   SomethingElseEnum.BLA = 1;
}

There is actually a problem in Python code generated by grpcio, where you get collision between message name and enum value.😄

bintoll commented 1 year ago

Hello! I have just started using this generator and I feel that this implementation is not correct. This codegen changes actually breaks the initial idea of code generation based on schema, because after this change the schema is no longer valid. Here is me setup: nestjs, @grpc/grpc-js, @grpc/proto-loader, class-validator to validate incoming payload, swagger to generated openapi schema based on code, and this generator to generate protobuf based on openapi schema. In code my enum looks like:

enum IdentifierType {
  EMAIL = 'EMAIL',
  PHONE = 'PHONE'
}

After using this generator I get this enum in protobuf:

enum IdentifierType {
  IdentifierType_EMAIL = 0;
  IdentifierType_PHONE = 1;
}

Which is not the same. It results in having the request payload:

{
  identifierType: 'IdentifierType_EMAIL'
}

However it must be:

{
  identifierType: 'EMAIL'
}

to pass the validation pipe.

I completely understand the limitation that each enum name has to be globally unique. From my point of view generator with default config should keep track of all enum values during the generation process and throw an error if there are some non unique enums and it should NOT automatically add this prefixes leading to the fact that the generated protobuf schema is not completable with the initial openapi schema. And for those who really want generator to add this prefixes there should be a custom config option enabling this, because again, the idea of using a generator is to get the exactly same schema based on other schema, but now it just breaks initial complitability.

UPD: This generally happens because @grpc/proto-loader transforms received enum number value to string using enum keys (this behavior was configured for @grpc/proto-loader which is quite usual I think). Using numeric enums solves this issue, how ever I still think that this feature to add prefixes in generated protobuf schema should be enabled with custom parameter and not by default.

bintoll commented 1 year ago

@wing328 Hello! Could you please comment this? I would not like to open another issue with same topic as long as there are thousands of them here. And as long as you maintain this generator, you know the specific better than no-one else.