From 98e2ed1987ec2a16c230f7ba917cf0397a33e18e Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Thu, 26 Nov 2020 15:39:02 +0100 Subject: [PATCH] Consistent ordering through ids. (#601) --- .../Assets/Queries/AssetQueryParser.cs | 6 +++++ .../Contents/Queries/ContentQueryParser.cs | 5 ++++ .../Assets/Queries/AssetQueryParserTests.cs | 24 ++++++++++++----- .../Queries/ContentQueryParserTests.cs | 26 +++++++++++++------ 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/AssetQueryParser.cs b/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/AssetQueryParser.cs index a8b089766..030ebc5ac 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/AssetQueryParser.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/AssetQueryParser.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Options; using Microsoft.OData; @@ -84,6 +85,11 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries result.Sort.Add(new SortNode(new List { "lastModified" }, SortOrder.Descending)); } + if (!result.Sort.Any(x => string.Equals(x.Path.ToString(), "id", StringComparison.OrdinalIgnoreCase))) + { + result.Sort.Add(new SortNode(new List { "id" }, SortOrder.Ascending)); + } + if (result.Take == long.MaxValue) { result.Take = options.DefaultPageSize; diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs index d2ca81076..ebaca8ef8 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs @@ -89,6 +89,11 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries result.Sort.Add(new SortNode(new List { "lastModified" }, SortOrder.Descending)); } + if (!result.Sort.Any(x => string.Equals(x.Path.ToString(), "id", StringComparison.OrdinalIgnoreCase))) + { + result.Sort.Add(new SortNode(new List { "id" }, SortOrder.Ascending)); + } + if (result.Take == long.MaxValue) { result.Take = options.DefaultPageSize; diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/AssetQueryParserTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/AssetQueryParserTests.cs index a96369156..1f4953d76 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/AssetQueryParserTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/AssetQueryParserTests.cs @@ -67,7 +67,7 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("FullText: 'Hello World'; Take: 100; Sort: fileName Ascending", parsed.ToString()); + Assert.Equal("FullText: 'Hello World'; Take: 100; Sort: fileName Ascending, id Ascending", parsed.ToString()); } [Fact] @@ -77,7 +77,7 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("Filter: fileName == 'ABC'; Take: 200; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Filter: fileName == 'ABC'; Take: 200; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -87,7 +87,7 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("Filter: fileName == 'ABC'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Filter: fileName == 'ABC'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -97,7 +97,7 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("FullText: 'Hello'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("FullText: 'Hello'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -107,7 +107,7 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -117,7 +117,17 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("Skip: 20; Take: 200; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Skip: 20; Take: 200; Sort: lastModified Descending, id Ascending", parsed.ToString()); + } + + [Fact] + public async Task Should_not_apply_id_ordering_twice() + { + var query = Q.Empty.WithODataQuery("$top=300&$skip=20&$orderby=id desc"); + + var parsed = await sut.ParseQueryAsync(requestContext, query); + + Assert.Equal("Skip: 20; Take: 200; Sort: id Descending", parsed.ToString()); } [Fact] @@ -130,7 +140,7 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries var parsed = await sut.ParseQueryAsync(requestContext, query); - Assert.Equal("Filter: tags == 'id1'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Filter: tags == 'id1'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } private static string Json(string text) diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentQueryParserTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentQueryParserTests.cs index c06a30db4..7671ef337 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentQueryParserTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentQueryParserTests.cs @@ -78,7 +78,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("FullText: 'Hello World'; Take: 100; Sort: data.firstName.iv Ascending", parsed.ToString()); + Assert.Equal("FullText: 'Hello World'; Take: 100; Sort: data.firstName.iv Ascending, id Ascending", parsed.ToString()); } [Fact] @@ -88,7 +88,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("Filter: data.firstName.iv == 'ABC'; Take: 200; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Filter: data.firstName.iv == 'ABC'; Take: 200; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -98,7 +98,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("Filter: data.firstName.iv == 'ABC'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Filter: data.firstName.iv == 'ABC'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -112,7 +112,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("Filter: data.firstName.iv == 'ABC'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Filter: data.firstName.iv == 'ABC'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -122,7 +122,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("FullText: 'Hello'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("FullText: 'Hello'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -136,7 +136,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("FullText: 'Hello'; Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("FullText: 'Hello'; Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -146,7 +146,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("Take: 30; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Take: 30; Sort: lastModified Descending, id Ascending", parsed.ToString()); } [Fact] @@ -156,7 +156,17 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var parsed = await sut.ParseQueryAsync(requestContext, schema, query); - Assert.Equal("Skip: 20; Take: 200; Sort: lastModified Descending", parsed.ToString()); + Assert.Equal("Skip: 20; Take: 200; Sort: lastModified Descending, id Ascending", parsed.ToString()); + } + + [Fact] + public async Task Should_not_apply_id_ordering_twice() + { + var query = Q.Empty.WithODataQuery("$top=300&$skip=20&$orderby=id desc"); + + var parsed = await sut.ParseQueryAsync(requestContext, schema, query); + + Assert.Equal("Skip: 20; Take: 200; Sort: id Descending", parsed.ToString()); } private static string Json(string text)