Skip to content

[csharp] Clean up ref/inner enum structure #6887

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

Merged
merged 8 commits into from
Jan 7, 2018

Conversation

jimschubert
Copy link
Contributor

@jimschubert jimschubert commented Nov 4, 2017

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: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

Description of the PR

This is a continuation of #6851, filed as a separate PR so we can have discussion of whether this is an appropriate workaround without blocking #6851. If this gets merged, it'll likely cause conflicts with #6873 (another continuation PR of #6851 for community discussion).

Enums defines as ref models have a different object structure
(CodegenModel) than those defined as inner enums (CodegenProperty). To
make these look as similar as possible, we walk all ref model enums and
reassign enumVars with the same properties inherited from the top level
CodegenProperty so CodegenModel enums can use the same templates as
inner enums

This PR is an attempt to resolve a question/comment in #5352 by @larssb.

/cc @mandrean

C# works differently from most languages in that enums are not
considered objects. This means default(EnumType) will choose a default
of the first enum option. This isn't desirable because it breaks the
required = false functionality of swagger specs, which defines a
property which isn't required to exist in the message body.

Rather than force consumers to use enum values such as UNSPECIFIED, UNKNOWN,
NOT_SET, etc... we can treat enums as primitives. This means any
non-required enum will become Nullable<EnumType> regardless of whether
it is defined as an inline enum or a referenced enum model.
Enums defines as ref models have a different object structure
(CodegenModel) than those defined as inner enums (CodegenProperty). To
make these look as similar as possible, we walk all ref model enums and
reassign enumVars with the same properties inherited from the top level
CodegenProperty so CodegenModel enums can use the same templates as
inner enums.
@jimschubert
Copy link
Contributor Author

Test case against spec at:

Generate with:

generate -l csharp -o out -i https://gist.githubusercontent.com/larssb/d00e4cf200d85423152df75703d84076/raw/17bb6550e1249b19ebfb215f87cf4132caab6149/octopusdeployOpenApi.json

Then, compile:

cd out && sh build.sh

Evaluate:

$ csharp -r:bin/IO.Swagger.dll -r:bin/Newtonsoft.Json.dll -r:bin/RestSharp.dll

image

@jimschubert jimschubert changed the title [csharp][wip] Clean up ref/inner enum structure [csharp] Clean up ref/inner enum structure Nov 9, 2017
@jimschubert
Copy link
Contributor Author

I've removed the wip on this, as I revisited and it seems to be a pretty good approach. I'll need to resolve merge conflicts in the next day or so.

* master: (101 commits)
  [Swift4] Allow for custom dateformatter to be used (swagger-api#6672)
  [haskell-http-client] fix bug when generating models-only (swagger-api#6931)
  fix typo: crediential => credential
  minor typo fix
  [csharp] fix enum serialization of first value (swagger-api#6873)
  [PHP] Improve docs and README (swagger-api#6935)
  Binary mode for file deserialization in python (swagger-api#6936)
  add python tornado test to travis
  [Python/tornado] add integration tests and fix bugs (swagger-api#6925)
  Fix PHP passes response body to ApiException (swagger-api#6923)
  [TypeScript][Node] Resolve TS2532 error (swagger-api#6932)
  skip "all" shell script
  minor formatting change
  Fixes Issue swagger-api#6841, Map for accessing additionalProperties is generated. (swagger-api#6886)
  add tsloughter as owner erlang
  WIP: initial commit for Erlang client generator (swagger-api#6502)
  add back php client test
  Switch Travis image from MacOS to Linux (swagger-api#6937)
  add link to ebook
  [Scala] Default case class Option types to None for non-required fields (swagger-api#6790)
  ...
@jimschubert
Copy link
Contributor Author

I haven't regenerated samples in this branch because there are multiple issues that I fixed in #6899.

@wing328 wing328 added this to the v2.4.0 milestone Jan 7, 2018
@wing328 wing328 changed the base branch from master to 2.4.0 January 7, 2018 04:09
@wing328 wing328 merged commit 64fafb5 into swagger-api:2.4.0 Jan 7, 2018
@jimschubert jimschubert deleted the csharp-enum-cleanup branch February 5, 2018 03:32
@copperorange
Copy link

Hi,

I just tried C# Enums again after having trouble before. I am experiencing the same issue as #5352 specifically https://github.com/swagger-api/swagger-codegen/issues/5352#issuecomment-335444175 where generating netcore code from editor.swagger.io with the following schema

# Enums StateHintEnum: type: string enum: - insert - update - delete

is generating

/// <summary> /// Gets or Sets StateHintEnum /// </summary> [JsonConverter(typeof(Newtonsoft.Json.Converters.StringEnumConverter))] public enum StateHintEnum { /// <summary> /// Enum INSERT for "insert" /// </summary> [EnumMember(Value = ""insert"")] INSERT = 0, /// <summary> /// Enum UPDATE for "update" /// </summary> [EnumMember(Value = ""update"")] UPDATE = 1, /// <summary> /// Enum DELETE for "delete" /// </summary> [EnumMember(Value = ""delete"")] DELETE = 2 }

Each of the values has a [double quote] "" on either side of the value which fails at compile time.

Please advise if there is any way to correctly format the codegen.

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.

3 participants