From 9585aa4a4fbd24abf548be920b4c82e9cbf22a97 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Fri, 29 Jun 2018 15:46:52 +0200 Subject: [PATCH] Fix content ordering when querying by ids. --- .../Contents/ContentQueryService.cs | 27 ++++--- .../Contents/IContentQueryService.cs | 2 +- .../Contents/QueryExecutionContext.cs | 2 +- .../CollectionExtensions.cs | 7 ++ .../Contents/ContentsController.cs | 4 +- .../Contents/ContentQueryServiceTests.cs | 72 +++++++++++-------- .../Contents/GraphQL/GraphQLQueriesTests.cs | 2 +- 7 files changed, 70 insertions(+), 46 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/ContentQueryService.cs b/src/Squidex.Domain.Apps.Entities/Contents/ContentQueryService.cs index 5ec1f122e..f2602ccbc 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/ContentQueryService.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/ContentQueryService.cs @@ -83,7 +83,7 @@ namespace Squidex.Domain.Apps.Entities.Contents throw new DomainObjectNotFoundException(id.ToString(), typeof(ISchemaEntity)); } - return TransformContent(context, schema, true, content); + return Transform(context, schema, true, content); } } @@ -100,11 +100,11 @@ namespace Squidex.Domain.Apps.Entities.Contents var contents = await contentRepository.QueryAsync(context.App, schema, parsedStatus, parsedQuery); - return TransformContents(context, schema, true, contents); + return Transform(context, schema, true, contents); } } - public async Task> QueryAsync(QueryContext context, HashSet ids) + public async Task> QueryAsync(QueryContext context, IList ids) { Guard.NotNull(context, nameof(context)); Guard.NotNull(ids, nameof(ids)); @@ -115,25 +115,32 @@ namespace Squidex.Domain.Apps.Entities.Contents { var parsedStatus = ParseStatus(context); - var contents = await contentRepository.QueryAsync(context.App, schema, parsedStatus, ids); + var contents = await contentRepository.QueryAsync(context.App, schema, parsedStatus, new HashSet(ids)); - return TransformContents(context, schema, false, contents); + return Sort(Transform(context, schema, false, contents), ids); } } - private IContentEntity TransformContent(QueryContext context, ISchemaEntity schema, bool checkType, IContentEntity content) + private IContentEntity Transform(QueryContext context, ISchemaEntity schema, bool checkType, IContentEntity content) { - return TransformContents(context, schema, checkType, Enumerable.Repeat(content, 1)).FirstOrDefault(); + return Transform(context, schema, checkType, Enumerable.Repeat(content, 1)).FirstOrDefault(); } - private IResultList TransformContents(QueryContext context, ISchemaEntity schema, bool checkType, IResultList contents) + private IResultList Transform(QueryContext context, ISchemaEntity schema, bool checkType, IResultList contents) { - var transformed = TransformContents(context, schema, checkType, (IEnumerable)contents); + var transformed = Transform(context, schema, checkType, (IEnumerable)contents); return ResultList.Create(transformed, contents.Total); } - private IEnumerable TransformContents(QueryContext context, ISchemaEntity schema, bool checkType, IEnumerable contents) + private IResultList Sort(IResultList contents, IList ids) + { + var sorted = ids.Select(id => contents.FirstOrDefault(x => x.Id == id)).Where(x => x != null); + + return ResultList.Create(sorted, contents.Total); + } + + private IEnumerable Transform(QueryContext context, ISchemaEntity schema, bool checkType, IEnumerable contents) { using (Profiler.TraceMethod()) { diff --git a/src/Squidex.Domain.Apps.Entities/Contents/IContentQueryService.cs b/src/Squidex.Domain.Apps.Entities/Contents/IContentQueryService.cs index 7f2c2c4b1..99658ba3b 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/IContentQueryService.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/IContentQueryService.cs @@ -14,7 +14,7 @@ namespace Squidex.Domain.Apps.Entities.Contents { public interface IContentQueryService { - Task> QueryAsync(QueryContext context, HashSet ids); + Task> QueryAsync(QueryContext context, IList ids); Task> QueryAsync(QueryContext context, string query); diff --git a/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs b/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs index 9d29cbb8e..fa8764f53 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs @@ -118,7 +118,7 @@ namespace Squidex.Domain.Apps.Entities.Contents { Guard.NotNull(ids, nameof(ids)); - var notLoadedContents = new HashSet(ids.Where(id => !cachedContents.ContainsKey(id))); + var notLoadedContents = ids.Where(id => !cachedContents.ContainsKey(id)).ToList(); if (notLoadedContents.Count > 0) { diff --git a/src/Squidex.Infrastructure/CollectionExtensions.cs b/src/Squidex.Infrastructure/CollectionExtensions.cs index 18bcc7ae9..8e021e970 100644 --- a/src/Squidex.Infrastructure/CollectionExtensions.cs +++ b/src/Squidex.Infrastructure/CollectionExtensions.cs @@ -14,6 +14,13 @@ namespace Squidex.Infrastructure { public static class CollectionExtensions { + public static IEnumerable Shuffle(this IEnumerable enumerable) + { + var random = new Random(); + + return enumerable.OrderBy(x => random.Next()).ToList(); + } + public static ImmutableDictionary SetItem(this ImmutableDictionary dictionary, TKey key, Func updater) { if (dictionary.TryGetValue(key, out var value)) diff --git a/src/Squidex/Areas/Api/Controllers/Contents/ContentsController.cs b/src/Squidex/Areas/Api/Controllers/Contents/ContentsController.cs index d99b7de22..dbf2e37e1 100644 --- a/src/Squidex/Areas/Api/Controllers/Contents/ContentsController.cs +++ b/src/Squidex/Areas/Api/Controllers/Contents/ContentsController.cs @@ -97,11 +97,11 @@ namespace Squidex.Areas.Api.Controllers.Contents [ApiCosts(2)] public async Task GetContents(string app, string name, [FromQuery] bool archived = false, [FromQuery] string ids = null) { - HashSet idsList = null; + List idsList = null; if (!string.IsNullOrWhiteSpace(ids)) { - idsList = new HashSet(); + idsList = new List(); foreach (var id in ids.Split(',')) { diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentQueryServiceTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentQueryServiceTests.cs index 96bfef62c..c83a35191 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentQueryServiceTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/ContentQueryServiceTests.cs @@ -35,12 +35,10 @@ namespace Squidex.Domain.Apps.Entities.Contents private readonly IContentVersionLoader contentVersionLoader = A.Fake(); private readonly IScriptEngine scriptEngine = A.Fake(); private readonly ISchemaEntity schema = A.Fake(); - private readonly IContentEntity content = A.Fake(); private readonly IAppEntity app = A.Fake(); private readonly IAppProvider appProvider = A.Fake(); private readonly Guid appId = Guid.NewGuid(); private readonly Guid schemaId = Guid.NewGuid(); - private readonly Guid contentId = Guid.NewGuid(); private readonly string appName = "my-app"; private readonly NamedContentData contentData = new NamedContentData(); private readonly NamedContentData contentTransformed = new NamedContentData(); @@ -58,11 +56,6 @@ namespace Squidex.Domain.Apps.Entities.Contents A.CallTo(() => app.Name).Returns(appName); A.CallTo(() => app.LanguagesConfig).Returns(LanguagesConfig.English); - A.CallTo(() => content.Id).Returns(contentId); - A.CallTo(() => content.Data).Returns(contentData); - A.CallTo(() => content.DataDraft).Returns(contentData); - A.CallTo(() => content.Status).Returns(Status.Published); - A.CallTo(() => schema.SchemaDef).Returns(new Schema("my-schema")); context = QueryContext.Create(app, user); @@ -120,19 +113,17 @@ namespace Squidex.Domain.Apps.Entities.Contents [MemberData(nameof(SingleRequestData))] public async Task Should_return_content_from_repository_and_transform(bool isFrontend, params Status[] status) { + var contentId = Guid.NewGuid(); + var content = CreateContent(contentId); + SetupClaims(isFrontend); + SetupScripting(contentId); A.CallTo(() => appProvider.GetSchemaAsync(appId, schemaId, false)) .Returns(schema); A.CallTo(() => contentRepository.FindContentAsync(app, schema, A.That.IsSameSequenceAs(status), contentId)) .Returns(content); - A.CallTo(() => schema.ScriptQuery) - .Returns(""); - - A.CallTo(() => scriptEngine.Transform(A.That.Matches(x => x.User == user && x.ContentId == contentId && ReferenceEquals(x.Data, contentData)), "")) - .Returns(contentTransformed); - var result = await sut.FindContentAsync(context.WithSchemaId(schemaId), contentId); Assert.Equal(contentTransformed, result.Data); @@ -142,17 +133,16 @@ namespace Squidex.Domain.Apps.Entities.Contents [Fact] public async Task Should_return_versioned_content_from_repository_and_transform() { + var contentId = Guid.NewGuid(); + var content = CreateContent(contentId); + + SetupScripting(contentId); + A.CallTo(() => appProvider.GetSchemaAsync(appId, schemaId, false)) .Returns(schema); A.CallTo(() => contentVersionLoader.LoadAsync(contentId, 10)) .Returns(content); - A.CallTo(() => schema.ScriptQuery) - .Returns(""); - - A.CallTo(() => scriptEngine.Transform(A.That.Matches(x => x.User == user && x.ContentId == contentId && ReferenceEquals(x.Data, contentData)), "")) - .Returns(contentTransformed); - var result = await sut.FindContentAsync(context.WithSchemaId(schemaId), contentId, 10); Assert.Equal(contentTransformed, result.Data); @@ -162,6 +152,8 @@ namespace Squidex.Domain.Apps.Entities.Contents [Fact] public async Task Should_throw_if_content_to_find_does_not_exist() { + var contentId = Guid.NewGuid(); + A.CallTo(() => appProvider.GetSchemaAsync(appId, schemaId, false)) .Returns(schema); @@ -183,8 +175,11 @@ namespace Squidex.Domain.Apps.Entities.Contents [MemberData(nameof(ManyRequestData))] public async Task Should_query_contents_by_query_from_repository_and_transform(int count, int total, bool isFrontend, bool archive, params Status[] status) { + var contentId = Guid.NewGuid(); + var content = CreateContent(contentId); + SetupClaims(isFrontend); - SetupFakeWithScripting(); + SetupScripting(contentId); A.CallTo(() => appProvider.GetSchemaAsync(appId, schemaId, false)) .Returns(schema); @@ -235,22 +230,20 @@ namespace Squidex.Domain.Apps.Entities.Contents [MemberData(nameof(ManyIdRequestData))] public async Task Should_query_contents_by_id_from_repository_and_transform(int count, int total, bool isFrontend, bool archive, params Status[] status) { - var ids = new HashSet(Enumerable.Range(0, count).Select(x => Guid.NewGuid())); + var ids = Enumerable.Range(0, count).Select(x => Guid.NewGuid()).ToList(); SetupClaims(isFrontend); - SetupFakeWithScripting(); + SetupScripting(ids.ToArray()); A.CallTo(() => appProvider.GetSchemaAsync(appId, schemaId, false)) .Returns(schema); - A.CallTo(() => contentRepository.QueryAsync(app, schema, A.That.IsSameSequenceAs(status), ids)) - .Returns(ResultList.Create(Enumerable.Repeat(content, count), total)); + A.CallTo(() => contentRepository.QueryAsync(app, schema, A.That.IsSameSequenceAs(status), A>.Ignored)) + .Returns(ResultList.Create(ids.Select(x => CreateContent(x)).Shuffle(), total)); var result = await sut.QueryAsync(context.WithSchemaId(schemaId).WithArchived(archive), ids); - Assert.Equal(contentData, result[0].Data); - Assert.Equal(content.Id, result[0].Id); - + Assert.Equal(ids, result.Select(x => x.Id).ToList()); Assert.Equal(total, result.Total); if (!isFrontend) @@ -273,13 +266,30 @@ namespace Squidex.Domain.Apps.Entities.Contents } } - private void SetupFakeWithScripting() + private void SetupScripting(params Guid[] contentId) { + var script = ""; + A.CallTo(() => schema.ScriptQuery) - .Returns(""); + .Returns(script); + + foreach (var id in contentId) + { + A.CallTo(() => scriptEngine.Transform(A.That.Matches(x => x.User == user && x.ContentId == id && x.Data == contentData), script)) + .Returns(contentTransformed); + } + } + + private IContentEntity CreateContent(Guid id, Status status = Status.Published) + { + var content = A.Fake(); + + A.CallTo(() => content.Id).Returns(id); + A.CallTo(() => content.Data).Returns(contentData); + A.CallTo(() => content.DataDraft).Returns(contentData); + A.CallTo(() => content.Status).Returns(status); - A.CallTo(() => scriptEngine.Transform(A.That.Matches(x => x.User == user && x.ContentId == contentId && ReferenceEquals(x.Data, contentData)), "")) - .Returns(contentTransformed); + return content; } } } \ No newline at end of file diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs index 03d7d7fac..287278209 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs @@ -635,7 +635,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL A.CallTo(() => contentQuery.FindContentAsync(ContextMatch(), contentId, EtagVersion.Any)) .Returns(content); - A.CallTo(() => contentQuery.QueryAsync(ContextMatch(), A>.That.Matches(x => x.Contains(contentRefId)))) + A.CallTo(() => contentQuery.QueryAsync(ContextMatch(), A>.That.IsSameSequenceAs(new[] { contentRefId }))) .Returns(ResultList.Create(refContents, 0)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query });