Skip to content

Commit 769b1d3

Browse files
lukemurraylukemurray
committed
clean up SelectWithNullCheck fix
1 parent 22ef3ca commit 769b1d3

4 files changed

Lines changed: 175 additions & 1 deletion

File tree

‎src/EntityGraphQL/Compiler/Util/ExpressionUtil.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,7 @@ bool finalExecution
585585
return (baseExp, null);
586586
var selector = Expression.Lambda(memberInit, currentContextParam);
587587
var isQueryable = baseExp.Type.IsGenericTypeQueryable();
588+
588589
Expression call;
589590
if (nullCheck || isAsync)
590591
call = Expression.Call(

‎src/EntityGraphQL/Extensions/EnumerableExtensions.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,19 @@ public static IEnumerable<TSource> WhereWhen<TSource>(this IEnumerable<TSource>
6565
return awaitedSource.Select(selector);
6666
}
6767

68+
/// <summary>
69+
/// Converts <c>Task{TCollection}</c> to <c>Task{IEnumerable{TSource}}</c>.
70+
/// Used internally when an async service field returns a concrete collection type
71+
/// (e.g. <c>Task{List{T}}</c>, <c>Task{ICollection{T}}</c>) rather than exactly
72+
/// <c>Task{IEnumerable{T}}</c>, so that <see cref="SelectWithNullCheck{TSource,TResult}(Task{IEnumerable{TSource}},Func{TSource,TResult})"/>
73+
/// can be resolved by <c>Expression.Call</c> (which requires an exact type match on generic arguments).
74+
/// </summary>
75+
internal static async Task<IEnumerable<TSource>> ToEnumerableTask<TCollection, TSource>(this Task<TCollection> task)
76+
where TCollection : IEnumerable<TSource>
77+
{
78+
return await task;
79+
}
80+
6881
public static IEnumerable<TResult>? SelectWithNullCheck<TSource, TResult>(this IEnumerable<TSource>? source, Func<TSource, TResult> selector, bool returnEmptyList)
6982
{
7083
if (source == null)

‎src/EntityGraphQL/Schema/Field.cs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
using System;
21
using System.Collections.Generic;
32
using System.Linq;
43
using System.Linq.Expressions;
4+
using System.Threading.Tasks;
55
using EntityGraphQL.Compiler;
66
using EntityGraphQL.Compiler.Util;
77
using EntityGraphQL.Extensions;
@@ -268,7 +268,35 @@ protected void SetUpField(LambdaExpression fieldExpression, bool withServices, b
268268
// do the above before unwrapping Task<>
269269
returnType = SchemaBuilder.GetReturnType(returnType, out bool returnsAsync);
270270
if (returnsAsync)
271+
{
271272
IsAsync = true;
273+
// Normalize Task<TCollection> → Task<IEnumerable<T>> in the stored expression so
274+
// all downstream compilation always sees a consistent Task<IEnumerable<T>> type.
275+
// Expression.Call (used in MakeSelectWithDynamicType) resolves generic overloads by
276+
// exact type match, so Task<List<T>>, Task<ICollection<T>> etc. would fail to match
277+
// the SelectWithNullCheck(Task<IEnumerable<T>>, …) overload without this.
278+
// We do it here — once at registration — rather than at every compilation site.
279+
if (ResolveExpression != null)
280+
{
281+
var exprType = ResolveExpression.Type;
282+
if (exprType.IsGenericType && exprType.GetGenericTypeDefinition() == typeof(Task<>))
283+
{
284+
var taskInner = exprType.GetGenericArguments()[0];
285+
var elementType = returnType.GetEnumerableOrArrayType();
286+
if (elementType != null)
287+
{
288+
var exactEnumType = typeof(IEnumerable<>).MakeGenericType(elementType);
289+
if (
290+
taskInner != exactEnumType
291+
&& taskInner.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>) && i.GetGenericArguments()[0] == elementType)
292+
)
293+
{
294+
ResolveExpression = Expression.Call(typeof(EnumerableExtensions), nameof(EnumerableExtensions.ToEnumerableTask), [taskInner, elementType], ResolveExpression);
295+
}
296+
}
297+
}
298+
}
299+
}
272300

273301
if (!isAsync && IsAsync)
274302
throw new EntityGraphQLSchemaException("Field is synchronous but returns an async type. Use ResolveAsync() or resolve the field expression with .GetAwaiter().GetResult()");

‎src/tests/EntityGraphQL.Tests/QueryTests/AsyncTests.cs‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,118 @@ public void TestAsyncServiceFieldReturningListOnSingleObjectFromQueryArg()
292292
Assert.Single(catalog);
293293
}
294294

295+
/// <summary>
296+
/// Same as TestAsyncServiceFieldReturningListOnSingleObjectFromQueryArg but toolCatalog
297+
/// is on Step (a list element), so the path is workflow -> steps (list) -> toolCatalog (service).
298+
/// </summary>
299+
[Fact]
300+
public void TestAsyncServiceFieldReturningListNestedInsideList()
301+
{
302+
var schema = SchemaBuilder.FromObject<WorkflowContext>();
303+
schema.AddType<WorkflowTool>("WorkflowTool").AddAllFields();
304+
305+
schema.UpdateType<Step>(type =>
306+
{
307+
type.AddField("toolCatalog", "Resolved tool catalog for this step.").ResolveAsync<IWorkflowToolCatalogService>((step, tools) => tools.GetWorkflowToolCatalogAsync(step.WorkflowId));
308+
});
309+
310+
var gql = new QueryRequest
311+
{
312+
Query =
313+
@"query BuilderGetWorkflow($id: String) {
314+
workflow(id: $id) {
315+
id
316+
steps {
317+
id
318+
toolCatalog { name }
319+
}
320+
}
321+
}",
322+
Variables = new QueryVariables { { "id", "wf-1" } },
323+
};
324+
325+
var context = new WorkflowContext();
326+
context.Workflows.Add(
327+
new Workflow
328+
{
329+
Id = "wf-1",
330+
Name = "My Workflow",
331+
Steps = [new Step { Id = "s-1", WorkflowId = "wf-1" }],
332+
}
333+
);
334+
335+
var serviceCollection = new ServiceCollection();
336+
serviceCollection.AddSingleton<IWorkflowToolCatalogService>(new WorkflowToolCatalogService());
337+
338+
var res = schema.ExecuteRequestWithContext(gql, context, serviceCollection.BuildServiceProvider(), null);
339+
Assert.Null(res.Errors);
340+
Assert.NotNull(res.Data);
341+
dynamic workflow = res.Data!["workflow"]!;
342+
Assert.Equal("wf-1", workflow.id);
343+
var steps = (IEnumerable<dynamic>)workflow.steps;
344+
Assert.Single(steps);
345+
var catalog = (IEnumerable<dynamic>)((dynamic)steps.First()).toolCatalog;
346+
Assert.Single(catalog);
347+
Assert.Equal("tool-a", ((dynamic)catalog.First()).name);
348+
}
349+
350+
/// <summary>
351+
/// Same as TestAsyncServiceFieldReturningListNestedInsideList but the service returns
352+
/// Task{List{T}} (a concrete collection type) rather than Task{IEnumerable{T}}.
353+
/// Verifies that SelectWithNullCheck can be called regardless of the concrete async return type.
354+
/// </summary>
355+
[Fact]
356+
public void TestAsyncServiceFieldReturningConcreteListNestedInsideList()
357+
{
358+
var schema = SchemaBuilder.FromObject<WorkflowContext>();
359+
schema.AddType<WorkflowTool>("WorkflowTool").AddAllFields();
360+
361+
schema.UpdateType<Step>(type =>
362+
{
363+
type.AddField("toolCatalog", "Resolved tool catalog for this step.")
364+
.ResolveAsync<IWorkflowToolCatalogServiceConcreteReturn>((step, tools) => tools.GetWorkflowToolCatalogAsync(step.WorkflowId));
365+
});
366+
367+
var gql = new QueryRequest
368+
{
369+
Query =
370+
@"query BuilderGetWorkflow($id: String) {
371+
workflow(id: $id) {
372+
id
373+
steps {
374+
id
375+
toolCatalog { name }
376+
}
377+
}
378+
}",
379+
Variables = new QueryVariables { { "id", "wf-1" } },
380+
};
381+
382+
var context = new WorkflowContext();
383+
context.Workflows.Add(
384+
new Workflow
385+
{
386+
Id = "wf-1",
387+
Name = "My Workflow",
388+
Steps = [new Step { Id = "s-1", WorkflowId = "wf-1" }],
389+
}
390+
);
391+
392+
var serviceCollection = new ServiceCollection();
393+
serviceCollection.AddSingleton<IWorkflowToolCatalogServiceConcreteReturn>(new WorkflowToolCatalogServiceConcreteReturn());
394+
395+
var res = schema.ExecuteRequestWithContext(gql, context, serviceCollection.BuildServiceProvider(), null);
396+
Assert.Null(res.Errors);
397+
Assert.NotNull(res.Data);
398+
dynamic workflow = res.Data!["workflow"]!;
399+
Assert.Equal("wf-1", workflow.id);
400+
var steps = (IEnumerable<dynamic>)workflow.steps;
401+
Assert.Single(steps);
402+
var catalog = (IEnumerable<dynamic>)((dynamic)steps.First()).toolCatalog;
403+
Assert.Single(catalog);
404+
Assert.Equal("tool-a", ((dynamic)catalog.First()).name);
405+
}
406+
295407
private class WorkflowContext
296408
{
297409
public List<Workflow> Workflows { get; set; } = [];
@@ -301,6 +413,13 @@ private class Workflow
301413
{
302414
public string Id { get; set; } = string.Empty;
303415
public string Name { get; set; } = string.Empty;
416+
public List<Step> Steps { get; set; } = [];
417+
}
418+
419+
private class Step
420+
{
421+
public string Id { get; set; } = string.Empty;
422+
public string WorkflowId { get; set; } = string.Empty;
304423
}
305424

306425
private class WorkflowTool
@@ -322,6 +441,19 @@ public System.Threading.Tasks.Task<IEnumerable<WorkflowTool>> GetWorkflowToolCat
322441
}
323442
}
324443

444+
private interface IWorkflowToolCatalogServiceConcreteReturn
445+
{
446+
System.Threading.Tasks.Task<List<WorkflowTool>> GetWorkflowToolCatalogAsync(string workflowId);
447+
}
448+
449+
private class WorkflowToolCatalogServiceConcreteReturn : IWorkflowToolCatalogServiceConcreteReturn
450+
{
451+
public System.Threading.Tasks.Task<List<WorkflowTool>> GetWorkflowToolCatalogAsync(string workflowId)
452+
{
453+
return System.Threading.Tasks.Task.FromResult(new List<WorkflowTool> { new WorkflowTool { Name = "tool-a" } });
454+
}
455+
}
456+
325457
private class JobContext
326458
{
327459
[GraphQLIgnore]

0 commit comments

Comments
 (0)