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

[FromUri] and complex types not prefixed correctly. #1200

Open yvdh opened 6 years ago

yvdh commented 6 years ago

Hello,

when creating a swagger from a http GET method that uses several [FromUri] complex types the argument names are not prefixed correctly.

E.g. I have a complex type Period: Public Class Period Public Property Start As DateTime = DateTime.MinValue Public Property [End] As DateTime = DateTime.MaxValue End Class

And a controller method public IHttpActionResult GetAppointments(int? agendaId = null, int? departmentId = null, [FromUri]ExternalId externalId = ExternalId.Empty, [FromUri]Period startTimeSearchPeriod = null, [FromUri]Period endTimeSearchPeriod = null, int? ticketNumber = null, ConditionFlags? conditions = null, bool includeReferences = false) { return RunUnitOfWork(new GetAppointments(this.Db, agendaId, departmentId, externalId, startTimeSearchPeriod, endTimeSearchPeriod, ticketNumber, conditions, includeReferences)); }

This should result in a swagger (SwashBuckle) with arguments { name: "searchPeriod.start", in: "query", required: false, type: "string", format: "date-time" }, { name: "searchPeriod.end", in: "query", required: false, type: "string", format: "date-time" },

However, NSWAG gives: { "type": "string", "name": "Start", "in": "query", "x-nullable": false, "description": "The start of the period. Minimum time as default", "format": "date-time" }, { "type": "string", "name": "End", "in": "query", "x-nullable": false, "description": "The end of the period. Maximum time as default", "format": "date-time" },

This means that you get duplicate arguments if there is more than one Period as an argument in the generated C# client.

Of course there is a workaround by using an encompassing model, but then again, this should work as such. Maybe I am doing something wrong? I am using NSWAGStudio.

Cheers Yves

RicoSuter commented 6 years ago

Should be simple to fix. The only issue i see is that this is a breaking change for some users...

yvdh commented 6 years ago

I understand how some people do not want the complex type prefix (especially in the swagger UI). It is, as far as I can tell however, standard to include it in the swagger, and this should be subservient to a nice UI.

Btw: great work, keep it up!

RicoSuter commented 6 years ago

How does web api handle this? Are both variants working with the parameter binder?

yvdh commented 6 years ago

Hi, I am not sure I understand the question. I use NSWAG to generate my DTO from my Models, then use those in my web api (controllers). The webapi layer has no idea about any of the models, that is the responsability of the service layer with it's units of work. I then use NSWAG again to generate the C# client code that is used by any client application, together with the DTO assemblies generated before. Obviously this works because for me my Models are one-on-one with my DTO's.

The odd thing is that NSWAG seemed to have solved the problem of complex type prefixes a while ago, but it seems broken again with the latest version. The prefix seems to be 'query' now ....

Worse, when including more than 2 complex types, the third type just disappears from the parameters list in the generated C# client code ...

Yves

RicoSuter commented 6 years ago

My question is how Web API handles these parameters, can you use for your sample either "searchPeriod.start" or "start" as parameter and ASP.NET still binds it correctly?

yvdh commented 6 years ago

I only use the generated clients in my applications. However, when using Swashbuckle (which uses searchPeriod.start), and breaking inside my controller I can confirm proper binding by .net!

image

image