diff --git a/src/Umbraco.Web/Routing/RedirectTrackingComponent.cs b/src/Umbraco.Web/Routing/RedirectTrackingComponent.cs index 77aa3c65a1..fcf6f55f44 100644 --- a/src/Umbraco.Web/Routing/RedirectTrackingComponent.cs +++ b/src/Umbraco.Web/Routing/RedirectTrackingComponent.cs @@ -1,16 +1,13 @@ using System; using System.Collections.Generic; using System.Linq; -using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Events; using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; -using Umbraco.Core.Sync; -using Umbraco.Web.Cache; -using Current = Umbraco.Web.Composing.Current; +using Umbraco.Web.PublishedCache; namespace Umbraco.Web.Routing { @@ -23,64 +20,17 @@ namespace Umbraco.Web.Routing /// recycle bin = moving to and from does nothing: to = the node is gone, where would we redirect? from = same public sealed class RedirectTrackingComponent : IComponent { - private const string ContextKey1 = "Umbraco.Web.Redirects.RedirectTrackingEventHandler.1"; - private const string ContextKey2 = "Umbraco.Web.Redirects.RedirectTrackingEventHandler.2"; - private const string ContextKey3 = "Umbraco.Web.Redirects.RedirectTrackingEventHandler.3"; + private const string _eventStateKey = "Umbraco.Web.Redirects.RedirectTrackingEventHandler"; private readonly IUmbracoSettingsSection _umbracoSettings; + private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; + private readonly IRedirectUrlService _redirectUrlService; - public RedirectTrackingComponent(IUmbracoSettingsSection umbracoSettings) + public RedirectTrackingComponent(IUmbracoSettingsSection umbracoSettings, IPublishedSnapshotAccessor publishedSnapshotAccessor, IRedirectUrlService redirectUrlService) { _umbracoSettings = umbracoSettings; - } - - private static Dictionary OldRoutes - { - get - { - var oldRoutes = (Dictionary) Current.UmbracoContext.HttpContext.Items[ContextKey3]; - if (oldRoutes == null) - Current.UmbracoContext.HttpContext.Items[ContextKey3] = oldRoutes = new Dictionary(); - return oldRoutes; - } - } - - private static bool HasOldRoutes - { - get - { - if (Current.UmbracoContext == null) return false; - if (Current.UmbracoContext.HttpContext == null) return false; - if (Current.UmbracoContext.HttpContext.Items[ContextKey3] == null) return false; - return true; - } - } - - private static bool LockedEvents - { - get => Moving && Current.UmbracoContext.HttpContext.Items[ContextKey2] != null; - set - { - if (Moving && value) - Current.UmbracoContext.HttpContext.Items[ContextKey2] = true; - else - Current.UmbracoContext.HttpContext.Items.Remove(ContextKey2); - } - } - - private static bool Moving - { - get => Current.UmbracoContext.HttpContext.Items[ContextKey1] != null; - set - { - if (value) - Current.UmbracoContext.HttpContext.Items[ContextKey1] = true; - else - { - Current.UmbracoContext.HttpContext.Items.Remove(ContextKey1); - Current.UmbracoContext.HttpContext.Items.Remove(ContextKey2); - } - } + _publishedSnapshotAccessor = publishedSnapshotAccessor; + _redirectUrlService = redirectUrlService; } public void Initialize() @@ -88,26 +38,11 @@ namespace Umbraco.Web.Routing // don't let the event handlers kick in if Redirect Tracking is turned off in the config if (_umbracoSettings.WebRouting.DisableRedirectUrlTracking) return; - // events are weird - // on 'published' we 'could' get the old or the new route depending on event handlers order - // so it is not reliable. getting the old route in 'publishing' to be sure and storing in http - // context. then for the same reason, we have to process these old items only when the cache - // is ready - // when moving, the moved node is also published, which is causing all sorts of troubles with - // descendants, so when moving, we lock events so that neither 'published' nor 'publishing' - // are processed more than once - // we cannot rely only on ContentCacheRefresher because when CacheUpdated triggers the old - // route is gone - // - // this is all very weird but it seems to work - ContentService.Publishing += ContentService_Publishing; ContentService.Published += ContentService_Published; ContentService.Moving += ContentService_Moving; ContentService.Moved += ContentService_Moved; - ContentCacheRefresher.CacheUpdated += ContentCacheRefresher_CacheUpdated; - // kill all redirects once a content is deleted //ContentService.Deleted += ContentService_Deleted; // BUT, doing it here would prevent content deletion due to FK @@ -119,102 +54,79 @@ namespace Umbraco.Web.Routing public void Terminate() { } - private static void ContentCacheRefresher_CacheUpdated(ContentCacheRefresher sender, CacheRefresherEventArgs args) + private void ContentService_Publishing(IContentService sender, PublishEventArgs args) { - // that event is a distributed even that triggers on all nodes - // BUT it should totally NOT run on nodes other that the one that handled the other events - // and besides, it cannot run on a background thread! - if (!HasOldRoutes) - return; - - // sanity checks - if (args.MessageType != MessageType.RefreshByPayload) - { - throw new InvalidOperationException("ContentCacheRefresher MessageType should be ByPayload."); - } - - if (args.MessageObject == null) - { - return; - } - - if (!(args.MessageObject is ContentCacheRefresher.JsonPayload[])) - { - throw new InvalidOperationException("ContentCacheRefresher MessageObject should be JsonPayload[]."); - } - - // manage routes - var removeKeys = new List(); - - foreach (var oldRoute in OldRoutes) - { - // assuming we cannot have 'CacheUpdated' for only part of the infos else we'd need - // to set a flag in 'Published' to indicate which entities have been refreshed ok - CreateRedirect(oldRoute.Key.ContentId, oldRoute.Key.Culture, oldRoute.Value.ContentKey, oldRoute.Value.OldRoute); - removeKeys.Add(oldRoute.Key); - } - - foreach (var k in removeKeys) - { - OldRoutes.Remove(k); - } - } - - private static void ContentService_Publishing(IContentService sender, PublishEventArgs args) - { - if (LockedEvents) return; - - var contentCache = Current.UmbracoContext.Content; + var oldRoutes = GetOldRoutes(args.EventState); foreach (var entity in args.PublishedEntities) { - var entityContent = contentCache.GetById(entity.Id); - if (entityContent == null) continue; + StoreOldRoute(entity, oldRoutes); + } + } - // get the default affected cultures by going up the tree until we find the first culture variant entity (default to no cultures) - var defaultCultures = entityContent.AncestorsOrSelf()?.FirstOrDefault(a => a.Cultures.Any())?.Cultures.Keys.ToArray() - ?? new[] {(string) null}; - foreach (var x in entityContent.DescendantsOrSelf()) - { - // if this entity defines specific cultures, use those instead of the default ones - var cultures = x.Cultures.Any() ? x.Cultures.Keys : defaultCultures; + private void ContentService_Published(IContentService sender, ContentPublishedEventArgs args) + { + var oldRoutes = GetOldRoutes(args.EventState); + CreateRedirects(oldRoutes); + } - foreach (var culture in cultures) - { - var route = contentCache.GetRouteById(x.Id, culture); - if (IsNotRoute(route)) return; - OldRoutes[new ContentIdAndCulture(x.Id, culture)] = new ContentKeyAndOldRoute(x.Key, route); - } - } + private void ContentService_Moving(IContentService sender, MoveEventArgs args) + { + var oldRoutes = GetOldRoutes(args.EventState); + foreach (var info in args.MoveInfoCollection) + { + StoreOldRoute(info.Entity, oldRoutes); + } + } + + private void ContentService_Moved(IContentService sender, MoveEventArgs args) + { + var oldRoutes = GetOldRoutes(args.EventState); + CreateRedirects(oldRoutes); + } + + private OldRoutesDictionary GetOldRoutes(IDictionary eventState) + { + if (! eventState.ContainsKey(_eventStateKey)) + { + eventState[_eventStateKey] = new OldRoutesDictionary(); } - LockedEvents = true; // we only want to see the "first batch" + return eventState[_eventStateKey] as OldRoutesDictionary; } - private static void ContentService_Published(IContentService sender, PublishEventArgs e) + private void StoreOldRoute(IContent entity, OldRoutesDictionary oldRoutes) { - // look note in CacheUpdated - // we might want to set a flag on the entities we are seeing here + var contentCache = _publishedSnapshotAccessor.PublishedSnapshot.Content; + var entityContent = contentCache.GetById(entity.Id); + if (entityContent == null) return; + + // get the default affected cultures by going up the tree until we find the first culture variant entity (default to no cultures) + var defaultCultures = entityContent.AncestorsOrSelf()?.FirstOrDefault(a => a.Cultures.Any())?.Cultures.Keys.ToArray() + ?? new[] { (string)null }; + foreach (var x in entityContent.DescendantsOrSelf()) + { + // if this entity defines specific cultures, use those instead of the default ones + var cultures = x.Cultures.Any() ? x.Cultures.Keys : defaultCultures; + + foreach (var culture in cultures) + { + var route = contentCache.GetRouteById(x.Id, culture); + if (IsNotRoute(route)) continue; + oldRoutes[new ContentIdAndCulture(x.Id, culture)] = new ContentKeyAndOldRoute(x.Key, route); + } + } } - private static void ContentService_Moving(IContentService sender, MoveEventArgs e) + private void CreateRedirects(OldRoutesDictionary oldRoutes) { - // TODO: Use the new e.EventState to track state between Moving/Moved events! - Moving = true; - } + var contentCache = _publishedSnapshotAccessor.PublishedSnapshot.Content; - private static void ContentService_Moved(IContentService sender, MoveEventArgs e) - { - Moving = false; - LockedEvents = false; - } - - private static void CreateRedirect(int contentId, string culture, Guid contentKey, string oldRoute) - { - var contentCache = Current.UmbracoContext.Content; - var newRoute = contentCache.GetRouteById(contentId, culture); - if (IsNotRoute(newRoute) || oldRoute == newRoute) return; - var redirectUrlService = Current.Services.RedirectUrlService; - redirectUrlService.Register(oldRoute, contentKey, culture); + foreach (var oldRoute in oldRoutes) + { + var newRoute = contentCache.GetRouteById(oldRoute.Key.ContentId, oldRoute.Key.Culture); + if (IsNotRoute(newRoute) || oldRoute.Value.OldRoute == newRoute) continue; + _redirectUrlService.Register(oldRoute.Value.OldRoute, oldRoute.Value.ContentKey, oldRoute.Key.Culture); + } } private static bool IsNotRoute(string route) @@ -243,5 +155,8 @@ namespace Umbraco.Web.Routing public Guid ContentKey => Item1; public string OldRoute => Item2; } + + private class OldRoutesDictionary : Dictionary + { } } }