diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 8c7d5fd8..cdc66454 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 f710c08f..ae619f03 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 37a93aeb..36ccf973 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; }