From 816107a348215b19eac85cbe3397da62b92a23b9 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Aug 2016 18:55:48 +0200 Subject: [PATCH 1/3] U4-8813 RedirectTrackingEventhandler always iterates over all descendants fixes SHA1 hashing (moves to a ext method), fixes col length for new db for the redirect table, updates migration len to 40 to store the correct hash format, don't iterate descendents if there's nothing changed, fixes issue with RedirectTrackingEventHandler with regards to operating outside of a web request (i.e. console app) --- .../Models/Rdbms/RedirectUrlDto.cs | 2 +- .../AddRedirectUrlTable.cs | 2 +- .../Repositories/RedirectUrlRepository.cs | 18 +-- src/Umbraco.Core/StringExtensions.cs | 30 ++++ .../RedirectTrackingEventHandler.cs | 153 ++++++++++++------ src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 6 files changed, 145 insertions(+), 62 deletions(-) rename src/Umbraco.Web/{Redirects => Routing}/RedirectTrackingEventHandler.cs (59%) diff --git a/src/Umbraco.Core/Models/Rdbms/RedirectUrlDto.cs b/src/Umbraco.Core/Models/Rdbms/RedirectUrlDto.cs index ed720e8bc1..ca65e4b9a1 100644 --- a/src/Umbraco.Core/Models/Rdbms/RedirectUrlDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/RedirectUrlDto.cs @@ -39,12 +39,12 @@ namespace Umbraco.Core.Models.Rdbms [Column("url")] [NullSetting(NullSetting = NullSettings.NotNull)] - //[Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoRedirectUrl", ForColumns = "url, createDateUtc")] public string Url { get; set; } [Column("urlHash")] [NullSetting(NullSetting = NullSettings.NotNull)] [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoRedirectUrl", ForColumns = "urlHash, contentKey, createDateUtc")] + [Length(40)] public string UrlHash { get; set; } } } diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/AddRedirectUrlTable.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/AddRedirectUrlTable.cs index ce6af9186c..508c0f284b 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/AddRedirectUrlTable.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/AddRedirectUrlTable.cs @@ -39,7 +39,7 @@ namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFiveZer .WithColumn("createDateUtc").AsDateTime().NotNullable() .WithColumn("url").AsString(2048).NotNullable() .WithColumn("contentKey").AsGuid().NotNullable() - .WithColumn("urlHash").AsString(20).NotNullable(); + .WithColumn("urlHash").AsString(40).NotNullable(); localContext.Create.Index("IX_" + umbracoRedirectUrlTableName).OnTable(umbracoRedirectUrlTableName) .OnColumn("urlHash") diff --git a/src/Umbraco.Core/Persistence/Repositories/RedirectUrlRepository.cs b/src/Umbraco.Core/Persistence/Repositories/RedirectUrlRepository.cs index 2c608592b8..7073aae560 100644 --- a/src/Umbraco.Core/Persistence/Repositories/RedirectUrlRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/RedirectUrlRepository.cs @@ -105,7 +105,7 @@ JOIN umbracoNode ON umbracoRedirectUrl.contentKey=umbracoNode.uniqueID"); ContentKey = redirectUrl.ContentKey, CreateDateUtc = redirectUrl.CreateDateUtc, Url = redirectUrl.Url, - UrlHash = HashUrl(redirectUrl.Url) + UrlHash = redirectUrl.Url.ToSHA1() }; } @@ -132,7 +132,7 @@ JOIN umbracoNode ON umbracoRedirectUrl.contentKey=umbracoNode.uniqueID"); public IRedirectUrl Get(string url, Guid contentKey) { - var urlHash = HashUrl(url); + var urlHash = url.ToSHA1(); var sql = GetBaseQuery(false).Where(x => x.Url == url && x.UrlHash == urlHash && x.ContentKey == contentKey); var dto = Database.Fetch(sql).FirstOrDefault(); return dto == null ? null : Map(dto); @@ -155,7 +155,7 @@ JOIN umbracoNode ON umbracoRedirectUrl.contentKey=umbracoNode.uniqueID"); public IRedirectUrl GetMostRecentUrl(string url) { - var urlHash = HashUrl(url); + var urlHash = url.ToSHA1(); var sql = GetBaseQuery(false) .Where(x => x.Url == url && x.UrlHash == urlHash) .OrderByDescending(x => x.CreateDateUtc, SqlSyntax); @@ -192,16 +192,6 @@ JOIN umbracoNode ON umbracoRedirectUrl.contentKey=umbracoNode.uniqueID"); var rules = result.Items.Select(Map); return rules; - } - - private static string HashUrl(string url) - { - using (var crypto = new SHA1CryptoServiceProvider()) - { - var inputBytes = Encoding.UTF8.GetBytes(url); - var hashedBytes = crypto.ComputeHash(inputBytes); - return Encoding.UTF8.GetString(hashedBytes); - } - } + } } } diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index b92df5f1cf..6600092421 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -750,6 +750,36 @@ namespace Umbraco.Core return stringBuilder.ToString(); } + /// + /// Converts the string to SHA1 + /// + /// referrs to itself + /// the md5 hashed string + public static string ToSHA1(this string stringToConvert) + { + //create an instance of the SHA1CryptoServiceProvider + var md5Provider = new SHA1CryptoServiceProvider(); + + //convert our string into byte array + var byteArray = Encoding.UTF8.GetBytes(stringToConvert); + + //get the hashed values created by our SHA1CryptoServiceProvider + var hashedByteArray = md5Provider.ComputeHash(byteArray); + + //create a StringBuilder object + var stringBuilder = new StringBuilder(); + + //loop to each each byte + foreach (var b in hashedByteArray) + { + //append it to our StringBuilder + stringBuilder.Append(b.ToString("x2").ToLower()); + } + + //return the hashed value + return stringBuilder.ToString(); + } + /// /// Decodes a string that was encoded with UrlTokenEncode diff --git a/src/Umbraco.Web/Redirects/RedirectTrackingEventHandler.cs b/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs similarity index 59% rename from src/Umbraco.Web/Redirects/RedirectTrackingEventHandler.cs rename to src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs index 417238bdbe..65cad094dc 100644 --- a/src/Umbraco.Web/Redirects/RedirectTrackingEventHandler.cs +++ b/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs @@ -1,18 +1,19 @@ using System; -using Umbraco.Core; -using Umbraco.Core.Models; -using Umbraco.Core.Services; -using Umbraco.Core.Publishing; -using Umbraco.Core.Events; -using Umbraco.Web.Routing; using System.Collections.Generic; using System.Linq; +using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; +using Umbraco.Core.Events; +using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Core.Publishing; +using Umbraco.Core.Services; +using Umbraco.Core.Sync; using Umbraco.Web.Cache; +using Umbraco.Web.PublishedCache; -namespace Umbraco.Web.Redirects +namespace Umbraco.Web.Routing { /// /// Implements an Application Event Handler for managing redirect urls tracking. @@ -24,9 +25,9 @@ namespace Umbraco.Web.Redirects /// public class RedirectTrackingEventHandler : ApplicationEventHandler { - 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 ContextKey1 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.1"; + private const string ContextKey2 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.2"; + private const string ContextKey3 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.3"; /// protected override void ApplicationStarting(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) @@ -89,62 +90,92 @@ namespace Umbraco.Web.Redirects // rolled back items have to be published, so publishing will take care of that } + /// + /// Tracks a documents URLs during publishing in the current request + /// private static Dictionary> OldRoutes { get { - if (UmbracoContext.Current == null) - return null; - var oldRoutes = (Dictionary>) UmbracoContext.Current.HttpContext.Items[ContextKey3]; - if (oldRoutes == null) - UmbracoContext.Current.HttpContext.Items[ContextKey3] = oldRoutes = new Dictionary>(); + var oldRoutes = RequestCache.GetCacheItem>>( + ContextKey3, + () => new Dictionary>()); return oldRoutes; } } private static bool LockedEvents { - get { return Moving && UmbracoContext.Current.HttpContext.Items[ContextKey2] != null; } + get + { + return Moving && RequestCache.GetCacheItem(ContextKey2) != null; + } set { if (Moving && value) - UmbracoContext.Current.HttpContext.Items[ContextKey2] = true; + { + //this forces true into the cache + RequestCache.GetCacheItem(ContextKey2, () => true); + } else - UmbracoContext.Current.HttpContext.Items.Remove(ContextKey2); + { + RequestCache.ClearCacheItem(ContextKey2); + } } } private static bool Moving { - get { return UmbracoContext.Current.HttpContext.Items[ContextKey1] != null; } + get { return RequestCache.GetCacheItem(ContextKey1) != null; } set { if (value) - UmbracoContext.Current.HttpContext.Items[ContextKey1] = true; + { + //this forces true into the cache + RequestCache.GetCacheItem(ContextKey1, () => true); + } else { - UmbracoContext.Current.HttpContext.Items.Remove(ContextKey1); - UmbracoContext.Current.HttpContext.Items.Remove(ContextKey2); + RequestCache.ClearCacheItem(ContextKey1); + RequestCache.ClearCacheItem(ContextKey2); } } } + /// + /// Before the items are published, we need to get it's current URL before it changes + /// + /// + /// private static void ContentService_Publishing(IPublishingStrategy sender, PublishEventArgs args) { if (LockedEvents) return; - var contentCache = UmbracoContext.Current.ContentCache; + var contentCache = GetPublishedCache(); + if (contentCache == null) return; + foreach (var entity in args.PublishedEntities) { - var entityContent = contentCache.GetById(entity.Id); - if (entityContent == null) continue; - foreach (var x in entityContent.DescendantsOrSelf()) + //don't continue if this entity hasn't changed with regards to anything + // that might change it's URLs + if (entity.IsDirty() == false) continue; + + if (entity.IsPropertyDirty("Name") + || entity.IsPropertyDirty(Constants.Conventions.Content.UrlName) + || entity.IsPropertyDirty(Constants.Conventions.Content.UrlAlias) + || Moving) { - var route = contentCache.GetRouteById(x.Id); - if (IsNotRoute(route)) continue; - var wk = UnwrapToKey(x); - if (wk == null) continue; - OldRoutes[x.Id] = Tuple.Create(wk.Key, route); + var entityContent = contentCache.GetById(entity.Id); + if (entityContent == null) continue; + foreach (var x in entityContent.DescendantsOrSelf()) + { + var route = contentCache.GetRouteById(x.Id); + if (IsNotRoute(route)) continue; + var wk = UnwrapToKey(x); + if (wk == null) continue; + + OldRoutes[x.Id] = Tuple.Create(wk.Key, route); + } } } @@ -165,23 +196,36 @@ namespace Umbraco.Web.Redirects return withKey; } + /// + /// Executed when the cache updates, which means we can know what the new URL is for a given document + /// + /// + /// private void PageCacheRefresher_CacheUpdated(PageCacheRefresher sender, CacheRefresherEventArgs cacheRefresherEventArgs) { - if (OldRoutes == null) - return; - - var removeKeys = new List(); - - foreach (var oldRoute in OldRoutes) + //This should only ever occur on the Master server when in load balancing since this will fire on all + // servers taking part in load balancing + var serverRole = ApplicationContext.Current.GetCurrentServerRole(); + if (serverRole == ServerRole.Master || serverRole == ServerRole.Single) { - // 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, oldRoute.Value.Item1, oldRoute.Value.Item2); - removeKeys.Add(oldRoute.Key); - } + if (RequestCache.GetCacheItem(ContextKey3) == null) + return; - foreach (var k in removeKeys) - OldRoutes.Remove(k); + try + { + 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, oldRoute.Value.Item1, oldRoute.Value.Item2); + } + } + finally + { + OldRoutes.Clear(); + RequestCache.ClearCacheItem(ContextKey3); + } + } } private static void ContentService_Published(IPublishingStrategy sender, PublishEventArgs e) @@ -203,7 +247,10 @@ namespace Umbraco.Web.Redirects private static void CreateRedirect(int contentId, Guid contentKey, string oldRoute) { - var contentCache = UmbracoContext.Current.ContentCache; + + var contentCache = GetPublishedCache(); + if (contentCache == null) return; + var newRoute = contentCache.GetRouteById(contentId); if (IsNotRoute(newRoute) || oldRoute == newRoute) return; var redirectUrlService = ApplicationContext.Current.Services.RedirectUrlService; @@ -216,5 +263,21 @@ namespace Umbraco.Web.Redirects // err/- if collision or anomaly or ... return route == null || route.StartsWith("err/"); } + + /// + /// Gets the current request cache to persist the values between handlers + /// + private static ContextualPublishedContentCache GetPublishedCache() + { + return UmbracoContext.Current == null ? null : UmbracoContext.Current.ContentCache; + } + + /// + /// Gets the current request cache to persist the values between handlers + /// + private static ICacheProvider RequestCache + { + get { return ApplicationContext.Current.ApplicationCache.RequestCache; } + } } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index d1cab1b52d..41f0256cab 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -390,7 +390,7 @@ - + From ad2a644bdf1c6adcbd184793a0df511ec6ef4cd5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 8 Aug 2016 12:07:43 +0200 Subject: [PATCH 2/3] temp save to track history since i need to roll this back --- .../Routing/RedirectTrackingEventHandler.cs | 173 ++++++++++++++---- 1 file changed, 136 insertions(+), 37 deletions(-) diff --git a/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs b/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs index 65cad094dc..65aac4658d 100644 --- a/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs +++ b/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Publishing; using Umbraco.Core.Services; +using Umbraco.Core.Strings; using Umbraco.Core.Sync; using Umbraco.Web.Cache; using Umbraco.Web.PublishedCache; @@ -25,9 +26,10 @@ namespace Umbraco.Web.Routing /// public class RedirectTrackingEventHandler : ApplicationEventHandler { - private const string ContextKey1 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.1"; - private const string ContextKey2 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.2"; - private const string ContextKey3 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.3"; + private const string ContextKey1 = "RedirectTrackingEventHandler.1"; + private const string ContextKey2 = "RedirectTrackingEventHandler.2"; + //private const string ContextKey3 = "RedirectTrackingEventHandler.3"; + private const string ContextKey4 = "RedirectTrackingEventHandler.4"; /// protected override void ApplicationStarting(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) @@ -90,20 +92,80 @@ namespace Umbraco.Web.Routing // rolled back items have to be published, so publishing will take care of that } + ///// + ///// Tracks a documents URLs during publishing in the current request + ///// + //private static Dictionary> OldRoutes + //{ + // get + // { + // var oldRoutes = RequestCache.GetCacheItem>>( + // ContextKey3, + // () => new Dictionary>()); + // return oldRoutes; + // } + //} + + private class PrePublishedContentContext + { + public static PrePublishedContentContext Empty + { + get { return new PrePublishedContentContext(null, null, null, null); } + } + /// Initializes a new instance of the class. + public PrePublishedContentContext(IContent entity, string urlSegment, ContextualPublishedContentCache contentCache, Func> descendentsDelegate) + { + if (entity == null) throw new ArgumentNullException("entity"); + if (contentCache == null) throw new ArgumentNullException("contentCache"); + if (descendentsDelegate == null) throw new ArgumentNullException("descendentsDelegate"); + if (string.IsNullOrWhiteSpace(urlSegment)) throw new ArgumentException("Value cannot be null or whitespace.", "urlSegment"); + Entity = entity; + UrlSegment = urlSegment; + ContentCache = contentCache; + DescendentsDelegate = descendentsDelegate; + } + + public IContent Entity { get; set; } + public string UrlSegment { get; set; } + public Func> DescendentsDelegate { get; set; } + public ContextualPublishedContentCache ContentCache { get; set; } + } + /// - /// Tracks a documents URLs during publishing in the current request + /// Tracks the current doc's entity, url segment and delegate to retrieve it's old descendents during publishing in the current request /// - private static Dictionary> OldRoutes + private static PrePublishedContentContext PrePublishedContent { get { - var oldRoutes = RequestCache.GetCacheItem>>( - ContextKey3, - () => new Dictionary>()); - return oldRoutes; + //return the item in the cache - otherwise initialize it to an empty instance + return RequestCache.GetCacheItem(ContextKey4, () => PrePublishedContentContext.Empty); + } + set + { + //clear it + RequestCache.ClearCacheItem(ContextKey4); + //force it into the cache + RequestCache.GetCacheItem(ContextKey4, () => value); } } + //private static Func> DescendentsOrSelfDelegate + //{ + // get + // { + // //return the item in the cache - otherwise initialize it to an empty string + // return RequestCache.GetCacheItem>>(ContextKey4, () => (() => Enumerable.Empty())); + // } + // set + // { + // //clear it + // RequestCache.ClearCacheItem(ContextKey4); + // //force it into the cache + // RequestCache.GetCacheItem>>(ContextKey4, () => value); + // } + //} + private static bool LockedEvents { get @@ -155,28 +217,26 @@ namespace Umbraco.Web.Routing if (contentCache == null) return; foreach (var entity in args.PublishedEntities) - { - //don't continue if this entity hasn't changed with regards to anything - // that might change it's URLs - if (entity.IsDirty() == false) continue; + { + var entityContent = contentCache.GetById(entity.Id); + if (entityContent == null) continue; - if (entity.IsPropertyDirty("Name") - || entity.IsPropertyDirty(Constants.Conventions.Content.UrlName) - || entity.IsPropertyDirty(Constants.Conventions.Content.UrlAlias) - || Moving) - { - var entityContent = contentCache.GetById(entity.Id); - if (entityContent == null) continue; - foreach (var x in entityContent.DescendantsOrSelf()) - { - var route = contentCache.GetRouteById(x.Id); - if (IsNotRoute(route)) continue; - var wk = UnwrapToKey(x); - if (wk == null) continue; - - OldRoutes[x.Id] = Tuple.Create(wk.Key, route); - } - } + PrePublishedContent = new PrePublishedContentContext(entity, entity.GetUrlSegment(), contentCache, () => entityContent.Descendants()); + + //if (Moving) + //{ + // var entityContent = contentCache.GetById(entity.Id); + // if (entityContent == null) continue; + // foreach (var x in entityContent.Descendants()) + // { + // var route = contentCache.GetRouteById(x.Id); + // if (IsNotRoute(route)) continue; + // var wk = UnwrapToKey(x); + // if (wk == null) continue; + + // OldRoutes[x.Id] = Tuple.Create(wk.Key, route); + // } + //} } LockedEvents = true; // we only want to see the "first batch" @@ -208,23 +268,62 @@ namespace Umbraco.Web.Routing var serverRole = ApplicationContext.Current.GetCurrentServerRole(); if (serverRole == ServerRole.Master || serverRole == ServerRole.Single) { - if (RequestCache.GetCacheItem(ContextKey3) == null) - return; + //copy local + var prePublishedContext = PrePublishedContent; + //cannot continue if this is empty + if (prePublishedContext.Entity == null) return; + + //cannot continue if there is no published cache + var contentCache = GetPublishedCache(); + if (contentCache == null) return; + + //get the entity id out of the event args to compare with the id stored during publishing + if (cacheRefresherEventArgs.MessageType != MessageType.RefreshById || cacheRefresherEventArgs.MessageType != MessageType.RefreshByInstance) return; + + var refreshedEntityId = cacheRefresherEventArgs.MessageType == MessageType.RefreshByInstance + ? ((IContent)cacheRefresherEventArgs.MessageObject).Id + : (int) cacheRefresherEventArgs.MessageObject; + + //if it's not the id that we're targeting, don't continue + if (refreshedEntityId != prePublishedContext.Entity.Id) return; + + //cannot continue if the entity is not found + var entityContent = contentCache.GetById(prePublishedContext.Entity.Id); + if (entityContent == null) return; + + //now we can check if the segment has changed + var newSegment = prePublishedContext.Entity.GetUrlSegment(); try { - foreach (var oldRoute in OldRoutes) + if (Moving || newSegment != prePublishedContext.UrlSegment) { + //it's changed! + // 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, oldRoute.Value.Item1, oldRoute.Value.Item2); + CreateRedirect(prePublishedContext.Entity.Id, prePublishedContext.Entity.Key, prePublishedContext.UrlSegment); + + //iterate the old descendents and get their old routes + foreach (var x in prePublishedContext.DescendentsDelegate()) + { + //get the old route from the old contextual cache + var oldRoute = prePublishedContext.ContentCache.GetRouteById(x.Id); + if (IsNotRoute(oldRoute)) continue; + var wk = UnwrapToKey(x); + if (wk == null) continue; + + CreateRedirect(wk.Id, wk.Key, oldRoute); + } } } finally { - OldRoutes.Clear(); - RequestCache.ClearCacheItem(ContextKey3); - } + //set all refs to null + prePublishedContext.Entity = null; + prePublishedContext.ContentCache = null; + prePublishedContext.DescendentsDelegate = null; + } } } From fee4a84c668dec8b5d020224eb3e6f0f9a58750c Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 8 Aug 2016 12:19:03 +0200 Subject: [PATCH 3/3] Revert temp commit, adds logic to check if the segment value has changed + lots of notes --- .../Strings/IUrlSegmentProvider.cs | 4 + .../Routing/RedirectTrackingEventHandler.cs | 181 +++++------------- 2 files changed, 49 insertions(+), 136 deletions(-) diff --git a/src/Umbraco.Core/Strings/IUrlSegmentProvider.cs b/src/Umbraco.Core/Strings/IUrlSegmentProvider.cs index dc3f4243e7..c3ef596ad0 100644 --- a/src/Umbraco.Core/Strings/IUrlSegmentProvider.cs +++ b/src/Umbraco.Core/Strings/IUrlSegmentProvider.cs @@ -26,5 +26,9 @@ namespace Umbraco.Core.Strings /// per content, in 1-to-1 multilingual configurations. Then there would be one /// url per culture. string GetUrlSegment(IContentBase content, CultureInfo culture); + + //TODO: For the 301 tracking, we need to add another extended interface to this so that + // the RedirectTrackingEventHandler can ask the IUrlSegmentProvider if the URL is changing. + // Currently the way it works is very hacky, see notes in: RedirectTrackingEventHandler.ContentService_Publishing } } diff --git a/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs b/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs index 65aac4658d..b0340fa7e4 100644 --- a/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs +++ b/src/Umbraco.Web/Routing/RedirectTrackingEventHandler.cs @@ -26,10 +26,9 @@ namespace Umbraco.Web.Routing /// public class RedirectTrackingEventHandler : ApplicationEventHandler { - private const string ContextKey1 = "RedirectTrackingEventHandler.1"; - private const string ContextKey2 = "RedirectTrackingEventHandler.2"; - //private const string ContextKey3 = "RedirectTrackingEventHandler.3"; - private const string ContextKey4 = "RedirectTrackingEventHandler.4"; + private const string ContextKey1 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.1"; + private const string ContextKey2 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.2"; + private const string ContextKey3 = "Umbraco.Web.Routing.RedirectTrackingEventHandler.3"; /// protected override void ApplicationStarting(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) @@ -92,80 +91,20 @@ namespace Umbraco.Web.Routing // rolled back items have to be published, so publishing will take care of that } - ///// - ///// Tracks a documents URLs during publishing in the current request - ///// - //private static Dictionary> OldRoutes - //{ - // get - // { - // var oldRoutes = RequestCache.GetCacheItem>>( - // ContextKey3, - // () => new Dictionary>()); - // return oldRoutes; - // } - //} - - private class PrePublishedContentContext - { - public static PrePublishedContentContext Empty - { - get { return new PrePublishedContentContext(null, null, null, null); } - } - /// Initializes a new instance of the class. - public PrePublishedContentContext(IContent entity, string urlSegment, ContextualPublishedContentCache contentCache, Func> descendentsDelegate) - { - if (entity == null) throw new ArgumentNullException("entity"); - if (contentCache == null) throw new ArgumentNullException("contentCache"); - if (descendentsDelegate == null) throw new ArgumentNullException("descendentsDelegate"); - if (string.IsNullOrWhiteSpace(urlSegment)) throw new ArgumentException("Value cannot be null or whitespace.", "urlSegment"); - Entity = entity; - UrlSegment = urlSegment; - ContentCache = contentCache; - DescendentsDelegate = descendentsDelegate; - } - - public IContent Entity { get; set; } - public string UrlSegment { get; set; } - public Func> DescendentsDelegate { get; set; } - public ContextualPublishedContentCache ContentCache { get; set; } - } - /// - /// Tracks the current doc's entity, url segment and delegate to retrieve it's old descendents during publishing in the current request + /// Tracks a documents URLs during publishing in the current request /// - private static PrePublishedContentContext PrePublishedContent + private static Dictionary> OldRoutes { get { - //return the item in the cache - otherwise initialize it to an empty instance - return RequestCache.GetCacheItem(ContextKey4, () => PrePublishedContentContext.Empty); - } - set - { - //clear it - RequestCache.ClearCacheItem(ContextKey4); - //force it into the cache - RequestCache.GetCacheItem(ContextKey4, () => value); + var oldRoutes = RequestCache.GetCacheItem>>( + ContextKey3, + () => new Dictionary>()); + return oldRoutes; } } - //private static Func> DescendentsOrSelfDelegate - //{ - // get - // { - // //return the item in the cache - otherwise initialize it to an empty string - // return RequestCache.GetCacheItem>>(ContextKey4, () => (() => Enumerable.Empty())); - // } - // set - // { - // //clear it - // RequestCache.ClearCacheItem(ContextKey4); - // //force it into the cache - // RequestCache.GetCacheItem>>(ContextKey4, () => value); - // } - //} - private static bool LockedEvents { get @@ -217,26 +156,34 @@ namespace Umbraco.Web.Routing if (contentCache == null) return; foreach (var entity in args.PublishedEntities) - { - var entityContent = contentCache.GetById(entity.Id); - if (entityContent == null) continue; - - PrePublishedContent = new PrePublishedContentContext(entity, entity.GetUrlSegment(), contentCache, () => entityContent.Descendants()); - - //if (Moving) - //{ - // var entityContent = contentCache.GetById(entity.Id); - // if (entityContent == null) continue; - // foreach (var x in entityContent.Descendants()) - // { - // var route = contentCache.GetRouteById(x.Id); - // if (IsNotRoute(route)) continue; - // var wk = UnwrapToKey(x); - // if (wk == null) continue; - - // OldRoutes[x.Id] = Tuple.Create(wk.Key, route); - // } - //} + { + //TODO: This is horrible - we need to check if the url segment for this entity is changing in + // order to determine if we need to make redirects for itself and all of it's descendents. + // The way this works right now (7.5.0) is that we re-lookup the entity that is currently being published which + // returns it's existing data in the db which we use to extract it's current segment. Then we compare that with + // the segment value returned from the current entity. + // In the future this will certainly cause some problems, to fix this we'd need to change the IUrlSegmentProvider + // to support being able to determine if a segment is going to change for an entity. See notes in IUrlSegmentProvider. + var oldEntity = ApplicationContext.Current.Services.ContentService.GetById(entity.Id); + if (oldEntity == null) continue; + var oldSegmentName = oldEntity.GetUrlSegment(); + + //if the segment has changed or we are moving, then process all descendent + // Urls and schedule them for creating a rewrite when publishing is done. + if (oldSegmentName != entity.GetUrlSegment() || Moving) + { + var entityContent = contentCache.GetById(entity.Id); + if (entityContent == null) continue; + foreach (var x in entityContent.DescendantsOrSelf()) + { + var route = contentCache.GetRouteById(x.Id); + if (IsNotRoute(route)) continue; + var wk = UnwrapToKey(x); + if (wk == null) continue; + + OldRoutes[x.Id] = Tuple.Create(wk.Key, route); + } + } } LockedEvents = true; // we only want to see the "first batch" @@ -268,62 +215,24 @@ namespace Umbraco.Web.Routing var serverRole = ApplicationContext.Current.GetCurrentServerRole(); if (serverRole == ServerRole.Master || serverRole == ServerRole.Single) { - //copy local - var prePublishedContext = PrePublishedContent; + //if the Old routes is empty do not continue + if (OldRoutes.Count == 0) + return; - //cannot continue if this is empty - if (prePublishedContext.Entity == null) return; - - //cannot continue if there is no published cache - var contentCache = GetPublishedCache(); - if (contentCache == null) return; - - //get the entity id out of the event args to compare with the id stored during publishing - if (cacheRefresherEventArgs.MessageType != MessageType.RefreshById || cacheRefresherEventArgs.MessageType != MessageType.RefreshByInstance) return; - - var refreshedEntityId = cacheRefresherEventArgs.MessageType == MessageType.RefreshByInstance - ? ((IContent)cacheRefresherEventArgs.MessageObject).Id - : (int) cacheRefresherEventArgs.MessageObject; - - //if it's not the id that we're targeting, don't continue - if (refreshedEntityId != prePublishedContext.Entity.Id) return; - - //cannot continue if the entity is not found - var entityContent = contentCache.GetById(prePublishedContext.Entity.Id); - if (entityContent == null) return; - - //now we can check if the segment has changed - var newSegment = prePublishedContext.Entity.GetUrlSegment(); try { - if (Moving || newSegment != prePublishedContext.UrlSegment) + foreach (var oldRoute in OldRoutes) { - //it's changed! - // 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(prePublishedContext.Entity.Id, prePublishedContext.Entity.Key, prePublishedContext.UrlSegment); - - //iterate the old descendents and get their old routes - foreach (var x in prePublishedContext.DescendentsDelegate()) - { - //get the old route from the old contextual cache - var oldRoute = prePublishedContext.ContentCache.GetRouteById(x.Id); - if (IsNotRoute(oldRoute)) continue; - var wk = UnwrapToKey(x); - if (wk == null) continue; - - CreateRedirect(wk.Id, wk.Key, oldRoute); - } + CreateRedirect(oldRoute.Key, oldRoute.Value.Item1, oldRoute.Value.Item2); } } finally { - //set all refs to null - prePublishedContext.Entity = null; - prePublishedContext.ContentCache = null; - prePublishedContext.DescendentsDelegate = null; - } + OldRoutes.Clear(); + RequestCache.ClearCacheItem(ContextKey3); + } } }