From ddacf344abc3ec375618e54ddfbb6f91ef703939 Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Thu, 12 Dec 2024 22:29:25 +0100 Subject: [PATCH] Make TryHandleUnresolvedSelections work --- .../InlineFragmentOperationRewriter.cs | 1 + .../Planning/OperationPlanner.cs | 32 +++++++-- .../Fusion.Execution.Tests/FragmentTests.cs | 39 ----------- .../__snapshots__/FragmentTests.Test.yaml | 68 ------------------- ...try_Selection_From_Different_Subgraph.yaml | 31 +++++++++ 5 files changed, 57 insertions(+), 114 deletions(-) delete mode 100644 src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Test.yaml diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/InlineFragmentOperationRewriter.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/InlineFragmentOperationRewriter.cs index f7ac20e75e8..d10ecb8dac7 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/InlineFragmentOperationRewriter.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/InlineFragmentOperationRewriter.cs @@ -4,6 +4,7 @@ namespace HotChocolate.Fusion.Planning; +// TODO: We need to merge selections public sealed class InlineFragmentOperationRewriter(CompositeSchema schema) { public DocumentNode RewriteDocument(DocumentNode document, string? operationName) diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/OperationPlanner.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/OperationPlanner.cs index 30464de59a7..7126dd41144 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/OperationPlanner.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Planning/OperationPlanner.cs @@ -1,4 +1,3 @@ -using System.Collections; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using HotChocolate.Fusion.Planning.Nodes; @@ -154,7 +153,6 @@ private bool TryPlanInlineFragmentSelection( { if (IsSelectionAlwaysSkipped(selection)) { - // TODO: How to reconcile this? continue; } @@ -437,7 +435,14 @@ private bool TryHandleUnresolvedSelections( break; case InlineFragmentPlanNode inlineFragmentNode: - // TODO: Do we have to do this? + foreach (var inlineFragmentSelection in inlineFragmentNode.Selections) + { + if (inlineFragmentSelection is FieldPlanNode field) + { + processedFields.Add(field.Field.Name); + } + } + break; default: @@ -446,13 +451,26 @@ private bool TryHandleUnresolvedSelections( } } - // TODO: Uuuuuuuuuuuuuuugly hack, just to make tests work for now - if (unresolvedSelections.OfType().Any()) + var unresolvedFields = new HashSet(); + foreach (var unresolvedSelection in unresolvedSelections) { - return true; + if (unresolvedSelection is UnresolvedField unresolvedField) + { + unresolvedFields.Add(unresolvedField.Field.Name); + } + else if (unresolvedSelection is UnresolvedInlineFragment unresolvedInlineFragment) + { + foreach (var inlineFragmentSelection in unresolvedInlineFragment.UnresolvedSelections) + { + if (inlineFragmentSelection is UnresolvedField field) + { + unresolvedFields.Add(field.Field.Name); + } + } + } } - return unresolvedSelections.Count == processedFields.Count; + return unresolvedFields.Count == processedFields.Count; } /// diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/FragmentTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/FragmentTests.cs index 187726981e3..b5aa4fb5f56 100644 --- a/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/FragmentTests.cs +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/FragmentTests.cs @@ -489,43 +489,4 @@ fragment ProductFragment2 on Product { // assert plan.MatchSnapshot(); } - - [Test] - [Skip("Doesn't work yet")] - public void Test() - { - // arrange - var compositeSchema = CreateCompositeSchema(); - - var request = Parse( - """ - query($id: ID!) { - reviewById(id: $id) { - body - author { - displayName - } - ...ReviewFragment - } - } - - fragment ReviewFragment on Review { - author { - id - displayName - reviews { - pageInfo { - hasNextPage - } - } - } - } - """); - - // act - var plan = PlanOperation(request, compositeSchema); - - // assert - plan.MatchSnapshot(); - } } diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Test.yaml b/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Test.yaml deleted file mode 100644 index 21cbd2d84e6..00000000000 --- a/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Test.yaml +++ /dev/null @@ -1,68 +0,0 @@ -request: - - document: >- - query($id: ID!) { - reviewById(id: $id) { - body - author { - displayName - } - author { - id - displayName - reviews { - pageInfo { - hasNextPage - } - } - } - } - } -nodes: - - id: 1 - schema: "REVIEWS" - operation: >- - query($id: ID!) { - reviewById(id: $id) { - body - author { - id - } - author { - id - reviews { - pageInfo { - hasNextPage - } - } - id - } - } - } - - id: 2 - schema: "ACCOUNTS" - operation: >- - query($__fusion_requirement_1: ID!) { - userById(id: $__fusion_requirement_1) { - displayName - } - } - requirements: - - name: "__fusion_requirement_1" - dependsOn: "1" - selectionSet: "reviewById.author" - field: "id" - type: "ID!" - - id: 3 - schema: "ACCOUNTS" - operation: >- - query($__fusion_requirement_2: ID!) { - userById(id: $__fusion_requirement_2) { - displayName - } - } - requirements: - - name: "__fusion_requirement_2" - dependsOn: "1" - selectionSet: "reviewById.author" - field: "id" - type: "ID!" diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Two_Fragments_On_Sub_Selection_With_Different_But_Same_Entry_Selection_From_Different_Subgraph.yaml b/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Two_Fragments_On_Sub_Selection_With_Different_But_Same_Entry_Selection_From_Different_Subgraph.yaml index dff81b1f8ef..f6c6193faa4 100644 --- a/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Two_Fragments_On_Sub_Selection_With_Different_But_Same_Entry_Selection_From_Different_Subgraph.yaml +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Execution.Tests/__snapshots__/FragmentTests.Two_Fragments_On_Sub_Selection_With_Different_But_Same_Entry_Selection_From_Different_Subgraph.yaml @@ -15,3 +15,34 @@ request: } } nodes: + - id: 1 + schema: "PRODUCTS" + operation: >- + query($slug: String!) { + productBySlug(slug: $slug) { + id + } + } + - id: 2 + schema: "REVIEWS" + operation: >- + query($__fusion_requirement_1: ID!) { + productById(id: $__fusion_requirement_1) { + reviews { + nodes { + body + } + } + reviews { + pageInfo { + hasNextPage + } + } + } + } + requirements: + - name: "__fusion_requirement_1" + dependsOn: "1" + selectionSet: "productBySlug" + field: "id" + type: "ID!"