Skip to content

Conversation

@jimschubert
Copy link
Contributor

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

NOTE: This is a continuation of #6851. opened separately to facilitate discussion of whether this is the best approach, and to verify that I haven't missed anything.

The EmitDefaultValue=false for string based enums will prevent the first
enum value from being serialized, because as 0 it is considered the
default.

This commit assigns an explicit numerical value to all non-integer
enums. This assignment has no effect on the
serialization/deserialization values, and only assigns the compiled
integer.

NOTE: This will have an effect of requiring recompilation of any code
that references the client/server models. This is because:

public enum Pet { Available }

Source files referencing Pet.Available as defined above will have a
constant 0 in place of the enum value.

public enum Pet { Available = 1 }

Source files referencing Pet.Available as defined above will have a
constant 1 in place of the enum value.

After compilation, Pet.Available in both instances lose their semantic
values and refer to the byte representation of their integral values.

For more info, see
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/enum

/cc @mandrean
/cc @wing328 (this change is a compile-time breaking change, see above)

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.
The EmitDefaultValue=false for string based enums will prevent the first
enum value from being serialized, because as 0 it is considered the
default.

This commit assigns an explicit numerical value to all non-integer
enums. This assignment has no effect on the
serialization/deserialization values, and only assigns the compiled
integer.

NOTE: This will have an effect of requiring recompilation of any code
that references the client/server models. This is because:

    public enum Pet { Available }

Source files referencing Pet.Available as defined above will have a
constant 0 in place of the enum value.

    public enum Pet { Available = 1 }

Source files referencing Pet.Available as defined above will have a
constant 1 in place of the enum value.

After compilation, Pet.Available in both instances lose their semantic
values and refer to the byte representation of their integral values.

For more info, see
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/enum
@jimschubert
Copy link
Contributor Author

This PR resolves the issue present in the bug…

Test case:

swagger: "2.0"
info:
  description: "This is a sample server Petstore server.  You can find out more about     Swagger at [http://swagger.io](http://swagger.io) or on [irc.freenode.net, #swagger](http://swagger.io/irc/).      For this sample, you can use the api key `special-key` to test the authorization     filters."
  version: "1.0.0"
  title: "Swagger Petstore"
  termsOfService: "http://swagger.io/terms/"
  contact:
    email: "apiteam@swagger.io"
  license:
    name: "Apache 2.0"
    url: "http://www.apache.org/licenses/LICENSE-2.0.html"
host: "petstore.swagger.io"
basePath: "/v2"
tags:
- name: "pet"
  description: "Everything about your Pets"
  externalDocs:
    description: "Find out more"
    url: "http://swagger.io"
- name: "store"
  description: "Access to Petstore orders"
- name: "user"
  description: "Operations about user"
  externalDocs:
    description: "Find out more about our store"
    url: "http://swagger.io"
schemes:
- "http"
paths:
  /pet:
    post:
      tags:
      - "pet"
      summary: "Add a new pet to the store"
      description: ""
      operationId: "addPet"
      consumes:
      - "application/json"
      - "application/xml"
      produces:
      - "application/xml"
      - "application/json"
      parameters:
      - in: "body"
        name: "body"
        description: "Pet object that needs to be added to the store"
        required: true
        schema:
          $ref: "#/definitions/Pet"
      responses:
        405:
          description: "Invalid input"
definitions:
  PetStatus:
    type: "string"
    description: "pet status in the store"
    enum:
    - "available"
    - "pending"
    - "sold"
  Pet:
    type: "object"
    required:
    - "name"
    - "status"
    properties:
      name:
        type: "string"
        example: "doggie"
      status:
        $ref: "#/definitions/PetStatus"

CLI options:

generate -l csharp -o out -i swagger.yaml

Evaluate:

$ csharp -r:bin/IO.Swagger.dll -r:bin/Newtonsoft.Json.dll -r:bin/RestSharp.dll
Mono C# Shell, type "help;" for help

Enter statements below.
csharp> using IO.Swagger.Model;
csharp> var pet = new Pet("Kitty McKittyface", PetStatus.Available).ToJson();
csharp> pet
"{
  "status": "available",
  "name": "Kitty McKittyface"
}"

@wing328 wing328 added this to the v2.3.0 milestone Nov 9, 2017
@wing328 wing328 closed this Nov 13, 2017
@wing328 wing328 reopened this Nov 13, 2017
@wing328 wing328 merged commit 28e2fce into swagger-api:master Nov 13, 2017
@jimschubert jimschubert deleted the csharp-enum-serialization-6459 branch November 13, 2017 12:18
jimschubert added a commit to jimschubert/swagger-codegen that referenced this pull request Nov 14, 2017
* 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)
  ...
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.

2 participants