From 7d4179154348fbbd156302ff5f59bbc92e91cd0f Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 3 Apr 2025 21:55:34 +0200 Subject: [PATCH] Ensure has children reflects only items with folder children when folders only are queried. (#18790) * Ensure has children reflects only items with folder children when folders only are queried. * Added supression for change to integration test public code. --------- Co-authored-by: Migaroez --- .../Implement/EntityRepository.cs | 23 +++-- .../CompatibilitySuppressions.xml | 7 ++ .../Services/EntityServiceTests.cs | 88 +++++++++++++++++-- 3 files changed, 105 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs index fdaadb3027..8ff1fe381b 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs @@ -452,10 +452,12 @@ internal class EntityRepository : RepositoryBase, IEntityRepositoryExtended return AddGroupBy(isContent, isMedia, isMember, sql, true); } + protected Sql GetBase(bool isContent, bool isMedia, bool isMember, Action>? filter, bool isCount = false) + => GetBase(isContent, isMedia, isMember, filter, [], isCount); + // gets the base SELECT + FROM [+ filter] sql // always from the 'current' content version - protected Sql GetBase(bool isContent, bool isMedia, bool isMember, Action>? filter, - bool isCount = false) + protected Sql GetBase(bool isContent, bool isMedia, bool isMember, Action>? filter, Guid[] objectTypes, bool isCount = false) { Sql sql = Sql(); @@ -469,8 +471,19 @@ internal class EntityRepository : RepositoryBase, IEntityRepositoryExtended .Select(x => x.NodeId, x => x.Trashed, x => x.ParentId, x => x.UserId, x => x.Level, x => x.Path) .AndSelect(x => x.SortOrder, x => x.UniqueId, x => x.Text, x => x.NodeObjectType, - x => x.CreateDate) - .Append(", COUNT(child.id) AS children"); + x => x.CreateDate); + + if (objectTypes.Length == 0) + { + sql.Append(", COUNT(child.id) AS children"); + } + else + { + // The following is safe from SQL injection as we are dealing with GUIDs, not strings. + // Upper-case is necessary for SQLite, and also works for SQL Server. + var objectTypesForInClause = string.Join("','", objectTypes.Select(x => x.ToString().ToUpperInvariant())); + sql.Append($", SUM(CASE WHEN child.nodeObjectType IN ('{objectTypesForInClause}') THEN 1 ELSE 0 END) AS children"); + } if (isContent || isMedia || isMember) { @@ -545,7 +558,7 @@ internal class EntityRepository : RepositoryBase, IEntityRepositoryExtended protected Sql GetBaseWhere(bool isContent, bool isMedia, bool isMember, bool isCount, Action>? filter, Guid[] objectTypes) { - Sql sql = GetBase(isContent, isMedia, isMember, filter, isCount); + Sql sql = GetBase(isContent, isMedia, isMember, filter, objectTypes, isCount); if (objectTypes.Length > 0) { sql.WhereIn(x => x.NodeObjectType, objectTypes); diff --git a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml index 9a446cdb11..880504a1d4 100644 --- a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml +++ b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml @@ -106,6 +106,13 @@ lib/net9.0/Umbraco.Tests.Integration.dll true + + CP0002 + M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.EntityServiceTests.CreateTestData + lib/net9.0/Umbraco.Tests.Integration.dll + lib/net9.0/Umbraco.Tests.Integration.dll + true + CP0002 M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.TemplateServiceTests.Deleting_Master_Template_Also_Deletes_Children diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs index d18caf52b4..20df8e690a 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs @@ -1,19 +1,18 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Collections.Generic; -using System.Linq; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Entities; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.ContentTypeEditing; using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement; using Umbraco.Cms.Tests.Common.Attributes; using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; -using Umbraco.Extensions; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; @@ -35,7 +34,7 @@ public class EntityServiceTests : UmbracoIntegrationTest await LanguageService.CreateAsync(_langEs, Constants.Security.SuperUserKey); } - CreateTestData(); + await CreateTestData(); } private Language? _langFr; @@ -57,6 +56,10 @@ public class EntityServiceTests : UmbracoIntegrationTest private IFileService FileService => GetRequiredService(); + private IContentTypeContainerService ContentTypeContainerService => GetRequiredService(); + + public IContentTypeEditingService ContentTypeEditingService => GetRequiredService(); + [Test] public void EntityService_Can_Get_Paged_Descendants_Ordering_Path() { @@ -381,6 +384,43 @@ public class EntityServiceTests : UmbracoIntegrationTest Assert.That(total, Is.EqualTo(10)); } + [Test] + public async Task EntityService_Can_Get_Paged_Document_Type_Children() + { + IEnumerable children = EntityService.GetPagedChildren( + _documentTypeRootContainerKey, + [UmbracoObjectTypes.DocumentTypeContainer], + [UmbracoObjectTypes.DocumentTypeContainer, UmbracoObjectTypes.DocumentType], + 0, + 10, + false, + out long totalRecords); + + 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). + } + + [Test] + public async Task EntityService_Can_Get_Paged_Document_Type_Children_For_Folders_Only() + { + IEnumerable children = EntityService.GetPagedChildren( + _documentTypeRootContainerKey, + [UmbracoObjectTypes.DocumentTypeContainer], + [UmbracoObjectTypes.DocumentTypeContainer], + 0, + 10, + false, + out long totalRecords); + + Assert.AreEqual(2, totalRecords); + Assert.AreEqual(2, children.Count()); + Assert.IsTrue(children.Single(x => x.Key == _documentTypeSubContainer1Key).HasChildren); // Has a single folder as a child. + Assert.IsFalse(children.Single(x => x.Key == _documentTypeSubContainer2Key).HasChildren); // Has a single document type as a child. + } + [Test] [LongRunning] public void EntityService_Can_Get_Paged_Media_Descendants() @@ -738,7 +778,7 @@ public class EntityServiceTests : UmbracoIntegrationTest var entities = EntityService.GetAll(UmbracoObjectTypes.DocumentType).ToArray(); Assert.That(entities.Any(), Is.True); - Assert.That(entities.Count(), Is.EqualTo(1)); + Assert.That(entities.Count(), Is.EqualTo(3)); } [Test] @@ -748,7 +788,7 @@ public class EntityServiceTests : UmbracoIntegrationTest var entities = EntityService.GetAll(objectTypeId).ToArray(); Assert.That(entities.Any(), Is.True); - Assert.That(entities.Count(), Is.EqualTo(1)); + Assert.That(entities.Count(), Is.EqualTo(3)); } [Test] @@ -757,7 +797,7 @@ public class EntityServiceTests : UmbracoIntegrationTest var entities = EntityService.GetAll().ToArray(); Assert.That(entities.Any(), Is.True); - Assert.That(entities.Count(), Is.EqualTo(1)); + Assert.That(entities.Count(), Is.EqualTo(3)); } [Test] @@ -885,7 +925,7 @@ public class EntityServiceTests : UmbracoIntegrationTest private Media _subfolder; private Media _subfolder2; - public void CreateTestData() + public async Task CreateTestData() { if (_isSetup == false) { @@ -942,6 +982,38 @@ public class EntityServiceTests : UmbracoIntegrationTest // Create and save sub folder -> 1061 _subfolder2 = MediaBuilder.CreateMediaFolder(_folderMediaType, _subfolder.Id); MediaService.Save(_subfolder2, -1); + + // Setup document type folder structure for tests on paged children with or without folders + await CreateStructureForPagedDocumentTypeChildrenTest(); } } + + private static readonly Guid _documentTypeRootContainerKey = Guid.NewGuid(); + private static readonly Guid _documentTypeSubContainer1Key = Guid.NewGuid(); + private static readonly Guid _documentTypeSubContainer2Key = Guid.NewGuid(); + private static readonly Guid _documentType1Key = Guid.NewGuid(); + + private async Task CreateStructureForPagedDocumentTypeChildrenTest() + { + // Structure created: + // - root container + // - sub container 1 + // - sub container 1b + // - sub container 2 + // - doc type 2 + // - doc type 1 + await ContentTypeContainerService.CreateAsync(_documentTypeRootContainerKey, "Root Container", null, Constants.Security.SuperUserKey); + await ContentTypeContainerService.CreateAsync(_documentTypeSubContainer1Key, "Sub Container 1", _documentTypeRootContainerKey, Constants.Security.SuperUserKey); + await ContentTypeContainerService.CreateAsync(_documentTypeSubContainer2Key, "Sub Container 2", _documentTypeRootContainerKey, Constants.Security.SuperUserKey); + await ContentTypeContainerService.CreateAsync(Guid.NewGuid(), "Sub Container 1b", _documentTypeSubContainer1Key, Constants.Security.SuperUserKey); + + var docType1Model = ContentTypeEditingBuilder.CreateBasicContentType("umbDocType1", "Doc Type 1"); + docType1Model.ContainerKey = _documentTypeRootContainerKey; + docType1Model.Key = _documentType1Key; + await ContentTypeEditingService.CreateAsync(docType1Model, Constants.Security.SuperUserKey); + + var docType2Model = ContentTypeEditingBuilder.CreateBasicContentType("umbDocType2", "Doc Type 2"); + docType2Model.ContainerKey = _documentTypeSubContainer2Key; + await ContentTypeEditingService.CreateAsync(docType2Model, Constants.Security.SuperUserKey); + } }