From 68b6f00f7389cf06bf683474f5c14afa14d91bd1 Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Fri, 5 Jun 2026 14:00:22 +0200 Subject: [PATCH] Respect [ID] typename on interface fields --- .../Types/Relay/Attributes/IDAttribute.cs | 4 +- .../Extensions/RelayIdFieldExtensions.cs | 19 +- .../Relay/Extensions/RelayIdFieldHelpers.cs | 117 ++++++ .../Types/Relay/IdAttributeTests.cs | 353 ++++++++++++++++++ .../Types/Relay/IdDescriptorTests.cs | 94 +++++ ...e_Member_Still_Rewrites_Type_To_ID.graphql | 15 + ...Interface_Forwards_TypeName_To_SDL.graphql | 28 ++ 7 files changed, 626 insertions(+), 4 deletions(-) create mode 100644 src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Abstract_Interface_Member_Still_Rewrites_Type_To_ID.graphql create mode 100644 src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Interface_Forwards_TypeName_To_SDL.graphql diff --git a/src/HotChocolate/Core/src/Types/Types/Relay/Attributes/IDAttribute.cs b/src/HotChocolate/Core/src/Types/Types/Relay/Attributes/IDAttribute.cs index eed4507d159..cfd5da96f18 100644 --- a/src/HotChocolate/Core/src/Types/Types/Relay/Attributes/IDAttribute.cs +++ b/src/HotChocolate/Core/src/Types/Types/Relay/Attributes/IDAttribute.cs @@ -104,7 +104,7 @@ protected internal override void TryConfigure( d.ID(TypeName); break; case IInterfaceFieldDescriptor d: - d.ID(); + d.ID(TypeName); break; } } @@ -188,7 +188,7 @@ protected internal override void TryConfigure( d.ID(); break; case IInterfaceFieldDescriptor d: - d.ID(); + d.ID(); break; } } diff --git a/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldExtensions.cs b/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldExtensions.cs index fb39ff1f0d8..4a758e2b795 100644 --- a/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldExtensions.cs +++ b/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldExtensions.cs @@ -129,9 +129,24 @@ public static IObjectFieldDescriptor ID(this IObjectFieldDescriptor descripto /// /// the descriptor - public static IInterfaceFieldDescriptor ID(this IInterfaceFieldDescriptor descriptor) + /// + /// Sets the type name of the relay id + /// + public static IInterfaceFieldDescriptor ID( + this IInterfaceFieldDescriptor descriptor, + string? typeName = null) + { + RelayIdFieldHelpers.ApplyIdToField(descriptor, typeName); + return descriptor; + } + + /// + /// + /// the type from which the type name is derived + /// + public static IInterfaceFieldDescriptor ID(this IInterfaceFieldDescriptor descriptor) { - RelayIdFieldHelpers.ApplyIdToField(descriptor); + RelayIdFieldHelpers.ApplyIdToField(descriptor); return descriptor; } } diff --git a/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldHelpers.cs b/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldHelpers.cs index a4eddab256f..9199cee3450 100644 --- a/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldHelpers.cs +++ b/src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldHelpers.cs @@ -1,4 +1,5 @@ using System.Collections; +using System.Reflection; using HotChocolate.Configuration; using HotChocolate.Features; using HotChocolate.Internal; @@ -85,6 +86,104 @@ internal static void ApplyIdToField( configuration.Tasks.Add(configurationTask); } + /// + /// Applies the .ID() to an interface + /// field descriptor + /// + /// + /// You most likely want to call `.ID()` directly and do not use this helper + /// + internal static void ApplyIdToInterfaceField( + InterfaceFieldConfiguration configuration, + NodeIdNameDefinitionUnion? nameDefinition = null, + TypeReference? dependsOn = null) + { + // The interface field has no executable pipeline of its own. We build the serializer at + // the interface field's completion and let it be copied onto an implementing object field + // that inherits the field. The interface type always completes before the implementing + // object type's fields are materialized, so the formatter is in place to be copied. + var configurationTask = new OnCompleteTypeSystemConfigurationTask( + (ctx, def) => AddSerializerToInterfaceField( + ctx, + (InterfaceFieldConfiguration)def, + nameDefinition), + configuration, + ApplyConfigurationOn.BeforeCompletion, + typeReference: dependsOn); + + configuration.Tasks.Add(configurationTask); + } + + private static void AddSerializerToInterfaceField( + ITypeCompletionContext completionContext, + InterfaceFieldConfiguration configuration, + NodeIdNameDefinitionUnion? nameDefinition) + { + // We only encode the value with the interface's type name when the value is produced by + // the interface itself (an explicit resolver or a default implementation that is inherited + // by the implementing type). When the interface member is abstract the implementing type + // supplies the value and must opt into the global id encoding by declaring its own ID field. + if (!ResolvesOnInterface(configuration)) + { + return; + } + + var typeInspector = completionContext.TypeInspector; + IExtendedType? resultType; + + if (configuration.ResultType is not null) + { + resultType = typeInspector.GetType(configuration.ResultType); + } + else if ((configuration.ResolverMember ?? configuration.Member) is { } member) + { + resultType = typeInspector.GetReturnType(member, true); + } + else if (configuration.Type is ExtendedTypeReference typeReference) + { + resultType = typeReference.Type; + } + else + { + throw ThrowHelper.RelayIdFieldHelpers_NoFieldType( + configuration.Name, + completionContext.Type); + } + + var serializerAccessor = completionContext.DescriptorContext.NodeIdSerializerAccessor; + + var typeName = GetIdTypeName(completionContext, nameDefinition, typeInspector); + typeName ??= completionContext.Type.Name; + SetSerializerInfos(completionContext.DescriptorContext, typeName, resultType); + + // The formatter captures only the type name and result type, so the same instance can be + // copied to and shared by every implementing object field that inherits this field. + configuration.FormatterDefinitions.Add( + CreateResultFormatter(typeName, resultType, serializerAccessor)); + } + + /// + /// Determines whether the value of the field is produced by the interface itself (an explicit + /// resolver or a default implementation) rather than supplied by the implementing type through + /// an abstract interface member. + /// + private static bool ResolvesOnInterface(InterfaceFieldConfiguration configuration) + { + var member = configuration.ResolverMember ?? configuration.Member; + + // An explicit resolver that is not bound to a member always produces the value itself. + if (member is null) + { + return true; + } + + var method = member as MethodInfo ?? (member as PropertyInfo)?.GetMethod; + + // A default implementation has a method body (it is not abstract), whereas an abstract + // interface member is implemented by the concrete type that supplies the value. + return method is { IsAbstract: false }; + } + internal static void ApplyIdToFieldCore( IDescriptor descriptor, NodeIdNameDefinitionUnion? nameDefinition) @@ -112,6 +211,24 @@ internal static void ApplyIdToFieldCore( } } } + else if (descriptor is IDescriptor interfaceFieldDescriptor) + { + var extend = interfaceFieldDescriptor.Extend(); + + // add serializer if globalID support is enabled. + if (extend.Context.Features.Get()?.IsEnabled == true) + { + if (nameDefinition?.Type != null) + { + var dependsOn = extend.Context.TypeInspector.GetTypeRef(nameDefinition.Type); + ApplyIdToInterfaceField(extend.Configuration, nameDefinition, dependsOn); + } + else + { + ApplyIdToInterfaceField(extend.Configuration, nameDefinition); + } + } + } } public static void ApplyIdToFieldCore( diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdAttributeTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdAttributeTests.cs index aa844168505..d5f4aade49c 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdAttributeTests.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdAttributeTests.cs @@ -1,6 +1,7 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Text; +using System.Text.Json; using HotChocolate.Configuration; using HotChocolate.Execution; using HotChocolate.Types.Descriptors; @@ -441,6 +442,221 @@ public async Task Id_Type_Is_Correctly_Inferred() schema.ToString().MatchSnapshot(); } + [Fact] + public async Task Id_On_Interface_Forwards_TypeName_To_Multiple_Implementers() + { + // arrange & act + // two object types inherit the same default-implementation interface field; both encode + // the value with the interface's forwarded type name from the shared serializer. + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + a { id } + b { id } + } + """); + + // assert + var operationResult = result.ExpectOperationResult(); + Assert.Empty(operationResult.Errors); + using var document = JsonDocument.Parse(operationResult.ToJson()); + var data = document.RootElement.GetProperty("data"); + Assert.Equal( + "Shared:7", + Encoding.UTF8.GetString( + Convert.FromBase64String(data.GetProperty("a").GetProperty("id").GetString()!))); + Assert.Equal( + "Shared:7", + Encoding.UTF8.GetString( + Convert.FromBase64String(data.GetProperty("b").GetProperty("id").GetString()!))); + } + + [Fact] + public async Task Id_On_Interface_Override_Uses_Concrete_Name_Inherit_Uses_Interface_Name() + { + // arrange & act + // A overrides the interface member and declares its own [ID("Bar")] (merge path). + // B inherits the interface default implementation [ID("Foo")] (copy path). + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + a { id } + b { id } + } + """); + + // assert + var operationResult = result.ExpectOperationResult(); + Assert.Empty(operationResult.Errors); + using var document = JsonDocument.Parse(operationResult.ToJson()); + var data = document.RootElement.GetProperty("data"); + // A encodes with the concrete name "Bar" and is not double-encoded. + Assert.Equal( + "Bar:1", + Encoding.UTF8.GetString( + Convert.FromBase64String(data.GetProperty("a").GetProperty("id").GetString()!))); + // B encodes with the interface default implementation's name "Foo". + Assert.Equal( + "Foo:100", + Encoding.UTF8.GetString( + Convert.FromBase64String(data.GetProperty("b").GetProperty("id").GetString()!))); + } + + [Fact] + public async Task Id_On_Interface_Forwards_TypeName_To_SDL() + { + // arrange & act + var schema = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddType() + .AddGlobalObjectIdentification(false) + .BuildSchemaAsync(); + + // assert + schema.MatchSnapshot(); + } + + [Fact] + public async Task Id_On_Interface_With_String_Name_Forwards_TypeName_When_Inherited() + { + // arrange & act + // the implementing object type does not redeclare the id field, so the interface's + // forwarded type name must flow through to the object field serializer. + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + namedNode { + id + } + } + """); + + // assert + // the decoded id proves the forwarded type name ("Foo") is used, not the inferred one. + Assert.Equal("Foo:1", Encoding.UTF8.GetString(Convert.FromBase64String(GetId(result, "namedNode")))); + } + + [Fact] + public async Task Id_On_Interface_With_Generic_Name_Forwards_TypeName_When_Inherited() + { + // arrange & act + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + genericNamedNode { + id + } + } + """); + + // assert + // the decoded id proves the generic type argument's GraphQL name ("Renamed") is used. + Assert.Equal( + "Renamed:1", + Encoding.UTF8.GetString(Convert.FromBase64String(GetId(result, "genericNamedNode")))); + } + + [Fact] + public async Task Id_On_Abstract_Interface_Member_Does_Not_Encode_Inherited_Value() + { + // arrange & act + // the interface declares an abstract [ID] member, so the implementing type supplies the + // value and must opt into the global id encoding itself. The inherited value stays raw. + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + abstractIdNode { + id + } + } + """); + + // assert + // the value is the raw "1", not the global id encoded "Foo:1". + Assert.Equal("1", GetId(result, "abstractIdNode")); + } + + [Fact] + public async Task Id_On_Abstract_Interface_Member_Still_Rewrites_Type_To_ID() + { + // arrange & act + var schema = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddGlobalObjectIdentification(false) + .BuildSchemaAsync(); + + // assert + // the type rewrite is unconditional, only the value serialization is gated off. + schema.MatchSnapshot(); + } + + [Fact] + public async Task Id_On_Abstract_Interface_Member_Encodes_When_Object_Opts_In() + { + // arrange & act + // the same abstract interface member, but the implementing object declares its own [ID], + // so the object opts into the global id encoding. + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + optInIdNode { + id + } + } + """); + + // assert + // the object's own [ID] uses the inferred object type name "OptInIdNode". + Assert.Equal( + "OptInIdNode:1", + Encoding.UTF8.GetString(Convert.FromBase64String(GetId(result, "optInIdNode")))); + } + [Fact] public async Task EnsureIdIsOnlyAppliedOnce() { @@ -868,4 +1084,141 @@ private static byte[] Combine(ReadOnlySpan s1, ReadOnlySpan s2) s2.CopyTo(buffer.AsSpan()[s1.Length..]); return buffer; } + + private static string GetId(IExecutionResult result, string fieldName) + { + var operationResult = result.ExpectOperationResult(); + Assert.Empty(operationResult.Errors); + using var document = JsonDocument.Parse(operationResult.ToJson()); + return document.RootElement + .GetProperty("data") + .GetProperty(fieldName) + .GetProperty("id") + .GetString()!; + } + + public class NamedNodeQuery + { + public INamedNode GetNamedNode() => new NamedNode(); + + public IGenericNamedNode GetGenericNamedNode() => new GenericNamedNode(); + } + + [InterfaceType("NamedNodeInterface")] + public interface INamedNode + { + [ID("Foo")] + int GetId() => 1; + } + + public class NamedNode : INamedNode; + + public class GenericNamedNodeQuery + { + public IGenericNamedNode GetGenericNamedNode() => new GenericNamedNode(); + } + + [InterfaceType("GenericNamedNodeInterface")] + public interface IGenericNamedNode + { + [ID] + int GetId() => 1; + } + + public class GenericNamedNode : IGenericNamedNode; + + [GraphQLName("Renamed")] + public class RenamedIdTarget + { + public int Id { get; set; } = 1; + } + + public class AbstractIdNodeQuery + { + public IAbstractIdNode GetAbstractIdNode() => new AbstractIdNode(); + } + + [InterfaceType("AbstractIdNodeInterface")] + public interface IAbstractIdNode + { + [ID("Foo")] + int Id { get; } + } + + public class AbstractIdNodeType : ObjectType + { + protected override void Configure(IObjectTypeDescriptor descriptor) + { + descriptor.Name("AbstractIdNode"); + descriptor.BindFieldsExplicitly(); + descriptor.Implements>(); + } + } + + public class AbstractIdNode : IAbstractIdNode + { + public int Id => 1; + } + + public class OptInIdNodeQuery + { + public IAbstractIdNode GetOptInIdNode() => new OptInIdNode(); + } + + public class OptInIdNodeType : ObjectType + { + protected override void Configure(IObjectTypeDescriptor descriptor) + { + descriptor.Name("OptInIdNode"); + descriptor.Implements>(); + descriptor.Field(t => t.Id).ID(); + } + } + + public class MultiQuery + { + public IMultiNode GetA() => new MultiA(); + public IMultiNode GetB() => new MultiB(); + } + + [InterfaceType("MultiNode")] + public interface IMultiNode + { + [ID("Shared")] + int GetId() => 7; + } + + public class MultiA : IMultiNode; + + public class MultiB : IMultiNode; + + public class OptInIdNode : IAbstractIdNode + { + public int Id => 1; + } + + public class OverrideQuery + { + public IOverrideNode GetA() => new OverrideA(); + public IOverrideNode GetB() => new OverrideB(); + } + + [InterfaceType("OverrideNode")] + public interface IOverrideNode + { + [ID("Foo")] + int GetId() => 100; + } + + // A overrides the member and declares its own [ID("Bar")]: A goes through the merge path + // (the object redeclares the field) and must encode with the concrete name "Bar". + public class OverrideA : IOverrideNode + { + [ID("Bar")] + public int GetId() => 1; + } + + // B inherits the default implementation: B goes through the copy path and must encode with + // the interface name "Foo". + public class OverrideB : IOverrideNode; } diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs index 7aab7cd2281..18da874eaff 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs @@ -1,3 +1,5 @@ +using System.Text; +using System.Text.Json; using HotChocolate.Execution; using Microsoft.Extensions.DependencyInjection; @@ -218,6 +220,68 @@ public async Task Id_Honors_CustomTypeNaming_Throws_On_InvalidInputs() result.MatchSnapshot(postFix: "InvalidArgs"); } + [Fact] + public async Task Id_On_Interface_Fluent_Forwards_TypeName_When_Inherited() + { + // arrange & act + // the implementing object type inherits the id field from the interface, so the + // fluent .ID("Foo") on the interface must flow through to the object field serializer. + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + namedNode { + id + } + } + """); + + // assert + var id = GetId(result, "namedNode", "id"); + Assert.Equal("Foo:1", Encoding.UTF8.GetString(Convert.FromBase64String(id))); + } + + [Fact] + public async Task Id_On_Interface_Fluent_Generic_Forwards_TypeName_When_Inherited() + { + // arrange & act + var result = + await new ServiceCollection() + .AddGraphQLServer() + .AddQueryType() + .AddType() + .AddGlobalObjectIdentification(false) + .ExecuteRequestAsync( + """ + { + namedNode { + anotherId + } + } + """); + + // assert + var id = GetId(result, "namedNode", "anotherId"); + Assert.Equal("Another:1", Encoding.UTF8.GetString(Convert.FromBase64String(id))); + } + + private static string GetId(IExecutionResult result, string fieldName, string idFieldName) + { + var operationResult = result.ExpectOperationResult(); + Assert.Empty(operationResult.Errors); + using var document = JsonDocument.Parse(operationResult.ToJson()); + return document.RootElement + .GetProperty("data") + .GetProperty(fieldName) + .GetProperty(idFieldName) + .GetString()!; + } + private static byte[] Combine(ReadOnlySpan s1, ReadOnlySpan s2) { var buffer = new byte[s1.Length + s2.Length]; @@ -226,6 +290,36 @@ private static byte[] Combine(ReadOnlySpan s1, ReadOnlySpan s2) return buffer; } + public class NamedNodeQueryType : ObjectType + { + protected override void Configure(IObjectTypeDescriptor descriptor) + { + descriptor.Name("Query"); + descriptor.Field("namedNode").Type().Resolve(_ => new NamedNode()); + } + } + + public class NamedNodeInterfaceType : InterfaceType + { + protected override void Configure(IInterfaceTypeDescriptor descriptor) + { + descriptor.Name("NamedNode"); + descriptor.Field("id").Type().Resolve(_ => 1).ID("Foo"); + descriptor.Field("anotherId").Type().Resolve(_ => 1).ID(); + } + } + + public class NamedNodeType : ObjectType + { + protected override void Configure(IObjectTypeDescriptor descriptor) + { + descriptor.Name("NamedNodeObject"); + descriptor.Implements(); + } + } + + public class NamedNode; + public class QueryType : ObjectType { protected override void Configure(IObjectTypeDescriptor descriptor) diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Abstract_Interface_Member_Still_Rewrites_Type_To_ID.graphql b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Abstract_Interface_Member_Still_Rewrites_Type_To_ID.graphql new file mode 100644 index 00000000000..17d35fe26f9 --- /dev/null +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Abstract_Interface_Member_Still_Rewrites_Type_To_ID.graphql @@ -0,0 +1,15 @@ +schema { + query: AbstractIdNodeQuery +} + +type AbstractIdNodeQuery { + abstractIdNode: AbstractIdNodeInterface! +} + +type AbstractIdNode implements AbstractIdNodeInterface { + id: ID! +} + +interface AbstractIdNodeInterface { + id: ID! +} diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Interface_Forwards_TypeName_To_SDL.graphql b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Interface_Forwards_TypeName_To_SDL.graphql new file mode 100644 index 00000000000..22bb52c7109 --- /dev/null +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Relay/__snapshots__/IdAttributeTests.Id_On_Interface_Forwards_TypeName_To_SDL.graphql @@ -0,0 +1,28 @@ +schema { + query: NamedNodeQuery +} + +type NamedNodeQuery { + namedNode: NamedNodeInterface! + genericNamedNode: GenericNamedNodeInterface! +} + +type GenericNamedNode implements GenericNamedNodeInterface { + id: ID! +} + +type NamedNode implements NamedNodeInterface { + id: ID! +} + +type Renamed { + id: Int! +} + +interface GenericNamedNodeInterface { + id: ID! +} + +interface NamedNodeInterface { + id: ID! +}