RicoSuter / NSwag

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

Fix: Optional parameters wrongly detected as required in API Explorer after updating from v11.20.1 to v12.0.0 #1759

Open OculiViridi opened 5 years ago

OculiViridi commented 5 years ago

After updating NSwag from v11.20.1 to 12.0.0, optional method's parameters are now detected as required.

This is the same code for both versions:

/// <summary>
/// Find anomalies by specified parameters
/// </summary>
/// <param name="machineId">ID of the machine that need to be considered for filter</param>
/// <param name="type">AnomalyType value that need to be considered for filter</param>
/// <param name="status">AnomalyStatus value that need to be considered for filter</param>
/// <remarks>Values of filter are considered in AND</remarks>
/// <response code="200">Successful operation</response>
/// <response code="400">Invalid filter values</response>
[HttpGet]
[ProducesResponseType(typeof(List<api.Anomaly>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.BadRequest)]
public async Task<ActionResult<List<api.Anomaly>>> List(long? machineId, AnomalyType? type, AnomalyStatus? status, CancellationToken cancellationToken)
{
    return Mapper.Map<List<api.Anomaly>>(await DcsMediator.Send(
        new GetAnomalyListByFilterQuery() { MachineId = machineId, Status = status, Type = type, IncludeMachine = true }, cancellationToken));
}

This is what I get with v11.20.1 (correct)

image

This is what I get now with v12.0.0 (wrong)

image

RicoSuter commented 5 years ago

Did you switch from WebApiToSwaggerGenerator to AspNetCoreToSwaggerGenerator?

RicoSuter commented 5 years ago

I think the option RequireParametersWithoutDefault is enabled and should be disabled to use ASP.NET Core's API explorer reported required info only...

OculiViridi commented 5 years ago

I think the option RequireParametersWithoutDefault is enabled and should be disabled to use ASP.NET Core's API explorer reported required info only...

@RSuter Where this parameter is located? I've no option currently set on my code about it.

RicoSuter commented 5 years ago

AddSwaggerDocument(settings => settings.RequireParametersWithoutDefault ...)

Questions:

OculiViridi commented 5 years ago
  • What .net core version are you using?

.NET Core 2.1 (2.1.6)

  • Is compatibility set to 2.1 in startup.cs?

Yes, .SetCompatibilityVersion(CompatibilityVersion.Version_2_1)

  • Is this openapi3 or swagger2?

Swagger 2. I'm using those methods:

@RSuter Just to be more clear I'm pointing out what I exactly need from NSwag:

So, I need to put together all the necessary configuration... I'm now trying to convert the old (v11.20.1) configuration into the new (v12.0.0) one, but it's not so straightforward... Maybe I can open a new Issue or post all the code here?

RicoSuter commented 5 years ago

Nginx/reverse proxy users, please review: #2196

BramMusters commented 5 years ago

I have a similar issue using .NET Core 2.2 and NSwag 12.3.0:

In Startup, I call AddSwaggerDocument(), (but also tried AddOpenAPIDocument()). My CompatibilityVersion is set to 2.2, but setting it to 2.1 doesn't help either.

In my controller, I have the following code: [HttpGet("{id?}")] public async Task<ActionResult<TEntityDto>> GetUpdateModel(int? id) The generated Swagger.json defines the id parameter as required, so obviously Swagger UI and the generated code think the parameter is required.

RicoSuter commented 5 years ago

With GetUpdateModel(int? id) I think the parameter is required but nullable...

I think you need to use GetUpdateModel(int? id = null) so that it is reported as optional by the ASP.NET Core api explorer.