diff --git a/QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs b/QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs new file mode 100644 index 0000000..22454fc --- /dev/null +++ b/QueryKit.IntegrationTests/Tests/DotNumberCultureTests.cs @@ -0,0 +1,55 @@ +namespace QueryKit.IntegrationTests.Tests; + +using System.Globalization; +using Exceptions; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using SharedTestingHelper.Fakes; +using WebApiTestProject.Entities; + +// Like v1.14.2, a '.' number on an integer property throws ParsingException in a culture +// whose decimal separator is not '.', and a '.' number on a decimal property still filters. +public class DotNumberCultureTests : TestBase +{ + [Fact] + public async Task dot_number_on_an_integer_property_throws_parsing_exception_in_de_de() + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var title = $"culture {Guid.NewGuid()}"; + var fakePersonOne = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithAge(5) + .WithRating(4.6M) + .Build(); + var fakePersonTwo = new FakeTestingPersonBuilder() + .WithTitle(title) + .WithAge(3) + .WithRating(4.4M) + .Build(); + await testingServiceScope.InsertAsync(fakePersonOne, fakePersonTwo); + + // Act + var originalCulture = CultureInfo.CurrentCulture; + IQueryable appliedQueryable; + try + { + CultureInfo.CurrentCulture = new CultureInfo("de-DE"); + var integerFilter = () => testingServiceScope.DbContext().People + .ApplyQueryKitFilter($"""Title == "{title}" && Age > 4.4"""); + integerFilter.Should().ThrowExactly(); + + appliedQueryable = testingServiceScope.DbContext().People + .ApplyQueryKitFilter($"""Title == "{title}" && Rating > 4.5"""); + } + finally + { + CultureInfo.CurrentCulture = originalCulture; + } + var people = await appliedQueryable.ToListAsync(); + + // Assert + people.Count.Should().Be(1); + people[0].Id.Should().Be(fakePersonOne.Id); + } +} diff --git a/QueryKit.UnitTests/DotNumberCultureTests.cs b/QueryKit.UnitTests/DotNumberCultureTests.cs new file mode 100644 index 0000000..89c99f4 --- /dev/null +++ b/QueryKit.UnitTests/DotNumberCultureTests.cs @@ -0,0 +1,66 @@ +namespace QueryKit.UnitTests; + +using System.Globalization; +using Exceptions; +using FluentAssertions; +using WebApiTestProject.Entities.Recipes; + +// In a culture whose decimal separator is not '.', v1.14.2 read only the part of a '.' number before the '.'. +// A '.' number that does not convert to the property type gives the same exception type as v1.14.2. +public class DotNumberCultureTests +{ + [Theory] + [InlineData("de-DE", "Rating > 4.4")] + [InlineData("fr-FR", "Rating > 4.4")] + [InlineData("de-DE", "Rating == -4.0")] + [InlineData("de-DE", "Rating > 4.4 || Title == \"x\"")] + [InlineData("de-DE", "Rating == .5")] + [InlineData("de-DE", "Rating ^^ [4.0]")] + [InlineData("de-DE", "Rating ^^ [\"x\", 4.0]")] + [InlineData("de-DE", "Ingredients.MinimumQuality == 0.0")] + [InlineData("de-DE", "Ingredients.QualityLevel > 4.5")] + [InlineData("de-DE", "Tags #== 2.0")] + [InlineData("de-DE", "(Rating, Title) == 4.0")] + public void dot_number_on_an_integer_property_throws_parsing_exception(string cultureName, string input) + { + var act = () => WithCulture(cultureName, () => FilterParser.ParseFilter(input)); + + act.Should().ThrowExactly(); + } + + [Theory] + [InlineData("en-US", "Rating > 4.4")] + [InlineData("de-DE", "Rating > 4,4")] + [InlineData("de-DE", "Rating > \"4.4\"")] + [InlineData("de-DE", "Rating ^^ [\"4.0\"]")] + [InlineData("de-DE", "Rating > @4.4")] + [InlineData("de-DE", "HaveMadeItMyself == 4.4")] + public void number_that_v1_14_2_also_converted_throws_format_exception(string cultureName, string input) + { + var act = () => WithCulture(cultureName, () => FilterParser.ParseFilter(input)); + + act.Should().ThrowExactly(); + } + + [Fact] + public void integer_value_still_filters_in_a_comma_culture() + { + var filterExpression = WithCulture("de-DE", () => FilterParser.ParseFilter("Rating > 4")); + + filterExpression.ToString().Should().Be("x => (x.Rating > 4)"); + } + + private static TResult WithCulture(string cultureName, Func action) + { + var originalCulture = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo(cultureName); + return action(); + } + finally + { + CultureInfo.CurrentCulture = originalCulture; + } + } +} diff --git a/QueryKit/FilterParser.cs b/QueryKit/FilterParser.cs index 06b22bb..2ec3935 100644 --- a/QueryKit/FilterParser.cs +++ b/QueryKit/FilterParser.cs @@ -256,6 +256,51 @@ from sign in Parse.Char('-').Optional().Select(x => x.IsDefined ? "-" : "") from number in Parse.DecimalInvariant select sign + number; + // v1.14.2 read a number only with the decimal separator of the current culture. + private static readonly Parser CultureNumberParser = + from sign in Parse.Char('-').Optional().Select(x => x.IsDefined ? "-" : "") + from number in Parse.Decimal + select sign + number; + + // The part of a number that v1.14.2 read: null when the current culture reads the whole number, + // else the part before the '.' (for example "4" of "4.5" in de-DE), or "" when the culture reads no number. + private static string? CultureNumberPrefix(string number) + { + var result = CultureNumberParser.TryParse(number); + if (!result.WasSuccessful) + { + return ""; + } + + return result.Value.Length == number.Length ? null : result.Value; + } + + // In a culture whose decimal separator is not '.', v1.14.2 read only the prefix of a '.' number (see CultureNumberPrefix), + // built the clause with that prefix, and then failed in the grammar at the '.'. + // If the clause builds with the whole number, the filter is valid. If it does not build, give the v1.14.2 result: + // the exception of the clause for the prefix, else ParsingException. In a '.' culture, nothing changes. + private static Expression BuildClauseLikeV1142(string? cultureNumberPrefix, string right, Func buildClause) + { + if (cultureNumberPrefix == null) + { + return buildClause(right); + } + + try + { + return buildClause(right); + } + catch (Exception exception) + { + if (cultureNumberPrefix.Length > 0) + { + buildClause(cultureNumberPrefix); + } + + throw new ParsingException(exception); + } + } + private static readonly Parser GuidFormatParser = Parse.Regex(@"[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}").Text(); private static readonly Parser RawStringLiteralParser = @@ -268,32 +313,35 @@ from closingQuotes in Parse.Char('"').Repeat(count).Text() // Carries whether the right-hand value was written as a quoted string literal (e.g. "id"). // This is needed to disambiguate a literal from a bare property reference (property-to-property // comparison) once the surrounding quotes have been stripped, since both are otherwise identical strings. - private readonly record struct RightSideValue(string Value, bool IsQuotedLiteral); + // CultureNumberPrefix is set when the value holds a number that only the '.' decimal point reads (see BuildClauseLikeV1142). + private readonly record struct RightSideValue(string Value, bool IsQuotedLiteral, string? CultureNumberPrefix = null); - private static readonly Parser> SquareBracketValuesParser = + private static readonly Parser> SquareBracketValuesParser = Parse.String("null").Text() .Or(GuidFormatParser) .Or(DateTimeFormatParser) .Or(TimeFormatParser) - .Or(ListNumberParser) - .Or(RawStringLiteralParser.Or(DoubleQuoteParser)) - .Or(Identifier) + .Select(v => (v, false)) + .Or(ListNumberParser.Select(v => (v, CultureNumberPrefix(v) != null))) + .Or(RawStringLiteralParser.Or(DoubleQuoteParser).Or(Identifier).Select(v => (v, false))) .DelimitedBy(Parse.Char(',').Token()); - private static readonly Parser SquareBracketParser = + // v1.14.2 failed in the grammar at a '.' list number, before it built the clause, so no part of the list is read. + private static readonly Parser SquareBracketParser = from openingBracket in Parse.Char('[') from content in SquareBracketValuesParser from closingBracket in Parse.Char(']') - select "[" + string.Join(",", content) + "]"; + select new RightSideValue("[" + string.Join(",", content.Select(x => x.Value)) + "]", false, + content.Any(x => x.IsDotOnlyNumber) ? "" : null); private static readonly Parser RightSideValueChoiceParser = Parse.String("null").Text().Select(v => new RightSideValue(v, false)) .Or(GuidFormatParser.Select(v => new RightSideValue(v, false))) .XOr(DateTimeFormatParser.Select(v => new RightSideValue(v, false))) .XOr(TimeFormatParser.Select(v => new RightSideValue(v, false))) - .XOr(NumberParser.Select(v => new RightSideValue(v, false))) + .XOr(NumberParser.Select(v => new RightSideValue(v, false, CultureNumberPrefix(v)))) .XOr((RawStringLiteralParser.Or(DoubleQuoteParser)).Select(v => new RightSideValue(v, true))) - .XOr(SquareBracketParser.Select(v => new RightSideValue(v, false))) + .XOr(SquareBracketParser) .XOr(Identifier.Select(v => new RightSideValue(v, false))); // Keep this last to try property paths only if nothing else matches private static readonly Parser RightSideValueParser = @@ -301,7 +349,9 @@ from atSign in Parse.Char('@').Optional() from leadingSpaces in Parse.WhiteSpace.Many() from value in RightSideValueChoiceParser from trailingSpaces in Parse.WhiteSpace.Many() - select atSign.IsDefined ? value with { Value = "@" + value.Value } : value; + select atSign.IsDefined + ? value with { Value = "@" + value.Value, CultureNumberPrefix = value.CultureNumberPrefix is { Length: > 0 } prefix ? "@" + prefix : value.CultureNumberPrefix } + : value; // Arithmetic expression parsers private static readonly Parser ArithmeticOperatorParser = @@ -770,9 +820,10 @@ private static Parser ComparisonExprParser(ParameterExpression pa var regularComparison = CreateLeftExprParser(parameter.Type, config) .SelectMany(reference => comparisonOperatorParser, (reference, op) => new { reference, op }) - .SelectMany(temp => rightSideValueParser, (temp, rightValue) => new { temp.reference, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral }) - .Select(temp => + .SelectMany(temp => rightSideValueParser, (temp, rightValue) => new { temp.reference, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral, cultureNumberPrefix = rightValue.CultureNumberPrefix }) + .Select(clause => BuildClauseLikeV1142(clause.cultureNumberPrefix, clause.right, right => { + var temp = clause with { right = right }; if (temp.reference.Kind == PropertyReferenceKind.CustomOperation) { return CreateCustomOperationExpression(parameter, temp.reference.Mapping!, temp.op, temp.right); @@ -928,7 +979,7 @@ private static Parser ComparisonExprParser(ParameterExpression pa return temp.op.GetExpression(leftExprForComparison, rightExpr, config?.DbContextType, ResolveCaseMode(propertyPath, config)); - }); + })); return propertyListComparison.Or(arithmeticComparison).Or(regularComparison); } @@ -1136,9 +1187,10 @@ private static Parser PropertyListComparisonExprParser( .SelectMany(properties => comparisonOperatorParser, (properties, op) => new { properties, op }) .SelectMany(temp => rightSideValueParser, - (temp, rightValue) => new { temp.properties, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral }) - .Select(temp => + (temp, rightValue) => new { temp.properties, temp.op, right = rightValue.Value, rightIsQuotedLiteral = rightValue.IsQuotedLiteral, cultureNumberPrefix = rightValue.CultureNumberPrefix }) + .Select(clause => BuildClauseLikeV1142(clause.cultureNumberPrefix, clause.right, right => { + var temp = clause with { right = right }; if (!temp.properties.Any()) { throw new InvalidOperationException("Property list cannot be empty"); @@ -1196,7 +1248,7 @@ private static Parser PropertyListComparisonExprParser( // If all properties were filtered out, the clause is ignored. v1.14.2 used true here, not true == true. return result ?? (RemovesIgnoredClauses(config) ? RemovedClauseExpression.Instance : Expression.Constant(true)); - }); + })); } private static Type? GetInnerGenericType(Type type)