From 8fef824dc1b354846129558e47eaef111927804c Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 9 Sep 2019 22:33:18 +1000 Subject: [PATCH 01/40] Moves rebuilding models and cache to background thread --- .../Cache/BackgroundSafeLiveFactory.cs | 86 +++++++++++++++++++ .../Cache/ContentTypeCacheRefresher.cs | 21 ++--- .../Cache/DataTypeCacheRefresher.cs | 21 ++--- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 4 +- src/Umbraco.Web/Umbraco.Web.csproj | 1 + 5 files changed, 101 insertions(+), 32 deletions(-) create mode 100644 src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs diff --git a/src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs b/src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs new file mode 100644 index 0000000000..d45c71d04d --- /dev/null +++ b/src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs @@ -0,0 +1,86 @@ +using System.Threading; +using System.Threading.Tasks; +using Umbraco.Core; +using Umbraco.Core.Models.PublishedContent; +using Umbraco.Core.Logging; +using Umbraco.Web.PublishedCache; +using Umbraco.Web.Scheduling; + +namespace Umbraco.Web.Cache +{ + public sealed class BackgroundSafeLiveFactory + { + private readonly IPublishedModelFactory _publishedModelFactory; + private readonly IPublishedSnapshotService _publishedSnapshotService; + private BackgroundTaskRunner _runner; + + public BackgroundSafeLiveFactory(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService, ILogger logger) + { + _publishedModelFactory = publishedModelFactory; + _publishedSnapshotService = publishedSnapshotService; + _runner = new BackgroundTaskRunner("RebuildModelsAndCache", logger); + } + + public void Execute(ContentTypeCacheRefresher.JsonPayload[] payloads) + { + _runner.TryAdd(new RebuildModelsAndCacheTask(payloads, _publishedModelFactory, _publishedSnapshotService)); + } + + public void Execute(DataTypeCacheRefresher.JsonPayload[] payloads) + { + _runner.TryAdd(new RebuildModelsAndCacheTask(payloads, _publishedModelFactory, _publishedSnapshotService)); + } + + private class RebuildModelsAndCacheTask : IBackgroundTask + { + private readonly DataTypeCacheRefresher.JsonPayload[] _dataTypePayloads; + private readonly ContentTypeCacheRefresher.JsonPayload[] _contentTypePayloads; + private readonly IPublishedModelFactory _publishedModelFactory; + private readonly IPublishedSnapshotService _publishedSnapshotService; + + private RebuildModelsAndCacheTask(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService) + { + _publishedModelFactory = publishedModelFactory; + _publishedSnapshotService = publishedSnapshotService; + } + + public RebuildModelsAndCacheTask(DataTypeCacheRefresher.JsonPayload[] payloads, IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService) + : this(publishedModelFactory, publishedSnapshotService) + { + _dataTypePayloads = payloads; + } + + public RebuildModelsAndCacheTask(ContentTypeCacheRefresher.JsonPayload[] payloads, IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService) + : this(publishedModelFactory, publishedSnapshotService) + { + _contentTypePayloads = payloads; + } + + public void Run() + { + // we have to refresh models before we notify the published snapshot + // service of changes, else factories may try to rebuild models while + // we are using the database to load content into caches + + _publishedModelFactory.WithSafeLiveFactory(() => + { + if (_dataTypePayloads != null) + _publishedSnapshotService.Notify(_dataTypePayloads); + if (_contentTypePayloads != null) + _publishedSnapshotService.Notify(_contentTypePayloads); + }); + } + + public Task RunAsync(CancellationToken token) + { + throw new System.NotImplementedException(); + } + + public bool IsAsync => false; + + public void Dispose() + { + } + } + } +} diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index 266cddf6d5..490adeb1d9 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -1,30 +1,23 @@ using System; -using System.Collections.Generic; using System.Linq; -using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; -using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Persistence.Repositories; -using Umbraco.Core.Persistence.Repositories.Implement; using Umbraco.Core.Services; using Umbraco.Core.Services.Changes; -using Umbraco.Web.PublishedCache; namespace Umbraco.Web.Cache { public sealed class ContentTypeCacheRefresher : PayloadCacheRefresherBase { - private readonly IPublishedSnapshotService _publishedSnapshotService; - private readonly IPublishedModelFactory _publishedModelFactory; + private readonly BackgroundSafeLiveFactory _backgroundModelFactory; private readonly IContentTypeCommonRepository _contentTypeCommonRepository; private readonly IdkMap _idkMap; - public ContentTypeCacheRefresher(AppCaches appCaches, IPublishedSnapshotService publishedSnapshotService, IPublishedModelFactory publishedModelFactory, IdkMap idkMap, IContentTypeCommonRepository contentTypeCommonRepository) + public ContentTypeCacheRefresher(AppCaches appCaches, BackgroundSafeLiveFactory backgroundModelFactory, IdkMap idkMap, IContentTypeCommonRepository contentTypeCommonRepository) : base(appCaches) { - _publishedSnapshotService = publishedSnapshotService; - _publishedModelFactory = publishedModelFactory; + _backgroundModelFactory = backgroundModelFactory; _idkMap = idkMap; _contentTypeCommonRepository = contentTypeCommonRepository; } @@ -86,12 +79,8 @@ namespace Umbraco.Web.Cache // don't try to be clever - refresh all MemberCacheRefresher.RefreshMemberTypes(AppCaches); - // we have to refresh models before we notify the published snapshot - // service of changes, else factories may try to rebuild models while - // we are using the database to load content into caches - - _publishedModelFactory.WithSafeLiveFactory(() => - _publishedSnapshotService.Notify(payloads)); + // refresh the models and cache + _backgroundModelFactory.Execute(payloads); // now we can trigger the event base.Refresh(payloads); diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index 1d41c0578b..c777da1971 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -1,26 +1,21 @@ using System; -using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; -using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors.ValueConverters; using Umbraco.Core.Services; -using Umbraco.Web.PublishedCache; - namespace Umbraco.Web.Cache { + public sealed class DataTypeCacheRefresher : PayloadCacheRefresherBase { - private readonly IPublishedSnapshotService _publishedSnapshotService; - private readonly IPublishedModelFactory _publishedModelFactory; + private readonly BackgroundSafeLiveFactory _backgroundModelFactory; private readonly IdkMap _idkMap; - public DataTypeCacheRefresher(AppCaches appCaches, IPublishedSnapshotService publishedSnapshotService, IPublishedModelFactory publishedModelFactory, IdkMap idkMap) + public DataTypeCacheRefresher(AppCaches appCaches, BackgroundSafeLiveFactory backgroundModelFactory, IdkMap idkMap) : base(appCaches) { - _publishedSnapshotService = publishedSnapshotService; - _publishedModelFactory = publishedModelFactory; + _backgroundModelFactory = backgroundModelFactory; _idkMap = idkMap; } @@ -62,12 +57,8 @@ namespace Umbraco.Web.Cache TagsValueConverter.ClearCaches(); SliderValueConverter.ClearCaches(); - // we have to refresh models before we notify the published snapshot - // service of changes, else factories may try to rebuild models while - // we are using the database to load content into caches - - _publishedModelFactory.WithSafeLiveFactory(() => - _publishedSnapshotService.Notify(payloads)); + // refresh the models and cache + _backgroundModelFactory.Execute(payloads); base.Refresh(payloads); } diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index e2b6313ca6..13d2a6b74c 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -73,7 +73,7 @@ namespace Umbraco.Web.Runtime // register accessors for cultures composition.RegisterUnique(); composition.RegisterUnique(); - + // register the http context and umbraco context accessors // we *should* use the HttpContextUmbracoContextAccessor, however there are cases when // we have no http context, eg when booting Umbraco or in background threads, so instead @@ -125,6 +125,8 @@ namespace Umbraco.Web.Runtime // register distributed cache composition.RegisterUnique(f => new DistributedCache()); + composition.RegisterUnique(); + // replace some services composition.RegisterUnique(); composition.RegisterUnique(); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 222d58d4dc..c7a576c94f 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -115,6 +115,7 @@ + From fdad3874145f91e35677eb20a17eb60c8d1e72b2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 9 Sep 2019 23:25:28 +1000 Subject: [PATCH 02/40] renames objects/methods and adds a lock to the routing mechanism --- .../Routing/UmbracoModuleTests.cs | 6 ++- ...groundPublishedSnapshotServiceNotifier.cs} | 48 +++++++++++++++---- .../Cache/ContentTypeCacheRefresher.cs | 6 +-- .../Cache/DataTypeCacheRefresher.cs | 6 +-- ...aseServerRegistrarAndMessengerComponent.cs | 4 -- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- src/Umbraco.Web/UmbracoInjectedModule.cs | 10 +++- 8 files changed, 61 insertions(+), 23 deletions(-) rename src/Umbraco.Web/Cache/{BackgroundSafeLiveFactory.cs => BackgroundPublishedSnapshotServiceNotifier.cs} (57%) diff --git a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs index 3e4c4f1ba9..9a3534cfc4 100644 --- a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs +++ b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs @@ -46,7 +46,11 @@ namespace Umbraco.Tests.Routing logger, null, // FIXME: PublishedRouter complexities... Mock.Of(), - Mock.Of() + Mock.Of(), + new Umbraco.Web.Cache.BackgroundPublishedSnapshotServiceNotifier( + Factory.GetInstance(), + Factory.GetInstance(), + Logger) ); runtime.Level = RuntimeLevel.Run; diff --git a/src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotServiceNotifier.cs similarity index 57% rename from src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs rename to src/Umbraco.Web/Cache/BackgroundPublishedSnapshotServiceNotifier.cs index d45c71d04d..725b1389ce 100644 --- a/src/Umbraco.Web/Cache/BackgroundSafeLiveFactory.cs +++ b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotServiceNotifier.cs @@ -8,29 +8,62 @@ using Umbraco.Web.Scheduling; namespace Umbraco.Web.Cache { - public sealed class BackgroundSafeLiveFactory + /// + /// Used to notify the of changes using a background thread + /// + /// + /// When in Pure Live mode, the models need to be rebuilt before the IPublishedSnapshotService is notified which can result in performance penalties so + /// this performs these actions on a background thread so the user isn't waiting for the rebuilding to occur on a UI thread. + /// + public sealed class BackgroundPublishedSnapshotServiceNotifier { private readonly IPublishedModelFactory _publishedModelFactory; private readonly IPublishedSnapshotService _publishedSnapshotService; - private BackgroundTaskRunner _runner; + private readonly BackgroundTaskRunner _runner; - public BackgroundSafeLiveFactory(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService, ILogger logger) + /// + /// Constructor + /// + /// + /// + /// + public BackgroundPublishedSnapshotServiceNotifier(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService, ILogger logger) { _publishedModelFactory = publishedModelFactory; _publishedSnapshotService = publishedSnapshotService; + + // TODO: We have the option to check if we are in live mode and only run on a background thread if that is the case, else run normally? + // IMO I think we should just always run on a background thread, then no matter what state the app is in, it's always doing a consistent operation. + _runner = new BackgroundTaskRunner("RebuildModelsAndCache", logger); } - public void Execute(ContentTypeCacheRefresher.JsonPayload[] payloads) + /// + /// Blocks until the background operation is completed + /// + public void Wait() => _runner.StoppedAwaitable.GetAwaiter().GetResult(); //TODO: do we need a try/catch? + + /// + /// Notify the of content type changes + /// + /// + public void NotifyWithSafeLiveFactory(ContentTypeCacheRefresher.JsonPayload[] payloads) { _runner.TryAdd(new RebuildModelsAndCacheTask(payloads, _publishedModelFactory, _publishedSnapshotService)); } - public void Execute(DataTypeCacheRefresher.JsonPayload[] payloads) + /// + /// Notify the of data type changes + /// + /// + public void NotifyWithSafeLiveFactory(DataTypeCacheRefresher.JsonPayload[] payloads) { _runner.TryAdd(new RebuildModelsAndCacheTask(payloads, _publishedModelFactory, _publishedSnapshotService)); } + /// + /// A simple background task that notifies the of changes + /// private class RebuildModelsAndCacheTask : IBackgroundTask { private readonly DataTypeCacheRefresher.JsonPayload[] _dataTypePayloads; @@ -71,10 +104,7 @@ namespace Umbraco.Web.Cache }); } - public Task RunAsync(CancellationToken token) - { - throw new System.NotImplementedException(); - } + public Task RunAsync(CancellationToken token) => throw new System.NotImplementedException(); public bool IsAsync => false; diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index 490adeb1d9..8ff82b2644 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -10,11 +10,11 @@ namespace Umbraco.Web.Cache { public sealed class ContentTypeCacheRefresher : PayloadCacheRefresherBase { - private readonly BackgroundSafeLiveFactory _backgroundModelFactory; + private readonly BackgroundPublishedSnapshotServiceNotifier _backgroundModelFactory; private readonly IContentTypeCommonRepository _contentTypeCommonRepository; private readonly IdkMap _idkMap; - public ContentTypeCacheRefresher(AppCaches appCaches, BackgroundSafeLiveFactory backgroundModelFactory, IdkMap idkMap, IContentTypeCommonRepository contentTypeCommonRepository) + public ContentTypeCacheRefresher(AppCaches appCaches, BackgroundPublishedSnapshotServiceNotifier backgroundModelFactory, IdkMap idkMap, IContentTypeCommonRepository contentTypeCommonRepository) : base(appCaches) { _backgroundModelFactory = backgroundModelFactory; @@ -80,7 +80,7 @@ namespace Umbraco.Web.Cache MemberCacheRefresher.RefreshMemberTypes(AppCaches); // refresh the models and cache - _backgroundModelFactory.Execute(payloads); + _backgroundModelFactory.NotifyWithSafeLiveFactory(payloads); // now we can trigger the event base.Refresh(payloads); diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index c777da1971..5363182a08 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -9,10 +9,10 @@ namespace Umbraco.Web.Cache public sealed class DataTypeCacheRefresher : PayloadCacheRefresherBase { - private readonly BackgroundSafeLiveFactory _backgroundModelFactory; + private readonly BackgroundPublishedSnapshotServiceNotifier _backgroundModelFactory; private readonly IdkMap _idkMap; - public DataTypeCacheRefresher(AppCaches appCaches, BackgroundSafeLiveFactory backgroundModelFactory, IdkMap idkMap) + public DataTypeCacheRefresher(AppCaches appCaches, BackgroundPublishedSnapshotServiceNotifier backgroundModelFactory, IdkMap idkMap) : base(appCaches) { _backgroundModelFactory = backgroundModelFactory; @@ -58,7 +58,7 @@ namespace Umbraco.Web.Cache SliderValueConverter.ClearCaches(); // refresh the models and cache - _backgroundModelFactory.Execute(payloads); + _backgroundModelFactory.NotifyWithSafeLiveFactory(payloads); base.Refresh(payloads); } diff --git a/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs b/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs index a68e137665..7850125970 100644 --- a/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs +++ b/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs @@ -1,12 +1,8 @@ using System; using System.Threading; using Umbraco.Core; -using Umbraco.Core.Compose; using Umbraco.Core.Composing; -using Umbraco.Core.Configuration; using Umbraco.Core.Logging; -using Umbraco.Core.Persistence; -using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Services.Changes; using Umbraco.Core.Sync; diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index 13d2a6b74c..d59caff63c 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -125,7 +125,7 @@ namespace Umbraco.Web.Runtime // register distributed cache composition.RegisterUnique(f => new DistributedCache()); - composition.RegisterUnique(); + composition.RegisterUnique(); // replace some services composition.RegisterUnique(); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index c7a576c94f..b5641e8aae 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -115,7 +115,7 @@ - + diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index db5372efa2..b16d86a8ba 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -18,6 +18,7 @@ using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Composing; using Umbraco.Web.PublishedCache; +using Umbraco.Web.Cache; namespace Umbraco.Web { @@ -48,6 +49,7 @@ namespace Umbraco.Web private readonly IPublishedRouter _publishedRouter; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IUmbracoContextFactory _umbracoContextFactory; + private readonly BackgroundPublishedSnapshotServiceNotifier _backgroundNotifier; public UmbracoInjectedModule( IGlobalSettings globalSettings, @@ -59,7 +61,8 @@ namespace Umbraco.Web ILogger logger, IPublishedRouter publishedRouter, IVariationContextAccessor variationContextAccessor, - IUmbracoContextFactory umbracoContextFactory) + IUmbracoContextFactory umbracoContextFactory, + BackgroundPublishedSnapshotServiceNotifier backgroundNotifier) { _combinedRouteCollection = new Lazy(CreateRouteCollection); @@ -73,6 +76,7 @@ namespace Umbraco.Web _publishedRouter = publishedRouter; _variationContextAccessor = variationContextAccessor; _umbracoContextFactory = umbracoContextFactory; + _backgroundNotifier = backgroundNotifier; } #region HttpModule event handlers @@ -136,6 +140,10 @@ namespace Umbraco.Web // do not process if this request is not a front-end routable page var isRoutableAttempt = EnsureUmbracoRoutablePage(umbracoContext, httpContext); + // If this page is probably front-end routable, block here until the backround notifier isn't busy + if (isRoutableAttempt) + _backgroundNotifier.Wait(); + // raise event here UmbracoModule.OnRouteAttempt(this, new RoutableAttemptEventArgs(isRoutableAttempt.Result, umbracoContext, httpContext)); if (isRoutableAttempt.Success == false) return; From 296afb18a46367dcd85d20e30eccd535da9ecb59 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 11:50:09 +1000 Subject: [PATCH 03/40] Renames and puts in temp fix to try to find out why we are binding to old models --- .../Routing/UmbracoModuleTests.cs | 2 +- ...=> BackgroundPublishedSnapshotNotifier.cs} | 20 +++++++++++++++---- .../Cache/ContentTypeCacheRefresher.cs | 4 ++-- .../Cache/DataTypeCacheRefresher.cs | 4 ++-- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- src/Umbraco.Web/UmbracoInjectedModule.cs | 20 +++++++++++++++---- 7 files changed, 39 insertions(+), 15 deletions(-) rename src/Umbraco.Web/Cache/{BackgroundPublishedSnapshotServiceNotifier.cs => BackgroundPublishedSnapshotNotifier.cs} (85%) diff --git a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs index 9a3534cfc4..34ac79a219 100644 --- a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs +++ b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs @@ -47,7 +47,7 @@ namespace Umbraco.Tests.Routing null, // FIXME: PublishedRouter complexities... Mock.Of(), Mock.Of(), - new Umbraco.Web.Cache.BackgroundPublishedSnapshotServiceNotifier( + new Umbraco.Web.Cache.BackgroundPublishedSnapshotNotifier( Factory.GetInstance(), Factory.GetInstance(), Logger) diff --git a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotServiceNotifier.cs b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs similarity index 85% rename from src/Umbraco.Web/Cache/BackgroundPublishedSnapshotServiceNotifier.cs rename to src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs index 725b1389ce..f1da2b6375 100644 --- a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotServiceNotifier.cs +++ b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs @@ -13,9 +13,11 @@ namespace Umbraco.Web.Cache /// /// /// When in Pure Live mode, the models need to be rebuilt before the IPublishedSnapshotService is notified which can result in performance penalties so - /// this performs these actions on a background thread so the user isn't waiting for the rebuilding to occur on a UI thread. + /// this performs these actions on a background thread so the user isn't waiting for the rebuilding to occur on a UI thread. + /// When using this, even when not in Pure Live mode, it still means that the cache is notified on a background thread. + /// In order to wait for the processing to complete, this class has a Wait() method which will block until the processing is finished. /// - public sealed class BackgroundPublishedSnapshotServiceNotifier + public sealed class BackgroundPublishedSnapshotNotifier { private readonly IPublishedModelFactory _publishedModelFactory; private readonly IPublishedSnapshotService _publishedSnapshotService; @@ -27,7 +29,7 @@ namespace Umbraco.Web.Cache /// /// /// - public BackgroundPublishedSnapshotServiceNotifier(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService, ILogger logger) + public BackgroundPublishedSnapshotNotifier(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService, ILogger logger) { _publishedModelFactory = publishedModelFactory; _publishedSnapshotService = publishedSnapshotService; @@ -41,7 +43,13 @@ namespace Umbraco.Web.Cache /// /// Blocks until the background operation is completed /// - public void Wait() => _runner.StoppedAwaitable.GetAwaiter().GetResult(); //TODO: do we need a try/catch? + /// Returns true if waiting was necessary + public bool Wait() + { + var running = _runner.IsRunning; + _runner.StoppedAwaitable.GetAwaiter().GetResult(); //TODO: do we need a try/catch? + return running; + } /// /// Notify the of content type changes @@ -101,6 +109,10 @@ namespace Umbraco.Web.Cache _publishedSnapshotService.Notify(_dataTypePayloads); if (_contentTypePayloads != null) _publishedSnapshotService.Notify(_contentTypePayloads); + + //Thread.Sleep(10000); + + var asdf = ""; }); } diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index 8ff82b2644..0d38f38362 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -10,11 +10,11 @@ namespace Umbraco.Web.Cache { public sealed class ContentTypeCacheRefresher : PayloadCacheRefresherBase { - private readonly BackgroundPublishedSnapshotServiceNotifier _backgroundModelFactory; + private readonly BackgroundPublishedSnapshotNotifier _backgroundModelFactory; private readonly IContentTypeCommonRepository _contentTypeCommonRepository; private readonly IdkMap _idkMap; - public ContentTypeCacheRefresher(AppCaches appCaches, BackgroundPublishedSnapshotServiceNotifier backgroundModelFactory, IdkMap idkMap, IContentTypeCommonRepository contentTypeCommonRepository) + public ContentTypeCacheRefresher(AppCaches appCaches, BackgroundPublishedSnapshotNotifier backgroundModelFactory, IdkMap idkMap, IContentTypeCommonRepository contentTypeCommonRepository) : base(appCaches) { _backgroundModelFactory = backgroundModelFactory; diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index 5363182a08..97367be495 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -9,10 +9,10 @@ namespace Umbraco.Web.Cache public sealed class DataTypeCacheRefresher : PayloadCacheRefresherBase { - private readonly BackgroundPublishedSnapshotServiceNotifier _backgroundModelFactory; + private readonly BackgroundPublishedSnapshotNotifier _backgroundModelFactory; private readonly IdkMap _idkMap; - public DataTypeCacheRefresher(AppCaches appCaches, BackgroundPublishedSnapshotServiceNotifier backgroundModelFactory, IdkMap idkMap) + public DataTypeCacheRefresher(AppCaches appCaches, BackgroundPublishedSnapshotNotifier backgroundModelFactory, IdkMap idkMap) : base(appCaches) { _backgroundModelFactory = backgroundModelFactory; diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index d59caff63c..4b4fb488e5 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -125,7 +125,7 @@ namespace Umbraco.Web.Runtime // register distributed cache composition.RegisterUnique(f => new DistributedCache()); - composition.RegisterUnique(); + composition.RegisterUnique(); // replace some services composition.RegisterUnique(); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index b5641e8aae..1a23ae7f65 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -115,7 +115,7 @@ - + diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index b16d86a8ba..99d167fbd3 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -19,6 +19,7 @@ using Umbraco.Core.Services; using Umbraco.Web.Composing; using Umbraco.Web.PublishedCache; using Umbraco.Web.Cache; +using Umbraco.Web.PublishedCache.NuCache; namespace Umbraco.Web { @@ -49,7 +50,7 @@ namespace Umbraco.Web private readonly IPublishedRouter _publishedRouter; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IUmbracoContextFactory _umbracoContextFactory; - private readonly BackgroundPublishedSnapshotServiceNotifier _backgroundNotifier; + private readonly BackgroundPublishedSnapshotNotifier _backgroundNotifier; public UmbracoInjectedModule( IGlobalSettings globalSettings, @@ -62,7 +63,7 @@ namespace Umbraco.Web IPublishedRouter publishedRouter, IVariationContextAccessor variationContextAccessor, IUmbracoContextFactory umbracoContextFactory, - BackgroundPublishedSnapshotServiceNotifier backgroundNotifier) + BackgroundPublishedSnapshotNotifier backgroundNotifier) { _combinedRouteCollection = new Lazy(CreateRouteCollection); @@ -141,8 +142,19 @@ namespace Umbraco.Web var isRoutableAttempt = EnsureUmbracoRoutablePage(umbracoContext, httpContext); // If this page is probably front-end routable, block here until the backround notifier isn't busy - if (isRoutableAttempt) - _backgroundNotifier.Wait(); + if (isRoutableAttempt) + { + //wait for the notifier to complete if it's in progress + if (_backgroundNotifier.Wait()) + { + // if we were waiting, we need to resync the snapshot + // TODO: This does not belong here! BUT we cannot Resync the snapshot on the background process because there is no snapshot... + // normally it would do that automatically but on a background thread it is null ... hrm.... + ((PublishedSnapshot)_publishedSnapshotService.PublishedSnapshotAccessor.PublishedSnapshot)?.Resync(); + var done = "done"; + } + } + // raise event here UmbracoModule.OnRouteAttempt(this, new RoutableAttemptEventArgs(isRoutableAttempt.Result, umbracoContext, httpContext)); From 596c745a19b604e444e505a09e4c913c7b6eb61f Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 13:35:59 +1000 Subject: [PATCH 04/40] Uses an event on UmbracoContext for when the snapshot is created so we can block until the background notifier is completed --- .../BackgroundPublishedSnapshotNotifier.cs | 4 -- src/Umbraco.Web/UmbracoContext.cs | 11 +++- src/Umbraco.Web/UmbracoInjectedModule.cs | 55 ++++++------------- 3 files changed, 28 insertions(+), 42 deletions(-) diff --git a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs index f1da2b6375..5efbf988a4 100644 --- a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs +++ b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs @@ -109,10 +109,6 @@ namespace Umbraco.Web.Cache _publishedSnapshotService.Notify(_dataTypePayloads); if (_contentTypePayloads != null) _publishedSnapshotService.Notify(_contentTypePayloads); - - //Thread.Sleep(10000); - - var asdf = ""; }); } diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index c0306e0a81..e4a921375e 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -59,7 +59,11 @@ namespace Umbraco.Web Security = webSecurity; // beware - we cannot expect a current user here, so detecting preview mode must be a lazy thing - _publishedSnapshot = new Lazy(() => publishedSnapshotService.CreatePublishedSnapshot(PreviewToken)); + _publishedSnapshot = new Lazy(() => + { + CreatingPublishedSnapshot?.Invoke(this, new EventArgs()); + return publishedSnapshotService.CreatePublishedSnapshot(PreviewToken); + }); // set the urls... // NOTE: The request will not be available during app startup so we can only set this to an absolute URL of localhost, this @@ -73,6 +77,11 @@ namespace Umbraco.Web UrlProvider = new UrlProvider(this, umbracoSettings.WebRouting, urlProviders, mediaUrlProviders, variationContextAccessor); } + /// + /// Raised when the published snapshot is being created + /// + internal event EventHandler CreatingPublishedSnapshot; + /// /// This is used internally for performance calculations, the ObjectCreated DateTime is set as soon as this /// object is instantiated which in the web site is created during the BeginRequest phase. diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 99d167fbd3..cbeee34444 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -2,7 +2,6 @@ using System.Collections; using System.Collections.Generic; using System.IO; -using System.Text; using System.Web; using System.Web.Routing; using Umbraco.Core; @@ -10,16 +9,10 @@ using Umbraco.Core.Configuration; using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Web.Routing; -using Umbraco.Web.Security; using Umbraco.Core.Exceptions; -using Umbraco.Core.Models.PublishedContent; -using Umbraco.Core.Persistence.FaultHandling; using Umbraco.Core.Security; -using Umbraco.Core.Services; using Umbraco.Web.Composing; -using Umbraco.Web.PublishedCache; using Umbraco.Web.Cache; -using Umbraco.Web.PublishedCache.NuCache; namespace Umbraco.Web { @@ -41,41 +34,26 @@ namespace Umbraco.Web public class UmbracoInjectedModule : IHttpModule { private readonly IGlobalSettings _globalSettings; - private readonly IUmbracoContextAccessor _umbracoContextAccessor; - private readonly IPublishedSnapshotService _publishedSnapshotService; - private readonly IUserService _userService; - private readonly UrlProviderCollection _urlProviders; private readonly IRuntimeState _runtime; private readonly ILogger _logger; private readonly IPublishedRouter _publishedRouter; - private readonly IVariationContextAccessor _variationContextAccessor; private readonly IUmbracoContextFactory _umbracoContextFactory; private readonly BackgroundPublishedSnapshotNotifier _backgroundNotifier; public UmbracoInjectedModule( IGlobalSettings globalSettings, - IUmbracoContextAccessor umbracoContextAccessor, - IPublishedSnapshotService publishedSnapshotService, - IUserService userService, - UrlProviderCollection urlProviders, IRuntimeState runtime, ILogger logger, IPublishedRouter publishedRouter, - IVariationContextAccessor variationContextAccessor, IUmbracoContextFactory umbracoContextFactory, BackgroundPublishedSnapshotNotifier backgroundNotifier) { _combinedRouteCollection = new Lazy(CreateRouteCollection); _globalSettings = globalSettings; - _umbracoContextAccessor = umbracoContextAccessor; - _publishedSnapshotService = publishedSnapshotService; - _userService = userService; - _urlProviders = urlProviders; _runtime = runtime; _logger = logger; _publishedRouter = publishedRouter; - _variationContextAccessor = variationContextAccessor; _umbracoContextFactory = umbracoContextFactory; _backgroundNotifier = backgroundNotifier; } @@ -104,9 +82,27 @@ namespace Umbraco.Web // ensure there's an UmbracoContext registered for the current request // registers the context reference so its disposed at end of request var umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext(httpContext); + umbracoContextReference.UmbracoContext.CreatingPublishedSnapshot += UmbracoContext_CreatingPublishedSnapshot; httpContext.DisposeOnPipelineCompleted(umbracoContextReference); } + /// + /// Event handler for when the UmbracoContext creates the published snapshot + /// + /// + /// + private void UmbracoContext_CreatingPublishedSnapshot(object sender, EventArgs e) + { + // Wait for the notifier to complete if it's in progress, this is required because + // Pure Live models along with content snapshots are updated on a background thread when schema + // (doc types, data types) are changed so if that is still processing we need to wait before we access + // the content snapshot to make sure it's the latest version. + if (_backgroundNotifier.Wait()) + { + _logger.Debug("Request was suspended while waiting for background cache notifications to complete"); + } + } + /// /// Processes the Umbraco Request /// @@ -141,21 +137,6 @@ namespace Umbraco.Web // do not process if this request is not a front-end routable page var isRoutableAttempt = EnsureUmbracoRoutablePage(umbracoContext, httpContext); - // If this page is probably front-end routable, block here until the backround notifier isn't busy - if (isRoutableAttempt) - { - //wait for the notifier to complete if it's in progress - if (_backgroundNotifier.Wait()) - { - // if we were waiting, we need to resync the snapshot - // TODO: This does not belong here! BUT we cannot Resync the snapshot on the background process because there is no snapshot... - // normally it would do that automatically but on a background thread it is null ... hrm.... - ((PublishedSnapshot)_publishedSnapshotService.PublishedSnapshotAccessor.PublishedSnapshot)?.Resync(); - var done = "done"; - } - } - - // raise event here UmbracoModule.OnRouteAttempt(this, new RoutableAttemptEventArgs(isRoutableAttempt.Result, umbracoContext, httpContext)); if (isRoutableAttempt.Success == false) return; From b997281e1f5b77fd362ac97f8a5295f65fafb702 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 14:10:26 +1000 Subject: [PATCH 05/40] Only waits for background processing to finish if it's a front-end request, required caching the check for a front-end request which required moving the check to it's own class. --- src/Umbraco.Web/RoutableDocumentLookup.cs | 109 ++++++++++++++++++ src/Umbraco.Web/Runtime/WebInitialComposer.cs | 1 + src/Umbraco.Web/Umbraco.Web.csproj | 1 + src/Umbraco.Web/UmbracoContext.cs | 24 +++- src/Umbraco.Web/UmbracoInjectedModule.cs | 102 +++------------- 5 files changed, 148 insertions(+), 89 deletions(-) create mode 100644 src/Umbraco.Web/RoutableDocumentLookup.cs diff --git a/src/Umbraco.Web/RoutableDocumentLookup.cs b/src/Umbraco.Web/RoutableDocumentLookup.cs new file mode 100644 index 0000000000..bc75798b09 --- /dev/null +++ b/src/Umbraco.Web/RoutableDocumentLookup.cs @@ -0,0 +1,109 @@ +using System; +using System.IO; +using System.Web; +using System.Web.Routing; +using Umbraco.Core; +using Umbraco.Core.Logging; +using Umbraco.Core.Configuration; + +namespace Umbraco.Web +{ + /// + /// Utility class used to check if the current request is for a front-end request + /// + /// + /// There are various checks to determine if this is a front-end request such as checking if the request is part of any reserved paths or existing MVC routes. + /// + public sealed class RoutableDocumentLookup + { + public RoutableDocumentLookup(ILogger logger, IGlobalSettings globalSettings) + { + _logger = logger; + _globalSettings = globalSettings; + _combinedRouteCollection = new Lazy(CreateRouteCollection); + } + + /// + /// Checks if the request is a document request (i.e. one that the module should handle) + /// + /// + /// + /// + public bool IsDocumentRequest(HttpContextBase httpContext, Uri uri) + { + var maybeDoc = true; + var lpath = uri.AbsolutePath.ToLowerInvariant(); + + // handle directory-urls used for asmx + // TODO: legacy - what's the point really? + var asmxPos = lpath.IndexOf(".asmx/", StringComparison.OrdinalIgnoreCase); + if (asmxPos >= 0) + { + // use uri.AbsolutePath, not path, 'cos path has been lowercased + httpContext.RewritePath(uri.AbsolutePath.Substring(0, asmxPos + 5), // filePath + uri.AbsolutePath.Substring(asmxPos + 5), // pathInfo + uri.Query.TrimStart('?')); + maybeDoc = false; + } + + // a document request should be + // /foo/bar/nil + // /foo/bar/nil/ + // /foo/bar/nil.aspx + // where /foo is not a reserved path + + // if the path contains an extension that is not .aspx + // then it cannot be a document request + var extension = Path.GetExtension(lpath); + if (maybeDoc && extension.IsNullOrWhiteSpace() == false && extension != ".aspx") + maybeDoc = false; + + // at that point, either we have no extension, or it is .aspx + + // if the path is reserved then it cannot be a document request + if (maybeDoc && _globalSettings.IsReservedPathOrUrl(lpath, httpContext, _combinedRouteCollection.Value)) + maybeDoc = false; + + //NOTE: No need to warn, plus if we do we should log the document, as this message doesn't really tell us anything :) + //if (!maybeDoc) + //{ + // Logger.Warn("Not a document"); + //} + return maybeDoc; + } + + /// + /// This is used to be passed into the GlobalSettings.IsReservedPathOrUrl and will include some 'fake' routes + /// used to determine if a path is reserved. + /// + /// + /// This is basically used to reserve paths dynamically + /// + private readonly Lazy _combinedRouteCollection; + private readonly ILogger _logger; + private readonly IGlobalSettings _globalSettings; + + private RouteCollection CreateRouteCollection() + { + var routes = new RouteCollection(); + + foreach (var route in RouteTable.Routes) + routes.Add(route); + + foreach (var reservedPath in UmbracoModule.ReservedPaths) + { + try + { + routes.Add("_umbreserved_" + reservedPath.ReplaceNonAlphanumericChars(""), + new Route(reservedPath.TrimStart('/'), new StopRoutingHandler())); + } + catch (Exception ex) + { + _logger.Error("Could not add reserved path route", ex); + } + } + + return routes; + } + } +} diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index 4b4fb488e5..7c353daa1d 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -126,6 +126,7 @@ namespace Umbraco.Web.Runtime composition.RegisterUnique(f => new DistributedCache()); composition.RegisterUnique(); + composition.RegisterUnique(); // replace some services composition.RegisterUnique(); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 1a23ae7f65..2cdff55e2d 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -230,6 +230,7 @@ + diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index e4a921375e..95a96be072 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -1,16 +1,21 @@ using System; using System.Collections.Generic; +using System.IO; using System.Web; +using System.Web.Routing; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Events; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Web; using Umbraco.Web.PublishedCache; using Umbraco.Web.Routing; using Umbraco.Web.Security; namespace Umbraco.Web { + /// /// Class that encapsulates Umbraco information of a specific HTTP request /// @@ -80,7 +85,7 @@ namespace Umbraco.Web /// /// Raised when the published snapshot is being created /// - internal event EventHandler CreatingPublishedSnapshot; + internal event TypedEventHandler CreatingPublishedSnapshot; /// /// This is used internally for performance calculations, the ObjectCreated DateTime is set as soon as this @@ -295,6 +300,23 @@ namespace Umbraco.Web _previewing = _previewToken.IsNullOrWhiteSpace() == false; } + private bool? _isDocumentRequest; + + /// + /// Checks if the request is a document request (i.e. one that the module should handle) + /// + /// + /// + /// + internal bool IsDocumentRequest(RoutableDocumentLookup docLookup) + { + if (_isDocumentRequest.HasValue) + return _isDocumentRequest.Value; + + _isDocumentRequest = docLookup.IsDocumentRequest(HttpContext, OriginalRequestUrl); + return _isDocumentRequest.Value; + } + // say we render a macro or RTE in a give 'preview' mode that might not be the 'current' one, // then due to the way it all works at the moment, the 'current' published snapshot need to be in the proper // default 'preview' mode - somehow we have to force it. and that could be recursive. diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index cbeee34444..70d9222dfc 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -39,6 +39,7 @@ namespace Umbraco.Web private readonly IPublishedRouter _publishedRouter; private readonly IUmbracoContextFactory _umbracoContextFactory; private readonly BackgroundPublishedSnapshotNotifier _backgroundNotifier; + private readonly RoutableDocumentLookup _routableDocumentLookup; public UmbracoInjectedModule( IGlobalSettings globalSettings, @@ -46,16 +47,16 @@ namespace Umbraco.Web ILogger logger, IPublishedRouter publishedRouter, IUmbracoContextFactory umbracoContextFactory, - BackgroundPublishedSnapshotNotifier backgroundNotifier) + BackgroundPublishedSnapshotNotifier backgroundNotifier, + RoutableDocumentLookup routableDocumentLookup) { - _combinedRouteCollection = new Lazy(CreateRouteCollection); - _globalSettings = globalSettings; _runtime = runtime; _logger = logger; _publishedRouter = publishedRouter; _umbracoContextFactory = umbracoContextFactory; _backgroundNotifier = backgroundNotifier; + _routableDocumentLookup = routableDocumentLookup; } #region HttpModule event handlers @@ -91,13 +92,16 @@ namespace Umbraco.Web /// /// /// - private void UmbracoContext_CreatingPublishedSnapshot(object sender, EventArgs e) + private void UmbracoContext_CreatingPublishedSnapshot(UmbracoContext sender, EventArgs e) { // Wait for the notifier to complete if it's in progress, this is required because // Pure Live models along with content snapshots are updated on a background thread when schema // (doc types, data types) are changed so if that is still processing we need to wait before we access // the content snapshot to make sure it's the latest version. - if (_backgroundNotifier.Wait()) + // We only want to wait if this is a front-end request (not a back office request) so need to first check + // for that and then wait. + + if (sender.IsDocumentRequest(_routableDocumentLookup) &&_backgroundNotifier.Wait()) { _logger.Debug("Request was suspended while waiting for background cache notifications to complete"); } @@ -183,18 +187,18 @@ namespace Umbraco.Web var reason = EnsureRoutableOutcome.IsRoutable; // ensure this is a document request - if (EnsureDocumentRequest(httpContext, uri) == false) + if (!context.IsDocumentRequest(_routableDocumentLookup)) { reason = EnsureRoutableOutcome.NotDocumentRequest; } // ensure the runtime is in the proper state // and deal with needed redirects, etc - else if (EnsureRuntime(httpContext, uri) == false) + else if (!EnsureRuntime(httpContext, uri)) { reason = EnsureRoutableOutcome.NotReady; } // ensure Umbraco has documents to serve - else if (EnsureHasContent(context, httpContext) == false) + else if (!EnsureHasContent(context, httpContext)) { reason = EnsureRoutableOutcome.NoContent; } @@ -202,55 +206,7 @@ namespace Umbraco.Web return Attempt.If(reason == EnsureRoutableOutcome.IsRoutable, reason); } - /// - /// Ensures that the request is a document request (i.e. one that the module should handle) - /// - /// - /// - /// - private bool EnsureDocumentRequest(HttpContextBase httpContext, Uri uri) - { - var maybeDoc = true; - var lpath = uri.AbsolutePath.ToLowerInvariant(); - - // handle directory-urls used for asmx - // TODO: legacy - what's the point really? - var asmxPos = lpath.IndexOf(".asmx/", StringComparison.OrdinalIgnoreCase); - if (asmxPos >= 0) - { - // use uri.AbsolutePath, not path, 'cos path has been lowercased - httpContext.RewritePath(uri.AbsolutePath.Substring(0, asmxPos + 5), // filePath - uri.AbsolutePath.Substring(asmxPos + 5), // pathInfo - uri.Query.TrimStart('?')); - maybeDoc = false; - } - - // a document request should be - // /foo/bar/nil - // /foo/bar/nil/ - // /foo/bar/nil.aspx - // where /foo is not a reserved path - - // if the path contains an extension that is not .aspx - // then it cannot be a document request - var extension = Path.GetExtension(lpath); - if (maybeDoc && extension.IsNullOrWhiteSpace() == false && extension != ".aspx") - maybeDoc = false; - - // at that point, either we have no extension, or it is .aspx - - // if the path is reserved then it cannot be a document request - if (maybeDoc && _globalSettings.IsReservedPathOrUrl(lpath, httpContext, _combinedRouteCollection.Value)) - maybeDoc = false; - - //NOTE: No need to warn, plus if we do we should log the document, as this message doesn't really tell us anything :) - //if (!maybeDoc) - //{ - // Logger.Warn("Not a document"); - //} - - return maybeDoc; - } + private bool EnsureRuntime(HttpContextBase httpContext, Uri uri) { @@ -506,36 +462,6 @@ namespace Umbraco.Web #endregion - /// - /// This is used to be passed into the GlobalSettings.IsReservedPathOrUrl and will include some 'fake' routes - /// used to determine if a path is reserved. - /// - /// - /// This is basically used to reserve paths dynamically - /// - private readonly Lazy _combinedRouteCollection; - - private RouteCollection CreateRouteCollection() - { - var routes = new RouteCollection(); - - foreach (var route in RouteTable.Routes) - routes.Add(route); - - foreach (var reservedPath in UmbracoModule.ReservedPaths) - { - try - { - routes.Add("_umbreserved_" + reservedPath.ReplaceNonAlphanumericChars(""), - new Route(reservedPath.TrimStart('/'), new StopRoutingHandler())); - } - catch (Exception ex) - { - _logger.Error("Could not add reserved path route", ex); - } - } - - return routes; - } + } } From 44fbdb7028df83184f959118f8888acf4f161a3f Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 14:23:42 +1000 Subject: [PATCH 06/40] renames class --- .../{RoutableDocumentLookup.cs => RoutableDocumentFilter.cs} | 4 ++-- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- src/Umbraco.Web/UmbracoContext.cs | 2 +- src/Umbraco.Web/UmbracoInjectedModule.cs | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) rename src/Umbraco.Web/{RoutableDocumentLookup.cs => RoutableDocumentFilter.cs} (97%) diff --git a/src/Umbraco.Web/RoutableDocumentLookup.cs b/src/Umbraco.Web/RoutableDocumentFilter.cs similarity index 97% rename from src/Umbraco.Web/RoutableDocumentLookup.cs rename to src/Umbraco.Web/RoutableDocumentFilter.cs index bc75798b09..3f666edb47 100644 --- a/src/Umbraco.Web/RoutableDocumentLookup.cs +++ b/src/Umbraco.Web/RoutableDocumentFilter.cs @@ -14,9 +14,9 @@ namespace Umbraco.Web /// /// There are various checks to determine if this is a front-end request such as checking if the request is part of any reserved paths or existing MVC routes. /// - public sealed class RoutableDocumentLookup + public sealed class RoutableDocumentFilter { - public RoutableDocumentLookup(ILogger logger, IGlobalSettings globalSettings) + public RoutableDocumentFilter(ILogger logger, IGlobalSettings globalSettings) { _logger = logger; _globalSettings = globalSettings; diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index 7c353daa1d..9e3413ba4d 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -126,7 +126,7 @@ namespace Umbraco.Web.Runtime composition.RegisterUnique(f => new DistributedCache()); composition.RegisterUnique(); - composition.RegisterUnique(); + composition.RegisterUnique(); // replace some services composition.RegisterUnique(); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 2cdff55e2d..0b1c3c6fe9 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -230,7 +230,7 @@ - + diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index 95a96be072..0f8a9575b4 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -308,7 +308,7 @@ namespace Umbraco.Web /// /// /// - internal bool IsDocumentRequest(RoutableDocumentLookup docLookup) + internal bool IsDocumentRequest(RoutableDocumentFilter docLookup) { if (_isDocumentRequest.HasValue) return _isDocumentRequest.Value; diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 70d9222dfc..7f10701e36 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -39,7 +39,7 @@ namespace Umbraco.Web private readonly IPublishedRouter _publishedRouter; private readonly IUmbracoContextFactory _umbracoContextFactory; private readonly BackgroundPublishedSnapshotNotifier _backgroundNotifier; - private readonly RoutableDocumentLookup _routableDocumentLookup; + private readonly RoutableDocumentFilter _routableDocumentLookup; public UmbracoInjectedModule( IGlobalSettings globalSettings, @@ -48,7 +48,7 @@ namespace Umbraco.Web IPublishedRouter publishedRouter, IUmbracoContextFactory umbracoContextFactory, BackgroundPublishedSnapshotNotifier backgroundNotifier, - RoutableDocumentLookup routableDocumentLookup) + RoutableDocumentFilter routableDocumentLookup) { _globalSettings = globalSettings; _runtime = runtime; From 0a12f86108000d2584534e7f6f9343a6a636a4f4 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 16:09:09 +1000 Subject: [PATCH 07/40] Moves the reserved path detection to RoutableDocumentFilter, adds caching to MVC route detection --- .../Configuration/GlobalSettings.cs | 1 - .../Configuration/GlobalSettingsExtensions.cs | 104 +------------ src/Umbraco.Web/RoutableDocumentFilter.cs | 147 ++++++++++++++---- .../AuthenticationOptionsExtensions.cs | 4 +- src/Umbraco.Web/UmbracoModule.cs | 5 - 5 files changed, 126 insertions(+), 135 deletions(-) diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs index 49f4481a59..a888e3c42b 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettings.cs @@ -33,7 +33,6 @@ namespace Umbraco.Core.Configuration /// private static void ResetInternal() { - GlobalSettingsExtensions.Reset(); _reservedPaths = null; _reservedUrls = null; HasSmtpServer = null; diff --git a/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs b/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs index 6bfb7ea904..fd4b57bf1a 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs @@ -1,6 +1,8 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Web; using System.Web.Routing; using Umbraco.Core.IO; @@ -9,21 +11,8 @@ namespace Umbraco.Core.Configuration { public static class GlobalSettingsExtensions { - /// - /// Used in unit testing to reset all config items, this is automatically called by GlobalSettings.Reset() - /// - internal static void Reset() - { - _reservedUrlsCache = null; - _mvcArea = null; - } - - private static readonly object Locker = new object(); - //make this volatile so that we can ensure thread safety with a double check lock - private static volatile string _reservedUrlsCache; - private static string _reservedPathsCache; - private static HashSet _reservedList = new HashSet(); private static string _mvcArea; + /// /// This returns the string of the MVC Area route. @@ -51,92 +40,5 @@ namespace Umbraco.Core.Configuration _mvcArea = path.TrimStart('~').TrimStart('/').Replace('/', '-').Trim().ToLower(); return _mvcArea; } - - /// - /// Determines whether the specified URL is reserved or is inside a reserved path. - /// - /// - /// The URL to check. - /// - /// true if the specified URL is reserved; otherwise, false. - /// - internal static bool IsReservedPathOrUrl(this IGlobalSettings globalSettings, string url) - { - if (_reservedUrlsCache == null) - { - lock (Locker) - { - if (_reservedUrlsCache == null) - { - // store references to strings to determine changes - _reservedPathsCache = globalSettings.ReservedPaths; - _reservedUrlsCache = globalSettings.ReservedUrls; - - // add URLs and paths to a new list - var newReservedList = new HashSet(); - foreach (var reservedUrlTrimmed in _reservedUrlsCache - .Split(new[] {","}, StringSplitOptions.RemoveEmptyEntries) - .Select(x => x.Trim().ToLowerInvariant()) - .Where(x => x.IsNullOrWhiteSpace() == false) - .Select(reservedUrl => IOHelper.ResolveUrl(reservedUrl).Trim().EnsureStartsWith("/")) - .Where(reservedUrlTrimmed => reservedUrlTrimmed.IsNullOrWhiteSpace() == false)) - { - newReservedList.Add(reservedUrlTrimmed); - } - - foreach (var reservedPathTrimmed in _reservedPathsCache - .Split(new[] {","}, StringSplitOptions.RemoveEmptyEntries) - .Select(x => x.Trim().ToLowerInvariant()) - .Where(x => x.IsNullOrWhiteSpace() == false) - .Select(reservedPath => IOHelper.ResolveUrl(reservedPath).Trim().EnsureStartsWith("/").EnsureEndsWith("/")) - .Where(reservedPathTrimmed => reservedPathTrimmed.IsNullOrWhiteSpace() == false)) - { - newReservedList.Add(reservedPathTrimmed); - } - - // use the new list from now on - _reservedList = newReservedList; - } - } - } - - //The url should be cleaned up before checking: - // * If it doesn't contain an '.' in the path then we assume it is a path based URL, if that is the case we should add an trailing '/' because all of our reservedPaths use a trailing '/' - // * We shouldn't be comparing the query at all - var pathPart = url.Split(new[] {'?'}, StringSplitOptions.RemoveEmptyEntries)[0].ToLowerInvariant(); - if (pathPart.Contains(".") == false) - { - pathPart = pathPart.EnsureEndsWith('/'); - } - - // return true if url starts with an element of the reserved list - return _reservedList.Any(x => pathPart.InvariantStartsWith(x)); - } - - /// - /// Determines whether the current request is reserved based on the route table and - /// whether the specified URL is reserved or is inside a reserved path. - /// - /// - /// - /// - /// The route collection to lookup the request in - /// - internal static bool IsReservedPathOrUrl(this IGlobalSettings globalSettings, string url, HttpContextBase httpContext, RouteCollection routes) - { - if (httpContext == null) throw new ArgumentNullException(nameof(httpContext)); - if (routes == null) throw new ArgumentNullException(nameof(routes)); - - //check if the current request matches a route, if so then it is reserved. - //TODO: This value should be cached! Else this is doing double routing in MVC every request! - var route = routes.GetRouteData(httpContext); - if (route != null) - return true; - - //continue with the standard ignore routine - return globalSettings.IsReservedPathOrUrl(url); - } - - } } diff --git a/src/Umbraco.Web/RoutableDocumentFilter.cs b/src/Umbraco.Web/RoutableDocumentFilter.cs index 3f666edb47..be2c8ad405 100644 --- a/src/Umbraco.Web/RoutableDocumentFilter.cs +++ b/src/Umbraco.Web/RoutableDocumentFilter.cs @@ -5,6 +5,12 @@ using System.Web.Routing; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Configuration; +using System.Threading; +using System.Collections.Generic; +using System.Linq; +using Umbraco.Core.IO; +using System.Collections.Concurrent; +using Umbraco.Core.Collections; namespace Umbraco.Web { @@ -16,13 +22,18 @@ namespace Umbraco.Web /// public sealed class RoutableDocumentFilter { - public RoutableDocumentFilter(ILogger logger, IGlobalSettings globalSettings) + public RoutableDocumentFilter(IGlobalSettings globalSettings) { - _logger = logger; _globalSettings = globalSettings; - _combinedRouteCollection = new Lazy(CreateRouteCollection); } + private static readonly ConcurrentDictionary _routeChecks = new ConcurrentDictionary(); + private readonly IGlobalSettings _globalSettings; + private object _locker = new object(); + private bool _isInit = false; + private int? _routeCount; + private HashSet _reservedList; + /// /// Checks if the request is a document request (i.e. one that the module should handle) /// @@ -61,7 +72,7 @@ namespace Umbraco.Web // at that point, either we have no extension, or it is .aspx // if the path is reserved then it cannot be a document request - if (maybeDoc && _globalSettings.IsReservedPathOrUrl(lpath, httpContext, _combinedRouteCollection.Value)) + if (maybeDoc && IsReservedPathOrUrl(lpath, httpContext, RouteTable.Routes)) maybeDoc = false; //NOTE: No need to warn, plus if we do we should log the document, as this message doesn't really tell us anything :) @@ -73,37 +84,121 @@ namespace Umbraco.Web } /// - /// This is used to be passed into the GlobalSettings.IsReservedPathOrUrl and will include some 'fake' routes - /// used to determine if a path is reserved. + /// Determines whether the specified URL is reserved or is inside a reserved path. /// - /// - /// This is basically used to reserve paths dynamically - /// - private readonly Lazy _combinedRouteCollection; - private readonly ILogger _logger; - private readonly IGlobalSettings _globalSettings; - - private RouteCollection CreateRouteCollection() + /// + /// The URL to check. + /// + /// true if the specified URL is reserved; otherwise, false. + /// + internal bool IsReservedPathOrUrl(string url) { - var routes = new RouteCollection(); - - foreach (var route in RouteTable.Routes) - routes.Add(route); - - foreach (var reservedPath in UmbracoModule.ReservedPaths) + LazyInitializer.EnsureInitialized(ref _reservedList, ref _isInit, ref _locker, () => { - try + // store references to strings to determine changes + var reservedPathsCache = _globalSettings.ReservedPaths; + var reservedUrlsCache = _globalSettings.ReservedUrls; + + // add URLs and paths to a new list + var newReservedList = new HashSet(); + foreach (var reservedUrlTrimmed in reservedUrlsCache + .Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries) + .Select(x => x.Trim().ToLowerInvariant()) + .Where(x => x.IsNullOrWhiteSpace() == false) + .Select(reservedUrl => IOHelper.ResolveUrl(reservedUrl).Trim().EnsureStartsWith("/")) + .Where(reservedUrlTrimmed => reservedUrlTrimmed.IsNullOrWhiteSpace() == false)) { - routes.Add("_umbreserved_" + reservedPath.ReplaceNonAlphanumericChars(""), - new Route(reservedPath.TrimStart('/'), new StopRoutingHandler())); + newReservedList.Add(reservedUrlTrimmed); } - catch (Exception ex) + + foreach (var reservedPathTrimmed in NormalizePaths(reservedPathsCache.Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries))) { - _logger.Error("Could not add reserved path route", ex); + newReservedList.Add(reservedPathTrimmed); + } + + foreach (var reservedPathTrimmed in NormalizePaths(ReservedPaths)) + { + newReservedList.Add(reservedPathTrimmed); + } + + // use the new list from now on + return newReservedList; + }); + + //The url should be cleaned up before checking: + // * If it doesn't contain an '.' in the path then we assume it is a path based URL, if that is the case we should add an trailing '/' because all of our reservedPaths use a trailing '/' + // * We shouldn't be comparing the query at all + var pathPart = url.Split(new[] { '?' }, StringSplitOptions.RemoveEmptyEntries)[0].ToLowerInvariant(); + if (pathPart.Contains(".") == false) + { + pathPart = pathPart.EnsureEndsWith('/'); + } + + // return true if url starts with an element of the reserved list + return _reservedList.Any(x => pathPart.InvariantStartsWith(x)); + } + + private IEnumerable NormalizePaths(IEnumerable paths) + { + return paths + .Select(x => x.Trim().ToLowerInvariant()) + .Where(x => x.IsNullOrWhiteSpace() == false) + .Select(reservedPath => IOHelper.ResolveUrl(reservedPath).Trim().EnsureStartsWith("/").EnsureEndsWith("/")) + .Where(reservedPathTrimmed => reservedPathTrimmed.IsNullOrWhiteSpace() == false); + } + + /// + /// Determines whether the current request is reserved based on the route table and + /// whether the specified URL is reserved or is inside a reserved path. + /// + /// + /// + /// + /// The route collection to lookup the request in + /// + internal bool IsReservedPathOrUrl(string url, HttpContextBase httpContext, RouteCollection routes) + { + if (httpContext == null) throw new ArgumentNullException(nameof(httpContext)); + if (routes == null) throw new ArgumentNullException(nameof(routes)); + + //This is some rudimentary code to check if the route table has changed at runtime, we're basically just keeping a count + //of the routes. This isn't fail safe but there's no way to monitor changes to the route table. Else we need to create a hash + //of all routes and then recompare but that will be annoying to do on each request and then we might as well just do the whole MVC + //route on each request like we were doing before instead of caching the result of GetRouteData. + var changed = false; + using (routes.GetReadLock()) + { + if (!_routeCount.HasValue || _routeCount.Value != routes.Count) + { + //the counts are not set or have changed, need to reset + changed = true; + } + } + if (changed) + { + using (routes.GetWriteLock()) + { + _routeCount = routes.Count; + + //try clearing each entry + foreach(var r in _routeChecks.Keys.ToList()) + _routeChecks.TryRemove(r, out _); } } - return routes; + //check if the current request matches a route, if so then it is reserved. + var hasRoute = _routeChecks.GetOrAdd(httpContext.Request.Url.AbsolutePath, x => routes.GetRouteData(httpContext) != null); + if (hasRoute) + return true; + + //continue with the standard ignore routine + return IsReservedPathOrUrl(url); } + + /// + /// This is used internally to track any registered callback paths for Identity providers. If the request path matches + /// any of the registered paths, then the module will let the request keep executing + /// + internal static readonly ConcurrentHashSet ReservedPaths = new ConcurrentHashSet(); } } diff --git a/src/Umbraco.Web/Security/AuthenticationOptionsExtensions.cs b/src/Umbraco.Web/Security/AuthenticationOptionsExtensions.cs index 33aabbaf94..b1b2755f4c 100644 --- a/src/Umbraco.Web/Security/AuthenticationOptionsExtensions.cs +++ b/src/Umbraco.Web/Security/AuthenticationOptionsExtensions.cs @@ -101,7 +101,7 @@ namespace Umbraco.Web.Security var path = (PathString) prop.GetValue(options); if (path.HasValue) { - UmbracoModule.ReservedPaths.TryAdd(path.ToString()); + RoutableDocumentFilter.ReservedPaths.TryAdd(path.ToString()); } } } @@ -112,7 +112,7 @@ namespace Umbraco.Web.Security } else { - UmbracoModule.ReservedPaths.TryAdd(callbackPath); + RoutableDocumentFilter.ReservedPaths.TryAdd(callbackPath); } } } diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index 048e521969..9065341ab9 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -103,10 +103,5 @@ namespace Umbraco.Web return end; } - /// - /// This is used internally to track any registered callback paths for Identity providers. If the request path matches - /// any of the registered paths, then the module will let the request keep executing - /// - internal static readonly ConcurrentHashSet ReservedPaths = new ConcurrentHashSet(); } } From 81d9ecc111babd04030d7a31ec805cd99239c5ca Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 16:58:19 +1000 Subject: [PATCH 08/40] fixing tests --- .../Configurations/GlobalSettingsTests.cs | 66 +-------------- .../Routing/RoutableDocumentFilterTests.cs | 80 +++++++++++++++++++ .../Routing/UmbracoModuleTests.cs | 8 +- src/Umbraco.Tests/Umbraco.Tests.csproj | 3 +- 4 files changed, 88 insertions(+), 69 deletions(-) create mode 100644 src/Umbraco.Tests/Routing/RoutableDocumentFilterTests.cs diff --git a/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs b/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs index 54080f05de..a9dc94d239 100644 --- a/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs +++ b/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs @@ -1,6 +1,4 @@ -using System.Web.Mvc; -using System.Web.Routing; -using Moq; +using Moq; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Composing; @@ -10,6 +8,7 @@ using Umbraco.Tests.TestHelpers; namespace Umbraco.Tests.Configurations { + [TestFixture] public class GlobalSettingsTests : BaseWebTest { @@ -54,66 +53,9 @@ namespace Umbraco.Tests.Configurations Assert.AreEqual(outcome, Current.Configs.Global().GetUmbracoMvcArea()); } - [TestCase("/umbraco/editContent.aspx")] - [TestCase("/install/default.aspx")] - [TestCase("/install/")] - [TestCase("/install")] - [TestCase("/install/?installStep=asdf")] - [TestCase("/install/test.aspx")] - public void Is_Reserved_Path_Or_Url(string url) - { - var globalSettings = TestObjects.GetGlobalSettings(); - Assert.IsTrue(globalSettings.IsReservedPathOrUrl(url)); - } - - [TestCase("/base/somebasehandler")] - [TestCase("/")] - [TestCase("/home.aspx")] - [TestCase("/umbraco-test")] - [TestCase("/install-test")] - [TestCase("/install.aspx")] - public void Is_Not_Reserved_Path_Or_Url(string url) - { - var globalSettings = TestObjects.GetGlobalSettings(); - Assert.IsFalse(globalSettings.IsReservedPathOrUrl(url)); - } + - [TestCase("/Do/Not/match", false)] - [TestCase("/Umbraco/RenderMvcs", false)] - [TestCase("/Umbraco/RenderMvc", true)] - [TestCase("/Umbraco/RenderMvc/Index", true)] - [TestCase("/Umbraco/RenderMvc/Index/1234", true)] - [TestCase("/Umbraco/RenderMvc/Index/1234/9876", false)] - [TestCase("/api", true)] - [TestCase("/api/WebApiTest", true)] - [TestCase("/api/WebApiTest/1234", true)] - [TestCase("/api/WebApiTest/Index/1234", false)] - public void Is_Reserved_By_Route(string url, bool shouldMatch) - { - //reset the app config, we only want to test routes not the hard coded paths - var globalSettingsMock = Mock.Get(Factory.GetInstance()); //this will modify the IGlobalSettings instance stored in the container - globalSettingsMock.Setup(x => x.ReservedPaths).Returns(""); - globalSettingsMock.Setup(x => x.ReservedUrls).Returns(""); - - var routes = new RouteCollection(); - - routes.MapRoute( - "Umbraco_default", - "Umbraco/RenderMvc/{action}/{id}", - new { controller = "RenderMvc", action = "Index", id = UrlParameter.Optional }); - routes.MapRoute( - "WebAPI", - "api/{controller}/{id}", - new { controller = "WebApiTestController", action = "Index", id = UrlParameter.Optional }); - - - var context = new FakeHttpContextFactory(url); - - - Assert.AreEqual( - shouldMatch, - globalSettingsMock.Object.IsReservedPathOrUrl(url, context.HttpContext, routes)); - } + } } diff --git a/src/Umbraco.Tests/Routing/RoutableDocumentFilterTests.cs b/src/Umbraco.Tests/Routing/RoutableDocumentFilterTests.cs new file mode 100644 index 0000000000..4079ea6c84 --- /dev/null +++ b/src/Umbraco.Tests/Routing/RoutableDocumentFilterTests.cs @@ -0,0 +1,80 @@ +using System.Web.Mvc; +using System.Web.Routing; +using Moq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Configuration; +using Umbraco.Tests.TestHelpers; +using Umbraco.Web; + +namespace Umbraco.Tests.Routing +{ + [TestFixture] + public class RoutableDocumentFilterTests : BaseWebTest + { + [TestCase("/umbraco/editContent.aspx")] + [TestCase("/install/default.aspx")] + [TestCase("/install/")] + [TestCase("/install")] + [TestCase("/install/?installStep=asdf")] + [TestCase("/install/test.aspx")] + public void Is_Reserved_Path_Or_Url(string url) + { + var globalSettings = TestObjects.GetGlobalSettings(); + var routableDocFilter = new RoutableDocumentFilter(globalSettings); + Assert.IsTrue(routableDocFilter.IsReservedPathOrUrl(url)); + } + + [TestCase("/base/somebasehandler")] + [TestCase("/")] + [TestCase("/home.aspx")] + [TestCase("/umbraco-test")] + [TestCase("/install-test")] + [TestCase("/install.aspx")] + public void Is_Not_Reserved_Path_Or_Url(string url) + { + var globalSettings = TestObjects.GetGlobalSettings(); + var routableDocFilter = new RoutableDocumentFilter(globalSettings); + Assert.IsFalse(routableDocFilter.IsReservedPathOrUrl(url)); + } + + [TestCase("/Do/Not/match", false)] + [TestCase("/Umbraco/RenderMvcs", false)] + [TestCase("/Umbraco/RenderMvc", true)] + [TestCase("/Umbraco/RenderMvc/Index", true)] + [TestCase("/Umbraco/RenderMvc/Index/1234", true)] + [TestCase("/Umbraco/RenderMvc/Index/1234/9876", false)] + [TestCase("/api", true)] + [TestCase("/api/WebApiTest", true)] + [TestCase("/api/WebApiTest/1234", true)] + [TestCase("/api/WebApiTest/Index/1234", false)] + public void Is_Reserved_By_Route(string url, bool shouldMatch) + { + //reset the app config, we only want to test routes not the hard coded paths + var globalSettingsMock = Mock.Get(Factory.GetInstance()); //this will modify the IGlobalSettings instance stored in the container + globalSettingsMock.Setup(x => x.ReservedPaths).Returns(""); + globalSettingsMock.Setup(x => x.ReservedUrls).Returns(""); + + var routableDocFilter = new RoutableDocumentFilter(globalSettingsMock.Object); + + var routes = new RouteCollection(); + + routes.MapRoute( + "Umbraco_default", + "Umbraco/RenderMvc/{action}/{id}", + new { controller = "RenderMvc", action = "Index", id = UrlParameter.Optional }); + routes.MapRoute( + "WebAPI", + "api/{controller}/{id}", + new { controller = "WebApiTestController", action = "Index", id = UrlParameter.Optional }); + + + var context = new FakeHttpContextFactory(url); + + + Assert.AreEqual( + shouldMatch, + routableDocFilter.IsReservedPathOrUrl(url, context.HttpContext, routes)); + } + } +} diff --git a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs index 34ac79a219..325f6860fd 100644 --- a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs +++ b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs @@ -38,19 +38,15 @@ namespace Umbraco.Tests.Routing _module = new UmbracoInjectedModule ( globalSettings, - Mock.Of(), - Factory.GetInstance(), - Factory.GetInstance(), - new UrlProviderCollection(new IUrlProvider[0]), runtime, logger, null, // FIXME: PublishedRouter complexities... - Mock.Of(), Mock.Of(), new Umbraco.Web.Cache.BackgroundPublishedSnapshotNotifier( Factory.GetInstance(), Factory.GetInstance(), - Logger) + Logger), + new RoutableDocumentFilter(globalSettings) ); runtime.Level = RuntimeLevel.Run; diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index f788168ddc..fcf73fbffa 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -120,6 +120,7 @@ + @@ -145,6 +146,7 @@ + @@ -455,7 +457,6 @@ - From de03130256c8dc5180dd15bcb670efc24047e49d Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 17:10:46 +1000 Subject: [PATCH 09/40] Don't fully regenerate models on startup, they will be regenerated if they are not there or if the hashes don't match so let MB use it's cached models if it can. --- .../PublishedCache/NuCache/PublishedSnapshotService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 15e6574b40..4239eda9f5 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -160,7 +160,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _domainStore = new SnapDictionary(); - publishedModelFactory.WithSafeLiveFactory(LoadCachesOnStartup); + LoadCachesOnStartup(); Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? default; int GetId(ContentStore store, Guid uid) => store.LiveSnapshot.Get(uid)?.Id ?? default; From c219c21eddec5e87397f0efb34c8933743d70aac Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 17:23:49 +1000 Subject: [PATCH 10/40] ensures tests and benchmark projects are building in Debug mode --- src/umbraco.sln | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/umbraco.sln b/src/umbraco.sln index 0522b56c0d..938532beb0 100644 --- a/src/umbraco.sln +++ b/src/umbraco.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio 15 -VisualStudioVersion = 15.0.27004.2005 +# Visual Studio Version 16 +VisualStudioVersion = 16.0.29209.152 MinimumVisualStudioVersion = 10.0.40219.1 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Umbraco.Web.UI", "Umbraco.Web.UI\Umbraco.Web.UI.csproj", "{4C4C194C-B5E4-4991-8F87-4373E24CC19F}" EndProject @@ -123,6 +123,7 @@ Global {31785BC3-256C-4613-B2F5-A1B0BDDED8C1}.Release|Any CPU.ActiveCfg = Release|Any CPU {31785BC3-256C-4613-B2F5-A1B0BDDED8C1}.Release|Any CPU.Build.0 = Release|Any CPU {5D3B8245-ADA6-453F-A008-50ED04BFE770}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {5D3B8245-ADA6-453F-A008-50ED04BFE770}.Debug|Any CPU.Build.0 = Debug|Any CPU {5D3B8245-ADA6-453F-A008-50ED04BFE770}.Release|Any CPU.ActiveCfg = Release|Any CPU {5D3B8245-ADA6-453F-A008-50ED04BFE770}.Release|Any CPU.Build.0 = Release|Any CPU {07FBC26B-2927-4A22-8D96-D644C667FECC}.Debug|Any CPU.ActiveCfg = Debug|Any CPU @@ -130,6 +131,7 @@ Global {07FBC26B-2927-4A22-8D96-D644C667FECC}.Release|Any CPU.ActiveCfg = Release|Any CPU {07FBC26B-2927-4A22-8D96-D644C667FECC}.Release|Any CPU.Build.0 = Release|Any CPU {3A33ADC9-C6C0-4DB1-A613-A9AF0210DF3D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {3A33ADC9-C6C0-4DB1-A613-A9AF0210DF3D}.Debug|Any CPU.Build.0 = Debug|Any CPU {3A33ADC9-C6C0-4DB1-A613-A9AF0210DF3D}.Release|Any CPU.ActiveCfg = Release|Any CPU {3A33ADC9-C6C0-4DB1-A613-A9AF0210DF3D}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection From 642c06879d561cf6f08b7683590707d64df17874 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 17:24:10 +1000 Subject: [PATCH 11/40] ensure we don't wait on background operations when not in PureLive --- src/Umbraco.Web/UmbracoInjectedModule.cs | 41 +++++++++++++++--------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 7f10701e36..f2e9e43a50 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -13,6 +13,7 @@ using Umbraco.Core.Exceptions; using Umbraco.Core.Security; using Umbraco.Web.Composing; using Umbraco.Web.Cache; +using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web { @@ -38,6 +39,7 @@ namespace Umbraco.Web private readonly ILogger _logger; private readonly IPublishedRouter _publishedRouter; private readonly IUmbracoContextFactory _umbracoContextFactory; + private readonly IPublishedModelFactory _publishedModelFactory; private readonly BackgroundPublishedSnapshotNotifier _backgroundNotifier; private readonly RoutableDocumentFilter _routableDocumentLookup; @@ -47,6 +49,7 @@ namespace Umbraco.Web ILogger logger, IPublishedRouter publishedRouter, IUmbracoContextFactory umbracoContextFactory, + IPublishedModelFactory publishedModelFactory, BackgroundPublishedSnapshotNotifier backgroundNotifier, RoutableDocumentFilter routableDocumentLookup) { @@ -55,6 +58,7 @@ namespace Umbraco.Web _logger = logger; _publishedRouter = publishedRouter; _umbracoContextFactory = umbracoContextFactory; + _publishedModelFactory = publishedModelFactory; _backgroundNotifier = backgroundNotifier; _routableDocumentLookup = routableDocumentLookup; } @@ -83,28 +87,35 @@ namespace Umbraco.Web // ensure there's an UmbracoContext registered for the current request // registers the context reference so its disposed at end of request var umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext(httpContext); - umbracoContextReference.UmbracoContext.CreatingPublishedSnapshot += UmbracoContext_CreatingPublishedSnapshot; + BindCreatingPublishedSnapshot(umbracoContextReference.UmbracoContext); httpContext.DisposeOnPipelineCompleted(umbracoContextReference); } /// - /// Event handler for when the UmbracoContext creates the published snapshot + /// Bind to the event handler for when the UmbracoContext creates the published snapshot when in PureLive mode /// - /// - /// - private void UmbracoContext_CreatingPublishedSnapshot(UmbracoContext sender, EventArgs e) + /// + /// + /// When we are not in PureLive mode it is not necessary to do this so we have to have a special check. + /// + private void BindCreatingPublishedSnapshot(UmbracoContext umbracoContext) { - // Wait for the notifier to complete if it's in progress, this is required because - // Pure Live models along with content snapshots are updated on a background thread when schema - // (doc types, data types) are changed so if that is still processing we need to wait before we access - // the content snapshot to make sure it's the latest version. - // We only want to wait if this is a front-end request (not a back office request) so need to first check - // for that and then wait. - - if (sender.IsDocumentRequest(_routableDocumentLookup) &&_backgroundNotifier.Wait()) + if (!_publishedModelFactory.IsLiveFactory()) return; + + umbracoContext.CreatingPublishedSnapshot += (UmbracoContext sender, EventArgs e) => { - _logger.Debug("Request was suspended while waiting for background cache notifications to complete"); - } + // Wait for the notifier to complete if it's in progress, this is required because + // Pure Live models along with content snapshots are updated on a background thread when schema + // (doc types, data types) are changed so if that is still processing we need to wait before we access + // the content snapshot to make sure it's the latest version. + // We only want to wait if this is a front-end request (not a back office request) so need to first check + // for that and then wait. + + if (sender.IsDocumentRequest(_routableDocumentLookup) && _backgroundNotifier.Wait()) + { + _logger.Debug("Request was suspended while waiting for background cache notifications to complete"); + } + }; } /// From 127c429959f62f11139a7074cb6b78c2c5a7b149 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 10 Sep 2019 17:27:17 +1000 Subject: [PATCH 12/40] fix tests --- src/Umbraco.Tests/Routing/UmbracoModuleTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs index 325f6860fd..060ce549dd 100644 --- a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs +++ b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs @@ -42,6 +42,7 @@ namespace Umbraco.Tests.Routing logger, null, // FIXME: PublishedRouter complexities... Mock.Of(), + Mock.Of(), new Umbraco.Web.Cache.BackgroundPublishedSnapshotNotifier( Factory.GetInstance(), Factory.GetInstance(), From 19716e3c96297d7a3db440b6c2c9ed26c95f62cd Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 11 Sep 2019 21:47:43 +1000 Subject: [PATCH 13/40] fixing tests --- src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs index 146cb23c1f..7ee75b1d9c 100644 --- a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs +++ b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs @@ -31,6 +31,7 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Persistence.Repositories; using Umbraco.Tests.LegacyXmlPublishedCache; using Umbraco.Tests.Testing.Objects.Accessors; +using Umbraco.Web.Cache; namespace Umbraco.Tests.TestHelpers { @@ -90,6 +91,8 @@ namespace Umbraco.Tests.TestHelpers factory.ResetForTests(); return factory; }); + + Composition.RegisterUnique(); } [OneTimeTearDown] From 942c6c75c5bb000030739669422d039882c1276d Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 11 Sep 2019 23:01:11 +1000 Subject: [PATCH 14/40] fixes tests --- .../Configuration/GlobalSettingsExtensions.cs | 13 ++++++++++--- .../Configurations/GlobalSettingsTests.cs | 8 +++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs b/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs index fd4b57bf1a..bc76caacee 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettingsExtensions.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Configuration public static class GlobalSettingsExtensions { private static string _mvcArea; - + /// /// This returns the string of the MVC Area route. @@ -29,6 +29,13 @@ namespace Umbraco.Core.Configuration { if (_mvcArea != null) return _mvcArea; + _mvcArea = GetUmbracoMvcAreaNoCache(globalSettings); + + return _mvcArea; + } + + internal static string GetUmbracoMvcAreaNoCache(this IGlobalSettings globalSettings) + { if (globalSettings.Path.IsNullOrWhiteSpace()) { throw new InvalidOperationException("Cannot create an MVC Area path without the umbracoPath specified"); @@ -37,8 +44,8 @@ namespace Umbraco.Core.Configuration var path = globalSettings.Path; if (path.StartsWith(SystemDirectories.Root)) // beware of TrimStart, see U4-2518 path = path.Substring(SystemDirectories.Root.Length); - _mvcArea = path.TrimStart('~').TrimStart('/').Replace('/', '-').Trim().ToLower(); - return _mvcArea; + return path.TrimStart('~').TrimStart('/').Replace('/', '-').Trim().ToLower(); } + } } diff --git a/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs b/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs index a9dc94d239..50ead4b702 100644 --- a/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs +++ b/src/Umbraco.Tests/Configurations/GlobalSettingsTests.cs @@ -46,11 +46,13 @@ namespace Umbraco.Tests.Configurations [TestCase("~/some-wacky/nestedPath", "/MyVirtualDir/NestedVDir/", "some-wacky-nestedpath")] public void Umbraco_Mvc_Area(string path, string rootPath, string outcome) { - var globalSettingsMock = Mock.Get(Factory.GetInstance()); //this will modify the IGlobalSettings instance stored in the container - globalSettingsMock.Setup(x => x.Path).Returns(IOHelper.ResolveUrl(path)); + var globalSettings = SettingsForTests.GenerateMockGlobalSettings(); + + var globalSettingsMock = Mock.Get(globalSettings); + globalSettingsMock.Setup(x => x.Path).Returns(() => IOHelper.ResolveUrl(path)); SystemDirectories.Root = rootPath; - Assert.AreEqual(outcome, Current.Configs.Global().GetUmbracoMvcArea()); + Assert.AreEqual(outcome, globalSettings.GetUmbracoMvcAreaNoCache()); } From 81b277314cacbbcf43e1c0f98695bc050d3caca9 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 12 Sep 2019 16:21:17 +1000 Subject: [PATCH 15/40] better logging --- src/Umbraco.Web/UmbracoInjectedModule.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index f2e9e43a50..e306628654 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -113,7 +113,7 @@ namespace Umbraco.Web if (sender.IsDocumentRequest(_routableDocumentLookup) && _backgroundNotifier.Wait()) { - _logger.Debug("Request was suspended while waiting for background cache notifications to complete"); + _logger.Debug("Request was suspended while waiting for background cache notifications to complete {RequestUrl}", umbracoContext.CleanedUmbracoUrl); } }; } From 2c27a631c9a8a08bf0e4cd185447a666cb9009aa Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Thu, 12 Sep 2019 23:14:10 +1000 Subject: [PATCH 16/40] Update src/Umbraco.Web/RoutableDocumentFilter.cs Co-Authored-By: Bjarke Berg --- src/Umbraco.Web/RoutableDocumentFilter.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Web/RoutableDocumentFilter.cs b/src/Umbraco.Web/RoutableDocumentFilter.cs index be2c8ad405..465480ce22 100644 --- a/src/Umbraco.Web/RoutableDocumentFilter.cs +++ b/src/Umbraco.Web/RoutableDocumentFilter.cs @@ -86,7 +86,6 @@ namespace Umbraco.Web /// /// Determines whether the specified URL is reserved or is inside a reserved path. /// - /// /// The URL to check. /// /// true if the specified URL is reserved; otherwise, false. From d7b950fe066b04434eacde556486af805af9ca12 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Thu, 12 Sep 2019 23:14:18 +1000 Subject: [PATCH 17/40] Update src/Umbraco.Web/RoutableDocumentFilter.cs Co-Authored-By: Bjarke Berg --- src/Umbraco.Web/RoutableDocumentFilter.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Web/RoutableDocumentFilter.cs b/src/Umbraco.Web/RoutableDocumentFilter.cs index 465480ce22..642c60f6d4 100644 --- a/src/Umbraco.Web/RoutableDocumentFilter.cs +++ b/src/Umbraco.Web/RoutableDocumentFilter.cs @@ -150,7 +150,6 @@ namespace Umbraco.Web /// Determines whether the current request is reserved based on the route table and /// whether the specified URL is reserved or is inside a reserved path. /// - /// /// /// /// The route collection to lookup the request in From c7c2f64b62a3391159bd417d4876a0cd27879301 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 12 Sep 2019 23:15:03 +1000 Subject: [PATCH 18/40] removes comments --- src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs index 5efbf988a4..e2b3c07cea 100644 --- a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs +++ b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs @@ -33,10 +33,6 @@ namespace Umbraco.Web.Cache { _publishedModelFactory = publishedModelFactory; _publishedSnapshotService = publishedSnapshotService; - - // TODO: We have the option to check if we are in live mode and only run on a background thread if that is the case, else run normally? - // IMO I think we should just always run on a background thread, then no matter what state the app is in, it's always doing a consistent operation. - _runner = new BackgroundTaskRunner("RebuildModelsAndCache", logger); } From aa9ece94b7d63d9565aafad0207d87cccb7084e5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 12 Sep 2019 23:37:17 +1000 Subject: [PATCH 19/40] naming convention and null check --- src/Umbraco.Web/RoutableDocumentFilter.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web/RoutableDocumentFilter.cs b/src/Umbraco.Web/RoutableDocumentFilter.cs index 642c60f6d4..9725ce0e14 100644 --- a/src/Umbraco.Web/RoutableDocumentFilter.cs +++ b/src/Umbraco.Web/RoutableDocumentFilter.cs @@ -27,7 +27,7 @@ namespace Umbraco.Web _globalSettings = globalSettings; } - private static readonly ConcurrentDictionary _routeChecks = new ConcurrentDictionary(); + private static readonly ConcurrentDictionary RouteChecks = new ConcurrentDictionary(); private readonly IGlobalSettings _globalSettings; private object _locker = new object(); private bool _isInit = false; @@ -179,13 +179,18 @@ namespace Umbraco.Web _routeCount = routes.Count; //try clearing each entry - foreach(var r in _routeChecks.Keys.ToList()) - _routeChecks.TryRemove(r, out _); + foreach(var r in RouteChecks.Keys.ToList()) + RouteChecks.TryRemove(r, out _); } } + var absPath = httpContext?.Request?.Url.AbsolutePath; + + if (absPath.IsNullOrWhiteSpace()) + return false; + //check if the current request matches a route, if so then it is reserved. - var hasRoute = _routeChecks.GetOrAdd(httpContext.Request.Url.AbsolutePath, x => routes.GetRouteData(httpContext) != null); + var hasRoute = RouteChecks.GetOrAdd(absPath, x => routes.GetRouteData(httpContext) != null); if (hasRoute) return true; From e5634d13f1070d5eb4817be47592127c3386dbe7 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 13 Sep 2019 09:55:53 +0200 Subject: [PATCH 20/40] Lazily create content and models in NuCache --- .../PublishedCache/NuCache/ContentNode.cs | 66 +++++++++++-------- .../NuCache/PublishedContent.cs | 5 +- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs index 7379277be4..fad402a0eb 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs @@ -75,17 +75,11 @@ namespace Umbraco.Web.PublishedCache.NuCache if (draftData == null && publishedData == null) throw new ArgumentException("Both draftData and publishedData cannot be null at the same time."); - if (draftData != null) - { - DraftContent = new PublishedContent(this, draftData, publishedSnapshotAccessor, variationContextAccessor); - DraftModel = DraftContent.CreateModel(); - } + _publishedSnapshotAccessor = publishedSnapshotAccessor; + _variationContextAccessor = variationContextAccessor; - if (publishedData != null) - { - PublishedContent = new PublishedContent(this, publishedData, publishedSnapshotAccessor, variationContextAccessor); - PublishedModel = PublishedContent.CreateModel(); - } + _draftData = draftData; + _publishedData = publishedData; } // clone @@ -105,14 +99,8 @@ namespace Umbraco.Web.PublishedCache.NuCache CreateDate = origin.CreateDate; CreatorId = origin.CreatorId; - var originDraft = origin.DraftContent; - var originPublished = origin.PublishedContent; - - DraftContent = originDraft == null ? null : new PublishedContent(this, originDraft); - PublishedContent = originPublished == null ? null : new PublishedContent(this, originPublished); - - DraftModel = DraftContent?.CreateModel(); - PublishedModel = PublishedContent?.CreateModel(); + _draftData = origin._draftData; + _publishedData = origin._publishedData; } // everything that is common to both draft and published versions @@ -131,15 +119,41 @@ namespace Umbraco.Web.PublishedCache.NuCache public readonly DateTime CreateDate; public readonly int CreatorId; - // draft and published version (either can be null, but not both) - // are the direct PublishedContent instances - public PublishedContent DraftContent; - public PublishedContent PublishedContent; + private ContentData _draftData; + private ContentData _publishedData; + private IVariationContextAccessor _variationContextAccessor; + private IPublishedSnapshotAccessor _publishedSnapshotAccessor; + + public bool HasPublished => _publishedData != null; + public bool HasPublishedCulture(string culture) => _publishedData != null && _publishedData.CultureInfos.ContainsKey(culture); // draft and published version (either can be null, but not both) // are models not direct PublishedContent instances - public IPublishedContent DraftModel; - public IPublishedContent PublishedModel; + private IPublishedContent _draftModel; + private IPublishedContent _publishedModel; + + private IPublishedContent GetModel(ref IPublishedContent model, ContentData contentData) + { + if (model != null) return model; + if (contentData == null) return null; + + // create the model - we want to be fast, so no lock here: we may create + // more than 1 instance, but the lock below ensures we only ever return + // 1 unique instance - and locking is a nice explicit way to ensure this + + var m = new PublishedContent(this, contentData, _publishedSnapshotAccessor, _variationContextAccessor).CreateModel(); + + // locking 'this' is not a best-practice but ContentNode is internal and + // we know what we do, so it is fine here and avoids allocating an object + lock (this) + { + return model = model ?? m; + } + } + + public IPublishedContent DraftModel => GetModel(ref _draftModel, _draftData); + + public IPublishedContent PublishedModel => GetModel(ref _publishedModel, _publishedData); public ContentNodeKit ToKit() => new ContentNodeKit @@ -147,8 +161,8 @@ namespace Umbraco.Web.PublishedCache.NuCache Node = this, ContentTypeId = ContentType.Id, - DraftData = DraftContent?.ContentData, - PublishedData = PublishedContent?.ContentData + DraftData = _draftData, + PublishedData = _publishedData }; } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs index 8fc2dc1006..bf4975714d 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs @@ -233,8 +233,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // invariant content items) // if there is no 'published' published content, no culture can be published - var hasPublished = _contentNode.PublishedContent != null; - if (!hasPublished) + if (!_contentNode.HasPublished) return false; // if there is a 'published' published content, and does not vary = published @@ -247,7 +246,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // there is a 'published' published content, and varies // = depends on the culture - return _contentNode.PublishedContent.ContentData.CultureInfos.ContainsKey(culture); + return _contentNode.HasPublishedCulture(culture); } #endregion From ba74b7543c82da2c81727c940b61323ad17aadd8 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 11:47:05 +1000 Subject: [PATCH 21/40] wraps package uninstall in a scope, creates temp SuspendSafeLiveFactory method for testing --- .../Packaging/PackageDataInstallation.cs | 158 ++++++++++-------- .../PublishedModelFactoryExtensions.cs | 96 ++++++++++- .../Editors/ContentTypeController.cs | 11 +- 3 files changed, 187 insertions(+), 78 deletions(-) diff --git a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs index 2f6b91edee..79e8d2fc00 100644 --- a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs @@ -11,7 +11,9 @@ using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Packaging; +using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; @@ -25,13 +27,16 @@ namespace Umbraco.Core.Packaging private readonly ILocalizationService _localizationService; private readonly IDataTypeService _dataTypeService; private readonly PropertyEditorCollection _propertyEditors; + private readonly IScopeProvider _scopeProvider; + private readonly IPublishedModelFactory _publishedModelFactory; private readonly IEntityService _entityService; private readonly IContentTypeService _contentTypeService; private readonly IContentService _contentService; public PackageDataInstallation(ILogger logger, IFileService fileService, IMacroService macroService, ILocalizationService localizationService, IDataTypeService dataTypeService, IEntityService entityService, IContentTypeService contentTypeService, - IContentService contentService, PropertyEditorCollection propertyEditors) + IContentService contentService, PropertyEditorCollection propertyEditors, IScopeProvider scopeProvider, + IPublishedModelFactory publishedModelFactory) { _logger = logger; _fileService = fileService; @@ -39,6 +44,8 @@ namespace Umbraco.Core.Packaging _localizationService = localizationService; _dataTypeService = dataTypeService; _propertyEditors = propertyEditors; + _scopeProvider = scopeProvider; + _publishedModelFactory = publishedModelFactory; _entityService = entityService; _contentTypeService = contentTypeService; _contentService = contentService; @@ -57,93 +64,98 @@ namespace Umbraco.Core.Packaging var removedDataTypes = new List(); var removedLanguages = new List(); - - //Uninstall templates - foreach (var item in package.Templates.ToArray()) + using(_publishedModelFactory.SuspendSafeLiveFactory()) //ensure that any PureLive models are not regenerated until after the bulk operation + using (var scope = _scopeProvider.CreateScope()) { - if (int.TryParse(item, out var nId) == false) continue; - var found = _fileService.GetTemplate(nId); - if (found != null) + //Uninstall templates + foreach (var item in package.Templates.ToArray()) { - removedTemplates.Add(found); - _fileService.DeleteTemplate(found.Alias, userId); + if (int.TryParse(item, out var nId) == false) continue; + var found = _fileService.GetTemplate(nId); + if (found != null) + { + removedTemplates.Add(found); + _fileService.DeleteTemplate(found.Alias, userId); + } + package.Templates.Remove(nId.ToString()); } - package.Templates.Remove(nId.ToString()); - } - //Uninstall macros - foreach (var item in package.Macros.ToArray()) - { - if (int.TryParse(item, out var nId) == false) continue; - var macro = _macroService.GetById(nId); - if (macro != null) + //Uninstall macros + foreach (var item in package.Macros.ToArray()) { - removedMacros.Add(macro); - _macroService.Delete(macro, userId); + if (int.TryParse(item, out var nId) == false) continue; + var macro = _macroService.GetById(nId); + if (macro != null) + { + removedMacros.Add(macro); + _macroService.Delete(macro, userId); + } + package.Macros.Remove(nId.ToString()); } - package.Macros.Remove(nId.ToString()); - } - //Remove Document Types - var contentTypes = new List(); - var contentTypeService = _contentTypeService; - foreach (var item in package.DocumentTypes.ToArray()) - { - if (int.TryParse(item, out var nId) == false) continue; - var contentType = contentTypeService.Get(nId); - if (contentType == null) continue; - contentTypes.Add(contentType); - package.DocumentTypes.Remove(nId.ToString(CultureInfo.InvariantCulture)); - } - - //Order the DocumentTypes before removing them - if (contentTypes.Any()) - { - // TODO: I don't think this ordering is necessary - var orderedTypes = (from contentType in contentTypes - orderby contentType.ParentId descending, contentType.Id descending - select contentType).ToList(); - removedContentTypes.AddRange(orderedTypes); - contentTypeService.Delete(orderedTypes, userId); - } - - //Remove Dictionary items - foreach (var item in package.DictionaryItems.ToArray()) - { - if (int.TryParse(item, out var nId) == false) continue; - var di = _localizationService.GetDictionaryItemById(nId); - if (di != null) + //Remove Document Types + var contentTypes = new List(); + var contentTypeService = _contentTypeService; + foreach (var item in package.DocumentTypes.ToArray()) { - removedDictionaryItems.Add(di); - _localizationService.Delete(di, userId); + if (int.TryParse(item, out var nId) == false) continue; + var contentType = contentTypeService.Get(nId); + if (contentType == null) continue; + contentTypes.Add(contentType); + package.DocumentTypes.Remove(nId.ToString(CultureInfo.InvariantCulture)); } - package.DictionaryItems.Remove(nId.ToString()); - } - //Remove Data types - foreach (var item in package.DataTypes.ToArray()) - { - if (int.TryParse(item, out var nId) == false) continue; - var dtd = _dataTypeService.GetDataType(nId); - if (dtd != null) + //Order the DocumentTypes before removing them + if (contentTypes.Any()) { - removedDataTypes.Add(dtd); - _dataTypeService.Delete(dtd, userId); + // TODO: I don't think this ordering is necessary + var orderedTypes = (from contentType in contentTypes + orderby contentType.ParentId descending, contentType.Id descending + select contentType).ToList(); + removedContentTypes.AddRange(orderedTypes); + contentTypeService.Delete(orderedTypes, userId); } - package.DataTypes.Remove(nId.ToString()); - } - //Remove Langs - foreach (var item in package.Languages.ToArray()) - { - if (int.TryParse(item, out var nId) == false) continue; - var lang = _localizationService.GetLanguageById(nId); - if (lang != null) + //Remove Dictionary items + foreach (var item in package.DictionaryItems.ToArray()) { - removedLanguages.Add(lang); - _localizationService.Delete(lang, userId); + if (int.TryParse(item, out var nId) == false) continue; + var di = _localizationService.GetDictionaryItemById(nId); + if (di != null) + { + removedDictionaryItems.Add(di); + _localizationService.Delete(di, userId); + } + package.DictionaryItems.Remove(nId.ToString()); } - package.Languages.Remove(nId.ToString()); + + //Remove Data types + foreach (var item in package.DataTypes.ToArray()) + { + if (int.TryParse(item, out var nId) == false) continue; + var dtd = _dataTypeService.GetDataType(nId); + if (dtd != null) + { + removedDataTypes.Add(dtd); + _dataTypeService.Delete(dtd, userId); + } + package.DataTypes.Remove(nId.ToString()); + } + + //Remove Langs + foreach (var item in package.Languages.ToArray()) + { + if (int.TryParse(item, out var nId) == false) continue; + var lang = _localizationService.GetLanguageById(nId); + if (lang != null) + { + removedLanguages.Add(lang); + _localizationService.Delete(lang, userId); + } + package.Languages.Remove(nId.ToString()); + } + + scope.Complete(); } // create a summary of what was actually removed, for PackagingService.UninstalledPackage diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs index de6eeb6a42..70bef75192 100644 --- a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Core @@ -27,9 +28,17 @@ namespace Umbraco.Core { lock (liveFactory.SyncRoot) { - //Call refresh on the live factory to re-compile the models - liveFactory.Refresh(); - action(); + if (_suspend != null) + { + //if we are currently suspended, queue the action + _suspend.Queue(action); + } + else + { + //Call refresh on the live factory to re-compile the models + liveFactory.Refresh(); + action(); + } } } else @@ -37,5 +46,86 @@ namespace Umbraco.Core action(); } } + + public static IDisposable SuspendSafeLiveFactory(this IPublishedModelFactory factory) + { + if (factory is ILivePublishedModelFactory liveFactory) + { + lock (liveFactory.SyncRoot) + { + if (_suspend == null) + { + _suspend = new SuspendSafeLiveFactory( + factory, + () => _suspend = null); //reset when it's done + } + return _suspend; + } + } + else + { + return new SuspendSafeLiveFactory(factory); //returns a noop version of IDisposable, this won't actually do anything + } + } + + private static SuspendSafeLiveFactory _suspend; + } + + internal class SuspendSafeLiveFactory : IDisposable + { + private readonly IPublishedModelFactory _factory; + private readonly Action _reset; + private readonly List _actions = new List(); + + public SuspendSafeLiveFactory(IPublishedModelFactory factory, Action reset = null) + { + _factory = factory; + _reset = reset; + } + + /// + /// Queue an action to execute on disposal after rebuild + /// + /// + public void Queue(Action action) + { + _actions.Add(action); + } + + #region IDisposable Support + private bool disposedValue = false; // To detect redundant calls + + protected virtual void Dispose(bool disposing) + { + if (!disposedValue) + { + if (disposing) + { + if (_factory is ILivePublishedModelFactory liveFactory) + { + lock (liveFactory.SyncRoot) + { + //Call refresh on the live factory to re-compile the models + liveFactory.Refresh(); + + //then we need to call all queued actions + foreach(var action in _actions) + action(); + } + } + _reset?.Invoke(); + } + disposedValue = true; + } + } + + // This code added to correctly implement the disposable pattern. + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + Dispose(true); + } + #endregion + } } diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs index e99cbdca4a..82cf3a5813 100644 --- a/src/Umbraco.Web/Editors/ContentTypeController.cs +++ b/src/Umbraco.Web/Editors/ContentTypeController.cs @@ -17,9 +17,11 @@ using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; +using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Packaging; using Umbraco.Core.Persistence; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; @@ -46,6 +48,8 @@ namespace Umbraco.Web.Editors { private readonly IEntityXmlSerializer _serializer; private readonly PropertyEditorCollection _propertyEditors; + private readonly IScopeProvider _scopeProvider; + private readonly IPublishedModelFactory _publishedModelFactory; public ContentTypeController(IEntityXmlSerializer serializer, ICultureDictionaryFactory cultureDictionaryFactory, @@ -53,11 +57,14 @@ namespace Umbraco.Web.Editors IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, PropertyEditorCollection propertyEditors, ServiceContext services, AppCaches appCaches, - IProfilingLogger logger, IRuntimeState runtimeState, UmbracoHelper umbracoHelper) + IProfilingLogger logger, IRuntimeState runtimeState, UmbracoHelper umbracoHelper, + IScopeProvider scopeProvider, IPublishedModelFactory publishedModelFactory) : base(cultureDictionaryFactory, globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, umbracoHelper) { _serializer = serializer; _propertyEditors = propertyEditors; + _scopeProvider = scopeProvider; + _publishedModelFactory = publishedModelFactory; } public int GetCount() @@ -520,7 +527,7 @@ namespace Umbraco.Web.Editors } var dataInstaller = new PackageDataInstallation(Logger, Services.FileService, Services.MacroService, Services.LocalizationService, - Services.DataTypeService, Services.EntityService, Services.ContentTypeService, Services.ContentService, _propertyEditors); + Services.DataTypeService, Services.EntityService, Services.ContentTypeService, Services.ContentService, _propertyEditors, _scopeProvider, _publishedModelFactory); var xd = new XmlDocument {XmlResolver = null}; xd.Load(filePath); From 5898b641747d282a654ab38dd8547887995730c1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 14:24:25 +1000 Subject: [PATCH 22/40] disable force rebuilding pure live models on schema changes --- src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs | 4 ++-- src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index 266cddf6d5..cf90b60d8a 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -90,8 +90,8 @@ namespace Umbraco.Web.Cache // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - _publishedModelFactory.WithSafeLiveFactory(() => - _publishedSnapshotService.Notify(payloads)); + //_publishedModelFactory.WithSafeLiveFactory(() => + _publishedSnapshotService.Notify(payloads); // now we can trigger the event base.Refresh(payloads); diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index 1d41c0578b..fb4f59fce4 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -66,8 +66,8 @@ namespace Umbraco.Web.Cache // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - _publishedModelFactory.WithSafeLiveFactory(() => - _publishedSnapshotService.Notify(payloads)); + //_publishedModelFactory.WithSafeLiveFactory(() => + _publishedSnapshotService.Notify(payloads); base.Refresh(payloads); } From a85d8f3e21abaccf7b4f2457180ffebb04a00054 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 14:25:02 +1000 Subject: [PATCH 23/40] ensure that we don't CreateModel during cache notification --- src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index d6631b779d..b4d409f66f 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -883,7 +883,7 @@ namespace Umbraco.Web.PublishedCache.NuCache return true; var link = GetParentLink(kit.Node); var node = link?.Value; - return node?.PublishedModel != null; + return node != null && node.HasPublished; } private ContentNode GenCloneLocked(LinkedNode link) From bc42adb2ce0ed5c6765a38271cccd939f8b7e739 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 14:31:27 +1000 Subject: [PATCH 24/40] removes notifying all content/media of content type changes since nucache model creation is lazy now --- .../NuCache/PublishedSnapshotService.cs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 15e6574b40..cde45ae116 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -790,21 +790,22 @@ namespace Umbraco.Web.PublishedCache.NuCache Notify(_contentStore, payloads, RefreshContentTypesLocked); Notify(_mediaStore, payloads, RefreshMediaTypesLocked); - if (_publishedModelFactory.IsLiveFactory()) - { - //In the case of Pure Live - we actually need to refresh all of the content and the media - //see https://github.com/umbraco/Umbraco-CMS/issues/5671 - //The underlying issue is that in Pure Live the ILivePublishedModelFactory will re-compile all of the classes/models - //into a new DLL for the application which includes both content types and media types. - //Since the models in the cache are based on these actual classes, all of the objects in the cache need to be updated - //to use the newest version of the class. - using (_contentStore.GetScopedWriteLock(_scopeProvider)) - using (_mediaStore.GetScopedWriteLock(_scopeProvider)) - { - NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var draftChanged, out var publishedChanged); - NotifyLocked(new[] { new MediaCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var anythingChanged); - } - } + //TODO: I don't think this is necessary with the changes to nucache now that calls to `CreateModel` are lazy! + //if (_publishedModelFactory.IsLiveFactory()) + //{ + // //In the case of Pure Live - we actually need to refresh all of the content and the media + // //see https://github.com/umbraco/Umbraco-CMS/issues/5671 + // //The underlying issue is that in Pure Live the ILivePublishedModelFactory will re-compile all of the classes/models + // //into a new DLL for the application which includes both content types and media types. + // //Since the models in the cache are based on these actual classes, all of the objects in the cache need to be updated + // //to use the newest version of the class. + // using (_contentStore.GetScopedWriteLock(_scopeProvider)) + // using (_mediaStore.GetScopedWriteLock(_scopeProvider)) + // { + // NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var draftChanged, out var publishedChanged); + // NotifyLocked(new[] { new MediaCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var anythingChanged); + // } + //} ((PublishedSnapshot)CurrentPublishedSnapshot)?.Resync(); } From fc821000f3ef9d6c6be2f6d7dec7be4215af7d0a Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 14:50:05 +1000 Subject: [PATCH 25/40] Adds ext methods for creating a flag to indicate model recompilation should occur --- .../PublishedContentExtensionsForModels.cs | 10 +--- .../PublishedModelFactoryExtensions.cs | 56 +++++++++++++++++++ .../Cache/ContentTypeCacheRefresher.cs | 3 +- .../Cache/DataTypeCacheRefresher.cs | 3 +- .../NuCache/PublishedSnapshotService.cs | 33 +++++------ 5 files changed, 78 insertions(+), 27 deletions(-) diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs index bfc65b70d6..8e68bdae52 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs @@ -25,15 +25,7 @@ namespace Umbraco.Core.Models.PublishedContent // get model // if factory returns nothing, throw - var model = Current.PublishedModelFactory.CreateModel(content); - if (model == null) - throw new Exception("Factory returned null."); - - // if factory returns a different type, throw - if (!(model is IPublishedContent publishedContent)) - throw new Exception($"Factory returned model of type {model.GetType().FullName} which does not implement IPublishedContent."); - - return publishedContent; + return Current.PublishedModelFactory.CreateModelWithSafeLiveFactoryRefreshCheck(content); } } } diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs index 70bef75192..c36643095b 100644 --- a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -47,6 +47,62 @@ namespace Umbraco.Core } } + /// + /// Creates a strongly typed model while checking if the factory is and if a refresh flag has been set, in which + /// case the models will be recompiled before model creation + /// + /// + /// + /// + internal static IPublishedContent CreateModelWithSafeLiveFactoryRefreshCheck(this IPublishedModelFactory factory, IPublishedContent content) + { + if (factory is ILivePublishedModelFactory liveFactory && _refresh) + { + lock (liveFactory.SyncRoot) + { + if (_refresh) + { + _refresh = false; + //Call refresh on the live factory to re-compile the models + liveFactory.Refresh(); + } + } + } + + var model = factory.CreateModel(content); + if (model == null) + throw new Exception("Factory returned null."); + + // if factory returns a different type, throw + if (!(model is IPublishedContent publishedContent)) + throw new Exception($"Factory returned model of type {model.GetType().FullName} which does not implement IPublishedContent."); + + return publishedContent; + } + + /// + /// Sets a flag to re-compile the models if the is + /// + /// + /// + internal static void WithSafeLiveFactoryRefreshSet(this IPublishedModelFactory factory, Action action) + { + if (factory is ILivePublishedModelFactory liveFactory) + { + lock (liveFactory.SyncRoot) + { + _refresh = true; + action(); + } + } + else + { + action(); + } + } + + private static volatile bool _refresh = false; + public static IDisposable SuspendSafeLiveFactory(this IPublishedModelFactory factory) { if (factory is ILivePublishedModelFactory liveFactory) diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index cf90b60d8a..4d339ff336 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -91,7 +91,8 @@ namespace Umbraco.Web.Cache // we are using the database to load content into caches //_publishedModelFactory.WithSafeLiveFactory(() => - _publishedSnapshotService.Notify(payloads); + _publishedModelFactory.WithSafeLiveFactoryRefreshSet(() => + _publishedSnapshotService.Notify(payloads)); // now we can trigger the event base.Refresh(payloads); diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index fb4f59fce4..b80542af44 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -67,7 +67,8 @@ namespace Umbraco.Web.Cache // we are using the database to load content into caches //_publishedModelFactory.WithSafeLiveFactory(() => - _publishedSnapshotService.Notify(payloads); + _publishedModelFactory.WithSafeLiveFactoryRefreshSet(() => + _publishedSnapshotService.Notify(payloads)); base.Refresh(payloads); } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index cde45ae116..66cc5aaed0 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -790,22 +790,23 @@ namespace Umbraco.Web.PublishedCache.NuCache Notify(_contentStore, payloads, RefreshContentTypesLocked); Notify(_mediaStore, payloads, RefreshMediaTypesLocked); - //TODO: I don't think this is necessary with the changes to nucache now that calls to `CreateModel` are lazy! - //if (_publishedModelFactory.IsLiveFactory()) - //{ - // //In the case of Pure Live - we actually need to refresh all of the content and the media - // //see https://github.com/umbraco/Umbraco-CMS/issues/5671 - // //The underlying issue is that in Pure Live the ILivePublishedModelFactory will re-compile all of the classes/models - // //into a new DLL for the application which includes both content types and media types. - // //Since the models in the cache are based on these actual classes, all of the objects in the cache need to be updated - // //to use the newest version of the class. - // using (_contentStore.GetScopedWriteLock(_scopeProvider)) - // using (_mediaStore.GetScopedWriteLock(_scopeProvider)) - // { - // NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var draftChanged, out var publishedChanged); - // NotifyLocked(new[] { new MediaCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var anythingChanged); - // } - //} + //TODO: I don't think this is necessary with the changes to nucache now that calls to `CreateModel` are lazy? + // but I may be dreaming here, if i remove this call and save a content type, then the cache doesn't render a lot of the content. hrm. + if (_publishedModelFactory.IsLiveFactory()) + { + //In the case of Pure Live - we actually need to refresh all of the content and the media + //see https://github.com/umbraco/Umbraco-CMS/issues/5671 + //The underlying issue is that in Pure Live the ILivePublishedModelFactory will re-compile all of the classes/models + //into a new DLL for the application which includes both content types and media types. + //Since the models in the cache are based on these actual classes, all of the objects in the cache need to be updated + //to use the newest version of the class. + using (_contentStore.GetScopedWriteLock(_scopeProvider)) + using (_mediaStore.GetScopedWriteLock(_scopeProvider)) + { + NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var draftChanged, out var publishedChanged); + NotifyLocked(new[] { new MediaCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out var anythingChanged); + } + } ((PublishedSnapshot)CurrentPublishedSnapshot)?.Resync(); } From ebada94a489c339aba31f2646cca5dca8bd56150 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 14:59:12 +1000 Subject: [PATCH 26/40] temp - removing the SuspendSafeLiveFactory for now --- .../Packaging/PackageDataInstallation.cs | 2 +- .../PublishedModelFactoryExtensions.cs | 58 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs index 79e8d2fc00..133bdd0668 100644 --- a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs @@ -64,7 +64,7 @@ namespace Umbraco.Core.Packaging var removedDataTypes = new List(); var removedLanguages = new List(); - using(_publishedModelFactory.SuspendSafeLiveFactory()) //ensure that any PureLive models are not regenerated until after the bulk operation + //using(_publishedModelFactory.SuspendSafeLiveFactory()) //ensure that any PureLive models are not regenerated until after the bulk operation using (var scope = _scopeProvider.CreateScope()) { //Uninstall templates diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs index c36643095b..e314746ffd 100644 --- a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -28,17 +28,17 @@ namespace Umbraco.Core { lock (liveFactory.SyncRoot) { - if (_suspend != null) - { - //if we are currently suspended, queue the action - _suspend.Queue(action); - } - else - { + //if (_suspend != null) + //{ + // //if we are currently suspended, queue the action + // _suspend.Queue(action); + //} + //else + //{ //Call refresh on the live factory to re-compile the models liveFactory.Refresh(); action(); - } + //} } } else @@ -103,28 +103,28 @@ namespace Umbraco.Core private static volatile bool _refresh = false; - public static IDisposable SuspendSafeLiveFactory(this IPublishedModelFactory factory) - { - if (factory is ILivePublishedModelFactory liveFactory) - { - lock (liveFactory.SyncRoot) - { - if (_suspend == null) - { - _suspend = new SuspendSafeLiveFactory( - factory, - () => _suspend = null); //reset when it's done - } - return _suspend; - } - } - else - { - return new SuspendSafeLiveFactory(factory); //returns a noop version of IDisposable, this won't actually do anything - } - } + //public static IDisposable SuspendSafeLiveFactory(this IPublishedModelFactory factory) + //{ + // if (factory is ILivePublishedModelFactory liveFactory) + // { + // lock (liveFactory.SyncRoot) + // { + // if (_suspend == null) + // { + // _suspend = new SuspendSafeLiveFactory( + // factory, + // () => _suspend = null); //reset when it's done + // } + // return _suspend; + // } + // } + // else + // { + // return new SuspendSafeLiveFactory(factory); //returns a noop version of IDisposable, this won't actually do anything + // } + //} - private static SuspendSafeLiveFactory _suspend; + //private static SuspendSafeLiveFactory _suspend; } internal class SuspendSafeLiveFactory : IDisposable From 4c775822c0733bebde8cca7b5203715d5fb67f4f Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 16:22:05 +1000 Subject: [PATCH 27/40] fixes null ref issues since the field values were not copied on clone --- src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs index fad402a0eb..5f8e81fd31 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs @@ -101,6 +101,8 @@ namespace Umbraco.Web.PublishedCache.NuCache _draftData = origin._draftData; _publishedData = origin._publishedData; + _publishedSnapshotAccessor = origin._publishedSnapshotAccessor; + _variationContextAccessor = origin._variationContextAccessor; } // everything that is common to both draft and published versions From bc40216ecaebdab07c8167a0fb283448bfede08a Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 16:31:16 +1000 Subject: [PATCH 28/40] Ensure package installation for data is executed within a single scope/trans --- .../Packaging/PackageDataInstallation.cs | 26 ++++++++++++++++++- .../Packaging/PackageInstallation.cs | 15 +---------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs index 133bdd0668..d488f438ff 100644 --- a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs @@ -51,7 +51,7 @@ namespace Umbraco.Core.Packaging _contentService = contentService; } - #region Uninstall + #region Install/Uninstall public UninstallationSummary UninstallPackageData(PackageDefinition package, int userId) { @@ -176,6 +176,30 @@ namespace Umbraco.Core.Packaging } + public InstallationSummary InstallPackageData(CompiledPackage compiledPackage, int userId) + { + using (var scope = _scopeProvider.CreateScope()) + { + var installationSummary = new InstallationSummary + { + DataTypesInstalled = ImportDataTypes(compiledPackage.DataTypes.ToList(), userId), + LanguagesInstalled = ImportLanguages(compiledPackage.Languages, userId), + DictionaryItemsInstalled = ImportDictionaryItems(compiledPackage.DictionaryItems, userId), + MacrosInstalled = ImportMacros(compiledPackage.Macros, userId), + TemplatesInstalled = ImportTemplates(compiledPackage.Templates.ToList(), userId), + DocumentTypesInstalled = ImportDocumentTypes(compiledPackage.DocumentTypes, userId) + }; + + //we need a reference to the imported doc types to continue + var importedDocTypes = installationSummary.DocumentTypesInstalled.ToDictionary(x => x.Alias, x => x); + + installationSummary.StylesheetsInstalled = ImportStylesheets(compiledPackage.Stylesheets, userId); + installationSummary.ContentInstalled = ImportContent(compiledPackage.Documents, importedDocTypes, userId); + + return installationSummary; + } + } + #endregion #region Content diff --git a/src/Umbraco.Core/Packaging/PackageInstallation.cs b/src/Umbraco.Core/Packaging/PackageInstallation.cs index d791295b38..a42ee1aeb2 100644 --- a/src/Umbraco.Core/Packaging/PackageInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageInstallation.cs @@ -90,21 +90,8 @@ namespace Umbraco.Core.Packaging public InstallationSummary InstallPackageData(PackageDefinition packageDefinition, CompiledPackage compiledPackage, int userId) { - var installationSummary = new InstallationSummary - { - DataTypesInstalled = _packageDataInstallation.ImportDataTypes(compiledPackage.DataTypes.ToList(), userId), - LanguagesInstalled = _packageDataInstallation.ImportLanguages(compiledPackage.Languages, userId), - DictionaryItemsInstalled = _packageDataInstallation.ImportDictionaryItems(compiledPackage.DictionaryItems, userId), - MacrosInstalled = _packageDataInstallation.ImportMacros(compiledPackage.Macros, userId), - TemplatesInstalled = _packageDataInstallation.ImportTemplates(compiledPackage.Templates.ToList(), userId), - DocumentTypesInstalled = _packageDataInstallation.ImportDocumentTypes(compiledPackage.DocumentTypes, userId) - }; + var installationSummary = _packageDataInstallation.InstallPackageData(compiledPackage, userId); - //we need a reference to the imported doc types to continue - var importedDocTypes = installationSummary.DocumentTypesInstalled.ToDictionary(x => x.Alias, x => x); - - installationSummary.StylesheetsInstalled = _packageDataInstallation.ImportStylesheets(compiledPackage.Stylesheets, userId); - installationSummary.ContentInstalled = _packageDataInstallation.ImportContent(compiledPackage.Documents, importedDocTypes, userId); installationSummary.Actions = CompiledPackageXmlParser.GetPackageActions(XElement.Parse(compiledPackage.Actions), compiledPackage.Name); installationSummary.MetaData = compiledPackage; installationSummary.FilesInstalled = packageDefinition.Files; From 47d3af9bb3772cbd09aaa49f8a6205d6c2d3f9a4 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 16:59:57 +1000 Subject: [PATCH 29/40] removes the test suspendable model factory code --- .../Packaging/PackageDataInstallation.cs | 5 +- .../PublishedModelFactoryExtensions.cs | 94 +------------------ .../Cache/ContentTypeCacheRefresher.cs | 1 - .../Cache/DataTypeCacheRefresher.cs | 1 - .../Editors/ContentTypeController.cs | 6 +- .../NuCache/PublishedSnapshotService.cs | 2 - 6 files changed, 6 insertions(+), 103 deletions(-) diff --git a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs index d488f438ff..b6dd49bd18 100644 --- a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs @@ -28,15 +28,13 @@ namespace Umbraco.Core.Packaging private readonly IDataTypeService _dataTypeService; private readonly PropertyEditorCollection _propertyEditors; private readonly IScopeProvider _scopeProvider; - private readonly IPublishedModelFactory _publishedModelFactory; private readonly IEntityService _entityService; private readonly IContentTypeService _contentTypeService; private readonly IContentService _contentService; public PackageDataInstallation(ILogger logger, IFileService fileService, IMacroService macroService, ILocalizationService localizationService, IDataTypeService dataTypeService, IEntityService entityService, IContentTypeService contentTypeService, - IContentService contentService, PropertyEditorCollection propertyEditors, IScopeProvider scopeProvider, - IPublishedModelFactory publishedModelFactory) + IContentService contentService, PropertyEditorCollection propertyEditors, IScopeProvider scopeProvider) { _logger = logger; _fileService = fileService; @@ -45,7 +43,6 @@ namespace Umbraco.Core.Packaging _dataTypeService = dataTypeService; _propertyEditors = propertyEditors; _scopeProvider = scopeProvider; - _publishedModelFactory = publishedModelFactory; _entityService = entityService; _contentTypeService = contentTypeService; _contentService = contentService; diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs index e314746ffd..0e3aa48c53 100644 --- a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -28,17 +28,9 @@ namespace Umbraco.Core { lock (liveFactory.SyncRoot) { - //if (_suspend != null) - //{ - // //if we are currently suspended, queue the action - // _suspend.Queue(action); - //} - //else - //{ - //Call refresh on the live factory to re-compile the models - liveFactory.Refresh(); - action(); - //} + //Call refresh on the live factory to re-compile the models + liveFactory.Refresh(); + action(); } } else @@ -103,85 +95,5 @@ namespace Umbraco.Core private static volatile bool _refresh = false; - //public static IDisposable SuspendSafeLiveFactory(this IPublishedModelFactory factory) - //{ - // if (factory is ILivePublishedModelFactory liveFactory) - // { - // lock (liveFactory.SyncRoot) - // { - // if (_suspend == null) - // { - // _suspend = new SuspendSafeLiveFactory( - // factory, - // () => _suspend = null); //reset when it's done - // } - // return _suspend; - // } - // } - // else - // { - // return new SuspendSafeLiveFactory(factory); //returns a noop version of IDisposable, this won't actually do anything - // } - //} - - //private static SuspendSafeLiveFactory _suspend; - } - - internal class SuspendSafeLiveFactory : IDisposable - { - private readonly IPublishedModelFactory _factory; - private readonly Action _reset; - private readonly List _actions = new List(); - - public SuspendSafeLiveFactory(IPublishedModelFactory factory, Action reset = null) - { - _factory = factory; - _reset = reset; - } - - /// - /// Queue an action to execute on disposal after rebuild - /// - /// - public void Queue(Action action) - { - _actions.Add(action); - } - - #region IDisposable Support - private bool disposedValue = false; // To detect redundant calls - - protected virtual void Dispose(bool disposing) - { - if (!disposedValue) - { - if (disposing) - { - if (_factory is ILivePublishedModelFactory liveFactory) - { - lock (liveFactory.SyncRoot) - { - //Call refresh on the live factory to re-compile the models - liveFactory.Refresh(); - - //then we need to call all queued actions - foreach(var action in _actions) - action(); - } - } - _reset?.Invoke(); - } - disposedValue = true; - } - } - - // This code added to correctly implement the disposable pattern. - public void Dispose() - { - // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - Dispose(true); - } - #endregion - } } diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index 4d339ff336..df81dd85e8 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -90,7 +90,6 @@ namespace Umbraco.Web.Cache // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - //_publishedModelFactory.WithSafeLiveFactory(() => _publishedModelFactory.WithSafeLiveFactoryRefreshSet(() => _publishedSnapshotService.Notify(payloads)); diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index b80542af44..f86073abdd 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -66,7 +66,6 @@ namespace Umbraco.Web.Cache // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - //_publishedModelFactory.WithSafeLiveFactory(() => _publishedModelFactory.WithSafeLiveFactoryRefreshSet(() => _publishedSnapshotService.Notify(payloads)); diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs index 82cf3a5813..7c63061159 100644 --- a/src/Umbraco.Web/Editors/ContentTypeController.cs +++ b/src/Umbraco.Web/Editors/ContentTypeController.cs @@ -49,7 +49,6 @@ namespace Umbraco.Web.Editors private readonly IEntityXmlSerializer _serializer; private readonly PropertyEditorCollection _propertyEditors; private readonly IScopeProvider _scopeProvider; - private readonly IPublishedModelFactory _publishedModelFactory; public ContentTypeController(IEntityXmlSerializer serializer, ICultureDictionaryFactory cultureDictionaryFactory, @@ -58,13 +57,12 @@ namespace Umbraco.Web.Editors ISqlContext sqlContext, PropertyEditorCollection propertyEditors, ServiceContext services, AppCaches appCaches, IProfilingLogger logger, IRuntimeState runtimeState, UmbracoHelper umbracoHelper, - IScopeProvider scopeProvider, IPublishedModelFactory publishedModelFactory) + IScopeProvider scopeProvider) : base(cultureDictionaryFactory, globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, umbracoHelper) { _serializer = serializer; _propertyEditors = propertyEditors; _scopeProvider = scopeProvider; - _publishedModelFactory = publishedModelFactory; } public int GetCount() @@ -527,7 +525,7 @@ namespace Umbraco.Web.Editors } var dataInstaller = new PackageDataInstallation(Logger, Services.FileService, Services.MacroService, Services.LocalizationService, - Services.DataTypeService, Services.EntityService, Services.ContentTypeService, Services.ContentService, _propertyEditors, _scopeProvider, _publishedModelFactory); + Services.DataTypeService, Services.EntityService, Services.ContentTypeService, Services.ContentService, _propertyEditors, _scopeProvider); var xd = new XmlDocument {XmlResolver = null}; xd.Load(filePath); diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 66cc5aaed0..15e6574b40 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -790,8 +790,6 @@ namespace Umbraco.Web.PublishedCache.NuCache Notify(_contentStore, payloads, RefreshContentTypesLocked); Notify(_mediaStore, payloads, RefreshMediaTypesLocked); - //TODO: I don't think this is necessary with the changes to nucache now that calls to `CreateModel` are lazy? - // but I may be dreaming here, if i remove this call and save a content type, then the cache doesn't render a lot of the content. hrm. if (_publishedModelFactory.IsLiveFactory()) { //In the case of Pure Live - we actually need to refresh all of the content and the media From 00c720413a7ac08b9707326845f0078a9ae5c06c Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 17:23:00 +1000 Subject: [PATCH 30/40] Uses reflection to call ResetModels to flag MB to recompile. --- .../PublishedContentExtensionsForModels.cs | 10 +++- .../PublishedModelFactoryExtensions.cs | 52 +++++-------------- .../Cache/ContentTypeCacheRefresher.cs | 2 +- .../Cache/DataTypeCacheRefresher.cs | 2 +- 4 files changed, 25 insertions(+), 41 deletions(-) diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs index 8e68bdae52..bfc65b70d6 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs @@ -25,7 +25,15 @@ namespace Umbraco.Core.Models.PublishedContent // get model // if factory returns nothing, throw - return Current.PublishedModelFactory.CreateModelWithSafeLiveFactoryRefreshCheck(content); + var model = Current.PublishedModelFactory.CreateModel(content); + if (model == null) + throw new Exception("Factory returned null."); + + // if factory returns a different type, throw + if (!(model is IPublishedContent publishedContent)) + throw new Exception($"Factory returned model of type {model.GetType().FullName} which does not implement IPublishedContent."); + + return publishedContent; } } } diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs index 0e3aa48c53..e30b87d15e 100644 --- a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -40,50 +40,28 @@ namespace Umbraco.Core } /// - /// Creates a strongly typed model while checking if the factory is and if a refresh flag has been set, in which - /// case the models will be recompiled before model creation - /// - /// - /// - /// - internal static IPublishedContent CreateModelWithSafeLiveFactoryRefreshCheck(this IPublishedModelFactory factory, IPublishedContent content) - { - if (factory is ILivePublishedModelFactory liveFactory && _refresh) - { - lock (liveFactory.SyncRoot) - { - if (_refresh) - { - _refresh = false; - //Call refresh on the live factory to re-compile the models - liveFactory.Refresh(); - } - } - } - - var model = factory.CreateModel(content); - if (model == null) - throw new Exception("Factory returned null."); - - // if factory returns a different type, throw - if (!(model is IPublishedContent publishedContent)) - throw new Exception($"Factory returned model of type {model.GetType().FullName} which does not implement IPublishedContent."); - - return publishedContent; - } - - /// - /// Sets a flag to re-compile the models if the is + /// Sets a flag to reset the ModelsBuilder models if the is /// /// /// - internal static void WithSafeLiveFactoryRefreshSet(this IPublishedModelFactory factory, Action action) + /// + /// This does not recompile the pure live models, only sets a flag to tell models builder to recompile when they are requested. + /// + internal static void WithSafeLiveFactoryReset(this IPublishedModelFactory factory, Action action) { if (factory is ILivePublishedModelFactory liveFactory) { lock (liveFactory.SyncRoot) { - _refresh = true; + // TODO: Fix this in 8.3! - We need to change the ILivePublishedModelFactory interface to have a Reset method and then when we have an embedded MB + // version we will publicize the ResetModels (and change the name to Reset). + // For now, this will suffice and we'll use reflection, there should be no other implementation of ILivePublishedModelFactory. + // Calling ResetModels resets the MB flag so that the next time EnsureModels is called (which is called when nucache lazily calls CreateModel) it will + // trigger the recompiling of pure live models. + var resetMethod = liveFactory.GetType().GetMethod("ResetModels", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + if (resetMethod != null) + resetMethod.Invoke(liveFactory, null); + action(); } } @@ -93,7 +71,5 @@ namespace Umbraco.Core } } - private static volatile bool _refresh = false; - } } diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index df81dd85e8..dc1a4701d4 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -90,7 +90,7 @@ namespace Umbraco.Web.Cache // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - _publishedModelFactory.WithSafeLiveFactoryRefreshSet(() => + _publishedModelFactory.WithSafeLiveFactoryReset(() => _publishedSnapshotService.Notify(payloads)); // now we can trigger the event diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index f86073abdd..0ddb8c4bf6 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -66,7 +66,7 @@ namespace Umbraco.Web.Cache // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - _publishedModelFactory.WithSafeLiveFactoryRefreshSet(() => + _publishedModelFactory.WithSafeLiveFactoryReset(() => _publishedSnapshotService.Notify(payloads)); base.Refresh(payloads); From c27e876c75633d552ab84e37bf48e2da3d39cc02 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 17:40:48 +1000 Subject: [PATCH 31/40] Removes BackgroundPublishedSnapshotNotifier --- .../Routing/UmbracoModuleTests.cs | 5 - .../TestHelpers/TestWithDatabaseBase.cs | 2 - .../BackgroundPublishedSnapshotNotifier.cs | 120 ------------------ src/Umbraco.Web/Runtime/WebInitialComposer.cs | 1 - src/Umbraco.Web/Umbraco.Web.csproj | 1 - src/Umbraco.Web/UmbracoInjectedModule.cs | 9 -- 6 files changed, 138 deletions(-) delete mode 100644 src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs diff --git a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs index 060ce549dd..e95fdfc87c 100644 --- a/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs +++ b/src/Umbraco.Tests/Routing/UmbracoModuleTests.cs @@ -42,11 +42,6 @@ namespace Umbraco.Tests.Routing logger, null, // FIXME: PublishedRouter complexities... Mock.Of(), - Mock.Of(), - new Umbraco.Web.Cache.BackgroundPublishedSnapshotNotifier( - Factory.GetInstance(), - Factory.GetInstance(), - Logger), new RoutableDocumentFilter(globalSettings) ); diff --git a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs index 7ee75b1d9c..9cc2b97445 100644 --- a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs +++ b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs @@ -91,8 +91,6 @@ namespace Umbraco.Tests.TestHelpers factory.ResetForTests(); return factory; }); - - Composition.RegisterUnique(); } [OneTimeTearDown] diff --git a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs b/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs deleted file mode 100644 index e2b3c07cea..0000000000 --- a/src/Umbraco.Web/Cache/BackgroundPublishedSnapshotNotifier.cs +++ /dev/null @@ -1,120 +0,0 @@ -using System.Threading; -using System.Threading.Tasks; -using Umbraco.Core; -using Umbraco.Core.Models.PublishedContent; -using Umbraco.Core.Logging; -using Umbraco.Web.PublishedCache; -using Umbraco.Web.Scheduling; - -namespace Umbraco.Web.Cache -{ - /// - /// Used to notify the of changes using a background thread - /// - /// - /// When in Pure Live mode, the models need to be rebuilt before the IPublishedSnapshotService is notified which can result in performance penalties so - /// this performs these actions on a background thread so the user isn't waiting for the rebuilding to occur on a UI thread. - /// When using this, even when not in Pure Live mode, it still means that the cache is notified on a background thread. - /// In order to wait for the processing to complete, this class has a Wait() method which will block until the processing is finished. - /// - public sealed class BackgroundPublishedSnapshotNotifier - { - private readonly IPublishedModelFactory _publishedModelFactory; - private readonly IPublishedSnapshotService _publishedSnapshotService; - private readonly BackgroundTaskRunner _runner; - - /// - /// Constructor - /// - /// - /// - /// - public BackgroundPublishedSnapshotNotifier(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService, ILogger logger) - { - _publishedModelFactory = publishedModelFactory; - _publishedSnapshotService = publishedSnapshotService; - _runner = new BackgroundTaskRunner("RebuildModelsAndCache", logger); - } - - /// - /// Blocks until the background operation is completed - /// - /// Returns true if waiting was necessary - public bool Wait() - { - var running = _runner.IsRunning; - _runner.StoppedAwaitable.GetAwaiter().GetResult(); //TODO: do we need a try/catch? - return running; - } - - /// - /// Notify the of content type changes - /// - /// - public void NotifyWithSafeLiveFactory(ContentTypeCacheRefresher.JsonPayload[] payloads) - { - _runner.TryAdd(new RebuildModelsAndCacheTask(payloads, _publishedModelFactory, _publishedSnapshotService)); - } - - /// - /// Notify the of data type changes - /// - /// - public void NotifyWithSafeLiveFactory(DataTypeCacheRefresher.JsonPayload[] payloads) - { - _runner.TryAdd(new RebuildModelsAndCacheTask(payloads, _publishedModelFactory, _publishedSnapshotService)); - } - - /// - /// A simple background task that notifies the of changes - /// - private class RebuildModelsAndCacheTask : IBackgroundTask - { - private readonly DataTypeCacheRefresher.JsonPayload[] _dataTypePayloads; - private readonly ContentTypeCacheRefresher.JsonPayload[] _contentTypePayloads; - private readonly IPublishedModelFactory _publishedModelFactory; - private readonly IPublishedSnapshotService _publishedSnapshotService; - - private RebuildModelsAndCacheTask(IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService) - { - _publishedModelFactory = publishedModelFactory; - _publishedSnapshotService = publishedSnapshotService; - } - - public RebuildModelsAndCacheTask(DataTypeCacheRefresher.JsonPayload[] payloads, IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService) - : this(publishedModelFactory, publishedSnapshotService) - { - _dataTypePayloads = payloads; - } - - public RebuildModelsAndCacheTask(ContentTypeCacheRefresher.JsonPayload[] payloads, IPublishedModelFactory publishedModelFactory, IPublishedSnapshotService publishedSnapshotService) - : this(publishedModelFactory, publishedSnapshotService) - { - _contentTypePayloads = payloads; - } - - public void Run() - { - // we have to refresh models before we notify the published snapshot - // service of changes, else factories may try to rebuild models while - // we are using the database to load content into caches - - _publishedModelFactory.WithSafeLiveFactory(() => - { - if (_dataTypePayloads != null) - _publishedSnapshotService.Notify(_dataTypePayloads); - if (_contentTypePayloads != null) - _publishedSnapshotService.Notify(_contentTypePayloads); - }); - } - - public Task RunAsync(CancellationToken token) => throw new System.NotImplementedException(); - - public bool IsAsync => false; - - public void Dispose() - { - } - } - } -} diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index 9e3413ba4d..87c0f46fba 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -125,7 +125,6 @@ namespace Umbraco.Web.Runtime // register distributed cache composition.RegisterUnique(f => new DistributedCache()); - composition.RegisterUnique(); composition.RegisterUnique(); // replace some services diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 1b8bc1b512..c6945a6b15 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -115,7 +115,6 @@ - diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 69af843644..75251abaeb 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.IO; using System.Web; using System.Web.Routing; using Umbraco.Core; @@ -12,8 +11,6 @@ using Umbraco.Web.Routing; using Umbraco.Core.Exceptions; using Umbraco.Core.Security; using Umbraco.Web.Composing; -using Umbraco.Web.Cache; -using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Web { @@ -39,8 +36,6 @@ namespace Umbraco.Web private readonly ILogger _logger; private readonly IPublishedRouter _publishedRouter; private readonly IUmbracoContextFactory _umbracoContextFactory; - private readonly IPublishedModelFactory _publishedModelFactory; - private readonly BackgroundPublishedSnapshotNotifier _backgroundNotifier; private readonly RoutableDocumentFilter _routableDocumentLookup; public UmbracoInjectedModule( @@ -49,8 +44,6 @@ namespace Umbraco.Web ILogger logger, IPublishedRouter publishedRouter, IUmbracoContextFactory umbracoContextFactory, - IPublishedModelFactory publishedModelFactory, - BackgroundPublishedSnapshotNotifier backgroundNotifier, RoutableDocumentFilter routableDocumentLookup) { _globalSettings = globalSettings; @@ -58,8 +51,6 @@ namespace Umbraco.Web _logger = logger; _publishedRouter = publishedRouter; _umbracoContextFactory = umbracoContextFactory; - _publishedModelFactory = publishedModelFactory; - _backgroundNotifier = backgroundNotifier; _routableDocumentLookup = routableDocumentLookup; } From de8d253a7ca9697b6007c48e2cc0d97307a7ceb3 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 17:44:50 +1000 Subject: [PATCH 32/40] obsoletes WithSafeLiveFactory --- src/Umbraco.Core/PublishedModelFactoryExtensions.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs index e30b87d15e..ac8c9f1be7 100644 --- a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Core @@ -16,12 +17,8 @@ namespace Umbraco.Core /// public static bool IsLiveFactory(this IPublishedModelFactory factory) => factory is ILivePublishedModelFactory; - /// - /// Executes an action with a safe live factory - /// - /// - /// If the factory is a live factory, ensures it is refreshed and locked while executing the action. - /// + [Obsolete("This method is no longer used or necessary and will be removed from future")] + [EditorBrowsable(EditorBrowsableState.Never)] public static void WithSafeLiveFactory(this IPublishedModelFactory factory, Action action) { if (factory is ILivePublishedModelFactory liveFactory) From 6d886e38109894a3a8adde3f6010baf9401e3eb3 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 16 Sep 2019 17:55:04 +1000 Subject: [PATCH 33/40] removes uneeded IsDocumentRequest method --- src/Umbraco.Web/UmbracoContext.cs | 19 +------------------ src/Umbraco.Web/UmbracoInjectedModule.cs | 2 +- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index 392eb03d64..347f79e51b 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -290,24 +290,7 @@ namespace Umbraco.Web _previewing = _previewToken.IsNullOrWhiteSpace() == false; } - - private bool? _isDocumentRequest; - - /// - /// Checks if the request is a document request (i.e. one that the module should handle) - /// - /// - /// - /// - internal bool IsDocumentRequest(RoutableDocumentFilter docLookup) - { - if (_isDocumentRequest.HasValue) - return _isDocumentRequest.Value; - - _isDocumentRequest = docLookup.IsDocumentRequest(HttpContext, OriginalRequestUrl); - return _isDocumentRequest.Value; - } - + // say we render a macro or RTE in a give 'preview' mode that might not be the 'current' one, // then due to the way it all works at the moment, the 'current' published snapshot need to be in the proper // default 'preview' mode - somehow we have to force it. and that could be recursive. diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 75251abaeb..6dc41d39ee 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -161,7 +161,7 @@ namespace Umbraco.Web var reason = EnsureRoutableOutcome.IsRoutable; // ensure this is a document request - if (!context.IsDocumentRequest(_routableDocumentLookup)) + if (!_routableDocumentLookup.IsDocumentRequest(httpContext, context.OriginalRequestUrl)) { reason = EnsureRoutableOutcome.NotDocumentRequest; } From 54110573bca12a79007856097290a41f2effac07 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 17 Sep 2019 17:00:21 +1000 Subject: [PATCH 34/40] dont hide model binding exceptions if in debug mode --- src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs b/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs index b9161dbea0..ad0c036bfc 100644 --- a/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs +++ b/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs @@ -6,7 +6,8 @@ using System.Web.Mvc; namespace Umbraco.Web.Mvc { /// - /// An exception filter checking if we get a or with the same model. in which case it returns a redirect to the same page after 1 sec. + /// An exception filter checking if we get a or with the same model. + /// In which case it returns a redirect to the same page after 1 sec if not in debug mode. /// internal class ModelBindingExceptionFilter : FilterAttribute, IExceptionFilter { @@ -14,7 +15,8 @@ namespace Umbraco.Web.Mvc public void OnException(ExceptionContext filterContext) { - if (!filterContext.ExceptionHandled + if (!filterContext.HttpContext.IsDebuggingEnabled + && !filterContext.ExceptionHandled && ((filterContext.Exception is ModelBindingException || filterContext.Exception is InvalidCastException) && IsMessageAboutTheSameModelType(filterContext.Exception.Message))) { From 7fdbb84c04e0837761a173b209cc6c7501be3d8a Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 17 Sep 2019 22:38:43 +1000 Subject: [PATCH 35/40] Fixes bug with nucache - it wasn't using gen/snapshots the way it was supposed to --- .../PublishedCache/NuCache/ContentStore.cs | 66 +++++++++++++------ 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 884ccb0f74..5157f9493e 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -470,7 +470,7 @@ namespace Umbraco.Web.PublishedCache.NuCache private bool BuildKit(ContentNodeKit kit, out LinkedNode parent) { // make sure parent exists - parent = GetParentLink(kit.Node); + parent = GetParentLink(kit.Node, _liveGen); if (parent == null) { _logger.Warn($"Skip item id={kit.Node.Id}, could not find parent id={kit.Node.ParentContentId}."); @@ -795,7 +795,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var id = content.FirstChildContentId; while (id > 0) { - var link = GetRequiredLinkedNode(id, "child"); + var link = GetRequiredLinkedNode(id, "child", _liveGen); ClearBranchLocked(link.Value); id = link.Value.NextSiblingContentId; } @@ -807,10 +807,20 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// /// - private LinkedNode GetRequiredLinkedNode(int id, string description) + private LinkedNode GetRequiredLinkedNode(int id, string description, long gen) { - if (_contentNodes.TryGetValue(id, out var link) && link.Value != null) - return link; + if (_contentNodes.TryGetValue(id, out var link)) + { + //find the correct snapshot + while (link != null && link.Gen != gen) + { + link = link.Next; + } + + if (link.Value != null) + return link; + } + throw new PanicException($"failed to get {description} with id={id}"); } @@ -818,11 +828,27 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// Gets the parent link node, may be null or root if ParentContentId is less than 0 /// - private LinkedNode GetParentLink(ContentNode content) + private LinkedNode GetParentLink(ContentNode content, long gen) { - if (content.ParentContentId < 0) return _root; + if (content.ParentContentId < 0) + { + var root = _root; + //find the correct snapshot + while (root != null && root.Gen != gen) + { + root = root.Next; + } + return root; + } - _contentNodes.TryGetValue(content.ParentContentId, out var link); + if (_contentNodes.TryGetValue(content.ParentContentId, out var link)) + { + //find the correct snapshot + while (link != null && link.Gen != gen) + { + link = link.Next; + } + } return link; } @@ -831,16 +857,16 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// /// - private LinkedNode GetRequiredParentLink(ContentNode content) + private LinkedNode GetRequiredParentLink(ContentNode content, long gen) { - return content.ParentContentId < 0 ? _root : GetRequiredLinkedNode(content.ParentContentId, "parent"); + return content.ParentContentId < 0 ? _root : GetRequiredLinkedNode(content.ParentContentId, "parent", gen); } private void RemoveTreeNodeLocked(ContentNode content) { var parentLink = content.ParentContentId < 0 ? _root - : GetRequiredLinkedNode(content.ParentContentId, "parent"); + : GetRequiredLinkedNode(content.ParentContentId, "parent", _liveGen); var parent = parentLink.Value; @@ -863,14 +889,14 @@ namespace Umbraco.Web.PublishedCache.NuCache if (content.NextSiblingContentId > 0) { - var nextLink = GetRequiredLinkedNode(content.NextSiblingContentId, "next sibling"); + var nextLink = GetRequiredLinkedNode(content.NextSiblingContentId, "next sibling", _liveGen); var next = GenCloneLocked(nextLink); next.PreviousSiblingContentId = content.PreviousSiblingContentId; } if (content.PreviousSiblingContentId > 0) { - var prevLink = GetRequiredLinkedNode(content.PreviousSiblingContentId, "previous sibling"); + var prevLink = GetRequiredLinkedNode(content.PreviousSiblingContentId, "previous sibling", _liveGen); var prev = GenCloneLocked(prevLink); prev.NextSiblingContentId = content.NextSiblingContentId; } @@ -883,7 +909,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { if (kit.Node.ParentContentId < 0) return true; - var link = GetParentLink(kit.Node); + var link = GetParentLink(kit.Node, _liveGen); var node = link?.Value; return node != null && node.HasPublished; } @@ -909,10 +935,12 @@ namespace Umbraco.Web.PublishedCache.NuCache /// private void AddTreeNodeLocked(ContentNode content, LinkedNode parentLink = null) { - parentLink = parentLink ?? GetRequiredParentLink(content); + parentLink = parentLink ?? GetRequiredParentLink(content, _liveGen); var parent = parentLink.Value; + var currentGen = parentLink.Gen; + // if parent has no children, clone parent + add as first child if (parent.FirstChildContentId < 0) { @@ -923,7 +951,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } // get parent's first child - var childLink = GetRequiredLinkedNode(parent.FirstChildContentId, "first child"); + var childLink = GetRequiredLinkedNode(parent.FirstChildContentId, "first child", currentGen); var child = childLink.Value; // if first, clone parent + insert as first child @@ -943,7 +971,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } // get parent's last child - var lastChildLink = GetRequiredLinkedNode(parent.LastChildContentId, "last child"); + var lastChildLink = GetRequiredLinkedNode(parent.LastChildContentId, "last child", currentGen); var lastChild = lastChildLink.Value; // if last, clone parent + append as last child @@ -968,7 +996,7 @@ namespace Umbraco.Web.PublishedCache.NuCache while (child.NextSiblingContentId > 0) { // get next child - var nextChildLink = GetRequiredLinkedNode(child.NextSiblingContentId, "next child"); + var nextChildLink = GetRequiredLinkedNode(child.NextSiblingContentId, "next child", currentGen); var nextChild = nextChildLink.Value; // if here, clone previous + append/insert @@ -1086,7 +1114,7 @@ namespace Umbraco.Web.PublishedCache.NuCache while (id > 0) { - var link = GetRequiredLinkedNode(id, "sibling"); + var link = GetRequiredLinkedNode(id, "root", gen); yield return link.Value; id = link.Value.NextSiblingContentId; } From 1eb1c9dc4dfeefb03895c277797e6514a34dc162 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 17 Sep 2019 22:50:57 +1000 Subject: [PATCH 36/40] allows for disabling the modelbinderexceptionfilter from config for testing, adds notes --- src/Umbraco.Web/Mvc/ContentModelBinder.cs | 5 ++++- src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/Mvc/ContentModelBinder.cs b/src/Umbraco.Web/Mvc/ContentModelBinder.cs index c6df52e007..052938807a 100644 --- a/src/Umbraco.Web/Mvc/ContentModelBinder.cs +++ b/src/Umbraco.Web/Mvc/ContentModelBinder.cs @@ -135,13 +135,16 @@ namespace Umbraco.Web.Mvc msg.Append(modelType.FullName); msg.Append("."); -// raise event, to give model factories a chance at reporting + // raise event, to give model factories a chance at reporting // the error with more details, and optionally request that // the application restarts. var args = new ModelBindingArgs(sourceType, modelType, msg); ModelBindingException?.Invoke(Instance, args); + // TODO: with all of the tests I've done i don't think restarting the app here is required anymore, + // when I don't have this code enabled and i get a model binding error and just refresh, it fixes itself. + // We'll leave this for now though. if (args.Restart) { msg.Append(" The application is restarting now."); diff --git a/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs b/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs index ad0c036bfc..19435d614c 100644 --- a/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs +++ b/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs @@ -1,4 +1,5 @@ using System; +using System.Configuration; using System.Net; using System.Text.RegularExpressions; using System.Web.Mvc; @@ -15,7 +16,7 @@ namespace Umbraco.Web.Mvc public void OnException(ExceptionContext filterContext) { - if (!filterContext.HttpContext.IsDebuggingEnabled + if (ConfigurationManager.AppSettings["Umbraco.Web.DisableModelBindingExceptionFilter"] != "true" && !filterContext.ExceptionHandled && ((filterContext.Exception is ModelBindingException || filterContext.Exception is InvalidCastException) && IsMessageAboutTheSameModelType(filterContext.Exception.Message))) From ef007382b5cb79c0b970135e79225831f1b00aa7 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 17 Sep 2019 22:58:06 +1000 Subject: [PATCH 37/40] only enable the ModelBindingExceptionFilter when running PureLive, there should be no other reason to have this filter --- src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs b/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs index 19435d614c..5d8fbdac2f 100644 --- a/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs +++ b/src/Umbraco.Web/Mvc/ModelBindingExceptionFilter.cs @@ -3,6 +3,8 @@ using System.Configuration; using System.Net; using System.Text.RegularExpressions; using System.Web.Mvc; +using Umbraco.Core; +using Umbraco.Web.Composing; namespace Umbraco.Web.Mvc { @@ -10,13 +12,17 @@ namespace Umbraco.Web.Mvc /// An exception filter checking if we get a or with the same model. /// In which case it returns a redirect to the same page after 1 sec if not in debug mode. /// + /// + /// This is only enabled when running PureLive + /// internal class ModelBindingExceptionFilter : FilterAttribute, IExceptionFilter { private static readonly Regex GetPublishedModelsTypesRegex = new Regex("Umbraco.Web.PublishedModels.(\\w+)", RegexOptions.Compiled); public void OnException(ExceptionContext filterContext) { - if (ConfigurationManager.AppSettings["Umbraco.Web.DisableModelBindingExceptionFilter"] != "true" + if (Current.PublishedModelFactory.IsLiveFactory() + && ConfigurationManager.AppSettings["Umbraco.Web.DisableModelBindingExceptionFilter"] != "true" && !filterContext.ExceptionHandled && ((filterContext.Exception is ModelBindingException || filterContext.Exception is InvalidCastException) && IsMessageAboutTheSameModelType(filterContext.Exception.Message))) From ab79cd74b8cdbc00160020911afef173490f70d9 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 17 Sep 2019 23:31:32 +1000 Subject: [PATCH 38/40] Fixes lazy iteration issue on package install --- .../Packaging/PackageDataInstallation.cs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs index b6dd49bd18..03e03afaaa 100644 --- a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs @@ -61,7 +61,6 @@ namespace Umbraco.Core.Packaging var removedDataTypes = new List(); var removedLanguages = new List(); - //using(_publishedModelFactory.SuspendSafeLiveFactory()) //ensure that any PureLive models are not regenerated until after the bulk operation using (var scope = _scopeProvider.CreateScope()) { //Uninstall templates @@ -202,9 +201,9 @@ namespace Umbraco.Core.Packaging #region Content - public IEnumerable ImportContent(IEnumerable docs, IDictionary importedDocumentTypes, int userId) + public IReadOnlyList ImportContent(IEnumerable docs, IDictionary importedDocumentTypes, int userId) { - return docs.SelectMany(x => ImportContent(x, -1, importedDocumentTypes, userId)); + return docs.SelectMany(x => ImportContent(x, -1, importedDocumentTypes, userId)).ToList(); } /// @@ -385,7 +384,7 @@ namespace Umbraco.Core.Packaging #region DocumentTypes - public IEnumerable ImportDocumentType(XElement docTypeElement, int userId) + public IReadOnlyList ImportDocumentType(XElement docTypeElement, int userId) { return ImportDocumentTypes(new[] { docTypeElement }, userId); } @@ -396,7 +395,7 @@ namespace Umbraco.Core.Packaging /// Xml to import /// Optional id of the User performing the operation. Default is zero (admin). /// An enumerable list of generated ContentTypes - public IEnumerable ImportDocumentTypes(IEnumerable docTypeElements, int userId) + public IReadOnlyList ImportDocumentTypes(IEnumerable docTypeElements, int userId) { return ImportDocumentTypes(docTypeElements.ToList(), true, userId); } @@ -408,7 +407,7 @@ namespace Umbraco.Core.Packaging /// Boolean indicating whether or not to import the /// Optional id of the User performing the operation. Default is zero (admin). /// An enumerable list of generated ContentTypes - public IEnumerable ImportDocumentTypes(IReadOnlyCollection unsortedDocumentTypes, bool importStructure, int userId) + public IReadOnlyList ImportDocumentTypes(IReadOnlyCollection unsortedDocumentTypes, bool importStructure, int userId) { var importedContentTypes = new Dictionary(); @@ -857,7 +856,7 @@ namespace Umbraco.Core.Packaging /// Xml to import /// Optional id of the user /// An enumerable list of generated DataTypeDefinitions - public IEnumerable ImportDataTypes(IReadOnlyCollection dataTypeElements, int userId) + public IReadOnlyList ImportDataTypes(IReadOnlyCollection dataTypeElements, int userId) { var dataTypes = new List(); @@ -986,13 +985,13 @@ namespace Umbraco.Core.Packaging /// Xml to import /// /// An enumerable list of dictionary items - public IEnumerable ImportDictionaryItems(IEnumerable dictionaryItemElementList, int userId) + public IReadOnlyList ImportDictionaryItems(IEnumerable dictionaryItemElementList, int userId) { var languages = _localizationService.GetAllLanguages().ToList(); return ImportDictionaryItems(dictionaryItemElementList, languages, null, userId); } - private IEnumerable ImportDictionaryItems(IEnumerable dictionaryItemElementList, List languages, Guid? parentId, int userId) + private IReadOnlyList ImportDictionaryItems(IEnumerable dictionaryItemElementList, List languages, Guid? parentId, int userId) { var items = new List(); foreach (var dictionaryItemElement in dictionaryItemElementList) @@ -1069,7 +1068,7 @@ namespace Umbraco.Core.Packaging /// Xml to import /// Optional id of the User performing the operation /// An enumerable list of generated languages - public IEnumerable ImportLanguages(IEnumerable languageElements, int userId) + public IReadOnlyList ImportLanguages(IEnumerable languageElements, int userId) { var list = new List(); foreach (var languageElement in languageElements) @@ -1098,7 +1097,7 @@ namespace Umbraco.Core.Packaging /// Xml to import /// Optional id of the User performing the operation /// - public IEnumerable ImportMacros(IEnumerable macroElements, int userId) + public IReadOnlyList ImportMacros(IEnumerable macroElements, int userId) { var macros = macroElements.Select(ParseMacroElement).ToList(); @@ -1188,7 +1187,7 @@ namespace Umbraco.Core.Packaging #region Stylesheets - public IEnumerable ImportStylesheets(IEnumerable stylesheetElements, int userId) + public IReadOnlyList ImportStylesheets(IEnumerable stylesheetElements, int userId) { var result = new List(); @@ -1256,7 +1255,7 @@ namespace Umbraco.Core.Packaging /// Xml to import /// Optional user id /// An enumerable list of generated Templates - public IEnumerable ImportTemplates(IReadOnlyCollection templateElements, int userId) + public IReadOnlyList ImportTemplates(IReadOnlyCollection templateElements, int userId) { var templates = new List(); From e6805b23e383748626ffb269dd5106352c868144 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 18 Sep 2019 00:10:02 +1000 Subject: [PATCH 39/40] Oops! didn't complete the scope, ok now package install works within a trans, fixes nucache changes to not pass around _liveGen since sometimes we want the latest that is not _liveGen --- .../Packaging/PackageDataInstallation.cs | 4 +- .../PublishedCache/NuCache/ContentStore.cs | 69 ++++++++++--------- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs index 03e03afaaa..281cc2c396 100644 --- a/src/Umbraco.Core/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Core/Packaging/PackageDataInstallation.cs @@ -191,7 +191,9 @@ namespace Umbraco.Core.Packaging installationSummary.StylesheetsInstalled = ImportStylesheets(compiledPackage.Stylesheets, userId); installationSummary.ContentInstalled = ImportContent(compiledPackage.Documents, importedDocTypes, userId); - + + scope.Complete(); + return installationSummary; } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 5157f9493e..ae439db579 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -470,7 +470,7 @@ namespace Umbraco.Web.PublishedCache.NuCache private bool BuildKit(ContentNodeKit kit, out LinkedNode parent) { // make sure parent exists - parent = GetParentLink(kit.Node, _liveGen); + parent = GetParentLink(kit.Node, null); if (parent == null) { _logger.Warn($"Skip item id={kit.Node.Id}, could not find parent id={kit.Node.ParentContentId}."); @@ -795,7 +795,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var id = content.FirstChildContentId; while (id > 0) { - var link = GetRequiredLinkedNode(id, "child", _liveGen); + var link = GetRequiredLinkedNode(id, "child", null); ClearBranchLocked(link.Value); id = link.Value.NextSiblingContentId; } @@ -806,21 +806,16 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// /// + /// the generation requested, null for the latest stored /// - private LinkedNode GetRequiredLinkedNode(int id, string description, long gen) + private LinkedNode GetRequiredLinkedNode(int id, string description, long? gen) { if (_contentNodes.TryGetValue(id, out var link)) { - //find the correct snapshot - while (link != null && link.Gen != gen) - { - link = link.Next; - } - - if (link.Value != null) + link = GetLinkedNodeGen(link, gen); + if (link != null && link.Value != null) return link; - } - + } throw new PanicException($"failed to get {description} with id={id}"); } @@ -828,27 +823,18 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// Gets the parent link node, may be null or root if ParentContentId is less than 0 /// - private LinkedNode GetParentLink(ContentNode content, long gen) + /// the generation requested, null for the latest stored + private LinkedNode GetParentLink(ContentNode content, long? gen) { if (content.ParentContentId < 0) { - var root = _root; - //find the correct snapshot - while (root != null && root.Gen != gen) - { - root = root.Next; - } + var root = GetLinkedNodeGen(_root, gen); return root; } if (_contentNodes.TryGetValue(content.ParentContentId, out var link)) - { - //find the correct snapshot - while (link != null && link.Gen != gen) - { - link = link.Next; - } - } + link = GetLinkedNodeGen(link, gen); + return link; } @@ -856,17 +842,36 @@ namespace Umbraco.Web.PublishedCache.NuCache /// Gets the linked parent node and if it doesn't exist throw a /// /// + /// the generation requested, null for the latest stored /// - private LinkedNode GetRequiredParentLink(ContentNode content, long gen) + private LinkedNode GetRequiredParentLink(ContentNode content, long? gen) { return content.ParentContentId < 0 ? _root : GetRequiredLinkedNode(content.ParentContentId, "parent", gen); } + /// + /// Iterates over the LinkedNode's generations to find the correct one + /// + /// + /// The generation requested, use null to avoid the lookup + /// + private LinkedNode GetLinkedNodeGen(LinkedNode link, long? gen) + { + if (!gen.HasValue) return link; + + //find the correct snapshot + while (link != null && link.Gen != gen) + { + link = link.Next; + } + return link; + } + private void RemoveTreeNodeLocked(ContentNode content) { var parentLink = content.ParentContentId < 0 ? _root - : GetRequiredLinkedNode(content.ParentContentId, "parent", _liveGen); + : GetRequiredLinkedNode(content.ParentContentId, "parent", null); var parent = parentLink.Value; @@ -889,14 +894,14 @@ namespace Umbraco.Web.PublishedCache.NuCache if (content.NextSiblingContentId > 0) { - var nextLink = GetRequiredLinkedNode(content.NextSiblingContentId, "next sibling", _liveGen); + var nextLink = GetRequiredLinkedNode(content.NextSiblingContentId, "next sibling", null); var next = GenCloneLocked(nextLink); next.PreviousSiblingContentId = content.PreviousSiblingContentId; } if (content.PreviousSiblingContentId > 0) { - var prevLink = GetRequiredLinkedNode(content.PreviousSiblingContentId, "previous sibling", _liveGen); + var prevLink = GetRequiredLinkedNode(content.PreviousSiblingContentId, "previous sibling", null); var prev = GenCloneLocked(prevLink); prev.NextSiblingContentId = content.NextSiblingContentId; } @@ -909,7 +914,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { if (kit.Node.ParentContentId < 0) return true; - var link = GetParentLink(kit.Node, _liveGen); + var link = GetParentLink(kit.Node, null); var node = link?.Value; return node != null && node.HasPublished; } @@ -935,7 +940,7 @@ namespace Umbraco.Web.PublishedCache.NuCache /// private void AddTreeNodeLocked(ContentNode content, LinkedNode parentLink = null) { - parentLink = parentLink ?? GetRequiredParentLink(content, _liveGen); + parentLink = parentLink ?? GetRequiredParentLink(content, null); var parent = parentLink.Value; From cc1e46ae056ed87c4e6d5eea8d84be80772b6845 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 18 Sep 2019 00:36:12 +1000 Subject: [PATCH 40/40] Simplifies ContentStore for all the instances it's trying to find a linked node by a gen --- .../PublishedCache/NuCache/ContentStore.cs | 60 +++++++------------ 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index ae439db579..c7fc389cb1 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -506,6 +506,14 @@ namespace Umbraco.Web.PublishedCache.NuCache public int Count => _contentNodes.Count; + /// + /// Get the most recent version of the LinkedNode stored in the dictionary for the supplied key + /// + /// + /// + /// + /// + /// private static LinkedNode GetHead(ConcurrentDictionary> dict, TKey key) where TValue : class { @@ -855,12 +863,13 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// The generation requested, use null to avoid the lookup /// - private LinkedNode GetLinkedNodeGen(LinkedNode link, long? gen) + private LinkedNode GetLinkedNodeGen(LinkedNode link, long? gen) + where TValue : class { if (!gen.HasValue) return link; - //find the correct snapshot - while (link != null && link.Gen != gen) + //find the correct snapshot, find the first that is <= the requested gen + while (link != null && link.Gen > gen) { link = link.Next; } @@ -1105,17 +1114,11 @@ namespace Umbraco.Web.PublishedCache.NuCache public IEnumerable GetAtRoot(long gen) { - var z = _root; - while (z != null) - { - if (z.Gen <= gen) - break; - z = z.Next; - } - if (z == null) + var root = GetLinkedNodeGen(_root, gen); + if (root == null) yield break; - var id = z.Value.FirstChildContentId; + var id = root.Value.FirstChildContentId; while (id > 0) { @@ -1130,13 +1133,8 @@ namespace Umbraco.Web.PublishedCache.NuCache { // look ma, no lock! var link = GetHead(dict, key); - while (link != null) - { - if (link.Gen <= gen) - return link.Value; // may be null - link = link.Next; - } - return null; + link = GetLinkedNodeGen(link, gen); + return link?.Value; // may be null } public IEnumerable GetAll(long gen) @@ -1146,17 +1144,9 @@ namespace Umbraco.Web.PublishedCache.NuCache var links = _contentNodes.Values.ToArray(); foreach (var l in links) { - var link = l; - while (link != null) - { - if (link.Gen <= gen) - { - if (link.Value != null) - yield return link.Value; - break; - } - link = link.Next; - } + var link = GetLinkedNodeGen(l, gen); + if (link?.Value != null) + yield return link.Value; } } @@ -1164,14 +1154,8 @@ namespace Umbraco.Web.PublishedCache.NuCache { var has = _contentNodes.Any(x => { - var link = x.Value; - while (link != null) - { - if (link.Gen <= gen && link.Value != null) - return true; - link = link.Next; - } - return false; + var link = GetLinkedNodeGen(x.Value, gen); + return link?.Value != null; }); return has == false; }