diff --git a/src/HotChocolate/Stitching/src/Stitching/Delegation/ExtractFieldQuerySyntaxRewriter.Context.cs b/src/HotChocolate/Stitching/src/Stitching/Delegation/ExtractFieldQuerySyntaxRewriter.Context.cs index 3d09454a052..f9ca9c3e510 100644 --- a/src/HotChocolate/Stitching/src/Stitching/Delegation/ExtractFieldQuerySyntaxRewriter.Context.cs +++ b/src/HotChocolate/Stitching/src/Stitching/Delegation/ExtractFieldQuerySyntaxRewriter.Context.cs @@ -34,18 +34,15 @@ public class Context public INamedOutputType? TypeContext { get; set; } - public DirectiveType Directive { get; set; } + public IOutputField? OutputField { get; set; } - public IOutputField OutputField { get; set; } + public IInputField? InputField { get; set; } - public IInputField InputField { get; set; } - - public IInputType InputType { get; set; } + public IInputType? InputType { get; set; } public ImmutableHashSet FragmentPath { get; set; } - public IDictionary Fragments - { get; } + public IDictionary Fragments { get; } public Context Clone() { diff --git a/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestHelper.cs b/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestHelper.cs index 0cdeb1eca00..74a2d1518f3 100644 --- a/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestHelper.cs +++ b/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestHelper.cs @@ -55,43 +55,53 @@ internal class MergeRequestHelper IQueryResult mergedResult, IEnumerable requests) { - var handledErrors = new HashSet(); - BufferedRequest? current = null; - QueryResultBuilder? resultBuilder = null; - - foreach (BufferedRequest request in requests) + try { - if (current is not null && resultBuilder is not null) - { - current.Promise.SetResult(resultBuilder.Create()); - } + var handledErrors = new HashSet(); + BufferedRequest? current = null; + QueryResultBuilder? resultBuilder = null; - try + foreach (BufferedRequest request in requests) { - current = request; - resultBuilder = ExtractResult(request.Aliases!, mergedResult, handledErrors); + if (current is not null && resultBuilder is not null) + { + current.Promise.SetResult(resultBuilder.Create()); + } + + try + { + current = request; + resultBuilder = ExtractResult(request.Aliases!, mergedResult, handledErrors); + } + catch (Exception ex) + { + current = null; + resultBuilder = null; + request.Promise.SetException(ex); + } } - catch (Exception ex) + + if (current is not null && resultBuilder is not null) { - current = null; - resultBuilder = null; - request.Promise.SetException(ex); + if (mergedResult.Errors is not null && + handledErrors.Count < mergedResult.Errors.Count) + { + foreach (IError error in mergedResult.Errors.Except(handledErrors)) + { + resultBuilder.AddError(error); + } + } + + handledErrors.Clear(); + current.Promise.SetResult(resultBuilder.Create()); } } - - if (current is not null && resultBuilder is not null) + catch (Exception ex) { - if (mergedResult.Errors is not null && - handledErrors.Count < mergedResult.Errors.Count) + foreach (BufferedRequest request in requests) { - foreach (IError error in mergedResult.Errors.Except(handledErrors)) - { - resultBuilder.AddError(error); - } + request.Promise.TrySetException(ex); } - - handledErrors.Clear(); - current.Promise.SetResult(resultBuilder.Create()); } } @@ -130,6 +140,7 @@ internal class MergeRequestHelper } } + // This method extracts the relevant data from a merged result for a specific result. private static QueryResultBuilder ExtractResult( IDictionary aliases, IQueryResult mergedResult, @@ -137,12 +148,42 @@ internal class MergeRequestHelper { var result = QueryResultBuilder.New(); - if (mergedResult.Data is not null) + // We first try to identify and copy data segments that belong to our specific result. + ExtractData(aliases, mergedResult, result); + + // After extracting the data, we will try to find errors that can be associated with + // our specific request for which we are trying to branch out the result. + ExtractErrors(aliases, mergedResult, handledErrors, result); + + // Last but not least we will copy all extensions and contextData over + // to the specific responses. + if (mergedResult.Extensions is not null) { - var data = new ResultMap(); - data.EnsureCapacity(aliases.Count); - var i = 0; + result.SetExtensions(mergedResult.Extensions); + } + if (mergedResult.ContextData is not null) + { + foreach (KeyValuePair item in mergedResult.ContextData) + { + result.SetContextData(item.Key, item.Value); + } + } + + return result; + } + + private static void ExtractData( + IDictionary aliases, + IQueryResult mergedResult, + QueryResultBuilder result) + { + var data = new ResultMap(); + data.EnsureCapacity(aliases.Count); + var i = 0; + + if (mergedResult.Data is not null) + { foreach (KeyValuePair alias in aliases) { if (mergedResult.Data.TryGetValue(alias.Key, out object? o)) @@ -150,10 +191,24 @@ internal class MergeRequestHelper data.SetValue(i++, alias.Value, o); } } - - result.SetData(data); } + else + { + foreach (KeyValuePair alias in aliases) + { + data.SetValue(i++, alias.Value, null); + } + } + + result.SetData(data); + } + private static void ExtractErrors( + IDictionary aliases, + IQueryResult mergedResult, + ICollection handledErrors, + QueryResultBuilder result) + { if (mergedResult.Errors is not null) { foreach (IError error in mergedResult.Errors) @@ -165,21 +220,6 @@ internal class MergeRequestHelper } } } - - if (mergedResult.Extensions is not null) - { - result.SetExtensions(mergedResult.Extensions); - } - - if (mergedResult.ContextData is not null) - { - foreach (KeyValuePair item in mergedResult.ContextData) - { - result.SetContextData(item.Key, item.Value); - } - } - - return result; } private static IError RewriteError(IError error, string responseName) diff --git a/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestRewriter.cs b/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestRewriter.cs index b0923cbfc3f..f67020f4e80 100644 --- a/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestRewriter.cs +++ b/src/HotChocolate/Stitching/src/Stitching/Requests/MergeRequestRewriter.cs @@ -102,6 +102,10 @@ protected override FieldNode RewriteField(FieldNode node, bool first) (p, c) => RewriteMany(p, c, RewriteArgument), current.WithArguments); + current = Rewrite(current, node.Directives, first, + (p, c) => RewriteMany(p, c, RewriteDirective), + current.WithDirectives); + if (node.SelectionSet != null) { current = Rewrite(current, node.SelectionSet, false, @@ -125,6 +129,23 @@ protected override FieldNode RewriteField(FieldNode node, bool first) false) : base.RewriteFragmentDefinition(node, false); + protected override DirectiveNode RewriteDirective( + DirectiveNode node, bool first) + { + if (node.Arguments.Count == 0) + { + return node; + } + + DirectiveNode current = node; + + current = Rewrite(current, current.Arguments, first, + (p, c) => RewriteMany(p, c, RewriteArgument), + current.WithArguments); + + return current; + } + protected override VariableNode RewriteVariable( VariableNode node, bool first) => node.WithName(node.Name.CreateNewName(_requestPrefix)); diff --git a/src/HotChocolate/Stitching/src/Stitching/Requests/RemoteRequestExecutor.cs b/src/HotChocolate/Stitching/src/Stitching/Requests/RemoteRequestExecutor.cs index ebf45e445f1..0b742f3a34d 100644 --- a/src/HotChocolate/Stitching/src/Stitching/Requests/RemoteRequestExecutor.cs +++ b/src/HotChocolate/Stitching/src/Stitching/Requests/RemoteRequestExecutor.cs @@ -27,13 +27,13 @@ internal sealed class RemoteRequestExecutor throw new ArgumentNullException(nameof(executor)); } - /// + /// public ISchema Schema => _executor.Schema; - /// + /// public IServiceProvider Services => _executor.Services; - /// + /// public Task ExecuteAsync( IQueryRequest request, CancellationToken cancellationToken = default) diff --git a/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedRedisSchemaTests.cs b/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedRedisSchemaTests.cs index 0878da82ddd..61654744621 100644 --- a/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedRedisSchemaTests.cs +++ b/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedRedisSchemaTests.cs @@ -48,7 +48,7 @@ public async Task AutoMerge_Schema() IHttpClientFactory httpClientFactory = CreateDefaultRemoteSchemas(configurationName); IDatabase database = _connection.GetDatabase(); - for (int i = 0; i < 10; i++) + for (var i = 0; i < 10; i++) { if (await database.SetLengthAsync(configurationName.Value) == 4) { @@ -142,7 +142,7 @@ public async Task AutoMerge_Execute() IHttpClientFactory httpClientFactory = CreateDefaultRemoteSchemas(configurationName); IDatabase database = _connection.GetDatabase(); - for (int i = 0; i < 10; i++) + for (var i = 0; i < 10; i++) { if (await database.SetLengthAsync(configurationName.Value) == 4) { @@ -187,7 +187,7 @@ public async Task AutoMerge_AddLocal_Field_Execute() IHttpClientFactory httpClientFactory = CreateDefaultRemoteSchemas(configurationName); IDatabase database = _connection.GetDatabase(); - for (int i = 0; i < 10; i++) + for (var i = 0; i < 10; i++) { if (await database.SetLengthAsync(configurationName.Value) == 4) { diff --git a/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedSchemaTests.cs b/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedSchemaTests.cs index d8c837de36f..0122b1741d3 100644 --- a/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedSchemaTests.cs +++ b/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/FederatedSchemaTests.cs @@ -126,6 +126,50 @@ public async Task AutoMerge_AddLocal_Field_Execute() result.ToJson().MatchSnapshot(); } + [Fact] + public async Task Directive_Variables_Are_Correctly_Rewritten() + { + // arrange + IHttpClientFactory httpClientFactory = CreateDefaultRemoteSchemas(); + + IRequestExecutor executor = + await new ServiceCollection() + .AddSingleton(httpClientFactory) + .AddGraphQL() + .AddQueryType(d => d.Name("Query").Field("local").Resolve("I am local.")) + .AddRemoteSchema(_accounts) + .AddRemoteSchema(_inventory) + .AddRemoteSchema(_products) + .AddRemoteSchema(_reviews) + .BuildRequestExecutorAsync(); + + // act + IExecutionResult result = await executor.ExecuteAsync( + @"query ($if1: Boolean! $if2: Boolean! $if3: Boolean! $if4: Boolean!) { + me { + id + alias1: name @include(if: $if1) + alias2: reviews @include(if: $if2) { + alias3: body @include(if: $if3) + alias4: product @include(if: $if4) { + upc + } + } + } + local + }", + new Dictionary + { + { "if1", true }, + { "if2", true }, + { "if3", true }, + { "if4", true }, + }); + + // assert + result.ToJson().MatchSnapshot(); + } + public TestServer CreateAccountsService() => Context.ServerFactory.Create( services => services diff --git a/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/__snapshots__/FederatedSchemaTests.Directive_Variables_Are_Correctly_Rewritten.snap b/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/__snapshots__/FederatedSchemaTests.Directive_Variables_Are_Correctly_Rewritten.snap new file mode 100644 index 00000000000..626d0a7718e --- /dev/null +++ b/src/HotChocolate/Stitching/test/Stitching.Tests/Integration/__snapshots__/FederatedSchemaTests.Directive_Variables_Are_Correctly_Rewritten.snap @@ -0,0 +1,23 @@ +{ + "data": { + "me": { + "id": 1, + "alias1": "Ada Lovelace", + "alias2": [ + { + "alias3": "Love it!", + "alias4": { + "upc": 1 + } + }, + { + "alias3": "Too expensive.", + "alias4": { + "upc": 2 + } + } + ] + }, + "local": "I am local." + } +} diff --git a/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/MergeRequestHelperTests.cs b/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/MergeRequestHelperTests.cs index 671a3f5db47..58718e338fd 100644 --- a/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/MergeRequestHelperTests.cs +++ b/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/MergeRequestHelperTests.cs @@ -148,5 +148,58 @@ public async Task Create_BufferedRequest_With_Mixed_Operations() .Select(t => Utf8GraphQLParser.Parse(t.Query!.AsSpan()).ToString(true))) .MatchSnapshot(); } + + [Fact] + public async Task Merge_Requests_With_Variables_On_Directives() + { + // arrange + ISchema schema = + await new ServiceCollection() + .AddGraphQL() + .AddCustomerSchema() + .BuildSchemaAsync(); + + var queryA = + @"query abc($id: ID $if: Boolean) { + customer(id: $id) { + name @include(id: $if) + } + }"; + + var queryB = + @"query abc($id: ID $if: Boolean) { + customer(id: $id) { + id @include(id: $if) + } + }"; + + IQueryRequest requestA = + QueryRequestBuilder.New() + .SetQuery(queryA) + .SetVariableValue("id", "1") + .SetVariableValue("if", true) + .Create(); + + IQueryRequest requestB = + QueryRequestBuilder.New() + .SetQuery(queryB) + .SetVariableValue("id", "1") + .SetVariableValue("if", true) + .Create(); + + var bufferedRequestA = BufferedRequest.Create(requestA, schema); + var bufferedRequestB = BufferedRequest.Create(requestB, schema); + + // act + IEnumerable<(IQueryRequest, IEnumerable)> mergeResult = + MergeRequestHelper.MergeRequests(new[] { bufferedRequestA, bufferedRequestB }); + + // assert + string.Join(Environment.NewLine + "-------" + Environment.NewLine, + mergeResult + .Select(t => t.Item1) + .Select(t => Utf8GraphQLParser.Parse(t.Query!.AsSpan()).ToString(true))) + .MatchSnapshot(); + } } } diff --git a/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/__snapshots__/MergeRequestHelperTests.Merge_Requests_With_Variables_On_Directives.snap b/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/__snapshots__/MergeRequestHelperTests.Merge_Requests_With_Variables_On_Directives.snap new file mode 100644 index 00000000000..0c63fba955e --- /dev/null +++ b/src/HotChocolate/Stitching/test/Stitching.Tests/Requests/__snapshots__/MergeRequestHelperTests.Merge_Requests_With_Variables_On_Directives.snap @@ -0,0 +1,8 @@ +query exec_batch($__0__id: ID, $__0__if: Boolean, $__1__id: ID, $__1__if: Boolean) { + __0__customer: customer(id: $__0__id) { + name @include(id: $__0__if) + } + __1__customer: customer(id: $__1__id) { + id @include(id: $__1__if) + } +}