From ac2632a2d60acd8a6d9d3532d65d6e8755e8f368 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Mon, 19 Oct 2026 20:33:08 +0300 Subject: [PATCH] fix(sort): reject a sort on a collection property A sort on a collection property passed the parse and failed when the query ran: InvalidOperationException in memory, and an EF translation error for a navigation collection. ApplyQueryKitSort now throws SortParsingException for a property that implements IEnumerable, except string and byte[]. Postgres can order by an array column, so a sort such as Tags worked there before. The same sort failed in memory, and the order has no clear meaning, so QueryKit now rejects it on every provider. Fixes #205 --- .../verify-querykit/features/sorting.md | 3 + .../Tests/DatabaseSortingTests.cs | 18 +++ QueryKit.UnitTests/CollectionSortTests.cs | 112 ++++++++++++++++++ QueryKit/Exceptions/SortParsingException.cs | 8 ++ QueryKit/SortParser.cs | 6 + 5 files changed, 147 insertions(+) create mode 100644 QueryKit.UnitTests/CollectionSortTests.cs diff --git a/.agents/skills/verify-querykit/features/sorting.md b/.agents/skills/verify-querykit/features/sorting.md index c575cdb..6d7acdb 100644 --- a/.agents/skills/verify-querykit/features/sorting.md +++ b/.agents/skills/verify-querykit/features/sorting.md @@ -8,6 +8,7 @@ A developer passes a comma-separated sort string to `ApplyQueryKitSort`, or pass - `sort-sieve-prefix` sorts descending with a `-` prefix (`-Rating`). - `sort-multi` sorts on more than one key, including a nested property (`Author.Name`). - `sort-aggregate` applies a filter, a sort, and a configuration in one `ApplyQueryKit(QueryKitData)` call. +- `sort-collection-rejected` rejects a sort on a collection property (`Tags`, `Labels`, `Ingredients`) with `SortParsingException`. ## How to get to it (user POV) @@ -26,9 +27,11 @@ Preconditions: - **Nested key and Sieve prefix.** Run `qk run sorting-multi --sort 'Author.Name, -Rating'`. Both targets give `["Plain Water", "Beef Stew", "Pancakes", "Salt Bread"]`. - **Filter then sort.** Run `qk run sorting-filter-then-sort --filter 'Author.Name == "Julia Child"' --sort 'Title desc'`. Both targets give `["Salt Bread", "Pancakes"]`. - **Aggregate with a configuration.** Run `qk run sorting-aggregate --aggregate --config aliases --filter 'chef == "Julia Child"' --sort 'name desc'`. The `input.api` is `ApplyQueryKit(QueryKitData)`. Both targets give `["Salt Bread", "Pancakes"]`. +- **Collection rejected.** Run `qk run sorting-collection --sort 'Title, Tags'`. Both targets give `error.type` `QueryKit.Exceptions.SortParsingException` with `isQueryKitException` true. The message says that `Tags` is a collection. Run it again with `Labels` (a `string[]`) and with `Ingredients` (a navigation collection). Each gives the same error. ## Gotchas - EF Core adds `r."Id"` and `a."Id"` after the QueryKit sort keys, because the driver uses split-query mode. Only the keys before them come from QueryKit. - A sort on a property with `PreventSort()` is ignored without an error. The rows come back in source order (see `configuration.md`). - An unknown sort property throws `SortParsingException` (see `error-handling.md`). +- Postgres can order by an array column, but QueryKit rejects the sort before the query runs. The same sort fails in memory, so QueryKit rejects it on every target. diff --git a/QueryKit.IntegrationTests/Tests/DatabaseSortingTests.cs b/QueryKit.IntegrationTests/Tests/DatabaseSortingTests.cs index 4655e46..2839507 100644 --- a/QueryKit.IntegrationTests/Tests/DatabaseSortingTests.cs +++ b/QueryKit.IntegrationTests/Tests/DatabaseSortingTests.cs @@ -462,4 +462,22 @@ public async Task can_sort_descending_by_nullable_navigation_property() recipes[1].Title.Should().Be($"RecipeZ_{uniqueId}"); // Zoe recipes[2].Title.Should().Be($"RecipeA_{uniqueId}"); // Alice } + + [Theory] + [InlineData("Tags")] + [InlineData("Tags desc")] + [InlineData("Ingredients")] + public async Task sort_on_a_collection_property_throws_sort_parsing_exception(string input) + { + // Arrange + var testingServiceScope = new TestingServiceScope(); + var queryableRecipes = testingServiceScope.DbContext().Recipes; + + // Act + var act = () => queryableRecipes.ApplyQueryKitSort(input).ToListAsync(); + + // Assert + await act.Should().ThrowAsync() + .WithMessage("*is a collection, and a collection has no single sort order."); + } } diff --git a/QueryKit.UnitTests/CollectionSortTests.cs b/QueryKit.UnitTests/CollectionSortTests.cs new file mode 100644 index 0000000..6db4c84 --- /dev/null +++ b/QueryKit.UnitTests/CollectionSortTests.cs @@ -0,0 +1,112 @@ +namespace QueryKit.UnitTests; + +using Configuration; +using Exceptions; +using FluentAssertions; + +// A collection has no single sort order, so a sort on a collection property is a parsing error. +public class CollectionSortTests +{ + [Theory] + [InlineData(nameof(Row.List))] + [InlineData(nameof(Row.ReadOnlyList))] + [InlineData(nameof(Row.Set))] + [InlineData(nameof(Row.Array))] + [InlineData(nameof(Row.Numbers))] + [InlineData(nameof(Row.Children))] + [InlineData("Parent.Tags")] + public void sort_on_a_collection_property_throws_sort_parsing_exception(string property) + { + var act = () => Rows().AsQueryable().ApplyQueryKitSort(property).ToList(); + + act.Should().Throw() + .WithMessage($"Parsing failed during sorting. '{property}' is a collection, and a collection has no single sort order."); + } + + [Theory] + [InlineData("List desc")] + [InlineData("-List")] + [InlineData("Name, List")] + public void sort_on_a_collection_property_throws_for_every_sort_form(string input) + { + var act = () => Rows().ApplyQueryKitSort(input).ToList(); + + act.Should().Throw(); + } + + [Fact] + public void sort_on_a_collection_property_throws_through_apply_querykit() + { + var act = () => Rows().AsQueryable().ApplyQueryKit(new QueryKitData { SortOrder = "List" }).ToList(); + + act.Should().Throw(); + } + + [Fact] + public void sort_on_a_collection_query_name_names_the_query_name() + { + var config = new QueryKitConfiguration(settings => + { + settings.Property(x => x.List).HasQueryName("tags"); + }); + + var act = () => Rows().AsQueryable().ApplyQueryKitSort("tags", config).ToList(); + + act.Should().Throw() + .WithMessage("Parsing failed during sorting. 'tags' is a collection, and a collection has no single sort order."); + } + + [Fact] + public void prevented_sort_on_a_collection_property_is_ignored() + { + var config = new QueryKitConfiguration(settings => + { + settings.Property(x => x.List).PreventSort(); + }); + + var result = Rows().AsQueryable().ApplyQueryKitSort("List, Name desc", config).ToList(); + + result.Select(x => x.Name).Should().Equal("b", "a"); + } + + [Fact] + public void sort_on_a_string_still_sorts() + { + var result = Rows().ApplyQueryKitSort("Name desc").ToList(); + + result.Select(x => x.Name).Should().Equal("b", "a"); + } + + // A database sorts a byte array as one binary value, for example a SQL Server rowversion. In memory, byte[] is not IComparable. + [Fact] + public void sort_on_a_byte_array_parses() + { + var sorts = SortParser.ParseSort("Version desc"); + + sorts.Should().ContainSingle().Which.Expression!.Body.ToString().Should().Be("Convert(x.Version, Object)"); + } + + private static Row[] Rows() => + [ + new("a", ["x"], [1], [1]), + new("b", ["y"], [2], [2]), + ]; + + public class Row(string name, string[] tags, int[] numbers, byte[] version) + { + public string Name { get; } = name; + public List List { get; } = [..tags]; + public IReadOnlyList ReadOnlyList { get; } = tags; + public HashSet Set { get; } = [..tags]; + public string[] Array { get; } = tags; + public List Numbers { get; } = [..numbers]; + public List Children { get; } = []; + public Parent Parent { get; } = new(tags); + public byte[] Version { get; } = version; + } + + public class Parent(string[] tags) + { + public List Tags { get; } = [..tags]; + } +} diff --git a/QueryKit/Exceptions/SortParsingException.cs b/QueryKit/Exceptions/SortParsingException.cs index d104ec9..2ed9fa6 100644 --- a/QueryKit/Exceptions/SortParsingException.cs +++ b/QueryKit/Exceptions/SortParsingException.cs @@ -6,4 +6,12 @@ public SortParsingException(string propertyName) : base($"Parsing failed during sorting. '{propertyName}' was not recognized.") { } + + private SortParsingException(string message, bool isMessage) + : base(message) + { + } + + internal static SortParsingException ForCollection(string propertyName) + => new($"Parsing failed during sorting. '{propertyName}' is a collection, and a collection has no single sort order.", isMessage: true); } diff --git a/QueryKit/SortParser.cs b/QueryKit/SortParser.cs index ecc71b0..0f94c7a 100644 --- a/QueryKit/SortParser.cs +++ b/QueryKit/SortParser.cs @@ -76,6 +76,12 @@ private static SortExpressionInfo CreateSortExpression(string sortClause, IsAscending = true }; } + + // A collection has no single sort order. A byte[] is one binary value, for example a SQL Server rowversion, so a database can sort it. + if (sortExpressionBody.Type.IsCollection() && sortExpressionBody.Type != typeof(byte[])) + { + throw SortParsingException.ForCollection(propertyName); + } var isAscending = direction == Ascending; return new SortExpressionInfo