From e44b241d4b147bae4ce5ec01e5f70ae74727cc84 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Tue, 13 Oct 2026 09:54:39 +1100 Subject: [PATCH] Load only what a projection reads from a collection A projection that reads a collection through LINQ, such as GroupIds = _.AccessGroups.Select(_ => _.AccessGroupId).ToList() reads AccessGroupId off the lambda's own parameter. ProjectionAnalyzer only followed member chains from the projection's parameter, so it recorded AccessGroups and nothing under it. A navigation with nothing recorded under it is loaded whole: every column of every item, with whatever the request selected under it loaded through an Include instead of a nested select. The scalars the lambdas of the chain read from an item are now recorded under the collection, so only they are loaded. The collection is still loaded whole when the lambdas need more than scalars of an item: the items themselves come out, a navigation of an item is read, an item is passed to a method, or there is no lambda. Version 36.1.0. --- src/Directory.Build.props | 2 +- .../GraphApi/ProjectionAnalyzer.cs | 202 +++++++++++++++++- src/Tests/ProjectionAnalyzerTests.cs | 88 ++++++++ 3 files changed, 290 insertions(+), 2 deletions(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 8c7d5fd8e..cdc664540 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;NU5104;CS1573;CS9107;NU1608;NU1109 - 36.0.0 + 36.1.0 preview 1.0.0 EntityFrameworkCore, EntityFramework, GraphQL diff --git a/src/GraphQL.EntityFramework/GraphApi/ProjectionAnalyzer.cs b/src/GraphQL.EntityFramework/GraphApi/ProjectionAnalyzer.cs index f710c08f6..ae619f031 100644 --- a/src/GraphQL.EntityFramework/GraphApi/ProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework/GraphApi/ProjectionAnalyzer.cs @@ -16,6 +16,93 @@ sealed class PropertyAccessVisitor(IReadOnlyCollection para public HashSet AccessedProperties { get; } = [with(StringComparer.OrdinalIgnoreCase)]; protected override Expression VisitMember(MemberExpression node) + { + var path = PathFromParameter(node); + if (path is not null) + { + AccessedProperties.Add(path); + } + + return base.VisitMember(node); + } + + /// + /// A collection read through LINQ, such as + /// _.Children.Where(child => child.Active).Select(child => child.Name), reads + /// properties of its items inside the lambdas, off a parameter that is not the + /// projection's own. Only the collection was recorded, and a navigation with nothing + /// recorded under it is loaded whole, with every column of every item. The properties are + /// recorded under the collection instead, so only they are loaded. + /// + protected override Expression VisitMethodCall(MethodCallExpression node) + { + AddItemProperties(node); + return base.VisitMethodCall(node); + } + + void AddItemProperties(MethodCallExpression node) + { + if (!IsLinq(node)) + { + return; + } + + // The calls from this one down to the collection they read + var lambdas = new List(); + Expression current = node; + while (current is MethodCallExpression call && + IsLinq(call)) + { + foreach (var argument in call.Arguments.Skip(1)) + { + if (StripQuote(argument) is LambdaExpression lambda) + { + lambdas.Add(lambda); + } + } + + current = StripConvert(call.Arguments[0]); + } + + if (current is not MemberExpression collection || + !TryGetItemType(collection.Type, out var itemType)) + { + return; + } + + var path = PathFromParameter(collection); + if (path is null) + { + return; + } + + // The items themselves come out, as from a Where with no Select after it, so + // whatever uses them can read any property + if (node.Type.IsAssignableTo(itemType) || + (TryGetItemType(node.Type, out var resultItemType) && + resultItemType.IsAssignableTo(itemType))) + { + return; + } + + var reads = new ItemReadVisitor(itemType); + foreach (var lambda in lambdas) + { + reads.Visit(lambda.Body); + } + + if (reads.ReadsWholeItem) + { + return; + } + + foreach (var name in reads.Properties) + { + AccessedProperties.Add($"{path}.{name}"); + } + } + + string? PathFromParameter(MemberExpression node) { var path = new List(); Expression? current = node; @@ -41,10 +128,123 @@ protected override Expression VisitMember(MemberExpression node) parameters.Contains(param) && path.Count > 0) { - AccessedProperties.Add(string.Join('.', path)); + return string.Join('.', path); + } + + return null; + } + } + + /// + /// What the lambdas of a LINQ chain read from the items of the collection. A scalar read + /// directly off an item is a property to load. Anything else, such as the item itself, a + /// navigation of it, or the item passed to a method, means the whole item is needed. + /// + sealed class ItemReadVisitor(Type itemType) : ExpressionVisitor + { + public HashSet Properties { get; } = [with(StringComparer.OrdinalIgnoreCase)]; + public bool ReadsWholeItem { get; private set; } + + protected override Expression VisitMember(MemberExpression node) + { + if (node.Expression is not null && + IsItem(StripConvert(node.Expression))) + { + if (IsScalar(node.Type)) + { + Properties.Add(node.Member.Name); + } + else + { + ReadsWholeItem = true; + } + + return node; } return base.VisitMember(node); } + + protected override Expression VisitParameter(ParameterExpression node) + { + if (IsItem(node)) + { + ReadsWholeItem = true; + } + + return node; + } + + // The parameters are the items coming in, not a use of them + protected override Expression VisitLambda(Expression node) + { + Visit(node.Body); + return node; + } + + bool IsItem(Expression expression) => + expression is ParameterExpression parameter && + parameter.Type.IsAssignableTo(itemType); + + static bool IsScalar(Type type) + { + type = Nullable.GetUnderlyingType(type) ?? type; + return type.IsValueType || + type == typeof(string) || + type == typeof(byte[]); + } + } + + static bool IsLinq(MethodCallExpression call) => + call.Arguments.Count > 0 && + (call.Method.DeclaringType == typeof(Enumerable) || + call.Method.DeclaringType == typeof(Queryable)); + + static Expression StripQuote(Expression expression) + { + if (expression is UnaryExpression { NodeType: ExpressionType.Quote } quote) + { + return quote.Operand; + } + + return expression; + } + + static Expression StripConvert(Expression expression) + { + while (expression is UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.TypeAs } unary) + { + expression = unary.Operand; + } + + return expression; + } + + // The entity a collection navigation holds. A string is a sequence of chars, and a byte + // array of bytes, but neither is a collection of items. + static bool TryGetItemType(Type type, [NotNullWhen(true)] out Type? itemType) + { + itemType = null; + if (type == typeof(string)) + { + return false; + } + + foreach (var candidate in type.GetInterfaces().Append(type)) + { + if (candidate.IsGenericType && + candidate.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + { + var argument = candidate.GetGenericArguments()[0]; + if (argument.IsClass && + argument != typeof(string)) + { + itemType = argument; + return true; + } + } + } + + return false; } } diff --git a/src/Tests/ProjectionAnalyzerTests.cs b/src/Tests/ProjectionAnalyzerTests.cs index 37a93aeb6..36ccf973e 100644 --- a/src/Tests/ProjectionAnalyzerTests.cs +++ b/src/Tests/ProjectionAnalyzerTests.cs @@ -101,4 +101,92 @@ public void NoCast() Assert.Contains("Name", paths); Assert.Contains("Status", paths); } + + class Item + { + public Guid Id { get; set; } + public string Name { get; set; } = null!; + public bool Active { get; set; } + public Nav? Nav { get; set; } + } + + class EntityWithItems : BaseEntity + { + public List Items { get; set; } = null!; + } + + static void AssertPaths(Expression> projection, params string[] expected) => + Assert.Equal( + expected.Order(), + ProjectionAnalyzer.ExtractPropertyPaths(projection).Order()); + + // The properties are read off the lambda's own parameter, so were not recorded, and the + // collection was loaded whole + [Fact] + public void CollectionSelect() => + AssertPaths( + _ => _.Items.Select(item => item.Id).ToList(), + "Items", "Items.Id"); + + [Fact] + public void CollectionChain() => + AssertPaths( + _ => _.Items + .Where(item => item.Active) + .OrderBy(item => item.Name) + .Select(item => item.Id) + .ToList(), + "Items", "Items.Active", "Items.Name", "Items.Id"); + + [Fact] + public void CollectionAggregate() => + AssertPaths( + _ => new + { + Any = _.Items.Any(item => item.Active), + Count = _.Items.Count(item => item.Name == "a") + }, + "Items", "Items.Active", "Items.Name"); + + // The items come out, so whatever uses them can read any property + [Fact] + public void CollectionReturningItemsIsWhole() => + AssertPaths( + _ => _.Items.Where(item => item.Active).ToList(), + "Items"); + + [Fact] + public void CollectionReturningAnItemIsWhole() => + AssertPaths( + _ => _.Items.FirstOrDefault(item => item.Active), + "Items"); + + [Fact] + public void CollectionSelectingTheItemIsWhole() => + AssertPaths( + _ => _.Items.Select(item => new { item, item.Name }).ToList(), + "Items"); + + // A navigation of an item is not a column of it + [Fact] + public void CollectionReadingItemNavigationIsWhole() => + AssertPaths( + _ => _.Items.Select(item => item.Nav!.Value).ToList(), + "Items"); + + [Fact] + public void CollectionPassingItemToMethodIsWhole() => + AssertPaths( + _ => _.Items.Select(item => Describe(item)).ToList(), + "Items"); + + // Nothing says which properties a count needs, so this is unchanged + [Fact] + public void CollectionWithNoLambdaIsWhole() => + AssertPaths( + _ => _.Items.Count(), + "Items"); + + static string Describe(Item item) => + item.Name; }