Skip to content

Commit 7d41791

Browse files
AndyButlandMigaroez
AndyButland
and
Migaroez
authored
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 <[email protected]>
1 parent f3658bf commit 7d41791

File tree

3 files changed

+105
-13
lines changed

3 files changed

+105
-13
lines changed

‎src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -452,10 +452,12 @@ protected Sql<ISqlContext> GetFullSqlForEntityType(bool isContent, bool isMedia,
452452
return AddGroupBy(isContent, isMedia, isMember, sql, true);
453453
}
454454

455+
protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember, Action<Sql<ISqlContext>>? filter, bool isCount = false)
456+
=> GetBase(isContent, isMedia, isMember, filter, [], isCount);
457+
455458
// gets the base SELECT + FROM [+ filter] sql
456459
// always from the 'current' content version
457-
protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember, Action<Sql<ISqlContext>>? filter,
458-
bool isCount = false)
460+
protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember, Action<Sql<ISqlContext>>? filter, Guid[] objectTypes, bool isCount = false)
459461
{
460462
Sql<ISqlContext> sql = Sql();
461463

@@ -469,8 +471,19 @@ protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember,
469471
.Select<NodeDto>(x => x.NodeId, x => x.Trashed, x => x.ParentId, x => x.UserId, x => x.Level,
470472
x => x.Path)
471473
.AndSelect<NodeDto>(x => x.SortOrder, x => x.UniqueId, x => x.Text, x => x.NodeObjectType,
472-
x => x.CreateDate)
473-
.Append(", COUNT(child.id) AS children");
474+
x => x.CreateDate);
475+
476+
if (objectTypes.Length == 0)
477+
{
478+
sql.Append(", COUNT(child.id) AS children");
479+
}
480+
else
481+
{
482+
// The following is safe from SQL injection as we are dealing with GUIDs, not strings.
483+
// Upper-case is necessary for SQLite, and also works for SQL Server.
484+
var objectTypesForInClause = string.Join("','", objectTypes.Select(x => x.ToString().ToUpperInvariant()));
485+
sql.Append($", SUM(CASE WHEN child.nodeObjectType IN ('{objectTypesForInClause}') THEN 1 ELSE 0 END) AS children");
486+
}
474487

475488
if (isContent || isMedia || isMember)
476489
{
@@ -545,7 +558,7 @@ protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember,
545558
protected Sql<ISqlContext> GetBaseWhere(bool isContent, bool isMedia, bool isMember, bool isCount,
546559
Action<Sql<ISqlContext>>? filter, Guid[] objectTypes)
547560
{
548-
Sql<ISqlContext> sql = GetBase(isContent, isMedia, isMember, filter, isCount);
561+
Sql<ISqlContext> sql = GetBase(isContent, isMedia, isMember, filter, objectTypes, isCount);
549562
if (objectTypes.Length > 0)
550563
{
551564
sql.WhereIn<NodeDto>(x => x.NodeObjectType, objectTypes);

‎tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@
106106
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
107107
<IsBaselineSuppression>true</IsBaselineSuppression>
108108
</Suppression>
109+
<Suppression>
110+
<DiagnosticId>CP0002</DiagnosticId>
111+
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.EntityServiceTests.CreateTestData</Target>
112+
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
113+
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
114+
<IsBaselineSuppression>true</IsBaselineSuppression>
115+
</Suppression>
109116
<Suppression>
110117
<DiagnosticId>CP0002</DiagnosticId>
111118
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.TemplateServiceTests.Deleting_Master_Template_Also_Deletes_Children</Target>

‎tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs‎

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
// Copyright (c) Umbraco.
22
// See LICENSE for more details.
33

4-
using System.Collections.Generic;
5-
using System.Linq;
64
using NUnit.Framework;
75
using Umbraco.Cms.Core;
86
using Umbraco.Cms.Core.Models;
97
using Umbraco.Cms.Core.Models.Entities;
108
using Umbraco.Cms.Core.Services;
9+
using Umbraco.Cms.Core.Services.ContentTypeEditing;
1110
using Umbraco.Cms.Infrastructure.Persistence;
11+
using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
1212
using Umbraco.Cms.Tests.Common.Attributes;
1313
using Umbraco.Cms.Tests.Common.Builders;
1414
using Umbraco.Cms.Tests.Common.Testing;
1515
using Umbraco.Cms.Tests.Integration.Testing;
16-
using Umbraco.Extensions;
1716

1817
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
1918

@@ -35,7 +34,7 @@ public async Task SetupTestData()
3534
await LanguageService.CreateAsync(_langEs, Constants.Security.SuperUserKey);
3635
}
3736

38-
CreateTestData();
37+
await CreateTestData();
3938
}
4039

4140
private Language? _langFr;
@@ -57,6 +56,10 @@ public async Task SetupTestData()
5756

5857
private IFileService FileService => GetRequiredService<IFileService>();
5958

59+
private IContentTypeContainerService ContentTypeContainerService => GetRequiredService<IContentTypeContainerService>();
60+
61+
public IContentTypeEditingService ContentTypeEditingService => GetRequiredService<IContentTypeEditingService>();
62+
6063
[Test]
6164
public void EntityService_Can_Get_Paged_Descendants_Ordering_Path()
6265
{
@@ -381,6 +384,43 @@ public void EntityService_Can_Get_Paged_Media_Children()
381384
Assert.That(total, Is.EqualTo(10));
382385
}
383386

387+
[Test]
388+
public async Task EntityService_Can_Get_Paged_Document_Type_Children()
389+
{
390+
IEnumerable<IEntitySlim> children = EntityService.GetPagedChildren(
391+
_documentTypeRootContainerKey,
392+
[UmbracoObjectTypes.DocumentTypeContainer],
393+
[UmbracoObjectTypes.DocumentTypeContainer, UmbracoObjectTypes.DocumentType],
394+
0,
395+
10,
396+
false,
397+
out long totalRecords);
398+
399+
Assert.AreEqual(3, totalRecords);
400+
Assert.AreEqual(3, children.Count());
401+
Assert.IsTrue(children.Single(x => x.Key == _documentTypeSubContainer1Key).HasChildren); // Has a single folder as a child.
402+
Assert.IsTrue(children.Single(x => x.Key == _documentTypeSubContainer2Key).HasChildren); // Has a single document type as a child.
403+
Assert.IsFalse(children.Single(x => x.Key == _documentType1Key).HasChildren); // Is a document type (has no children).
404+
}
405+
406+
[Test]
407+
public async Task EntityService_Can_Get_Paged_Document_Type_Children_For_Folders_Only()
408+
{
409+
IEnumerable<IEntitySlim> children = EntityService.GetPagedChildren(
410+
_documentTypeRootContainerKey,
411+
[UmbracoObjectTypes.DocumentTypeContainer],
412+
[UmbracoObjectTypes.DocumentTypeContainer],
413+
0,
414+
10,
415+
false,
416+
out long totalRecords);
417+
418+
Assert.AreEqual(2, totalRecords);
419+
Assert.AreEqual(2, children.Count());
420+
Assert.IsTrue(children.Single(x => x.Key == _documentTypeSubContainer1Key).HasChildren); // Has a single folder as a child.
421+
Assert.IsFalse(children.Single(x => x.Key == _documentTypeSubContainer2Key).HasChildren); // Has a single document type as a child.
422+
}
423+
384424
[Test]
385425
[LongRunning]
386426
public void EntityService_Can_Get_Paged_Media_Descendants()
@@ -738,7 +778,7 @@ public void EntityService_Can_Find_All_ContentTypes_By_UmbracoObjectTypes()
738778
var entities = EntityService.GetAll(UmbracoObjectTypes.DocumentType).ToArray();
739779

740780
Assert.That(entities.Any(), Is.True);
741-
Assert.That(entities.Count(), Is.EqualTo(1));
781+
Assert.That(entities.Count(), Is.EqualTo(3));
742782
}
743783

744784
[Test]
@@ -748,7 +788,7 @@ public void EntityService_Can_Find_All_ContentTypes_By_UmbracoObjectType_Id()
748788
var entities = EntityService.GetAll(objectTypeId).ToArray();
749789

750790
Assert.That(entities.Any(), Is.True);
751-
Assert.That(entities.Count(), Is.EqualTo(1));
791+
Assert.That(entities.Count(), Is.EqualTo(3));
752792
}
753793

754794
[Test]
@@ -757,7 +797,7 @@ public void EntityService_Can_Find_All_ContentTypes_By_Type()
757797
var entities = EntityService.GetAll<IContentType>().ToArray();
758798

759799
Assert.That(entities.Any(), Is.True);
760-
Assert.That(entities.Count(), Is.EqualTo(1));
800+
Assert.That(entities.Count(), Is.EqualTo(3));
761801
}
762802

763803
[Test]
@@ -885,7 +925,7 @@ public void ReserveId()
885925
private Media _subfolder;
886926
private Media _subfolder2;
887927

888-
public void CreateTestData()
928+
public async Task CreateTestData()
889929
{
890930
if (_isSetup == false)
891931
{
@@ -942,6 +982,38 @@ public void CreateTestData()
942982
// Create and save sub folder -> 1061
943983
_subfolder2 = MediaBuilder.CreateMediaFolder(_folderMediaType, _subfolder.Id);
944984
MediaService.Save(_subfolder2, -1);
985+
986+
// Setup document type folder structure for tests on paged children with or without folders
987+
await CreateStructureForPagedDocumentTypeChildrenTest();
945988
}
946989
}
990+
991+
private static readonly Guid _documentTypeRootContainerKey = Guid.NewGuid();
992+
private static readonly Guid _documentTypeSubContainer1Key = Guid.NewGuid();
993+
private static readonly Guid _documentTypeSubContainer2Key = Guid.NewGuid();
994+
private static readonly Guid _documentType1Key = Guid.NewGuid();
995+
996+
private async Task CreateStructureForPagedDocumentTypeChildrenTest()
997+
{
998+
// Structure created:
999+
// - root container
1000+
// - sub container 1
1001+
// - sub container 1b
1002+
// - sub container 2
1003+
// - doc type 2
1004+
// - doc type 1
1005+
await ContentTypeContainerService.CreateAsync(_documentTypeRootContainerKey, "Root Container", null, Constants.Security.SuperUserKey);
1006+
await ContentTypeContainerService.CreateAsync(_documentTypeSubContainer1Key, "Sub Container 1", _documentTypeRootContainerKey, Constants.Security.SuperUserKey);
1007+
await ContentTypeContainerService.CreateAsync(_documentTypeSubContainer2Key, "Sub Container 2", _documentTypeRootContainerKey, Constants.Security.SuperUserKey);
1008+
await ContentTypeContainerService.CreateAsync(Guid.NewGuid(), "Sub Container 1b", _documentTypeSubContainer1Key, Constants.Security.SuperUserKey);
1009+
1010+
var docType1Model = ContentTypeEditingBuilder.CreateBasicContentType("umbDocType1", "Doc Type 1");
1011+
docType1Model.ContainerKey = _documentTypeRootContainerKey;
1012+
docType1Model.Key = _documentType1Key;
1013+
await ContentTypeEditingService.CreateAsync(docType1Model, Constants.Security.SuperUserKey);
1014+
1015+
var docType2Model = ContentTypeEditingBuilder.CreateBasicContentType("umbDocType2", "Doc Type 2");
1016+
docType2Model.ContainerKey = _documentTypeSubContainer2Key;
1017+
await ContentTypeEditingService.CreateAsync(docType2Model, Constants.Security.SuperUserKey);
1018+
}
9471019
}

0 commit comments

Comments
 (0)