RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.77k stars 1.29k forks source link

Add support for explicit inclusion of binding attributes in C# Controller generation #1440

Open goldsam opened 6 years ago

goldsam commented 6 years ago

My swagger file uses query parameters for string list types, but the generated code does not include the necessary [FromUri] attribute to ensure query parameter bindings are used. Instead, the default WebAPI rules for parameter binding assume content/body bindings. This forces me to add this attributes in my deriving classes which is redundant considering I already specified a query parameter binding in my swagger file.

RicoSuter commented 6 years ago

Currently the controller generation has some flaws which have to be improved (e.g. not working with ASP.NET Core, etc.). Because I personally don't use these in professional project, this is not on my prio list...

goldsam commented 6 years ago

Do you have enough time that if I submitted a pull request you could give it some attention?

RicoSuter commented 6 years ago

Sure..

JensDel commented 5 years ago

I am currently working on a swagger file which also contains a list of Enum (string), so is there an update on this matter? Thanks anyway for the already awesome code generator.

gillesdb commented 5 years ago

In OAS 3.0 you can have 4 different parameter types:

  1. path parameters, such as /users/{id}
  2. query parameters, such as /users?role=admin
  3. header parameters, such as X-MyHeader: Value
  4. cookie parameters, which are passed in the Cookie header, such as Cookie: debug=0; csrftoken=BUSe35dohU3O1MZvDCU

For this issue, we are only interested in the first 2. (3 is supported via #2033 , 4 not supported.)

1 & 2 are supported by NSwag but when you specify a complex type, webAPI assumes that it will be passed via the body of the request. How to fix: -> ASP.NET: add [FromUri] before each parameter. -> ASP.NET Core: you have the possibility to work more granular: [FromQuery], [FromRoute]

So a possible fix could be something like: {% if parameter.IsBody %}[{{ AspNetNamespace }}.FromBody] {% elseif parameter.IsPath %}[{{ AspNetNamespace }}.{% if IsAspNetCore -%}FromRoute{% if parameter.IsValidIdentifier == false %}(Name = "{{ parameter.Name }}"){% endif %}{% else %}FromUri{% endif -%}] {% elseif parameter.IsQuery %}[{{ AspNetNamespace }}.{% if IsAspNetCore -%}FromQuery{% if parameter.IsValidIdentifier == false %}(Name = "{{ parameter.Name }}"){% endif %}{% else %}FromUri{% endif -%}]

What do you think of this possible solution, could this work? Or am I missing something? (I would like to have this validated before I do a PR.)

RicoSuter commented 5 years ago

but when you specify a complex type

I think OpenAPI 3/Swagger does not support complex path or query parameters.. can you post a sample?

gillesdb commented 4 years ago

Query parameters can be primitive values, arrays and objects. https://stackoverflow.com/questions/38187187/use-object-type-query-param-in-swagger-documentation

gillesdb commented 4 years ago

Example arrays: /pets?petId=1&petId=2

openapi: 3.0.0
info:
  version: 1.0.0
  title: Pet API
paths:
  /pets:
    get:
      parameters:
        - in: query
          name: petId
          description: Retrieve multiple pets
          required: false
          schema:
            type: array
            items:
              type: string

Example objects: /pets?pet_id=1&name=bas

openapi: 3.0.0
info:
  version: 1.0.0
  title: Pet API
paths:
  /pets:
    get:
      parameters:
        - in: query
          name: petId
          description: Retrieve multiple pets
          required: false
          schema:
            type: object
            properties:
              pet_id: 
                type: integer
              name:
                type: string
          style: form
          explode: true