Skip to content

fix(filter)!: resolve only public members #161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pdevito3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(filter)!: resolve only public members #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions QueryKit.IntegrationTests/Tests/PropertyResolverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ namespace QueryKit.IntegrationTests.Tests;

using Bogus;
using Configuration;
using Exceptions;
using FluentAssertions;
using Microsoft.EntityFrameworkCore;
using SharedTestingHelper.Fakes;
Expand Down Expand Up @@ -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<UnknownFilterPropertyException>()
.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;
Expand All @@ -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);
}
}
44 changes: 16 additions & 28 deletions QueryKit.UnitTests/PropertyResolverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<MemberLookupModel>(input);

filterExpression.ToDisplayString().Should().Be(expected);
}

[Fact]
public void non_public_member_filters_the_rows()
{
var models = new List<MemberLookupModel>
{
new(internalScore: 50, rank: 7),
new(internalScore: 20, rank: 3),
};

var result = models.ApplyQueryKitFilter("InternalScore > 30 && secretRank == 7").ToList();
var act = () => FilterParser.ParseFilter<MemberLookupModel>(input);

result.Should().ContainSingle().Which.Should().BeSameAs(models[0]);
act.Should().ThrowExactly<UnknownFilterPropertyException>()
.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 =>
Expand All @@ -791,21 +778,22 @@ public void non_public_member_filters_when_unknown_properties_are_allowed()

var filterExpression = FilterParser.ParseFilter<MemberLookupModel>(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 =>
{
config.Property<MemberLookupModel>(x => x.InternalScore).HasQueryName("score");
});

var filterExpression = FilterParser.ParseFilter<MemberLookupModel>(input, config);
var act = () => FilterParser.ParseFilter<MemberLookupModel>(input, config);

filterExpression.ToDisplayString().Should().Be("x => (x.InternalScore > 30)");
act.Should().ThrowExactly<UnknownFilterPropertyException>()
.WithMessage("The filter property 'InternalScore' was not recognized.");
}

[Fact]
Expand Down
20 changes: 8 additions & 12 deletions QueryKit/PropertyResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
}

Expand All @@ -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 &&
Expand Down
Loading