From 519ab69bb6ebc7c50f7f491fa277819776a0ccb7 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sat, 3 Oct 2026 15:15:18 +0300 Subject: [PATCH] fix(filter)!: find HasConversion by the property path The 1.x fix finds the conversion by the property path only where v1.14.2 threw. This change uses the property path for every HasConversion lookup, so a query name no longer changes how a converted property is filtered. A number, an enum, or a Guid is still read by its own type, so HasConversion() on these properties keeps the v1.14.2 result with a query name, and no longer throws without one. BREAKING CHANGE: a null literal on a converted reference type without a query name matches null rows, not rows equal to a value built from the text null. A child path such as Email.Value on a converted parent with a query name compares the parent. A converted Guid without a query name gives a Guid constant instead of new Guid("...") in the expression text. The rows are the same. --- .../Tests/HasConversionTests.cs | 112 ++++++++++++++++-- QueryKit.UnitTests/HasConversionTests.cs | 93 +++++++++++++-- QueryKit/FilterParser.cs | 63 +++------- README.md | 2 + 4 files changed, 207 insertions(+), 63 deletions(-) diff --git a/QueryKit.IntegrationTests/Tests/HasConversionTests.cs b/QueryKit.IntegrationTests/Tests/HasConversionTests.cs index f3c0d8a..2c71895 100644 --- a/QueryKit.IntegrationTests/Tests/HasConversionTests.cs +++ b/QueryKit.IntegrationTests/Tests/HasConversionTests.cs @@ -100,11 +100,19 @@ public async Task can_filter_by_email_property_path_when_query_name_and_has_conv } [Fact] - public async Task email_value_with_query_name_and_has_conversion_compares_the_child() + public async Task can_filter_by_email_value_with_query_name_and_has_conversion() { // Arrange var testingServiceScope = new TestingServiceScope(); - var input = """Email.Value == "a@x.com" """; + var testEmail = $"{Guid.NewGuid()}@example.com"; + var person = new FakeTestingPersonBuilder() + .WithEmail(testEmail) + .Build(); + var personTwo = new FakeTestingPersonBuilder().Build(); + + await testingServiceScope.InsertAsync(person, personTwo); + + var input = $"""Email.Value == "{testEmail}" """; var config = new QueryKitConfiguration(config => { config.Property(x => x.Email).HasQueryName("mail").HasConversion(); @@ -113,11 +121,11 @@ public async Task email_value_with_query_name_and_has_conversion_compares_the_ch // Act var queryablePeople = testingServiceScope.DbContext().People; var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); - var act = () => appliedQueryable.ToListAsync(); + var people = await appliedQueryable.ToListAsync(); // Assert - await act.Should().ThrowExactlyAsync() - .WithMessage("The LINQ expression*could not be translated*"); + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); } [Fact] @@ -154,7 +162,96 @@ public async Task can_filter_by_null_email_with_query_name_and_has_conversion() } [Fact] - public async Task null_email_with_has_conversion_matches_no_row() + public async Task can_filter_by_nullable_int_with_query_name_and_has_conversion() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var title = Guid.NewGuid().ToString(); + var person = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithAge(41) + .Build(); + var personTwo = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithAge(42) + .Build(); + await testingServiceScope.InsertAsync(person, personTwo); + + var input = $"""Title == "{title}" && years == 41"""; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Age).HasQueryName("years").HasConversion(); + }); + + // Act + var people = await testingServiceScope.DbContext().People + .ApplyQueryKitFilter(input, config) + .ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + + [Fact] + public async Task can_filter_by_enum_with_query_name_and_has_conversion() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var title = Guid.NewGuid().ToString(); + var person = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithBirthMonth(BirthMonthEnum.March) + .Build(); + var personTwo = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithBirthMonth(BirthMonthEnum.April) + .Build(); + await testingServiceScope.InsertAsync(person, personTwo); + + var input = $"""Title == "{title}" && month == March"""; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.BirthMonth).HasQueryName("month").HasConversion(); + }); + + // Act + var people = await testingServiceScope.DbContext().People + .ApplyQueryKitFilter(input, config) + .ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + + [Fact] + public async Task can_filter_by_guid_with_query_name_and_has_conversion() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var person = new FakeTestingPersonBuilder().Build(); + var personTwo = new FakeTestingPersonBuilder().Build(); + await testingServiceScope.InsertAsync(person, personTwo); + + var input = $"""identifier == "{person.Id}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("identifier").HasConversion(); + }); + + // Act + var people = await testingServiceScope.DbContext().People + .ApplyQueryKitFilter(input, config) + .ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + + [Fact] + public async Task can_filter_by_null_email_with_has_conversion() { // Arrange var testingServiceScope = new TestingServiceScope(); @@ -182,7 +279,8 @@ public async Task null_email_with_has_conversion_matches_no_row() var people = await appliedQueryable.ToListAsync(); // Assert - people.Should().BeEmpty(); + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); } [Fact] diff --git a/QueryKit.UnitTests/HasConversionTests.cs b/QueryKit.UnitTests/HasConversionTests.cs index 3f09312..b09c3f1 100644 --- a/QueryKit.UnitTests/HasConversionTests.cs +++ b/QueryKit.UnitTests/HasConversionTests.cs @@ -1,6 +1,7 @@ namespace QueryKit.UnitTests; using Configuration; +using Exceptions; using FluentAssertions; using WebApiTestProject.Entities; @@ -167,7 +168,7 @@ public void can_filter_nested_property_with_query_name_and_has_conversion() } [Fact] - public void child_property_of_converted_parent_with_query_name_compares_the_child() + public void child_property_of_converted_parent_with_query_name_compares_parent() { // Arrange var input = """Email.Value == "a@x.com" """; @@ -185,8 +186,8 @@ public void child_property_of_converted_parent_with_query_name_compares_the_chil var filterWithoutQueryName = FilterParser.ParseFilter(input, configWithoutQueryName); // Assert - filterWithQueryName.ToDisplayString().Should().Be("""x => (x.Email.Value == "a@x.com")"""); - filterWithoutQueryName.ToDisplayString().Should().Be("""x => (x.Email == new EmailAddress("a@x.com"))"""); + filterWithQueryName.ToDisplayString().Should().Be("""x => (x.Email == new EmailAddress("a@x.com"))"""); + filterWithQueryName.ToDisplayString().Should().Be(filterWithoutQueryName.ToDisplayString()); } [Fact] @@ -252,7 +253,7 @@ public void can_filter_null_on_nullable_struct_with_has_conversion() } [Fact] - public void null_on_reference_type_with_has_conversion_matches_no_row() + public void can_filter_null_on_reference_type_with_has_conversion() { // Arrange var rows = EmailRows(); @@ -266,7 +267,8 @@ public void null_on_reference_type_with_has_conversion_matches_no_row() var result = rows.ApplyQueryKitFilter("""Email == null""", config).ToList(); // Assert - result.Should().BeEmpty(); + result.Count.Should().Be(1); + result[0].Email.Should().BeNull(); } [Fact] @@ -362,8 +364,10 @@ public void can_filter_property_list_with_lowercase_path_query_name_and_has_conv result[0].Name.Should().Be("two"); } - [Fact] - public void int_with_query_name_and_has_conversion_compares_the_int() + [Theory] + [InlineData("count")] + [InlineData("Number")] + public void int_with_has_conversion_compares_the_int(string queryName) { // Arrange var rows = new List @@ -373,11 +377,11 @@ public void 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(queryName).HasConversion(); }); // Act - var result = rows.ApplyQueryKitFilter("""count == 2""", config).ToList(); + var result = rows.ApplyQueryKitFilter($"{queryName} == 2", config).ToList(); // Assert result.Count.Should().Be(1); @@ -385,16 +389,65 @@ public void int_with_query_name_and_has_conversion_compares_the_int() } [Fact] - public void guid_with_query_name_and_has_conversion_compares_a_guid_constant() + public void nullable_int_with_query_name_and_has_conversion_compares_the_int() { // Arrange + var rows = new List + { + new() { Number = 1 }, + new() { Number = 2 }, + new() { Number = null } + }; var config = new QueryKitConfiguration(config => { - config.Property(x => x.Id).HasQueryName("identifier").HasConversion(); + config.Property(x => x.Number).HasQueryName("count").HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("count == 2", config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Number.Should().Be(2); + } + + [Theory] + [InlineData("level")] + [InlineData("Level")] + public void enum_with_has_conversion_compares_the_enum(string queryName) + { + // Arrange + var rows = new List + { + new() { Level = LevelKind.Low }, + new() { Level = LevelKind.High } + }; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Level).HasQueryName(queryName).HasConversion(); }); // Act - var filter = FilterParser.ParseFilter($"identifier == \"{KnownGuid}\"", config); + var result = rows.ApplyQueryKitFilter($"{queryName} == High", config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Level.Should().Be(LevelKind.High); + } + + [Theory] + [InlineData("identifier")] + [InlineData("Id")] + public void guid_with_has_conversion_compares_a_guid_constant(string queryName) + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName(queryName).HasConversion(); + }); + + // Act + var filter = FilterParser.ParseFilter($"{queryName} == \"{KnownGuid}\"", config); // Assert filter.ToDisplayString().Should().Be($"x => (x.Id == {KnownGuid})"); @@ -462,4 +515,20 @@ private class NumberRow { public int Number { get; set; } } + + private class NullableNumberRow + { + public int? Number { get; set; } + } + + private enum LevelKind + { + Low, + High + } + + private class LevelRow + { + public LevelKind Level { get; set; } + } } diff --git a/QueryKit/FilterParser.cs b/QueryKit/FilterParser.cs index 84a850b..7da12b6 100644 --- a/QueryKit/FilterParser.cs +++ b/QueryKit/FilterParser.cs @@ -533,11 +533,11 @@ private static DateTimeOffset ParseDateTimeOffset(string value) // A value that does not convert to the property type throws ParsingException with the value, the type, and the // property name as the caller wrote it (the query name, not the member path). private static Expression CreateRightExpr(Expression leftExpr, string right, bool rightIsQuotedLiteral, ComparisonOperator op, - string propertyName, IQueryKitConfiguration? config = null, string? propertyPath = null, string? memberPath = null) + string propertyName, IQueryKitConfiguration? config = null, string? propertyPath = null) { try { - return CreateRightExprForProperty(leftExpr, right, rightIsQuotedLiteral, op, config, propertyPath, memberPath); + return CreateRightExprForProperty(leftExpr, right, rightIsQuotedLiteral, op, config, propertyPath); } catch (InvalidFilterValueException e) { @@ -546,7 +546,7 @@ private static Expression CreateRightExpr(Expression leftExpr, string right, boo } private static Expression CreateRightExprForProperty(Expression leftExpr, string right, bool rightIsQuotedLiteral, ComparisonOperator op, - IQueryKitConfiguration? config, string? propertyPath, string? memberPath) + IQueryKitConfiguration? config, string? propertyPath) { var targetType = leftExpr.Type; @@ -594,66 +594,41 @@ private static Expression CreateRightExprForProperty(Expression leftExpr, string } } - // Check if this property uses HasConversion - if (config?.PropertyMappings != null && !string.IsNullOrEmpty(propertyPath)) + // 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)) { - var propertyConfig = config.PropertyMappings.GetPropertyInfoByQueryName(propertyPath); + var propertyConfig = config.PropertyMappings.GetPropertyInfo(propertyPath); if (propertyConfig?.UsesConversion == true && propertyConfig.ConversionTargetType != null) { // For HasConversion properties, try to create a constant of the original type // by constructing it from the string value using a constructor that takes the target type if (propertyConfig.ConversionTargetType == typeof(string)) { - var stringCtor = leftExpr.Type.GetConstructor(new[] { typeof(string) }); - if (stringCtor != null) + // A null literal compares against null instead of being passed to the constructor + var underlyingType = Nullable.GetUnderlyingType(leftExpr.Type); + if (right == "null" && (!leftExpr.Type.IsValueType || underlyingType != null)) { - return Expression.New(stringCtor, FilterValue.Create(right, typeof(string))); + return Expression.Constant(null, leftExpr.Type); } - // v1.14.2 compared a nullable struct with a string here and threw, so construct the underlying type instead. - // A null literal keeps the v1.14.2 result. - if (right != "null" && CreateStringConversionRightExpr(leftExpr.Type, right) is { } nullableStructExpr) + // Nullable structs are constructed from their underlying type, then converted back + var stringCtor = (underlyingType ?? leftExpr.Type).GetConstructor(new[] { typeof(string) }); + if (stringCtor != null) { - return nullableStructExpr; + Expression constructed = Expression.New(stringCtor, FilterValue.Create(right, typeof(string))); + return underlyingType == null ? constructed : Expression.Convert(constructed, leftExpr.Type); } } // For other conversion types, fall back to using the conversion target type targetType = propertyConfig.ConversionTargetType; } - else if (!CanCreateRightExprFromType(targetType) && - config.PropertyMappings.GetPropertyInfo(memberPath ?? propertyPath) is { UsesConversion: true } pathConfig && - pathConfig.ConversionTargetType == typeof(string)) - { - // The lookup by query name above misses a property with a different query name. v1.14.2 then threw, - // because it can not read a value of this type, so find the conversion by the property path instead. - return CreateStringConversionRightExpr(leftExpr.Type, right) ?? CreateRightExprFromType(targetType, right, rightIsQuotedLiteral, op); - } } return CreateRightExprFromType(targetType, right, rightIsQuotedLiteral, op); } - // Builds the right side for a property with HasConversion() from a constructor that takes a string. - // A null literal compares against null, and a nullable struct is constructed from its underlying type. - private static Expression? CreateStringConversionRightExpr(Type leftType, string right) - { - var underlyingType = Nullable.GetUnderlyingType(leftType); - if (right == "null" && (!leftType.IsValueType || underlyingType != null)) - { - return Expression.Constant(null, leftType); - } - - var stringCtor = (underlyingType ?? leftType).GetConstructor(new[] { typeof(string) }); - if (stringCtor == null) - { - return null; - } - - Expression constructed = Expression.New(stringCtor, FilterValue.Create(right, typeof(string))); - return underlyingType == null ? constructed : Expression.Convert(constructed, leftType); - } - // True when CreateRightExprFromType can read a value of the type. For other types it throws. private static bool CanCreateRightExprFromType(Type type) { @@ -1038,7 +1013,7 @@ private static Parser ComparisonExprParser(ParameterExpression pa var guidStringExpr = HandleGuidConversion(leftExpr, leftExpr.Type); // For a Guid with HasConversion(), v1.14.2 built the right side as a Guid and threw, so build it for the string. - var guidConfig = config?.PropertyMappings?.GetPropertyInfoByQueryName(guidPropertyPath); + var guidConfig = config?.PropertyMappings?.GetPropertyInfo(guidPropertyPath); var leftExprForRightSide = guidConfig?.UsesConversion == true && guidConfig.ConversionTargetType == typeof(string) ? guidStringExpr : leftExpr; @@ -1253,7 +1228,7 @@ private static Expression CreateLeftExpr(ParameterExpression parameter, Property nestedMemberExpression.Expression is MemberExpression parentExpression) { var parentPropertyPath = GetPropertyPath(parentExpression, parameter); - var parentPropertyConfig = config?.PropertyMappings?.GetPropertyInfoByQueryName(parentPropertyPath); + var parentPropertyConfig = config?.PropertyMappings?.GetPropertyInfo(parentPropertyPath); if (parentPropertyConfig?.UsesConversion == true) { @@ -1406,7 +1381,7 @@ private static Parser PropertyListComparisonExprParser( leftExpr = HandleGuidConversion(leftExpr, leftExpr.Type); } - var rightExpr = CreateRightExpr(leftExpr, temp.right, temp.rightIsQuotedLiteral, temp.op, fullPropPath, config, fullPropPath, reference.Path); + var rightExpr = CreateRightExpr(leftExpr, temp.right, temp.rightIsQuotedLiteral, temp.op, fullPropPath, config, reference.Path); var comparison = temp.op.GetExpression(leftExpr, rightExpr, config?.DbContextType, ResolveCaseMode(reference.Path, config)); // Combine with AND for negative operators, OR for positive operators diff --git a/README.md b/README.md index b88da46..2f43ba7 100644 --- a/README.md +++ b/README.md @@ -968,6 +968,8 @@ var people = _dbContext.People This allows you to use `Email == "value"` syntax instead of `Email.Value == "value"` when the property is configured with HasConversion in EF Core. The `HasConversion()` method tells QueryKit what the conversion target type is so it can handle the type conversion properly. +QueryKit finds the conversion by the property, with or without a query name. A `null` value, such as `email == null`, compares the property against null. A nullable struct property also uses the conversion. QueryKit reads a number, an enum, or a Guid by its own type, so `HasConversion()` on these properties does not change the filter. + > **Important:** When using `HasConversion` in EF Core, you MUST configure the property in QueryKit using `x => x.Email`, not `x => x.Email.Value`. The conversion is on the parent property, so pointing to the nested `.Value` property will cause EF Core translation errors. Use `x => x.Email.Value` only when using `ComplexProperty` or `OwnsOne` without HasConversion. ## Sorting