From aa86dfa9560fce3d77c73c78065beb114304d0c2 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 22 Feb 2019 19:13:25 +0100 Subject: [PATCH] Bugfix exception during install, live models collision --- .../Composing/ComponentCollectionBuilder.cs | 2 +- .../PublishedModelFactoryExtensions.cs | 33 +++++++++++++++++++ src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../PublishedContent/NuCacheTests.cs | 4 +-- .../Scoping/ScopedNuCacheTests.cs | 5 +-- .../ContentTypeServiceVariantsTests.cs | 5 +-- .../Cache/ContentTypeCacheRefresher.cs | 17 ++-------- .../Cache/DataTypeCacheRefresher.cs | 17 ++-------- .../NuCache/PublishedSnapshotService.cs | 14 +++++--- 9 files changed, 58 insertions(+), 40 deletions(-) create mode 100644 src/Umbraco.Core/PublishedModelFactoryExtensions.cs diff --git a/src/Umbraco.Core/Composing/ComponentCollectionBuilder.cs b/src/Umbraco.Core/Composing/ComponentCollectionBuilder.cs index 59a272125e90..47968d3568d9 100644 --- a/src/Umbraco.Core/Composing/ComponentCollectionBuilder.cs +++ b/src/Umbraco.Core/Composing/ComponentCollectionBuilder.cs @@ -30,7 +30,7 @@ protected override IEnumerable CreateItems(IFactory factory) protected override IComponent CreateItem(IFactory factory, Type itemType) { - using (_logger.DebugDuration($"Creating {itemType.FullName}.", $"Created {itemType.FullName}.", thresholdMilliseconds: LogThresholdMilliseconds)) + using (_logger.DebugDuration($"Creating {itemType.FullName}.", $"Created {itemType.FullName}.", thresholdMilliseconds: LogThresholdMilliseconds)) { return base.CreateItem(factory, itemType); } diff --git a/src/Umbraco.Core/PublishedModelFactoryExtensions.cs b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs new file mode 100644 index 000000000000..4e026490a486 --- /dev/null +++ b/src/Umbraco.Core/PublishedModelFactoryExtensions.cs @@ -0,0 +1,33 @@ +using System; +using Umbraco.Core.Models.PublishedContent; + +namespace Umbraco.Core +{ + /// + /// Provides extension methods for . + /// + public static class PublishedModelFactoryExtensions + { + /// + /// 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. + /// + public static void WithSafeLiveFactory(this IPublishedModelFactory factory, Action action) + { + if (factory is ILivePublishedModelFactory liveFactory) + { + lock (liveFactory.SyncRoot) + { + liveFactory.Refresh(); + action(); + } + } + else + { + action(); + } + } + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 6b4f9bd10b54..c0149ccadb92 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -212,6 +212,7 @@ + diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs index 703ae3f0e711..3142e1038e7b 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs @@ -180,8 +180,8 @@ private void Init() dataSource, globalSettings, new SiteDomainHelper(), - contentTypeServiceBaseFactory, - Mock.Of()); + Mock.Of(), + Mock.Of()); // invariant is the current default _variationAccesor.VariationContext = new VariationContext(); diff --git a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs index 33f6af86385c..9b1300ee2317 100644 --- a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs @@ -97,8 +97,9 @@ protected override IPublishedSnapshotService CreatePublishedSnapshotService() documentRepository, mediaRepository, memberRepository, DefaultCultureAccessor, new DatabaseDataSource(), - Factory.GetInstance(), new SiteDomainHelper(), contentTypeServiceBaseFactory, - Factory.GetInstance()); + Factory.GetInstance(), new SiteDomainHelper(), + Factory.GetInstance(), + Mock.Of()); } protected UmbracoContext GetUmbracoContextNu(string url, int templateId = 1234, RouteData routeData = null, bool setSingleton = false, IUmbracoSettingsSection umbracoSettings = null, IEnumerable urlProviders = null) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 94518b8dfd6f..b5bf9e61f87f 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -70,8 +70,9 @@ protected override IPublishedSnapshotService CreatePublishedSnapshotService() documentRepository, mediaRepository, memberRepository, DefaultCultureAccessor, new DatabaseDataSource(), - Factory.GetInstance(), new SiteDomainHelper(), contentTypeServiceBaseFactory, - Factory.GetInstance()); + Factory.GetInstance(), new SiteDomainHelper(), + Factory.GetInstance(), + Mock.Of()); } public class LocalServerMessenger : ServerMessengerBase diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index a7ebff911f29..493381a5a39c 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; @@ -82,20 +83,8 @@ public override void Refresh(JsonPayload[] payloads) // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - // ReSharper disable once SuspiciousTypeConversion.Global - if (_publishedModelFactory is ILivePublishedModelFactory live) - { - lock (live.SyncRoot) - { - live.Refresh(); - _publishedSnapshotService.Notify(payloads); - } - } - else - { - // ReSharper disable once InconsistentlySynchronizedField - _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 547b64968e91..1d41c0578bef 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -1,4 +1,5 @@ using System; +using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; @@ -65,20 +66,8 @@ public override void Refresh(JsonPayload[] payloads) // service of changes, else factories may try to rebuild models while // we are using the database to load content into caches - // ReSharper disable once SuspiciousTypeConversion.Global - if (_publishedModelFactory is ILivePublishedModelFactory live) - { - lock (live.SyncRoot) - { - live.Refresh(); - _publishedSnapshotService.Notify(payloads); - } - } - else - { - // ReSharper disable once InconsistentlySynchronizedField - _publishedSnapshotService.Notify(payloads); - } + _publishedModelFactory.WithSafeLiveFactory(() => + _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 1abb250f18c8..9ede3657de00 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -44,9 +44,9 @@ class PublishedSnapshotService : PublishedSnapshotServiceBase private readonly IMemberRepository _memberRepository; private readonly IGlobalSettings _globalSettings; private readonly ISiteDomainHelper _siteDomainHelper; - private readonly IContentTypeBaseServiceProvider _contentTypeBaseServiceProvider; private readonly IEntityXmlSerializer _entitySerializer; private readonly IDefaultCultureAccessor _defaultCultureAccessor; + private readonly IPublishedModelFactory _publishedModelFactory; // volatile because we read it with no lock private volatile bool _isReady; @@ -88,8 +88,8 @@ class PublishedSnapshotService : PublishedSnapshotServiceBase IUmbracoContextAccessor umbracoContextAccessor, ILogger logger, IScopeProvider scopeProvider, IDocumentRepository documentRepository, IMediaRepository mediaRepository, IMemberRepository memberRepository, IDefaultCultureAccessor defaultCultureAccessor, - IDataSource dataSource, IGlobalSettings globalSettings, ISiteDomainHelper siteDomainHelper, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, - IEntityXmlSerializer entitySerializer) + IDataSource dataSource, IGlobalSettings globalSettings, ISiteDomainHelper siteDomainHelper, + IEntityXmlSerializer entitySerializer, IPublishedModelFactory publishedModelFactory) : base(publishedSnapshotAccessor, variationContextAccessor) { //if (Interlocked.Increment(ref _singletonCheck) > 1) @@ -107,7 +107,7 @@ class PublishedSnapshotService : PublishedSnapshotServiceBase _defaultCultureAccessor = defaultCultureAccessor; _globalSettings = globalSettings; _siteDomainHelper = siteDomainHelper; - _contentTypeBaseServiceProvider = contentTypeBaseServiceProvider; + _publishedModelFactory = publishedModelFactory; // we need an Xml serializer here so that the member cache can support XPath, // for members this is done by navigating the serialized-to-xml member @@ -167,7 +167,7 @@ class PublishedSnapshotService : PublishedSnapshotServiceBase _domainStore = new SnapDictionary(); - LoadCaches(); + _publishedModelFactory.WithSafeLiveFactory(LoadCaches); Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? default; int GetId(ContentStore store, Guid uid) => store.LiveSnapshot.Get(uid)?.Id ?? default; @@ -571,6 +571,10 @@ private void LoadDomainsLocked() // because now we should ALWAYS run with the database server messenger, and then the RefreshAll will // be processed as soon as we are configured and the messenger processes instructions. + // note: notifications for content type and data type changes should be invoked with the + // pure live model factory, if any, locked and refreshed - see ContentTypeCacheRefresher and + // DataTypeCacheRefresher + public override void Notify(ContentCacheRefresher.JsonPayload[] payloads, out bool draftChanged, out bool publishedChanged) { // no cache, trash everything