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

Update graphql.net to 4.6.1 #10782

Merged
merged 15 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from 13 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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Net;
using System.Net.Mime;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using GraphQL;
using GraphQL.Execution;
Expand All @@ -15,7 +16,6 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Formatters;
using Microsoft.Extensions.DependencyInjection;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using OrchardCore.Apis.GraphQL.Queries;
using OrchardCore.Apis.GraphQL.ValidationRules;
Expand All @@ -28,7 +28,6 @@ public class GraphQLMiddleware
private readonly RequestDelegate _next;
private readonly GraphQLSettings _settings;
private readonly IDocumentExecuter _executer;

internal static readonly Encoding _utf8Encoding = new UTF8Encoding(false);
private readonly static MediaType _jsonMediaType = new MediaType("application/json");
private readonly static MediaType _graphQlMediaType = new MediaType("application/graphql");
Expand Down Expand Up @@ -78,8 +77,6 @@ private bool IsGraphQLRequest(HttpContext context)

private async Task ExecuteAsync(HttpContext context, ISchemaFactory schemaService, IDocumentWriter documentWriter)
{


GraphQLRequest request = null;

// c.f. https://graphql.org/learn/serving-over-http/#post-request
Expand Down Expand Up @@ -151,7 +148,6 @@ private async Task ExecuteAsync(HttpContext context, ISchemaFactory schemaServic
_.OperationName = request.OperationName;
_.Inputs = request.Variables.ToInputs();
_.UserContext = _settings.BuildUserContext?.Invoke(context);
_.RequestServices = context.RequestServices;
_.ValidationRules = DocumentValidator.CoreRules
.Concat(context.RequestServices.GetServices<IValidationRule>());
_.ComplexityConfiguration = new ComplexityConfiguration
Expand All @@ -161,16 +157,31 @@ private async Task ExecuteAsync(HttpContext context, ISchemaFactory schemaServic
FieldImpact = _settings.FieldImpact
};
_.Listeners.Add(dataLoaderDocumentListener);
_.RequestServices = context.RequestServices;
});

context.Response.StatusCode = (int)(result.Errors == null || result.Errors.Count == 0
? HttpStatusCode.OK
: result.Errors.Any(x => x.Code == RequiresPermissionValidationRule.ErrorCode)
: result.Errors.Any(x => (x is ValidationError && (x as ValidationError).Number == RequiresPermissionValidationRule.ErrorCode))
MikeKry marked this conversation as resolved.
Show resolved Hide resolved
? HttpStatusCode.Unauthorized
: HttpStatusCode.BadRequest);

context.Response.ContentType = MediaTypeNames.Application.Json;
await documentWriter.WriteAsync(context.Response.Body, result);

await WriteAsync(context.Response.Body, result, documentWriter);
}

private async Task WriteAsync<T>(Stream stream2, T value, IDocumentWriter documentWriter, CancellationToken cancellationToken = default)
{
// needs to be always async, otherwise __schema request is not working, direct write into response does not work as serialize is using sync method inside
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that somewhere in our grapqhl code with have a call to a Serialize method (NewtonSoft?) that is not async and this is triggered in IDocumentWriter.WriteaAsync.
If buffering is required, maybe use a smaller reused buffer instead of a single one as big as the payload, and pool it too? Another option might be to use BufferedStream between documentWriter and context.Response.Body.

cancellationToken.ThrowIfCancellationRequested();

using (MemoryStream stream = new MemoryStream())
Copy link
Member

Choose a reason for hiding this comment

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

Might use BufferedStream so not all the payload is serialized in memory.

{
await documentWriter.WriteAsync(stream, value);
stream.Seek(0, SeekOrigin.Begin);
await stream.CopyToAsync(stream2, cancellationToken);
}
}

private static GraphQLRequest CreateRequestFromQueryString(HttpContext context, bool validateQueryKey = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ public class OrchardFieldNameConverter : INameConverter
{
private readonly INameConverter _defaultConverter = new CamelCaseNameConverter();

public string NameForArgument(string fieldName, IComplexGraphType parentGraphType, FieldType field)
// todo: custom argument name?
public string NameForArgument(string argumentName, IComplexGraphType parentGraphType, FieldType field)
{
return _defaultConverter.NameForArgument(fieldName, parentGraphType, field);
return _defaultConverter.NameForArgument(argumentName, parentGraphType, field);
}

// TODO: check functionality
public string NameForField(string fieldName, IComplexGraphType parentGraphType)
{
var attributes = parentGraphType?.GetType().GetCustomAttributes(typeof(GraphQLFieldNameAttribute), true);
Expand All @@ -22,7 +24,6 @@ public string NameForField(string fieldName, IComplexGraphType parentGraphType)
{
foreach (GraphQLFieldNameAttribute attribute in attributes)
{

if (attribute.Field == fieldName)
{
return attribute.Mapped;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ namespace OrchardCore.Apis.GraphQL.Services
public class SchemaService : ISchemaFactory
{
private readonly IEnumerable<ISchemaBuilder> _schemaBuilders;
private readonly IServiceProvider _serviceProvider;
private readonly SemaphoreSlim _schemaGenerationSemaphore = new SemaphoreSlim(1, 1);
private readonly ConcurrentDictionary<ISchemaBuilder, string> _identifiers = new ConcurrentDictionary<ISchemaBuilder, string>();

private ISchema _schema;

public SchemaService(IEnumerable<ISchemaBuilder> schemaBuilders)
public SchemaService(IEnumerable<ISchemaBuilder> schemaBuilders, IServiceProvider serviceProvider)
{
_schemaBuilders = schemaBuilders;
_serviceProvider = serviceProvider;
}

public async Task<ISchema> GetSchemaAsync()
Expand Down Expand Up @@ -63,7 +65,7 @@ public async Task<ISchema> GetSchemaAsync()

var serviceProvider = ShellScope.Services;

var schema = new Schema(new SelfActivatingServiceProvider(serviceProvider))
var schema = new Schema(new SelfActivatingServiceProvider(_serviceProvider))
{
Query = new ObjectGraphType { Name = "Query" },
Mutation = new ObjectGraphType { Name = "Mutation" },
Expand Down
10 changes: 5 additions & 5 deletions src/OrchardCore.Modules/OrchardCore.Apis.GraphQL/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using GraphQL.Validation;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Server.Kestrel.Core;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
Expand Down Expand Up @@ -40,16 +39,17 @@ public override void ConfigureServices(IServiceCollection services)
services.AddSingleton<IDocumentWriter, DocumentWriter>();
services.AddSingleton<IDataLoaderContextAccessor, DataLoaderContextAccessor>();
services.AddSingleton<IDocumentExecutionListener, DataLoaderDocumentListener>();

services.AddSingleton<ISchemaFactory, SchemaService>();
services.AddScoped<IValidationRule, MaxNumberOfResultsValidationRule>();
services.AddScoped<IValidationRule, RequiresPermissionValidationRule>();

services.AddSingleton<IErrorInfoProvider>(services =>
{
var settings = services.GetRequiredService<IOptions<GraphQLSettings>>();
return new ErrorInfoProvider(new ErrorInfoProviderOptions { ExposeExceptionStackTrace = settings.Value.ExposeExceptions });
});

services.AddSingleton<ISchemaFactory, SchemaService>();
services.AddScoped<IValidationRule, MaxNumberOfResultsValidationRule>();
services.AddScoped<IValidationRule, RequiresPermissionValidationRule>();

services.AddScoped<IPermissionProvider, Permissions>();
services.AddTransient<INavigationProvider, AdminMenu>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,67 +12,61 @@ public class MaxNumberOfResultsValidationRule : IValidationRule
{
private readonly int _maxNumberOfResults;
private readonly MaxNumberOfResultsValidationMode _maxNumberOfResultsValidationMode;
private readonly IStringLocalizer<MaxNumberOfResultsValidationRule> _localizer;
private readonly ILogger<MaxNumberOfResultsValidationRule> _logger;

public MaxNumberOfResultsValidationRule(IOptions<GraphQLSettings> options)
public MaxNumberOfResultsValidationRule(IOptions<GraphQLSettings> options, IStringLocalizer<MaxNumberOfResultsValidationRule> localizer, ILogger<MaxNumberOfResultsValidationRule> logger)
{
var settings = options.Value;
_maxNumberOfResults = settings.MaxNumberOfResults;
_maxNumberOfResultsValidationMode = settings.MaxNumberOfResultsValidationMode;
_localizer = localizer;
_logger = logger;
}

public Task<INodeVisitor> ValidateAsync(ValidationContext validationContext)
{
// Todo: EnterLeaveListener has been removed and the signatures of INodeVisitor.Enter and INodeVisitor.Leave have changed. NodeVisitors class has been added in its place.
// https://graphql-dotnet.github.io/docs/migrations/migration4/
// Ex: https://github.com/graphql-dotnet/graphql-dotnet/issues/2406
INodeVisitor result = new NodeVisitors();
return Task.FromResult(result);
return Task.FromResult((INodeVisitor)new NodeVisitors(
new MatchingNodeVisitor<Argument>((arg, visitorContext) =>
{
if ((arg.Name == "first" || arg.Name == "last") && arg.Value != null)
{
var context = (GraphQLUserContext)validationContext.UserContext;

//return new EnterLeaveListener(_ =>
//{
// _.Match<Argument>(arg =>
// {
// if ((arg.Name == "first" || arg.Name == "last") && arg.Value != null)
// {
// var context = (GraphQLUserContext)validationContext.UserContext;
int? value = null;

// int? value = null;
if (arg.Value is IntValue)
{
value = ((IntValue)arg.Value)?.Value;
}
else
{
if (validationContext.Inputs.TryGetValue(arg.Value.ToString(), out var input))
{
value = (int?)input;
}
}

// if (arg.Value is IntValue)
// {
// value = ((IntValue)arg.Value)?.Value;
// }
// else
// {
// if (validationContext.Inputs.TryGetValue(arg.Value.ToString(), out var input))
// {
// value = (int?)input;
// }
// }
if (value.HasValue && value > _maxNumberOfResults)
{
var errorMessage = _localizer["'{0}' exceeds the maximum number of results for '{1}' ({2})", value.Value, arg.Name, _maxNumberOfResults];

// if (value.HasValue && value > _maxNumberOfResults)
// {
// var localizer = context.ServiceProvider.GetService<IStringLocalizer<MaxNumberOfResultsValidationRule>>();
// var errorMessage = localizer["'{0}' exceeds the maximum number of results for '{1}' ({2})", value.Value, arg.Name, _maxNumberOfResults];

// if (_maxNumberOfResultsValidationMode == MaxNumberOfResultsValidationMode.Enabled)
// {
// validationContext.ReportError(new ValidationError(
// validationContext.OriginalQuery,
// "ArgumentInputError",
// errorMessage,
// arg));
// }
// else
// {
// var logger = context.ServiceProvider.GetService<ILogger<MaxNumberOfResultsValidationMode>>();
// logger.LogInformation(errorMessage);
// arg.Value = new IntValue(_maxNumberOfResults); // if disabled mode we just log info and override the arg to be maxvalue
// }
// }
// }
// });
//});
if (_maxNumberOfResultsValidationMode == MaxNumberOfResultsValidationMode.Enabled)
{
validationContext.ReportError(new ValidationError(
validationContext.Document.OriginalQuery,
"ArgumentInputError",
errorMessage,
arg));
}
else
{
_logger.LogInformation(errorMessage);
arg = new Argument(arg.NameNode, new IntValue(_maxNumberOfResults)); // if disabled mode we just log info and override the arg to be maxvalue
}
}
}
})));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,13 @@ public RequiresPermissionValidationRule(IAuthorizationService authorizationServi

public async Task<INodeVisitor> ValidateAsync(ValidationContext validationContext)
{
var operationType = OperationType.Query;
// shouldn't we access UserContext from validationcontext inside MatchingNodeVisitor actions?
Copy link
Member

Choose a reason for hiding this comment

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

probably :)

var userContext = (GraphQLUserContext)validationContext.UserContext;

return await Task.FromResult(new NodeVisitors(
new MatchingNodeVisitor<Operation>(async (astType, validationContext) =>
{
operationType = astType.OperationType;
await AuthorizeOperationAsync(astType, validationContext, userContext, operationType, astType.Name);
await AuthorizeOperationAsync(astType, validationContext, userContext, astType.OperationType, astType.Name);
}),
new MatchingNodeVisitor<ObjectField>(async (objectFieldAst, validationContext) =>
{
Expand All @@ -58,18 +57,6 @@ public async Task<INodeVisitor> ValidateAsync(ValidationContext validationContex
));
}


//private static bool Authorize(IProvideMetadata type, GraphQLUserContext context)
//{
// //var authorizationManager = context.ServiceProvider.GetService<IAuthorizationService>();

// //// awaitable IValidationRule in graphql dotnet is coming soon:
// //// https://github.com/graphql-dotnet/graphql-dotnet/issues/1140
// //return type.GetPermissions().All(x => authorizationManager.AuthorizeAsync(context.User, x.Permission, x.Resource).GetAwaiter().GetResult());


//}

private async Task AuthorizeOperationAsync(INode node, ValidationContext validationContext, GraphQLUserContext userContext, OperationType? operationType, string operationName)
{
if (operationType == OperationType.Mutation && !(await _authorizationService.AuthorizeAsync(userContext.User, Permissions.ExecuteGraphQLMutations)))
Expand All @@ -84,7 +71,8 @@ private async Task AuthorizeOperationAsync(INode node, ValidationContext validat

private async Task AuthorizeNodePermissionAsync(INode node, IFieldType fieldType, ValidationContext validationContext, GraphQLUserContext userContext)
{
if (!fieldType.HasPermissions()) {
if (!fieldType.HasPermissions())
{
return;
}

Expand All @@ -98,7 +86,7 @@ private async Task AuthorizeNodePermissionAsync(INode node, IFieldType fieldType
if (!authorizationResult)
AddPermissionValidationError(validationContext, node, fieldType.Name);
}
else
else
{
var tasks = new List<Task<bool>>(permissions.Count());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using GraphQL.Resolvers;
using GraphQL.Types;
using Newtonsoft.Json.Linq;
using OrchardCore.ContentFields.Fields;
using OrchardCore.ContentFields.GraphQL.Types;
using OrchardCore.ContentManagement;
Expand Down Expand Up @@ -81,7 +82,7 @@ public class ContentFieldsProvider : IContentFieldProvider
Description = "Multi text field",
FieldType = typeof(ListGraphType<StringGraphType>),
UnderlyingType = typeof(MultiTextField),
FieldAccessor = field => (string[]) field.Content.Values
FieldAccessor = field => ((JArray)field.Content.Values)?.ToObject<string[]>()
Copy link
Member

Choose a reason for hiding this comment

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

ok you can remove the comment .. these newtsonsoft refs make me cry tho .. we gotta sortt it out been system.text.json compliant at some point :P (its fine cos there is other stuff like this already)

Copy link
Member

Choose a reason for hiding this comment

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

I was about to say the same thing before seeing the comment. We shouldn't tie it to NewtonSoft. If the field is MultiTextField then cast to that, or map to it, and access its own properties.

}
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public FieldType GetField(ContentPartFieldDefinition field)
Type = queryGraphType,
Resolver = new FuncFieldResolver<ContentElement, ContentElement>(context =>
{
var typeToResolve = context.FieldDefinition.ResolvedType.GetType().BaseType.GetGenericArguments().First(); // ReturnType.GetType().BaseType.GetGenericArguments().First();
var typeToResolve = context.FieldDefinition.ResolvedType.GetType().BaseType.GetGenericArguments().First();

// Check if part has been collapsed by trying to get the parent part.
var contentPart = context.Source.Get(typeof(ContentPart), field.PartDefinition.Name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ public MediaAssetObjectType()
.Resolve(x =>
{
var path = x.Source.Path;
var context = (GraphQLUserContext)x.UserContext;
var mediaFileStore = x.RequestServices.GetService<IMediaFileStore>();
return mediaFileStore.MapPathToPublicUrl(path);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public MediaFieldQueryObjectType()
.Resolve(x =>
{
var paths = x.Page(x.Source.Paths);
var context = (GraphQLUserContext)x.UserContext;
var mediaFileStore = x.RequestServices.GetService<IMediaFileStore>();
return paths.Select(p => mediaFileStore.MapPathToPublicUrl(p));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Threading.Tasks;
using GraphQL;
using GraphQL.Builders;
using GraphQL.Types;
using OrchardCore.Apis.GraphQL.Resolvers;

namespace OrchardCore.Apis.GraphQL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static bool HasPopulatedArgument(this IResolveFieldContext source, string
{
if (source.Arguments?.ContainsKey(argumentName) ?? false)
{
return !string.IsNullOrEmpty((source.Arguments[argumentName].Value ?? string.Empty).ToString());
return !string.IsNullOrEmpty(source.Arguments[argumentName].Value?.ToString());
};

return false;
Expand All @@ -25,7 +25,7 @@ public static bool HasPopulatedArgument<TSource>(this IResolveFieldContext<TSour
{
if (source.Arguments?.ContainsKey(argumentName) ?? false)
{
return !string.IsNullOrEmpty((source.Arguments[argumentName].Value ?? string.Empty).ToString());
return !string.IsNullOrEmpty(source.Arguments[argumentName].Value?.ToString());
};

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace OrchardCore.Apis.GraphQL
public class GraphQLUserContext : Dictionary<string, object>
{
public ClaimsPrincipal User { get; set; }

public SemaphoreSlim ExecutionContextLock { get; } = new SemaphoreSlim(1, 1);
}
}
Loading