Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -452,25 +452,38 @@
return AddGroupBy(isContent, isMedia, isMember, sql, true);
}

protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember, Action<Sql<ISqlContext>>? 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<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember, Action<Sql<ISqlContext>>? filter,
bool isCount = false)
protected Sql<ISqlContext> GetBase(bool isContent, bool isMedia, bool isMember, Action<Sql<ISqlContext>>? filter, Guid[] objectTypes, bool isCount = false)
{
Sql<ISqlContext> sql = Sql();

if (isCount)
{
sql.SelectCount();
}
else
{
sql
.Select<NodeDto>(x => x.NodeId, x => x.Trashed, x => x.ParentId, x => x.UserId, x => x.Level,
x => x.Path)
.AndSelect<NodeDto>(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");
}

Check warning on line 486 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ Getting worse: Complex Method

GetBase increases in cyclomatic complexity from 14 to 15, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

if (isContent || isMedia || isMember)
{
Expand Down Expand Up @@ -545,7 +558,7 @@
protected Sql<ISqlContext> GetBaseWhere(bool isContent, bool isMedia, bool isMember, bool isCount,
Action<Sql<ISqlContext>>? filter, Guid[] objectTypes)
{
Sql<ISqlContext> sql = GetBase(isContent, isMedia, isMember, filter, isCount);
Sql<ISqlContext> sql = GetBase(isContent, isMedia, isMember, filter, objectTypes, isCount);
if (objectTypes.Length > 0)
{
sql.WhereIn<NodeDto>(x => x.NodeObjectType, objectTypes);
Expand Down
7 changes: 7 additions & 0 deletions tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.EntityServiceTests.CreateTestData</Target>
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.TemplateServiceTests.Deleting_Master_Template_Also_Deletes_Children</Target>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
// Copyright (c) Umbraco.

Check warning on line 1 in tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ Getting worse: Code Duplication

introduced similar code in: EntityService_Can_Get_Paged_Document_Type_Children,EntityService_Can_Get_Paged_Document_Type_Children_For_Folders_Only. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.
// 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;

Expand All @@ -35,7 +34,7 @@
await LanguageService.CreateAsync(_langEs, Constants.Security.SuperUserKey);
}

CreateTestData();
await CreateTestData();
}

private Language? _langFr;
Expand All @@ -57,6 +56,10 @@

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

private IContentTypeContainerService ContentTypeContainerService => GetRequiredService<IContentTypeContainerService>();

public IContentTypeEditingService ContentTypeEditingService => GetRequiredService<IContentTypeEditingService>();

[Test]
public void EntityService_Can_Get_Paged_Descendants_Ordering_Path()
{
Expand Down Expand Up @@ -381,6 +384,43 @@
Assert.That(total, Is.EqualTo(10));
}

[Test]
public async Task EntityService_Can_Get_Paged_Document_Type_Children()
{
IEnumerable<IEntitySlim> 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<IEntitySlim> 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()
Expand Down Expand Up @@ -738,7 +778,7 @@
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]
Expand All @@ -748,7 +788,7 @@
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]
Expand All @@ -757,7 +797,7 @@
var entities = EntityService.GetAll<IContentType>().ToArray();

Assert.That(entities.Any(), Is.True);
Assert.That(entities.Count(), Is.EqualTo(1));
Assert.That(entities.Count(), Is.EqualTo(3));
}

[Test]
Expand Down Expand Up @@ -885,7 +925,7 @@
private Media _subfolder;
private Media _subfolder2;

public void CreateTestData()
public async Task CreateTestData()
{
if (_isSetup == false)
{
Expand Down Expand Up @@ -942,6 +982,38 @@
// 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);
}
}