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

Option for omitting zero enum value from generated Swagger #888

Closed
jgiles opened this issue Mar 2, 2019 · 11 comments · Fixed by #2480
Closed

Option for omitting zero enum value from generated Swagger #888

jgiles opened this issue Mar 2, 2019 · 11 comments · Fixed by #2480

Comments

@jgiles
Copy link
Contributor

jgiles commented Mar 2, 2019

It's pretty typical practice in proto APIs for enum types with no natural default to declare the 0 value as UNKNOWN (or similar). Often UNKNOWN is not a valid option, but is declared to differentiate the "unset" case.

JSON semantics are different: The "unset" case is explicit, because the field is not present in the map object.

Currently, the generated Swagger spec exposes such an "UNKNOWN" option as a legitimate choice, and records it as the default choice. For cases where "UNKNOWN" is not a legitimate choice and should not be recorded as such in API docs/generated client code.

Bug reports:

Fill in the following sections with explanations of what's gone wrong.

Steps you follow to reproduce the error:

Example input:

enum Type {
    UNKNOWN = 0;
    FOO = 1;
    BAR = 2;
}

Current Swagger type definition output:

    "Type": {
      "type": "string",
      "enum": [
        "UNKNOWN",
        "FOO",
        "BAR"
      ],
      "default": "UNKNOWN"
    }

What did you expect to happen instead:

An annotation or other option to emit:

    "Type": {
      "type": "string",
      "enum": [
        "FOO",
        "BAR"
      ]
    }

What's your theory on why it isn't working:

Current behavior is the safe default.

An option to adjust this would most likely take the form of a new EnumOption annotation, with boolean field omit_default or similar to control this behavior: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/options/annotations.proto

@johanbrandhorst
Copy link
Collaborator

Thanks for the detailed feature request. I think this sounds okay, but you're sure you want it as an annotation rather than a generator option to remove all the zero enums in a document?

@jgiles
Copy link
Contributor Author

jgiles commented Mar 2, 2019

Well, I think all of the enums in my proto files would benefit from this behavior, but there could be enums where there is a natural default (or the zero value is otherwise valid/intentional), and you could imagine some project having a mix of those two cases.
But perhaps a flag would be sufficient for the vast majority of cases - if you control the enum definitions, you can structure them consistently.

@johanbrandhorst
Copy link
Collaborator

Considering you're the first to ask for this, if it's good enough for you it's probably good enough for an MVP :). Do you intend to submit a PR to cover this behaviour change?

@jgiles
Copy link
Contributor Author

jgiles commented Mar 3, 2019

If a PR for adding the generator command-line flag would be well-received, I can try to take a stab at it at some point :-)

@achew22
Copy link
Collaborator

achew22 commented Mar 3, 2019

I assure you it would be well received 😁

@gpopovic
Copy link

gpopovic commented Oct 1, 2019

Any news on this?

@meigy-nefeli
Copy link

We would love to have this capability too. Just thought I'll chime in a +1 vote. Any plans on doing this anytime soon?

@johanbrandhorst
Copy link
Collaborator

I don't know of anyone working actively on this at this time. Would you like to contribute this feature?

@hiroyoshii
Copy link
Contributor

Hi, I'd like to work on this if possible.

@johanbrandhorst
Copy link
Collaborator

Sure, do you need anymore information than what is covered in the issue already? A command line flag would mean adding an entry here:

var (
importPrefix = flag.String("import_prefix", "", "prefix to be added to go package paths for imported proto files")
file = flag.String("file", "-", "where to load data from")
allowDeleteBody = flag.Bool("allow_delete_body", false, "unless set, HTTP DELETE methods may not have a body")
grpcAPIConfiguration = flag.String("grpc_api_configuration", "", "path to file which describes the gRPC API Configuration in YAML format")
allowMerge = flag.Bool("allow_merge", false, "if set, generation one OpenAPI file out of multiple protos")
mergeFileName = flag.String("merge_file_name", "apidocs", "target OpenAPI file name prefix after merge")
useJSONNamesForFields = flag.Bool("json_names_for_fields", true, "if disabled, the original proto name will be used for generating OpenAPI definitions")
repeatedPathParamSeparator = flag.String("repeated_path_param_separator", "csv", "configures how repeated fields should be split. Allowed values are `csv`, `pipes`, `ssv` and `tsv`")
versionFlag = flag.Bool("version", false, "print the current version")
allowRepeatedFieldsInBody = flag.Bool("allow_repeated_fields_in_body", false, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option")
includePackageInTags = flag.Bool("include_package_in_tags", false, "if unset, the gRPC service name is added to the `Tags` field of each operation. If set and the `package` directive is shown in the proto file, the package name will be prepended to the service name")
useFQNForOpenAPIName = flag.Bool("fqn_for_openapi_name", false, "if set, the object's OpenAPI names will use the fully qualified names from the proto definition (ie my.package.MyMessage.MyInnerMessage). DEPRECATED: prefer `openapi_naming_strategy=fqn`")
openAPINamingStrategy = flag.String("openapi_naming_strategy", "", "use the given OpenAPI naming strategy. Allowed values are `legacy`, `fqn`, `simple`. If unset, either `legacy` or `fqn` are selected, depending on the value of the `fqn_for_openapi_name` flag")
useGoTemplate = flag.Bool("use_go_templates", false, "if set, you can use Go templates in protofile comments")
disableDefaultErrors = flag.Bool("disable_default_errors", false, "if set, disables generation of default errors. This is useful if you have defined custom error handling")
enumsAsInts = flag.Bool("enums_as_ints", false, "whether to render enum values as integers, as opposed to string values")
simpleOperationIDs = flag.Bool("simple_operation_ids", false, "whether to remove the service prefix in the operationID generation. Can introduce duplicate operationIDs, use with caution.")
proto3OptionalNullable = flag.Bool("proto3_optional_nullable", false, "whether Proto3 Optional fields should be marked as x-nullable")
openAPIConfiguration = flag.String("openapi_configuration", "", "path to file which describes the OpenAPI Configuration in YAML format")
generateUnboundMethods = flag.Bool("generate_unbound_methods", false, "generate swagger metadata even for RPC methods that have no HttpRule annotation")
recursiveDepth = flag.Int("recursive-depth", 1000, "maximum recursion count allowed for a field type")
)
and pulling it into the logic that renders the enum values. Let me know if you need more help!

@hiroyoshii
Copy link
Contributor

Thanks @johanbrandhorst.
I have created a PR: #2480. Let me know if I missed something or need to change anything.

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

Successfully merging a pull request may close this issue.

6 participants