Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize tracked references #16996

Merged
merged 7 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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 @@ -3,6 +3,7 @@
using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Api.Common.ViewModels.Pagination;
using Umbraco.Cms.Api.Management.ViewModels;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services;
Expand All @@ -22,7 +23,7 @@ public AreReferencedDocumentController(ITrackedReferencesService trackedReferenc
}

/// <summary>
/// Gets a page list of the items used in any kind of relation from selected keys.
/// Gets a paged list of the items used in any kind of relation from selected keys.
/// </summary>
/// <remarks>
/// Used when bulk deleting content/media and bulk unpublishing content (delete and unpublish on List view).
Expand All @@ -37,11 +38,11 @@ public async Task<ActionResult<PagedViewModel<ReferenceByIdModel>>> GetPagedRefe
int skip = 0,
int take = 20)
{
PagedModel<RelationItemModel> distinctByKeyItemsWithReferencedRelations = await _trackedReferencesSkipTakeService.GetPagedItemsWithRelationsAsync(ids, skip, take, true);
PagedModel<Guid> distinctByKeyItemsWithReferencedRelations = await _trackedReferencesSkipTakeService.GetPagedKeysWithDependentReferencesAsync(ids, Constants.ObjectTypes.Document, skip, take);
Migaroez marked this conversation as resolved.
Show resolved Hide resolved
var pagedViewModel = new PagedViewModel<ReferenceByIdModel>
{
Total = distinctByKeyItemsWithReferencedRelations.Total,
Items = _umbracoMapper.MapEnumerable<RelationItemModel, ReferenceByIdModel>(distinctByKeyItemsWithReferencedRelations.Items),
Items = _umbracoMapper.MapEnumerable<Guid, ReferenceByIdModel>(distinctByKeyItemsWithReferencedRelations.Items),
};

return await Task.FromResult(pagedViewModel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public ReferencedByDocumentController(ITrackedReferencesService trackedReference
}

/// <summary>
/// Gets a page list of tracked references for the current item, so you can see where an item is being used.
/// Gets a paged list of tracked references for the current item, so you can see where an item is being used.
/// </summary>
/// <remarks>
/// Used by info tabs on content, media etc. and for the delete and unpublish of single items.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public ReferencedDescendantsDocumentController(ITrackedReferencesService tracked
}

/// <summary>
/// Gets a page list of the child nodes of the current item used in any kind of relation.
/// Gets a paged list of the descendant nodes of the current item used in any kind of relation.
/// </summary>
/// <remarks>
/// Used when deleting and unpublishing a single item to check if this item has any descending items that are in any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Api.Common.ViewModels.Pagination;
using Umbraco.Cms.Api.Management.ViewModels;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services;
Expand Down Expand Up @@ -38,11 +39,11 @@ public async Task<ActionResult<PagedViewModel<ReferenceByIdModel>>> GetPagedRefe
int skip = 0,
int take = 20)
{
PagedModel<RelationItemModel> distinctByKeyItemsWithReferencedRelations = await _trackedReferencesSkipTakeService.GetPagedItemsWithRelationsAsync(ids, skip, take, true);
PagedModel<Guid> distinctByKeyItemsWithReferencedRelations = await _trackedReferencesSkipTakeService.GetPagedKeysWithDependentReferencesAsync(ids, Constants.ObjectTypes.Media, skip, take);
var pagedViewModel = new PagedViewModel<ReferenceByIdModel>
{
Total = distinctByKeyItemsWithReferencedRelations.Total,
Items = _umbracoMapper.MapEnumerable<RelationItemModel, ReferenceByIdModel>(distinctByKeyItemsWithReferencedRelations.Items),
Items = _umbracoMapper.MapEnumerable<Guid, ReferenceByIdModel>(distinctByKeyItemsWithReferencedRelations.Items),
};

return await Task.FromResult(pagedViewModel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public void DefineMaps(IUmbracoMapper mapper)
mapper.Define<RelationItemModel, MediaReferenceResponseModel>((source, context) => new MediaReferenceResponseModel(), Map);
mapper.Define<RelationItemModel, DefaultReferenceResponseModel>((source, context) => new DefaultReferenceResponseModel(), Map);
mapper.Define<RelationItemModel, ReferenceByIdModel>((source, context) => new ReferenceByIdModel(), Map);
mapper.Define<Guid, ReferenceByIdModel>((source, context) => new ReferenceByIdModel(), Map);
}

// Umbraco.Code.MapAll
Expand Down Expand Up @@ -56,4 +57,10 @@ private void Map(RelationItemModel source, ReferenceByIdModel target, MapperCont
{
target.Id = source.NodeKey;
}

// Umbraco.Code.MapAll
private void Map(Guid source, ReferenceByIdModel target, MapperContext context)
{
target.Id = source;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,14 @@ IEnumerable<RelationItemModel> GetPagedDescendantsInReferences(
long take,
bool filterMustBeIsDependency,
out long totalRecords);

Task<PagedModel<Guid>> GetPagedNodeKeysWithDependantReferencesAsync(
Migaroez marked this conversation as resolved.
Show resolved Hide resolved
ISet<Guid> keys,
Guid nodeObjectTypeId,
long skip,
long take)
{
IEnumerable<RelationItemModel> pagedItems = GetPagedItemsWithRelations(keys, skip, take, true, out var total);
return Task.FromResult(new PagedModel<Guid>(total, pagedItems.Select(i => i.NodeKey)));
}
}
6 changes: 6 additions & 0 deletions src/Umbraco.Core/Services/ITrackedReferencesService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,10 @@ PagedModel<RelationItemModel> GetPagedItemsWithRelations(int[] ids, long skip, l
/// <returns>A paged result of <see cref="RelationItemModel" /> objects.</returns>
Task<PagedModel<RelationItemModel>> GetPagedItemsWithRelationsAsync(ISet<Guid> keys, long skip, long take,
bool filterMustBeIsDependency);

Task<PagedModel<Guid>> GetPagedKeysWithDependentReferencesAsync(ISet<Guid> keys, Guid nodeObjectTypeId, long skip, long take)
{
PagedModel<RelationItemModel> pagedItems = GetPagedItemsWithRelationsAsync(keys, skip, take, true).GetAwaiter().GetResult();
return Task.FromResult(new PagedModel<Guid>(pagedItems.Total, pagedItems.Items.Select(i => i.NodeKey)));
}
}
6 changes: 6 additions & 0 deletions src/Umbraco.Core/Services/TrackedReferencesService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.Extensions.DependencyInjection;

Check notice on line 1 in src/Umbraco.Core/Services/TrackedReferencesService.cs

View check run for this annotation

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

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 85.37% to 84.44%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Persistence.Repositories;
Expand Down Expand Up @@ -141,4 +141,10 @@

return await Task.FromResult(pagedModel);
}

public async Task<PagedModel<Guid>> GetPagedKeysWithDependentReferencesAsync(ISet<Guid> keys, Guid objectTypeId, long skip, long take)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
return await _trackedReferencesRepository.GetPagedNodeKeysWithDependantReferencesAsync(keys, objectTypeId, skip, take);
}
}
18 changes: 18 additions & 0 deletions src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,30 @@
/// <param name="alias2">An optional alias for Dto 2 table.</param>
/// <param name="alias3">An optional alias for Dto 3 table.</param>
/// <returns>The Sql statement.</returns>
public static Sql<ISqlContext> Where<TDto1, TDto2, TDto3>(this Sql<ISqlContext> sql, Expression<Func<TDto1, TDto2, TDto3, bool>> predicate, string? alias1 = null, string? alias2 = null, string? alias3 = null)
{
var (s, a) = sql.SqlContext.VisitDto(predicate, alias1, alias2, alias3);
return sql.Where(s, a);
}

Check notice on line 64 in src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs

View check run for this annotation

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

ℹ New issue: Excess Number of Function Arguments

Where has 5 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.

/// <summary>
/// Appends a WHERE clause to the Sql statement.
/// </summary>
/// <typeparam name="TDto1">The type of Dto 1.</typeparam>
/// <typeparam name="TDto2">The type of Dto 2.</typeparam>
/// <typeparam name="TDto3">The type of Dto 3.</typeparam>
/// <param name="sql">The Sql statement.</param>
/// <param name="predicate">A predicate to transform and append to the Sql statement.</param>
/// <param name="alias1">An optional alias for Dto 1 table.</param>
/// <param name="alias2">An optional alias for Dto 2 table.</param>
/// <param name="alias3">An optional alias for Dto 3 table.</param>
/// <returns>The Sql statement.</returns>
public static Sql<ISqlContext> Where<TDto1, TDto2, TDto3>(this Sql<ISqlContext> sql, Expression<Func<TDto1, TDto2, TDto3, bool>> predicate, string? alias1 = null, string? alias2 = null, string? alias3 = null)
{
var (s, a) = sql.SqlContext.VisitDto(predicate, alias1, alias2, alias3);

Check notice on line 80 in src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs

View check run for this annotation

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

ℹ New issue: Excess Number of Function Arguments

Where has 5 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
return sql.Where(s, a);
}

/// <summary>
/// Appends a WHERE IN clause to the Sql statement.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using NPoco;

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TrackedReferencesRepository.cs

View check run for this annotation

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

ℹ Getting worse: Overall Code Complexity

The mean cyclomatic complexity increases from 4.08 to 4.54, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TrackedReferencesRepository.cs

View check run for this annotation

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

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 91.67% to 90.38%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Persistence.Repositories;
Expand Down Expand Up @@ -477,95 +477,134 @@
return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
}

public IEnumerable<RelationItemModel> GetPagedDescendantsInReferences(
Guid parentKey,
public async Task<PagedModel<Guid>> GetPagedNodeKeysWithDependantReferencesAsync(
ISet<Guid> keys,
Guid nodeObjectTypeId,
long skip,
long take,
long take)
{
if (_scopeAccessor.AmbientScope is null)
{
throw new InvalidOperationException("Can not execute without a valid AmbientScope");
}

Sql<ISqlContext>? sql = _scopeAccessor.AmbientScope.Database.SqlContext.Sql()
.SelectDistinct<NodeDto>(node => node.UniqueId)
.From<NodeDto>()
.InnerJoin<RelationDto>()
.On<NodeDto, RelationDto>((node, relation) =>
node.NodeId == relation.ParentId || node.NodeId == relation.ParentId || node.NodeId == relation.ChildId)
.InnerJoin<RelationTypeDto>()
.On<RelationDto, RelationTypeDto>((relation, relationType) => relation.RelationType == relationType.Id && relationType.IsDependency)
.Where<NodeDto, RelationDto, RelationTypeDto>(
(node, relation, relationType)
=> node.NodeObjectType == nodeObjectTypeId
&& keys.Contains(node.UniqueId)
&& (node.NodeId == relation.ChildId
|| (relationType.Dual && relation.ParentId == node.NodeId)));

var totalRecords = _scopeAccessor.AmbientScope.Database.Count(sql);

// no need to process further if no records are found
if (totalRecords < 1)
{
return new PagedModel<Guid>(totalRecords, Enumerable.Empty<Guid>());
}

// Ordering is required for paging
sql = sql.OrderBy<NodeDto>(node => node.UniqueId);

IEnumerable<Guid> result = await _scopeAccessor.AmbientScope.Database.SkipTakeAsync<Guid>(skip, take, sql);

return new PagedModel<Guid>(totalRecords, result);
}

Check warning on line 520 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TrackedReferencesRepository.cs

View check run for this annotation

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

❌ New issue: Complex Method

GetPagedNodeKeysWithDependantReferencesAsync has a cyclomatic complexity of 10, 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.

public IEnumerable<RelationItemModel> GetPagedDescendantsInReferences(Guid parentKey, long skip, long take,
bool filterMustBeIsDependency,
out long totalRecords)
{
SqlSyntax.ISqlSyntaxProvider? syntax = _scopeAccessor.AmbientScope?.Database.SqlContext.SqlSyntax;

// Gets the path of the parent with ",%" added
Sql<ISqlContext>? subsubQuery = _scopeAccessor.AmbientScope?.Database.SqlContext.Sql()
.Select(syntax?.GetConcat("[node].[path]", "',%'"))
.From<NodeDto>("node")
.Where<NodeDto>(x => x.UniqueId == parentKey, "node");

// Gets the descendants of the parent node
Sql<ISqlContext>? subQuery = _scopeAccessor.AmbientScope?.Database.SqlContext.Sql()
.Select<NodeDto>(x => x.NodeId)
.From<NodeDto>()
.WhereLike<NodeDto>(x => x.Path, subsubQuery);

Sql<ISqlContext> innerUnionSql = GetInnerUnionSql();
Sql<ISqlContext>? sql = _scopeAccessor.AmbientScope?.Database.SqlContext.Sql().SelectDistinct(
"[x].[id] as nodeId",
"[n].[uniqueId] as nodeKey",
"[n].[text] as nodeName",
"[n].[nodeObjectType] as nodeObjectType",
"[d].[published] as nodePublished",
"[ct].[icon] as contentTypeIcon",
"[ct].[alias] as contentTypeAlias",
"[ctn].[text] as contentTypeName",
"[x].[alias] as relationTypeAlias",
"[x].[name] as relationTypeName",
"[x].[isDependency] as relationTypeIsDependency",
"[x].[dual] as relationTypeIsBidirectional")
.From<NodeDto>("n")
.InnerJoinNested(innerUnionSql, "x")
.On<NodeDto, UnionHelperDto>((n, x) => n.NodeId == x.Id, "n", "x")
.LeftJoin<ContentDto>("c").On<NodeDto, ContentDto>(
(left, right) => left.NodeId == right.NodeId,
aliasLeft: "n",
aliasRight: "c")
.LeftJoin<ContentTypeDto>("ct")
.On<ContentDto, ContentTypeDto>(
(left, right) => left.ContentTypeId == right.NodeId,
aliasLeft: "c",
aliasRight: "ct")
.LeftJoin<NodeDto>("ctn")
.On<ContentTypeDto, NodeDto>(
(left, right) => left.NodeId == right.NodeId,
aliasLeft: "ct",
aliasRight: "ctn")
.LeftJoin<DocumentDto>("d")
.On<NodeDto, DocumentDto>(
(left, right) => left.NodeId == right.NodeId,
aliasLeft: "n",
aliasRight: "d");
sql = sql?.WhereIn(
(System.Linq.Expressions.Expression<Func<NodeDto, object?>>)(x => x.NodeId),
subQuery,
"n");

if (filterMustBeIsDependency)
{
sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
}

// find the count before ordering
totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;

RelationItemDto[] pagedResult;

//Only to all this, if there is items
if (totalRecords > 0)
{
// Ordering is required for paging
sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");

pagedResult =
_scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
Array.Empty<RelationItemDto>();
}
else
{
pagedResult = Array.Empty<RelationItemDto>();
}

return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
}

Check warning on line 607 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TrackedReferencesRepository.cs

View check run for this annotation

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

❌ New issue: Large Method

GetPagedDescendantsInReferences has 72 lines, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.

[Obsolete("Use overload that takes keys instead of ids. This will be removed in Umbraco 15.")]
public IEnumerable<RelationItemModel> GetPagedItemsWithRelations(
Expand Down
19 changes: 19 additions & 0 deletions src/Umbraco.Infrastructure/Persistence/SqlContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq.Expressions;

Check warning on line 1 in src/Umbraco.Infrastructure/Persistence/SqlContextExtensions.cs

View check run for this annotation

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

❌ New issue: Primitive Obsession

In this module, 42.9% of all function arguments are primitive types, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check warning on line 1 in src/Umbraco.Infrastructure/Persistence/SqlContextExtensions.cs

View check run for this annotation

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

❌ New issue: String Heavy Function Arguments

In this module, 42.9% of all arguments to its 8 functions are strings. The threshold for string arguments is 39.0%. The functions in this file have a high ratio of strings as arguments. Avoid adding more.
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.Querying;

Expand Down Expand Up @@ -57,6 +57,25 @@
return (visited, visitor.GetSqlParameters());
}

/// <summary>
/// Visit an expression.
/// </summary>
/// <typeparam name="TDto1">The type of the first DTO.</typeparam>
/// <typeparam name="TDto2">The type of the second DTO.</typeparam>
/// <typeparam name="TDto3">The type of the third DTO.</typeparam>
/// <param name="sqlContext">An <see cref="ISqlContext" />.</param>
/// <param name="expression">An expression to visit.</param>
/// <param name="alias1">An optional table alias for the first DTO.</param>
/// <param name="alias2">An optional table alias for the second DTO.</param>
/// <param name="alias3">An optional table alias for the third DTO.</param>
/// <returns>A SQL statement, and arguments, corresponding to the expression.</returns>
public static (string Sql, object[] Args) VisitDto<TDto1, TDto2, TDto3, TOut>(this ISqlContext sqlContext, Expression<Func<TDto1, TDto2, TDto3, TOut>> expression, string? alias1 = null, string? alias2 = null, string? alias3 = null)
{
var visitor = new PocoToSqlExpressionVisitor<TDto1, TDto2, TDto3>(sqlContext, alias1, alias2, alias3);
var visited = visitor.Visit(expression);
return (visited, visitor.GetSqlParameters());
}

Check warning on line 77 in src/Umbraco.Infrastructure/Persistence/SqlContextExtensions.cs

View check run for this annotation

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

❌ New issue: Excess Number of Function Arguments

VisitDto has 5 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.

/// <summary>
/// Visit an expression.
/// </summary>
Expand All @@ -70,12 +89,12 @@
/// <param name="alias2">An optional table alias for the second DTO.</param>
/// <param name="alias3">An optional table alias for the third DTO.</param>
/// <returns>A SQL statement, and arguments, corresponding to the expression.</returns>
public static (string Sql, object[] Args) VisitDto<TDto1, TDto2, TDto3, TOut>(this ISqlContext sqlContext, Expression<Func<TDto1, TDto2, TDto3, TOut>> expression, string? alias1 = null, string? alias2 = null, string? alias3 = null)
{
var visitor = new PocoToSqlExpressionVisitor<TDto1, TDto2, TDto3>(sqlContext, alias1, alias2, alias3);
var visited = visitor.Visit(expression);
return (visited, visitor.GetSqlParameters());
}

Check warning on line 97 in src/Umbraco.Infrastructure/Persistence/SqlContextExtensions.cs

View check run for this annotation

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

❌ New issue: Excess Number of Function Arguments

VisitDto has 5 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.

/// <summary>
/// Visit an expression.
Expand Down
Loading