Fixed issue where types were merged incorrectly (#3554)

michaelstaib · web-flow · commit 002da3a2bf86 · 2021-04-20T11:54:34.000+02:00
diff --git a/src/HotChocolate/Core/src/Types/Configuration/TypeInitializer.cs b/src/HotChocolate/Core/src/Types/Configuration/TypeInitializer.cs
@@ -525,15 +525,12 @@ private void CompileResolvers()
                 RegisteredResolver registered = item.Value;
                 if (registered.Field is FieldMember member)
                 {
-                    ResolverDescriptor descriptor =
-                        registered.IsSourceResolver
-                            ? new ResolverDescriptor(
-                                registered.SourceType,
-                                member)
-                            : new ResolverDescriptor(
-                                registered.ResolverType,
-                                registered.SourceType,
-                                member);
+                    ResolverDescriptor descriptor = new(
+                        registered.SourceType,
+                        member,
+                        resolverType: registered.IsSourceResolver
+                            ? null 
+                            : registered.ResolverType);
                     _resolvers[item.Key] = registered.WithField(
                         ResolverCompiler.Resolve.Compile(descriptor));
                 }
diff --git a/src/HotChocolate/Core/src/Types/Resolvers/Expressions/ResolverDescriptor.cs b/src/HotChocolate/Core/src/Types/Resolvers/Expressions/ResolverDescriptor.cs
@@ -1,39 +1,44 @@
 ﻿using System;
 
+#nullable enable
+
 namespace HotChocolate.Resolvers.Expressions
 {
     /// <summary>
     /// Describes a resolver that is based on a resolver type.
     /// </summary>
     internal class ResolverDescriptor
     {
+        /// <summary>
+        /// Creates a new instance of <see cref="ResolverType"/>
+        /// </summary>
         public ResolverDescriptor(
-            Type resolverType,
             Type sourceType,
-            FieldMember field)
+            FieldMember field,
+            Type? resolverType = null)
         {
-            ResolverType = resolverType
-                ?? throw new ArgumentNullException(nameof(resolverType));
             SourceType = sourceType
                 ?? throw new ArgumentNullException(nameof(sourceType));
             Field = field
                 ?? throw new ArgumentNullException(nameof(field));
-        }
 
-        public ResolverDescriptor(
-            Type sourceType,
-            FieldMember field)
-        {
-            SourceType = sourceType
-                ?? throw new ArgumentNullException(nameof(sourceType));
-            Field = field
-                ?? throw new ArgumentNullException(nameof(field));
+            ResolverType = resolverType == typeof(object) ? null : resolverType;
         }
 
-        public Type ResolverType { get; }
+        /// <summary>
+        /// Gets the resolver type.
+        /// If a resolver type is the <see cref="Field"/> belongs to this type.
+        /// </summary>
+        public Type? ResolverType { get; }
 
+        /// <summary>
+        /// Gets the source type aka runtime type of a GraphQL type.
+        /// </summary>
         public Type SourceType { get; }
 
+        /// <summary>
+        /// Gets the member that shall be compiled to a resolver.
+        /// </summary>
         public FieldMember Field { get; }
     }
 }
diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Definitions/ObjectTypeDefinition.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Definitions/ObjectTypeDefinition.cs
@@ -39,7 +39,7 @@ public override Type RuntimeType
         public bool IsExtension { get; set; }
 
         public IList<ITypeReference> Interfaces =>
-            _interfaces ??=new List<ITypeReference>();
+            _interfaces ??= new List<ITypeReference>();
 
         public IBindableList<ObjectFieldDefinition> Fields { get; } =
             new BindableList<ObjectFieldDefinition>();
@@ -168,14 +168,14 @@ protected internal void MergeInto(ObjectTypeDefinition target)
 
                 if (removeField)
                 {
-                    if(targetField is not null)
+                    if (targetField is not null)
                     {
                         target.Fields.Remove(targetField);
                     }
                 }
                 else if (targetField is null || replaceField)
                 {
-                    if(targetField is not null)
+                    if (targetField is not null)
                     {
                         target.Fields.Remove(targetField);
                     }
@@ -184,21 +184,33 @@ protected internal void MergeInto(ObjectTypeDefinition target)
                     field.CopyTo(newField);
                     newField.SourceType = target.RuntimeType;
 
-                    if (newField.Member is not null && newField.ResolverMember is null)
-                    {
-                        newField.ResolverMember = newField.Member;
-                        newField.Member = targetField?.Member;
-                    }
+                    SetResolverMember(newField, targetField);
 
                     target.Fields.Add(newField);
                 }
                 else
                 {
+                    SetResolverMember(field, targetField);
                     field.MergeInto(targetField);
                 }
             }
 
             target.IsOfType ??= IsOfType;
         }
+
+        private static void SetResolverMember(
+            ObjectFieldDefinition sourceField,
+            ObjectFieldDefinition? targetField)
+        {
+            // we prepare the field that is merged in to use the resolver member instead of member.
+            // this will ensure that the original source type member is preserved after we have
+            // merged the type extensions.
+
+            if (sourceField.Member is not null && sourceField.ResolverMember is null)
+            {
+                sourceField.ResolverMember = sourceField.Member;
+                sourceField.Member = targetField?.Member;
+            }
+        }
     }
 }
diff --git a/src/HotChocolate/Core/src/Types/Types/Relay/Descriptors/NodeDescriptorBase.cs b/src/HotChocolate/Core/src/Types/Types/Relay/Descriptors/NodeDescriptorBase.cs
@@ -69,9 +69,9 @@ public IObjectFieldDescriptor ResolveNodeWith<TResolver>(
                 FieldResolver resolver =
                     ResolverCompiler.Resolve.Compile(
                         new ResolverDescriptor(
-                            typeof(TResolver),
                             typeof(object),
-                            new FieldMember("_", "_", m)));
+                            new FieldMember("_", "_", m),
+                            resolverType: typeof(TResolver)));
                 return ResolveNode(resolver.Resolver);
             }
 
@@ -90,9 +90,9 @@ public IObjectFieldDescriptor ResolveNodeWith(MethodInfo method)
             FieldResolver resolver =
                 ResolverCompiler.Resolve.Compile(
                     new ResolverDescriptor(
-                        method.DeclaringType ?? typeof(object),
                         typeof(object),
-                        new FieldMember("_", "_", method)));
+                        new FieldMember("_", "_", method),
+                        resolverType: method.DeclaringType ?? typeof(object)));
 
             return ResolveNode(resolver.Resolver);
         }
diff --git a/src/HotChocolate/Core/src/Types/Types/Relay/Descriptors/NodeDescriptor~2.cs b/src/HotChocolate/Core/src/Types/Types/Relay/Descriptors/NodeDescriptor~2.cs
@@ -71,9 +71,9 @@ public IObjectFieldDescriptor ResolveNodeWith<TResolver>(
                 FieldResolver resolver =
                     ResolverCompiler.Resolve.Compile(
                         new ResolverDescriptor(
-                            typeof(TResolver),
                             typeof(object),
-                            new FieldMember("_", "_", m)));
+                            new FieldMember("_", "_", m),
+                            resolverType: typeof(TResolver)));
                 return ResolveNode(resolver.Resolver);
             }
 
@@ -93,9 +93,9 @@ public IObjectFieldDescriptor ResolveNodeWith(
             FieldResolver resolver =
                 ResolverCompiler.Resolve.Compile(
                     new ResolverDescriptor(
-                        method.DeclaringType ?? typeof(object),
                         typeof(object),
-                        new FieldMember("_", "_", method)));
+                        new FieldMember("_", "_", method),
+                        resolverType: method.DeclaringType ?? typeof(object)));
             return ResolveNode(resolver.Resolver);
         }
 
diff --git a/src/HotChocolate/Core/test/Types.Tests/Resolvers/ResolverDescriptorTests.cs b/src/HotChocolate/Core/test/Types.Tests/Resolvers/ResolverDescriptorTests.cs
@@ -0,0 +1,53 @@
+using System.Linq;
+using HotChocolate.Resolvers.Expressions;
+using Xunit;
+
+namespace HotChocolate.Resolvers
+{
+    public class ResolverDescriptorTests
+    {
+        [Fact]
+        public void Create_With_ResolverType_Object()
+        {
+            var descriptor = new ResolverDescriptor(
+                typeof(string),
+                new FieldMember("a", "b",  typeof(object).GetMembers().First()),
+                resolverType: typeof(object));
+
+            Assert.Equal(typeof(string), descriptor.SourceType);
+            Assert.Null(descriptor.ResolverType);
+            Assert.NotNull(descriptor.Field.Member);
+            Assert.Equal("a", descriptor.Field.TypeName.Value);
+            Assert.Equal("b", descriptor.Field.FieldName.Value);
+        }
+
+        [Fact]
+        public void Create_With_ResolverType_Null()
+        {
+            var descriptor = new ResolverDescriptor(
+                typeof(string),
+                new FieldMember("a", "b",  typeof(object).GetMembers().First()));
+
+            Assert.Equal(typeof(string), descriptor.SourceType);
+            Assert.Null(descriptor.ResolverType);
+            Assert.NotNull(descriptor.Field.Member);
+            Assert.Equal("a", descriptor.Field.TypeName.Value);
+            Assert.Equal("b", descriptor.Field.FieldName.Value);
+        }
+
+        [Fact]
+        public void Create_With_ResolverType_Int()
+        {
+            var descriptor = new ResolverDescriptor(
+                typeof(string),
+                new FieldMember("a", "b",  typeof(object).GetMembers().First()),
+                resolverType: typeof(int));
+
+            Assert.Equal(typeof(string), descriptor.SourceType);
+            Assert.Equal(typeof(int), descriptor.ResolverType);
+            Assert.NotNull(descriptor.Field.Member);
+            Assert.Equal("a", descriptor.Field.TypeName.Value);
+            Assert.Equal("b", descriptor.Field.FieldName.Value);
+        }
+    }
+}
diff --git a/src/HotChocolate/Core/test/Types.Tests/Resolvers/ResolverPropertyGeneratorTests.cs b/src/HotChocolate/Core/test/Types.Tests/Resolvers/ResolverPropertyGeneratorTests.cs
@@ -265,9 +265,9 @@ public async Task Compile_TaskObjMethod_NoParams_Resolver()
             Type type = typeof(Resolvers);
             MemberInfo resolverMember = type.GetMethod(nameof(Resolvers.ObjectTaskResolver));
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -287,9 +287,9 @@ public async Task Compile_TaskStringMethod_NoParams_Resolver()
             Type type = typeof(Resolvers);
             MemberInfo resolverMember = type.GetMethod(nameof(Resolvers.StringTaskResolver));
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -310,9 +310,9 @@ public async Task Compile_TaskStringMethod_WithParams_Resolver()
             MemberInfo resolverMember =
                 type.GetMethod(nameof(Resolvers.StringTaskResolverWithArg));
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -333,9 +333,9 @@ public async Task Compile_ObjMethod_NoParams_Resolver()
             Type type = typeof(Resolvers);
             MemberInfo resolverMember = type.GetMethod(nameof(Resolvers.ObjectResolver));
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -355,9 +355,9 @@ public async Task Compile_StringMethod_NoParams_Resolver()
             Type type = typeof(Resolvers);
             MemberInfo resolverMember = type.GetMethod(nameof(Resolvers.StringResolver));
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -378,9 +378,9 @@ public async Task Compile_StringMethod_WithParams_Resolver()
             MemberInfo resolverMember =
                 type.GetMethod(nameof(Resolvers.StringResolverWithArg));
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -402,9 +402,9 @@ public async Task Compile_ObjTaskProperty_Resolver()
             MemberInfo resolverMember =
                 type.GetProperty("ObjectTaskStringProp");
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -425,9 +425,9 @@ public async Task Compile_StringTaskProperty_Resolver()
             MemberInfo resolverMember =
                 type.GetProperty("StringTaskResolverProp");
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
@@ -448,9 +448,9 @@ public async Task Compile_StringProperty_Resolver()
             MemberInfo resolverMember =
                 type.GetProperty("StringProp");
             var resolverDescriptor = new ResolverDescriptor(
-                type,
                 typeof(Entity),
-                new FieldMember("A", "b", resolverMember!));
+                new FieldMember("A", "b", resolverMember!),
+                resolverType: type);
 
             // act
             var compiler = new ResolveCompiler();
diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/ObjectTypeExtensionTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/ObjectTypeExtensionTests.cs
@@ -588,6 +588,20 @@ public async Task Replace_Field_With_The_Same_Name_Execute()
                 .MatchSnapshotAsync();
         }
 
+        [Fact]
+        public async Task Extended_Field_Overwrites_Extended_Field()
+        {
+            Snapshot.FullName();
+
+            await new ServiceCollection()
+                .AddGraphQL()
+                .AddQueryType()
+                .AddTypeExtension<ExtensionA>()
+                .AddTypeExtension<ExtensionB>()
+                .ExecuteRequestAsync("{ foo }")
+                .MatchSnapshotAsync();
+        }
+
         public class FooType
             : ObjectType<Foo>
         {
@@ -843,5 +857,17 @@ public class Replace_Field_PersonResolvers_2
             public string SomeId([Parent] IPersonDto dto, string arg = "abc") =>
                 dto.SomeId() + arg;
         }
+
+        [ExtendObjectType(OperationTypeNames.Query)]
+        public class ExtensionA
+        {
+            public string Foo() => "abc";
+        }
+
+        [ExtendObjectType(OperationTypeNames.Query)]
+        public class ExtensionB
+        {
+            public string Foo() => "def";
+        }
     }
 }
diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/__snapshots__/ObjectTypeExtensionTests.Extended_Field_Overwrites_Extended_Field.snap b/src/HotChocolate/Core/test/Types.Tests/Types/__snapshots__/ObjectTypeExtensionTests.Extended_Field_Overwrites_Extended_Field.snap

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public override Type RuntimeType`
`39`	`39`	`public bool IsExtension { get; set; }`
`40`	`40`
`41`	`41`	`public IList<ITypeReference> Interfaces =>`
`42`		`- _interfaces ??=new List<ITypeReference>();`
	`42`	`+ _interfaces ??= new List<ITypeReference>();`
`43`	`43`
`44`	`44`	`public IBindableList<ObjectFieldDefinition> Fields { get; } =`
`45`	`45`	`new BindableList<ObjectFieldDefinition>();`
`@@ -168,14 +168,14 @@ protected internal void MergeInto(ObjectTypeDefinition target)`
`168`	`168`
`169`	`169`	`if (removeField)`
`170`	`170`	`{`
`171`		`- if(targetField is not null)`
	`171`	`+ if (targetField is not null)`
`172`	`172`	`{`
`173`	`173`	`target.Fields.Remove(targetField);`
`174`	`174`	`}`
`175`	`175`	`}`
`176`	`176`	`else if (targetField is null \|\| replaceField)`
`177`	`177`	`{`
`178`		`- if(targetField is not null)`
	`178`	`+ if (targetField is not null)`
`179`	`179`	`{`
`180`	`180`	`target.Fields.Remove(targetField);`
`181`	`181`	`}`
`@@ -184,21 +184,33 @@ protected internal void MergeInto(ObjectTypeDefinition target)`
`184`	`184`	`field.CopyTo(newField);`
`185`	`185`	`newField.SourceType = target.RuntimeType;`
`186`	`186`
`187`		`- if (newField.Member is not null && newField.ResolverMember is null)`
`188`		`- {`
`189`		`- newField.ResolverMember = newField.Member;`
`190`		`- newField.Member = targetField?.Member;`
`191`		`- }`
	`187`	`+ SetResolverMember(newField, targetField);`
`192`	`188`
`193`	`189`	`target.Fields.Add(newField);`
`194`	`190`	`}`
`195`	`191`	`else`
`196`	`192`	`{`
	`193`	`+ SetResolverMember(field, targetField);`
`197`	`194`	`field.MergeInto(targetField);`
`198`	`195`	`}`
`199`	`196`	`}`
`200`	`197`
`201`	`198`	`target.IsOfType ??= IsOfType;`
`202`	`199`	`}`
	`200`	`+`
	`201`	`+ private static void SetResolverMember(`
	`202`	`+ ObjectFieldDefinition sourceField,`
	`203`	`+ ObjectFieldDefinition? targetField)`
	`204`	`+ {`
	`205`	`+ // we prepare the field that is merged in to use the resolver member instead of member.`
	`206`	`+ // this will ensure that the original source type member is preserved after we have`
	`207`	`+ // merged the type extensions.`
	`208`	`+`
	`209`	`+ if (sourceField.Member is not null && sourceField.ResolverMember is null)`
	`210`	`+ {`
	`211`	`+ sourceField.ResolverMember = sourceField.Member;`
	`212`	`+ sourceField.Member = targetField?.Member;`
	`213`	`+ }`
	`214`	`+ }`
`203`	`215`	`}`
`204`	`216`	`}`
-Original file line number
+Diff line change
 +{
 +  "data": {
 +    "foo": "def"
 +  }
 +}