From d08d141bcf5eb16a5add5655a80f34ac93ac48fb Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 31 Oct 2023 12:38:44 +0100 Subject: [PATCH] Updates and support for re-use of CMS logic in Deploy (#14990) * Adds additional parameter to IFileSource.GetFilesAsync allowing the caller to continue on a file not found exception. * Moved redirect tracking and creation logic out of handler into a service, allowing for re-use in Deploy. * Reverted breaking change in IFileSource by obsoleting old method and --- src/Umbraco.Core/Deploy/IFileSource.cs | 18 +- src/Umbraco.Core/Routing/IRedirectTracker.cs | 23 +++ .../UmbracoBuilder.CoreServices.cs | 4 + .../Routing/RedirectTracker.cs | 125 +++++++++++++ .../Routing/RedirectTrackingHandler.cs | 173 ++---------------- 5 files changed, 179 insertions(+), 164 deletions(-) create mode 100644 src/Umbraco.Core/Routing/IRedirectTracker.cs create mode 100644 src/Umbraco.Infrastructure/Routing/RedirectTracker.cs diff --git a/src/Umbraco.Core/Deploy/IFileSource.cs b/src/Umbraco.Core/Deploy/IFileSource.cs index ed169b9df5..e56f9a715e 100644 --- a/src/Umbraco.Core/Deploy/IFileSource.cs +++ b/src/Umbraco.Core/Deploy/IFileSource.cs @@ -68,18 +68,24 @@ public interface IFileSource /// A collection of file types which can store the files. void GetFiles(IEnumerable udis, IFileTypeCollection fileTypes); + // TODO (V14): Remove obsolete method and default implementation for GetFilesAsync overloads. + /// /// Gets files and store them using a file store. /// /// The udis of the files to get. /// A collection of file types which can store the files. /// A cancellation token. + [Obsolete("Please use the method overload taking all parameters. This method overload will be removed in Umbraco 14.")] Task GetFilesAsync(IEnumerable udis, IFileTypeCollection fileTypes, CancellationToken token); - ///// - ///// Gets the content of a file as a bytes array. - ///// - ///// A file entity identifier. - ///// A byte array containing the file content. - // byte[] GetFileBytes(StringUdi Udi); + /// + /// Gets files and store them using a file store. + /// + /// The udis of the files to get. + /// A collection of file types which can store the files. + /// A flag indicating whether to continue if a file isn't found or to stop and throw a FileNotFoundException. + /// A cancellation token. + Task GetFilesAsync(IEnumerable udis, IFileTypeCollection fileTypes, bool continueOnFileNotFound, CancellationToken token) + => GetFilesAsync(udis, fileTypes, token); } diff --git a/src/Umbraco.Core/Routing/IRedirectTracker.cs b/src/Umbraco.Core/Routing/IRedirectTracker.cs new file mode 100644 index 0000000000..2b0c8649a9 --- /dev/null +++ b/src/Umbraco.Core/Routing/IRedirectTracker.cs @@ -0,0 +1,23 @@ +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Routing +{ + /// + /// Determines and records redirects for a content item following an update that may change it's public URL. + /// + public interface IRedirectTracker + { + /// + /// Stores the existing routes for a content item before update. + /// + /// The content entity updated. + /// The dictionary of routes for population. + void StoreOldRoute(IContent entity, Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> oldRoutes); + + /// + /// Creates appropriate redirects for the content item following an update. + /// + /// The populated dictionary of old routes; + void CreateRedirects(IDictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> oldRoutes); + } +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 3e12664f03..1c21352af2 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -52,6 +52,7 @@ using Umbraco.Cms.Infrastructure.Migrations.PostMigrations; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_0_0.DataTypes; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.Mappers; +using Umbraco.Cms.Infrastructure.Routing; using Umbraco.Cms.Infrastructure.Runtime; using Umbraco.Cms.Infrastructure.Runtime.RuntimeModeValidators; using Umbraco.Cms.Infrastructure.Scoping; @@ -216,6 +217,9 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddSingleton(); builder.Services.AddTransient(); + + builder.Services.AddSingleton(); + builder.AddInstaller(); // Services required to run background jobs (with out the handler) diff --git a/src/Umbraco.Infrastructure/Routing/RedirectTracker.cs b/src/Umbraco.Infrastructure/Routing/RedirectTracker.cs new file mode 100644 index 0000000000..a1247313d2 --- /dev/null +++ b/src/Umbraco.Infrastructure/Routing/RedirectTracker.cs @@ -0,0 +1,125 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Web; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Routing +{ + internal class RedirectTracker : IRedirectTracker + { + private readonly IUmbracoContextFactory _umbracoContextFactory; + private readonly IVariationContextAccessor _variationContextAccessor; + private readonly ILocalizationService _localizationService; + private readonly IRedirectUrlService _redirectUrlService; + private readonly ILogger _logger; + + public RedirectTracker( + IUmbracoContextFactory umbracoContextFactory, + IVariationContextAccessor variationContextAccessor, + ILocalizationService localizationService, + IRedirectUrlService redirectUrlService, + ILogger logger) + { + _umbracoContextFactory = umbracoContextFactory; + _variationContextAccessor = variationContextAccessor; + _localizationService = localizationService; + _redirectUrlService = redirectUrlService; + _logger = logger; + } + + /// + public void StoreOldRoute(IContent entity, Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> oldRoutes) + { + using UmbracoContextReference reference = _umbracoContextFactory.EnsureUmbracoContext(); + IPublishedContentCache? contentCache = reference.UmbracoContext.Content; + IPublishedContent? entityContent = contentCache?.GetById(entity.Id); + if (entityContent is 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 = new Lazy(() => entityContent.AncestorsOrSelf().FirstOrDefault(a => a.Cultures.Any())?.Cultures.Keys.ToArray() ?? Array.Empty()); + + // Get all language ISO codes (in case we're dealing with invariant content with variant ancestors) + var languageIsoCodes = new Lazy(() => _localizationService.GetAllLanguages().Select(x => x.IsoCode).ToArray()); + + foreach (IPublishedContent publishedContent in entityContent.DescendantsOrSelf(_variationContextAccessor)) + { + // If this entity defines specific cultures, use those instead of the default ones + IEnumerable cultures = publishedContent.Cultures.Any() ? publishedContent.Cultures.Keys : defaultCultures.Value; + + foreach (var culture in cultures) + { + try + { + var route = contentCache?.GetRouteById(publishedContent.Id, culture); + if (IsValidRoute(route)) + { + oldRoutes[(publishedContent.Id, culture)] = (publishedContent.Key, route); + } + else if (string.IsNullOrEmpty(culture)) + { + // Retry using all languages, if this is invariant but has a variant ancestor. + foreach (string languageIsoCode in languageIsoCodes.Value) + { + route = contentCache?.GetRouteById(publishedContent.Id, languageIsoCode); + if (IsValidRoute(route)) + { + oldRoutes[(publishedContent.Id, languageIsoCode)] = (publishedContent.Key, route); + } + } + } + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not register redirects because the old route couldn't be retrieved for content ID {ContentId} and culture '{Culture}'.", publishedContent.Id, culture); + } + } + } + } + + /// + public void CreateRedirects(IDictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> oldRoutes) + { + if (!oldRoutes.Any()) + { + return; + } + + using UmbracoContextReference reference = _umbracoContextFactory.EnsureUmbracoContext(); + IPublishedContentCache? contentCache = reference.UmbracoContext.Content; + if (contentCache == null) + { + _logger.LogWarning("Could not track redirects because there is no published content cache available on the current published snapshot."); + return; + } + + foreach (((int contentId, string culture), (Guid contentKey, string oldRoute)) in oldRoutes) + { + try + { + var newRoute = contentCache.GetRouteById(contentId, culture); + if (!IsValidRoute(newRoute) || oldRoute == newRoute) + { + continue; + } + + _redirectUrlService.Register(oldRoute, contentKey, culture); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not track redirects because the new route couldn't be retrieved for content ID {ContentId} and culture '{Culture}'.", contentId, culture); + } + } + } + + private static bool IsValidRoute([NotNullWhen(true)] string? route) => route is not null && !route.StartsWith("err/"); + } +} diff --git a/src/Umbraco.Infrastructure/Routing/RedirectTrackingHandler.cs b/src/Umbraco.Infrastructure/Routing/RedirectTrackingHandler.cs index 0e4ad2e9c6..c2bb444e2c 100644 --- a/src/Umbraco.Infrastructure/Routing/RedirectTrackingHandler.cs +++ b/src/Umbraco.Infrastructure/Routing/RedirectTrackingHandler.cs @@ -1,17 +1,11 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Notifications; -using Umbraco.Cms.Core.PublishedCache; -using Umbraco.Cms.Core.Services; using Umbraco.Extensions; namespace Umbraco.Cms.Core.Routing; @@ -30,44 +24,17 @@ public sealed class RedirectTrackingHandler : INotificationHandler { private const string NotificationStateKey = "Umbraco.Cms.Core.Routing.RedirectTrackingHandler"; - private readonly ILogger _logger; - private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; - private readonly IRedirectUrlService _redirectUrlService; - private readonly IVariationContextAccessor _variationContextAccessor;private readonly ILocalizationService _localizationService; + private readonly IOptionsMonitor _webRoutingSettings; + private readonly IRedirectTracker _redirectTracker; public RedirectTrackingHandler( - ILogger logger, IOptionsMonitor webRoutingSettings, - IPublishedSnapshotAccessor publishedSnapshotAccessor, - IRedirectUrlService redirectUrlService, - IVariationContextAccessor variationContextAccessor, - ILocalizationService localizationService){ - _logger = logger; + IRedirectTracker redirectTracker) + { _webRoutingSettings = webRoutingSettings; - _publishedSnapshotAccessor = publishedSnapshotAccessor; - _redirectUrlService = redirectUrlService; - _variationContextAccessor = variationContextAccessor; - _localizationService = localizationService; - } - - [Obsolete("Use ctor with all params")] - public RedirectTrackingHandler( - ILogger logger, - IOptionsMonitor webRoutingSettings, - IPublishedSnapshotAccessor publishedSnapshotAccessor, - IRedirectUrlService redirectUrlService, - IVariationContextAccessor variationContextAccessor) - :this( - logger, - webRoutingSettings, - publishedSnapshotAccessor, - redirectUrlService, - variationContextAccessor, - StaticServiceProvider.Instance.GetRequiredService()) - { - - } + _redirectTracker = redirectTracker; + } public void Handle(ContentMovedNotification notification) => CreateRedirectsForOldRoutes(notification); @@ -79,150 +46,40 @@ public sealed class RedirectTrackingHandler : public void Handle(ContentPublishingNotification notification) => StoreOldRoutes(notification.PublishedEntities, notification); - private static bool IsNotRoute(string? route) => - - // null if content not found - // err/- if collision or anomaly or ... - route == null || route.StartsWith("err/"); - private void StoreOldRoutes(IEnumerable entities, IStatefulNotification notification) { - // don't let the notification handlers kick in if Redirect Tracking is turned off in the config + // Don't let the notification handlers kick in if redirect tracking is turned off in the config. if (_webRoutingSettings.CurrentValue.DisableRedirectUrlTracking) { return; } - OldRoutesDictionary oldRoutes = GetOldRoutes(notification); + Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> oldRoutes = GetOldRoutes(notification); foreach (IContent entity in entities) { - StoreOldRoute(entity, oldRoutes); + _redirectTracker.StoreOldRoute(entity, oldRoutes); } } private void CreateRedirectsForOldRoutes(IStatefulNotification notification) { - // don't let the notification handlers kick in if Redirect Tracking is turned off in the config + // Don't let the notification handlers kick in if redirect tracking is turned off in the config. if (_webRoutingSettings.CurrentValue.DisableRedirectUrlTracking) { return; } - OldRoutesDictionary oldRoutes = GetOldRoutes(notification); - CreateRedirects(oldRoutes); + Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> oldRoutes = GetOldRoutes(notification); + _redirectTracker.CreateRedirects(oldRoutes); } - private OldRoutesDictionary GetOldRoutes(IStatefulNotification notification) + private Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> GetOldRoutes(IStatefulNotification notification) { if (notification.State.ContainsKey(NotificationStateKey) == false) { - notification.State[NotificationStateKey] = new OldRoutesDictionary(); + notification.State[NotificationStateKey] = new Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)>(); } - return (OldRoutesDictionary?)notification.State[NotificationStateKey] ?? new OldRoutesDictionary(); - } - - private void StoreOldRoute(IContent entity, OldRoutesDictionary oldRoutes) - { - if (!_publishedSnapshotAccessor.TryGetPublishedSnapshot(out IPublishedSnapshot? publishedSnapshot)) - { - return; - } - - IPublishedContentCache? contentCache = publishedSnapshot?.Content; - IPublishedContent? entityContent = contentCache?.GetById(entity.Id); - if (entityContent is 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() - ?? Array.Empty(); - - foreach (IPublishedContent publishedContent in entityContent.DescendantsOrSelf(_variationContextAccessor)) - { - // if this entity defines specific cultures, use those instead of the default ones - IEnumerable cultures = - publishedContent.Cultures.Any() ? publishedContent.Cultures.Keys : defaultCultures; - - foreach (var culture in cultures) - { - var route = contentCache?.GetRouteById(publishedContent.Id, culture); - if (!IsNotRoute(route)) - { - oldRoutes[new ContentIdAndCulture(publishedContent.Id, culture)] = new ContentKeyAndOldRoute(publishedContent.Key, route!); - } - else if (string.IsNullOrEmpty(culture)) - { - // Retry using all languages, if this is invariant but has a variant ancestor - var languages = _localizationService.GetAllLanguages(); - foreach (var language in languages) - { - route = contentCache?.GetRouteById(publishedContent.Id, language.IsoCode); - if (!IsNotRoute(route)) - { - oldRoutes[new ContentIdAndCulture(publishedContent.Id, language.IsoCode)] = - new ContentKeyAndOldRoute(publishedContent.Key, route!); - } - } - }} - } - } - - private void CreateRedirects(OldRoutesDictionary oldRoutes) - { - if (!_publishedSnapshotAccessor.TryGetPublishedSnapshot(out IPublishedSnapshot? publishedSnapshot)) - { - return; - } - - IPublishedContentCache? contentCache = publishedSnapshot?.Content; - - if (contentCache == null) - { - _logger.LogWarning("Could not track redirects because there is no current published snapshot available."); - return; - } - - foreach (KeyValuePair 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 class ContentIdAndCulture : Tuple - { - public ContentIdAndCulture(int contentId, string culture) - : base(contentId, culture) - { - } - - public int ContentId => Item1; - - public string Culture => Item2; - } - - private class ContentKeyAndOldRoute : Tuple - { - public ContentKeyAndOldRoute(Guid contentKey, string oldRoute) - : base(contentKey, oldRoute) - { - } - - public Guid ContentKey => Item1; - - public string OldRoute => Item2; - } - - private class OldRoutesDictionary : Dictionary - { + return (Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)>?)notification.State[NotificationStateKey]!; } }