Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implementations of new HttpContent sync methods. #38635

Merged
merged 15 commits into from
Jul 10, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ public static IEnumerable<object[]> DecompressedResponse_MethodSpecified_Decompr
}
}

private HttpRequestMessage CreateRequest(HttpMethod method, Uri uri, Version version) =>
new HttpRequestMessage(method, uri) { Version = version };

[Theory]
[MemberData(nameof(DecompressedResponse_MethodSpecified_DecompressedContentReturned_MemberData))]
public async Task DecompressedResponse_MethodSpecified_DecompressedContentReturned(
Expand Down Expand Up @@ -162,11 +165,17 @@ await server.AcceptConnectionAsync(async connection =>
[Theory, MemberData(nameof(RemoteServersAndCompressionUris))]
public async Task GetAsync_SetAutomaticDecompression_ContentDecompressed(Configuration.Http.RemoteServer remoteServer, Uri uri)
{
// Sync API supported only up to HTTP/1.1
if (!TestAsync && remoteServer.HttpVersion.Major >= 2)
{
return;
}

HttpClientHandler handler = CreateHttpClientHandler();
handler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;
using (HttpClient client = CreateHttpClientForRemoteServer(remoteServer, handler))
{
using (HttpResponseMessage response = await client.GetAsync(uri))
using (HttpResponseMessage response = await client.SendAsync(TestAsync, CreateRequest(HttpMethod.Get, uri, remoteServer.HttpVersion)))
{
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
string responseContent = await response.Content.ReadAsStringAsync();
Expand All @@ -184,10 +193,16 @@ public async Task GetAsync_SetAutomaticDecompression_ContentDecompressed(Configu
[Theory, MemberData(nameof(RemoteServersAndCompressionUris))]
public async Task GetAsync_SetAutomaticDecompression_HeadersRemoved(Configuration.Http.RemoteServer remoteServer, Uri uri)
{
// Sync API supported only up to HTTP/1.1
if (!TestAsync && remoteServer.HttpVersion.Major >= 2)
{
return;
}

HttpClientHandler handler = CreateHttpClientHandler();
handler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;
using (HttpClient client = CreateHttpClientForRemoteServer(remoteServer, handler))
using (HttpResponseMessage response = await client.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead))
using (HttpResponseMessage response = await client.SendAsync(TestAsync, CreateRequest(HttpMethod.Get, uri, remoteServer.HttpVersion), HttpCompletionOption.ResponseHeadersRead))
Comment on lines -190 to +205
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this accomplishing? Shouldn't the client's DefaultRequestVersion be the same as remoteServer.HttpVersion here? HttpClient does essentially the same thing as this CreateRequest when you call GetAsync.

Copy link
Member Author

@ManickaP ManickaP Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SendAsync|Send ignores HttpClient.DefaultRequestVersion since it accepts fully constructed request. The default initialization of HttpRequestMessage will set it to 1.1. And because we plug in VersionChecker set up with the remote server version, it will blow up for 2.0 servers.

If it's about why Send versus Get, then it's because we weren't testing decompression with sync code paths at all due to using GetAsync (helpers didn't get sync overloads). And we had an error in missing overrides for decompression content 😞 So I changed the tests to use SendAsync|Send instead and now we're testing both, sync and async.

{
Assert.Equal(HttpStatusCode.OK, response.StatusCode);

Expand Down Expand Up @@ -236,7 +251,7 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
client.DefaultRequestHeaders.Add("Accept-Encoding", manualAcceptEncodingHeaderValues);
}

Task<HttpResponseMessage> clientTask = client.GetAsync(url);
Task<HttpResponseMessage> clientTask = client.SendAsync(TestAsync, CreateRequest(HttpMethod.Get, url, UseVersion));
Task<List<string>> serverTask = server.AcceptConnectionSendResponseAndCloseAsync();
await TaskTimeoutExtensions.WhenAllOrAnyFailed(new Task[] { clientTask, serverTask });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,25 @@ public VersionCheckerHttpHandler(HttpMessageHandler innerHandler, Version expect
_expectedVersion = expectedVersion;
}

#if NETCOREAPP
protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
{
if (request.Version != _expectedVersion)
{
throw new Exception($"Unexpected request version: expected {_expectedVersion}, saw {request.Version}");
}

HttpResponseMessage response = base.Send(request, cancellationToken);

if (response.Version != _expectedVersion)
{
throw new Exception($"Unexpected response version: expected {_expectedVersion}, saw {response.Version}");
}

return response;
}
#endif

protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
if (request.Version != _expectedVersion)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace System.Net.Http.Json
{
public sealed partial class JsonContent : System.Net.Http.HttpContent
{
protected override void SerializeToStream(System.IO.Stream stream, System.Net.TransportContext? context, System.Threading.CancellationToken cancellationToken) { }
protected override System.Threading.Tasks.Task SerializeToStreamAsync(System.IO.Stream stream, System.Net.TransportContext? context, System.Threading.CancellationToken cancellationToken) { throw null; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ public static JsonContent Create(object? inputValue, Type inputType, MediaTypeHe
=> new JsonContent(inputValue, inputType, mediaType, options);

protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context)
=> SerializeToStreamAsyncCore(stream, CancellationToken.None);
=> SerializeToStreamAsyncCore(stream, async: true, CancellationToken.None);

protected override bool TryComputeLength(out long length)
{
length = 0;
return false;
}

private async Task SerializeToStreamAsyncCore(Stream targetStream, CancellationToken cancellationToken)
private async Task SerializeToStreamAsyncCore(Stream targetStream, bool async, CancellationToken cancellationToken)
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
{
Encoding? targetEncoding = GetEncoding(Headers.ContentType?.CharSet);

Expand All @@ -70,15 +70,34 @@ private async Task SerializeToStreamAsyncCore(Stream targetStream, CancellationT
Stream transcodingStream = Encoding.CreateTranscodingStream(targetStream, targetEncoding, Encoding.UTF8, leaveOpen: true);
try
{
await JsonSerializer.SerializeAsync(transcodingStream, Value, ObjectType, _jsonSerializerOptions, cancellationToken).ConfigureAwait(false);
if (async)
{
await JsonSerializer.SerializeAsync(transcodingStream, Value, ObjectType, _jsonSerializerOptions, cancellationToken).ConfigureAwait(false);
}
else
{
// Have to use Utf8JsonWriter because JsonSerializer doesn't support sync serialization into stream directly.
// ToDo: Remove Utf8JsonWriter usage after https://github.com/dotnet/runtime/issues/1574
using Utf8JsonWriter writer = new Utf8JsonWriter(transcodingStream);
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
JsonSerializer.Serialize(writer, Value, ObjectType, _jsonSerializerOptions);
}
}
finally
{
// DisposeAsync will flush any partial write buffers. In practice our partial write
// Dispose/DisposeAsync will flush any partial write buffers. In practice our partial write
// buffers should be empty as we expect JsonSerializer to emit only well-formed UTF-8 data.
await transcodingStream.DisposeAsync().ConfigureAwait(false);
if (async)
{
await transcodingStream.DisposeAsync().ConfigureAwait(false);
}
else
{
transcodingStream.Dispose();
}
}
#else
Debug.Assert(async);

using (TranscodingWriteStream transcodingStream = new TranscodingWriteStream(targetStream, targetEncoding))
{
await JsonSerializer.SerializeAsync(transcodingStream, Value, ObjectType, _jsonSerializerOptions, cancellationToken).ConfigureAwait(false);
Expand All @@ -91,7 +110,21 @@ private async Task SerializeToStreamAsyncCore(Stream targetStream, CancellationT
}
else
{
await JsonSerializer.SerializeAsync(targetStream, Value, ObjectType, _jsonSerializerOptions, cancellationToken).ConfigureAwait(false);
if (async)
{
await JsonSerializer.SerializeAsync(targetStream, Value, ObjectType, _jsonSerializerOptions, cancellationToken).ConfigureAwait(false);
}
else
{
#if NETCOREAPP
// Have to use Utf8JsonWriter because JsonSerializer doesn't support sync serialization into stream directly.
// ToDo: Remove Utf8JsonWriter usage after https://github.com/dotnet/runtime/issues/1574
using Utf8JsonWriter writer = new Utf8JsonWriter(targetStream);
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
JsonSerializer.Serialize(writer, Value, ObjectType, _jsonSerializerOptions);
#else
Debug.Assert(async);
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
#endif
}
}
}
ManickaP marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ namespace System.Net.Http.Json
{
public sealed partial class JsonContent
{
protected override void SerializeToStream(Stream stream, TransportContext? context, CancellationToken cancellationToken)
=> SerializeToStreamAsyncCore(stream, async: false, cancellationToken).GetAwaiter().GetResult();

protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context, CancellationToken cancellationToken)
=> SerializeToStreamAsyncCore(stream, cancellationToken);
=> SerializeToStreamAsyncCore(stream, async: true, cancellationToken);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.Tasks;
Expand All @@ -13,17 +13,10 @@ namespace System.Net.Http.Json.Functional.Tests
{
public class HttpClientJsonExtensionsTests
{
private static readonly JsonSerializerOptions s_defaultSerializerOptions
= new JsonSerializerOptions
{
PropertyNameCaseInsensitive = true,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase
};

[Fact]
public async Task TestGetFromJsonAsync()
{
const string json = @"{""Name"":""David"",""Age"":24}";
string json = Person.Create().Serialize();
HttpHeaderData header = new HttpHeaderData("Content-Type", "application/json");
List<HttpHeaderData> headers = new List<HttpHeaderData> { header };

Expand Down Expand Up @@ -89,7 +82,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
async server => {
HttpRequestData request = await server.HandleRequestAsync();
ValidateRequest(request);
Person per = JsonSerializer.Deserialize<Person>(request.Body, s_defaultSerializerOptions);
Person per = JsonSerializer.Deserialize<Person>(request.Body, JsonOptions.DefaultSerializerOptions);
per.Validate();
});
}
Expand Down Expand Up @@ -121,7 +114,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
async server => {
HttpRequestData request = await server.HandleRequestAsync();
ValidateRequest(request);
Person obj = JsonSerializer.Deserialize<Person>(request.Body, s_defaultSerializerOptions);
Person obj = JsonSerializer.Deserialize<Person>(request.Body, JsonOptions.DefaultSerializerOptions);
obj.Validate();
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
Expand Down Expand Up @@ -154,7 +154,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
[Fact]
public async Task TestGetFromJsonAsyncTextPlainUtf16Async()
{
const string json = @"{""Name"":""David"",""Age"":24}";
string json = Person.Create().Serialize();
await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
async (handler, uri) =>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net.Http.Headers;
using System.Net.Test.Common;
using System.Runtime.CompilerServices;
using System.Text;
using System.Text.Json;
using System.Threading.Tasks;
using Xunit;

namespace System.Net.Http.Json.Functional.Tests
{
public class JsonContentTests
public abstract class JsonContentTestsBase
{
protected abstract Task<HttpResponseMessage> SendAsync(HttpClient client, HttpRequestMessage request);

private class Foo { }
private class Bar { }
Expand Down Expand Up @@ -80,7 +82,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(

var request = new HttpRequestMessage(HttpMethod.Post, uri);
request.Content = content;
await client.SendAsync(request);
await SendAsync(client, request);
}
},
async server => {
Expand Down Expand Up @@ -112,7 +114,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
var request = new HttpRequestMessage(HttpMethod.Post, uri);
MediaTypeHeaderValue mediaType = MediaTypeHeaderValue.Parse("foo/bar; charset=utf-8");
request.Content = JsonContent.Create(Person.Create(), mediaType: mediaType);
await client.SendAsync(request);
await SendAsync(client, request);
}
},
async server => {
Expand Down Expand Up @@ -159,7 +161,7 @@ public void JsonContentThrowsOnIncompatibleTypeAsync()
}

[Fact]
public static async Task ValidateUtf16IsTranscodedAsync()
public async Task ValidateUtf16IsTranscodedAsync()
{
await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
async (handler, uri) =>
Expand All @@ -170,7 +172,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
MediaTypeHeaderValue mediaType = MediaTypeHeaderValue.Parse("application/json; charset=utf-16");
// Pass new options to avoid using the Default Web Options that use camelCase.
request.Content = JsonContent.Create(Person.Create(), mediaType: mediaType, options: new JsonSerializerOptions());
await client.SendAsync(request);
await SendAsync(client, request);
}
},
async server => {
Expand All @@ -193,7 +195,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
EnsureDefaultOptions dummyObj = new EnsureDefaultOptions();
var request = new HttpRequestMessage(HttpMethod.Post, uri);
request.Content = JsonContent.Create(dummyObj);
await client.SendAsync(request);
await SendAsync(client, request);
}
},
server => server.HandleRequestAsync());
Expand All @@ -213,7 +215,7 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
content.Headers.ContentType = null;

request.Content = content;
await client.SendAsync(request);
await SendAsync(client, request);
}
},
async server => {
Expand All @@ -222,4 +224,9 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
});
}
}

public class JsonContentTests_Async : JsonContentTestsBase
{
protected override Task<HttpResponseMessage> SendAsync(HttpClient client, HttpRequestMessage request) => client.SendAsync(request);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.IO;
using System.Net.Http.Headers;
using System.Net.Test.Common;
using System.Runtime.CompilerServices;
using System.Text;
using System.Text.Json;
using System.Threading.Tasks;
using Xunit;

namespace System.Net.Http.Json.Functional.Tests
{
public class JsonContentTests_Sync : JsonContentTestsBase
{
protected override Task<HttpResponseMessage> SendAsync(HttpClient client, HttpRequestMessage request) => Task.Run(() => client.Send(request));

[Fact]
public void JsonContent_CopyTo_Succeeds()
{
Person person = Person.Create();
using JsonContent content = JsonContent.Create(person);
using MemoryStream stream = new MemoryStream();
// HttpContent.CopyTo internally calls overriden JsonContent.SerializeToStream, which is the targeted method of this test.
content.CopyTo(stream, null, default);
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
stream.Seek(0, SeekOrigin.Begin);
using StreamReader reader = new StreamReader(stream);
string json = reader.ReadToEnd();
Assert.Equal(person.Serialize(JsonOptions.DefaultSerializerOptions), json);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
<Compile Include="JsonContentTests.cs" />
<Compile Include="TestClasses.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'">
<Compile Include="JsonContentTests.netcoreapp.cs" />
</ItemGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\Net\Capability.Security.cs"
Link="Common\System\Net\Capability.Security.cs" />
Expand Down
Loading