OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
855 stars 473 forks source link

Routing not integrating with dotnet core async controller methods #1613

Open NetTecture opened 6 years ago

NetTecture commented 6 years ago

7.0.1 RTM vs. dotnet core sdk 402.

A post method in an Odata Controller with the signature:

public IActionResult Post([FromBody] Api.Odata.Account account) {

is found and routed to. The same as async:

public async Task PostAsync([FromBody] Api.Odata.Account account) {

is not (Not found, 404). Note the Async name suffix, which is dotnet coding standard compliant.

https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/

Naming convention By convention, you append "Async" to the names of methods that have an async modifier.

It seems integrated automated routes are not written with the .net naming conventions under consideration. It works if I change the name back to Post:

public async Task<IActionResult> Post([FromBody] Api.Odata.Account account) {

But then the method is not naming convention compliant.

biaol-odata commented 6 years ago

Currently we are following naming conventions for ASP.NET for identifying HTTP methods. It will be an added feature to also recognize methods suffixed with "Async" per dotnet coding conventions.

NetTecture commented 6 years ago

@biaol-odata Actually you are NOT. Stop. Pretending. I have QUOTED the relevant part of the naming convention. It does not say "the suffix is optional" and if you use a standard parser it markes thsoe method as having the wrong name. It is ok for you not to support them - but please, stop pretending the rest of us are idiots that can not read the languiage spec. You do NOT follow naing conventions for ASP.NET.

biaol-odata commented 6 years ago

@NetTecture Sorry that you feel like this. That is not our intention at all. The OData team looked at this issue in our team meeting yesterday and the note captures our thoughts. There might be some confusion between the "ASP.NET" naming convention and "dotnet" naming convention mentioned in above messages. The "ASP.NET" naming convention is what current WebAPI-OData follows, specially using controller methods such as "Post" matching to HTTP POST; and the one you quoted is for the "dotnet" convention, and we understand suffix of "Async" is a typical naming pattern in async method call such as "MyFuncAsync()" being async-flavor of "MyFunc()". We can make further clarification as needed. Thanks for your consistent support of WebAPI-OData.

NetTecture commented 6 years ago

Well, according to the dotnet and the asp.net team there are no asp.net naming conventions. The asp.net roslyn analyzer nicely demands the Async suffix for async methods.

LarsKemmann commented 5 years ago

I see a "P4" label on this issue and I'm assuming that means something like "quite low on our priority list." While I can understand that (begrudgingly :)), can you at least (please) call out this issue prominently in the documentation? It cost me over an hour of troubleshooting this morning before finally figuring out that there might be a reason all of the announcement blog posts don't show any kind of async code. For those of us used to the modern conventions introduced in .NET Core, this is jarring. Thanks for your consideration!