From 7f7f0d2ab2a2017192d4e1a4e68c8f0bc439cddf Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 31 Jan 2023 17:27:26 +0100 Subject: [PATCH] Fixed type inference issue when using any with schema-first. (#5740) --- .../Execution/Pipeline/ExceptionMiddleware.cs | 1 - .../Conventions/DefaultTypeInspector.cs | 34 ++-- .../Descriptors/Conventions/ITypeInspector.cs | 7 +- .../src/Types/Types/Helpers/TypeMemHelper.cs | 1 + .../Core/src/Types/Types/InputObjectType.cs | 5 +- .../Interceptors/ResolverTypeInterceptor.cs | 2 +- .../test/Execution.Tests/SchemaFirstTests.cs | 147 +++++++++++++++--- 7 files changed, 160 insertions(+), 37 deletions(-) diff --git a/src/HotChocolate/Core/src/Execution/Pipeline/ExceptionMiddleware.cs b/src/HotChocolate/Core/src/Execution/Pipeline/ExceptionMiddleware.cs index 8c163a95556..76f23a667c3 100644 --- a/src/HotChocolate/Core/src/Execution/Pipeline/ExceptionMiddleware.cs +++ b/src/HotChocolate/Core/src/Execution/Pipeline/ExceptionMiddleware.cs @@ -38,4 +38,3 @@ public async ValueTask InvokeAsync(IRequestContext context) } } } - diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DefaultTypeInspector.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DefaultTypeInspector.cs index 1d100b0eb21..8118f00abbb 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DefaultTypeInspector.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DefaultTypeInspector.cs @@ -48,7 +48,8 @@ public DefaultTypeInspector(bool ignoreRequiredAttribute = false) public ReadOnlySpan GetMembers( Type type, bool includeIgnored = false, - bool includeStatic = false) + bool includeStatic = false, + bool allowObject = false) { if (type is null) { @@ -70,7 +71,7 @@ public DefaultTypeInspector(bool ignoreRequiredAttribute = false) foreach (var member in members) { - if (CanBeHandled(member, includeIgnored)) + if (CanBeHandled(member, includeIgnored, allowObject)) { temp[next++] = member; } @@ -683,7 +684,10 @@ public bool CollectNullability(IExtendedType type, Span buffer, out int w return false; } - private bool CanBeHandled(MemberInfo member, bool includeIgnored) + private bool CanBeHandled( + MemberInfo member, + bool includeIgnored, + bool allowObjectType) { if (IsSystemMember(member)) { @@ -714,15 +718,16 @@ private bool CanBeHandled(MemberInfo member, bool includeIgnored) if (member is PropertyInfo property) { - return CanHandleReturnType(member, property.PropertyType) && + return CanHandleReturnType(member, property.PropertyType, allowObjectType) && property.GetIndexParameters().Length == 0; } - if (member is MethodInfo method && CanHandleReturnType(member, method.ReturnType)) + if (member is MethodInfo method && + CanHandleReturnType(member, method.ReturnType, allowObjectType)) { foreach (var parameter in method.GetParameters()) { - if (!CanHandleParameter(parameter)) + if (!CanHandleParameter(parameter, allowObjectType)) { return false; @@ -735,7 +740,10 @@ private bool CanBeHandled(MemberInfo member, bool includeIgnored) return false; } - private static bool CanHandleReturnType(MemberInfo member, Type returnType) + private static bool CanHandleReturnType( + MemberInfo member, + Type returnType, + bool allowObjectType) { if (returnType == typeof(void) || returnType == typeof(Task) || @@ -748,7 +756,7 @@ private static bool CanHandleReturnType(MemberInfo member, Type returnType) returnType == typeof(Task) || returnType == typeof(ValueTask)) { - return HasConfiguration(member); + return allowObjectType || HasConfiguration(member); } if (typeof(IAsyncResult).IsAssignableFrom(returnType)) @@ -797,7 +805,7 @@ private static bool CanHandleReturnType(MemberInfo member, Type returnType) return true; } - private static bool CanHandleParameter(ParameterInfo parameter) + private static bool CanHandleParameter(ParameterInfo parameter, bool allowObjectType) { // schema, object type and object field can be injected into a resolver, so // we allow these as parameter type. @@ -811,8 +819,12 @@ private static bool CanHandleParameter(ParameterInfo parameter) } // All other types may cause errors and need to have an explicit configuration. - if (parameterType == typeof(object) || - typeof(ITypeSystemMember).IsAssignableFrom(parameterType)) + if (parameterType == typeof(object)) + { + return allowObjectType || HasConfiguration(parameter); + } + + if (typeof(ITypeSystemMember).IsAssignableFrom(parameterType)) { return HasConfiguration(parameter); } diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/ITypeInspector.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/ITypeInspector.cs index 45acccb56da..43f6327a150 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/ITypeInspector.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/ITypeInspector.cs @@ -3,7 +3,6 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; using HotChocolate.Internal; -using HotChocolate.Types.Relay; #nullable enable @@ -27,13 +26,17 @@ public interface ITypeInspector : IConvention /// /// Specifies if static members shall be returned. /// + /// + /// Specifies if object is allowed as parameter or return type without a type attribute. + /// /// /// Returns the relevant members of a object or input object. /// ReadOnlySpan GetMembers( Type type, bool includeIgnored = false, - bool includeStatic = false); + bool includeStatic = false, + bool allowObject = false); /// /// Defines if a member shall be ignored. This method interprets ignore attributes. diff --git a/src/HotChocolate/Core/src/Types/Types/Helpers/TypeMemHelper.cs b/src/HotChocolate/Core/src/Types/Types/Helpers/TypeMemHelper.cs index f0b71e75117..6939e893ba1 100644 --- a/src/HotChocolate/Core/src/Types/Types/Helpers/TypeMemHelper.cs +++ b/src/HotChocolate/Core/src/Types/Types/Helpers/TypeMemHelper.cs @@ -102,6 +102,7 @@ public static void Clear() Interlocked.Exchange(ref _inputFieldDefinitionMap, null); Interlocked.Exchange(ref _inputFieldMap, null); Interlocked.Exchange(ref _directiveArgumentMap, null); + Interlocked.Exchange(ref _argumentNameMap, null); Interlocked.Exchange(ref _memberSet, null); Interlocked.Exchange(ref _nameSet, null); } diff --git a/src/HotChocolate/Core/src/Types/Types/InputObjectType.cs b/src/HotChocolate/Core/src/Types/Types/InputObjectType.cs index 8cc0c4bbc3a..c29f80fa12b 100644 --- a/src/HotChocolate/Core/src/Types/Types/InputObjectType.cs +++ b/src/HotChocolate/Core/src/Types/Types/InputObjectType.cs @@ -7,10 +7,11 @@ namespace HotChocolate.Types; /// +/// /// A GraphQL Input Object defines a set of input fields; the input fields are either scalars, /// enums, or other input objects. This allows arguments to accept arbitrarily complex structs. -/// -/// In this example, an Input Object called Point2D describes x and y inputs: +/// +/// In this example, an Input Object called Point2D describes x and y inputs: /// /// /// input Point2D { diff --git a/src/HotChocolate/Core/src/Types/Types/Interceptors/ResolverTypeInterceptor.cs b/src/HotChocolate/Core/src/Types/Types/Interceptors/ResolverTypeInterceptor.cs index 618341f6534..78bc739d024 100644 --- a/src/HotChocolate/Core/src/Types/Types/Interceptors/ResolverTypeInterceptor.cs +++ b/src/HotChocolate/Core/src/Types/Types/Interceptors/ResolverTypeInterceptor.cs @@ -371,7 +371,7 @@ private void CollectResolverMembers(CompletionContext context, string typeName) private void CollectSourceMembers(CompletionContext context, Type runtimeType) { - foreach (var member in _typeInspector.GetMembers(runtimeType)) + foreach (var member in _typeInspector.GetMembers(runtimeType, allowObject: true)) { var name = _naming.GetMemberName(member, MemberKind.ObjectField); context.Members[name] = member; diff --git a/src/HotChocolate/Core/test/Execution.Tests/SchemaFirstTests.cs b/src/HotChocolate/Core/test/Execution.Tests/SchemaFirstTests.cs index 9958ba39426..5cdaf244633 100644 --- a/src/HotChocolate/Core/test/Execution.Tests/SchemaFirstTests.cs +++ b/src/HotChocolate/Core/test/Execution.Tests/SchemaFirstTests.cs @@ -1,6 +1,11 @@ +#nullable enable using System.Threading.Tasks; +using CookieCrumble; using HotChocolate.Tests; +using Microsoft.Extensions.DependencyInjection; +using Newtonsoft.Json; using Xunit; +using JsonSerializer = System.Text.Json.JsonSerializer; namespace HotChocolate.Execution; @@ -13,9 +18,9 @@ public async Task BindObjectTypeImplicit() var schema = SchemaBuilder.New() .AddDocumentFromString( @"type Query { - test: String - testProp: String - }") + test: String + testProp: String + }") .AddResolver() .Create(); @@ -68,13 +73,13 @@ public async Task EnumAsOutputType() var schema = SchemaBuilder.New() .AddDocumentFromString( @"type Query { - enumValue: FooEnum - } + enumValue: FooEnum + } - enum FooEnum { - BAR - BAZ - }") + enum FooEnum { + BAR + BAZ + }") .AddResolver("Query") .Create(); @@ -121,18 +126,20 @@ public async Task InputObjectWithEnum() // arrange var schema = SchemaBuilder.New() .AddDocumentFromString( - @"type Query { - enumInInputObject(payload:Payload) : String - } + """ + type Query { + enumInInputObject(payload: Payload) : String + } - input Payload { - value: FooEnum - } + input Payload { + value: FooEnum + } - enum FooEnum { - BAR - BAZ - }") + enum FooEnum { + BAR + BAZ + } + """) .AddResolver("Query") .AddResolver() .Create(); @@ -147,6 +154,71 @@ enum FooEnum { result.MatchSnapshot(); } + /// + /// https://github.com/ChilliCream/graphql-platform/issues/5730 + /// + [Fact] + public async Task Issue_5730() + { + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddDocumentFromString( + """ + schema { + query: Query + mutation: Mutation + } + + type Query { + dummy: String! + } + + type Mutation { + changeChannelParameters( + input: ChangeChannelParameterInput!) + : ChangeChannelParameterPayload! + } + + input ChangeChannelParameterInput { + parameterChangeInfo: [ParameterValuePair!]! + } + + input ParameterValuePair { + value: Any + } + + type ChangeChannelParameterPayload { + message: String! + } + + scalar Any + """) + .AddResolver("Query") + .AddResolver("Mutation") + .ExecuteRequestAsync( + """ + mutation { + changeChannelParameters(input: { + parameterChangeInfo: [ { value: { a: "b" } } ] + }) { + message + } + } + """); + + result.MatchInlineSnapshot( + """ + { + "data": { + "changeChannelParameters": { + "message": "b" + } + } + } + """); + } + public class Query { public string GetTest() @@ -199,4 +271,39 @@ public enum FooEnum Baz, BazBar } -} \ No newline at end of file + + public class Query5730 + { + public string Dummy => "Don't care"; + } + + public class Mutation5730 + { + public Task ChangeChannelParametersAsync( + ChangeChannelParameterInput input, + CancellationToken _) + { + var message = Assert.IsType( + Assert.IsType>( + input.ParameterChangeInfo[0].Value)["a"]); + + return Task.FromResult(new ChangeChannelParameterPayload { Message = message }); + } + } + + public record ChangeChannelParameterInput + { + public ParameterValuePair[] ParameterChangeInfo { get; set; } = + Array.Empty(); + } + + public record ParameterValuePair + { + public object? Value { get; set; } + } + + public record ChangeChannelParameterPayload + { + public string Message { get; init; } = string.Empty; + } +}