[csharp] Treat enum models consistently #6851
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
I'll rebuild samples after others have reviewed the changes, to limit noise. |
jimschubert
force-pushed
the
csharp-refmodel-enums
branch
from
fdd60c7 to
72b9ab6
Compare
|
See linked issue, reporter has verified this branch against about 400 models with no additional concerns. |
This was referenced Nov 2, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 regardless of whether
it is defined as an inline enum or a referenced enum model.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif 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\.3.0.0branch for changes related to OpenAPI spec 3.0. Default:master.Description of the PR
This allows us to reference enums defined as RefModel using enum logic, rather than object logic. That is,
default(EnumType?)rather thandefault(EnumType).I've made some test-specific modifications to DefaultGenerator, such that integration tests can override the switches we pull from System properties and exclude the swagger codegen metadata (for test maintainability).
See #6690 et al.
/cc @mandrean @wing328