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 @@ -6,6 +6,7 @@
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.Services;
using Umbraco.Extensions;

namespace Umbraco.Cms.Api.Management.Factories;
Expand All @@ -18,21 +19,35 @@ public abstract class ContentCollectionPresentationFactory<TContent, TCollection
{
private readonly FlagProviderCollection _flagProviderCollection;
private readonly IUmbracoMapper _mapper;
private readonly IUserService _userService;

[Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
protected ContentCollectionPresentationFactory(IUmbracoMapper mapper)
: this(
mapper,
StaticServiceProvider.Instance.GetRequiredService<FlagProviderCollection>())
protected ContentCollectionPresentationFactory(
IUmbracoMapper mapper,
FlagProviderCollection flagProviderCollection,
IUserService userService)
{
_mapper = mapper;
_flagProviderCollection = flagProviderCollection;
_userService = userService;
}

[Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
protected ContentCollectionPresentationFactory(
IUmbracoMapper mapper,
FlagProviderCollection flagProviderCollection)
: this(
mapper,
flagProviderCollection,
StaticServiceProvider.Instance.GetRequiredService<IUserService>())
{
}

[Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
protected ContentCollectionPresentationFactory(IUmbracoMapper mapper)
: this(
mapper,
StaticServiceProvider.Instance.GetRequiredService<FlagProviderCollection>())
{
_mapper = mapper;
_flagProviderCollection = flagProviderCollection;
}

public async Task<List<TCollectionResponseModel>> CreateCollectionModelAsync(ListViewPagedModel<TContent> contentCollection)
Expand All @@ -46,15 +61,18 @@ public async Task<List<TCollectionResponseModel>> CreateCollectionModelAsync(Lis
.WhereNotNull()
.ToArray();

// Pre-resolve all unique creator/writer user names in a single batch call.
Dictionary<int, string?> userNameDictionary = ResolveUserNames(collectionItemsResult.Items);

List<TCollectionResponseModel> collectionResponseModels =
_mapper.MapEnumerable<TContent, TCollectionResponseModel>(collectionItemsResult.Items, context =>
{
context.SetIncludedProperties(collectionPropertyAliases);
context.SetUserNameDictionary(userNameDictionary);
});

await SetUnmappedProperties(contentCollection, collectionResponseModels);


await PopulateFlags(collectionResponseModels);

return collectionResponseModels;
Expand All @@ -69,4 +87,27 @@ private async Task PopulateFlags(IEnumerable<TCollectionResponseModel> models)
await signProvider.PopulateFlagsAsync(models);
}
}

private Dictionary<int, string?> ResolveUserNames(IEnumerable<TContent> items)
{
var uniqueUserIds = new HashSet<int>();
foreach (TContent item in items)
{
uniqueUserIds.Add(item.CreatorId);
uniqueUserIds.Add(item.WriterId);
}

// Filter out the default 0 ID (unset CreatorId/WriterId from TreeEntityBase) that won't
// resolve to a user. Seed it as null so CommonMapper won't fall back to per-item GetProfileById.
Dictionary<int, string?> result = _userService
.GetUsersById(uniqueUserIds.Where(id => id != 0).ToArray())
.ToDictionary(u => u.Id, u => u.Name);

if (uniqueUserIds.Contains(0))
{
result[0] = null;
}

return result;
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
using Microsoft.Extensions.DependencyInjection;
using Lucene.Net.Util;
using Umbraco.Cms.Api.Management.Services.Flags;
using Umbraco.Cms.Api.Management.ViewModels;
using Umbraco.Cms.Api.Management.ViewModels.Document;
using Umbraco.Cms.Api.Management.ViewModels.Document.Collection;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services;
using Umbraco.Extensions;

namespace Umbraco.Cms.Api.Management.Factories;

Expand All @@ -14,29 +15,82 @@ public class DocumentCollectionPresentationFactory : ContentCollectionPresentati
private readonly IPublicAccessService _publicAccessService;
private readonly IEntityService _entityService;

[ActivatorUtilitiesConstructor]
public DocumentCollectionPresentationFactory(IUmbracoMapper mapper, FlagProviderCollection flagProviders, IPublicAccessService publicAccessService, IEntityService entityService, IUserService userService)
: base(mapper, flagProviders, userService)
{
_publicAccessService = publicAccessService;
_entityService = entityService;
}

[Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
public DocumentCollectionPresentationFactory(IUmbracoMapper mapper, FlagProviderCollection flagProviders, IPublicAccessService publicAccessService, IEntityService entityService)
: base(mapper, flagProviders)
{
_publicAccessService = publicAccessService;
_entityService = entityService;
}

[Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
public DocumentCollectionPresentationFactory(IUmbracoMapper mapper, IPublicAccessService publicAccessService, IEntityService entityService)
: base(mapper)
: base(mapper)
{
_publicAccessService = publicAccessService;
_entityService = entityService;
}

/// <inheritdoc/>
protected override Task SetUnmappedProperties(ListViewPagedModel<IContent> contentCollection, List<DocumentCollectionResponseModel> collectionResponseModels)
{
// Retrieve all public access entries once (single scope) instead of
// calling IsProtected per item which creates N scopes.
var protectedNodeIds = new HashSet<int>(
_publicAccessService.GetAll().Select(entry => entry.ProtectedNodeId));

// All items in a collection are siblings (same parent), so with omitSelf
// they share the same ancestor array. Compute once, reuse for all.
IEnumerable<ReferenceByIdModel>? sharedAncestors = null;

// Create a lookup of content items by key for efficient matching when looping the response models.
var contentByKey = contentCollection.Items.Items.ToDictionary(x => x.Key);

foreach (DocumentCollectionResponseModel item in collectionResponseModels)
{
IContent? matchingContentItem = contentCollection.Items.Items.FirstOrDefault(x => x.Key == item.Id);
if (matchingContentItem is null)
if (contentByKey.TryGetValue(item.Id, out IContent? matchingContentItem) is false)
{
continue;
}

item.IsProtected = _publicAccessService.IsProtected(matchingContentItem).Success;
item.Ancestors = _entityService.GetPathKeys(matchingContentItem, omitSelf: true)
.Select(x => new ReferenceByIdModel(x));
item.IsProtected = IsProtected(matchingContentItem, protectedNodeIds);
sharedAncestors ??= _entityService.GetPathKeys(matchingContentItem, omitSelf: true)
.Select(x => new ReferenceByIdModel(x))
.ToArray();
item.Ancestors = sharedAncestors;
}

return Task.CompletedTask;
}

/// <summary>
/// Checks whether a content item is protected by public access, using a pre-fetched
/// set of protected node IDs.
/// </summary>
private static bool IsProtected(IContent content, HashSet<int> protectedNodeIds)
{
if (protectedNodeIds.Count == 0)
{
return false;
}

// Walk the content path from deepest to shallowest, checking for a match.
int[] pathIds = content.Path.EnsureEndsWith("," + content.Id).GetIdsFromPathReversed();
foreach (var id in pathIds)
{
if (id != Core.Constants.System.Root && protectedNodeIds.Contains(id))
{
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
using Umbraco.Cms.Api.Management.Services.Flags;
using Umbraco.Cms.Api.Management.ViewModels.Media;
using Umbraco.Cms.Api.Management.ViewModels.Media.Collection;
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services;

namespace Umbraco.Cms.Api.Management.Factories;

public class MediaCollectionPresentationFactory : ContentCollectionPresentationFactory<IMedia, MediaCollectionResponseModel, MediaValueResponseModel, MediaVariantResponseModel>, IMediaCollectionPresentationFactory
{
public MediaCollectionPresentationFactory(IUmbracoMapper mapper, FlagProviderCollection flagProviders, IUserService userService)
: base(mapper, flagProviders, userService)
{
}

[Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 18.")]
public MediaCollectionPresentationFactory(IUmbracoMapper mapper)
: base(mapper)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Umbraco.Cms.Api.Management.ViewModels.Document.Collection;
using Umbraco.Cms.Api.Management.ViewModels.Document.Item;
using Umbraco.Cms.Api.Management.ViewModels.Tree;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services;
using Constants = Umbraco.Cms.Core.Constants;
Expand All @@ -18,15 +17,13 @@
private const string Alias = Constants.Conventions.Flags.Prefix + "ScheduledForPublish";

private readonly IContentService _contentService;
private readonly IIdKeyMap _keyMap;

/// <summary>
/// Initializes a new instance of the <see cref="HasScheduleFlagProvider"/> class.
/// </summary>
public HasScheduleFlagProvider(IContentService contentService, IIdKeyMap keyMap)
public HasScheduleFlagProvider(IContentService contentService)
{
_contentService = contentService;
_keyMap = keyMap;
}

/// <inheritdoc/>
Expand All @@ -40,16 +37,11 @@
public Task PopulateFlagsAsync<TItem>(IEnumerable<TItem> items)
where TItem : IHasFlags
{
IDictionary<int, IEnumerable<ContentSchedule>> schedules = _contentService.GetContentSchedulesByIds(items.Select(x => x.Id).ToArray());
foreach (TItem item in items)
TItem[] itemsArray = items.ToArray();
IDictionary<Guid, IEnumerable<ContentSchedule>> schedules = _contentService.GetContentSchedulesByKeys(itemsArray.Select(x => x.Id).ToArray());
foreach (TItem item in itemsArray)
{
Attempt<int> itemId = _keyMap.GetIdForKey(item.Id, UmbracoObjectTypes.Document);
if (itemId.Success is false)
{
continue;
}

if (!schedules.TryGetValue(itemId.Result, out IEnumerable<ContentSchedule>? contentSchedules))
if (schedules.TryGetValue(item.Id, out IEnumerable<ContentSchedule>? contentSchedules) is false)

Check notice on line 44 in src/Umbraco.Cms.Api.Management/Services/Flags/HasScheduleFlagProvider.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ No longer an issue: Complex Method

PopulateFlagsAsync is no longer above the threshold for cyclomatic complexity
{
continue;
}
Expand Down
15 changes: 13 additions & 2 deletions src/Umbraco.Core/Models/Mapping/CommonMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public CommonMapper(IUserService userService)
/// <param name="context">The mapper context.</param>
/// <returns>The name of the owner, or null if not found.</returns>
public string? GetOwnerName(IContentBase source, MapperContext context)
=> source.GetCreatorProfile(_userService)?.Name;
=> GetUserName(source.CreatorId, context, () => source.GetCreatorProfile(_userService));

/// <summary>
/// Gets the name of the creator (last writer) of the content.
Expand All @@ -37,5 +37,16 @@ public CommonMapper(IUserService userService)
/// <param name="context">The mapper context.</param>
/// <returns>The name of the creator, or null if not found.</returns>
public string? GetCreatorName(IContent source, MapperContext context)
=> source.GetWriterProfile(_userService)?.Name;
=> GetUserName(source.WriterId, context, () => source.GetWriterProfile(_userService));

private static string? GetUserName(int userId, MapperContext context, Func<IProfile?> fallback)
{
Dictionary<int, string?>? userNames = context.GetUserNameDictionary();
if (userNames is not null && userNames.TryGetValue(userId, out var name))
{
return name;
}

return fallback()?.Name;
}
}
13 changes: 13 additions & 0 deletions src/Umbraco.Core/Models/Mapping/MapperContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public static class MapperContextExtensions
private const string CultureKey = "Map.Culture";
private const string SegmentKey = "Map.Segment";
private const string IncludedPropertiesKey = "Map.IncludedProperties";
private const string UserNameDictionaryKey = "Map.UserNameDictionary";

/// <summary>
/// Gets the context culture.
Expand Down Expand Up @@ -46,4 +47,16 @@ public static class MapperContextExtensions
/// </summary>
public static void SetIncludedProperties(this MapperContext context, string[] properties) =>
context.Items[IncludedPropertiesKey] = properties;

/// <summary>
/// Gets a pre-resolved dictionary mapping user IDs to display names.
/// </summary>
public static Dictionary<int, string?>? GetUserNameDictionary(this MapperContext context) =>
context.HasItems && context.Items.TryGetValue(UserNameDictionaryKey, out var obj) && obj is Dictionary<int, string?> d ? d : null;

/// <summary>
/// Sets a pre-resolved dictionary mapping user IDs to display names.
/// </summary>
public static void SetUserNameDictionary(this MapperContext context, Dictionary<int, string?> userNames) =>
context.Items[UserNameDictionaryKey] = userNames;
}
31 changes: 27 additions & 4 deletions src/Umbraco.Core/Services/ContentPermissionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,20 +195,43 @@
var authorizedKeys = new HashSet<Guid>();
int[]? startNodeIds = user.CalculateContentStartNodeIds(_entityService, _appCaches);

// Check path access and collect all unique node IDs across all paths.
var pathDataByKey = new Dictionary<Guid, int[]>();
var allUniqueNodeIds = new HashSet<int>();

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);
int[] pathIds = entityPath.Path.GetIdsFromPathReversed();
pathDataByKey[entityPath.Key] = pathIds;
allUniqueNodeIds.UnionWith(pathIds);
}

if (pathDataByKey.Count == 0)
{
return Task.FromResult<ISet<Guid>>(authorizedKeys);
}

// Single batch query for permissions on ALL unique node IDs across all paths.
EntityPermissionCollection allPermissions = _userService.GetPermissions(user, [.. allUniqueNodeIds]);

// Resolve permissions per entity using the batch results.
foreach ((Guid key, int[] pathIds) in pathDataByKey)
{
var pathNodeIdSet = new HashSet<int>(pathIds);
EntityPermission[] relevantPermissions = allPermissions
.Where(p => pathNodeIdSet.Contains(p.EntityId))
.ToArray();

EntityPermissionSet permissionSet = UserService.CalculatePermissionsForPathForUser(relevantPermissions, pathIds);
ISet<string> permissionSetPermissions = permissionSet.GetAllPermissions();
if (permissionsToCheck.All(p => permissionSetPermissions.Contains(p)))
{
authorizedKeys.Add(entityPath.Key);
authorizedKeys.Add(key);

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

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Complex Method

FilterAuthorizedAccessAsync has a cyclomatic complexity of 9, 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.

Check warning on line 234 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.
}
}

Expand Down
Loading
Loading