From 9ff3525c300d1bb08a38b3f517ecfb5d92ae4b7e Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Tue, 29 Sep 2026 20:31:05 +0300 Subject: [PATCH] fix: honor HasConversion when a property has a HasQueryName alias The filter parser replaces a query name with the property path before it parses. The HasConversion lookups then searched by query name, so they missed the configuration. The filter then failed or returned the wrong rows. The conversion lookups now use the property path. The fix also corrects four related faults in the same code path: - A null literal on a converted property compares against null. Before, the parser built a value from the text "null". - A converted Nullable struct is built from its underlying type. - Guid string operators pass the string expression to the right side. - Property lists use the resolved member path, so a lowercase path still finds the conversion. Closes #106 --- .../Tests/HasConversionTests.cs | 204 ++++++++++ QueryKit.UnitTests/HasConversionTests.cs | 383 ++++++++++++++++++ QueryKit/FilterParser.cs | 28 +- 3 files changed, 608 insertions(+), 7 deletions(-) create mode 100644 QueryKit.UnitTests/HasConversionTests.cs diff --git a/QueryKit.IntegrationTests/Tests/HasConversionTests.cs b/QueryKit.IntegrationTests/Tests/HasConversionTests.cs index 43d93de..d30a624 100644 --- a/QueryKit.IntegrationTests/Tests/HasConversionTests.cs +++ b/QueryKit.IntegrationTests/Tests/HasConversionTests.cs @@ -40,4 +40,208 @@ public async Task can_filter_by_email_with_has_conversion() people.Count.Should().Be(1); 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 can_filter_by_email_value_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 = $"""Email.Value == "{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_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 can_filter_by_nested_postal_code_with_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 = $"""PhysicalAddress.PostalCode == "{postalCode}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.PhysicalAddress.PostalCode).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_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() + { + // 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.ToString()[..13]}" """; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("identifier").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 new file mode 100644 index 0000000..2eab3c9 --- /dev/null +++ b/QueryKit.UnitTests/HasConversionTests.cs @@ -0,0 +1,383 @@ +namespace QueryKit.UnitTests; + +using Configuration; +using FluentAssertions; +using WebApiTestProject.Entities; + +public class HasConversionTests +{ + private static readonly Guid KnownGuid = Guid.Parse("ab7afb17-abca-4530-9f8f-bd1497e0f6be"); + + [Fact] + public void can_filter_struct_with_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).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 can_filter_struct_with_query_name_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("wrappedid").HasConversion(); + }); + + // Act + var result = WrappedIdRows().ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Name.Should().Be("two"); + } + + [Fact] + public void can_filter_struct_with_has_conversion_configured_before_query_name() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasConversion().HasQueryName("wrappedid"); + }); + + // Act + var result = WrappedIdRows().ApplyQueryKitFilter("""wrappedid == "2" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Name.Should().Be("two"); + } + + [Fact] + public void can_filter_struct_with_query_name_differing_only_in_case_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("ID").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 can_filter_struct_with_not_equals_query_name_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("wrappedid").HasConversion(); + }); + + // Act + var result = WrappedIdRows().ApplyQueryKitFilter("""wrappedid != "2" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Name.Should().Be("one"); + } + + [Fact] + public void can_filter_by_property_path_when_query_name_and_has_conversion_are_configured() + { + // 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 can_filter_reference_type_with_query_name_differing_only_in_case_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email!).HasQueryName("email").HasConversion(); + }); + + // Act + var result = EmailRows().ApplyQueryKitFilter("""email == "b@x.com" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Email!.Value.Should().Be("b@x.com"); + } + + [Fact] + public void can_filter_reference_type_with_query_name_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email!).HasQueryName("mail").HasConversion(); + }); + + // Act + var result = EmailRows().ApplyQueryKitFilter("""mail == "b@x.com" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Email!.Value.Should().Be("b@x.com"); + } + + [Fact] + public void can_filter_nested_property_with_query_name_and_has_conversion() + { + // Arrange + var rows = new List + { + new() { Owner = new Owner { Contact = new EmailAddressRecord("a@x.com") } }, + new() { Owner = new Owner { Contact = new EmailAddressRecord("b@x.com") } } + }; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Owner.Contact!).HasQueryName("contact").HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("""contact == "b@x.com" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Owner.Contact!.Value.Should().Be("b@x.com"); + } + + [Fact] + public void child_property_of_converted_parent_with_query_name_compares_parent() + { + // Arrange + var input = """Email.Value == "a@x.com" """; + var configWithQueryName = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasQueryName("mail").HasConversion(); + }); + var configWithoutQueryName = new QueryKitConfiguration(config => + { + config.Property(x => x.Email).HasConversion(); + }); + + // Act + var filterWithQueryName = FilterParser.ParseFilter(input, configWithQueryName); + var filterWithoutQueryName = FilterParser.ParseFilter(input, configWithoutQueryName); + + // Assert + filterWithQueryName.ToString().Should().Be("""x => (x.Email == new EmailAddress("a@x.com"))"""); + filterWithQueryName.ToString().Should().Be(filterWithoutQueryName.ToString()); + } + + [Fact] + public void can_filter_property_list_with_lowercase_path_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).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 can_filter_nullable_struct_with_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!).HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("""Id == "2" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Id.Should().Be(new WrappedId(2)); + } + + [Fact] + public void can_filter_null_on_nullable_struct_with_has_conversion() + { + // Arrange + var rows = new List + { + new() { Id = new WrappedId(1) }, + new() { Id = null } + }; + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id!).HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("""Id == null""", config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Id.Should().BeNull(); + } + + [Fact] + public void can_filter_null_on_reference_type_with_has_conversion() + { + // Arrange + var rows = EmailRows(); + rows.Add(new EmailRow { Email = null }); + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email!).HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("""Email == null""", config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Email.Should().BeNull(); + } + + [Fact] + public void can_filter_null_on_reference_type_with_query_name_and_has_conversion() + { + // Arrange + var rows = EmailRows(); + rows.Add(new EmailRow { Email = null }); + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Email!).HasQueryName("mail").HasConversion(); + }); + + // Act + var result = rows.ApplyQueryKitFilter("""mail == null""", config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Email.Should().BeNull(); + } + + [Fact] + public void can_filter_guid_with_contains_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasConversion(); + }); + + // Act + var result = GuidRows().ApplyQueryKitFilter("""Id @= "ab7afb17" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Id.Should().Be(KnownGuid); + } + + [Fact] + public void can_filter_guid_with_contains_query_name_and_has_conversion() + { + // Arrange + var config = new QueryKitConfiguration(config => + { + config.Property(x => x.Id).HasQueryName("identifier").HasConversion(); + }); + + // Act + var result = GuidRows().ApplyQueryKitFilter("""identifier @= "ab7afb17" """, config).ToList(); + + // Assert + result.Count.Should().Be(1); + result[0].Id.Should().Be(KnownGuid); + } + + private static List WrappedIdRows() => new() + { + new() { Id = new WrappedId(1), Name = "one" }, + new() { Id = new WrappedId(2), Name = "two" } + }; + + private static List EmailRows() => new() + { + new() { Email = new EmailAddressRecord("a@x.com") }, + new() { Email = new EmailAddressRecord("b@x.com") } + }; + + private static List GuidRows() => new() + { + new() { Id = KnownGuid }, + new() { Id = Guid.NewGuid() } + }; + + private readonly record struct WrappedId + { + public WrappedId(int value) => Value = value; + public WrappedId(string value) : this(int.Parse(value)) { } + public int Value { get; } + } + + private record EmailAddressRecord(string Value); + + private class WrappedIdRow + { + public WrappedId Id { get; set; } + public string Name { get; set; } = null!; + } + + private class NullableWrappedIdRow + { + public WrappedId? Id { get; set; } + } + + private class EmailRow + { + public EmailAddressRecord? Email { get; set; } + } + + private class Owner + { + public EmailAddressRecord? Contact { get; set; } + } + + private class OwnerRow + { + public Owner Owner { get; set; } = new(); + } + + private class GuidRow + { + public Guid Id { get; set; } + } +} diff --git a/QueryKit/FilterParser.cs b/QueryKit/FilterParser.cs index 25eabaa..b269640 100644 --- a/QueryKit/FilterParser.cs +++ b/QueryKit/FilterParser.cs @@ -286,17 +286,26 @@ private static Expression CreateRightExpr(Expression leftExpr, string right, Com // Check if this property uses HasConversion if (config?.PropertyMappings != null && !string.IsNullOrEmpty(propertyPath)) { - 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) }); + // 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.Constant(null, leftExpr.Type); + } + + // Nullable structs are constructed from their underlying type, then converted back + var stringCtor = (underlyingType ?? leftExpr.Type).GetConstructor(new[] { typeof(string) }); if (stringCtor != null) { - return Expression.New(stringCtor, Expression.Constant(right, typeof(string))); + Expression constructed = Expression.New(stringCtor, Expression.Constant(right, typeof(string))); + return underlyingType == null ? constructed : Expression.Convert(constructed, leftExpr.Type); } } @@ -677,7 +686,7 @@ private static Parser ComparisonExprParser(ParameterExpression pa if (temp.op.IsStringComparisonOperator()) { var guidStringExpr = HandleGuidConversion(temp.leftExpr, temp.leftExpr.Type); - return temp.op.GetExpression(guidStringExpr, CreateRightExpr(temp.leftExpr, temp.right, temp.op, config, guidPropertyPath), + return temp.op.GetExpression(guidStringExpr, CreateRightExpr(guidStringExpr, temp.right, temp.op, config, guidPropertyPath), config?.DbContextType, ResolveCaseMode(guidPropertyPath, config)); } @@ -929,7 +938,7 @@ private static Parser ComparisonExprParser(ParameterExpression pa } // Check if this property uses HasConversion - var currentPropertyConfig = config?.PropertyMappings?.GetPropertyInfoByQueryName(fullPropPath); + var currentPropertyConfig = config?.PropertyMappings?.GetPropertyInfo(fullPropPath); if (currentPropertyConfig?.UsesConversion == true) { // For HasConversion properties, return the property expression as-is @@ -943,7 +952,7 @@ private static Parser ComparisonExprParser(ParameterExpression pa 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) { @@ -1145,6 +1154,11 @@ private static Parser PropertyListComparisonExprParser( } } + // Use the resolved member path for HasConversion support, since the typed path can differ in casing + var resolvedPropPath = leftExpr is MemberExpression listMemberExpr + ? GetPropertyPath(listMemberExpr, parameter) + : fullPropPath; + // Handle GUID conversion for string operators if ((leftExpr.Type == typeof(Guid) || leftExpr.Type == typeof(Guid?)) && temp.op.IsStringComparisonOperator()) @@ -1152,7 +1166,7 @@ private static Parser PropertyListComparisonExprParser( leftExpr = HandleGuidConversion(leftExpr, leftExpr.Type); } - var rightExpr = CreateRightExpr(leftExpr, temp.right, temp.op, config, fullPropPath); + var rightExpr = CreateRightExpr(leftExpr, temp.right, temp.op, config, resolvedPropPath); var comparison = temp.op.GetExpression(leftExpr, rightExpr, config?.DbContextType, ResolveCaseMode(fullPropPath, config)); // Combine with AND for negative operators, OR for positive operators