From cc02ee174d46ad8612b7fc105ff47eeb8ac72cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20Dang=C3=A5rden?= Date: Tue, 8 Sep 2026 15:18:55 +0200 Subject: [PATCH] Translate the LINQ operators that materialise a sequence ToDictionary, ToLookup and GroupBy were classified Unsupported, so a client writing e.Nodes.ToDictionary(n => n.ResourceReference, n => n, comparer) got "the LINQ operator 'ToDictionary' cannot be translated to a GraphQL selection" instead of a query. Nothing about them is untranslatable: the query tree only records which fields are mentioned and where they hang, and their lambdas read fields of a single sequence, which is the pass-through shape - the keying itself happens on the client after the response is deserialized. Add them, along with the operators the list had never caught up with (Append, Contains, CountBy, Index, Prepend, Shuffle, TryGetNonEnumeratedCount), and say in LinqOperatorKind what is left: operators that combine several sequences or fold into an accumulator. The list stays an allow-list rather than becoming shape based. An operator we have not listed throws, which is loud and one line to fix; guessing pass-through for an unknown operator would instead produce a query missing fields, and that surfaces as nulls in a deserialized object with no error anywhere. Pass-through lambdas now bind their later parameters too when the parameter is a sequence of the element type, so GroupBy's result selector - (key, orders) => ... - can select through orders instead of reporting it as unbound. The Unsupported message gained a hint naming the way out, since the operator can almost always be applied to the result of ExecuteAsync, and translation failures now throw GraphQueryTranslationException carrying the offending Expression. It derives from NotSupportedException, which is what the parser threw before, so existing catch clauses keep working. --- .../GraphQueryTranslationException.cs | 35 +++++++++++ .../Visitors/LinqOperator.cs | 19 +++++- .../Visitors/QueryExpressionVisitor.cs | 49 +++++++++++++-- .../ExpressionParserTests.cs | 63 +++++++++++++++++-- test/Linq2GraphQL.Tests/QueryOperatorTests.cs | 46 +++++++++++++- 5 files changed, 196 insertions(+), 16 deletions(-) create mode 100644 src/Linq2GraphQL.Client/Exceptions/GraphQueryTranslationException.cs diff --git a/src/Linq2GraphQL.Client/Exceptions/GraphQueryTranslationException.cs b/src/Linq2GraphQL.Client/Exceptions/GraphQueryTranslationException.cs new file mode 100644 index 00000000..aaf409f7 --- /dev/null +++ b/src/Linq2GraphQL.Client/Exceptions/GraphQueryTranslationException.cs @@ -0,0 +1,35 @@ +using System.Linq.Expressions; + +namespace Linq2GraphQL.Client; + +/// +/// Thrown when an expression passed to Include or Select cannot be turned into a GraphQL +/// selection, either because it uses a construct that has no GraphQL equivalent or because it reads +/// something that is not part of the query. +/// +/// +/// The query text does not exist yet when translation fails, so unlike +/// and this +/// exception carries the offending instead. +/// It derives from , which is what the parser threw before this type +/// existed, so callers that catch that keep working. +/// +public class GraphQueryTranslationException : NotSupportedException +{ + public GraphQueryTranslationException(string message, Expression expression, string memberName = null) + : base(message) + { + Expression = expression; + MemberName = memberName; + } + + /// + /// The part of the expression that could not be translated. + /// + public Expression Expression { get; } + + /// + /// The LINQ operator or GraphQL member the failure is about, when it is about a single one. + /// + public string MemberName { get; } +} diff --git a/src/Linq2GraphQL.Client/Visitors/LinqOperator.cs b/src/Linq2GraphQL.Client/Visitors/LinqOperator.cs index 5bedfbfc..22879a3a 100644 --- a/src/Linq2GraphQL.Client/Visitors/LinqOperator.cs +++ b/src/Linq2GraphQL.Client/Visitors/LinqOperator.cs @@ -15,15 +15,18 @@ internal enum LinqOperatorKind Projection, /// - /// The operator keeps the source element type, or reduces the sequence to one of its elements or to a - /// scalar computed from them (Where, OrderBy, First, Count, ...). + /// The operator keeps the source element type, or reduces the sequence to one of its elements, to a + /// scalar computed from them, or to another container holding them (Where, OrderBy, + /// First, Count, ToDictionary, GroupBy, ...). /// The selection stays on the source node; any lambda is a client side predicate or key selector whose /// members still have to be fetched for it to work. /// PassThrough, /// - /// A known LINQ operator that cannot be translated, typically because it combines several sequences. + /// A known LINQ operator that cannot be translated, because it combines several sequences + /// (Concat, Join, Zip, ...) or folds the elements into an accumulator that the + /// selection cannot follow (Aggregate). /// Unsupported } @@ -40,12 +43,15 @@ internal static class LinqOperator [ "All", "Any", + "Append", "AsEnumerable", "AsQueryable", "Average", "Cast", "Chunk", + "Contains", "Count", + "CountBy", "DefaultIfEmpty", "Distinct", "DistinctBy", @@ -53,6 +59,8 @@ internal static class LinqOperator "ElementAtOrDefault", "First", "FirstOrDefault", + "GroupBy", + "Index", "Last", "LastOrDefault", "LongCount", @@ -65,7 +73,9 @@ internal static class LinqOperator "OrderBy", "OrderByDescending", "OrderDescending", + "Prepend", "Reverse", + "Shuffle", "Single", "SingleOrDefault", "Skip", @@ -78,8 +88,11 @@ internal static class LinqOperator "ThenBy", "ThenByDescending", "ToArray", + "ToDictionary", "ToHashSet", "ToList", + "ToLookup", + "TryGetNonEnumeratedCount", "Where" ]; diff --git a/src/Linq2GraphQL.Client/Visitors/QueryExpressionVisitor.cs b/src/Linq2GraphQL.Client/Visitors/QueryExpressionVisitor.cs index ef1c1bf9..08094570 100644 --- a/src/Linq2GraphQL.Client/Visitors/QueryExpressionVisitor.cs +++ b/src/Linq2GraphQL.Client/Visitors/QueryExpressionVisitor.cs @@ -131,7 +131,11 @@ private QueryNode ResolveCall(MethodCallExpression call) case LinqOperatorKind.Unsupported: throw Unsupported(call, - $"the LINQ operator '{call.Method.Name}' cannot be translated to a GraphQL selection"); + $"the LINQ operator '{call.Method.Name}' has no GraphQL equivalent and the selection cannot " + + "follow what it returns", + $"Select the fields the query needs and call '{call.Method.Name}' on the result of " + + "ExecuteAsync instead.", + call.Method.Name); default: // An ordinary method such as string.ToUpper(). It selects nothing itself, but its target and @@ -155,7 +159,8 @@ private QueryNode ResolveGraphMethod(MethodCallExpression call) if (parent == null) { throw Unsupported(call, - $"the target of '{call.Method.Name}' is not part of the query"); + $"the target of '{call.Method.Name}' is not part of the query", + memberName: call.Method.Name); } return parent.AddChildNode(new QueryNode(call.Method, arguments: GetArguments(call))); @@ -181,9 +186,21 @@ private QueryNode ResolveLinqOperator(MethodCallExpression call, LinqOperatorKin { // Predicates and key selectors run on the client, so the members they touch must be fetched, but // the selection itself stays on the sequence. + var elementType = GetElementType(call.Arguments[0].Type); + foreach (var lambda in lambdas) { Bind(lambda.Parameters[0], source); + + // GroupBy's result selector takes (key, elements): the elements are the sequence itself. + for (var i = 1; i < lambda.Parameters.Count; i++) + { + if (elementType != null && GetElementType(lambda.Parameters[i].Type) == elementType) + { + Bind(lambda.Parameters[i], source); + } + } + Select(lambda.Body); } @@ -217,6 +234,21 @@ private QueryNode ResolveLinqOperator(MethodCallExpression call, LinqOperatorKin return projected; } + /// + /// The element type of a sequence type, or null when the type is not a sequence. + /// + private static Type GetElementType(Type type) + { + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + { + return type.GetGenericArguments()[0]; + } + + return type.GetInterfaces() + .FirstOrDefault(e => e.IsGenericType && e.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + ?.GetGenericArguments()[0]; + } + private static List GetLambdas(MethodCallExpression call) { var lambdas = new List(); @@ -298,9 +330,10 @@ private QueryNode Scope(ParameterExpression parameter) return node; } - throw new NotSupportedException( + throw new GraphQueryTranslationException( $"Cannot translate '{parameter.Name}' of type '{parameter.Type.Name}': it is not bound to a field of " + - "the query. Only the parameters of the lambdas passed to Include and Select can be used to select fields."); + "the query. Only the parameters of the lambdas passed to Include and Select can be used to select fields.", + parameter); } private static Expression Unwrap(Expression expression) @@ -325,8 +358,12 @@ private static Expression Unwrap(Expression expression) return null; } - private static NotSupportedException Unsupported(Expression expression, string reason) + private static GraphQueryTranslationException Unsupported(Expression expression, string reason, + string hint = null, string memberName = null) { - return new NotSupportedException($"Cannot translate '{expression}' into a GraphQL selection: {reason}."); + var message = $"Cannot translate '{expression}' into a GraphQL selection: {reason}."; + + return new GraphQueryTranslationException(hint == null ? message : message + " " + hint, expression, + memberName); } } diff --git a/test/Linq2GraphQL.Tests/ExpressionParserTests.cs b/test/Linq2GraphQL.Tests/ExpressionParserTests.cs index 08ef9cfb..7b532695 100644 --- a/test/Linq2GraphQL.Tests/ExpressionParserTests.cs +++ b/test/Linq2GraphQL.Tests/ExpressionParserTests.cs @@ -282,13 +282,67 @@ public void CapturedValue_SelectsNothing() ShouldSelect((OrdersConnection e) => new { e.TotalCount, Captured = captured.OrderId }, "totalCount*"); } + [Fact] + public void MaterialisingOperator_FetchesWhatItReads() + { + ShouldSelect((OrdersConnection e) => e.Nodes.ToDictionary(n => n.OrderId, n => n.Customer), + """ + nodes* + nodes.orderId* + nodes.customer* + """); + } + + [Fact] + public void MaterialisingOperator_WithComparer_FetchesWhatItReads() + { + ShouldSelect( + (OrdersConnection e) => e.Nodes.ToDictionary(n => n.Customer.CustomerName, n => n, + StringComparer.OrdinalIgnoreCase), + """ + nodes* + nodes.customer + nodes.customer.customerName* + """); + } + + [Fact] + public void GroupBy_FetchesTheKeyFields() + { + ShouldSelect((OrdersConnection e) => e.Nodes.GroupBy(n => n.OrderId), + """ + nodes* + nodes.orderId* + """); + } + + [Fact] + public void GroupBy_WithResultSelector_BindsTheElements() + { + ShouldSelect( + (OrdersConnection e) => e.Nodes.GroupBy(n => n.Customer.CustomerName, + (name, orders) => orders.Select(o => o.OrderId)), + """ + nodes* + nodes.customer + nodes.customer.customerName* + nodes.orderId* + """); + } + [Fact] public void UnsupportedOperator_IsReported() { - var exception = Should.Throw(() => - Parse((OrdersConnection e) => e.Nodes.GroupBy(n => n.OrderId))); + var exception = Should.Throw(() => + Parse((OrdersConnection e) => e.Nodes.Concat(e.Nodes))); + + exception.Message.ShouldContain("Concat"); + exception.Message.ShouldContain("ExecuteAsync"); + exception.MemberName.ShouldBe("Concat"); + exception.Expression.ShouldNotBeNull(); - exception.Message.ShouldContain("GroupBy"); + // The parser threw NotSupportedException before the exception got a name of its own. + exception.ShouldBeAssignableTo(); } [Fact] @@ -302,8 +356,9 @@ public void UnboundParameter_IsReported() var root = new QueryNode(typeof(OrdersConnection), "root", null, null, true); - var exception = Should.Throw(() => Utilities.ParseExpression(lambda, root)); + var exception = Should.Throw(() => Utilities.ParseExpression(lambda, root)); exception.Message.ShouldContain("stray"); + exception.Expression.ShouldBe(stray); } } diff --git a/test/Linq2GraphQL.Tests/QueryOperatorTests.cs b/test/Linq2GraphQL.Tests/QueryOperatorTests.cs index 4d36d8e6..594acdcb 100644 --- a/test/Linq2GraphQL.Tests/QueryOperatorTests.cs +++ b/test/Linq2GraphQL.Tests/QueryOperatorTests.cs @@ -1,3 +1,4 @@ +using Linq2GraphQL.Client; using Linq2GraphQL.TestClient; using Shouldly; @@ -115,14 +116,53 @@ public async Task SelectMany_WithResultSelector() result.ShouldAllBe(e => e.CustomerName != null && e.OrderId != Guid.Empty); } + [Fact] + public async Task ToDictionary_FetchesTheKeyAndElementFields() + { + var query = sampleClient + .Query + .Orders() + .Select(e => e.Nodes.ToDictionary(n => n.OrderId.ToString(), n => n.Customer.CustomerName, + StringComparer.OrdinalIgnoreCase)); + + var request = await query.GetRequestAsync(); + request.Query.ShouldContain("customerName"); + request.Query.ShouldContain("orderId"); + + var result = await query.ExecuteAsync(); + + result.ShouldNotBeEmpty(); + result.Values.ShouldAllBe(e => e != null); + } + + [Fact] + public async Task GroupBy_FetchesTheKeyFields() + { + var query = sampleClient + .Query + .Orders() + .Select(e => e.Nodes + .GroupBy(n => n.Customer.CustomerName, (name, orders) => orders.Select(o => o.OrderId))); + + var request = await query.GetRequestAsync(); + request.Query.ShouldContain("customerName"); + + var result = (await query.ExecuteAsync()).ToList(); + + result.ShouldNotBeEmpty(); + result.SelectMany(e => e).ShouldAllBe(e => e != Guid.Empty); + } + [Fact] public async Task UnsupportedOperator_IsReported() { - var exception = Should.Throw(() => sampleClient + var exception = Should.Throw(() => sampleClient .Query .Orders() - .Select(e => e.Nodes.GroupBy(n => n.OrderId))); + .Select(e => e.Nodes.Concat(e.Nodes))); - exception.Message.ShouldContain("GroupBy"); + exception.Message.ShouldContain("Concat"); + exception.Message.ShouldContain("ExecuteAsync"); + exception.MemberName.ShouldBe("Concat"); } }