RicoSuter / NSwag

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

BindRequired vs. Required #2710

Open Leon99 opened 4 years ago

Leon99 commented 4 years ago

While migrating from Swashbuckle, we noticed that [Required] no longer works. According to https://github.com/RicoSuter/NSwag/wiki/AspNetCoreOpenApiDocumentGenerator, [BindRequired] should be used instead. However, starting from ASP.NET Core 3, referencing BindRequiredAttribute requires using Microsoft.NET.Sdk.Web, which can be undesirable in some cases (e.g. when the type is located in a library that is reused in a non-ASP.NET project). Contrary to that, RequiredAttribute lives in System.ComponentModel.DataAnnotations, which is much more lightweight and not ASP.NET-specific.

Is there a reason not to use [Required] for marking required members?

Leon99 commented 4 years ago

Also, according to https://docs.microsoft.com/en-us/aspnet/core/mvc/models/model-binding?view=aspnetcore-3.1, [BindRequired]

Can only be applied to model properties, not to method parameters

which is misleading, but they don't seem to care

RicoSuter commented 4 years ago

[Required] (or the Required field in the JsonPropertyAttribute) is still used for regular JSON serialized objects (eg POST body or responses). However, aspnet core primitive params or complex [FromQuery] properrties which are required must be marked with [BindRequired] as they are not JSON serialized but handled/modelbound by asp