From 8d6d7cd09b5fe30eb6c25904208b3e142cf8c66b Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 18 Oct 2026 23:20:01 +0300 Subject: [PATCH] fix(filter): treat every IEnumerable property as a collection A filter on a collection property failed for IReadOnlyList, IReadOnlyCollection, HashSet, arrays, and classes that inherit from List. The == operator and the other comparison operators failed on a direct collection property of every type. - Detect a collection and its element type from IEnumerable with one pair of helpers in Extensions.cs. - A comparison on a direct collection property matches when any element matches, the same as a nested path. - == null and != null on a collection property check the property for null. A nested path keeps the element comparison. - != on a collection emits !(z == value), because EF Core 10 with Npgsql can not translate z != value on a primitive collection. Fixes #201 --- .../skills/verify-querykit/features/README.md | 2 +- .../features/collections-and-nested.md | 7 + .../verify-querykit/harness/Driver/Model.cs | 2 + .../Tests/CollectionTypeFilterTests.cs | 71 +++++++ QueryKit.UnitTests/AliasCultureTests.cs | 2 +- .../CollectionTypeFilterTests.cs | 191 ++++++++++++++++++ QueryKit.UnitTests/FilterParserTests.cs | 4 +- QueryKit.UnitTests/HasConversionTests.cs | 2 +- QueryKit/Extensions.cs | 27 ++- QueryKit/FilterParser.cs | 40 ++-- QueryKit/Operators/ComparisonOperator.cs | 57 +++--- QueryKit/PropertyResolver.cs | 9 +- 12 files changed, 348 insertions(+), 66 deletions(-) create mode 100644 QueryKit.IntegrationTests/Tests/CollectionTypeFilterTests.cs create mode 100644 QueryKit.UnitTests/CollectionTypeFilterTests.cs diff --git a/.agents/skills/verify-querykit/features/README.md b/.agents/skills/verify-querykit/features/README.md index d0a8fdb..87fab11 100644 --- a/.agents/skills/verify-querykit/features/README.md +++ b/.agents/skills/verify-querykit/features/README.md @@ -12,7 +12,7 @@ In the commands below, `qk` means `.agents/skills/verify-querykit/qk`. ## Seed data -The driver model is `Recipe`. Every recipe has one `Author` and zero or more `Ingredients`. The seed data is fixed in `harness/Driver/Model.cs`. +The driver model is `Recipe`. Every recipe has one `Author` and zero or more `Ingredients`. `Labels` is a `string[]` (a Postgres `text[]` column) with the same values as `Tags`, which is a `List`. The seed data is fixed in `harness/Driver/Model.cs`. | Title | Author.Name | Rating | Price | IsVegetarian | Visibility | CreatedAt (UTC) | DateOfOrigin | Sku | Serving | ServeTime | Directions | Tags | Ingredients (Name:Stock) | |---|---|---|---|---|---|---|---|---|---|---|---|---|---| diff --git a/.agents/skills/verify-querykit/features/collections-and-nested.md b/.agents/skills/verify-querykit/features/collections-and-nested.md index 35f2049..736409e 100644 --- a/.agents/skills/verify-querykit/features/collections-and-nested.md +++ b/.agents/skills/verify-querykit/features/collections-and-nested.md @@ -9,6 +9,9 @@ A developer filters on a property of a related object with a dotted path, and on - `collection-all` matches when all children match (`Ingredients.Stock %>= 1`). - `collection-count` compares the number of children (`Ingredients #== 0`). - `collection-primitive-has` matches a value in a primitive list (`Tags ^$`, `Tags ^$*`, `Tags !^$`). +- `collection-primitive-compare` compares each value in a primitive list with a comparison operator (`Tags == "dinner"`, `Tags != "dinner"`, `Tags _= "br"`). The filter matches when any value matches. +- `collection-primitive-null` checks the list itself for null (`Tags == null`, `Tags != null`). +- `collection-array` gives the same results on an array property (`Labels`) as on a list property (`Tags`). ## How to get to it (user POV) @@ -27,9 +30,13 @@ Preconditions: - **All children.** Run `qk run collections-all --filter 'Ingredients.Stock %>= 1'`. Both targets give `Beef Stew`, `Salt Bread`, and `Plain Water`. The `expression` contains `.All(`. - **Count.** Run `qk run collections-count --filter 'Ingredients #== 0'`. Both targets give `["Plain Water"]`. - **Primitive list, case-insensitive.** Run `qk run collections-has --filter 'Tags ^$* "winner"'`. Both targets give `["Salt Bread"]`. The stored tag is `Winner`. +- **Primitive list, compare.** Run `qk run collections-compare --filter 'Tags == "dinner"'`. Both targets give `["Beef Stew"]`. The `expression` contains `.Any(`. Then run `qk run collections-compare-not --filter 'Tags != "dinner"'`. Both targets give `Pancakes` and `Salt Bread`. +- **Primitive list, null.** Run `qk run collections-null --filter 'Tags != null'`. Both targets give all four recipes. The `expression` has no `.Any(`. +- **Array property.** Run each filter above again with `Labels` in place of `Tags`. Each filter gives the same `titles` as the `Tags` filter. The `sql` reads the `text[]` column `r."Labels"`. ## Gotchas - `All` is true for an empty collection. So `Plain Water`, which has no ingredients, matches `Ingredients.Stock %>= 1`. - `Tags ^$ "winner"` without `*` does not match `Winner`. Use `^$*` for a case-insensitive match. +- `Tags != "dinner"` matches a recipe that has any tag other than `dinner`. It does not match a recipe with no tags. Use `!^$` to get the recipes that do not have the tag. - The driver adds `Include` for `Author` and `Ingredients` after QueryKit runs. The first statement in `sql` is the filter query. EF Core loads the ingredients in a second statement. diff --git a/.agents/skills/verify-querykit/harness/Driver/Model.cs b/.agents/skills/verify-querykit/harness/Driver/Model.cs index a4ff3ca..634c5cf 100644 --- a/.agents/skills/verify-querykit/harness/Driver/Model.cs +++ b/.agents/skills/verify-querykit/harness/Driver/Model.cs @@ -37,6 +37,7 @@ public class Recipe public string? Serving { get; set; } public TimeOnly? ServeTime { get; set; } public List Tags { get; set; } = []; + public string[] Labels { get; set; } = []; public Guid AuthorId { get; set; } public Author Author { get; set; } = null!; public List Ingredients { get; set; } = []; @@ -104,6 +105,7 @@ private static Recipe Recipe(int n, string title, Author author, int rating, dec Serving = serving, ServeTime = serveTime, Tags = tags, + Labels = tags.ToArray(), AuthorId = author.Id, Author = author, Ingredients = ingredients diff --git a/QueryKit.IntegrationTests/Tests/CollectionTypeFilterTests.cs b/QueryKit.IntegrationTests/Tests/CollectionTypeFilterTests.cs new file mode 100644 index 0000000..7b9f632 --- /dev/null +++ b/QueryKit.IntegrationTests/Tests/CollectionTypeFilterTests.cs @@ -0,0 +1,71 @@ +namespace QueryKit.IntegrationTests.Tests; + +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using SharedTestingHelper.Fakes.Recipes; + +// A list endpoint often projects an entity to a DTO that exposes a collection as IReadOnlyList, and then applies the filter. +public class CollectionTypeFilterTests : TestBase +{ + private static readonly (string Operation, string[] Expected)[] Cases = + [ + ("""^$ "dinner" """, ["stew"]), + ("""!^$ "dinner" """, ["pancakes", "bread", "water"]), + ("""== "dinner" """, ["stew"]), + ("""==* "DINNER" """, ["stew"]), + ("""!= "dinner" """, ["pancakes", "bread"]), + ("""!=* "DINNER" """, ["pancakes", "bread"]), + ("""%!= "dinner" """, ["pancakes", "bread", "water"]), + ("""%== "dinner" """, ["stew", "water"]), + ("""@= "ee" """, ["pancakes"]), + ("""_= "br" """, ["pancakes", "bread"]), + ("#== 2", ["pancakes", "bread"]), + ]; + + public static TheoryData Filters() + { + var data = new TheoryData(); + foreach (var property in new[] { nameof(RecipeDto.List), nameof(RecipeDto.ReadOnlyList), nameof(RecipeDto.ReadOnlyCollection) }) + foreach (var (operation, expected) in Cases) + data.Add(property, operation, expected); + return data; + } + + [Theory] + [MemberData(nameof(Filters))] + public async Task collection_filter_on_a_projected_dto_matches_for_every_collection_type(string property, string operation, string[] expectedTitles) + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var prefix = $"collection type {Guid.NewGuid()} "; + var pancakes = new FakeRecipeBuilder().WithTitle($"{prefix}pancakes").Build().SetTags(["breakfast", "sweet"]); + var stew = new FakeRecipeBuilder().WithTitle($"{prefix}stew").Build().SetTags(["dinner"]); + var bread = new FakeRecipeBuilder().WithTitle($"{prefix}bread").Build().SetTags(["bread", "Winner"]); + var water = new FakeRecipeBuilder().WithTitle($"{prefix}water").Build().SetTags([]); + await testingServiceScope.InsertAsync(pancakes, stew, bread, water); + + // Act + var recipes = await testingServiceScope.DbContext().Recipes + .Where(x => x.Title.StartsWith(prefix)) + .Select(x => new RecipeDto + { + Title = x.Title, + List = x.Tags, + ReadOnlyList = x.Tags, + ReadOnlyCollection = x.Tags, + }) + .ApplyQueryKitFilter($"{property} {operation}") + .ToListAsync(); + + // Assert + recipes.Select(x => x.Title[prefix.Length..]).Should().BeEquivalentTo(expectedTitles); + } + + public class RecipeDto + { + public string Title { get; init; } = null!; + public List List { get; init; } = null!; + public IReadOnlyList ReadOnlyList { get; init; } = null!; + public IReadOnlyCollection ReadOnlyCollection { get; init; } = null!; + } +} diff --git a/QueryKit.UnitTests/AliasCultureTests.cs b/QueryKit.UnitTests/AliasCultureTests.cs index ea5919e..b455c60 100644 --- a/QueryKit.UnitTests/AliasCultureTests.cs +++ b/QueryKit.UnitTests/AliasCultureTests.cs @@ -58,7 +58,7 @@ public void query_name_does_not_match_with_the_dotted_and_dotless_i_rules_of_tr_ { var config = new QueryKitConfiguration(settings => { - settings.Property(x => x.Rating).HasQueryName(queryName); + settings.Property(x => x.Rating!).HasQueryName(queryName); }); var turkish = () => WithCulture("tr-TR", () => FilterParser.ParseFilter(input, config)); diff --git a/QueryKit.UnitTests/CollectionTypeFilterTests.cs b/QueryKit.UnitTests/CollectionTypeFilterTests.cs new file mode 100644 index 0000000..25b5f27 --- /dev/null +++ b/QueryKit.UnitTests/CollectionTypeFilterTests.cs @@ -0,0 +1,191 @@ +namespace QueryKit.UnitTests; + +using System.Collections.ObjectModel; +using FluentAssertions; + +// A filter on a collection property gives the same rows for every type that implements IEnumerable. +public class CollectionTypeFilterTests +{ + public static readonly string[] CollectionProperties = + [ + nameof(Tagged.List), nameof(Tagged.ReadOnlyList), nameof(Tagged.ReadOnlyCollection), nameof(Tagged.Collection), + nameof(Tagged.Set), nameof(Tagged.Array), nameof(Tagged.Enumerable), nameof(Tagged.Custom), + ]; + + private static readonly (string Operation, string[] Expected)[] TextCases = + [ + ("""^$ "dinner" """, ["stew"]), + ("""!^$ "dinner" """, ["pancakes", "bread", "water"]), + ("""^$* "WINNER" """, ["bread"]), + ("""== "dinner" """, ["stew"]), + ("""==* "DINNER" """, ["stew"]), + ("""!= "dinner" """, ["pancakes", "bread"]), + ("""!=* "DINNER" """, ["pancakes", "bread"]), + ("""%!= "dinner" """, ["pancakes", "bread", "water"]), + ("""%== "dinner" """, ["stew", "water"]), + ("""@= "ee" """, ["pancakes"]), + ("""_= "br" """, ["pancakes", "bread"]), + ("""_-= "er" """, ["stew", "bread"]), + ("#== 2", ["pancakes", "bread"]), + ("#> 0", ["pancakes", "stew", "bread"]), + ]; + + public static TheoryData TextFilters() + { + var data = new TheoryData(); + foreach (var property in CollectionProperties) + foreach (var (operation, expected) in TextCases) + data.Add(property, operation, expected); + return data; + } + + [Theory] + [MemberData(nameof(TextFilters))] + public void text_collection_filter_matches_for_every_collection_type(string property, string operation, string[] expected) + { + var result = TaggedRows().AsQueryable().ApplyQueryKitFilter($"{property} {operation}").ToList(); + + result.Select(x => x.Name).Should().BeEquivalentTo(expected); + } + + [Theory] + [InlineData(nameof(Scored.Array), "> 6", new[] { "a" })] + [InlineData(nameof(Scored.ReadOnlyList), "> 6", new[] { "a" })] + [InlineData(nameof(Scored.Array), "== 5", new[] { "b" })] + [InlineData(nameof(Scored.ReadOnlyList), "== 5", new[] { "b" })] + [InlineData(nameof(Scored.Array), "^$ 5", new[] { "b" })] + [InlineData(nameof(Scored.ReadOnlyList), "^$ 5", new[] { "b" })] + [InlineData(nameof(Scored.Array), "#== 0", new[] { "c" })] + [InlineData(nameof(Scored.ReadOnlyList), "#== 0", new[] { "c" })] + public void number_collection_filter_matches_for_every_collection_type(string property, string operation, string[] expected) + { + var rows = new[] + { + new Scored("a", [1, 9]), + new Scored("b", [5]), + new Scored("c", []), + }; + + var result = rows.AsQueryable().ApplyQueryKitFilter($"{property} {operation}").ToList(); + + result.Select(x => x.Name).Should().BeEquivalentTo(expected); + } + + [Theory] + [InlineData("Tags == null", new[] { "none" })] + [InlineData("Tags != null", new[] { "empty", "one" })] + [InlineData("Labels == null", new[] { "none" })] + [InlineData("Labels != null", new[] { "empty", "one" })] + [InlineData("ReadOnlyTags == null", new[] { "none" })] + [InlineData("ReadOnlyTags != null", new[] { "empty", "one" })] + public void null_comparison_on_a_collection_checks_the_collection_for_null(string input, string[] expected) + { + var rows = new[] + { + new NullableTagged { Name = "none" }, + new NullableTagged { Name = "empty", Tags = [], Labels = [], ReadOnlyTags = [] }, + new NullableTagged { Name = "one", Tags = ["x"], Labels = ["x"], ReadOnlyTags = ["x"] }, + }; + + var result = rows.AsQueryable().ApplyQueryKitFilter(input).ToList(); + + result.Select(x => x.Name).Should().BeEquivalentTo(expected); + } + + [Theory] + [InlineData("ChildArray.Name == null", new[] { "second" })] + [InlineData("ChildReadOnlyList.Name == null", new[] { "second" })] + [InlineData("ChildReadOnlyList.Name != null", new[] { "first", "second" })] + public void null_comparison_on_a_nested_path_compares_each_element(string input, string[] expected) + { + var rows = new[] + { + new Parent("first", [new Child("a")]), + new Parent("second", [new Child("a"), new Child(null)]), + }; + + var result = rows.AsQueryable().ApplyQueryKitFilter(input).ToList(); + + result.Select(x => x.Name).Should().BeEquivalentTo(expected); + } + + [Theory] + [InlineData("""ChildArray.Name == "b" """)] + [InlineData("""ChildReadOnlyList.Name == "b" """)] + [InlineData("""ChildCustom.Name == "b" """)] + public void nested_path_through_a_collection_matches_for_every_collection_type(string input) + { + var rows = new[] + { + new Parent("first", [new Child("a")]), + new Parent("second", [new Child("a"), new Child("b")]), + }; + + var result = rows.AsQueryable().ApplyQueryKitFilter(input).ToList(); + + result.Select(x => x.Name).Should().Equal("second"); + } + + private static Tagged[] TaggedRows() => + [ + new("pancakes", ["breakfast", "sweet"]), + new("stew", ["dinner"]), + new("bread", ["bread", "Winner"]), + new("water", []), + ]; + + public class TagList : List + { + public TagList(IEnumerable tags) : base(tags) + { + } + } + + public class Tagged(string name, string[] tags) + { + public string Name { get; } = name; + public List List { get; } = [..tags]; + public IReadOnlyList ReadOnlyList { get; } = tags.ToList(); + public IReadOnlyCollection ReadOnlyCollection { get; } = new ReadOnlyCollection(tags); + public ICollection Collection { get; } = tags.ToList(); + public HashSet Set { get; } = [..tags]; + public string[] Array { get; } = tags; + public IEnumerable Enumerable { get; } = tags; + public TagList Custom { get; } = new(tags); + } + + public class Scored(string name, int[] scores) + { + public string Name { get; } = name; + public int[] Array { get; } = scores; + public IReadOnlyList ReadOnlyList { get; } = scores.ToList(); + } + + public class NullableTagged + { + public string Name { get; init; } = null!; + public List? Tags { get; init; } + public string[]? Labels { get; init; } + public IReadOnlyList? ReadOnlyTags { get; init; } + } + + public class ChildList : List + { + public ChildList(IEnumerable children) : base(children) + { + } + } + + public class Parent(string name, Child[] children) + { + public string Name { get; } = name; + public Child[] ChildArray { get; } = children; + public IReadOnlyList ChildReadOnlyList { get; } = children; + public ChildList ChildCustom { get; } = new(children); + } + + public class Child(string? name) + { + public string? Name { get; } = name; + } +} diff --git a/QueryKit.UnitTests/FilterParserTests.cs b/QueryKit.UnitTests/FilterParserTests.cs index f5c84c1..158a1f1 100644 --- a/QueryKit.UnitTests/FilterParserTests.cs +++ b/QueryKit.UnitTests/FilterParserTests.cs @@ -607,7 +607,7 @@ public void simple_child_collection_for_string_not_equal() var input = """Ingredients.Name != "flour" """; var filterExpression = FilterParser.ParseFilter(input); filterExpression.ToDisplayString().Should() - .Be(""""x => x.Ingredients.Select(y => y.Name).Any(z => (z != "flour"))""""); + .Be(""""x => x.Ingredients.Select(y => y.Name).Any(z => Not((z == "flour")))""""); } [Fact] @@ -616,7 +616,7 @@ public void simple_child_collection_for_string_case_insensitive_not_equal() var input = """Ingredients.Name !=* "flour" """; var filterExpression = FilterParser.ParseFilter(input); filterExpression.ToDisplayString().Should() - .Be(""""x => x.Ingredients.Select(y => y.Name).Any(z => (z.ToLower() != "flour".ToLower()))""""); + .Be(""""x => x.Ingredients.Select(y => y.Name).Any(z => Not((z.ToLower() == "flour".ToLower())))""""); } [Fact] diff --git a/QueryKit.UnitTests/HasConversionTests.cs b/QueryKit.UnitTests/HasConversionTests.cs index b09c3f1..35716ef 100644 --- a/QueryKit.UnitTests/HasConversionTests.cs +++ b/QueryKit.UnitTests/HasConversionTests.cs @@ -400,7 +400,7 @@ public void nullable_int_with_query_name_and_has_conversion_compares_the_int() }; var config = new QueryKitConfiguration(config => { - config.Property(x => x.Number).HasQueryName("count").HasConversion(); + config.Property(x => x.Number!).HasQueryName("count").HasConversion(); }); // Act diff --git a/QueryKit/Extensions.cs b/QueryKit/Extensions.cs index d5025cf..3d29b00 100644 --- a/QueryKit/Extensions.cs +++ b/QueryKit/Extensions.cs @@ -6,4 +6,29 @@ public static bool IsNullable(this Type type) { return type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>); } -} \ No newline at end of file + + // A collection is a type that implements IEnumerable, for example List, IReadOnlyList, HashSet, or T[]. + // A string is not a collection. + public static bool IsCollection(this Type type) + { + return type.GetCollectionElementType() != null; + } + + // The T of IEnumerable. A type such as Dictionary or a class that inherits from List has other generic arguments, so read the interface. + public static Type? GetCollectionElementType(this Type type) + { + if (type == typeof(string)) + { + return null; + } + + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + { + return type.GetGenericArguments()[0]; + } + + return type.GetInterfaces() + .FirstOrDefault(x => x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + ?.GetGenericArguments()[0]; + } +} diff --git a/QueryKit/FilterParser.cs b/QueryKit/FilterParser.cs index 1b5daad..fa4e395 100644 --- a/QueryKit/FilterParser.cs +++ b/QueryKit/FilterParser.cs @@ -599,6 +599,13 @@ private static Expression CreateRightExprForProperty(Expression leftExpr, string } } + // == null and != null on a collection property compare the property with null. On a nested path (a Select), they compare each element with null. + if (right == "null" && !rightIsQuotedLiteral && leftExpr is MemberExpression && targetType.IsCollection() + && (op.Name == ComparisonOperator.EqualsOperator().Name || op.Name == ComparisonOperator.NotEqualsOperator().Name)) + { + return Expression.Constant(null, targetType); + } + // Check if this property uses HasConversion. QueryKit reads a type that it knows (a number, an enum, a Guid) by its own type, // like v1.14.2 did for a property with a query name, so the conversion applies only to a type that it can not read. if (config?.PropertyMappings != null && !string.IsNullOrEmpty(propertyPath) && !CanCreateRightExprFromType(targetType)) @@ -638,12 +645,12 @@ private static Expression CreateRightExprForProperty(Expression leftExpr, string private static bool CanCreateRightExprFromType(Type type) { var targetType = TransformTargetTypeIfNullable(type); - return IsEnumerable(type) || TypeConversionFunctions.ContainsKey(targetType) || targetType.IsEnum || targetType == typeof(object); + return type.IsCollection() || TypeConversionFunctions.ContainsKey(targetType) || targetType.IsEnum || targetType == typeof(object); } private static Expression CreateRightExprFromType(Type leftExprType, string right, bool rightIsQuotedLiteral, ComparisonOperator op) { - var isEnumerable = IsEnumerable(leftExprType); + var isEnumerable = leftExprType.IsCollection(); var targetType = leftExprType; if (isEnumerable) { @@ -651,7 +658,7 @@ private static Expression CreateRightExprFromType(Type leftExprType, string righ { return FilterValue.Create(intVal, typeof(int)); } - targetType = targetType.GetGenericArguments()[0]; + targetType = targetType.GetCollectionElementType()!; return CreateRightExprFromType(targetType, right, rightIsQuotedLiteral, op); } @@ -850,17 +857,6 @@ private static Type TransformTargetTypeIfNullable(Type targetType) return targetType; } - private static bool IsEnumerable(Type targetType) - { - if (targetType == typeof(string)) - { - return false; - } - return targetType.IsGenericType && targetType.GetGenericTypeDefinition() == typeof(IEnumerable<>) || - targetType.GetInterfaces() - .Any(x => x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IEnumerable<>)); - } - // New arithmetic-aware comparison parser - only matches expressions in parentheses with arithmetic operators private static Parser ArithmeticComparisonExprParser(ParameterExpression parameter, IQueryKitConfiguration? config) { @@ -1280,16 +1276,16 @@ private static Expression CreateMemberExpression(ParameterExpression parameter, { return memberPath.Split('.').Aggregate((Expression)parameter, (expr, memberName) => { - if (expr is MemberExpression member && IsEnumerable(member.Type)) + if (expr is MemberExpression member && member.Type.IsCollection()) { - var genericArgType = member.Type.GetGenericArguments()[0]; + var genericArgType = member.Type.GetCollectionElementType()!; var innerParameter = Expression.Parameter(genericArgType, "y"); Expression lambdaBody = Expression.PropertyOrField(innerParameter, memberName); var propertyType = lambdaBody.Type; - if (IsEnumerable(propertyType)) + if (propertyType.IsCollection()) { - propertyType = propertyType.GetGenericArguments()[0]; + propertyType = propertyType.GetCollectionElementType()!; var selectManyMethod = typeof(Enumerable).GetMethods() .First(m => m.Name == "SelectMany" && m.GetParameters().Length == 2) @@ -1325,8 +1321,8 @@ private static Expression CreateMemberExpression(ParameterExpression parameter, var lambdaBody = Expression.PropertyOrField(innerParameter, memberName); var propertyType = lambdaBody.Type; - var linqMethod = IsEnumerable(propertyType) ? "SelectMany" : "Select"; - var resultType = IsEnumerable(propertyType) ? propertyType.GetGenericArguments()[0] : propertyType; + var linqMethod = propertyType.IsCollection() ? "SelectMany" : "Select"; + var resultType = propertyType.IsCollection() ? propertyType.GetCollectionElementType()! : propertyType; var selectMethod = typeof(Enumerable).GetMethods() .First(m => m.Name == linqMethod && m.GetParameters().Length == 2) @@ -1416,12 +1412,12 @@ private static Parser PropertyListComparisonExprParser( private static Type? GetInnerGenericType(Type type) { - if (!IsEnumerable(type)) + if (!type.IsCollection()) { return type; } - var innerGenericType = type.GetGenericArguments()[0]; + var innerGenericType = type.GetCollectionElementType()!; return GetInnerGenericType(innerGenericType); } diff --git a/QueryKit/Operators/ComparisonOperator.cs b/QueryKit/Operators/ComparisonOperator.cs index ad8e760..7d06e91 100644 --- a/QueryKit/Operators/ComparisonOperator.cs +++ b/QueryKit/Operators/ComparisonOperator.cs @@ -1,6 +1,5 @@ namespace QueryKit.Operators; -using System.Collections; using System.Linq.Expressions; using System.Reflection; using Ardalis.SmartEnum; @@ -205,7 +204,7 @@ public EqualsType(bool caseInsensitive = false, bool usesAll = false) : base("== public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, Expression.Equal, UsesAll); } @@ -244,9 +243,10 @@ public NotEqualsType(bool caseInsensitive = false, bool usesAll = false) : base( public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { - return GetCollectionExpression(left, right, Expression.NotEqual, UsesAll); + // EF Core 10 with Npgsql can not translate Any(z => z != value) on a primitive collection. Any(z => !(z == value)) translates to the same SQL. + return GetCollectionExpression(left, right, (x, y) => Expression.Not(Expression.Equal(x, y)), UsesAll); } if (CaseInsensitive && left.Type == typeof(string) && right.Type == typeof(string)) @@ -283,7 +283,7 @@ public GreaterThanType(bool caseInsensitive = false, bool usesAll = false) : bas public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, Expression.GreaterThan, UsesAll); } @@ -304,7 +304,7 @@ public LessThanType(bool caseInsensitive = false, bool usesAll = false) : base(" public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, Expression.LessThan, UsesAll); } @@ -324,7 +324,7 @@ public GreaterThanOrEqualType(bool caseInsensitive = false, bool usesAll = false } public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, Expression.GreaterThanOrEqual, UsesAll); } @@ -344,7 +344,7 @@ public LessThanOrEqualType(bool caseInsensitive = false, bool usesAll = false) : public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, Expression.LessThanOrEqual, UsesAll); } @@ -367,7 +367,7 @@ public override Expression GetExpression(Expression left, Expression right, T { EnsureStringValueIsNotNull(right, Operator()); - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, "Contains", false, UsesAll); } @@ -404,7 +404,7 @@ public override Expression GetExpression(Expression left, Expression right, T { EnsureStringValueIsNotNull(right, Operator()); - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, "StartsWith", false, UsesAll); } @@ -441,7 +441,7 @@ public override Expression GetExpression(Expression left, Expression right, T { EnsureStringValueIsNotNull(right, Operator()); - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, "EndsWith", false, UsesAll); } @@ -478,7 +478,7 @@ public override Expression GetExpression(Expression left, Expression right, T { EnsureStringValueIsNotNull(right, Operator()); - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, "Contains", true, UsesAll); } @@ -515,7 +515,7 @@ public override Expression GetExpression(Expression left, Expression right, T { EnsureStringValueIsNotNull(right, Operator()); - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, "StartsWith", true, UsesAll); } @@ -552,7 +552,7 @@ public override Expression GetExpression(Expression left, Expression right, T { EnsureStringValueIsNotNull(right, Operator()); - if (left.Type.IsGenericType && left.Type.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, "EndsWith", true, UsesAll); } @@ -787,11 +787,7 @@ public HasType(bool caseInsensitive = false, bool usesAll = false) : base("^$", public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && - (left.Type.GetGenericTypeDefinition() == typeof(List<>) || - left.Type.GetGenericTypeDefinition() == typeof(ICollection<>) || - left.Type.GetGenericTypeDefinition() == typeof(IList<>) || - typeof(IEnumerable<>).IsAssignableFrom(left.Type.GetGenericTypeDefinition()))) + if (ComparesElements(left, right)) { return GetCollectionExpression(left, right, Expression.Equal, UsesAll); } @@ -810,11 +806,7 @@ public DoesNotHaveType(bool caseInsensitive = false, bool usesAll = false) : bas public override bool IsCountOperator() => false; public override Expression GetExpression(Expression left, Expression right, Type? dbContextType, CaseInsensitiveMode caseMode = CaseInsensitiveMode.Lower) { - if (left.Type.IsGenericType && - (left.Type.GetGenericTypeDefinition() == typeof(List<>) || - left.Type.GetGenericTypeDefinition() == typeof(ICollection<>) || - left.Type.GetGenericTypeDefinition() == typeof(IList<>) || - typeof(IEnumerable<>).IsAssignableFrom(left.Type.GetGenericTypeDefinition()))) + if (ComparesElements(left, right)) { return Expression.Not(GetCollectionExpression(left, right, Expression.Equal, UsesAll)); } @@ -1012,9 +1004,13 @@ internal static List GetAliasMatches(IQueryKitConfiguratio return matches; } + // A comparison on a collection compares each element with the right side. A right side of the collection type itself (a null) compares the whole collection. + private static bool ComparesElements(Expression left, Expression right) + => left.Type.IsCollection() && right.Type != left.Type; + private Expression GetCollectionExpression(Expression left, Expression right, Func comparisonFunction, bool usesAll) { - var xParameter = Expression.Parameter(left.Type.GetGenericArguments()[0], "z"); + var xParameter = Expression.Parameter(left.Type.GetCollectionElementType()!, "z"); Expression body; if (CaseInsensitive && xParameter.Type == typeof(string) && right.Type == typeof(string)) @@ -1033,14 +1029,14 @@ private Expression GetCollectionExpression(Expression left, Expression right, Fu var anyMethod = typeof(Enumerable) .GetMethods() .Single(m => m.Name == (usesAll ? "All" : "Any") && m.GetParameters().Length == 2) - .MakeGenericMethod(left.Type.GetGenericArguments()[0]); + .MakeGenericMethod(left.Type.GetCollectionElementType()!); return Expression.Call(anyMethod, left, anyLambda); } private Expression GetCollectionExpression(Expression left, Expression right, string methodName, bool negate, bool usesAll) { - var xParameter = Expression.Parameter(left.Type.GetGenericArguments()[0], "z"); + var xParameter = Expression.Parameter(left.Type.GetCollectionElementType()!, "z"); Expression body; if (CaseInsensitive && xParameter.Type == typeof(string) && right.Type == typeof(string)) @@ -1059,7 +1055,7 @@ private Expression GetCollectionExpression(Expression left, Expression right, st var anyMethod = typeof(Enumerable) .GetMethods() .Single(m => m.Name == (usesAll ? "All" : "Any") && m.GetParameters().Length == 2) - .MakeGenericMethod(left.Type.GetGenericArguments()[0]); + .MakeGenericMethod(left.Type.GetCollectionElementType()!); return negate ? Expression.Not(Expression.Call(anyMethod, left, anyLambda)) @@ -1068,13 +1064,12 @@ private Expression GetCollectionExpression(Expression left, Expression right, st private Expression GetCountExpression(Expression left, Expression right, string methodName) { - var leftAsEnumerableType = left.Type.GetInterface(nameof(IEnumerable)); - if (leftAsEnumerableType == null) + var leftGenericType = left.Type.GetCollectionElementType(); + if (leftGenericType == null) { throw new QueryKitParsingException("Left expression should be of type IEnumerable"); } - var leftGenericType = left.Type.GetGenericArguments()[0]; var rightType = right.Type; if (rightType != typeof(int)) diff --git a/QueryKit/PropertyResolver.cs b/QueryKit/PropertyResolver.cs index 8d60f98..539de05 100644 --- a/QueryKit/PropertyResolver.cs +++ b/QueryKit/PropertyResolver.cs @@ -101,9 +101,9 @@ private static PropertyReference Resolve(Type rootType, string reference, string foreach (var segment in path.Split('.')) { - while (IsCollection(currentType)) + while (currentType.IsCollection()) { - currentType = currentType.GetGenericArguments()[0]; + currentType = currentType.GetCollectionElementType()!; } var member = (MemberInfo?)currentType.GetProperty(segment, PublicMemberFlags) @@ -128,9 +128,4 @@ private static PropertyReference Resolve(Type rootType, string reference, string private const BindingFlags PublicMemberFlags = BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance; private const BindingFlags NonPublicMemberFlags = BindingFlags.IgnoreCase | BindingFlags.NonPublic | BindingFlags.Instance; - - private static bool IsCollection(Type type) - => type != typeof(string) && type.IsGenericType && - (type.GetGenericTypeDefinition() == typeof(IEnumerable<>) || - type.GetInterfaces().Any(x => x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IEnumerable<>))); }