dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.37k stars 9.99k forks source link

DefaultApiConventions are missing Status400BadRequest responses for GET requests #9375

Open cremor opened 5 years ago

cremor commented 5 years ago

Describe the bug

The DefaultApiConventions should define a Status400BadRequest response for Get and Find methods.

To Reproduce

Steps to reproduce the behavior:

  1. Using this version of ASP.NET Core '2.2'
  2. Create a Web API project
  3. Apply the [ApiConventionType(typeof(DefaultApiConventions))] attribute to the controller or assembly.
  4. Analyse the structure returned by ApiExplorer interfaces (or add Swashbuckle.AspNetCore to quickly see the same data in Swagger-UI).
  5. Check the possible responses of api/values/{id} methods. Note that Status400BadRequest is not defined as a possible response.
  6. Call the api/values/{id} with a string as id. Note the response ("400 BadRequest").

Expected behavior

Becaues GET can return "400 BadRequest", there should be a Status400BadRequest response defined for GET methods, just like there is for all POST, PUT and DELETE.

Additional context

I know that these conventions were created after the default scaffolding template "Web API Controller with EF Core context", which doesn't return BadRequest in Get. But neither does Delete, which accepts the same parameter as Get. And thanks to automatic HTTP 400 responses both Get and Delete will return "400 BadRequest" if model binding fails (e.g. if you call the action with a string parameter but your id is an integer).

Output of dotnet --info:

Click to expand .NET Core SDK (gemäß "global.json"): Version: 2.2.106 Commit: aa79b139a8 Laufzeitumgebung: OS Name: Windows OS Version: 10.0.16299 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\2.2.106\ Host (useful for support): Version: 2.2.4 Commit: f95848e524 .NET Core SDKs installed: 2.1.505 [C:\Program Files\dotnet\sdk] 2.2.106 [C:\Program Files\dotnet\sdk] .NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] To install additional .NET Core runtimes or SDKs: https://aka.ms/dotnet-download
mkArtakMSFT commented 5 years ago

Thanks for contacting us, @cremor. @glennc what needs to be done here?

cremor commented 4 years ago

Since #8855 is now both closed and locked, please don't forget it when working on this issue. As per comment from @pranavkm:

The most likely solution we would go with would be to create a separate convention and have users explicitly opt in. If you'd like we could use #9375 to consider fixing both of these using this approach.

mkArtakMSFT commented 4 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.