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

Fixed type inference issue when using any with schema-first. #5740

Merged
merged 3 commits into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,3 @@ public async ValueTask InvokeAsync(IRequestContext context)
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public DefaultTypeInspector(bool ignoreRequiredAttribute = false)
public ReadOnlySpan<MemberInfo> GetMembers(
Type type,
bool includeIgnored = false,
bool includeStatic = false)
bool includeStatic = false,
bool allowObject = false)
{
if (type is null)
{
Expand All @@ -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;
}
Expand Down Expand Up @@ -683,7 +684,10 @@ public bool CollectNullability(IExtendedType type, Span<bool?> 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))
{
Expand Down Expand Up @@ -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;
Expand All @@ -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) ||
Expand All @@ -748,7 +756,7 @@ private static bool CanHandleReturnType(MemberInfo member, Type returnType)
returnType == typeof(Task<object>) ||
returnType == typeof(ValueTask<object>))
{
return HasConfiguration(member);
return allowObjectType || HasConfiguration(member);
}

if (typeof(IAsyncResult).IsAssignableFrom(returnType))
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using HotChocolate.Internal;
using HotChocolate.Types.Relay;

#nullable enable

Expand All @@ -27,13 +26,17 @@ public interface ITypeInspector : IConvention
/// <param name="includeStatic">
/// Specifies if static members shall be returned.
/// </param>
/// <param name="allowObject">
/// Specifies if object is allowed as parameter or return type without a type attribute.
/// </param>
/// <returns>
/// Returns the relevant members of a object or input object.
/// </returns>
ReadOnlySpan<MemberInfo> GetMembers(
Type type,
bool includeIgnored = false,
bool includeStatic = false);
bool includeStatic = false,
bool allowObject = false);

/// <summary>
/// Defines if a member shall be ignored. This method interprets ignore attributes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions src/HotChocolate/Core/src/Types/Types/InputObjectType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
namespace HotChocolate.Types;

/// <summary>
/// <para>
/// 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:
/// </para>
/// <para>In this example, an Input Object called Point2D describes x and y inputs:</para>
///
/// <code>
/// input Point2D {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
147 changes: 127 additions & 20 deletions src/HotChocolate/Core/test/Execution.Tests/SchemaFirstTests.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -13,9 +18,9 @@ public async Task BindObjectTypeImplicit()
var schema = SchemaBuilder.New()
.AddDocumentFromString(
@"type Query {
test: String
testProp: String
}")
test: String
testProp: String
}")
.AddResolver<Query>()
.Create();

Expand Down Expand Up @@ -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<EnumQuery>("Query")
.Create();

Expand Down Expand Up @@ -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<EnumQuery>("Query")
.AddResolver<Payload>()
.Create();
Expand All @@ -147,6 +154,71 @@ enum FooEnum {
result.MatchSnapshot();
}

/// <summary>
/// https://github.com/ChilliCream/graphql-platform/issues/5730
/// </summary>
[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<Query5730>("Query")
.AddResolver<Mutation5730>("Mutation")
.ExecuteRequestAsync(
"""
mutation {
changeChannelParameters(input: {
parameterChangeInfo: [ { value: { a: "b" } } ]
}) {
message
}
}
""");

result.MatchInlineSnapshot(
"""
{
"data": {
"changeChannelParameters": {
"message": "b"
}
}
}
""");
}

public class Query
{
public string GetTest()
Expand Down Expand Up @@ -199,4 +271,39 @@ public enum FooEnum
Baz,
BazBar
}
}

public class Query5730
{
public string Dummy => "Don't care";
}

public class Mutation5730
{
public Task<ChangeChannelParameterPayload> ChangeChannelParametersAsync(
ChangeChannelParameterInput input,
CancellationToken _)
{
var message = Assert.IsType<string>(
Assert.IsType<Dictionary<string, object>>(
input.ParameterChangeInfo[0].Value)["a"]);

return Task.FromResult(new ChangeChannelParameterPayload { Message = message });
}
}

public record ChangeChannelParameterInput
{
public ParameterValuePair[] ParameterChangeInfo { get; set; } =
Array.Empty<ParameterValuePair>();
}

public record ParameterValuePair
{
public object? Value { get; set; }
}

public record ChangeChannelParameterPayload
{
public string Message { get; init; } = string.Empty;
}
}