graphql-dotnet: InputObjectGraphType resolve function ignored

Description

By defining something like:

    public class CreateUserInputValue
    {
        public int? OtherName { get; set; }
    }

    public class CreateUserInput : InputObjectGraphType<CreateUserInputValue>
    {
        public CreateUserInput()
        {
            Field<IdGraphType>()
                .Name("id")
                .Resolve(context =>
                {
                    return context.Source.OtherName;
                });
        }
    }

it doesn’t seem like id is correctly mapped to OtherName.

Steps to reproduce

With the code above and by calling:

mutation {
  test(input:{id:1244}) {
    id
  }
}

with:

    public class Mutation : ObjectGraphType
    {
        public Mutation()
        {
            Field<NonNullGraphType<User>>()
                .Name("test")
                .Argument<NonNullGraphType<CreateUserInput>>("input", "")
                .ResolveAsync(async context =>
                {
                    var input = context.GetArgument<CreateUserInputValue>("input");
                    // ... breakpoint
                    return null;
                });
        }
    }

the OtherName value is null.

Expected result

I would expect OtherName to be 1244.

Actual result

OtherName is null.

Environment

<PackageReference Include="GraphQL" Version="2.4.0" />

About this issue

Original URL
State: open
Created 5 years ago
Reactions: 1
Comments: 44 (28 by maintainers)

Most upvoted comments

We are delaying the release of version 4. In the course of its preparation, errors were discovered that require fixing. In addition, we have added a few more features.

sungam3r on Feb 8, 2021

I’d like to explain, but I don’t understand it well enough yet. It really requires quite a bit of investigation along with time to determine the least number of changes in order to achieve the goal here.

Below is a rough draft of the changes I think are required.

When GraphQL deserializes a complex input type from a literal, something like this should happen:

InputObjectGraphType has a ParseLiteral and ParseValue method similar to scalars
ParseLiteral is called with some type of IResolveInputFieldContext parameter
The implementation can query the fields with something like GetField<> (similar to GetArgument<>), in order to populate the object it is creating with its child fields.
Calling GetField<> to get a child scalar calls ParseLiteral of the scalar, whereas calling GetField<> to get a child complex object calls ParseLiteral of the InputObjectGraphType.
Eventually the created object is returned. All of the above code will have a default implementation which works, hopefully, in the same manner that it does now. This should be easy for a InputObjectGraphType<T> - see ObjectExtensions.ToObject.

Then:

Calling GetArgument<> on a IResolveFieldContext runs the above code, and then if the return value is not of the type requested, it will attempt a conversion via the ValueConverter.

Similar changes are made for parsing variables (vs literals). All the variables can then be parsed a single time prior to execution of the document, rather than twice as it is now.

When all that is accomplished, we may wish to change how fields are defined on InputObjectGraphTypes, perhaps to remove the resolve parameter, as it will be the graph type that defines how to serialize the value into an object (similar to scalars), not a parameter on the field.

One significant bug this process will fix is the fact that currently when parsing variables, values get passed through both ParseValue and ParseLiteral. This should never occur, and will not occur with this new code.

It will also be much easier to understand, debug, and change how complex objects are deserialized from a literal or variables. It will also deduplicate a bunch of this related code which is spread into multiple areas of the project currently.

Another benefit is that when input union types are introduced into the spec, it should be a breeze to implement them.

As I said before, I think this consists of basically a complete rewrite of the input object parsing code, particularly for variables. It’s no small change. There will sure to be challenges and complications to overcome, and on top of everything, we must try to retain a certain degree of compatibility with existing code.

As to where to begin, that’s a tough question. I’m not really sure. Maybe the best idea is to just start rewriting ExecutionHelper.GetVariableValue from scratch and see where that leads. I started by trying to separate parsing scalar variables from literals but it wasn’t getting me anywhere.

Shane32 on Dec 17, 2020

Of course a Resolve Function per FIeld would be a fancy way to use it as well

As I said earlier, it requires a change in design and most likely change the name of this method so as not to introduce people to delusion. Resolve works ONLY for output types, not input ones.

sungam3r on Mar 26, 2021

See: https://graphql-dotnet.github.io/docs/migrations/migration4#input-object-custom-deserializers-aka-resolver

Shane32 on Mar 26, 2021

@mkhatuntcov-korewireless The new ParseDictionary method in #2165 should allow you to fully customize input object deserialization behavior. It does not use the Resolve field at the moment, but any functionality that would otherwise be within Resolve you can replicate within ParseDictionary. Is that satisfactory? I think in the end we will be removing the Resolve field from input object graphs, or perhaps replacing the signature with something appropriate to input graphs, but this will not be done for v4.

Can we close this issue? @sungam3r @mkhatuntcov-korewireless Do we want to keep it open?

Shane32 on Feb 24, 2021

I’m not sure, but it might help if #354 (or something similar) was completed first.

Shane32 on Dec 17, 2020

I’m planning, if I have time, to work this into version 4. At present I don’t have time. It will be a rather major rewrite.

Shane32 on Dec 17, 2020

@gitmustashio I’m trying to map a GraphQL input field to a specific C# input property which is not necessarily the one with a similar name.

shoe-diamente on Jul 1, 2019