diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Partitioners/SelectionSetByTypePartitioner.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Partitioners/SelectionSetByTypePartitioner.cs index 56918302116..13d4e682fee 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Partitioners/SelectionSetByTypePartitioner.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/Partitioners/SelectionSetByTypePartitioner.cs @@ -125,9 +125,15 @@ private void CollectSelections( } else { - foreach (var possibleType in schema.GetPossibleTypes(type, includeInaccessible: true)) + // The branches are limited to the object types the enclosing selection set can + // yield, as an interface type condition can be implemented by types that are not + // possible types of that selection set. + foreach (var possibleType in schema.GetPossibleTypes(context.SharedType, includeInaccessible: true)) { - AddSelectionsForConcreteType(context, possibleType, selectionsWithPath, cloneSelectionSets: true); + if (MatchesEnclosingTypeConditions(context, possibleType)) + { + AddSelectionsForConcreteType(context, possibleType, selectionsWithPath, cloneSelectionSets: true); + } } } } @@ -137,6 +143,38 @@ private void CollectSelections( } } + /// + /// Determines whether the specified object type satisfies all type conditions + /// on the current type path. + /// + private bool MatchesEnclosingTypeConditions(Context context, FusionObjectTypeDefinition type) + { + foreach (var typeCondition in context.TypePath) + { + if (!ContainsType(schema.GetPossibleTypes(typeCondition, includeInaccessible: true), type)) + { + return false; + } + } + + return true; + } + + private static bool ContainsType( + ImmutableArray possibleTypes, + FusionObjectTypeDefinition type) + { + foreach (var possibleType in possibleTypes) + { + if (ReferenceEquals(possibleType, type)) + { + return true; + } + } + + return false; + } + private void AddSelectionsForConcreteType( Context context, FusionObjectTypeDefinition type, diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/AbstractLookupFanoutPlanningTests.cs b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/AbstractLookupFanoutPlanningTests.cs index faeabbffded..4b848a84971 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/AbstractLookupFanoutPlanningTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/AbstractLookupFanoutPlanningTests.cs @@ -61,6 +61,53 @@ ... on Book { reviewsCount } MatchSnapshot(plan); } + [Fact] + public void Plan_Should_Prune_NonNode_Implementor_When_InterfaceFragment_Is_In_NodeSelectionSet() + { + // arrange + // The Manageable fragment fans out over all Manageable implementors, but the enclosing + // node selection set only ever yields Node implementors, so Draft must not get a branch. + var schema = CreateManageableNodeSchema(); + + // act + var plan = PlanOperation( + schema, + """ + query($id: ID!) { + node(id: $id) { + __typename + ... on Node { id } + ... on Manageable { canEdit } + } + } + """); + + // assert + MatchSnapshot(plan); + } + + [Fact] + public void Plan_Should_Prune_NonNode_Implementor_When_NodeSelectionSet_Has_No_Shared_Selections() + { + // arrange + var schema = CreateManageableNodeSchema(); + + // act + var plan = PlanOperation( + schema, + """ + query($id: ID!) { + node(id: $id) { + __typename + ... on Manageable { canEdit } + } + } + """); + + // assert + MatchSnapshot(plan); + } + // sku is co-located with the products root in "a", reviews in "r", Book-only title in "books". private static FusionSchemaDefinition CreateProductTitleSchema() => ComposeSchema( @@ -151,4 +198,23 @@ type Query { type Magazine @key(fields: "id") { id: ID! title: String } """); + + // Ticket is both Manageable and a Node, Draft is only Manageable and has no id. + private static FusionSchemaDefinition CreateManageableNodeSchema() + => ComposeSchema( + """ + # name: tickets + schema { query: Query } + + type Query { + node(id: ID!): Node @lookup + draft: Draft + } + + interface Node { id: ID! } + interface Manageable { canEdit: Boolean! } + + type Ticket implements Node & Manageable { id: ID! canEdit: Boolean! } + type Draft implements Manageable { canEdit: Boolean! name: String } + """); } diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/SelectionSetByTypePartitionerTests.cs b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/SelectionSetByTypePartitionerTests.cs index 502b22702ea..18a864bdb6f 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/SelectionSetByTypePartitionerTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/SelectionSetByTypePartitionerTests.cs @@ -274,6 +274,64 @@ ... on Votable { """); } + [Fact] + public void Selections_On_Interface_Skips_Implementors_That_Are_Not_Possible_Types() + { + // arrange + var source1 = new TestSourceSchema( + """ + type Query { + node(id: ID!): Node @lookup + draft: Draft + } + + interface Node { + id: ID! + } + + interface Votable { + viewerHasUpvoted: Boolean! + } + + type Discussion implements Node & Votable { + id: ID! + title: String! + viewerHasUpvoted: Boolean! + } + + type Draft implements Votable { + name: String! + viewerHasUpvoted: Boolean! + } + """); + var schema = ComposeSchema(source1); + + var doc = Utf8GraphQLParser.Parse( + """ + query($id: ID!) { + node(id: $id) { + ... on Votable { + viewerHasUpvoted + } + } + } + """); + + // act + var result = Partition(schema, doc); + + // assert + MatchInlineSnapshot( + result, + """ + Shared: null + + Discussion: { + viewerHasUpvoted + } + """); + } + [Fact] public void Concrete_Type_Selections_Within_Interface() { @@ -399,6 +457,71 @@ ... on Votable { """); } + [Fact] + public void Nested_Interface_Selections_Skip_Implementors_Outside_Outer_Interface() + { + // arrange + // Announcement is a Node and Commentable, but not Votable, so the nested Commentable + // fragment must not produce an Announcement branch. + var source1 = new TestSourceSchema( + """ + type Query { + node(id: ID!): Node @lookup + } + + interface Node { + id: ID! + } + + interface Votable { + viewerHasUpvoted: Boolean! + } + + interface Commentable { + commentCount: Int! + } + + type Discussion implements Node & Votable & Commentable { + id: ID! + viewerHasUpvoted: Boolean! + commentCount: Int! + } + + type Announcement implements Node & Commentable { + id: ID! + commentCount: Int! + } + """); + var schema = ComposeSchema(source1); + + var doc = Utf8GraphQLParser.Parse( + """ + query($id: ID!) { + node(id: $id) { + ... on Votable { + ... on Commentable { + commentCount + } + } + } + } + """); + + // act + var result = Partition(schema, doc); + + // assert + MatchInlineSnapshot( + result, + """ + Shared: null + + Discussion: { + commentCount + } + """); + } + [Fact] public void Spread_On_Type_Of_SelectionSet_Is_Part_Of_Shared_Selections() { diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/AbstractLookupFanoutPlanningTests.Plan_Should_Prune_NonNode_Implementor_When_InterfaceFragment_Is_In_NodeSelectionSet.yaml b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/AbstractLookupFanoutPlanningTests.Plan_Should_Prune_NonNode_Implementor_When_InterfaceFragment_Is_In_NodeSelectionSet.yaml new file mode 100644 index 00000000000..aa714790f53 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/AbstractLookupFanoutPlanningTests.Plan_Should_Prune_NonNode_Implementor_When_InterfaceFragment_Is_In_NodeSelectionSet.yaml @@ -0,0 +1,53 @@ +operation: + - document: | + query($id: ID!) { + node(id: $id) { + __typename + id + ... on Manageable { + canEdit + } + } + } + hash: 123456789101112 + searchSpace: 1 + expandedNodes: 1 +nodes: + - id: 1 + type: Node + idValue: $id + responseName: node + branches: + - Ticket: 3 + fallback: 2 + - id: 2 + type: Operation + operation: | + query Op_123456789101112_2($id: ID!) { + node(id: $id) { + __typename + id + } + } + forwardedVariables: + - id + dependencies: + - id: 1 + - id: 3 + type: Operation + schema: tickets + operation: | + query Op_123456789101112_3($id: ID!) { + node(id: $id) { + __typename + ... on Ticket { + __typename + id + canEdit + } + } + } + forwardedVariables: + - id + dependencies: + - id: 1 diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/AbstractLookupFanoutPlanningTests.Plan_Should_Prune_NonNode_Implementor_When_NodeSelectionSet_Has_No_Shared_Selections.yaml b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/AbstractLookupFanoutPlanningTests.Plan_Should_Prune_NonNode_Implementor_When_NodeSelectionSet_Has_No_Shared_Selections.yaml new file mode 100644 index 00000000000..b8eb687756f --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/AbstractLookupFanoutPlanningTests.Plan_Should_Prune_NonNode_Implementor_When_NodeSelectionSet_Has_No_Shared_Selections.yaml @@ -0,0 +1,50 @@ +operation: + - document: | + query($id: ID!) { + node(id: $id) { + __typename + ... on Manageable { + canEdit + } + } + } + hash: 123456789101112 + searchSpace: 1 + expandedNodes: 1 +nodes: + - id: 1 + type: Node + idValue: $id + responseName: node + branches: + - Ticket: 3 + fallback: 2 + - id: 2 + type: Operation + operation: | + query Op_123456789101112_2($id: ID!) { + node(id: $id) { + __typename + } + } + forwardedVariables: + - id + dependencies: + - id: 1 + - id: 3 + type: Operation + schema: tickets + operation: | + query Op_123456789101112_3($id: ID!) { + node(id: $id) { + __typename + ... on Ticket { + __typename + canEdit + } + } + } + forwardedVariables: + - id + dependencies: + - id: 1