[Fusion] Fix bug with shared parent selection (#8378)

Author: tobias-tengler

src/HotChocolate/Fusion/src/Core/Planning/Pipeline/ExecutionStepDiscoveryMiddleware.cs

@@ -196,7 +196,8 @@ parentSelectionPath is null
                 }
 
                 executionStep.AllSelections.Add(selection);
-                executionStep.RootSelections.Add(new RootSelection(selection, resolver));
+                var rootSelection = new RootSelection(selection, resolver);
+                executionStep.RootSelections.Add(rootSelection);
 
                 if (resolver is not null)
                 {
@@ -205,7 +206,7 @@ parentSelectionPath is null
 
                 if (selection.SelectionSet is not null)
                 {
-                    CollectNestedSelections(
+                    var couldPlanChildSelections = CollectNestedSelections(
                         context,
                         backlog,
                         operation,
@@ -216,6 +217,15 @@ parentSelectionPath is null
                         preferBatching,
                         context.ParentSelections,
                         subgraphsInContext);
+
+                    if (!couldPlanChildSelections)
+                    {
+                        executionStep.AllSelections.Remove(selection);
+                        executionStep.RootSelections.Remove(rootSelection);
+                        executionStep.SelectionResolvers.Remove(selection);
+
+                        (leftovers ??=[]).Add(selection);
+                    }
                 }
 
                 path.RemoveAt(pathIndex);
@@ -295,7 +305,7 @@ private void HandleSpecialQuerySelections(
         }
     }
 
-    private void CollectNestedSelections(
+    private bool CollectNestedSelections(
         QueryPlanContext context,
         Queue<BacklogItem> backlog,
         IOperation operation,
@@ -312,9 +322,10 @@ private void CollectNestedSelections(
             preferBatching = parentSelection.Type.IsListType();
         }
 
+        var overallCouldPlanSelections = false;
         foreach (var possibleType in operation.GetPossibleTypes(parentSelection))
         {
-            CollectNestedSelections(
+            var couldPlanSelections = CollectNestedSelections(
                 context,
                 backlog,
                 operation,
@@ -326,10 +337,17 @@ private void CollectNestedSelections(
                 preferBatching,
                 parentSelectionLookup,
                 subgraphsInContext);
+
+            if (couldPlanSelections)
+            {
+                overallCouldPlanSelections = true;
+            }
         }
+
+        return overallCouldPlanSelections;
     }
 
-    private void CollectNestedSelections(
+    private bool CollectNestedSelections(
         QueryPlanContext context,
         Queue<BacklogItem> backlog,
         IOperation operation,
@@ -399,7 +417,7 @@ private void CollectNestedSelections(
 
                 if (selection.SelectionSet is not null)
                 {
-                    CollectNestedSelections(
+                    var couldPlanChildSelections = CollectNestedSelections(
                         context,
                         backlog,
                         operation,
@@ -410,6 +428,14 @@ private void CollectNestedSelections(
                         preferBatching,
                         parentSelectionLookup,
                         subgraphsInContext);
+
+                    if (!couldPlanChildSelections)
+                    {
+                        executionStep.AllSelections.Remove(selection);
+                        executionStep.SelectionResolvers.Remove(selection);
+
+                        (leftovers ??=[]).Add(selection);
+                    }
                 }
             }
             else
@@ -422,6 +448,16 @@ private void CollectNestedSelections(
 
         if (leftovers is not null)
         {
+            var isTypeResolvableFromOtherSubgraph = declaringType.Resolvers
+                .Any(r => r.SubgraphName != executionStep.SubgraphName);
+
+            var couldNotResolveAnySelections = leftovers.Count == selectionSet.Selections.Count;
+
+            if (couldNotResolveAnySelections && !isTypeResolvableFromOtherSubgraph)
+            {
+                return false;
+            }
+
             var selectionSetPath = CreateSelectionPath(rootSelectionPath, path);
 
             if (selectionSetPath is not null)
@@ -444,6 +480,8 @@ private void CollectNestedSelections(
                 preferBatching
             );
         }
+
+        return true;
     }
 
     private static void TryEnqueueBacklogItem(
@@ -644,7 +682,6 @@ private void TrySetEntityResolver(
         string subgraph,
         SelectionExecutionStep executionStep,
         HashSet<string> subgraphsInContext)
-
     {
         if (parentSelection is null || !selectionSetTypeMetadata.Resolvers.ContainsResolvers(subgraph))
         {

src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs

@@ -2442,7 +2442,7 @@ type ResaleSurveyFeedback {
         await snapshot.MatchMarkdownAsync();
     }
 
-    [Fact(Skip = "Fix this test in the new planner")]
+    [Fact]
     public async Task Field_Below_Shared_Field_Only_Available_On_One_Subgraph_Type_Of_Shared_Field_Not_Node()
     {
         // arrange
@@ -2664,6 +2664,132 @@ type Query {
         MatchMarkdownSnapshot(request, result);
     }
 
+    [Fact]
+    public async Task Viewer_Bug_1()
+    {
+        // arrange
+        var subgraphA = await TestSubgraph.CreateAsync(
+            """
+            type Query {
+              exclusiveSubgraphA: ExclusiveSubgraphA
+              viewer: Viewer
+            }
+
+            type ExclusiveSubgraphA {
+              id: ID!
+            }
+
+            type Viewer {
+              name: String
+            }
+            """);
+        var subgraphB = await TestSubgraph.CreateAsync(
+            """
+            type Query {
+              viewer: Viewer
+            }
+
+            type Viewer {
+              exclusiveSubgraphB: String
+            }
+            """);
+
+        using var subgraphs = new TestSubgraphCollection(output, [subgraphA, subgraphB]);
+        var executor = await subgraphs.GetExecutorAsync();
+
+        var request = Parse(
+            """
+            query testQuery {
+              exclusiveSubgraphA {
+                __typename
+              }
+              viewer {
+                exclusiveSubgraphB
+              }
+            }
+            """);
+
+        // act
+        await using var result = await executor.ExecuteAsync(
+            OperationRequestBuilder
+                .New()
+                .SetDocument(request)
+                .Build());
+
+        // assert
+        var snapshot = new Snapshot();
+        CollectSnapshotData(snapshot, request, result);
+        await snapshot.MatchMarkdownAsync();
+    }
+
+    [Fact]
+    public async Task Viewer_Bug_2()
+    {
+        // arrange
+        var subgraphA = await TestSubgraph.CreateAsync(
+            """
+            type Query {
+              exclusiveSubgraphA: ExclusiveSubgraphA
+              viewer: Viewer
+            }
+
+            type ExclusiveSubgraphA {
+              id: ID!
+            }
+
+            type Viewer {
+              subType: SubType
+            }
+
+            type SubType {
+              subgraphA: String
+            }
+            """);
+        var subgraphB = await TestSubgraph.CreateAsync(
+            """
+            type Query {
+              viewer: Viewer
+            }
+
+            type Viewer {
+              subType: SubType
+            }
+
+            type SubType {
+              subgraphB: String
+            }
+            """);
+
+        using var subgraphs = new TestSubgraphCollection(output, [subgraphA, subgraphB]);
+        var executor = await subgraphs.GetExecutorAsync();
+
+        var request = Parse(
+            """
+            query testQuery {
+              exclusiveSubgraphA {
+                __typename
+              }
+              viewer {
+                subType {
+                  subgraphB
+                }
+              }
+            }
+            """);
+
+        // act
+        await using var result = await executor.ExecuteAsync(
+            OperationRequestBuilder
+                .New()
+                .SetDocument(request)
+                .Build());
+
+        // assert
+        var snapshot = new Snapshot();
+        CollectSnapshotData(snapshot, request, result);
+        await snapshot.MatchMarkdownAsync();
+    }
+
     [Fact]
     public async Task Two_Arguments_Differing_Nullability_Does_Not_Duplicate_Forwarded_Variables()
     {

src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.Field_Below_Shared_Field_Only_Available_On_One_Subgraph_Type_Of_Shared_Field_Not_Node.md

@@ -93,3 +93,4 @@ query($productId: ID!) {
   }
 }
 ```
+

src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.Viewer_Bug_1.md

@@ -0,0 +1,73 @@
+# Viewer_Bug_1
+
+## Result
+
+```json
+{
+  "data": {
+    "exclusiveSubgraphA": {
+      "__typename": "ExclusiveSubgraphA"
+    },
+    "viewer": {
+      "exclusiveSubgraphB": "string"
+    }
+  }
+}
+```
+
+## Request
+
+```graphql
+query testQuery {
+  exclusiveSubgraphA {
+    __typename
+  }
+  viewer {
+    exclusiveSubgraphB
+  }
+}
+```
+
+## QueryPlan Hash
+
+```text
+1A0ADF8B4DE457E0B7C85E08324A394F9F2FEB91
+```
+
+## QueryPlan
+
+```json
+{
+  "document": "query testQuery { exclusiveSubgraphA { __typename } viewer { exclusiveSubgraphB } }",
+  "operation": "testQuery",
+  "rootNode": {
+    "type": "Sequence",
+    "nodes": [
+      {
+        "type": "Parallel",
+        "nodes": [
+          {
+            "type": "Resolve",
+            "subgraph": "Subgraph_1",
+            "document": "query testQuery_1 { exclusiveSubgraphA { __typename } }",
+            "selectionSetId": 0
+          },
+          {
+            "type": "Resolve",
+            "subgraph": "Subgraph_2",
+            "document": "query testQuery_2 { viewer { exclusiveSubgraphB } }",
+            "selectionSetId": 0
+          }
+        ]
+      },
+      {
+        "type": "Compose",
+        "selectionSetIds": [
+          0
+        ]
+      }
+    ]
+  }
+}
+```
+