From 41cc708bdd9427ca4efc03ae52a5d08aa3d41930 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Fri, 2 Oct 2026 00:10:40 +0300 Subject: [PATCH] fix(filter): filter a query name with HasConversion where v1.14.2 threw A struct, a nullable struct, a reference type, or a nested property with a query name and HasConversion() threw "Unsupported value". A filter by the property path and a property list also threw. The parser now finds the conversion by the property path when the lookup by query name misses, and only where v1.14.2 threw. A Guid with @= and HasConversion() now compares the string form instead of throwing. Every input that v1.14.2 accepted gives the same result as before. Fixes #106 --- .../Tests/HasConversionTests.cs | 199 ++++++++++++++++++ QueryKit.UnitTests/HasConversionTests.cs | 164 +++++++++++---- QueryKit/FilterParser.cs | 58 ++++- 3 files changed, 371 insertions(+), 50 deletions(-) diff --git a/QueryKit.IntegrationTests/Tests/HasConversionTests.cs b/QueryKit.IntegrationTests/Tests/HasConversionTests.cs index 0d120b0..f3c0d8a 100644 --- a/QueryKit.IntegrationTests/Tests/HasConversionTests.cs +++ b/QueryKit.IntegrationTests/Tests/HasConversionTests.cs @@ -41,6 +41,150 @@ public async Task can_filter_by_email_with_has_conversion() people[0].Id.Should().Be(person.Id); } + [Fact] + public async Task can_filter_by_email_with_query_name_and_has_conversion() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + 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 = $"""mail == "{testEmail}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasQueryName("mail").HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People; + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + + [Fact] + public async Task can_filter_by_email_property_path_when_query_name_and_has_conversion_are_configured() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + 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 == "{testEmail}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasQueryName("mail").HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People; + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + + [Fact] + public async Task email_value_with_query_name_and_has_conversion_compares_the_child() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var input = """Email.Value == "a@x.com" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasQueryName("mail").HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People; + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var act = () => appliedQueryable.ToListAsync(); + + // Assert + await act.Should().ThrowExactlyAsync() + .WithMessage("The LINQ expression*could not be translated*"); + } + + [Fact] + public async Task can_filter_by_null_email_with_query_name_and_has_conversion() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var title = Guid.NewGuid().ToString(); + var person = new FakeTestingPersonBuilder() + .WithTitle(title) + .Build(); + person.Email = null!; + var personTwo = new FakeTestingPersonBuilder() + .WithTitle(title) + .Build(); + + await testingServiceScope.InsertAsync(person, personTwo); + + var input = """mail == null"""; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasQueryName("mail").HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People + .Where(x => x.Title == title); + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + + [Fact] + public async Task null_email_with_has_conversion_matches_no_row() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var title = Guid.NewGuid().ToString(); + var person = new FakeTestingPersonBuilder() + .WithTitle(title) + .Build(); + person.Email = null!; + var personTwo = new FakeTestingPersonBuilder() + .WithTitle(title) + .Build(); + + await testingServiceScope.InsertAsync(person, personTwo); + + var input = """Email == null"""; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People + .Where(x => x.Title == title); + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Should().BeEmpty(); + } + [Fact] public async Task can_filter_by_nested_postal_code_with_has_conversion() { @@ -70,6 +214,35 @@ public async Task can_filter_by_nested_postal_code_with_has_conversion() people[0].Id.Should().Be(person.Id); } + [Fact] + public async Task can_filter_by_nested_postal_code_with_query_name_and_has_conversion() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var postalCode = Guid.NewGuid().ToString("N")[..10]; + var person = new FakeTestingPersonBuilder() + .WithPhysicalAddress(new Address("Line1", "Line2", "City", "State", postalCode, "Country")) + .Build(); + var personTwo = new FakeTestingPersonBuilder().Build(); + + await testingServiceScope.InsertAsync(person, personTwo); + + var input = $"""zip == "{postalCode}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.PhysicalAddress.PostalCode).HasQueryName("zip").HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People; + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } + [Fact] public async Task can_filter_guid_with_contains_query_name_and_has_conversion() { @@ -95,4 +268,30 @@ public async Task can_filter_guid_with_contains_query_name_and_has_conversion() people.Count.Should().Be(1); people[0].Id.Should().Be(person.Id); } + + [Fact] + public async Task can_filter_guid_with_contains_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 = $"""Id @= "{person.Id.ToString()[..13]}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasConversion(); + }); + + // Act + var queryablePeople = testingServiceScope.DbContext().People; + var appliedQueryable = queryablePeople.ApplyQueryKitFilter(input, config); + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(person.Id); + } } \ No newline at end of file diff --git a/QueryKit.UnitTests/HasConversionTests.cs b/QueryKit.UnitTests/HasConversionTests.cs index e7df106..3f09312 100644 --- a/QueryKit.UnitTests/HasConversionTests.cs +++ b/QueryKit.UnitTests/HasConversionTests.cs @@ -1,7 +1,6 @@ namespace QueryKit.UnitTests; using Configuration; -using Exceptions; using FluentAssertions; using WebApiTestProject.Entities; @@ -27,7 +26,7 @@ public void can_filter_struct_with_has_conversion() } [Fact] - public void struct_with_query_name_and_has_conversion_throws() + public void can_filter_struct_with_query_name_and_has_conversion() { // Arrange var config = new QueryKitConfiguration(config => @@ -36,16 +35,15 @@ public void struct_with_query_name_and_has_conversion_throws() }); // Act - var act = () => WrappedIdRows().ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); + var result = WrappedIdRows().ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value '2' for type 'WrappedId'"); + result.Count.Should().Be(1); + result[0].Name.Should().Be("two"); } [Fact] - public void struct_with_has_conversion_configured_before_query_name_throws() + public void can_filter_struct_with_has_conversion_configured_before_query_name() { // Arrange var config = new QueryKitConfiguration(config => @@ -54,12 +52,11 @@ public void struct_with_has_conversion_configured_before_query_name_throws() }); // Act - var act = () => WrappedIdRows().ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); + var result = WrappedIdRows().ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value '2' for type 'WrappedId'"); + result.Count.Should().Be(1); + result[0].Name.Should().Be("two"); } [Fact] @@ -80,7 +77,7 @@ public void can_filter_struct_with_query_name_differing_only_in_case_and_has_con } [Fact] - public void struct_with_not_equals_query_name_and_has_conversion_throws() + public void can_filter_struct_with_not_equals_query_name_and_has_conversion() { // Arrange var config = new QueryKitConfiguration(config => @@ -89,16 +86,15 @@ public void struct_with_not_equals_query_name_and_has_conversion_throws() }); // Act - var act = () => WrappedIdRows().ApplyQueryKitFilter("""wrappedid != "2" """, config).ToList(); + var result = WrappedIdRows().ApplyQueryKitFilter("""wrappedid != "2" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value '2' for type 'WrappedId'"); + result.Count.Should().Be(1); + result[0].Name.Should().Be("one"); } [Fact] - public void property_path_with_query_name_and_has_conversion_configured_throws() + public void can_filter_by_property_path_when_query_name_and_has_conversion_are_configured() { // Arrange var config = new QueryKitConfiguration(config => @@ -107,12 +103,11 @@ public void property_path_with_query_name_and_has_conversion_configured_throws() }); // Act - var act = () => WrappedIdRows().ApplyQueryKitFilter("""Id == "2" """, config).ToList(); + var result = WrappedIdRows().ApplyQueryKitFilter("""Id == "2" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value '2' for type 'WrappedId'"); + result.Count.Should().Be(1); + result[0].Name.Should().Be("two"); } [Fact] @@ -133,7 +128,7 @@ public void can_filter_reference_type_with_query_name_differing_only_in_case_and } [Fact] - public void reference_type_with_query_name_and_has_conversion_throws() + public void can_filter_reference_type_with_query_name_and_has_conversion() { // Arrange var config = new QueryKitConfiguration(config => @@ -142,16 +137,15 @@ public void reference_type_with_query_name_and_has_conversion_throws() }); // Act - var act = () => EmailRows().ApplyQueryKitFilter("""mail == "b@x.com" """, config).ToList(); + var result = EmailRows().ApplyQueryKitFilter("""mail == "b@x.com" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value 'b@x.com' for type 'EmailAddressRecord'"); + result.Count.Should().Be(1); + result[0].Email!.Value.Should().Be("b@x.com"); } [Fact] - public void nested_property_with_query_name_and_has_conversion_throws() + public void can_filter_nested_property_with_query_name_and_has_conversion() { // Arrange var rows = new List @@ -165,12 +159,11 @@ public void nested_property_with_query_name_and_has_conversion_throws() }); // Act - var act = () => rows.ApplyQueryKitFilter("""contact == "b@x.com" """, config).ToList(); + var result = rows.ApplyQueryKitFilter("""contact == "b@x.com" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value 'b@x.com' for type 'EmailAddressRecord'"); + result.Count.Should().Be(1); + result[0].Owner.Contact!.Value.Should().Be("b@x.com"); } [Fact] @@ -214,7 +207,7 @@ public void can_filter_property_list_with_lowercase_path_and_has_conversion() } [Fact] - public void nullable_struct_with_has_conversion_throws() + public void can_filter_nullable_struct_with_has_conversion() { // Arrange var rows = new List @@ -229,12 +222,11 @@ public void nullable_struct_with_has_conversion_throws() }); // Act - var act = () => rows.ApplyQueryKitFilter("""Id == "2" """, config).ToList(); + var result = rows.ApplyQueryKitFilter("""Id == "2" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("The binary operator Equal is not defined for the types*"); + result.Count.Should().Be(1); + result[0].Id.Should().Be(new WrappedId(2)); } [Fact] @@ -278,7 +270,7 @@ public void null_on_reference_type_with_has_conversion_matches_no_row() } [Fact] - public void null_on_reference_type_with_query_name_and_has_conversion_throws() + public void can_filter_null_on_reference_type_with_query_name_and_has_conversion() { // Arrange var rows = EmailRows(); @@ -289,16 +281,15 @@ public void null_on_reference_type_with_query_name_and_has_conversion_throws() }); // Act - var act = () => rows.ApplyQueryKitFilter("""mail == null""", config).ToList(); + var result = rows.ApplyQueryKitFilter("""mail == null""", config).ToList(); // Assert - act.Should().ThrowExactly() - .WithInnerExceptionExactly() - .WithMessage("Unsupported value 'null' for type 'EmailAddressRecord'"); + result.Count.Should().Be(1); + result[0].Email.Should().BeNull(); } [Fact] - public void guid_with_contains_and_has_conversion_throws() + public void can_filter_guid_with_contains_and_has_conversion() { // Arrange var config = new QueryKitConfiguration(config => @@ -307,11 +298,11 @@ public void guid_with_contains_and_has_conversion_throws() }); // Act - var act = () => GuidRows().ApplyQueryKitFilter("""Id @= "ab7afb17" """, config).ToList(); + var result = GuidRows().ApplyQueryKitFilter("""Id @= "ab7afb17" """, config).ToList(); // Assert - act.Should().ThrowExactly() - .WithMessage("Expression of type 'System.Guid' cannot be used for parameter of type 'System.String'*"); + result.Count.Should().Be(1); + result[0].Id.Should().Be(KnownGuid); } [Fact] @@ -331,6 +322,84 @@ public void can_filter_guid_with_contains_query_name_and_has_conversion() result[0].Id.Should().Be(KnownGuid); } + [Fact] + public void can_filter_nullable_struct_with_query_name_and_has_conversion() + { + // Arrange + var rows = new List + { + new() { Id = new WrappedId(1) }, + new() { Id = new WrappedId(2) }, + new() { Id = null } + }; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id!).HasQueryName("wrappedid").HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Id.Should().Be(new WrappedId(2)); + } + + [Fact] + public void can_filter_property_list_with_lowercase_path_query_name_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("wrappedid").HasConversion(); + }); + + // Act + var result = WrappedIdRows().ApplyQueryKitFilter("""(id) == "2" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Name.Should().Be("two"); + } + + [Fact] + public void int_with_query_name_and_has_conversion_compares_the_int() + { + // Arrange + var rows = new List + { + new() { Number = 1 }, + new() { Number = 2 } + }; + var config = new QueryKitConfiguration(config => + { + 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); + } + + [Fact] + public void guid_with_query_name_and_has_conversion_compares_a_guid_constant() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("identifier").HasConversion(); + }); + + // Act + var filter = FilterParser.ParseFilter($"identifier == \"{KnownGuid}\"", config); + + // Assert + filter.ToDisplayString().Should().Be($"x => (x.Id == {KnownGuid})"); + } + private static List WrappedIdRows() => new() { new() { Id = new WrappedId(1), Name = "one" }, @@ -388,4 +457,9 @@ private class GuidRow { public Guid Id { get; set; } } + + private class NumberRow + { + public int Number { get; set; } + } } diff --git a/QueryKit/FilterParser.cs b/QueryKit/FilterParser.cs index 297e9db..07ead9d 100644 --- a/QueryKit/FilterParser.cs +++ b/QueryKit/FilterParser.cs @@ -459,7 +459,7 @@ private static DateTimeOffset ToParameterOffset(DateTimeOffset value) }; private static Expression CreateRightExpr(Expression leftExpr, string right, bool rightIsQuotedLiteral, ComparisonOperator op, - IQueryKitConfiguration? config = null, string? propertyPath = null) + IQueryKitConfiguration? config = null, string? propertyPath = null, string? memberPath = null) { var targetType = leftExpr.Type; @@ -522,16 +522,58 @@ private static Expression CreateRightExpr(Expression leftExpr, string right, boo { return Expression.New(stringCtor, FilterValue.Create(right, typeof(string))); } + + // 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) + { + return nullableStructExpr; + } } - + // 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) + { + var targetType = TransformTargetTypeIfNullable(type); + return IsEnumerable(type) || TypeConversionFunctions.ContainsKey(targetType) || targetType.IsEnum || targetType == typeof(object); + } + private static Expression CreateRightExprFromType(Type leftExprType, string right, bool rightIsQuotedLiteral, ComparisonOperator op) { var isEnumerable = IsEnumerable(leftExprType); @@ -876,7 +918,13 @@ private static Parser ComparisonExprParser(ParameterExpression pa if (temp.op.IsStringComparisonOperator()) { var guidStringExpr = HandleGuidConversion(leftExpr, leftExpr.Type); - return temp.op.GetExpression(guidStringExpr, CreateRightExpr(leftExpr, temp.right, temp.rightIsQuotedLiteral, temp.op, config, guidPropertyPath), + + // 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 leftExprForRightSide = guidConfig?.UsesConversion == true && guidConfig.ConversionTargetType == typeof(string) + ? guidStringExpr + : leftExpr; + return temp.op.GetExpression(guidStringExpr, CreateRightExpr(leftExprForRightSide, temp.right, temp.rightIsQuotedLiteral, temp.op, config, guidPropertyPath), config?.DbContextType, ResolveCaseMode(guidPropertyPath, config)); } @@ -1260,7 +1308,7 @@ private static Parser PropertyListComparisonExprParser( leftExpr = HandleGuidConversion(leftExpr, leftExpr.Type); } - var rightExpr = CreateRightExpr(leftExpr, temp.right, temp.rightIsQuotedLiteral, temp.op, config, fullPropPath); + var rightExpr = CreateRightExpr(leftExpr, temp.right, temp.rightIsQuotedLiteral, temp.op, config, fullPropPath, reference.Path); var comparison = temp.op.GetExpression(leftExpr, rightExpr, config?.DbContextType, ResolveCaseMode(fullPropPath, config)); // Combine with AND for negative operators, OR for positive operators