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

Use buffer streams to solve the problem with synchronous reads. #733

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions samples/Samples.Server/Samples.Server.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

<ItemGroup>
<ProjectReference Include="..\..\src\Authorization.AspNetCore\Authorization.AspNetCore.csproj" />
<ProjectReference Include="..\..\src\Transports.AspNetCore.NewtonsoftJson\Transports.AspNetCore.NewtonsoftJson.csproj" />
<ProjectReference Include="..\..\src\Transports.AspNetCore.SystemTextJson\Transports.AspNetCore.SystemTextJson.csproj" />
<ProjectReference Include="..\..\src\Transports.Subscriptions.WebSockets\Transports.Subscriptions.WebSockets.csproj" />
<ProjectReference Include="..\..\src\Ui.GraphiQL\Ui.GraphiQL.csproj" />
Expand Down
12 changes: 11 additions & 1 deletion samples/Samples.Server/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public void ConfigureServices(IServiceCollection services)
{
services.AddSingleton<IChat, Chat>();

MicrosoftDI.GraphQLBuilderExtensions.AddGraphQL(services)
var graphql = MicrosoftDI.GraphQLBuilderExtensions.AddGraphQL(services)
.AddSubscriptionDocumentExecuter()
.AddServer(true)
.AddSchema<ChatSchema>()
Expand All @@ -44,12 +44,22 @@ public void ConfigureServices(IServiceCollection services)
var logger = options.RequestServices.GetRequiredService<ILogger<Startup>>();
options.UnhandledExceptionDelegate = ctx => logger.LogError("{Error} occurred", ctx.OriginalException.Message);
})

.AddSystemTextJson()
.AddErrorInfoProvider<CustomErrorInfoProvider>()
.Configure<ErrorInfoProviderOptions>(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment())
.AddWebSockets()
.AddDataLoader()
.AddGraphTypes(typeof(ChatSchema).Assembly);

if (Configuration["serializer:type"] == "NewtonsoftJson")
{
graphql.AddNewtonsoftJson();
}
else
{
graphql.AddSystemTextJson();
}
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
Expand Down
6 changes: 5 additions & 1 deletion samples/Samples.Server/appsettings.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
{
"Logging": {
"IncludeScopes": false,
"Debug": {
Expand All @@ -10,6 +10,10 @@
"LogLevel": {
"Default": "Debug"
}
},
"serializer": {
// SystemTextJson or NewtonsoftJson
"type": "SystemTextJson"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using GraphQL.Execution;
using GraphQL.NewtonsoftJson;
using Microsoft.AspNetCore.WebUtilities;
using Newtonsoft.Json;

namespace GraphQL.Server.Transports.AspNetCore.NewtonsoftJson
{
public sealed class BufferingDocumentWriter : IDocumentWriter
{
private readonly DocumentWriter _documentWriter;

public BufferingDocumentWriter(Action<JsonSerializerSettings> action, IErrorInfoProvider errorInfoProvider)
{
_documentWriter = new DocumentWriter(action, errorInfoProvider);
}

public async Task WriteAsync<T>(Stream stream, T value, CancellationToken cancellationToken = default)
{
await using (var bufferStream = new FileBufferingWriteStream())
{
await _documentWriter.WriteAsync(bufferStream, value, cancellationToken);

await bufferStream.DrainBufferAsync(stream, cancellationToken);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using GraphQL.Execution;
using GraphQL.NewtonsoftJson;
using GraphQL.Server.Transports.AspNetCore;
using GraphQL.Server.Transports.AspNetCore.NewtonsoftJson;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -31,7 +30,7 @@ public static IGraphQLBuilder AddNewtonsoftJson(this IGraphQLBuilder builder,
Action<JsonSerializerSettings> configureSerializerSettings = null)
{
builder.Services.AddSingleton<IGraphQLRequestDeserializer>(p => new GraphQLRequestDeserializer(configureDeserializerSettings ?? (_ => { })));
builder.Services.Replace(ServiceDescriptor.Singleton<IDocumentWriter>(p => new DocumentWriter(configureSerializerSettings ?? (_ => { }), p.GetService<IErrorInfoProvider>() ?? new ErrorInfoProvider())));
builder.Services.Replace(ServiceDescriptor.Singleton<IDocumentWriter>(p => new BufferingDocumentWriter(configureSerializerSettings ?? (_ => { }), p.GetService<IErrorInfoProvider>() ?? new ErrorInfoProvider())));

return builder;
}
Expand All @@ -55,7 +54,7 @@ public static DI.IGraphQLBuilder AddNewtonsoftJson(this DI.IGraphQLBuilder build
Action<JsonSerializerSettings> configureSerializerSettings = null)
{
builder.Register<IGraphQLRequestDeserializer>(p => new GraphQLRequestDeserializer(configureDeserializerSettings ?? (_ => { })), DI.ServiceLifetime.Singleton);
NewtonsoftJson.GraphQLBuilderExtensions.AddNewtonsoftJson(builder, configureSerializerSettings);
builder.AddDocumentWriter(p => new BufferingDocumentWriter(configureSerializerSettings ?? (_ => { }), p.GetService<IErrorInfoProvider>() ?? new ErrorInfoProvider()));

return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading.Tasks;
using GraphQL.NewtonsoftJson;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.WebUtilities;
using Newtonsoft.Json;

namespace GraphQL.Server.Transports.AspNetCore.NewtonsoftJson
Expand All @@ -14,6 +15,9 @@ namespace GraphQL.Server.Transports.AspNetCore.NewtonsoftJson
/// </summary>
public class GraphQLRequestDeserializer : IGraphQLRequestDeserializer
{
// https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.NewtonsoftJson/src/MvcNewtonsoftJsonOptions.cs
private const int MemoryBufferThreshold = 1024 * 30;

private readonly JsonSerializer _serializer;

public GraphQLRequestDeserializer(Action<JsonSerializerSettings> configure)
Expand All @@ -31,50 +35,57 @@ public GraphQLRequestDeserializer(Action<JsonSerializerSettings> configure)
_serializer = JsonSerializer.Create(settings); // it's thread safe https://stackoverflow.com/questions/36186276/is-the-json-net-jsonserializer-threadsafe
}

public Task<GraphQLRequestDeserializationResult> DeserializeFromJsonBodyAsync(HttpRequest httpRequest, CancellationToken cancellationToken = default)
public async Task<GraphQLRequestDeserializationResult> DeserializeFromJsonBodyAsync(HttpRequest httpRequest, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();

// Do not explicitly or implicitly (via using, etc.) call dispose because StreamReader will dispose inner stream.
// This leads to the inability to use the stream further by other consumers/middlewares of the request processing
// pipeline. In fact, it is absolutely not dangerous not to dispose StreamReader as it does not perform any useful
// work except for the disposing inner stream.
var reader = new StreamReader(httpRequest.Body);

var result = new GraphQLRequestDeserializationResult { IsSuccessful = true };

using (var jsonReader = new JsonTextReader(reader) { CloseInput = false })
// From: https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs
await using (var readStream = new FileBufferingReadStream(httpRequest.Body, MemoryBufferThreshold))
{
int firstChar = reader.Peek();
await readStream.DrainAsync(cancellationToken);
readStream.Seek(0L, SeekOrigin.Begin);

cancellationToken.ThrowIfCancellationRequested();
// Do not explicitly or implicitly (via using, etc.) call dispose because StreamReader will dispose inner stream.
// This leads to the inability to use the stream further by other consumers/middlewares of the request processing
// pipeline. In fact, it is absolutely not dangerous not to dispose StreamReader as it does not perform any useful
// work except for the disposing inner stream.
var reader = new StreamReader(readStream);

try
var result = new GraphQLRequestDeserializationResult { IsSuccessful = true };

using (var jsonReader = new JsonTextReader(reader) { CloseInput = false })
{
switch (firstChar)
int firstChar = reader.Peek();

cancellationToken.ThrowIfCancellationRequested();

try
{
case '{':
result.Single = ToGraphQLRequest(_serializer.Deserialize<InternalGraphQLRequest>(jsonReader));
break;
case '[':
result.Batch = _serializer.Deserialize<InternalGraphQLRequest[]>(jsonReader)
.Select(ToGraphQLRequest)
.ToArray();
break;
default:
result.IsSuccessful = false;
result.Exception = GraphQLRequestDeserializationException.InvalidFirstChar();
break;
switch (firstChar)
{
case '{':
result.Single = ToGraphQLRequest(_serializer.Deserialize<InternalGraphQLRequest>(jsonReader));
break;
case '[':
result.Batch = _serializer.Deserialize<InternalGraphQLRequest[]>(jsonReader)
.Select(ToGraphQLRequest)
.ToArray();
break;
default:
result.IsSuccessful = false;
result.Exception = GraphQLRequestDeserializationException.InvalidFirstChar();
break;
}
}
catch (JsonException e)
{
result.IsSuccessful = false;
result.Exception = new GraphQLRequestDeserializationException(e);
}
}
catch (JsonException e)
{
result.IsSuccessful = false;
result.Exception = new GraphQLRequestDeserializationException(e);
}
}

return Task.FromResult(result);
return result;
}
}

public Inputs DeserializeInputsFromJson(string json) => json?.ToInputs();
Expand Down