Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[API Proposal]: HttpClient.PatchAsJsonAsync() #60531

Closed
Tracked by #63762
ikesnowy opened this issue Oct 18, 2021 · 6 comments · Fixed by #60672
Closed
Tracked by #63762

[API Proposal]: HttpClient.PatchAsJsonAsync() #60531

ikesnowy opened this issue Oct 18, 2021 · 6 comments · Fixed by #60672
Assignees
Labels
api-approved API was approved in API review, it can be implemented api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json Cost:S Work that requires one engineer up to 1 week Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@ikesnowy
Copy link
Contributor

ikesnowy commented Oct 18, 2021

Background and motivation

#45405

It's natural to have PatchAsJsonAsync when we already got PostAsJsonAsync and PutAsJsonAsync.

API Proposal

namespace  System.Net.Http.Json
{
    public static class HttpClientJsonExtensions
    {
        public static Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);

        public static Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);

        public static Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, CancellationToken cancellationToken);

        public static Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, CancellationToken cancellationToken);

        public static Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo, CancellationToken cancellationToken = default);

        public static Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo, CancellationToken cancellationToken = default);
    }
}

API Usage

var client = new HttpClient();
client.DefaultRequestHeaders.Add("Accept", "application/vnd.github.v3+json");
var body = new { Sha = "sha" };
var response = await client.PatchAsJsonAsync("https://api.github.com/repos/octocat/hello-world/git/refs/REF", body);
if (response.IsSuccessfulStatusCode)
{
    Console.WriteLine("Patched!");
}

Alternative Designs

No response

Risks

No response

@ikesnowy ikesnowy added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 18, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels Oct 18, 2021
@ghost
Copy link

ghost commented Oct 18, 2021

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

Issue Details

Background and motivation

#45405

It's natural to have PatchAsJsonAsync when we already got PostAsJsonAsync and PutAsJsonAsync.

API Proposal

namespace  System.Net.Http.Json
{
    public static class HttpClientJsonExtensions
    {
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, CancellationToken cancellationToken = default);
    }
}

API Usage

var client = new HttpClient();
client.DefaultRequestHeaders.Add("Accept", "application/vnd.github.v3+json");
var body = new { Sha = "sha" };
var response = await client.PatchAsJsonAsync("https://api.github.com/repos/octocat/hello-world/git/refs/REF", body);
if (response.IsSuccessfulStatusCode)
{
    Console.WriteLine("Patched!");
}

Alternative Designs

No response

Risks

No response

Author: ikesnowy
Assignees: -
Labels:

api-suggestion, area-System.Net.Http, untriaged

Milestone: -

@ghost
Copy link

ghost commented Oct 18, 2021

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

Issue Details

Background and motivation

#45405

It's natural to have PatchAsJsonAsync when we already got PostAsJsonAsync and PutAsJsonAsync.

API Proposal

namespace  System.Net.Http.Json
{
    public static class HttpClientJsonExtensions
    {
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, CancellationToken cancellationToken = default);
    }
}

API Usage

var client = new HttpClient();
client.DefaultRequestHeaders.Add("Accept", "application/vnd.github.v3+json");
var body = new { Sha = "sha" };
var response = await client.PatchAsJsonAsync("https://api.github.com/repos/octocat/hello-world/git/refs/REF", body);
if (response.IsSuccessfulStatusCode)
{
    Console.WriteLine("Patched!");
}

Alternative Designs

No response

Risks

No response

Author: ikesnowy
Assignees: -
Labels:

api-suggestion, area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Oct 18, 2021
@eiriktsarpalis eiriktsarpalis added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Oct 18, 2021
@eiriktsarpalis
Copy link
Member

@ikesnowy thank you for the API proposal. I've marked it ready for review.

@eiriktsarpalis eiriktsarpalis self-assigned this Oct 18, 2021
@bartonjs
Copy link
Member

bartonjs commented Oct 19, 2021

Video

Looks good as proposed

namespace  System.Net.Http.Json
{
    public static partial class HttpClientJsonExtensions
    {
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, JsonTypeInfo<TValue> jsonTypeInfo, CancellationToken cancellationToken = default);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, string? requestUri, TValue value, CancellationToken cancellationToken);
        public Task<HttpResponseMessage> PatchAsJsonAsync<TValue>(this HttpClient client, Uri? requestUri, TValue value, CancellationToken cancellationToken);
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Oct 19, 2021
@eiriktsarpalis eiriktsarpalis added the help wanted [up-for-grabs] Good issue for external contributors label Oct 19, 2021
@scalablecory scalablecory removed the help wanted [up-for-grabs] Good issue for external contributors label Oct 19, 2021
@scalablecory
Copy link
Contributor

@bartonjs @eiriktsarpalis the defaults in the proposed API create an ambiguity when calling as client.PatchAsJsonAsync(uri, value).

Removing up-for-grabs, please add back when resolved.

@ikesnowy
Copy link
Contributor Author

@scalablecory

Sorry, I updated the proposal, should be ok now

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Oct 20, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Oct 25, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 24, 2021
@eiriktsarpalis eiriktsarpalis added User Story A single user-facing feature. Can be grouped under an epic. Cost:S Work that requires one engineer up to 1 week Team:Libraries labels Jan 31, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
api-approved API was approved in API review, it can be implemented api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json Cost:S Work that requires one engineer up to 1 week Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants