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