diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs index 0413096e320..ba6b9764a84 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.Defer.cs @@ -741,10 +741,11 @@ is not { } providerStep lifted, droppedStepIds); - // Record the parent-scope requirements on the descriptor. + // Record the parent-scope requirements on the descriptor. A step without a + // parent dependency is served entirely by other steps in this incremental plan. foreach (var step in rewrittenIncrementalPlan) { - if (step is not OperationPlanStep operationStep) + if (step is not OperationPlanStep operationStep || operationStep.ParentDependencies.IsEmpty) { continue; } @@ -1541,7 +1542,8 @@ private static (OperationPlanStep? ConsumingStep, OperationRequirement Requireme /// /// Finds every step in that targets either /// or the operation root and whose every selected field - /// is consumed as a requirement by one of its dependents. + /// is consumed as a requirement at or above by one of + /// its dependents. /// private static ImmutableArray FindDeferRequirementProducers( ImmutableList incrementalPlanSteps, @@ -1555,7 +1557,7 @@ private static ImmutableArray FindDeferRequirementProducers( && operationStep.SchemaName is not null && !operationStep.Dependents.IsEmpty && (operationStep.Target.Equals(anchorPath) || operationStep.Target.IsRoot) - && IsPureRequirementProducer(operationStep, incrementalPlanSteps)) + && IsPureRequirementProducer(operationStep, incrementalPlanSteps, anchorPath)) { producers.Add(operationStep); } @@ -1566,12 +1568,13 @@ private static ImmutableArray FindDeferRequirementProducers( /// /// Determines whether every field selects is consumed as a - /// requirement by one of its dependents, meaning the step carries no output of its - /// own beyond what those dependents need from it. + /// requirement at or above by one of its dependents, + /// meaning the step carries no output of its own beyond what the enclosing scope can supply. /// private static bool IsPureRequirementProducer( OperationPlanStep step, - ImmutableList incrementalPlanSteps) + ImmutableList incrementalPlanSteps, + SelectionPath anchorPath) { var ownFields = new HashSet(StringComparer.Ordinal); CollectLeafFieldNames(step.Definition.SelectionSet, ownFields); @@ -1591,6 +1594,13 @@ private static bool IsPureRequirementProducer( foreach (var (_, requirement) in dependentStep.Requirements) { + // A requirement below the anchor reads a value this step fetches as + // deferred output, which the enclosing scope does not reach. + if (!requirement.Path.IsParentOfOrSame(anchorPath)) + { + continue; + } + var fieldName = requirement.InternalAlias ?? ExtractRootFieldName(requirement.Map.ToString()); if (fieldName is not null) diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs index 920c85407ce..78ff241dc5b 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs @@ -56,7 +56,7 @@ public OperationPlanner( _options = options; } - public static Version Version { get; } = new(2, 2, 0); + public static Version Version { get; } = new(2, 3, 0); internal OperationPlannerOptions Options => _options; diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/DeferTests.cs b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/DeferTests.cs index 4a020ad1e1e..3e527279826 100644 --- a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/DeferTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/DeferTests.cs @@ -2221,6 +2221,99 @@ ... @defer { await MatchSnapshotAsync(gateway, request, result, stableStream: true); } + [Fact] + public async Task Defer_Should_ResolveNestedLookupInDeferredPayload_When_DeferredFieldTraversesSubgraphs() + { + // arrange + using var server1 = CreateSourceSchema( + "A", + b => b.AddQueryType()); + + using var server2 = CreateSourceSchema( + "B", + b => b.AddQueryType()); + + using var server3 = CreateSourceSchema( + "C", + b => b.AddQueryType()); + + using var gateway = await CreateCompositeSchemaAsync( + [ + ("A", server1), + ("B", server2), + ("C", server3) + ]); + + // act + using var client = GraphQLHttpClient.Create(gateway.CreateClient()); + + var request = new OperationRequest( + """ + { + products { + name + ... @defer(label: "deeper") { + related { + name + } + } + } + } + """); + + using var result = await client.PostAsync( + request, + new Uri("http://localhost:5000/graphql"), + TestContext.Current.CancellationToken); + + // assert + await MatchSnapshotAsync(gateway, request, result, stableStream: true); + } + + private static class NestedLookupSourceA + { + public class Query + { + public Product[] GetProducts() => [new(1), new(2)]; + + [Lookup] + [Internal] + public Product? GetProductById([ID] int id) => new(id); + } + + public record Product([property: ID] int Id); + } + + private static class NestedLookupSourceB + { + public class Query + { + [Lookup] + [Internal] + public Product? GetProductById([ID] int id) => new(id); + } + + public record Product([property: ID] int Id) + { + public Product Related => new(Id + 10); + } + } + + private static class NestedLookupSourceC + { + public class Query + { + [Lookup] + [Internal] + public Product? GetProductById([ID] int id) => new(id); + } + + public record Product([property: ID] int Id) + { + public string Name => $"Product {Id}"; + } + } + private static class MixedRequirementSourceA { public class Query diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Nested_With_Forwarded_Variable_At_Each_Level.yaml b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Nested_With_Forwarded_Variable_At_Each_Level.yaml index 081c09e31f6..6b53318ed62 100644 --- a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Nested_With_Forwarded_Variable_At_Each_Level.yaml +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Nested_With_Forwarded_Variable_At_Each_Level.yaml @@ -514,9 +514,6 @@ operationPlan: - 1 parentNodeId: 1 requirements: - - name: __fusion_1_id - selectionMap: >- - id - name: __fusion_2_id selectionMap: >- id diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Should_ResolveNestedLookupInDeferredPayload_When_DeferredFieldTraversesSubgraphs.yaml b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Should_ResolveNestedLookupInDeferredPayload_When_DeferredFieldTraversesSubgraphs.yaml new file mode 100644 index 00000000000..aab025c2d0e --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/__snapshots__/DeferTests.Defer_Should_ResolveNestedLookupInDeferredPayload_When_DeferredFieldTraversesSubgraphs.yaml @@ -0,0 +1,363 @@ +title: Defer_Should_ResolveNestedLookupInDeferredPayload_When_DeferredFieldTraversesSubgraphs +request: + document: | + { + products { + name + ... @defer(label: "deeper") { + related { + name + } + } + } + } +stableResponseStream: | + { + "kind": "stable-stream", + "payloadCount": 2, + "initial": { + "data": { + "products": [ + { + "name": "Product 1" + }, + { + "name": "Product 2" + } + ] + } + }, + "pending": [ + { + "id": "0", + "path": [ + "products" + ], + "label": "deeper" + } + ], + "incremental": [ + { + "id": "0", + "path": [ + "products" + ], + "subPath": [ + 0 + ], + "data": { + "related": { + "name": "Product 11" + } + } + }, + { + "id": "0", + "path": [ + "products" + ], + "subPath": [ + 1 + ], + "data": { + "related": { + "name": "Product 12" + } + } + } + ], + "completed": [ + { + "id": "0" + } + ], + "final": { + "data": { + "products": [ + { + "name": "Product 1", + "related": { + "name": "Product 11" + } + }, + { + "name": "Product 2", + "related": { + "name": "Product 12" + } + } + ] + } + } + } +sourceSchemas: + - name: A + schema: | + schema { + query: Query + } + + type Query { + products: [Product!]! + productById(id: ID!): Product @lookup @internal + } + + type Product @key(fields: "id") { + id: ID! + } + interactions: + - request: + accept: application/graphql-response+json; charset=utf-8, application/json; charset=utf-8, application/jsonl; charset=utf-8, text/event-stream; charset=utf-8 + document: | + query Op_62b0b849_1 { + products { + id + } + } + response: + results: + - | + { + "data": { + "products": [ + { + "id": "1" + }, + { + "id": "2" + } + ] + } + } + - name: B + schema: | + schema { + query: Query + } + + type Query { + productById(id: ID!): Product @lookup @internal + } + + type Product @key(fields: "id") { + id: ID! + related: Product! + } + interactions: + - request: + accept: application/jsonl; charset=utf-8, text/event-stream; charset=utf-8, application/graphql-response+json; charset=utf-8, application/json; charset=utf-8 + document: | + query Op_defer_1($__fusion_1_id: ID!) { + productById(id: $__fusion_1_id) { + related { + id + } + } + } + variables: | + [ + { + "__fusion_1_id": "1" + }, + { + "__fusion_1_id": "2" + } + ] + response: + contentType: application/jsonl; charset=utf-8 + results: + - | + { + "data": { + "productById": { + "related": { + "id": "11" + } + } + } + } + - | + { + "data": { + "productById": { + "related": { + "id": "12" + } + } + } + } + - name: C + schema: | + schema { + query: Query + } + + type Query { + productById(id: ID!): Product @lookup @internal + } + + type Product @key(fields: "id") { + id: ID! + name: String! + } + interactions: + - request: + accept: application/jsonl; charset=utf-8, text/event-stream; charset=utf-8, application/graphql-response+json; charset=utf-8, application/json; charset=utf-8 + document: | + query Op_62b0b849_2($__fusion_1_id: ID!) { + productById(id: $__fusion_1_id) { + name + } + } + variables: | + [ + { + "__fusion_1_id": "1" + }, + { + "__fusion_1_id": "2" + } + ] + response: + contentType: application/jsonl; charset=utf-8 + results: + - | + { + "data": { + "productById": { + "name": "Product 1" + } + } + } + - | + { + "data": { + "productById": { + "name": "Product 2" + } + } + } + - request: + accept: application/jsonl; charset=utf-8, text/event-stream; charset=utf-8, application/graphql-response+json; charset=utf-8, application/json; charset=utf-8 + document: | + query Op_defer_2($__fusion_2_id: ID!) { + productById(id: $__fusion_2_id) { + name + } + } + variables: | + [ + { + "__fusion_2_id": "11" + }, + { + "__fusion_2_id": "12" + } + ] + response: + contentType: application/jsonl; charset=utf-8 + results: + - | + { + "data": { + "productById": { + "name": "Product 11" + } + } + } + - | + { + "data": { + "productById": { + "name": "Product 12" + } + } + } +operationPlan: + operation: + - document: | + { + products { + name + id @fusion__requirement + } + } + hash: 63f2d1e7b181db80e47d921ff44e5f27 + searchSpace: 1 + expandedNodes: 2 + nodes: + - id: 1 + type: Operation + schema: A + operation: | + query Op_62b0b849_1 { + products { + id + } + } + - id: 2 + type: Operation + schema: C + operation: | + query Op_62b0b849_2($__fusion_1_id: ID!) { + productById(id: $__fusion_1_id) { + name + } + } + source: $.productById + target: $.products + requirements: + - name: __fusion_1_id + selectionMap: >- + id + dependencies: + - id: 1 + deliveryGroups: + - id: 0 + path: $.products + label: deeper + incrementalPlans: + - deliveryGroupIds: + - 0 + parentNodeId: 2 + requirements: + - name: __fusion_1_id + selectionMap: >- + id + nodes: + - id: 1 + type: Operation + schema: B + operation: | + query Op_defer_1($__fusion_1_id: ID!) { + productById(id: $__fusion_1_id) { + related { + id + } + } + } + source: $.productById + target: $.products + requirements: + - name: __fusion_1_id + selectionMap: >- + id + dependencies: + - parentNodeId: 1 + - id: 2 + type: Operation + schema: C + operation: | + query Op_defer_2($__fusion_2_id: ID!) { + productById(id: $__fusion_2_id) { + name + } + } + source: $.productById + target: $.products.related + requirements: + - name: __fusion_2_id + selectionMap: >- + id + dependencies: + - id: 1 diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/ApolloDeferPlanningTests.cs b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/ApolloDeferPlanningTests.cs new file mode 100644 index 00000000000..7732dde5fc6 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/ApolloDeferPlanningTests.cs @@ -0,0 +1,57 @@ +namespace HotChocolate.Fusion.Planning; + +public sealed class ApolloDeferPlanningTests : FusionTestBase +{ + [Fact] + public void Plan_Should_KeepNestedLookupRequirementInDeferredScope_When_DeferTraversesSubgraphs() + { + // arrange + var schema = ComposeSchema( + """ + extend schema @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"]) + + type Query { + products: [Product!]! + } + + type Product @key(fields: "id") { + id: ID! + } + """, + """ + extend schema @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"]) + + type Product @key(fields: "id") { + id: ID! + related: Product! + } + """, + """ + extend schema @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"]) + + type Product @key(fields: "id") { + id: ID! + name: String! + } + """); + + // act + var plan = PlanOperation( + schema, + """ + { + products { + name + ... @defer(label: "deeper") { + related { + name + } + } + } + } + """); + + // assert + MatchSnapshot(plan); + } +} diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/ApolloDeferPlanningTests.Plan_Should_KeepNestedLookupRequirementInDeferredScope_When_DeferTraversesSubgraphs.yaml b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/ApolloDeferPlanningTests.Plan_Should_KeepNestedLookupRequirementInDeferredScope_When_DeferTraversesSubgraphs.yaml new file mode 100644 index 00000000000..3c9c3ade1d0 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Planning/__snapshots__/ApolloDeferPlanningTests.Plan_Should_KeepNestedLookupRequirementInDeferredScope_When_DeferTraversesSubgraphs.yaml @@ -0,0 +1,93 @@ +operation: + - document: | + { + products { + name + id @fusion__requirement + } + } + hash: 123456789101112 + searchSpace: 1 + expandedNodes: 2 +nodes: + - id: 1 + type: Operation + schema: a + operation: | + query Op_123456789101112_1 { + products { + id + } + } + - id: 2 + type: ApolloOperation + schema: c + operation: | + query($representations: [_Any!]!) { + _entities(representations: $representations) { + ... on Product { + name + } + } + } + source: $._entities + target: $.products + requirements: + - name: __fusion_1_id + selectionMap: >- + id + dependencies: + - id: 1 +deliveryGroups: + - id: 0 + path: $.products + label: deeper +incrementalPlans: + - deliveryGroupIds: + - 0 + parentNodeId: 2 + requirements: + - name: __fusion_1_id + selectionMap: >- + id + nodes: + - id: 1 + type: ApolloOperation + schema: b + operation: | + query($representations: [_Any!]!) { + _entities(representations: $representations) { + ... on Product { + related { + id + } + } + } + } + source: $._entities + target: $.products + requirements: + - name: __fusion_1_id + selectionMap: >- + id + dependencies: + - parentNodeId: 1 + - id: 2 + type: ApolloOperation + schema: c + operation: | + query($representations: [_Any!]!) { + _entities(representations: $representations) { + ... on Product { + name + } + } + } + source: $._entities + target: $.products.related + requirements: + - name: __fusion_2_id + selectionMap: >- + id + dependencies: + - id: 1