Only root fields of subscriptions should set StreamResolver; only obj…

…ect output types should set Resolver (#3574)

docs2/site/docs/migrations/migration7.md

@@ -598,3 +598,12 @@ require them to resolve to GraphQL Object types. GraphQL.NET 7.2+ now enforces t
 Due to the above validation, resolvers defined on fields of interface types are never be used by
 GraphQL.NET. As such, resolvers are not generated for fields of `AutoRegisteringInterfaceGraphType`s
 in version 7.2+.
+
+### 18. `SchemaValidationVisitor` has more runtime checks
+
+Starting with version 7.4, the `StreamResolver` property should be set exclusively for root fields
+in subscriptions, while the `Resolver` property should only be assigned to object output types'
+fields. Fields within interface types and input object types should not set these
+properties. This modification may lead to potential disruptions in your applications, as the
+schema may trigger an exception during initialization. To eliminate this exception, simply
+cease assigning these properties for the fields specified in the exception message.

src/GraphQL.StarWars/Types/CharacterInterface.cs

@@ -10,12 +10,10 @@ public CharacterInterface()
         Name = "Character";
 
         Field<NonNullGraphType<StringGraphType>>("id")
-            .Description("The id of the character.")
-            .Resolve(context => context.Source.Id);
+            .Description("The id of the character.");
 
         Field<StringGraphType>("name")
-            .Description("The name of the character.")
-            .Resolve(context => context.Source.Name);
+            .Description("The name of the character.");
 
         Field<ListGraphType<CharacterInterface>>("friends");
         Field<ConnectionType<CharacterInterface, EdgeType<CharacterInterface>>>("friendsConnection");

src/GraphQL.Tests/Bugs/Bug1889.cs

@@ -58,7 +58,7 @@ public RGraphInterface()
     {
         Name = "RInterface";
 
-        Field<NonNullGraphType<StringGraphType>>("Value").Resolve(ctx => ctx.Source.Value);
+        Field<NonNullGraphType<StringGraphType>>("Value");
     }
 }
 
@@ -78,7 +78,7 @@ public AGraphInterface()
     {
         Name = "AInterface";
 
-        Field<NonNullGraphType<RGraphInterface>>("R").Resolve(ctx => ctx.Source.methodUsedToGetRValue());
+        Field<NonNullGraphType<RGraphInterface>>("R");
     }
 }
 

src/GraphQL.Tests/Initialization/SchemaInitializationTests.cs

@@ -97,11 +97,33 @@ public void Passing_GraphType_InsteadOf_ClrType_Should_Produce_Friendly_Error()
 
     // https://github.com/graphql-dotnet/graphql-dotnet/pull/3571
     [Fact]
-    public void Deprecate_Required_Arguments_And_Input_Fields_Produce_Friendly_Error()
+    public void Deprecate_Required_Arguments_And_Input_Fields_Should_Produce_Friendly_Error()
     {
         ShouldThrow<Issue3571Schema1, InvalidOperationException>("The required argument 'flag' of field 'MyQuery.str' has no default value so `@deprecated` directive must not be applied to this argument. To deprecate a required argument, it must first be made optional by either changing the type to nullable or adding a default value.");
         ShouldThrow<Issue3571Schema2, InvalidOperationException>("The required input field 'age' of an Input Object 'PersonInput' has no default value so `@deprecated` directive must not be applied to this input field. To deprecate an input field, it must first be made optional by either changing the type to nullable or adding a default value.");
     }
+
+    // https://github.com/graphql-dotnet/graphql-dotnet/issues/2994
+    [Fact]
+    public void StreamResolver_On_Wrong_Fields_Should_Produce_Friendly_Error()
+    {
+        ShouldThrow<SchemaWithFieldStreamResolverOnNonRootSubscriptionField, InvalidOperationException>("The field 'str' of an Object type 'MyQuery' must not have StreamResolver set. You should set StreamResolver only for the root fields of subscriptions.");
+        ShouldThrow<SchemaWithFieldStreamResolverOnFieldOfInterface, InvalidOperationException>("The field 'id' of an Interface type 'My' must not have StreamResolver set. You should set StreamResolver only for the root fields of subscriptions.");
+        ShouldThrow<SchemaWithFieldStreamResolverOnFieldOfInputObject, InvalidOperationException>("The field 'name' of an Input Object type 'PersonInput' must not have StreamResolver set. You should set StreamResolver only for the root fields of subscriptions.");
+    }
+
+    // https://github.com/graphql-dotnet/graphql-dotnet/issues/1176
+    [Fact]
+    public void Resolver_On_InputField_Should_Produce_Friendly_Error()
+    {
+        ShouldThrow<SchemaWithInputFieldResolver, InvalidOperationException>("The field 'name' of an Input Object type 'PersonInput' must not have Resolver set. You should set Resolver only for fields of object output types.");
+    }
+
+    [Fact]
+    public void Resolver_On_InterfaceField_Should_Produce_Friendly_Error()
+    {
+        ShouldThrow<SchemaWithFieldResolverOnFieldOfInterface, InvalidOperationException>("The field 'id' of an Interface type 'My' must not have Resolver set. Each interface is translated to a concrete type during request execution. You should set Resolver only for fields of object output types.");
+    }
 }
 
 public class EmptyQuerySchema : Schema
@@ -458,3 +480,111 @@ private class Person
         public int Age { get; set; }
     }
 }
+
+public class SchemaWithFieldStreamResolverOnNonRootSubscriptionField : Schema
+{
+    public SchemaWithFieldStreamResolverOnNonRootSubscriptionField()
+    {
+        var type = new ObjectGraphType { Name = "MyQuery" };
+        type.Field<StringGraphType>("str")
+            .ResolveStream(_ => new Subscription.SampleObservable<string>())
+            .Resolve(_ => "abc");
+        Query = type;
+    }
+}
+
+public class SchemaWithFieldStreamResolverOnFieldOfInterface : Schema
+{
+    public SchemaWithFieldStreamResolverOnFieldOfInterface()
+    {
+        var type = new ObjectGraphType { Name = "MyQuery" };
+        type.Field<MyInterface>("hero");
+        Query = type;
+    }
+
+    private class MyInterface : InterfaceGraphType
+    {
+        public MyInterface()
+        {
+            Name = "My";
+
+            Field<StringGraphType>("id").ResolveStream(_ => new Subscription.SampleObservable<string>());
+        }
+    }
+}
+
+public class SchemaWithFieldResolverOnFieldOfInterface : Schema
+{
+    public SchemaWithFieldResolverOnFieldOfInterface()
+    {
+        var type = new ObjectGraphType { Name = "MyQuery" };
+        type.Field<MyInterface>("hero").Resolve(_ => null);
+        Query = type;
+    }
+
+    private class MyInterface : InterfaceGraphType
+    {
+        public MyInterface()
+        {
+            Name = "My";
+
+            Field<StringGraphType>("id").Resolve(_ => "abc");
+        }
+    }
+}
+
+public class SchemaWithFieldStreamResolverOnFieldOfInputObject : Schema
+{
+    public SchemaWithFieldStreamResolverOnFieldOfInputObject()
+    {
+        var type = new ObjectGraphType { Name = "MyQuery" };
+        type.Field<StringGraphType>("str")
+            .Argument<PersonInput>("person")
+            .Resolve(_ => "abc");
+        Query = type;
+    }
+
+    private class PersonInput : InputObjectGraphType<Person>
+    {
+        public PersonInput()
+        {
+            Field(x => x.Name).ResolveStream(_ => new Subscription.SampleObservable<string>());
+            Field(x => x.Age);
+        }
+    }
+
+    private class Person
+    {
+        public string Name { get; set; }
+
+        public int Age { get; set; }
+    }
+}
+
+public class SchemaWithInputFieldResolver : Schema
+{
+    public SchemaWithInputFieldResolver()
+    {
+        var type = new ObjectGraphType { Name = "MyQuery" };
+        type.Field<StringGraphType>("str")
+            .Argument<PersonInput>("person")
+            .Resolve(_ => "abc");
+        Query = type;
+    }
+
+    private class PersonInput : InputObjectGraphType<Person>
+    {
+        public PersonInput()
+        {
+            Field(x => x.Name).Resolve(_ => "abc");
+            Field(x => x.Age);
+        }
+    }
+
+    private class Person
+    {
+        public string Name { get; set; }
+
+        public int Age { get; set; }
+    }
+}

src/GraphQL.Tests/Types/AutoRegisteringInterfaceGraphTypeTests.cs

@@ -270,7 +270,7 @@ public void Argument_VerifyType(string fieldName, string argumentName, Type? arg
     [InlineData(nameof(ArgumentTestsInterface.IdIntArg), "arg1", 123, null)]
     [InlineData(nameof(ArgumentTestsInterface.TypedArg), "arg1", "123", null)]
     [InlineData(nameof(ArgumentTestsInterface.MultipleArgs), "arg1", "hello", 123)]
-    public async Task Argument_ResolverTests(string fieldName, string arg1Name, object? arg1Value, int? arg2Value)
+    public void Argument_ResolverTests(string fieldName, string arg1Name, object? arg1Value, int? arg2Value)
     {
         var graphType = new AutoRegisteringInterfaceGraphType<ArgumentTestsInterface>();
         var fieldType = graphType.Fields.Find(fieldName).ShouldNotBeNull();
@@ -294,9 +294,7 @@ public async Task Argument_ResolverTests(string fieldName, string arg1Name, obje
         serviceCollection.AddSingleton<string>("testService");
         using var provider = serviceCollection.BuildServiceProvider();
         context.RequestServices = provider;
-        fieldType.Resolver.ShouldNotBeNull();
-        var ex = await Should.ThrowAsync<InvalidOperationException>(async () => await fieldType.Resolver!.ResolveAsync(context).ConfigureAwait(false)).ConfigureAwait(false);
-        ex.Message.ShouldBe("This field resolver should never be called. It is only used to prevent the default field resolver from being used.");
+        fieldType.Resolver.ShouldBeNull();
     }
 
     [Fact]
@@ -342,29 +340,11 @@ public void CanOverrideFieldList()
     [InlineData("Field4")]
     [InlineData("Field6AltName")]
     [InlineData("Field7")]
-    public async Task FieldResolvers_Invalid(string fieldName)
+    public void FieldResolvers_Of_InterfaceGraphType_ShouldBe_Null(string fieldName)
     {
         var graph = new AutoRegisteringInterfaceGraphType<TestInterface>();
         var field = graph.Fields.Find(fieldName).ShouldNotBeNull();
-        var resolver = field.Resolver.ShouldNotBeNull();
-        var obj = new TestClass();
-        var ex = await Should.ThrowAsync<InvalidOperationException>(async () => await resolver.ResolveAsync(new ResolveFieldContext
-        {
-            Source = obj,
-            FieldDefinition = new FieldType { Name = fieldName },
-        }).ConfigureAwait(false)).ConfigureAwait(false);
-        ex.Message.ShouldBe("This field resolver should never be called. It is only used to prevent the default field resolver from being used.");
-    }
-
-    [Fact]
-    public async Task ThrowsWhenSourceNull()
-    {
-        var graphType = new AutoRegisteringInterfaceGraphType<NullSourceFailureTest>();
-        var context = new ResolveFieldContext();
-        (await Should.ThrowAsync<InvalidOperationException>(async () => await graphType.Fields.Find("Example1")!.Resolver!.ResolveAsync(context).ConfigureAwait(false)).ConfigureAwait(false))
-            .Message.ShouldBe("This field resolver should never be called. It is only used to prevent the default field resolver from being used.");
-        (await Should.ThrowAsync<InvalidOperationException>(async () => await graphType.Fields.Find("Example2")!.Resolver!.ResolveAsync(context).ConfigureAwait(false)).ConfigureAwait(false))
-            .Message.ShouldBe("This field resolver should never be called. It is only used to prevent the default field resolver from being used.");
+        field.Resolver.ShouldBeNull();
     }
 
     [Fact]
@@ -664,12 +644,6 @@ private interface NoDefaultConstructorTestInterface
         string Example2();
     }
 
-    private interface NullSourceFailureTest
-    {
-        bool Example1 { get; set; }
-        string Example2();
-    }
-
     private interface FieldTests
     {
         [Name("Test1")]
@@ -800,17 +774,6 @@ protected override FieldType CreateField(MemberInfo memberInfo)
         }
     }
 
-    private class TestClass : TestInterface
-    {
-        public int Field1 { get; set; } = 1;
-        public int Field2 => 2;
-        public int Field3 { set { } }
-        public int Field4() => 4;
-        [Name("Field6AltName")]
-        public int Field6 => 6;
-        public Task<int> Field7 => Task.FromResult(7);
-    }
-
     private interface TestInterface
     {
         int Field1 { get; set; }

src/GraphQL/Types/Collections/SchemaTypes.cs

@@ -469,9 +469,9 @@ public void ApplyMiddleware(Func<FieldMiddlewareDelegate, FieldMiddlewareDelegat
 
             foreach (var item in Dictionary)
             {
-                if (item.Value is IComplexGraphType complex)
+                if (item.Value is IObjectGraphType obj)
                 {
-                    foreach (var field in complex.Fields.List)
+                    foreach (var field in obj.Fields.List)
                     {
                         var inner = field.Resolver ?? (field.StreamResolver == null ? NameFieldResolver.Instance : SourceFieldResolver.Instance);
 

src/GraphQL/Types/Composite/AutoRegisteringOutputHelper.cs

@@ -9,9 +9,6 @@ namespace GraphQL.Types;
 /// </summary>
 internal static class AutoRegisteringOutputHelper
 {
-    private static readonly IFieldResolver _invalidFieldResolver = new FuncFieldResolver<object?>(_ => throw new InvalidOperationException("This field resolver should never be called. It is only used to prevent the default field resolver from being used."));
-    private static readonly ISourceStreamResolver _invalidStreamResolver = new SourceStreamResolver<object?>((Func<IResolveFieldContext, IObservable<object?>>)(_ => throw new InvalidOperationException("This source stream resolver should never be called. It is only used to prevent the default source stream resolver from being used.")));
-
     /// <summary>
     /// Configures query arguments and a field resolver for the specified <see cref="FieldType"/>, overwriting
     /// any existing configuration within <see cref="FieldType.Arguments"/>, <see cref="FieldType.Resolver"/>
@@ -104,8 +101,9 @@ public static void BuildFieldType(
         }
         if (buildMemberInstanceExpressionFunc == null)
         {
-            fieldType.Resolver = _invalidFieldResolver;
-            fieldType.StreamResolver = _invalidStreamResolver;
+            // interface types do not set resolvers
+            fieldType.Resolver = null;
+            fieldType.StreamResolver = null;
         }
     }
 

src/GraphQL/Types/Composite/ComplexGraphType.cs

@@ -510,11 +510,13 @@ public virtual FieldBuilder<TSourceType, TProperty> Field<TProperty>(
 
             var builder = CreateBuilder<TProperty>(type)
                 .Name(name)
-                .Resolve(new ExpressionFieldResolver<TSourceType, TProperty>(expression))
                 .Description(expression.DescriptionOf())
                 .DeprecationReason(expression.DeprecationReasonOf())
                 .DefaultValue(expression.DefaultValueOf());
 
+            if (this is IObjectGraphType)
+                builder.Resolve(new ExpressionFieldResolver<TSourceType, TProperty>(expression));
+
             if (expression.Body is MemberExpression expr)
             {
                 builder.FieldType.Metadata[ORIGINAL_EXPRESSION_PROPERTY_NAME] = expr.Member.Name;

src/GraphQL/Types/Fields/FieldType.cs

@@ -77,7 +77,7 @@ public Type? Type
         public IFieldResolver? Resolver { get; set; }
 
         /// <summary>
-        /// Gets or sets a subscription resolver for the field. Only applicable to subscription fields.
+        /// Gets or sets a subscription resolver for the field. Only applicable to the root fields of subscription.
         /// </summary>
         public ISourceStreamResolver? StreamResolver { get; set; }
     }

src/GraphQL/Utilities/Visitors/SchemaValidationVisitor.cs

@@ -63,6 +63,9 @@ public override void VisitObjectFieldDefinition(FieldType field, IObjectGraphTyp
                 throw new InvalidOperationException($"The field '{field.Name}' of an Object type '{type.Name}' must be an output type.");
 
             ValidateFieldArgumentsUniqueness(field, type);
+
+            if (field.StreamResolver != null && type != schema.Subscription)
+                throw new InvalidOperationException($"The field '{field.Name}' of an Object type '{type.Name}' must not have StreamResolver set. You should set StreamResolver only for the root fields of subscriptions.");
         }
 
         /// <inheritdoc/>
@@ -129,6 +132,12 @@ public override void VisitInterfaceFieldDefinition(FieldType field, IInterfaceGr
                 throw new InvalidOperationException($"The field '{field.Name}' of an Interface type '{type.Name}' must be an output type.");
 
             ValidateFieldArgumentsUniqueness(field, type);
+
+            if (field.StreamResolver != null && type != schema.Subscription)
+                throw new InvalidOperationException($"The field '{field.Name}' of an Interface type '{type.Name}' must not have StreamResolver set. You should set StreamResolver only for the root fields of subscriptions.");
+
+            if (field.Resolver != null)
+                throw new InvalidOperationException($"The field '{field.Name}' of an Interface type '{type.Name}' must not have Resolver set. Each interface is translated to a concrete type during request execution. You should set Resolver only for fields of object output types.");
         }
 
         /// <inheritdoc/>
@@ -210,6 +219,12 @@ public override void VisitInputObjectFieldDefinition(FieldType field, IInputObje
             // 2.4
             if (field.ResolvedType is NonNullGraphType && field.DefaultValue is null && field.DeprecationReason is not null)
                 throw new InvalidOperationException($"The required input field '{field.Name}' of an Input Object '{type.Name}' has no default value so `@deprecated` directive must not be applied to this input field. To deprecate an input field, it must first be made optional by either changing the type to nullable or adding a default value.");
+
+            if (field.StreamResolver != null)
+                throw new InvalidOperationException($"The field '{field.Name}' of an Input Object type '{type.Name}' must not have StreamResolver set. You should set StreamResolver only for the root fields of subscriptions.");
+
+            if (field.Resolver != null)
+                throw new InvalidOperationException($"The field '{field.Name}' of an Input Object type '{type.Name}' must not have Resolver set. You should set Resolver only for fields of object output types.");
         }
 
         #endregion