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 <elm@umbraco.dk>
Co-authored-by: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com>

src/Umbraco.Core/Cache/Refreshers/Implement/ContentCacheRefresher.cs

@@ -19,6 +19,8 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
     private readonly IDomainService _domainService;
     private readonly IDocumentUrlService _documentUrlService;
     private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
+    private readonly IDocumentNavigationManagementService _documentNavigationManagementService;
+    private readonly IContentService _contentService;
     private readonly IIdKeyMap _idKeyMap;
     private readonly IPublishedSnapshotService _publishedSnapshotService;
 
@@ -40,7 +42,9 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
             eventAggregator,
             factory,
             StaticServiceProvider.Instance.GetRequiredService<IDocumentUrlService>(),
-            StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationQueryService>()
+            StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationQueryService>(),
+            StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationManagementService>(),
+            StaticServiceProvider.Instance.GetRequiredService<IContentService>()
             )
     {
 
@@ -55,7 +59,9 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
         IEventAggregator eventAggregator,
         ICacheRefresherNotificationFactory factory,
         IDocumentUrlService documentUrlService,
-        IDocumentNavigationQueryService documentNavigationQueryService)
+        IDocumentNavigationQueryService documentNavigationQueryService,
+        IDocumentNavigationManagementService documentNavigationManagementService,
+        IContentService contentService)
         : base(appCaches, serializer, eventAggregator, factory)
     {
         _publishedSnapshotService = publishedSnapshotService;
@@ -63,6 +69,8 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
         _domainService = domainService;
         _documentUrlService = documentUrlService;
         _documentNavigationQueryService = documentNavigationQueryService;
+        _documentNavigationManagementService = documentNavigationManagementService;
+        _contentService = contentService;
     }
 
     #region Indirect
@@ -126,6 +134,7 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
 
             HandleRouting(payload);
 
+            HandleNavigation(payload);
             _idKeyMap.ClearCache(payload.Id);
             if (payload.Key.HasValue)
             {
@@ -172,6 +181,102 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase<ContentCac
         base.Refresh(payloads);
     }
 
+    private void HandleNavigation(JsonPayload payload)
+    {
+        if (payload.Key is null)
+        {
+            return;
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.Remove))
+        {
+            _documentNavigationManagementService.MoveToBin(payload.Key.Value);
+            _documentNavigationManagementService.RemoveFromBin(payload.Key.Value);
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll))
+        {
+            _documentNavigationManagementService.RebuildAsync();
+            _documentNavigationManagementService.RebuildBinAsync();
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshNode))
+        {
+            IContent? content = _contentService.GetById(payload.Id);
+
+            if (content is null)
+            {
+                return;
+            }
+
+            HandleNavigationForSingleContent(content);
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch))
+        {
+            IContent? content = _contentService.GetById(payload.Id);
+
+            if (content is null)
+            {
+                return;
+            }
+
+            IEnumerable<IContent> 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))

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<MediaCacheRefresherNotification, MediaCacheRefresher.JsonPayload>
 {
     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<MediaCacheRe
         IPublishedSnapshotService publishedSnapshotService,
         IIdKeyMap idKeyMap,
         IEventAggregator eventAggregator,
-        ICacheRefresherNotificationFactory factory)
+        ICacheRefresherNotificationFactory factory,
+        IMediaNavigationQueryService mediaNavigationQueryService,
+        IMediaNavigationManagementService mediaNavigationManagementService,
+        IMediaService mediaService)
         : base(appCaches, serializer, eventAggregator, factory)
     {
         _publishedSnapshotService = publishedSnapshotService;
         _idKeyMap = idKeyMap;
+        _mediaNavigationQueryService = mediaNavigationQueryService;
+        _mediaNavigationManagementService = mediaNavigationManagementService;
+        _mediaService = mediaService;
     }
 
     #region Indirect
@@ -84,23 +94,23 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase<MediaCacheRe
                 _idKeyMap.ClearCache(payload.Id);
             }
 
-            if (!mediaCache.Success)
+            if (mediaCache.Success)
             {
-                continue;
+                // 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<IMedia, int>(payload.Id));
+                mediaCache.Result?.Clear(RepositoryCacheKeys.GetKey<IMedia, Guid?>(payload.Key));
+
+                // remove those that are in the branch
+                if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.RefreshBranch | TreeChangeTypes.Remove))
+                {
+                    var pathid = "," + payload.Id + ",";
+                    mediaCache.Result?.ClearOfType<IMedia>((_, 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<IMedia, int>(payload.Id));
-            mediaCache.Result?.Clear(RepositoryCacheKeys.GetKey<IMedia, Guid?>(payload.Key));
-
-            // remove those that are in the branch
-            if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.RefreshBranch | TreeChangeTypes.Remove))
-            {
-                var pathid = "," + payload.Id + ",";
-                mediaCache.Result?.ClearOfType<IMedia>((_, v) => v.Path?.Contains(pathid) ?? false);
-            }
+            HandleNavigation(payload);
         }
 
         _publishedSnapshotService.Notify(payloads, out var hasPublishedDataChanged);
@@ -110,9 +120,107 @@ public sealed class MediaCacheRefresher : PayloadCacheRefresherBase<MediaCacheRe
             AppCaches.ClearPartialViewCache();
         }
 
+
         base.Refresh(payloads);
     }
 
+    private void HandleNavigation(JsonPayload payload)
+    {
+        if (payload.Key is null)
+        {
+            return;
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.Remove))
+        {
+            _mediaNavigationManagementService.MoveToBin(payload.Key.Value);
+            _mediaNavigationManagementService.RemoveFromBin(payload.Key.Value);
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll))
+        {
+            _mediaNavigationManagementService.RebuildAsync();
+            _mediaNavigationManagementService.RebuildBinAsync();
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshNode))
+        {
+            IMedia? media = _mediaService.GetById(payload.Id);
+
+            if (media is null)
+            {
+                return;
+            }
+
+            HandleNavigationForSingleMedia(media);
+        }
+
+        if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch))
+        {
+            IMedia? media = _mediaService.GetById(payload.Id);
+
+            if (media is null)
+            {
+                return;
+            }
+
+            IEnumerable<IMedia> 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();

src/Umbraco.Core/Factories/NavigationFactory.cs

@@ -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
-{
-    /// <summary>
-    ///     Builds a dictionary of NavigationNode objects from a given dataset.
-    /// </summary>
-    /// <param name="nodesStructure">A dictionary of <see cref="NavigationNode" /> objects with key corresponding to their unique Guid.</param>
-    /// <param name="entities">The <see cref="INavigationModel" /> objects used to build the navigation nodes dictionary.</param>
-    public static void BuildNavigationDictionary(ConcurrentDictionary<Guid, NavigationNode> nodesStructure, IEnumerable<INavigationModel> entities)
-    {
-        var entityList = entities.ToList();
-        Dictionary<int, Guid> 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);
-            }
-        }
-    }
-}

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<IContent>? _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<ContentService>();
     }
@@ -103,7 +100,6 @@ public class ContentService : RepositoryService, IContentService
             shortStringHelper,
             cultureImpactFactory,
             userIdKeyResolver,
-            StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationManagementService>(),
             StaticServiceProvider.Instance.GetRequiredService<PropertyEditorCollection>())
     {
     }
@@ -136,7 +132,6 @@ public class ContentService : RepositoryService, IContentService
             shortStringHelper,
             cultureImpactFactory,
             StaticServiceProvider.Instance.GetRequiredService<IUserIdKeyResolver>(),
-            StaticServiceProvider.Instance.GetRequiredService<IDocumentNavigationManagementService>(),
             StaticServiceProvider.Instance.GetRequiredService<PropertyEditorCollection>())
     {
     }
@@ -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<Guid, Guid?> 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
 
-    /// <summary>
-    ///     Enlists an action in the current scope context to update the in-memory navigation structure
-    ///     when the scope completes successfully.
-    /// </summary>
-    /// <param name="enlistingActionKey">The unique key identifying the action to be enlisted.</param>
-    /// <param name="updateNavigation">The action to be performed for updating the in-memory navigation structure.</param>
-    /// <exception cref="NullReferenceException">Thrown when the scope context is null and therefore cannot be used.</exception>
-    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();
-            }
-        });
-    }
 }

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<IMediaNavigationManagementService>())
-        {
-        }
-
         [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<IUserIdKeyResolver>(),
-                StaticServiceProvider.Instance.GetRequiredService<IMediaNavigationManagementService>())
+                StaticServiceProvider.Instance.GetRequiredService<IUserIdKeyResolver>())
         {
         }
 
@@ -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
 
-        /// <summary>
-        ///     Enlists an action in the current scope context to update the in-memory navigation structure
-        ///     when the scope completes successfully.
-        /// </summary>
-        /// <param name="enlistingActionKey">The unique key identifying the action to be enlisted.</param>
-        /// <param name="updateNavigation">The action to be performed for updating the in-memory navigation structure.</param>
-        /// <exception cref="NullReferenceException">Thrown when the scope context is null and therefore cannot be used.</exception>
-        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();
-                }
-            });
-        }
     }
 }

src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs

@@ -13,6 +13,8 @@ internal abstract class ContentNavigationServiceBase
     private readonly INavigationRepository _navigationRepository;
     private ConcurrentDictionary<Guid, NavigationNode> _navigationStructure = new();
     private ConcurrentDictionary<Guid, NavigationNode> _recycleBinNavigationStructure = new();
+    private IList<Guid> _roots = new List<Guid>();
+    private IList<Guid> _recycleBinRoots = new List<Guid>();
 
     protected ContentNavigationServiceBase(ICoreScopeProvider coreScopeProvider, INavigationRepository navigationRepository)
     {
@@ -34,7 +36,7 @@ internal abstract class ContentNavigationServiceBase
         => TryGetParentKeyFromStructure(_navigationStructure, childKey, out parentKey);
 
     public bool TryGetRootKeys(out IEnumerable<Guid> rootKeys)
-        => TryGetRootKeysFromStructure(_navigationStructure, out rootKeys);
+        => TryGetRootKeysFromStructure(_roots, out rootKeys);
 
     public bool TryGetChildrenKeys(Guid parentKey, out IEnumerable<Guid> 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<INavigationModel> navigationModels = _navigationRepository.GetTrashedContentNodesByObjectType(objectTypeKey);
-            NavigationFactory.BuildNavigationDictionary(_recycleBinNavigationStructure, navigationModels);
+            BuildNavigationDictionary(_recycleBinNavigationStructure, _recycleBinRoots,  navigationModels);
         }
         else
         {
             IEnumerable<INavigationModel> 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<Guid, NavigationNode> structure, out IEnumerable<Guid> rootKeys)
+    private bool TryGetRootKeysFromStructure(IList<Guid> input, out IEnumerable<Guid> 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<Guid, NavigationNode> nodesStructure, IList<Guid> roots, IEnumerable<INavigationModel> entities)
+    {
+        var entityList = entities.ToList();
+        IDictionary<int, Guid> 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);
+            }
+        }
+    }
 }

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<IServerMessenger, ScopedRepositoryTests.LocalServerMessenger>();
+        builder.AddNotificationHandler<ContentTreeChangeNotification, ContentTreeChangeDistributedCacheNotificationHandler>();
+    }
 }

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<IServerMessenger, ScopedRepositoryTests.LocalServerMessenger>();
+        builder.AddNotificationHandler<MediaTreeChangeNotification, MediaTreeChangeDistributedCacheNotificationHandler>();
+    }
 }