Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
255f14e
Introduce new method overloads and repository implentation, such that…
AndyButland Jan 20, 2026
5106da9
Use non-obsolete method overloads throughout.
AndyButland Jan 20, 2026
47abde9
Add unit tests to verify property value retrieval.
AndyButland Jan 20, 2026
9e9acc9
Don't load templates for collection view content retrieval.
AndyButland Jan 21, 2026
1c74f6a
Optimize access checks by verifying the full collection rather than o…
AndyButland Jan 21, 2026
5b414d0
Added obsoletion messages and aligned behaviour of content and media …
AndyButland Jan 21, 2026
bb098f2
Apply suggestions from code review
AndyButland Jan 21, 2026
88e0463
Return key in TreeEntityPath collection response, avoiding a later lo…
AndyButland Jan 21, 2026
38a502d
Merge branch 'main' into v17/improvement/optimize-collection-view-pro…
AndyButland Jan 21, 2026
7d7e0f3
Resolve breaking changes to interfaces.
AndyButland Jan 21, 2026
1680b96
Fix further breaking change.
AndyButland Jan 21, 2026
dcfc548
Merge branch 'main' into v17/improvement/optimize-collection-view-pro…
AndyButland Jan 23, 2026
cb569b3
Additional assert for test verifying property loading for a non-exist…
AndyButland Jan 23, 2026
eb43149
Refactored repositories to avoid having method parameters related to …
AndyButland Jan 23, 2026
5e18dad
Remove check that verifies all provided keys are found when doing per…
AndyButland Jan 23, 2026
6cd8bac
Introduce variable for permission set permissions.
AndyButland Jan 23, 2026
9ce4328
Provide functional default implementation on FilterAuthorizedAsync.
AndyButland Jan 23, 2026
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
30 changes: 30 additions & 0 deletions src/Umbraco.Core/Persistence/Repositories/IContentRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public interface IContentRepository<in TId, TEntity> : IReadWriteQueryRepository
/// Gets paged content items.
/// </summary>
/// <remarks>Here, <paramref name="filter" /> can be null but <paramref name="ordering" /> cannot.</remarks>
[Obsolete("Please use the method overload with all parameters. Scheduled for removal in Umbraco 19.")]
IEnumerable<TEntity> GetPage(
IQuery<TEntity>? query,
long pageIndex,
Expand All @@ -81,5 +82,34 @@ IEnumerable<TEntity> GetPage(
IQuery<TEntity>? filter,
Ordering? ordering);

/// <summary>
/// Gets paged content items.
/// </summary>
/// <param name="query">The base query for content items.</param>
/// <param name="pageIndex">The page index (zero-based).</param>
/// <param name="pageSize">The number of items per page.</param>
/// <param name="totalRecords">Output parameter with total record count.</param>
/// <param name="propertyAliases">
/// Optional array of property aliases to load. If null, all properties are loaded.
/// If empty array, no custom properties are loaded (only system properties).
/// </param>
/// <param name="filter">Optional filter query.</param>
/// <param name="ordering">The ordering specification.</param>
/// <param name="loadTemplates">
/// Whether to load templates. Set to false for performance optimization when templates are not needed
/// (e.g., collection views). Default is true. Only applies to Document content; ignored for Media/Member.
/// </param>
/// <returns>A collection of content items for the specified page.</returns>
/// <remarks>Here, <paramref name="filter" /> can be null but <paramref name="ordering" /> cannot.</remarks>
IEnumerable<TEntity> GetPage(
IQuery<TEntity>? query,
long pageIndex,
int pageSize,
out long totalRecords,
string[]? propertyAliases,
IQuery<TEntity>? filter,
Ordering? ordering,
bool loadTemplates = true);

ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options);
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,11 @@ public async Task<bool> IsDeniedForCultures(IUser currentUser, ISet<string> cult
// If we can't find the content item(s) then we can't determine whether you are denied access.
return result is not (ContentAuthorizationStatus.Success or ContentAuthorizationStatus.NotFound);
}

/// <inheritdoc/>
public async Task<ISet<Guid>> FilterAuthorizedAsync(
IUser currentUser,
IEnumerable<Guid> contentKeys,
ISet<string> permissionsToCheck) =>
await _contentPermissionService.FilterAuthorizedAccessAsync(currentUser, contentKeys, permissionsToCheck);
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,15 @@ Task<bool> IsDeniedAtRecycleBinLevelAsync(IUser currentUser, string permissionTo
Task<bool> IsDeniedAtRecycleBinLevelAsync(IUser currentUser, ISet<string> permissionsToCheck);

Task<bool> IsDeniedForCultures(IUser currentUser, ISet<string> culturesToCheck);

/// <summary>
/// Filters the specified content keys to only those the user has access to.
/// </summary>
/// <param name="currentUser">The current user.</param>
/// <param name="contentKeys">The keys of the content items to filter.</param>
/// <param name="permissionsToCheck">The collection of permissions to authorize.</param>
/// <returns>Returns the keys of content items the user has access to.</returns>
// TODO (V18): Remove default implementation.
Task<ISet<Guid>> FilterAuthorizedAsync(IUser currentUser, IEnumerable<Guid> contentKeys, ISet<string> permissionsToCheck)
=> Task.FromResult<ISet<Guid>>(new HashSet<Guid>());
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,14 @@ Task<bool> IsDeniedAsync(IUser currentUser, Guid mediaKey)
/// <param name="currentUser">The current user.</param>
/// <returns>Returns <c>true</c> if authorization is successful, otherwise <c>false</c>.</returns>
Task<bool> IsDeniedAtRecycleBinLevelAsync(IUser currentUser);

/// <summary>
/// Filters the specified media keys to only those the user has access to.
/// </summary>
/// <param name="currentUser">The current user.</param>
/// <param name="mediaKeys">The keys of the media items to filter.</param>
/// <returns>Returns the keys of media items the user has access to.</returns>
// TODO (V18): Remove default implementation.
Task<ISet<Guid>> FilterAuthorizedAsync(IUser currentUser, IEnumerable<Guid> mediaKeys)
=> Task.FromResult<ISet<Guid>>(new HashSet<Guid>());
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,8 @@ public async Task<bool> IsDeniedAtRecycleBinLevelAsync(IUser currentUser)
// If we can't find the media item(s) then we can't determine whether you are denied access.
return result is not (MediaAuthorizationStatus.Success or MediaAuthorizationStatus.NotFound);
}

/// <inheritdoc/>
public async Task<ISet<Guid>> FilterAuthorizedAsync(IUser currentUser, IEnumerable<Guid> mediaKeys) =>
await _mediaPermissionService.FilterAuthorizedAccessAsync(currentUser, mediaKeys);
}
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Services/ContentEditingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ protected override IContent New(string? name, int parentId, IContentType content
protected override OperationResult? Delete(IContent content, int userId) => ContentService.Delete(content, userId);

protected override IEnumerable<IContent> GetPagedChildren(int parentId, int pageIndex, int pageSize, out long total)
=> ContentService.GetPagedChildren(parentId, pageIndex, pageSize, out total);
=> ContentService.GetPagedChildren(parentId, pageIndex, pageSize, out total, propertyAliases: null, filter: null, ordering: null);

protected override ContentEditingOperationStatus Sort(IEnumerable<IContent> items, int userId)
{
Expand Down
79 changes: 73 additions & 6 deletions src/Umbraco.Core/Services/ContentPermissionService.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
using System.Globalization;
using Umbraco.Cms.Core.Cache;

Check warning on line 1 in src/Umbraco.Core/Services/ContentPermissionService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Overall Code Complexity

This module has a mean cyclomatic complexity of 4.63 across 8 functions. The mean complexity threshold is 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Entities;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Services.AuthorizationStatus;
using Umbraco.Extensions;

Expand Down Expand Up @@ -37,19 +37,38 @@
IEnumerable<Guid> contentKeys,
ISet<string> permissionsToCheck)
{
var contentItems = _contentService.GetByIds(contentKeys).ToArray();
Guid[] keysArray = contentKeys.ToArray();

if (contentItems.Length == 0)
if (keysArray.Length == 0)
{
return Task.FromResult(ContentAuthorizationStatus.Success);
}

// Use GetAllPaths instead of loading full content items - we only need paths for authorization
TreeEntityPath[] entityPaths = _entityService.GetAllPaths(UmbracoObjectTypes.Document, keysArray).ToArray();

if (entityPaths.Length == 0)
{
return Task.FromResult(ContentAuthorizationStatus.NotFound);
}

if (contentItems.Any(contentItem => user.HasPathAccess(contentItem, _entityService, _appCaches) == false))
// Check if all requested keys were found
if (entityPaths.Length != keysArray.Length)
{
return Task.FromResult(ContentAuthorizationStatus.UnauthorizedMissingPathAccess);
return Task.FromResult(ContentAuthorizationStatus.NotFound);
}

// Check path access using the paths directly
int[]? startNodeIds = user.CalculateContentStartNodeIds(_entityService, _appCaches);
foreach (TreeEntityPath entityPath in entityPaths)
{
if (ContentPermissions.HasPathAccess(entityPath.Path, startNodeIds, Constants.System.RecycleBinContent) == false)
{
return Task.FromResult(ContentAuthorizationStatus.UnauthorizedMissingPathAccess);
}
}

return Task.FromResult(HasPermissionAccess(user, contentItems.Select(c => c.Path), permissionsToCheck)
return Task.FromResult(HasPermissionAccess(user, entityPaths.Select(p => p.Path), permissionsToCheck)
? ContentAuthorizationStatus.Success
: ContentAuthorizationStatus.UnauthorizedMissingPermissionAccess);
}
Expand Down Expand Up @@ -150,6 +169,54 @@
: ContentAuthorizationStatus.UnauthorizedMissingCulture;
}

/// <inheritdoc/>
public Task<ISet<Guid>> FilterAuthorizedAccessAsync(
IUser user,
IEnumerable<Guid> contentKeys,
ISet<string> permissionsToCheck)
{
Guid[] keysArray = [.. contentKeys];

if (keysArray.Length == 0)
{
return Task.FromResult<ISet<Guid>>(new HashSet<Guid>());
}

// Retrieve paths in a a single database query for all keys.
TreeEntityPath[] entityPaths = [.. _entityService.GetAllPaths(UmbracoObjectTypes.Document, keysArray)];

if (entityPaths.Length == 0)
{
return Task.FromResult<ISet<Guid>>(new HashSet<Guid>());
}

var authorizedKeys = new HashSet<Guid>();
int[]? startNodeIds = user.CalculateContentStartNodeIds(_entityService, _appCaches);

foreach (TreeEntityPath entityPath in entityPaths)
{
// Check path access
if (ContentPermissions.HasPathAccess(entityPath.Path, startNodeIds, Constants.System.RecycleBinContent) == false)
{
continue;
}

// Check permission access
EntityPermissionSet permissionSet = _userService.GetPermissionsForPath(user, entityPath.Path);
if (permissionsToCheck.All(p => permissionSet.GetAllPermissions().Contains(p)))
{
// Get the key for this entity's ID
Attempt<Guid> keyAttempt = _entityService.GetKey(entityPath.Id, UmbracoObjectTypes.Document);
if (keyAttempt.Success)
{
authorizedKeys.Add(keyAttempt.Result);
}
}
}

return Task.FromResult<ISet<Guid>>(authorizedKeys);
}

Check warning on line 218 in src/Umbraco.Core/Services/ContentPermissionService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Bumpy Road Ahead

FilterAuthorizedAccessAsync has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.

/// <summary>
/// Check the implicit/inherited permissions of a user for given content items.
/// </summary>
Expand Down
11 changes: 8 additions & 3 deletions src/Umbraco.Core/Services/ContentService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Immutable;

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

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Code Duplication

reduced similar code in: GetPagedChildren,GetPagedOfType. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

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

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 4.07 to 4.05, 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.
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -840,7 +840,12 @@
}

/// <inheritdoc />
[Obsolete("Please use the method overload with all parameters. Scheduled for removal in Umbraco 19.")]
public IEnumerable<IContent> GetPagedChildren(int id, long pageIndex, int pageSize, out long totalChildren, IQuery<IContent>? filter = null, Ordering? ordering = null)
=> GetPagedChildren(id, pageIndex, pageSize, out totalChildren, propertyAliases: null, filter: filter, ordering: ordering);

/// <inheritdoc />
public IEnumerable<IContent> GetPagedChildren(int id, long pageIndex, int pageSize, out long totalChildren, string[]? propertyAliases, IQuery<IContent>? filter, Ordering? ordering, bool loadTemplates = true)
{
if (pageIndex < 0)
{
Expand All @@ -859,7 +864,7 @@
scope.ReadLock(Constants.Locks.ContentTree);

IQuery<IContent>? query = Query<IContent>()?.Where(x => x.ParentId == id);
return _documentRepository.GetPage(query, pageIndex, pageSize, out totalChildren, filter, ordering);
return _documentRepository.GetPage(query, pageIndex, pageSize, out totalChildren, propertyAliases, filter, ordering, loadTemplates);
}
}

Expand Down Expand Up @@ -918,7 +923,7 @@
throw new ArgumentNullException(nameof(ordering));
}

return _documentRepository.GetPage(query, pageIndex, pageSize, out totalChildren, filter, ordering);
return _documentRepository.GetPage(query, pageIndex, pageSize, out totalChildren, propertyAliases: null, filter, ordering);
}

/// <summary>
Expand Down Expand Up @@ -1009,7 +1014,7 @@
scope.ReadLock(Constants.Locks.ContentTree);
IQuery<IContent>? query = Query<IContent>()?
.Where(x => x.Path.StartsWith(Constants.System.RecycleBinContentPathPrefix));
return _documentRepository.GetPage(query, pageIndex, pageSize, out totalRecords, filter, ordering);
return _documentRepository.GetPage(query, pageIndex, pageSize, out totalRecords, propertyAliases: null, filter, ordering);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Core/Services/EntityXmlSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public XElement Serialize(
while (page * pageSize < total)
{
IEnumerable<IContent> children =
_contentService.GetPagedChildren(content.Id, page++, pageSize, out total);
_contentService.GetPagedChildren(content.Id, page++, pageSize, out total, propertyAliases: null, filter: null, ordering: null);
SerializeChildren(children, xml, published);
}
}
Expand Down Expand Up @@ -678,7 +678,7 @@ private void SerializeChildren(IEnumerable<IContent> children, XElement xml, boo
while (page * pageSize < total)
{
IEnumerable<IContent> grandChildren =
_contentService.GetPagedChildren(child.Id, page++, pageSize, out total);
_contentService.GetPagedChildren(child.Id, page++, pageSize, out total, propertyAliases: null, filter: null, ordering: null);

// recurse
SerializeChildren(grandChildren, childXml, published);
Expand Down
11 changes: 11 additions & 0 deletions src/Umbraco.Core/Services/IContentPermissionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,15 @@ Task<ContentAuthorizationStatus> AuthorizeBinAccessAsync(IUser user, string perm
/// <param name="culturesToCheck">The collection of cultures to authorize.</param>
/// <returns>A task resolving into a <see cref="ContentAuthorizationStatus"/>.</returns>
Task<ContentAuthorizationStatus> AuthorizeCultureAccessAsync(IUser user, ISet<string> culturesToCheck);

/// <summary>
/// Filters content keys to only those the user has access to.
/// </summary>
/// <param name="user"><see cref="IUser" /> to authorize.</param>
/// <param name="contentKeys">The identifiers of the content items to filter.</param>
/// <param name="permissionsToCheck">The collection of permissions to authorize.</param>
/// <returns>A task resolving into the set of authorized content keys.</returns>
// TODO (V18): Remove default implementation.
Task<ISet<Guid>> FilterAuthorizedAccessAsync(IUser user, IEnumerable<Guid> contentKeys, ISet<string> permissionsToCheck)
=> Task.FromResult<ISet<Guid>>(new HashSet<Guid>());
}
44 changes: 43 additions & 1 deletion src/Umbraco.Core/Services/IContentSearchService{TContent}.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,56 @@
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models;

namespace Umbraco.Cms.Core.Services;

/// <summary>
/// Defines methods for searching and retrieving child content items of a specified parent, with support for filtering,
/// ordering, and paging.
/// </summary>
/// <typeparam name="TContent">The type of content item to search for. Must implement <see cref="IContentBase"/>.</typeparam>
public interface IContentSearchService<TContent>
where TContent : class, IContentBase
{
/// <summary>
/// Searches for children of a content item.
/// </summary>
/// <param name="query">The search query.</param>
/// <param name="parentId">The parent content item key.</param>
/// <param name="ordering">The ordering.</param>
/// <param name="skip">The number of items to skip.</param>
/// <param name="take">The number of items to take.</param>
/// <returns>A paged model of content items.</returns>
[Obsolete("Please use the method overload with all parameters. Scheduled for removal in Umbraco 19.")]
Task<PagedModel<TContent>> SearchChildrenAsync(
string? query,
Guid? parentId,
Ordering? ordering,
int skip = 0,
int take = 100)
=> SearchChildrenAsync(query, parentId, propertyAliases: null, ordering: ordering, skip: skip, take: take);

Check warning on line 29 in src/Umbraco.Core/Services/IContentSearchService{TContent}.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Excess Number of Function Arguments

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

/// <summary>
/// Searches for children of a content item with optional property filtering.
/// </summary>
/// <param name="query">The search query.</param>
/// <param name="parentId">The parent content item key.</param>
/// <param name="propertyAliases">
/// The property aliases to load. If null, all properties are loaded.
/// If empty array, no custom properties are loaded.
/// </param>
/// <param name="ordering">The ordering.</param>
/// <param name="loadTemplates">
/// Whether to load templates. Set to false for performance optimization when templates are not needed
/// (e.g., collection views). Default is true. Only applies to Document content; ignored for Media/Member.
/// </param>
/// <param name="skip">The number of items to skip.</param>
/// <param name="take">The number of items to take.</param>
/// <returns>A paged model of content items.</returns>
Task<PagedModel<TContent>> SearchChildrenAsync(
string? query,
Guid? parentId,
string[]? propertyAliases,
Ordering? ordering,
bool loadTemplates = true,
int skip = 0,
int take = 100);
}
20 changes: 20 additions & 0 deletions src/Umbraco.Core/Services/IContentService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,28 @@ IContent CreateBlueprintFromContent(IContent blueprint, string name, int userId
/// <param name="totalRecords">Total number of documents.</param>
/// <param name="filter">Query filter.</param>
/// <param name="ordering">Ordering infos.</param>
[Obsolete("Please use the method overload with all parameters. Scheduled for removal in Umbraco 19.")]
IEnumerable<IContent> GetPagedChildren(int id, long pageIndex, int pageSize, out long totalRecords, IQuery<IContent>? filter = null, Ordering? ordering = null);

/// <summary>
/// Gets child documents of a parent with optional property filtering.
/// </summary>
/// <param name="id">The parent identifier.</param>
/// <param name="pageIndex">The page number.</param>
/// <param name="pageSize">The page size.</param>
/// <param name="totalRecords">Total number of documents.</param>
/// <param name="propertyAliases">
/// The property aliases to load. If null, all properties are loaded.
/// If empty array, no custom properties are loaded.
/// </param>
/// <param name="filter">Query filter.</param>
/// <param name="ordering">Ordering infos.</param>
/// <param name="loadTemplates">
/// Whether to load templates. Set to false for performance optimization when templates are not needed
/// (e.g., collection views). Default is true.
/// </param>
IEnumerable<IContent> GetPagedChildren(int id, long pageIndex, int pageSize, out long totalRecords, string[]? propertyAliases, IQuery<IContent>? filter, Ordering? ordering, bool loadTemplates = true);

/// <summary>
/// Gets descendant documents of a given parent.
/// </summary>
Expand Down
10 changes: 10 additions & 0 deletions src/Umbraco.Core/Services/IMediaPermissionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,14 @@ Task<MediaAuthorizationStatus> AuthorizeAccessAsync(IUser user, Guid mediaKey)
/// <param name="user"><see cref="IUser" /> to authorize.</param>
/// <returns>A task resolving into a <see cref="MediaAuthorizationStatus"/>.</returns>
Task<MediaAuthorizationStatus> AuthorizeBinAccessAsync(IUser user);

/// <summary>
/// Filters media keys to only those the user has access to.
/// </summary>
/// <param name="user"><see cref="IUser" /> to authorize.</param>
/// <param name="mediaKeys">The identifiers of the media items to filter.</param>
/// <returns>A task resolving into the set of authorized media keys.</returns>
// TODO (V18): Remove default implementation.
Task<ISet<Guid>> FilterAuthorizedAccessAsync(IUser user, IEnumerable<Guid> mediaKeys)
=> Task.FromResult<ISet<Guid>>(new HashSet<Guid>());
}
Loading
Loading