graphql-dotnet / conventions

GraphQL Conventions Library for .NET
MIT License
233 stars 63 forks source link

The field 'field1' of an Input Object type 'InputObject' must not have Resolver set. #258

Closed fgroen closed 1 year ago

fgroen commented 1 year ago

When cloning this repository and upgrading the GraphQL.DataLoader to 7.4.0/7.4.1 errors pop up when running the unit tests. I can track them to this change in GraphQL.NET: https://github.com/graphql-dotnet/graphql-dotnet/pull/3574

I could use a bit of assistance to get this up and running again. Do you have some suggestions on how to solve this maybe?

    GraphQL.ExecutionError: Error executing document. The field 'field1' of an Input Object type 'InputObject' must not have Resolver set. You should set Resolver only for fields of object output types.
 ---> GraphQL.Conventions.Execution.FieldResolutionException: Error executing document. The field 'field1' of an Input Object type 'InputObject' must not have Resolver set. You should set Resolver only for fields of object output types.
 ---> System.InvalidOperationException: The field 'field1' of an Input Object type 'InputObject' must not have Resolver set. You should set Resolver only for fields of object output types.
   at GraphQL.Utilities.SchemaValidationVisitor.VisitInputObjectFieldDefinition(FieldType field, IInputObjectGraphType type, ISchema schema) in /_/src/GraphQL/Utilities/Visitors/SchemaValidationVisitor.cs:line 227
   at GraphQL.SchemaExtensions.Run(ISchemaNodeVisitor visitor, ISchema schema) in /_/src/GraphQL/Extensions/SchemaExtensions.cs:line 298
   at GraphQL.Types.Schema.Validate() in /_/src/GraphQL/Types/Schema.cs:line 489
   at GraphQL.Types.Schema.CreateAndInitializeSchemaTypes() in /_/src/GraphQL/Types/Schema.cs:line 459
   at GraphQL.Types.Schema.Initialize() in /_/src/GraphQL/Types/Schema.cs:line 196
   at GraphQL.DocumentExecuter.CoreExecuteAsync(ExecutionOptions options) in /_/src/GraphQL/Execution/DocumentExecuter.cs:line 100
   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---
sungam3r commented 1 year ago

See GraphTypeInfo.IsOutputType and GraphTypeInfo.IsInputType. First, they should be set properly. Second - GraphTypeAdapter.DeriveField should not set Resolver/StreamResolver for fieldInfo if fieldInfo.Type.IsInputType is true.

Shane32 commented 1 year ago

@sungam3r I am not happy that we have made breaking changes within v7. At minimum, we should have an opt-out for these types of breaking changes, or some way that the end-user can workaround any issue. We should not require a third-party library, such as the conventions project, to make changes during v7's lifetime, unless we are fixing a bug that demonstrably causes a problem. In this case, setting the resolver did no harm during execution.

Is there a workaround that can be provided to prevent the issue from occurring without a change in the conventions project? Is there a way to write a schema validation rule that executes after initialization of SchemaTypes and prior to the built-in schema initialization checks? Can the built-in schema initialization checks be bypassed? These answers should be provided within the migration notes or in a similarly-accessible location.

At the moment, the easiest way to bypass is likely (untested):

class MySchema : Schema
{
    //...
    protected override void Validate()
    {
        CoerceInputTypeDefaultValues();
        // note: skipping schema validation checks for conventions version _._._
    }
}
tlil commented 1 year ago

Ah, this auto-closed :) Merged @sungam3r's #259 – @fgroen, can you give that change a shot?

fgroen commented 1 year ago

Thanks @tlil / @sungam3r for this (very fast) fix! It looks good on our end. I will do some further testing with the rest, but this looks good to me!

sungam3r commented 1 year ago

These answers should be provided within the migration notes or in a similarly-accessible location.

I agree, in general. For now I posted #259 to fix this and move on.

fgroen commented 1 year ago

@tlil / @sungam3r Who is in charge of updating the latest release for conventions?

Shane32 commented 1 year ago

I can issue a release.

Shane32 commented 1 year ago

Released as 7.3.0: