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 11 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
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
Expand Down Expand Up @@ -28,7 +29,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 +78,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 +149,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 +158,20 @@ 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);

// changed in V4
var encodedBytes = _utf8Encoding.GetBytes(await documentWriter.WriteToStringAsync(result));
await context.Response.Body.WriteAsync(encodedBytes, 0, encodedBytes.Length); // documentWriter causes problems when querying _schema
Copy link
Member

Choose a reason for hiding this comment

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

can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

documentWriter.WriteAsync throws error when grahpql tries to get schema
"System.InvalidOperationException: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead."

I did not find the cause and using workaround with AllowSynchronousIO does not seem ok to me.

Copy link
Member

Choose a reason for hiding this comment

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

this does ring a bell ... is this an issue in the ag/graphql4 branch currently then ? i have no idea why i changed it .... yours looks closer to the original tbh

Copy link
Member

Choose a reason for hiding this comment

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

Needs more thoughts, will buffer all responses, and the big ones could be an issue in terms of perf. Maybe at least use an ArrayPool?

}

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ public class WhereInputObjectGraphType : WhereInputObjectGraphType<object>

public class WhereInputObjectGraphType<TSourceType> : InputObjectGraphType<TSourceType>
{
// arguments of typed input graph types return typed object, without additional input fields (_in, _contains,..)
// so we return dictionary as it was before.
public override object ParseDictionary(IDictionary<string, object> value)
{
return value;
}

// Applies to all types
public static Dictionary<string, string> EqualityOperators = new Dictionary<string, string>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Collections.Generic;
using GraphQL.Types;

namespace OrchardCore.ContentManagement.GraphQL
Expand Down
Loading