From 04ba12297fa3bc147c43ba765d86fde88e700176 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 30 Sep 2024 16:43:05 +0200 Subject: [PATCH] Handle navigation updates in cache refeshers (#17161) * Handle navigation updates in cache refeshers * Same for media cache refreshers * Clean up * More clean up and renaming content to media * Update src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs Co-authored-by: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> --------- Co-authored-by: Elitsa Co-authored-by: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> --- .../Implement/ContentCacheRefresher.cs | 109 +++++++++++++- .../Implement/MediaCacheRefresher.cs | 138 ++++++++++++++++-- .../Factories/NavigationFactory.cs | 42 ------ src/Umbraco.Core/Services/ContentService.cs | 116 --------------- src/Umbraco.Core/Services/MediaService.cs | 117 +-------------- .../ContentNavigationServiceBase.cs | 72 ++++++++- .../DocumentNavigationServiceTestsBase.cs | 11 ++ .../MediaNavigationServiceTestsBase.cs | 10 ++ 8 files changed, 318 insertions(+), 297 deletions(-) delete mode 100644 src/Umbraco.Core/Factories/NavigationFactory.cs diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs index f0a9ef93e6..d2f3e0a6cd 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs @@ -19,6 +19,8 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase(), - StaticServiceProvider.Instance.GetRequiredService() + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService() ) { @@ -55,7 +59,9 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase descendants = _contentService.GetPagedDescendants(content.Id, 0, int.MaxValue, out _); + foreach (IContent descendant in content.Yield().Concat(descendants)) + { + HandleNavigationForSingleContent(descendant); + } + } + } + + private void HandleNavigationForSingleContent(IContent content) + { + // First creation + if (ExistsInNavigation(content.Key) is false && ExistsInNavigationBin(content.Key) is false) + { + _documentNavigationManagementService.Add(content.Key, GetParentKey(content)); + if (content.Trashed) + { + // If created as trashed, move to bin + _documentNavigationManagementService.MoveToBin(content.Key); + } + } + else if (ExistsInNavigation(content.Key) && ExistsInNavigationBin(content.Key) is false) + { + if (content.Trashed) + { + // It must have been trashed + _documentNavigationManagementService.MoveToBin(content.Key); + } + else + { + // It must have been saved. Check if parent is different + if (_documentNavigationQueryService.TryGetParentKey(content.Key, out var oldParentKey)) + { + Guid? newParentKey = GetParentKey(content); + if (oldParentKey != newParentKey) + { + _documentNavigationManagementService.Move(content.Key, newParentKey); + } + } + } + } + else if (ExistsInNavigation(content.Key) is false && ExistsInNavigationBin(content.Key)) + { + if (content.Trashed is false) + { + // It must have been restored + _documentNavigationManagementService.RestoreFromBin(content.Key, GetParentKey(content)); + } + } + } + + private Guid? GetParentKey(IContent content) => (content.ParentId == -1) ? null : _idKeyMap.GetKeyForId(content.ParentId, UmbracoObjectTypes.Document).Result; + + private bool ExistsInNavigation(Guid contentKey) => _documentNavigationQueryService.TryGetParentKey(contentKey, out _); + + private bool ExistsInNavigationBin(Guid contentKey) => _documentNavigationQueryService.TryGetParentKeyInBin(contentKey, out _); + private void HandleRouting(JsonPayload payload) { if(payload.ChangeTypes.HasType(TreeChangeTypes.Remove)) diff --git a/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs b/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs index e42f8ce4f0..bcc8630e57 100644 --- a/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/Refreshers/Implement/MediaCacheRefresher.cs @@ -6,6 +6,7 @@ using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.Changes; +using Umbraco.Cms.Core.Services.Navigation; using Umbraco.Extensions; namespace Umbraco.Cms.Core.Cache; @@ -13,6 +14,9 @@ namespace Umbraco.Cms.Core.Cache; public sealed class MediaCacheRefresher : PayloadCacheRefresherBase { private readonly IIdKeyMap _idKeyMap; + private readonly IMediaNavigationQueryService _mediaNavigationQueryService; + private readonly IMediaNavigationManagementService _mediaNavigationManagementService; + private readonly IMediaService _mediaService; private readonly IPublishedSnapshotService _publishedSnapshotService; public MediaCacheRefresher( @@ -21,11 +25,17 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase(payload.Id)); + mediaCache.Result?.Clear(RepositoryCacheKeys.GetKey(payload.Key)); + + // remove those that are in the branch + if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.RefreshBranch | TreeChangeTypes.Remove)) + { + var pathid = "," + payload.Id + ","; + mediaCache.Result?.ClearOfType((_, v) => v.Path?.Contains(pathid) ?? false); + } } - // repository cache - // it *was* done for each pathId but really that does not make sense - // only need to do it for the current media - mediaCache.Result?.Clear(RepositoryCacheKeys.GetKey(payload.Id)); - mediaCache.Result?.Clear(RepositoryCacheKeys.GetKey(payload.Key)); - - // remove those that are in the branch - if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.RefreshBranch | TreeChangeTypes.Remove)) - { - var pathid = "," + payload.Id + ","; - mediaCache.Result?.ClearOfType((_, v) => v.Path?.Contains(pathid) ?? false); - } + HandleNavigation(payload); } _publishedSnapshotService.Notify(payloads, out var hasPublishedDataChanged); @@ -110,9 +120,107 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase descendants = _mediaService.GetPagedDescendants(media.Id, 0, int.MaxValue, out _); + foreach (IMedia descendant in media.Yield().Concat(descendants)) + { + HandleNavigationForSingleMedia(descendant); + } + } + } + + private void HandleNavigationForSingleMedia(IMedia media) + { + // First creation + if (ExistsInNavigation(media.Key) is false && ExistsInNavigationBin(media.Key) is false) + { + _mediaNavigationManagementService.Add(media.Key, GetParentKey(media)); + if (media.Trashed) + { + // If created as trashed, move to bin + _mediaNavigationManagementService.MoveToBin(media.Key); + } + } + else if (ExistsInNavigation(media.Key) && ExistsInNavigationBin(media.Key) is false) + { + if (media.Trashed) + { + // It must have been trashed + _mediaNavigationManagementService.MoveToBin(media.Key); + } + else + { + // It must have been saved. Check if parent is different + if (_mediaNavigationQueryService.TryGetParentKey(media.Key, out var oldParentKey)) + { + Guid? newParentKey = GetParentKey(media); + if (oldParentKey != newParentKey) + { + _mediaNavigationManagementService.Move(media.Key, newParentKey); + } + } + } + } + else if (ExistsInNavigation(media.Key) is false && ExistsInNavigationBin(media.Key)) + { + if (media.Trashed is false) + { + // It must have been restored + _mediaNavigationManagementService.RestoreFromBin(media.Key, GetParentKey(media)); + } + } + } + + private Guid? GetParentKey(IMedia media) => (media.ParentId == -1) ? null : _idKeyMap.GetKeyForId(media.ParentId, UmbracoObjectTypes.Media).Result; + + private bool ExistsInNavigation(Guid contentKey) => _mediaNavigationQueryService.TryGetParentKey(contentKey, out _); + + private bool ExistsInNavigationBin(Guid contentKey) => _mediaNavigationQueryService.TryGetParentKeyInBin(contentKey, out _); + + // these events should never trigger // everything should be JSON public override void RefreshAll() => throw new NotSupportedException(); diff --git a/src/Umbraco.Core/Factories/NavigationFactory.cs b/src/Umbraco.Core/Factories/NavigationFactory.cs deleted file mode 100644 index a95cbf68a5..0000000000 --- a/src/Umbraco.Core/Factories/NavigationFactory.cs +++ /dev/null @@ -1,42 +0,0 @@ -using System.Collections.Concurrent; -using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.Navigation; - -namespace Umbraco.Cms.Core.Factories; - -internal static class NavigationFactory -{ - /// - /// Builds a dictionary of NavigationNode objects from a given dataset. - /// - /// A dictionary of objects with key corresponding to their unique Guid. - /// The objects used to build the navigation nodes dictionary. - public static void BuildNavigationDictionary(ConcurrentDictionary nodesStructure, IEnumerable entities) - { - var entityList = entities.ToList(); - Dictionary idToKeyMap = entityList.ToDictionary(x => x.Id, x => x.Key); - - foreach (INavigationModel entity in entityList) - { - var node = new NavigationNode(entity.Key); - nodesStructure[entity.Key] = node; - - // We don't set the parent for items under root, it will stay null - if (entity.ParentId == -1) - { - continue; - } - - if (idToKeyMap.TryGetValue(entity.ParentId, out Guid parentKey) is false) - { - continue; - } - - // If the parent node exists in the nodesStructure, add the node to the parent's children (parent is set as well) - if (nodesStructure.TryGetValue(parentKey, out NavigationNode? parentNode)) - { - parentNode.AddChild(node); - } - } - } -} diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index d73528984a..223f0d9119 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -35,7 +35,6 @@ public class ContentService : RepositoryService, IContentService private readonly IShortStringHelper _shortStringHelper; private readonly ICultureImpactFactory _cultureImpactFactory; private readonly IUserIdKeyResolver _userIdKeyResolver; - private readonly IDocumentNavigationManagementService _documentNavigationManagementService; private readonly PropertyEditorCollection _propertyEditorCollection; private IQuery? _queryNotTrashed; @@ -55,7 +54,6 @@ public class ContentService : RepositoryService, IContentService IShortStringHelper shortStringHelper, ICultureImpactFactory cultureImpactFactory, IUserIdKeyResolver userIdKeyResolver, - IDocumentNavigationManagementService documentNavigationManagementService, PropertyEditorCollection propertyEditorCollection) : base(provider, loggerFactory, eventMessagesFactory) { @@ -69,7 +67,6 @@ public class ContentService : RepositoryService, IContentService _shortStringHelper = shortStringHelper; _cultureImpactFactory = cultureImpactFactory; _userIdKeyResolver = userIdKeyResolver; - _documentNavigationManagementService = documentNavigationManagementService; _propertyEditorCollection = propertyEditorCollection; _logger = loggerFactory.CreateLogger(); } @@ -103,7 +100,6 @@ public class ContentService : RepositoryService, IContentService shortStringHelper, cultureImpactFactory, userIdKeyResolver, - StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService()) { } @@ -136,7 +132,6 @@ public class ContentService : RepositoryService, IContentService shortStringHelper, cultureImpactFactory, StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService()) { } @@ -1063,18 +1058,6 @@ public class ContentService : RepositoryService, IContentService // have always changed if it's been saved in the back office but that's not really fail safe. _documentRepository.Save(content); - // Updates in-memory navigation structure - we only handle new items, other updates are not a concern - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.Save-with-contentSchedule", - () => - { - _documentNavigationManagementService.Add(content.Key, GetParent(content)?.Key); - if (content.Trashed) - { - _documentNavigationManagementService.MoveToBin(content.Key); - } - }); - if (contentSchedule != null) { _documentRepository.PersistContentSchedule(content, contentSchedule); @@ -1138,18 +1121,6 @@ public class ContentService : RepositoryService, IContentService content.WriterId = userId; _documentRepository.Save(content); - - // Updates in-memory navigation structure - we only handle new items, other updates are not a concern - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.Save", - () => - { - _documentNavigationManagementService.Add(content.Key, GetParent(content)?.Key); - if (content.Trashed) - { - _documentNavigationManagementService.MoveToBin(content.Key); - } - }); } scope.Notifications.Publish( @@ -2339,26 +2310,6 @@ public class ContentService : RepositoryService, IContentService } DoDelete(content); - - if (content.Trashed) - { - // Updates in-memory navigation structure for recycle bin items - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.DeleteLocked-trashed", - () => _documentNavigationManagementService.RemoveFromBin(content.Key)); - } - else - { - // Updates in-memory navigation structure for both documents and recycle bin items - // as the item needs to be deleted whether it is in the recycle bin or not - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.DeleteLocked", - () => - { - _documentNavigationManagementService.MoveToBin(content.Key); - _documentNavigationManagementService.RemoveFromBin(content.Key); - }); - } } // TODO: both DeleteVersions methods below have an issue. Sort of. They do NOT take care of files the way @@ -2583,8 +2534,6 @@ public class ContentService : RepositoryService, IContentService // trash indicates whether we are trashing, un-trashing, or not changing anything private void PerformMoveLocked(IContent content, int parentId, IContent? parent, int userId, ICollection<(IContent, string)> moves, bool? trash) { - // Needed to update the in-memory navigation structure - var cameFromRecycleBin = content.ParentId == Constants.System.RecycleBinContent; content.WriterId = userId; content.ParentId = parentId; @@ -2633,33 +2582,6 @@ public class ContentService : RepositoryService, IContentService } } while (total > pageSize); - - if (parentId == Constants.System.RecycleBinContent) - { - // Updates in-memory navigation structure for both document items and recycle bin items - // as we are moving to recycle bin - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.PerformMoveLocked-to-recycle-bin", - () => _documentNavigationManagementService.MoveToBin(content.Key)); - } - else - { - if (cameFromRecycleBin) - { - // Updates in-memory navigation structure for both document items and recycle bin items - // as we are restoring from recycle bin - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.PerformMoveLocked-restore", - () => _documentNavigationManagementService.RestoreFromBin(content.Key, parent?.Key)); - } - else - { - // Updates in-memory navigation structure - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.PerformMoveLocked", - () => _documentNavigationManagementService.Move(content.Key, parent?.Key)); - } - } } private void PerformMoveContentLocked(IContent content, int userId, bool? trash) @@ -2864,20 +2786,6 @@ public class ContentService : RepositoryService, IContentService } } - if (navigationUpdates.Count > 0) - { - // Updates in-memory navigation structure - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.Copy", - () => - { - foreach (Tuple update in navigationUpdates) - { - _documentNavigationManagementService.Add(update.Item1, update.Item2); - } - }); - } - // not handling tags here, because // - tags should be handled by the content repository // - a copy is unpublished and therefore has no impact on tags in DB @@ -3822,28 +3730,4 @@ public class ContentService : RepositoryService, IContentService #endregion - /// - /// Enlists an action in the current scope context to update the in-memory navigation structure - /// when the scope completes successfully. - /// - /// The unique key identifying the action to be enlisted. - /// The action to be performed for updating the in-memory navigation structure. - /// Thrown when the scope context is null and therefore cannot be used. - private void UpdateInMemoryNavigationStructure(string enlistingActionKey, Action updateNavigation) - { - IScopeContext? scopeContext = ScopeProvider.Context; - - if (scopeContext is null) - { - throw new NullReferenceException($"The {nameof(scopeContext)} is null and cannot be used."); - } - - scopeContext.Enlist(enlistingActionKey, completed => - { - if (completed) - { - updateNavigation(); - } - }); - } } diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index ae334447e9..93431618b2 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -29,7 +29,6 @@ namespace Umbraco.Cms.Core.Services private readonly IEntityRepository _entityRepository; private readonly IShortStringHelper _shortStringHelper; private readonly IUserIdKeyResolver _userIdKeyResolver; - private readonly IMediaNavigationManagementService _mediaNavigationManagementService; private readonly MediaFileManager _mediaFileManager; @@ -45,8 +44,7 @@ namespace Umbraco.Cms.Core.Services IMediaTypeRepository mediaTypeRepository, IEntityRepository entityRepository, IShortStringHelper shortStringHelper, - IUserIdKeyResolver userIdKeyResolver, - IMediaNavigationManagementService mediaNavigationManagementService) + IUserIdKeyResolver userIdKeyResolver) : base(provider, loggerFactory, eventMessagesFactory) { _mediaFileManager = mediaFileManager; @@ -56,36 +54,7 @@ namespace Umbraco.Cms.Core.Services _entityRepository = entityRepository; _shortStringHelper = shortStringHelper; _userIdKeyResolver = userIdKeyResolver; - _mediaNavigationManagementService = mediaNavigationManagementService; } - - [Obsolete("Use non-obsolete constructor. Scheduled for removal in V16.")] - public MediaService( - ICoreScopeProvider provider, - MediaFileManager mediaFileManager, - ILoggerFactory loggerFactory, - IEventMessagesFactory eventMessagesFactory, - IMediaRepository mediaRepository, - IAuditRepository auditRepository, - IMediaTypeRepository mediaTypeRepository, - IEntityRepository entityRepository, - IShortStringHelper shortStringHelper, - IUserIdKeyResolver userIdKeyResolver) - : this( - provider, - mediaFileManager, - loggerFactory, - eventMessagesFactory, - mediaRepository, - auditRepository, - mediaTypeRepository, - entityRepository, - shortStringHelper, - userIdKeyResolver, - StaticServiceProvider.Instance.GetRequiredService()) - { - } - [Obsolete("Use constructor that takes IUserIdKeyResolver as a parameter, scheduled for removal in V15")] public MediaService( ICoreScopeProvider provider, @@ -107,8 +76,7 @@ namespace Umbraco.Cms.Core.Services mediaTypeRepository, entityRepository, shortStringHelper, - StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService()) + StaticServiceProvider.Instance.GetRequiredService()) { } @@ -801,11 +769,6 @@ namespace Umbraco.Cms.Core.Services _mediaRepository.Save(media); - // Updates in-memory navigation structure - we only handle new items, other updates are not a concern - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.MediaService.Save", - () => _mediaNavigationManagementService.Add(media.Key, GetParent(media)?.Key)); - scope.Notifications.Publish(new MediaSavedNotification(media, eventMessages).WithStateFrom(savingNotification)); // TODO: See note about suppressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(media, TreeChangeTypes.RefreshNode, eventMessages)); @@ -847,11 +810,6 @@ namespace Umbraco.Cms.Core.Services } _mediaRepository.Save(media); - - // Updates in-memory navigation structure - we only handle new items, other updates are not a concern - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.ContentService.Save-collection", - () => _mediaNavigationManagementService.Add(media.Key, GetParent(media)?.Key)); } scope.Notifications.Publish(new MediaSavedNotification(mediasA, messages).WithStateFrom(savingNotification)); @@ -923,26 +881,6 @@ namespace Umbraco.Cms.Core.Services } DoDelete(media); - - if (media.Trashed) - { - // Updates in-memory navigation structure for recycle bin items - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.MediaService.DeleteLocked-trashed", - () => _mediaNavigationManagementService.RemoveFromBin(media.Key)); - } - else - { - // Updates in-memory navigation structure for both media and recycle bin items - // as the item needs to be deleted whether it is in the recycle bin or not - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.MediaService.DeleteLocked", - () => - { - _mediaNavigationManagementService.MoveToBin(media.Key); - _mediaNavigationManagementService.RemoveFromBin(media.Key); - }); - } } //TODO: both DeleteVersions methods below have an issue. Sort of. They do NOT take care of files the way @@ -1177,33 +1115,6 @@ namespace Umbraco.Cms.Core.Services } while (total > pageSize); - - if (parentId == Constants.System.RecycleBinMedia) - { - // Updates in-memory navigation structure for both media items and recycle bin items - // as we are moving to recycle bin - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.MediaService.PerformMoveLocked-to-recycle-bin", - () => _mediaNavigationManagementService.MoveToBin(media.Key)); - } - else - { - if (cameFromRecycleBin) - { - // Updates in-memory navigation structure for both media items and recycle bin items - // as we are restoring from recycle bin - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.MediaService.PerformMoveLocked-restore", - () => _mediaNavigationManagementService.RestoreFromBin(media.Key, parent?.Key)); - } - else - { - // Updates in-memory navigation structure - UpdateInMemoryNavigationStructure( - "Umbraco.Cms.Core.Services.MediaService.PerformMoveLocked", - () => _mediaNavigationManagementService.Move(media.Key, parent?.Key)); - } - } } private void PerformMoveMediaLocked(IMedia media, bool? trash) @@ -1511,29 +1422,5 @@ namespace Umbraco.Cms.Core.Services #endregion - /// - /// Enlists an action in the current scope context to update the in-memory navigation structure - /// when the scope completes successfully. - /// - /// The unique key identifying the action to be enlisted. - /// The action to be performed for updating the in-memory navigation structure. - /// Thrown when the scope context is null and therefore cannot be used. - private void UpdateInMemoryNavigationStructure(string enlistingActionKey, Action updateNavigation) - { - IScopeContext? scopeContext = ScopeProvider.Context; - - if (scopeContext is null) - { - throw new NullReferenceException($"The {nameof(scopeContext)} is null and cannot be used."); - } - - scopeContext.Enlist(enlistingActionKey, completed => - { - if (completed) - { - updateNavigation(); - } - }); - } } } diff --git a/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs b/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs index 4f2d1e1c9b..fb6c7a0381 100644 --- a/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs +++ b/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs @@ -13,6 +13,8 @@ internal abstract class ContentNavigationServiceBase private readonly INavigationRepository _navigationRepository; private ConcurrentDictionary _navigationStructure = new(); private ConcurrentDictionary _recycleBinNavigationStructure = new(); + private IList _roots = new List(); + private IList _recycleBinRoots = new List(); protected ContentNavigationServiceBase(ICoreScopeProvider coreScopeProvider, INavigationRepository navigationRepository) { @@ -34,7 +36,7 @@ internal abstract class ContentNavigationServiceBase => TryGetParentKeyFromStructure(_navigationStructure, childKey, out parentKey); public bool TryGetRootKeys(out IEnumerable rootKeys) - => TryGetRootKeysFromStructure(_navigationStructure, out rootKeys); + => TryGetRootKeysFromStructure(_roots, out rootKeys); public bool TryGetChildrenKeys(Guid parentKey, out IEnumerable childrenKeys) => TryGetChildrenKeysFromStructure(_navigationStructure, parentKey, out childrenKeys); @@ -87,6 +89,10 @@ internal abstract class ContentNavigationServiceBase return false; // Parent node doesn't exist } } + else + { + _roots.Add(key); + } var newNode = new NavigationNode(key); if (_navigationStructure.TryAdd(key, newNode) is false) @@ -111,10 +117,19 @@ internal abstract class ContentNavigationServiceBase return false; // Cannot move a node to itself } + _roots.Remove(key); // Just in case + NavigationNode? targetParentNode = null; - if (targetParentKey.HasValue && _navigationStructure.TryGetValue(targetParentKey.Value, out targetParentNode) is false) + if (targetParentKey.HasValue) { - return false; // Target parent doesn't exist + if (_navigationStructure.TryGetValue(targetParentKey.Value, out targetParentNode) is false) + { + return false; // Target parent doesn't exist + } + } + else + { + _roots.Add(key); } // Remove the node from its current parent's children list @@ -136,6 +151,8 @@ internal abstract class ContentNavigationServiceBase return false; // Node doesn't exist } + _recycleBinRoots.Remove(key); + RemoveDescendantsRecursively(nodeToRemove); return _recycleBinNavigationStructure.TryRemove(key, out _); @@ -158,6 +175,7 @@ internal abstract class ContentNavigationServiceBase // Set the new parent for the node (if parent node is null - the node is moved to root) targetParentNode?.AddChild(nodeToRestore); + // Restore the node and its descendants from the recycle bin to the main structure RestoreNodeAndDescendantsRecursively(nodeToRestore); @@ -187,12 +205,12 @@ internal abstract class ContentNavigationServiceBase if (trashed) { IEnumerable navigationModels = _navigationRepository.GetTrashedContentNodesByObjectType(objectTypeKey); - NavigationFactory.BuildNavigationDictionary(_recycleBinNavigationStructure, navigationModels); + BuildNavigationDictionary(_recycleBinNavigationStructure, _recycleBinRoots, navigationModels); } else { IEnumerable navigationModels = _navigationRepository.GetContentNodesByObjectType(objectTypeKey); - NavigationFactory.BuildNavigationDictionary(_navigationStructure, navigationModels); + BuildNavigationDictionary(_navigationStructure, _roots, navigationModels); } } @@ -209,10 +227,11 @@ internal abstract class ContentNavigationServiceBase return false; } - private bool TryGetRootKeysFromStructure(ConcurrentDictionary structure, out IEnumerable rootKeys) + private bool TryGetRootKeysFromStructure(IList input, out IEnumerable rootKeys) { // TODO can we make this more efficient? - rootKeys = structure.Values.Where(x => x.Parent is null).Select(x => x.Key); + rootKeys = input.ToArray(); + return true; } @@ -323,6 +342,9 @@ internal abstract class ContentNavigationServiceBase private void AddDescendantsToRecycleBinRecursively(NavigationNode node) { + _recycleBinRoots.Add(node.Key); + _roots.Remove(node.Key); + foreach (NavigationNode child in node.Children) { AddDescendantsToRecycleBinRecursively(child); @@ -346,6 +368,12 @@ internal abstract class ContentNavigationServiceBase private void RestoreNodeAndDescendantsRecursively(NavigationNode node) { + if (node.Parent is null) + { + _roots.Add(node.Key); + } + _recycleBinRoots.Remove(node.Key); + foreach (NavigationNode child in node.Children) { RestoreNodeAndDescendantsRecursively(child); @@ -357,4 +385,34 @@ internal abstract class ContentNavigationServiceBase } } } + + private static void BuildNavigationDictionary(ConcurrentDictionary nodesStructure, IList roots, IEnumerable entities) + { + var entityList = entities.ToList(); + IDictionary idToKeyMap = entityList.ToDictionary(x => x.Id, x => x.Key); + + foreach (INavigationModel entity in entityList) + { + var node = new NavigationNode(entity.Key); + nodesStructure[entity.Key] = node; + + // We don't set the parent for items under root, it will stay null + if (entity.ParentId == -1) + { + roots.Add(entity.Key); + continue; + } + + if (idToKeyMap.TryGetValue(entity.ParentId, out Guid parentKey) is false) + { + continue; + } + + // If the parent node exists in the nodesStructure, add the node to the parent's children (parent is set as well) + if (nodesStructure.TryGetValue(parentKey, out NavigationNode? parentNode)) + { + parentNode.AddChild(node); + } + } + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTestsBase.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTestsBase.cs index d4325f4674..375ddb8391 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTestsBase.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTestsBase.cs @@ -1,11 +1,16 @@ +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.Navigation; +using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; +using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping; namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; @@ -48,4 +53,10 @@ public abstract class DocumentNavigationServiceTestsBase : UmbracoIntegrationTes InvariantName = name, Key = key, }; + + protected override void CustomTestSetup(IUmbracoBuilder builder) + { + builder.Services.AddUnique(); + builder.AddNotificationHandler(); + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTestsBase.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTestsBase.cs index 6d9e693320..6952823b9c 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTestsBase.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTestsBase.cs @@ -1,11 +1,15 @@ using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.Navigation; +using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; +using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping; namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; @@ -48,4 +52,10 @@ public abstract class MediaNavigationServiceTestsBase : UmbracoIntegrationTest InvariantName = name, Key = key, }; + + protected override void CustomTestSetup(IUmbracoBuilder builder) + { + builder.Services.AddUnique(); + builder.AddNotificationHandler(); + } }