dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.12k stars 4.7k forks source link

System.Net.HttpClient.DeleteAsync needs to also accept content parameter like the rest of the methods. #41473

Open akshays2112 opened 4 years ago

akshays2112 commented 4 years ago

Problem:

In Spotify API endpoint there is a scenario in which you are issuing a Delete command but the data cannot be in the Query String but has to be in the Body/Payload.

Suggest this Solution

The solution is for System.Net.HttpClient.DeletAsync to be overloaded to accept a content parameter like the rest of the methods.

ghost commented 4 years ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

karelz commented 4 years ago

@akshays2112 did you try to use a workaround: SendAsync with DELETE and Content? Would that work? I wonder how common pattern this is. If not too much, then a workaround might be enough maybe?

ManickaP commented 4 years ago

IMHO sending a content in DELETE won't be a very common scenario. Also, other methods which are not expected to carry a payload do not accept HttpContent parameter, e.g.: GetAsync, GetByteArrayAsync and other Get helpers.

akshays2112 commented 4 years ago

I have not tried SendAsync. Will look into trying to use it for a workaround and get back to you. Well, I do not want to comment on how common. All I can tell you is contributing to completing SpotifyApi.NetCore to complete it I ran into this problem. Only 4 endpoints to cover so I thought I would try and ask. Thanks again for the swift response.

akshays2112 commented 4 years ago

SendAsync does not seem to support HttpContent as a parameter either. :( Any other suggestions for a workaround.

ManickaP commented 4 years ago

HttpContent is part of HttpRequestMessage (which is passed to SendAsync): https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs#L61-L61

akshays2112 commented 4 years ago

Awesome!!! Link to code is really what I was going to ask for next. Will look at the code.

Cool so did not see it on Itellisense dropdown in VS2019. Ok going to try this next.

Remembered why I actually missed it. Was looking for it to be in the constructor as a parameter like the rest of the methods. Was used to that pattern.

akshays2112 commented 4 years ago

Need more time. Got involved in something interesting I started on Twitter and buried in a lot of housework. Will work on it soon.

scalablecory commented 4 years ago

I agree this seems rare. I don't know if we need a new API for it, as the current SendAsync should work for advanced cases. The RFC has a note indicating that although this is not disallowed, some implementations have issues with it:

   A payload within a DELETE request message has no defined semantics;
   sending a payload body on a DELETE request might cause some existing
   implementations to reject the request.
akshays2112 commented 4 years ago

@scalablecory makes a lot of sense. Even in all the endpoints I did I never ran into this until the very end.

akshays2112 commented 4 years ago

All I would like is maybe SendAsync can have an overload that will accept ContentType as a constructor parameter like the regular pattern.

I agree since SendAsync works just fine like @scalablecory said it is up to you whether you want to commoditize some of these uncommon cases and make it super simple for even advanced coders.

I am happy to see @karelz has already added this to v6 milestone.

For me, the beauty of sticking to Microsoft platform for coding, though I can easily code the same thing on UNIX/LINUX always where I started, is the ease of use was Microsoft Technologies/Platforms biggest USP (Unique Selling Point).

AraHaan commented 3 years ago

I think there should also be an DeleteAsJson for when you want the Delete request to have an json serialized body of an object that contains values it should send. I am currently getting bit by this issue myself too.

Because of this I had to add my own:

public static class HttpClientJsonExtensions
{
    public static async Task<HttpResponseMessage> DeleteAsJsonAsync<TValue>(this HttpClient client, string requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default)
    {
        var json = JsonSerializer.Serialize(value, options);
        using var request = new HttpRequestMessage(HttpMethod.Delete, requestUri)
        {
            Content = new StringContent(json),
        };
        return await client.SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false);
    }
}
patheikkinen commented 3 months ago

Teltonika's router configuration API uses DELETE in a manner that would make developers benefit from this feature.