search

swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
17.02k stars 6.03k forks source link

C# - Lists of enums not properly generated #2714

Closed JoshuaWhatley closed 8 years ago

JoshuaWhatley commented 8 years ago

SwashBuckle will generate a swagger json similar to the following for List. The generated client code contains a couple mistakes. 

public enum {
    [EnumMember(Value = "food")]
    Food,

    [EnumMember(Value = "bar")]
    Bar
}

public ResponseContract(GoodEnumEnum? GoodEnum = null, List<listOfEnumsEnum?> ListOfEnums = null)
{
    this.GoodEnum = GoodEnum;
    this.ListOfEnums = ListOfEnums;
}

/// <summary>
/// Some Description
/// </summary>
/// <value>Some Description</value>
[DataMember(Name="listOfEnums", EmitDefaultValue=false)]
public List<string> ListOfEnums { get; set; }

Swagger Doc:

{
    "swagger": "2.0",
    "info": {
        "version": "V1",
        "title": "Title"
    },
    "host": "example.com",
  "schemes": [ "https" ],
  "paths": {
        "/v1.0/example": {
            "post": {
                "tags": ["Tag"],
                "summary": "Summary",
                "operationId": "Some Id",
                "consumes": ["application/json", "text/json", "application/xml", "text/xml", "application/x-www-form-urlencoded"],
                "produces": ["application/json", "text/json", "application/xml", "text/xml"],
                "parameters": [{
                    "name": "request",
                    "in": "body",
                    "description": "request",
                    "required": true,
                    "schema": {
                        "$ref": "#/definitions/ResponseContract"
                    }
                }],
                "responses": {
                    "200": {
                        "description": "Some description",
                        "schema": {
                            "$ref": "#/definitions/ResponseContract"
                        }
                    }
                },
                "deprecated": false
            }
        }
    },
    "definitions": {
        "ResponseContract": {
            "description": "Some Description",
            "type": "object",
            "properties": {
                "goodEnum": {
                    "description": "Some Description",
                    "enum": ["food", "bar"],
                    "type": "string"
                },
                "listOfEnums": {
                    "description": "Some Description",
                    "type": "array",
                    "items": {
                        "enum": ["food", "bar"],
                        "type": "string"
                    },
                    "readOnly": true
                }
            }
        }
    }
}

I have a fix, but it is leveraging a hack written by a previous contributor. I'll submit a pull request so that we can discuss.

wing328 commented 8 years ago

@JoshuaWhatley thanks for reporting the issue. We've a fix here: https://github.com/swagger-api/swagger-codegen/pull/2508

Do you mind performing a fix to see if it works for you?

Also we welcome PR on top of #2508 to make it better.

JoshuaWhatley commented 8 years ago

One remaining issue after applying the fix in #2508 -The generic type in the list is correct in the constructor, but incorrect in the property

Expected

public List<ListOfEnumsEnum> ListOfEnums

Actual

public List<string> ListOfEnums

I'll take a look at it.

wing328 commented 8 years ago

@JoshuaWhatley thanks (I'll do a rebase on the latest master later today)

JoshuaWhatley commented 8 years ago

I believe this issue is present for Dictionaries as well.

Expected

public Dictionary<string, InnerEnum> EnumDictionary

Actual

public Dictionary<string, string> EnumDictionary

Swagger doc:

{
  "swagger": "2.0",
  "info": {
    "version": "V1",
    "title": "Title"
  },
  "host": "example.com",
  "schemes": [ "https" ],
  "paths": {
    "/v1.0/example": {
      "post": {
        "tags": [ "Tag" ],
        "summary": "Summary",
        "operationId": "Some Id",
        "consumes": [ "application/json", "text/json", "application/xml", "text/xml", "application/x-www-form-urlencoded" ],
        "produces": [ "application/json", "text/json", "application/xml", "text/xml" ],
        "parameters": [
          {
            "name": "request",
            "in": "body",
            "description": "request",
            "required": true,
            "schema": {
              "$ref": "#/definitions/ResponseContract"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "Some description",
            "schema": {
              "$ref": "#/definitions/ResponseContract"
            }
          }
        },
        "deprecated": false
      }
    }
  },
  "definitions": {
    "ResponseContract": {
      "description": "Some Description",
      "type": "object",
      "properties": {
        "enumDictionary": {
          "description": "Some Description",
          "type": "object",
          "additionalProperties": {
            "enum": [ "food", "bar" ],
            "type": "string"
          },
          "readOnly": true
        },
        "goodEnum": {
          "description": "Some Description",
          "enum": [ "food", "bar" ],
          "type": "string"
        }
      }
    }
  }
}
wing328 commented 8 years ago

Right, I don't think I've done anything specified to Array or Dictionary yet.

JoshuaWhatley commented 8 years ago

Understood, I thought it should be mentioned. Great work by the way, this is a super cool project.

wing328 commented 8 years ago

@JoshuaWhatley thanks for the positive feedback :)

It's worth mentioning to you that you can also generate ASP.NET5 server stub using Swagger Codegen given the OpenAPI/Swagger spec.

wing328 commented 8 years ago

@JoshuaWhatley I've merged https://github.com/swagger-api/swagger-codegen/pull/2508 into master.

We'll work on enum support for array and dictionary later this week or early next week.

aws-na-ss-usplu commented 8 years ago

Novice: Our very small two cents ;-)

We currently develop solutions in two worlds… LoopBack/Node.js & WebApi/Asp.net

We would love to have a solution that complements each others interpretation of the concepts around

We currently use swagger to generate both client/server code for LoopBack & .NET, and client code in Java and PHP. When it comes to Enum and Additional Properties we have to deal with exceptions because like HTML specs each community is interpreting the specifications a little differently rather than each community discussing the discrepancy to improve the understanding and interpreting of the Swagger Specs.

Here is StrongLooop / Loopback discussion around Enums

Here is how AutoRest community is currently dealing with Enums with AutoRest

We still need to investigate where the StrongLoops/ IBM thoughts on implementing Enum, and we plan to review shortly.

We just propose that all participants in providing a Swagger Code Generators to deal with Enum to converge on a solution rather than have a lot of silo implementations of swagger generic extensions for Enum.

We have investigated the following code generators to date. Non have a consist implementation to deal with Additional Properties or Enum

Appreciate feedback, concerns and issues with our thoughts…

jimschubert commented 8 years ago

I haven't looked at the issue at hand with Enums, Lists, or Dictionaries. @wing328 did you make any progress on this issue?

@aws-na-ss-usplu Since you've put a lot of research into your comment, I wanted to address it somewhat in depth.

Primitive data types in the Swagger Specification are based on the types supported by the JSON-Schema Draft 4. Models are described using the Schema Object which is a subset of JSON Schema Draft 4. Swagger Specification

The Loopback issue you pointed to is more about implementing enum support correctly in the datasource abstraction and emitting the swagger definition correctly against the JSON Schema enum specification (http://json-schema.org/latest/json-schema-validation.html#anchor76).

Your first link to Autorest highlights the main problem. There can be no standard representation across all languages for an enum, because not all languages support enums and those that do don't support them in exactly the same way.

For example, these are all valid enums in C#:

public enum Color { Red, Green, Blue }

public enum ColorDecimal 
{ 
    Red  = 16711680, 
    Green = 65280, 
    Blue = 255
}

[Flags]
public enum Permissions
{
    CREATE        = 1 << 0,
    READ            = 1 << 1,
    UPDATE        = 1 << 2,
    DELETE        = 1 << 3
}

[Flags]
public enum AuthorizationBits : Long
{
    Minimal                    = 1 << 0,
    ReadArticles            = 1 << 15 ,
    RemoveComments  = 1 << 20,
    Moderation               = 1 << 62
}

public enum HttpStatusCode
{
    // Informational 1xx
    Continue = 100,
    SwitchingProtocols = 101,

    // Successful 2xx
    OK = 200,
    Created = 201,
    Accepted = 202,
    NonAuthoritativeInformation = 203,
    NoContent = 204,
    ResetContent = 205,
    PartialContent = 206,
    //…
}

And this is a valid Java enum:

public enum Planet {
    MERCURY (3.303e+23, 2.4397e6),
    VENUS   (4.869e+24, 6.0518e6),
    EARTH   (5.976e+24, 6.37814e6),
    MARS    (6.421e+23, 3.3972e6),
    JUPITER (1.9e+27,   7.1492e7),
    SATURN  (5.688e+26, 6.0268e7),
    URANUS  (8.686e+25, 2.5559e7),
    NEPTUNE (1.024e+26, 2.4746e7);

    private final double mass;   // in kilograms
    private final double radius; // in meters
    Planet(double mass, double radius) {
        this.mass = mass;
        this.radius = radius;
    }
    private double mass() { return mass; }
    private double radius() { return radius; }

    // universal gravitational constant  (m3 kg-1 s-2)
    public static final double G = 6.67300E-11;

    double surfaceGravity() {
        return G * mass / (radius * radius);
    }
    double surfaceWeight(double otherMass) {
        return otherMass * surfaceGravity();
    }
    public static void main(String[] args) {
        if (args.length != 1) {
            System.err.println("Usage: java Planet <earth_weight>");
            System.exit(-1);
        }
        double earthWeight = Double.parseDouble(args[0]);
        double mass = earthWeight/EARTH.surfaceGravity();
        for (Planet p : Planet.values())
           System.out.printf("Your weight on %s is %f%n",
                             p, p.surfaceWeight(mass));
    }
}

To support a universal definition of an enum would be complex. I'll touch on this briefly, since you asked for some feedback.

I'm assuming this complexity is why Microsoft has prefixed their vendor extensions with x-ms-... there's just too many Microsoft concerns related to enums. In my examples above, Color could easily be output to the JSON Schema definition of a string representing the values with at least one value in the array. To support ColorDecimal, you would then have to apply some extension that supports a key/value pair. Then, you can add some extension to indicate to clients that the Permissions enum is a bitmask (a special case of enum).

At this point, most languages will be able to support the enum definition natively, even if it's quite a bit of code.

Now if you try to support AuthorizationBits consistently across languages, you'll start running into issues with some clients. For instance, we know that all numbers in JavaScript are 64-bit floating point numbers. Many people don't know, however, that only the first 52 bits of a JavaScript number contain a value. Then, 11 bits are for holding an exponent, and the last bit is for the sign. You can read more here. If you put a 64 bit value into an enum's value, some clients like JavaScript won't store a 64 bit value. You can work around this in different ways, but they're all hacky. It's obvious from this one example that you can't make all languages happy with a generic definition.

Then, you have an example of a valid enum in Java. Planet is a singleton structure which doesn't just assign one value to one key... it applies a tuple of key/values to one enum. For instance:

Planet:
    MERCURY => ( (mass => 3.303e+23), (radius => 2.4397e6) )

At this point, you'd need to indicate that Planet has complex values which store complex metadata about those values. This is really just an object. It would be better for whatever swagger definition generator used in the Java code to represent this enum as an immutable object rather than an enum.

That last example brings it back around to the Loopback example. Their implementation may have additional metadata applied to the enum. It may result in an object representation rather than an enum.

wing328 commented 8 years ago

UPDATE: For dictionary property/member (enum or not), there's a fundamental issue that I'm working on.

jimschubert commented 8 years ago

Awesome. Thanks for the update.

wing328 commented 8 years ago

@JoshuaWhatley @jimschubert UPDATE: I've filed https://github.com/swagger-api/swagger-codegen/pull/3199 to support array/map (any dimensions) of enum in C#.

JoshuaWhatley commented 8 years ago

That's exciting! I'll take a look. Thanks for looking into this, it is very appreciated!

aws-na-ss-usplu commented 8 years ago

@jimschubert, Understood… The question in not how each language will interpret enumerated types. The question is how JSON Schema represents an enumerated type so Swagger can allow code generators to be more effective and allow interoperability between distributed systems.

The question is not how to get disparities between software implementers to agree on how they should implement enumerated types. If that was possible then world peace is achievable and global warming would not be a discussion. :laughing:

Similar issue existed a long time ago (10+ years ago) when XML/XSD /SOAP/WSDL where the cool kids :sunglasses: on the block. The XML Schema Pattern defined Common Data Structures so that the concept of an enumeration is usable by all code generators. These tools still exist and allow Java solutions to communicate with .NET solutions or Loopback solutions using the enumerated types defined in the schema.

Mea Culpa, it appears that my question was not clear. The concern is will each faction work towards a consensus with usage of JSON, JSON Schema, and Swagger / OPEN API so the code generators interpret an Enum is an Enum regardless of the underling implementation by each interested party.

My intention is to raise a concern so that each group can provide input into shaping the use of JSON so that it is as meaningful as XML yet hopeful without all of the bloat and verboseness. We just had a industry accord on Swagger vs WADL vs RAML vs (the others) since this past winter that OPEN API (Swagger) is leading the pack.

wing328 commented 8 years ago

@JoshuaWhatley @jimschubert PR merged into master.

wing328 commented 8 years ago

@JoshuaWhatley closing this issue for the time being. Let us know if you've any questions.

@aws-na-ss-usplu I would suggest you to open a ticket for OpenAPI spec to start the discussion if you haven't done so: https://github.com/OAI/OpenAPI-Specification/issues/new