From 87849848e838861e5a0bb2e2d825e7b48217c45c Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Thu, 1 Oct 2026 20:36:09 +0300 Subject: [PATCH] fix(filter)!: resolve only public members A filter segment now matches only a public property or a public field, ignoring case. An internal, protected, or private member is an unknown property. This reverts the non-public lookup that the v1.14.2 restore brought back. An indexer stays an unknown property. BREAKING CHANGE: a filter on an internal, protected, or private member now throws UnknownFilterPropertyException, also through a query name. With AllowUnknownProperties, the clause becomes True == True and does not filter. Make the member public to filter on it. --- .../Tests/PropertyResolverTests.cs | 32 +++++--------- QueryKit.UnitTests/PropertyResolverTests.cs | 44 +++++++------------ QueryKit/PropertyResolver.cs | 20 ++++----- 3 files changed, 36 insertions(+), 60 deletions(-) diff --git a/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs b/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs index 432436b..4fae340 100644 --- a/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs +++ b/QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs @@ -2,6 +2,7 @@ namespace QueryKit.IntegrationTests.Tests; using Bogus; using Configuration; +using Exceptions; using FluentAssertions; using Microsoft.EntityFrameworkCore; using SharedTestingHelper.Fakes; @@ -182,42 +183,34 @@ public async Task query_name_that_is_not_a_plain_identifier_filters_by_its_prope } [Fact] - public async Task non_public_mapped_property_filters_in_the_database() + public void non_public_mapped_property_is_an_unknown_property() { // Arrange var testingServiceScope = new TestingServiceScope(); - var nickname = new Faker().Lorem.Sentence(); - var fakePerson = new FakeTestingPersonBuilder().Build(); - fakePerson.Nickname = nickname; - var otherPerson = new FakeTestingPersonBuilder().Build(); - otherPerson.Nickname = new Faker().Lorem.Sentence(); - await testingServiceScope.InsertAsync(fakePerson, otherPerson); - - var input = $"""nickname == "{nickname}" """; + var input = $"""nickname == "{new Faker().Lorem.Sentence()}" """; // Act - var queryable = testingServiceScope.DbContext().People.ApplyQueryKitFilter(input); - var people = await queryable.ToListAsync(); + var act = () => testingServiceScope.DbContext().People.ApplyQueryKitFilter(input); // Assert - queryable.ToQueryString().Should().Contain("""p.nickname = """); - people.Should().ContainSingle(); - people[0].Id.Should().Be(fakePerson.Id); + act.Should().ThrowExactly() + .WithMessage("The filter property 'nickname' was not recognized."); } [Fact] - public async Task non_public_mapped_property_filters_when_unknown_properties_are_allowed() + public async Task non_public_mapped_property_clause_does_not_filter_when_unknown_properties_are_allowed() { // Arrange var testingServiceScope = new TestingServiceScope(); + var title = new Faker().Lorem.Sentence(); var nickname = new Faker().Lorem.Sentence(); - var fakePerson = new FakeTestingPersonBuilder().Build(); + var fakePerson = new FakeTestingPersonBuilder().WithTitle(title).Build(); fakePerson.Nickname = nickname; - var otherPerson = new FakeTestingPersonBuilder().Build(); + var otherPerson = new FakeTestingPersonBuilder().WithTitle(title).Build(); otherPerson.Nickname = new Faker().Lorem.Sentence(); await testingServiceScope.InsertAsync(fakePerson, otherPerson); - var input = $"""Nickname == "{nickname}" """; + var input = $"""Nickname == "{nickname}" && Title == "{title}" """; var config = new QueryKitConfiguration(config => { config.AllowUnknownProperties = true; @@ -229,7 +222,6 @@ public async Task non_public_mapped_property_filters_when_unknown_properties_are .ToListAsync(); // Assert - people.Should().ContainSingle(); - people[0].Id.Should().Be(fakePerson.Id); + people.Should().HaveCount(2); } } diff --git a/QueryKit.UnitTests/PropertyResolverTests.cs b/QueryKit.UnitTests/PropertyResolverTests.cs index 040943f..cf61af7 100644 --- a/QueryKit.UnitTests/PropertyResolverTests.cs +++ b/QueryKit.UnitTests/PropertyResolverTests.cs @@ -753,35 +753,22 @@ public void unknown_property_in_arithmetic_throws_an_argument_exception() } [Theory] - [InlineData("InternalScore > 30", "x => (x.InternalScore > 30)")] - [InlineData("internalscore > 30", "x => (x.InternalScore > 30)")] - [InlineData("""ProtectedNote == "a" """, """x => (x.ProtectedNote == "a")""")] - [InlineData("secretRank == 7", "x => (x.secretRank == 7)")] - [InlineData("""Owner.InternalAlias == "Ann" """, """x => (x.Owner.InternalAlias == "Ann")""")] - [InlineData("(InternalScore, Rating) > 3", "x => ((x.InternalScore > 3) OrElse (x.Rating > 3))")] - public void non_public_member_filters_like_a_public_member(string input, string expected) + [InlineData("InternalScore > 30", "InternalScore")] + [InlineData("internalscore > 30", "internalscore")] + [InlineData("""ProtectedNote == "a" """, "ProtectedNote")] + [InlineData("secretRank == 7", "secretRank")] + [InlineData("""Owner.InternalAlias == "Ann" """, "InternalAlias")] + [InlineData("(InternalScore, Rating) > 3", "InternalScore")] + public void non_public_member_is_an_unknown_property(string input, string unknownProperty) { - var filterExpression = FilterParser.ParseFilter(input); - - filterExpression.ToDisplayString().Should().Be(expected); - } - - [Fact] - public void non_public_member_filters_the_rows() - { - var models = new List - { - new(internalScore: 50, rank: 7), - new(internalScore: 20, rank: 3), - }; - - var result = models.ApplyQueryKitFilter("InternalScore > 30 && secretRank == 7").ToList(); + var act = () => FilterParser.ParseFilter(input); - result.Should().ContainSingle().Which.Should().BeSameAs(models[0]); + act.Should().ThrowExactly() + .WithMessage($"The filter property '{unknownProperty}' was not recognized."); } [Fact] - public void non_public_member_filters_when_unknown_properties_are_allowed() + public void non_public_member_clause_is_true_equals_true_when_unknown_properties_are_allowed() { var input = """secretRank > 100 || Rating == 1"""; var config = new QueryKitConfiguration(config => @@ -791,11 +778,11 @@ public void non_public_member_filters_when_unknown_properties_are_allowed() var filterExpression = FilterParser.ParseFilter(input, config); - filterExpression.ToDisplayString().Should().Be("x => ((x.secretRank > 100) OrElse (x.Rating == 1))"); + filterExpression.ToDisplayString().Should().Be("x => ((True == True) OrElse (x.Rating == 1))"); } [Fact] - public void query_name_on_a_non_public_member_filters_by_that_member() + public void query_name_on_a_non_public_member_throws_unknown_property() { var input = """score > 30"""; var config = new QueryKitConfiguration(config => @@ -803,9 +790,10 @@ public void query_name_on_a_non_public_member_filters_by_that_member() config.Property(x => x.InternalScore).HasQueryName("score"); }); - var filterExpression = FilterParser.ParseFilter(input, config); + var act = () => FilterParser.ParseFilter(input, config); - filterExpression.ToDisplayString().Should().Be("x => (x.InternalScore > 30)"); + act.Should().ThrowExactly() + .WithMessage("The filter property 'InternalScore' was not recognized."); } [Fact] diff --git a/QueryKit/PropertyResolver.cs b/QueryKit/PropertyResolver.cs index 4645ef5..404b5c5 100644 --- a/QueryKit/PropertyResolver.cs +++ b/QueryKit/PropertyResolver.cs @@ -76,10 +76,9 @@ internal static PropertyReference Resolve(Type rootType, string reference, IQuer return PropertyReference.NotMember(PropertyReferenceKind.Unknown, reference, null, unknownSegment!); } - // Matches each segment to a member, ignoring case, in the order of Expression.PropertyOrField like v1.14.2: - // a public property, a public field, a non-public property, and then a non-public field. An indexer does not match. + // Matches each segment to a public member, ignoring case: a public property, and then a public field. An indexer does not match. // A segment after a collection resolves on the element type. - // After a collection, only public properties match: the first segment in the exact case, a later segment in any case. + // After a collection, only properties match: the first segment in the exact case, a later segment in any case. // A segment after a collection that does not match throws NullReferenceException. private static string? ResolveMemberPath(Type rootType, string path, out string? unknownSegment) { @@ -98,22 +97,20 @@ internal static PropertyReference Resolve(Type rootType, string reference, IQuer MemberInfo? member; if (firstAfterCollection || afterCollection) { - member = (firstAfterCollection ? currentType.GetProperty(segment) : currentType.GetProperty(segment, PublicMemberFlags)) + member = (firstAfterCollection ? currentType.GetProperty(segment) : currentType.GetProperty(segment, MemberFlags)) ?? throw new NullReferenceException(); afterCollection = true; } else { - member = (MemberInfo?)currentType.GetProperty(segment, PublicMemberFlags) - ?? (MemberInfo?)currentType.GetField(segment, PublicMemberFlags) - ?? (MemberInfo?)currentType.GetProperty(segment, NonPublicMemberFlags) - ?? currentType.GetField(segment, NonPublicMemberFlags); + member = (MemberInfo?)currentType.GetProperty(segment, MemberFlags) + ?? currentType.GetField(segment, MemberFlags); } if (member == null || member is PropertyInfo indexer && indexer.GetIndexParameters().Length > 0) { - // v1.14.2 named an unknown member by the name of the public property with that name, if there was one. - unknownSegment = currentType.GetProperty(segment, PublicMemberFlags)?.Name ?? segment; + // An indexer is named by its property name, like v1.14.2. + unknownSegment = member?.Name ?? segment; return null; } @@ -125,8 +122,7 @@ internal static PropertyReference Resolve(Type rootType, string reference, IQuer return string.Join(".", memberNames); } - private const BindingFlags PublicMemberFlags = BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance; - private const BindingFlags NonPublicMemberFlags = BindingFlags.IgnoreCase | BindingFlags.NonPublic | BindingFlags.Instance; + private const BindingFlags MemberFlags = BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance; private static bool IsCollection(Type type) => type != typeof(string) && type.IsGenericType &&