From 63ed1eec410b76a7e41b3c036a2f4bfa0137a637 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 7 Aug 2025 12:35:04 +0200 Subject: [PATCH] Adds support for the "folders only" flag on retrieving siblings of a node. (#19861) * Adds support for the "folders only" flag on retrieving siblings of a node. * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Updated test code. * Removed double secondary ordering by node Id and ensured we include this clause for all sort orders. * Ensure that ordering by node Id is always added only once and last, and only if it's not already been included in the order by clause. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Tree/SiblingsDataTypeTreeController.cs | 7 +- .../Tree/SiblingsDocumentTreeController.cs | 4 +- ...SiblingsDocumentBlueprintTreeController.cs | 10 +- .../SiblingsDocumentTypeTreeController.cs | 10 +- .../Media/Tree/SiblingsMediaTreeController.cs | 4 +- .../Tree/SiblingsMediaTypeTreeController.cs | 12 ++- .../Tree/SiblingsTemplateTreeController.cs | 4 +- .../Tree/EntityTreeControllerBase.cs | 2 +- .../Tree/FolderTreeControllerBase.cs | 44 ++++++--- .../Entities/UserStartNodeEntitiesService.cs | 4 +- .../Repositories/IEntityRepository.cs | 4 +- src/Umbraco.Core/Services/EntityService.cs | 6 +- src/Umbraco.Core/Services/IEntityService.cs | 4 +- .../Implement/EntityRepository.cs | 98 ++++++++++++++----- .../Services/EntityServiceTests.cs | 70 +++++++++---- .../appsettings.Tests.json | 2 +- .../Services/EntityServiceTests.cs | 2 +- 17 files changed, 202 insertions(+), 85 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Controllers/DataType/Tree/SiblingsDataTypeTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/DataType/Tree/SiblingsDataTypeTreeController.cs index 310bbb4563..d487df48ff 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/DataType/Tree/SiblingsDataTypeTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/DataType/Tree/SiblingsDataTypeTreeController.cs @@ -15,6 +15,9 @@ public class SiblingsDataTypeTreeController : DataTypeTreeControllerBase [HttpGet("siblings")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after) - => GetSiblings(target, before, after); + public async Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, bool foldersOnly = false) + { + RenderFoldersOnly(foldersOnly); + return await GetSiblings(target, before, after); + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs index cf2f1b4f38..850eb9f856 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs @@ -36,9 +36,9 @@ public class SiblingsDocumentTreeController : DocumentTreeControllerBase [HttpGet("siblings")] [MapToApiVersion("1.0")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null) + public async Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null) { IgnoreUserStartNodesForDataType(dataTypeId); - return GetSiblings(target, before, after); + return await GetSiblings(target, before, after); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/DocumentBlueprint/Tree/SiblingsDocumentBlueprintTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/DocumentBlueprint/Tree/SiblingsDocumentBlueprintTreeController.cs index 528d494f77..6c5e5314b6 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/DocumentBlueprint/Tree/SiblingsDocumentBlueprintTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/DocumentBlueprint/Tree/SiblingsDocumentBlueprintTreeController.cs @@ -16,10 +16,14 @@ public class SiblingsDocumentBlueprintTreeController : DocumentBlueprintTreeCont [HttpGet("siblings")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings( + public async Task>> Siblings( CancellationToken cancellationToken, Guid target, int before, - int after) => - GetSiblings(target, before, after); + int after, + bool foldersOnly = false) + { + RenderFoldersOnly(foldersOnly); + return await GetSiblings(target, before, after); + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/Tree/SiblingsDocumentTypeTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/Tree/SiblingsDocumentTypeTreeController.cs index 9de9f5e1bc..c0c36ab9d8 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/Tree/SiblingsDocumentTypeTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/DocumentType/Tree/SiblingsDocumentTypeTreeController.cs @@ -15,10 +15,14 @@ public class SiblingsDocumentTypeTreeController : DocumentTypeTreeControllerBase [HttpGet("siblings")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings( + public async Task>> Siblings( CancellationToken cancellationToken, Guid target, int before, - int after) => - GetSiblings(target, before, after); + int after, + bool foldersOnly = false) + { + RenderFoldersOnly(foldersOnly); + return await GetSiblings(target, before, after); + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs index 8195d03cac..012816dbfc 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs @@ -25,9 +25,9 @@ public class SiblingsMediaTreeController : MediaTreeControllerBase [HttpGet("siblings")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null) + public async Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null) { IgnoreUserStartNodesForDataType(dataTypeId); - return GetSiblings(target, before, after); + return await GetSiblings(target, before, after); } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/MediaType/Tree/SiblingsMediaTypeTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/MediaType/Tree/SiblingsMediaTypeTreeController.cs index 4b445ea889..f4dd06d632 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/MediaType/Tree/SiblingsMediaTypeTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/MediaType/Tree/SiblingsMediaTypeTreeController.cs @@ -15,6 +15,14 @@ public class SiblingsMediaTypeTreeController : MediaTypeTreeControllerBase [HttpGet("siblings")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after) - => GetSiblings(target, before, after); + public async Task>> Siblings( + CancellationToken cancellationToken, + Guid target, + int before, + int after, + bool foldersOnly = false) + { + RenderFoldersOnly(foldersOnly); + return await GetSiblings(target, before, after); + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Template/Tree/SiblingsTemplateTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Template/Tree/SiblingsTemplateTreeController.cs index 60390caa22..f566dd52f6 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Template/Tree/SiblingsTemplateTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Template/Tree/SiblingsTemplateTreeController.cs @@ -15,10 +15,10 @@ public class SiblingsTemplateTreeController : TemplateTreeControllerBase [HttpGet("siblings")] [ProducesResponseType(typeof(SubsetViewModel), StatusCodes.Status200OK)] - public Task>> Siblings( + public async Task>> Siblings( CancellationToken cancellationToken, Guid target, int before, int after) => - GetSiblings(target, before, after); + await GetSiblings(target, before, after); } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs index 80c39c5fa0..2f5a62b9bb 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs @@ -127,7 +127,7 @@ public abstract class EntityTreeControllerBase : ManagementApiControllerB EntityService .GetSiblings( target, - ItemObjectType, + [ItemObjectType], before, after, out totalBefore, diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Tree/FolderTreeControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Tree/FolderTreeControllerBase.cs index a7c201372c..4e52bbe4e8 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Tree/FolderTreeControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Tree/FolderTreeControllerBase.cs @@ -51,6 +51,24 @@ public abstract class FolderTreeControllerBase : NamedEntityTreeControlle take, out totalItems); + protected override IEntitySlim[] GetSiblingEntities(Guid target, int before, int after, out long totalBefore, out long totalAfter) + { + totalBefore = 0; + totalAfter = 0; + + UmbracoObjectTypes[] siblingObjectTypes = GetObjectTypes(); + + return EntityService.GetSiblings( + target, + siblingObjectTypes, + before, + after, + out totalBefore, + out totalAfter, + ordering: ItemOrdering) + .ToArray(); + } + protected override TItem MapTreeItemViewModel(Guid? parentKey, IEntitySlim entity) { TItem viewModel = base.MapTreeItemViewModel(parentKey, entity); @@ -93,19 +111,19 @@ public abstract class FolderTreeControllerBase : NamedEntityTreeControlle { totalItems = 0; - UmbracoObjectTypes[] childObjectTypes = _foldersOnly ? [FolderObjectType] : [FolderObjectType, ItemObjectType]; + UmbracoObjectTypes[] childObjectTypes = GetObjectTypes(); - IEntitySlim[] itemEntities = EntityService.GetPagedChildren( - parentKey, - [FolderObjectType, ItemObjectType], - childObjectTypes, - skip, - take, - false, - out totalItems, - ordering: ItemOrdering) - .ToArray(); - - return itemEntities; + return EntityService.GetPagedChildren( + parentKey, + [FolderObjectType, ItemObjectType], + childObjectTypes, + skip, + take, + false, + out totalItems, + ordering: ItemOrdering) + .ToArray(); } + + private UmbracoObjectTypes[] GetObjectTypes() => _foldersOnly ? [FolderObjectType] : [FolderObjectType, ItemObjectType]; } diff --git a/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs b/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs index 89bd1ef981..02adf136e5 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs +++ b/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs @@ -190,7 +190,7 @@ public class UserStartNodeEntitiesService : IUserStartNodeEntitiesService if (userStartNodePaths.Any(path => $"{targetParent?.Path},".StartsWith($"{path},"))) { // The requested parent of the target is one of the user start nodes (or a descendant of one), all siblings are by definition allowed. - siblings = _entityService.GetSiblings(targetKey, umbracoObjectType, before, after, out totalBefore, out totalAfter, ordering: ordering).ToArray(); + siblings = _entityService.GetSiblings(targetKey, [umbracoObjectType], before, after, out totalBefore, out totalAfter, ordering: ordering).ToArray(); return ChildUserAccessEntities(siblings, userStartNodePaths); } @@ -206,7 +206,7 @@ public class UserStartNodeEntitiesService : IUserStartNodeEntitiesService // Even though we know the IDs of the allowed sibling entities to fetch, we still use a Query to yield correctly sorted children. IQuery query = _scopeProvider.CreateQuery().Where(x => allowedSiblingIds.Contains(x.Id)); - siblings = _entityService.GetSiblings(targetKey, umbracoObjectType, before, after, out totalBefore, out totalAfter, query, ordering).ToArray(); + siblings = _entityService.GetSiblings(targetKey, [umbracoObjectType], before, after, out totalBefore, out totalAfter, query, ordering).ToArray(); return ChildUserAccessEntities(siblings, userStartNodePaths); } diff --git a/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs index 0ad4e42260..47cf15c3fc 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs @@ -22,7 +22,7 @@ public interface IEntityRepository : IRepository /// /// Gets sibling entities of a specified target entity, within a given range before and after the target, ordered as specified. /// - /// The object type key of the entities. + /// The object type keys of the entities. /// The key of the target entity whose siblings are to be retrieved. /// The number of siblings to retrieve before the target entity. /// The number of siblings to retrieve after the target entity. @@ -32,7 +32,7 @@ public interface IEntityRepository : IRepository /// Outputs the total number of siblings after the target entity. /// Enumerable of sibling entities. IEnumerable GetSiblings( - Guid objectType, + ISet objectTypes, Guid targetKey, int before, int after, diff --git a/src/Umbraco.Core/Services/EntityService.cs b/src/Umbraco.Core/Services/EntityService.cs index 9778e58a32..6ba02cc880 100644 --- a/src/Umbraco.Core/Services/EntityService.cs +++ b/src/Umbraco.Core/Services/EntityService.cs @@ -321,7 +321,7 @@ public class EntityService : RepositoryService, IEntityService /// public IEnumerable GetSiblings( Guid key, - UmbracoObjectTypes objectType, + IEnumerable objectTypes, int before, int after, out long totalBefore, @@ -343,8 +343,10 @@ public class EntityService : RepositoryService, IEntityService using ICoreScope scope = ScopeProvider.CreateCoreScope(); + var objectTypeGuids = objectTypes.Select(x => x.GetGuid()).ToHashSet(); + IEnumerable siblings = _entityRepository.GetSiblings( - objectType.GetGuid(), + objectTypeGuids, key, before, after, diff --git a/src/Umbraco.Core/Services/IEntityService.cs b/src/Umbraco.Core/Services/IEntityService.cs index 768a746b00..1099c4af74 100644 --- a/src/Umbraco.Core/Services/IEntityService.cs +++ b/src/Umbraco.Core/Services/IEntityService.cs @@ -174,7 +174,7 @@ public interface IEntityService /// Gets sibling entities of a specified target entity, within a given range before and after the target, ordered as specified. /// /// The key of the target entity whose siblings are to be retrieved. - /// The object type key of the entities. + /// The object types of the entities. /// The number of siblings to retrieve before the target entity. Needs to be greater or equal to 0. /// The number of siblings to retrieve after the target entity. Needs to be greater or equal to 0. /// An optional filter to apply to the result set. @@ -184,7 +184,7 @@ public interface IEntityService /// Enumerable of sibling entities. IEnumerable GetSiblings( Guid key, - UmbracoObjectTypes objectType, + IEnumerable objectTypes, int before, int after, out long totalBefore, diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs index 86bdd88267..d98657578a 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs @@ -95,10 +95,6 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend ApplyOrdering(ref sql, ordering); } - // TODO: we should be able to do sql = sql.OrderBy(x => Alias(x.NodeId, "NodeId")); but we can't because the OrderBy extension don't support Alias currently - // no matter what we always must have node id ordered at the end - sql = ordering.Direction == Direction.Ascending ? sql.OrderBy("NodeId") : sql.OrderByDescending("NodeId"); - // for content we must query for ContentEntityDto entities to produce the correct culture variant entity names var pageIndexToFetch = pageIndex + 1; IEnumerable dtos; @@ -147,7 +143,7 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend /// public IEnumerable GetSiblings( - Guid objectType, + ISet objectTypes, Guid targetKey, int before, int after, @@ -167,31 +163,29 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend Sql orderingSql = Sql(); ApplyOrdering(ref orderingSql, ordering); - // Get all children of the parent node which is not trashed, ordered by SortOrder, and assign each a row number. + // Get all children of the parent node which are not trashed and match the provided object types. + // Order by SortOrder, and assign each a row number. // These row numbers are important, we need them to select the "before" and "after" siblings of the target node. Sql rowNumberSql = Sql() .Select($"ROW_NUMBER() OVER ({orderingSql.SQL}) AS rn") .AndSelect(n => n.UniqueId) .From() - .Where(x => x.ParentId == parentId && x.Trashed == false); + .Where(x => x.ParentId == parentId && x.Trashed == false) + .WhereIn(x => x.NodeObjectType, objectTypes); - // Apply the filter if provided. Note that in doing this, we'll add more parameters to the query, so need to track - // how many so we can offset the parameter indexes for the "before" and "after" values added later. - int beforeAfterParameterIndexOffset = 0; + // Apply the filter if provided. if (filter != null) { foreach (Tuple filterClause in filter.GetWhereClauses()) { rowNumberSql.Where(filterClause.Item1, filterClause.Item2); - - // We need to offset by one for each non-array parameter in the filter clause. - // If a query is created using Contains or some other set based operation, we'll get both the array and the - // items in the array provided in the where clauses. It's only the latter that count for applying parameters - // to the SQL statement, and hence we should only offset by them. - beforeAfterParameterIndexOffset += filterClause.Item2.Count(x => !x.GetType().IsArray); } } + // By applying additional where clauses with parameters containing an unknown number of elements, the position of the parameters in + // the final query for before and after positions will increase. So we need to calculate the offset based on the provided values. + int beforeAfterParameterIndexOffset = GetBeforeAfterParameterOffset(objectTypes, filter); + // Find the specific row number of the target node. // We need this to determine the bounds of the row numbers to select. Sql targetRowSql = Sql() @@ -226,7 +220,31 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend return []; } - return PerformGetAll(objectType, ordering, sql => sql.WhereIn(x => x.UniqueId, keys)); + // To re-use this method we need to provide a single object type. By convention for folder based trees, we provide the primary object type last. + return PerformGetAll(objectTypes.ToArray(), ordering, sql => sql.WhereIn(x => x.UniqueId, keys)); + } + + private static int GetBeforeAfterParameterOffset(ISet objectTypes, IQuery? filter) + { + int beforeAfterParameterIndexOffset = 0; + + // Increment for each object type. + beforeAfterParameterIndexOffset += objectTypes.Count; + + // Increment for the provided filter. + if (filter != null) + { + foreach (Tuple filterClause in filter.GetWhereClauses()) + { + // We need to offset by one for each non-array parameter in the filter clause. + // If a query is created using Contains or some other set based operation, we'll get both the array and the + // items in the array provided in the where clauses. It's only the latter that count for applying parameters + // to the SQL statement, and hence we should only offset by them. + beforeAfterParameterIndexOffset += filterClause.Item2.Count(x => !x.GetType().IsArray); + } + } + + return beforeAfterParameterIndexOffset; } private long GetNumberOfSiblingsOutsideSiblingRange( @@ -316,16 +334,16 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend } private IEnumerable PerformGetAll( - Guid objectType, + Guid[] objectTypes, Ordering ordering, Action>? filter = null) { - var isContent = objectType == Constants.ObjectTypes.Document || - objectType == Constants.ObjectTypes.DocumentBlueprint; - var isMedia = objectType == Constants.ObjectTypes.Media; - var isMember = objectType == Constants.ObjectTypes.Member; + var isContent = objectTypes.Contains(Constants.ObjectTypes.Document) || + objectTypes.Contains(Constants.ObjectTypes.DocumentBlueprint); + var isMedia = objectTypes.Contains(Constants.ObjectTypes.Media); + var isMember = objectTypes.Contains(Constants.ObjectTypes.Member); - Sql sql = GetFullSqlForEntityType(isContent, isMedia, isMember, objectType, ordering, filter); + Sql sql = GetFullSqlForEntityType(isContent, isMedia, isMember, objectTypes, ordering, filter); return GetEntities(sql, isContent, isMedia, isMember); } @@ -572,8 +590,17 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend Guid objectType, Ordering ordering, Action>? filter) + => GetFullSqlForEntityType(isContent, isMedia, isMember, [objectType], ordering, filter); + + protected Sql GetFullSqlForEntityType( + bool isContent, + bool isMedia, + bool isMember, + Guid[] objectTypes, + Ordering ordering, + Action>? filter) { - Sql sql = GetBaseWhere(isContent, isMedia, isMember, false, filter, new[] { objectType }); + Sql sql = GetBaseWhere(isContent, isMedia, isMember, false, filter, objectTypes); AddGroupBy(isContent, isMedia, isMember, sql, false); ApplyOrdering(ref sql, ordering); @@ -788,6 +815,8 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend Ordering? runner = ordering; + Direction lastDirection = Direction.Ascending; + bool orderingIncludesNodeId = false; do { @@ -799,7 +828,10 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend case "PATH": orderBy = SqlSyntax.GetQuotedColumn(NodeDto.TableName, "path"); break; - + case "NODEID": + orderBy = runner.OrderBy; + orderingIncludesNodeId = true; + break; default: orderBy = runner.OrderBy ?? string.Empty; break; @@ -814,11 +846,25 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend sql.OrderByDescending(orderBy); } + lastDirection = runner.Direction; + runner = runner.Next; } while (runner is not null); - + // If we haven't already included the node Id in the order by clause, order by node Id as well to ensure consistent results + // when the provided sort yields entities with the same value. + if (orderingIncludesNodeId is false) + { + if (lastDirection == Direction.Ascending) + { + sql.OrderBy(x => x.NodeId); + } + else + { + sql.OrderByDescending(x => x.NodeId); + } + } } #endregion diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs index 644e7a0e05..090cc3c1f0 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs @@ -385,9 +385,14 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest Assert.That(total, Is.EqualTo(10)); } - [Test] - public async Task EntityService_Can_Get_Paged_Document_Type_Children() + [TestCase("")] + [TestCase("sortOrder")] + [TestCase("nodeId")] + public async Task EntityService_Can_Get_Paged_Document_Type_Children(string orderBy) { + Ordering? ordering = string.IsNullOrEmpty(orderBy) + ? null + : Ordering.By(orderBy, Direction.Ascending); IEnumerable children = EntityService.GetPagedChildren( _documentTypeRootContainerKey, [UmbracoObjectTypes.DocumentTypeContainer], @@ -395,13 +400,14 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest 0, 10, false, - out long totalRecords); + out long totalRecords, + ordering: ordering); Assert.AreEqual(3, totalRecords); Assert.AreEqual(3, children.Count()); Assert.IsTrue(children.Single(x => x.Key == _documentTypeSubContainer1Key).HasChildren); // Has a single folder as a child. Assert.IsTrue(children.Single(x => x.Key == _documentTypeSubContainer2Key).HasChildren); // Has a single document type as a child. - Assert.IsFalse(children.Single(x => x.Key == _documentType1Key).HasChildren); // Is a document type (has no children). + Assert.IsFalse(children.Single(x => x.Key == _documentType1Key).HasChildren); // Is a document type (has no children). } [Test] @@ -934,11 +940,11 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_ReturnsExpectedSiblings() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); var target = children[1]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, out long totalBefore, out long totalAfter).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 1, 1, out long totalBefore, out long totalAfter).ToArray(); Assert.AreEqual(0, totalBefore); Assert.AreEqual(7, totalAfter); Assert.AreEqual(3, result.Length); @@ -950,13 +956,13 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_SkipsTrashedEntities() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); var trash = children[1]; ContentService.MoveToRecycleBin(trash); var target = children[2]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, out long totalBefore, out long totalAfter).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 1, 1, out long totalBefore, out long totalAfter).ToArray(); Assert.AreEqual(0, totalBefore); Assert.AreEqual(6, totalAfter); Assert.AreEqual(3, result.Length); @@ -969,7 +975,7 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_SkipsFilteredEntities_UsingFilterWithSet() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); // Apply a filter that excludes the child at index 1. We'd expect to not get this, but // get still get one previous sibling, i.e. the entity at index 0. @@ -977,7 +983,7 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest IQuery filter = ScopeProvider.CreateQuery().Where(x => !keysToExclude.Contains(x.Key)); var target = children[2]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, out long totalBefore, out long totalAfter, filter).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 1, 1, out long totalBefore, out long totalAfter, filter).ToArray(); Assert.AreEqual(0, totalBefore); Assert.AreEqual(6, totalAfter); Assert.AreEqual(3, result.Length); @@ -990,7 +996,7 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_SkipsFilteredEntities_UsingFilterWithoutSet() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); // Apply a filter that excludes the child at index 1. We'd expect to not get this, but // get still get one previous sibling, i.e. the entity at index 0. @@ -998,7 +1004,7 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest IQuery filter = ScopeProvider.CreateQuery().Where(x => x.Key != keyToExclude); var target = children[2]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, out long totalBefore, out long totalAfter, filter).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 1, 1, out long totalBefore, out long totalAfter, filter).ToArray(); Assert.AreEqual(0, totalBefore); Assert.AreEqual(6, totalAfter); Assert.AreEqual(3, result.Length); @@ -1011,13 +1017,13 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_RespectsOrdering() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); // Order the children by name to ensure the ordering works when differing from the default sort order, the name is a GUID. children = children.OrderBy(x => x.Name).ToList(); var target = children[1]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, out long totalBefore, out long totalAfter, ordering: Ordering.By(nameof(NodeDto.Text))).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 1, 1, out long totalBefore, out long totalAfter, ordering: Ordering.By(nameof(NodeDto.Text))).ToArray(); Assert.AreEqual(0, totalBefore); Assert.AreEqual(7, totalAfter); Assert.AreEqual(3, result.Length); @@ -1029,10 +1035,10 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_IgnoresOutOfBoundsLower() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); var target = children[1]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 100, 1, out long totalBefore, out long totalAfter).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 100, 1, out long totalBefore, out long totalAfter).ToArray(); Assert.AreEqual(0, totalBefore); Assert.AreEqual(7, totalAfter); Assert.AreEqual(3, result.Length); @@ -1044,10 +1050,10 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest [Test] public void EntityService_Siblings_IgnoresOutOfBoundsUpper() { - var children = CreateSiblingsTestData(); + var children = CreateDocumentSiblingsTestData(); var target = children[^2]; - var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 100, out long totalBefore, out long totalAfter).ToArray(); + var result = EntityService.GetSiblings(target.Key, [UmbracoObjectTypes.Document], 1, 100, out long totalBefore, out long totalAfter).ToArray(); Assert.AreEqual(7, totalBefore); Assert.AreEqual(0, totalAfter); Assert.AreEqual(3, result.Length); @@ -1056,7 +1062,33 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest Assert.IsTrue(result[^3].Key == children[^3].Key); } - private List CreateSiblingsTestData() + [TestCase(true)] + [TestCase(false)] + public async Task EntityService_Siblings_FiltersByObjectTypes(bool foldersOnly) + { + // For testing these scenarios we can use the data setup in CreateStructureForPagedDocumentTypeChildrenTest. + + var objectTypes = new List { UmbracoObjectTypes.DocumentTypeContainer }; + if (foldersOnly is false) + { + objectTypes.Add(UmbracoObjectTypes.DocumentType); + } + + var result = EntityService.GetSiblings(_documentTypeSubContainer2Key, objectTypes, 1, 1, out long totalBefore, out long totalAfter).ToArray(); + Assert.AreEqual(0, totalBefore); + Assert.AreEqual(0, totalAfter); + + var expectedCount = foldersOnly ? 2 : 3; + Assert.AreEqual(expectedCount, result.Length); + Assert.IsTrue(result[0].Key == _documentTypeSubContainer1Key); + Assert.IsTrue(result[1].Key == _documentTypeSubContainer2Key); + if (foldersOnly is false) + { + Assert.IsTrue(result[2].Key == _documentType1Key); + } + } + + private List CreateDocumentSiblingsTestData() { var contentType = ContentTypeService.Get("umbTextpage"); diff --git a/tests/Umbraco.Tests.Integration/appsettings.Tests.json b/tests/Umbraco.Tests.Integration/appsettings.Tests.json index 1d4bbf18ef..7963824674 100644 --- a/tests/Umbraco.Tests.Integration/appsettings.Tests.json +++ b/tests/Umbraco.Tests.Integration/appsettings.Tests.json @@ -11,7 +11,7 @@ }, "Tests": { "Database": { - "DatabaseType": "SQLite", + "DatabaseType": "SQLite", // "SQLite", "LocalDb" "PrepareThreadCount": 4, "SchemaDatabaseCount": 4, "EmptyDatabasesCount": 2, diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/EntityServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/EntityServiceTests.cs index 0e8371cb5d..8032d185af 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/EntityServiceTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/EntityServiceTests.cs @@ -23,7 +23,7 @@ public class EntityServiceTests if (shouldThrow) { - Assert.Throws(() => sut.GetSiblings(Guid.NewGuid(), UmbracoObjectTypes.Document, before, after, out _, out _)); + Assert.Throws(() => sut.GetSiblings(Guid.NewGuid(), [UmbracoObjectTypes.Document], before, after, out _, out _)); } }