From 9799c550f4c02f46049aa3a91455677c686a1dd4 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 29 Jan 2024 08:48:45 +0100 Subject: [PATCH 01/10] Create shadow file systems in configured LocalTempPath (#15637) --- src/Umbraco.Core/IO/ShadowWrapper.cs | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Core/IO/ShadowWrapper.cs b/src/Umbraco.Core/IO/ShadowWrapper.cs index 2f2e0a991c..ca82c67bd2 100644 --- a/src/Umbraco.Core/IO/ShadowWrapper.cs +++ b/src/Umbraco.Core/IO/ShadowWrapper.cs @@ -7,19 +7,21 @@ namespace Umbraco.Cms.Core.IO; internal class ShadowWrapper : IFileSystem, IFileProviderFactory { - private static readonly string ShadowFsPath = Constants.SystemDirectories.TempData.EnsureEndsWith('/') + "ShadowFs"; - private readonly IHostingEnvironment _hostingEnvironment; - private readonly IIOHelper _ioHelper; + private const string ShadowFsPath = "ShadowFs"; - private readonly Func? _isScoped; + private readonly IIOHelper _ioHelper; + private readonly IHostingEnvironment _hostingEnvironment; private readonly ILoggerFactory _loggerFactory; private readonly string _shadowPath; + private readonly Func? _isScoped; + private string? _shadowDir; private ShadowFileSystem? _shadowFileSystem; public ShadowWrapper(IFileSystem innerFileSystem, IIOHelper ioHelper, IHostingEnvironment hostingEnvironment, ILoggerFactory loggerFactory, string shadowPath, Func? isScoped = null) { InnerFileSystem = innerFileSystem; + _ioHelper = ioHelper ?? throw new ArgumentNullException(nameof(ioHelper)); _hostingEnvironment = hostingEnvironment ?? throw new ArgumentNullException(nameof(hostingEnvironment)); _loggerFactory = loggerFactory; @@ -35,18 +37,19 @@ internal class ShadowWrapper : IFileSystem, IFileProviderFactory { get { - if (_isScoped is not null && _shadowFileSystem is not null) + Func? isScoped = _isScoped; + if (isScoped is not null && _shadowFileSystem is not null) { - var isScoped = _isScoped!(); + bool? scoped = isScoped(); // if the filesystem is created *after* shadowing starts, it won't be shadowing - // better not ignore that situation and raised a meaningful (?) exception - if (isScoped.HasValue && isScoped.Value && _shadowFileSystem == null) + // better not ignore that situation and raise a meaningful (?) exception + if (scoped.HasValue && scoped.Value && _shadowFileSystem == null) { throw new Exception("The filesystems are shadowing, but this filesystem is not."); } - return isScoped.HasValue && isScoped.Value + return scoped.HasValue && scoped.Value ? _shadowFileSystem : InnerFileSystem; } @@ -56,8 +59,7 @@ internal class ShadowWrapper : IFileSystem, IFileProviderFactory } /// - public IFileProvider? Create() => - InnerFileSystem.TryCreateFileProvider(out IFileProvider? fileProvider) ? fileProvider : null; + public IFileProvider? Create() => InnerFileSystem.TryCreateFileProvider(out IFileProvider? fileProvider) ? fileProvider : null; public IEnumerable GetDirectories(string path) => FileSystem.GetDirectories(path); @@ -69,8 +71,7 @@ internal class ShadowWrapper : IFileSystem, IFileProviderFactory public void AddFile(string path, Stream stream) => FileSystem.AddFile(path, stream); - public void AddFile(string path, Stream stream, bool overrideExisting) => - FileSystem.AddFile(path, stream, overrideExisting); + public void AddFile(string path, Stream stream, bool overrideExisting) => FileSystem.AddFile(path, stream, overrideExisting); public IEnumerable GetFiles(string path) => FileSystem.GetFiles(path); @@ -107,8 +108,7 @@ internal class ShadowWrapper : IFileSystem, IFileProviderFactory { var id = GuidUtils.ToBase32String(Guid.NewGuid(), idLength); - var virt = ShadowFsPath + "/" + id; - var shadowDir = hostingEnvironment.MapPathContentRoot(virt); + var shadowDir = Path.Combine(hostingEnvironment.LocalTempPath, ShadowFsPath, id); if (Directory.Exists(shadowDir)) { continue; @@ -129,10 +129,10 @@ internal class ShadowWrapper : IFileSystem, IFileProviderFactory // note: no thread-safety here, because ShadowFs is thread-safe due to the check // on ShadowFileSystemsScope.None - and if None is false then we should be running // in a single thread anyways - var virt = Path.Combine(ShadowFsPath, id, _shadowPath); - _shadowDir = _hostingEnvironment.MapPathContentRoot(virt); + var rootUrl = Path.Combine(ShadowFsPath, id, _shadowPath); + _shadowDir = Path.Combine(_hostingEnvironment.LocalTempPath, rootUrl); Directory.CreateDirectory(_shadowDir); - var tempfs = new PhysicalFileSystem(_ioHelper, _hostingEnvironment, _loggerFactory.CreateLogger(), _shadowDir, _hostingEnvironment.ToAbsolute(virt)); + var tempfs = new PhysicalFileSystem(_ioHelper, _hostingEnvironment, _loggerFactory.CreateLogger(), _shadowDir, rootUrl); _shadowFileSystem = new ShadowFileSystem(InnerFileSystem, tempfs); } @@ -160,7 +160,7 @@ internal class ShadowWrapper : IFileSystem, IFileProviderFactory // shadowPath make be path/to/dir, remove each dir = dir!.Replace('/', Path.DirectorySeparatorChar); - var min = _hostingEnvironment.MapPathContentRoot(ShadowFsPath).Length; + var min = Path.Combine(_hostingEnvironment.LocalTempPath, ShadowFsPath).Length; var pos = dir.LastIndexOf(Path.DirectorySeparatorChar); while (pos > min) { From 860afb35f2b41482bd78010192722703cbdb4bfa Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 30 Jan 2024 15:53:21 +0100 Subject: [PATCH 02/10] Bump version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index e22162ce3c..c04379412a 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "10.8.3", + "version": "10.8.4", "assemblyVersion": { "precision": "build" }, From bc1ddaed8cf2527f9af917c1b611520540c36902 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Wed, 10 Jan 2024 12:22:36 +0100 Subject: [PATCH 03/10] Run the same cleanup with scaffolding content as when copying. (#15541) * Run the same cleanup with scaffolding content as when copying. - Added a new ContentScaffoldedNotification - Published the notification when a new scaffold has been created from a blueprint (content template) - Linked up the ComplextPEContent handler to do the same cleanup for the new notification as when copying. - registered handlers to the event for blocklist, blockgrid and nested content * PR pattern matching suggestion Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Sven Geusens Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> (cherry picked from commit dff90c6ec07d986f4b11b1f0e4c1f203b65d604d) --- .../ContentScaffoldedNotification.cs | 18 +++++++++++ .../Notifications/ScaffoldedNotification.cs | 23 ++++++++++++++ .../UmbracoBuilder.CoreServices.cs | 3 ++ ...ropertyEditorContentNotificationHandler.cs | 15 ++++++++- .../Controllers/ContentController.cs | 31 ++++++++++++++----- 5 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs create mode 100644 src/Umbraco.Core/Notifications/ScaffoldedNotification.cs diff --git a/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs new file mode 100644 index 0000000000..47eda5468d --- /dev/null +++ b/src/Umbraco.Core/Notifications/ContentScaffoldedNotification.cs @@ -0,0 +1,18 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Notifications; + +/// +/// Notification that is send out when a Content item has been scaffolded from an original item and basic cleaning has been performed +/// +public sealed class ContentScaffoldedNotification : ScaffoldedNotification +{ + public ContentScaffoldedNotification(IContent original, IContent scaffold, int parentId, EventMessages messages) + : base(original, scaffold, parentId, messages) + { + } +} diff --git a/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs new file mode 100644 index 0000000000..f64bfd3933 --- /dev/null +++ b/src/Umbraco.Core/Notifications/ScaffoldedNotification.cs @@ -0,0 +1,23 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; + +namespace Umbraco.Cms.Core.Notifications; + +public abstract class ScaffoldedNotification : CancelableObjectNotification + where T : class +{ + protected ScaffoldedNotification(T original, T scaffold, int parentId, EventMessages messages) + : base(original, messages) + { + Scaffold = scaffold; + ParentId = parentId; + } + + public T Original => Target; + + public T Scaffold { get; } + + public int ParentId { get; } +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index c65e50024c..bd6f2e9b38 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -358,10 +358,13 @@ public static partial class UmbracoBuilderExtensions builder .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() + .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs index 2b4ac75042..ce867d29e4 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ComplexPropertyEditorContentNotificationHandler.cs @@ -10,9 +10,16 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; +/// +/// Handles nested Udi keys when +/// - saving: Empty keys get generated +/// - copy: keys get replaced by new ones while keeping references intact +/// - scaffolding: keys get replaced by new ones while keeping references intact +/// public abstract class ComplexPropertyEditorContentNotificationHandler : INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler { protected abstract string EditorAlias { get; } @@ -31,6 +38,12 @@ public abstract class ComplexPropertyEditorContentNotificationHandler : } } + public void Handle(ContentScaffoldedNotification notification) + { + IEnumerable props = notification.Scaffold.GetPropertiesByEditor(EditorAlias); + UpdatePropertyValues(props, false); + } + protected abstract string FormatPropertyValue(string rawJson, bool onlyMissingKeys); private void UpdatePropertyValues(IEnumerable props, bool onlyMissingKeys) diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 8beec57812..0a1a82b3eb 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -16,6 +16,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Models.Validation; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Routing; @@ -623,17 +624,33 @@ public class ContentController : ContentControllerBase [OutgoingEditorModelEvent] public ActionResult GetEmptyBlueprint(int blueprintId, int parentId) { - IContent? blueprint = _contentService.GetBlueprintById(blueprintId); - if (blueprint == null) + IContent? scaffold; + using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { - return NotFound(); + IContent? blueprint = _contentService.GetBlueprintById(blueprintId); + if (blueprint is null) + { + return NotFound(); + } + scaffold = (IContent)blueprint.DeepClone(); + + scaffold.Id = 0; + scaffold.Name = string.Empty; + scaffold.ParentId = parentId; + + var scaffoldedNotification = new ContentScaffoldedNotification(blueprint, scaffold, parentId, new EventMessages()); + if (scope.Notifications.PublishCancelable(scaffoldedNotification)) + { + scope.Complete(); + return Problem("Scaffolding was cancelled"); + } + + scope.Complete(); } - blueprint.Id = 0; - blueprint.Name = string.Empty; - blueprint.ParentId = parentId; - ContentItemDisplay? mapped = _umbracoMapper.Map(blueprint); + + ContentItemDisplay? mapped = _umbracoMapper.Map(scaffold); if (mapped is not null) { From 7a04baf9efd06e242f95ab0cb33f45265f72c5ac Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> Date: Thu, 11 Jan 2024 12:46:31 +0100 Subject: [PATCH 04/10] V10: Pass in variation context to published cache (#15563) * Make sure that we always have variation context * Fix references (cherry picked from commit 57b3a196bf920e380b274746fbb4c17f10a55b30) --- .../PublishedCache/PublishedCacheBase.cs | 22 ++++++++++++++++--- .../ContentCache.cs | 2 +- .../MediaCache.cs | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs b/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs index 3e961ce434..ffb2ef4278 100644 --- a/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs +++ b/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs @@ -1,6 +1,8 @@ using System.Xml.XPath; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Xml; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Core.PublishedCache; @@ -9,10 +11,24 @@ public abstract class PublishedCacheBase : IPublishedCache { private readonly IVariationContextAccessor? _variationContextAccessor; - public PublishedCacheBase(IVariationContextAccessor variationContextAccessor) => _variationContextAccessor = - variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor)); - protected PublishedCacheBase(bool previewDefault) => PreviewDefault = previewDefault; + [Obsolete("Use ctor with all parameters. This will be removed in V15")] + public PublishedCacheBase(IVariationContextAccessor variationContextAccessor) + : this(variationContextAccessor, false) + { + } + + [Obsolete("Use ctor with all parameters. This will be removed in V15")] + protected PublishedCacheBase(bool previewDefault) + : this(StaticServiceProvider.Instance.GetRequiredService(), previewDefault) + { + } + + public PublishedCacheBase(IVariationContextAccessor variationContextAccessor, bool previewDefault) + { + _variationContextAccessor = variationContextAccessor; + PreviewDefault = previewDefault; + } public bool PreviewDefault { get; } diff --git a/src/Umbraco.PublishedCache.NuCache/ContentCache.cs b/src/Umbraco.PublishedCache.NuCache/ContentCache.cs index d8a5c0bc04..1686f6aacf 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentCache.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentCache.cs @@ -36,7 +36,7 @@ public class ContentCache : PublishedCacheBase, IPublishedContentCache, INavigab IDomainCache domainCache, IOptions globalSettings, IVariationContextAccessor variationContextAccessor) - : base(previewDefault) + : base(variationContextAccessor, previewDefault) { _snapshot = snapshot; _snapshotCache = snapshotCache; diff --git a/src/Umbraco.PublishedCache.NuCache/MediaCache.cs b/src/Umbraco.PublishedCache.NuCache/MediaCache.cs index 626e2fe36c..68c23c5a34 100644 --- a/src/Umbraco.PublishedCache.NuCache/MediaCache.cs +++ b/src/Umbraco.PublishedCache.NuCache/MediaCache.cs @@ -17,7 +17,7 @@ public class MediaCache : PublishedCacheBase, IPublishedMediaCache, INavigableDa #region Constructors public MediaCache(bool previewDefault, ContentStore.Snapshot snapshot, IVariationContextAccessor variationContextAccessor) - : base(previewDefault) + : base(variationContextAccessor, previewDefault) { _snapshot = snapshot; _variationContextAccessor = variationContextAccessor; From b18b6cc5e7d06747e0d08df74053062ff414dec1 Mon Sep 17 00:00:00 2001 From: Aleksander Date: Thu, 4 Aug 2022 14:29:20 +0200 Subject: [PATCH 05/10] Pass cache level to properties when creating published content in nucache (cherry picked from commit d9d2b66e8580bc0cbdd42739a92cf9df16b4e96e) # Conflicts: # src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs (cherry picked from commit 040495f359f8577197c6281dd0afb84b9e7debdc) --- src/Umbraco.PublishedCache.NuCache/PublishedContent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedContent.cs b/src/Umbraco.PublishedCache.NuCache/PublishedContent.cs index 3544ab35bc..f84df0644d 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedContent.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedContent.cs @@ -56,7 +56,7 @@ internal class PublishedContent : PublishedContentBase // add one property per property type - this is required, for the indexing to work // if contentData supplies pdatas, use them, else use null contentData.Properties.TryGetValue(propertyType.Alias, out PropertyData[]? pdatas); // else will be null - properties[i++] = new Property(propertyType, this, pdatas, _publishedSnapshotAccessor); + properties[i++] = new Property(propertyType, this, pdatas, _publishedSnapshotAccessor, propertyType.CacheLevel); } PropertiesArray = properties; From 3e28e10cdfcf0bd25a0504eb29a35c8996cab13e Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 1 Feb 2024 09:55:09 +0100 Subject: [PATCH 06/10] Skip cache refresher operations for content blueprints (#15633) * Skip cache refresher operations for content blueprints * Fix JsonPayload deserialization error by adding a default constructor and property initializers * Obsolete JsonPayload constructor and update usages --- .../Cache/ContentCacheRefresher.cs | 23 ++- .../Cache/LanguageCacheRefresher.cs | 10 +- .../Cache/DistributedCacheExtensions.cs | 19 +- .../IndexingNotificationHandler.Content.cs | 12 +- .../PublishedSnapshotService.cs | 17 +- .../Umbraco.Core/Cache/RefresherTests.cs | 25 ++- ...PublishedSnapshotServiceCollectionTests.cs | 182 ++++++++++++++---- 7 files changed, 232 insertions(+), 56 deletions(-) diff --git a/src/Umbraco.Core/Cache/ContentCacheRefresher.cs b/src/Umbraco.Core/Cache/ContentCacheRefresher.cs index a515d5c5d1..779b22fe68 100644 --- a/src/Umbraco.Core/Cache/ContentCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/ContentCacheRefresher.cs @@ -84,8 +84,8 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase((k, v) => v.Path?.Contains(pathid) ?? false); } - // if the item is being completely removed, we need to refresh the domains cache if any domain was assigned to the content - if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.Remove)) + // if the item is not a blueprint and is being completely removed, we need to refresh the domains cache if any domain was assigned to the content + if (payload.Blueprint is false && payload.ChangeTypes.HasTypesAny(TreeChangeTypes.Remove)) { idsRemoved.Add(payload.Id); } @@ -120,7 +120,11 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase x.Blueprint is false)) + { + // Only notify if the payload contains actual (non-blueprint) contents + NotifyPublishedSnapshotService(_publishedSnapshotService, AppCaches, payloads); + } base.Refresh(payloads); } @@ -157,8 +161,13 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase payloads = changes - .Select(x => new ContentCacheRefresher.JsonPayload(x.Item.Id, x.Item.Key, x.ChangeTypes)); + IEnumerable payloads = changes.Select(x => new ContentCacheRefresher.JsonPayload() + { + Id = x.Item.Id, + Key = x.Item.Key, + ChangeTypes = x.ChangeTypes, + Blueprint = x.Item.Blueprint + }); dc.RefreshByPayload(ContentCacheRefresher.UniqueId, payloads); } diff --git a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs index 66eb61ce3c..f06384ad27 100644 --- a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs +++ b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs @@ -51,13 +51,15 @@ public sealed class ContentIndexingNotificationHandler : INotificationHandler(); - } - + deleteBatch ??= new HashSet(); deleteBatch.Add(payload.Id); } else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs index 42e17b603a..b7efaab17e 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs @@ -210,7 +210,16 @@ internal class PublishedSnapshotService : IPublishedSnapshotService // they require. using (_contentStore.GetScopedWriteLock(_scopeProvider)) { - NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, null, TreeChangeTypes.RefreshAll) }, out _, out _); + NotifyLocked( + new[] + { + new ContentCacheRefresher.JsonPayload() + { + ChangeTypes = TreeChangeTypes.RefreshAll + } + }, + out _, + out _); } using (_mediaStore.GetScopedWriteLock(_scopeProvider)) @@ -852,6 +861,12 @@ internal class PublishedSnapshotService : IPublishedSnapshotService // contentStore is write-locked during changes - see note above, calls to this method are wrapped in contentStore.GetScopedWriteLock foreach (ContentCacheRefresher.JsonPayload payload in payloads) { + if (payload.Blueprint) + { + // Skip blueprints + continue; + } + _logger.LogDebug("Notified {ChangeTypes} for content {ContentId}", payload.ChangeTypes, payload.Id); if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs index 6fcd37adb7..e509742cb9 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; using Newtonsoft.Json; using NUnit.Framework; using Umbraco.Cms.Core.Cache; @@ -19,8 +18,10 @@ public class RefresherTests { new MediaCacheRefresher.JsonPayload(1234, Guid.NewGuid(), TreeChangeTypes.None), }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].Key, payload[0].Key); Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); @@ -31,13 +32,21 @@ public class RefresherTests { ContentCacheRefresher.JsonPayload[] source = { - new ContentCacheRefresher.JsonPayload(1234, Guid.NewGuid(), TreeChangeTypes.None), + new ContentCacheRefresher.JsonPayload() + { + Id = 1234, + Key = Guid.NewGuid(), + ChangeTypes = TreeChangeTypes.None + } }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].Key, payload[0].Key); Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); + Assert.AreEqual(source[0].Blueprint, payload[0].Blueprint); } [Test] @@ -47,8 +56,10 @@ public class RefresherTests { new ContentTypeCacheRefresher.JsonPayload("xxx", 1234, ContentTypeChangeTypes.None), }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].ItemType, payload[0].ItemType); Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); @@ -61,8 +72,10 @@ public class RefresherTests { new DataTypeCacheRefresher.JsonPayload(1234, Guid.NewGuid(), true), }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].Key, payload[0].Key); Assert.AreEqual(source[0].Removed, payload[0].Removed); @@ -71,10 +84,14 @@ public class RefresherTests [Test] public void DomainCacheRefresherCanDeserializeJsonPayload() { - DomainCacheRefresher.JsonPayload[] - source = { new DomainCacheRefresher.JsonPayload(1234, DomainChangeTypes.None) }; + DomainCacheRefresher.JsonPayload[] source = + { + new DomainCacheRefresher.JsonPayload(1234, DomainChangeTypes.None) + }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].ChangeType, payload[0].ChangeType); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs index a0e48069a3..6d37be596a 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs @@ -1,6 +1,3 @@ -using System; -using System.Collections.Generic; -using System.Linq; using NUnit.Framework; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; @@ -359,7 +356,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(10, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 10, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _); // changes that *I* make are immediately visible on the current snapshot var documents = snapshot.Content.GetAtRoot().ToArray(); @@ -393,7 +399,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _); // changes that *I* make are immediately visible on the current snapshot var documents = snapshot.Content.GetAtRoot().ToArray(); @@ -451,7 +466,11 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] { - new ContentCacheRefresher.JsonPayload(kit.Node.ParentContentId, Guid.Empty, TreeChangeTypes.RefreshBranch), + new ContentCacheRefresher.JsonPayload() + { + Id = kit.Node.ParentContentId, + ChangeTypes = TreeChangeTypes.RefreshBranch + } }, out _, out _); @@ -517,11 +536,19 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( new[] - { - // removal must come first - new ContentCacheRefresher.JsonPayload(2, Guid.Empty, TreeChangeTypes.RefreshBranch), - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshBranch), - }, + { + // removal must come first + new ContentCacheRefresher.JsonPayload() + { + Id = 2, + ChangeTypes = TreeChangeTypes.RefreshBranch + }, + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, out _, out _); @@ -572,7 +599,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify - which ensures there are 2 generations in the cache meaning each LinkedNode has a Next value. SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(4, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 4, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _); // refresh the branch again, this used to show the issue where a null ref exception would occur // because in the ClearBranchLocked logic, when SetValueLocked was called within a recursive call @@ -580,7 +616,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // this value before recursing. Assert.DoesNotThrow(() => SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(4, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _)); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 4, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _)); } [Test] @@ -760,11 +805,23 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(3, Guid.Empty, TreeChangeTypes.Remove), // remove last - new ContentCacheRefresher.JsonPayload(5, Guid.Empty, TreeChangeTypes.Remove), // remove middle - new ContentCacheRefresher.JsonPayload(9, Guid.Empty, TreeChangeTypes.Remove), // remove first - }, + { + new ContentCacheRefresher.JsonPayload() // remove last + { + Id = 3, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove middle + { + Id = 5, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove first + { + Id = 9, + ChangeTypes = TreeChangeTypes.Remove + } + }, out _, out _); @@ -780,11 +837,23 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.Remove), // remove first - new ContentCacheRefresher.JsonPayload(8, Guid.Empty, TreeChangeTypes.Remove), // remove - new ContentCacheRefresher.JsonPayload(7, Guid.Empty, TreeChangeTypes.Remove), // remove - }, + { + new ContentCacheRefresher.JsonPayload() // remove first + { + Id = 1, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove + { + Id = 8, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove + { + Id = 7, + ChangeTypes = TreeChangeTypes.Remove + } + }, out _, out _); @@ -824,8 +893,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] { - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshBranch), - new ContentCacheRefresher.JsonPayload(2, Guid.Empty, TreeChangeTypes.RefreshNode), + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshBranch + }, + new ContentCacheRefresher.JsonPayload() + { + Id = 2, + ChangeTypes = TreeChangeTypes.RefreshNode + } }, out _, out _); @@ -888,7 +965,17 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT var parentNode = parentNodes[0]; AssertLinkedNode(parentNode.contentNode, -1, -1, -1, 2, 2); - SnapshotService.Notify(new[] { new ContentCacheRefresher.JsonPayload(2, Guid.Empty, TreeChangeTypes.Remove) }, out _, out _); + SnapshotService.Notify( + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 2, + ChangeTypes = TreeChangeTypes.Remove + } + }, + out _, + out _); parentNodes = contentStore.Test.GetValues(1); parentNode = parentNodes[0]; @@ -945,9 +1032,13 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(3, Guid.Empty, TreeChangeTypes.Remove), // remove middle child - }, + { + new ContentCacheRefresher.JsonPayload() // remove middle child + { + Id = 3, + ChangeTypes = TreeChangeTypes.Remove + } + }, out _, out _); @@ -1014,7 +1105,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT Assert.IsFalse(contentStore.Test.NextGen); SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshNode) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshNode + } + }, + out _, + out _); Assert.AreEqual(2, contentStore.Test.LiveGen); Assert.IsTrue(contentStore.Test.NextGen); @@ -1085,7 +1185,17 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT published ? rootKit.PublishedData : null); NuCacheContentService.ContentKits[1] = kit; - SnapshotService.Notify(new[] { new ContentCacheRefresher.JsonPayload(1, Guid.Empty, changeType) }, out _, out _); + SnapshotService.Notify( + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = changeType + } + }, + out _, + out _); Assert.AreEqual(assertGen, contentStore.Test.LiveGen); Assert.IsTrue(contentStore.Test.NextGen); @@ -1163,9 +1273,13 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(3, Guid.Empty, TreeChangeTypes.RefreshBranch), // remove middle child - }, + { + new ContentCacheRefresher.JsonPayload() // remove middle child + { + Id = 3, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, out _, out _); From 118ac8e230b6252ffa5f0d236220117de8f593ca Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 1 Feb 2024 09:58:42 +0100 Subject: [PATCH 07/10] V10+ version of https://github.com/umbraco/Umbraco-CMS/pull/15638 (#15664) --- .../Filters/ContentSaveValidationAttribute.cs | 2 +- src/Umbraco.Web.BackOffice/ModelBinders/BlueprintItemBinder.cs | 2 +- src/Umbraco.Web.BackOffice/ModelBinders/ContentItemBinder.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs index 33017ada88..09ccd0fb1b 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs @@ -18,7 +18,7 @@ namespace Umbraco.Cms.Web.BackOffice.Filters; /// Validates the incoming model along with if the user is allowed to perform the /// operation /// -internal sealed class ContentSaveValidationAttribute : TypeFilterAttribute +public sealed class ContentSaveValidationAttribute : TypeFilterAttribute { public ContentSaveValidationAttribute(bool skipUserAccessValidation = false) : base(typeof(ContentSaveValidationFilter)) diff --git a/src/Umbraco.Web.BackOffice/ModelBinders/BlueprintItemBinder.cs b/src/Umbraco.Web.BackOffice/ModelBinders/BlueprintItemBinder.cs index bc07497fcd..bf8c7372bc 100644 --- a/src/Umbraco.Web.BackOffice/ModelBinders/BlueprintItemBinder.cs +++ b/src/Umbraco.Web.BackOffice/ModelBinders/BlueprintItemBinder.cs @@ -7,7 +7,7 @@ using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Web.BackOffice.ModelBinders; -internal class BlueprintItemBinder : ContentItemBinder +public class BlueprintItemBinder : ContentItemBinder { private readonly IContentService _contentService; diff --git a/src/Umbraco.Web.BackOffice/ModelBinders/ContentItemBinder.cs b/src/Umbraco.Web.BackOffice/ModelBinders/ContentItemBinder.cs index 0842ca2051..c73a45f904 100644 --- a/src/Umbraco.Web.BackOffice/ModelBinders/ContentItemBinder.cs +++ b/src/Umbraco.Web.BackOffice/ModelBinders/ContentItemBinder.cs @@ -13,7 +13,7 @@ namespace Umbraco.Cms.Web.BackOffice.ModelBinders; /// /// The model binder for /// -internal class ContentItemBinder : IModelBinder +public class ContentItemBinder : IModelBinder { private readonly IContentService _contentService; private readonly IContentTypeService _contentTypeService; From e37cf30690539e6a981caece32c44f4416149afd Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 1 Feb 2024 09:55:09 +0100 Subject: [PATCH 08/10] Skip cache refresher operations for content blueprints (#15633) * Skip cache refresher operations for content blueprints * Fix JsonPayload deserialization error by adding a default constructor and property initializers * Obsolete JsonPayload constructor and update usages (cherry picked from commit 3e28e10cdfcf0bd25a0504eb29a35c8996cab13e) --- .../Cache/ContentCacheRefresher.cs | 23 ++- .../Cache/LanguageCacheRefresher.cs | 10 +- .../Cache/DistributedCacheExtensions.cs | 19 +- .../IndexingNotificationHandler.Content.cs | 12 +- .../PublishedSnapshotService.cs | 17 +- .../Umbraco.Core/Cache/RefresherTests.cs | 25 ++- ...PublishedSnapshotServiceCollectionTests.cs | 182 ++++++++++++++---- 7 files changed, 232 insertions(+), 56 deletions(-) diff --git a/src/Umbraco.Core/Cache/ContentCacheRefresher.cs b/src/Umbraco.Core/Cache/ContentCacheRefresher.cs index a515d5c5d1..779b22fe68 100644 --- a/src/Umbraco.Core/Cache/ContentCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/ContentCacheRefresher.cs @@ -84,8 +84,8 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase((k, v) => v.Path?.Contains(pathid) ?? false); } - // if the item is being completely removed, we need to refresh the domains cache if any domain was assigned to the content - if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.Remove)) + // if the item is not a blueprint and is being completely removed, we need to refresh the domains cache if any domain was assigned to the content + if (payload.Blueprint is false && payload.ChangeTypes.HasTypesAny(TreeChangeTypes.Remove)) { idsRemoved.Add(payload.Id); } @@ -120,7 +120,11 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase x.Blueprint is false)) + { + // Only notify if the payload contains actual (non-blueprint) contents + NotifyPublishedSnapshotService(_publishedSnapshotService, AppCaches, payloads); + } base.Refresh(payloads); } @@ -157,8 +161,13 @@ public sealed class ContentCacheRefresher : PayloadCacheRefresherBase payloads = changes - .Select(x => new ContentCacheRefresher.JsonPayload(x.Item.Id, x.Item.Key, x.ChangeTypes)); + IEnumerable payloads = changes.Select(x => new ContentCacheRefresher.JsonPayload() + { + Id = x.Item.Id, + Key = x.Item.Key, + ChangeTypes = x.ChangeTypes, + Blueprint = x.Item.Blueprint + }); dc.RefreshByPayload(ContentCacheRefresher.UniqueId, payloads); } diff --git a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs index 66eb61ce3c..f06384ad27 100644 --- a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs +++ b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs @@ -51,13 +51,15 @@ public sealed class ContentIndexingNotificationHandler : INotificationHandler(); - } - + deleteBatch ??= new HashSet(); deleteBatch.Add(payload.Id); } else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs index 42e17b603a..b7efaab17e 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs @@ -210,7 +210,16 @@ internal class PublishedSnapshotService : IPublishedSnapshotService // they require. using (_contentStore.GetScopedWriteLock(_scopeProvider)) { - NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, null, TreeChangeTypes.RefreshAll) }, out _, out _); + NotifyLocked( + new[] + { + new ContentCacheRefresher.JsonPayload() + { + ChangeTypes = TreeChangeTypes.RefreshAll + } + }, + out _, + out _); } using (_mediaStore.GetScopedWriteLock(_scopeProvider)) @@ -852,6 +861,12 @@ internal class PublishedSnapshotService : IPublishedSnapshotService // contentStore is write-locked during changes - see note above, calls to this method are wrapped in contentStore.GetScopedWriteLock foreach (ContentCacheRefresher.JsonPayload payload in payloads) { + if (payload.Blueprint) + { + // Skip blueprints + continue; + } + _logger.LogDebug("Notified {ChangeTypes} for content {ContentId}", payload.ChangeTypes, payload.Id); if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs index 6fcd37adb7..e509742cb9 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/RefresherTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; using Newtonsoft.Json; using NUnit.Framework; using Umbraco.Cms.Core.Cache; @@ -19,8 +18,10 @@ public class RefresherTests { new MediaCacheRefresher.JsonPayload(1234, Guid.NewGuid(), TreeChangeTypes.None), }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].Key, payload[0].Key); Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); @@ -31,13 +32,21 @@ public class RefresherTests { ContentCacheRefresher.JsonPayload[] source = { - new ContentCacheRefresher.JsonPayload(1234, Guid.NewGuid(), TreeChangeTypes.None), + new ContentCacheRefresher.JsonPayload() + { + Id = 1234, + Key = Guid.NewGuid(), + ChangeTypes = TreeChangeTypes.None + } }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].Key, payload[0].Key); Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); + Assert.AreEqual(source[0].Blueprint, payload[0].Blueprint); } [Test] @@ -47,8 +56,10 @@ public class RefresherTests { new ContentTypeCacheRefresher.JsonPayload("xxx", 1234, ContentTypeChangeTypes.None), }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].ItemType, payload[0].ItemType); Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].ChangeTypes, payload[0].ChangeTypes); @@ -61,8 +72,10 @@ public class RefresherTests { new DataTypeCacheRefresher.JsonPayload(1234, Guid.NewGuid(), true), }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].Key, payload[0].Key); Assert.AreEqual(source[0].Removed, payload[0].Removed); @@ -71,10 +84,14 @@ public class RefresherTests [Test] public void DomainCacheRefresherCanDeserializeJsonPayload() { - DomainCacheRefresher.JsonPayload[] - source = { new DomainCacheRefresher.JsonPayload(1234, DomainChangeTypes.None) }; + DomainCacheRefresher.JsonPayload[] source = + { + new DomainCacheRefresher.JsonPayload(1234, DomainChangeTypes.None) + }; + var json = JsonConvert.SerializeObject(source); var payload = JsonConvert.DeserializeObject(json); + Assert.AreEqual(source[0].Id, payload[0].Id); Assert.AreEqual(source[0].ChangeType, payload[0].ChangeType); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs index a0e48069a3..6d37be596a 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedCache/PublishedSnapshotServiceCollectionTests.cs @@ -1,6 +1,3 @@ -using System; -using System.Collections.Generic; -using System.Linq; using NUnit.Framework; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; @@ -359,7 +356,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(10, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 10, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _); // changes that *I* make are immediately visible on the current snapshot var documents = snapshot.Content.GetAtRoot().ToArray(); @@ -393,7 +399,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _); // changes that *I* make are immediately visible on the current snapshot var documents = snapshot.Content.GetAtRoot().ToArray(); @@ -451,7 +466,11 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] { - new ContentCacheRefresher.JsonPayload(kit.Node.ParentContentId, Guid.Empty, TreeChangeTypes.RefreshBranch), + new ContentCacheRefresher.JsonPayload() + { + Id = kit.Node.ParentContentId, + ChangeTypes = TreeChangeTypes.RefreshBranch + } }, out _, out _); @@ -517,11 +536,19 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( new[] - { - // removal must come first - new ContentCacheRefresher.JsonPayload(2, Guid.Empty, TreeChangeTypes.RefreshBranch), - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshBranch), - }, + { + // removal must come first + new ContentCacheRefresher.JsonPayload() + { + Id = 2, + ChangeTypes = TreeChangeTypes.RefreshBranch + }, + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, out _, out _); @@ -572,7 +599,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify - which ensures there are 2 generations in the cache meaning each LinkedNode has a Next value. SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(4, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 4, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _); // refresh the branch again, this used to show the issue where a null ref exception would occur // because in the ClearBranchLocked logic, when SetValueLocked was called within a recursive call @@ -580,7 +616,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // this value before recursing. Assert.DoesNotThrow(() => SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(4, Guid.Empty, TreeChangeTypes.RefreshBranch) }, out _, out _)); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 4, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, + out _, + out _)); } [Test] @@ -760,11 +805,23 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(3, Guid.Empty, TreeChangeTypes.Remove), // remove last - new ContentCacheRefresher.JsonPayload(5, Guid.Empty, TreeChangeTypes.Remove), // remove middle - new ContentCacheRefresher.JsonPayload(9, Guid.Empty, TreeChangeTypes.Remove), // remove first - }, + { + new ContentCacheRefresher.JsonPayload() // remove last + { + Id = 3, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove middle + { + Id = 5, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove first + { + Id = 9, + ChangeTypes = TreeChangeTypes.Remove + } + }, out _, out _); @@ -780,11 +837,23 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT // notify SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.Remove), // remove first - new ContentCacheRefresher.JsonPayload(8, Guid.Empty, TreeChangeTypes.Remove), // remove - new ContentCacheRefresher.JsonPayload(7, Guid.Empty, TreeChangeTypes.Remove), // remove - }, + { + new ContentCacheRefresher.JsonPayload() // remove first + { + Id = 1, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove + { + Id = 8, + ChangeTypes = TreeChangeTypes.Remove + }, + new ContentCacheRefresher.JsonPayload() // remove + { + Id = 7, + ChangeTypes = TreeChangeTypes.Remove + } + }, out _, out _); @@ -824,8 +893,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] { - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshBranch), - new ContentCacheRefresher.JsonPayload(2, Guid.Empty, TreeChangeTypes.RefreshNode), + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshBranch + }, + new ContentCacheRefresher.JsonPayload() + { + Id = 2, + ChangeTypes = TreeChangeTypes.RefreshNode + } }, out _, out _); @@ -888,7 +965,17 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT var parentNode = parentNodes[0]; AssertLinkedNode(parentNode.contentNode, -1, -1, -1, 2, 2); - SnapshotService.Notify(new[] { new ContentCacheRefresher.JsonPayload(2, Guid.Empty, TreeChangeTypes.Remove) }, out _, out _); + SnapshotService.Notify( + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 2, + ChangeTypes = TreeChangeTypes.Remove + } + }, + out _, + out _); parentNodes = contentStore.Test.GetValues(1); parentNode = parentNodes[0]; @@ -945,9 +1032,13 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(3, Guid.Empty, TreeChangeTypes.Remove), // remove middle child - }, + { + new ContentCacheRefresher.JsonPayload() // remove middle child + { + Id = 3, + ChangeTypes = TreeChangeTypes.Remove + } + }, out _, out _); @@ -1014,7 +1105,16 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT Assert.IsFalse(contentStore.Test.NextGen); SnapshotService.Notify( - new[] { new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshNode) }, out _, out _); + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = TreeChangeTypes.RefreshNode + } + }, + out _, + out _); Assert.AreEqual(2, contentStore.Test.LiveGen); Assert.IsTrue(contentStore.Test.NextGen); @@ -1085,7 +1185,17 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT published ? rootKit.PublishedData : null); NuCacheContentService.ContentKits[1] = kit; - SnapshotService.Notify(new[] { new ContentCacheRefresher.JsonPayload(1, Guid.Empty, changeType) }, out _, out _); + SnapshotService.Notify( + new[] + { + new ContentCacheRefresher.JsonPayload() + { + Id = 1, + ChangeTypes = changeType + } + }, + out _, + out _); Assert.AreEqual(assertGen, contentStore.Test.LiveGen); Assert.IsTrue(contentStore.Test.NextGen); @@ -1163,9 +1273,13 @@ public class PublishedSnapshotServiceCollectionTests : PublishedSnapshotServiceT SnapshotService.Notify( new[] - { - new ContentCacheRefresher.JsonPayload(3, Guid.Empty, TreeChangeTypes.RefreshBranch), // remove middle child - }, + { + new ContentCacheRefresher.JsonPayload() // remove middle child + { + Id = 3, + ChangeTypes = TreeChangeTypes.RefreshBranch + } + }, out _, out _); From cbf9f9bcd199d7ca0412be3071d275556f10b7ba Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Tue, 6 Feb 2024 09:53:40 +0100 Subject: [PATCH 09/10] Merge pull request from GHSA-gvpc-3pj6-4m9w * Add MarkDownPropertyValueEditor with html sanitizer * Implement IMarkdownSanitizer. --- .../DependencyInjection/UmbracoBuilder.cs | 3 +- .../MarkDownPropertyValueEditor.cs | 39 +++++++++++++++++++ .../PropertyEditors/MarkdownPropertyEditor.cs | 8 ++++ .../Security/IMarkdownSanitizer.cs | 14 +++++++ .../Security/NoopMarkdownSanitizer.cs | 8 ++++ 5 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs create mode 100644 src/Umbraco.Core/Security/IMarkdownSanitizer.cs create mode 100644 src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index dcf1ee5183..5df7157830 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -318,8 +318,9 @@ namespace Umbraco.Cms.Core.DependencyInjection Services.AddSingleton(); Services.AddSingleton(); - // Register a noop IHtmlSanitizer to be replaced + // Register a noop IHtmlSanitizer & IMarkdownSanitizer to be replaced Services.AddUnique(); + Services.AddUnique(); Services.AddUnique(); Services.AddUnique(); diff --git a/src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs b/src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs new file mode 100644 index 0000000000..637a971d68 --- /dev/null +++ b/src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs @@ -0,0 +1,39 @@ +using Umbraco.Cms.Core.IO; +using Umbraco.Cms.Core.Models.Editors; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.PropertyEditors; + +/// +/// A custom value editor to ensure that macro syntax is parsed when being persisted and formatted correctly for +/// display in the editor +/// +internal class MarkDownPropertyValueEditor : DataValueEditor +{ + private readonly IMarkdownSanitizer _markdownSanitizer; + + public MarkDownPropertyValueEditor( + ILocalizedTextService localizedTextService, + IShortStringHelper shortStringHelper, + IJsonSerializer jsonSerializer, + IIOHelper ioHelper, + DataEditorAttribute attribute, + IMarkdownSanitizer markdownSanitizer) + : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) => _markdownSanitizer = markdownSanitizer; + + public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) + { + if (string.IsNullOrWhiteSpace(editorValue.Value?.ToString())) + { + return null; + } + + var sanitized = _markdownSanitizer.Sanitize(editorValue.Value.ToString()!); + + return sanitized.NullOrWhiteSpaceAsNull(); + } +} diff --git a/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs b/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs index 4bc17c8cfc..b3f92d3e0e 100644 --- a/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.IO; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Web.Common.DependencyInjection; @@ -50,4 +51,11 @@ public class MarkdownPropertyEditor : DataEditor /// protected override IConfigurationEditor CreateConfigurationEditor() => new MarkdownConfigurationEditor(_ioHelper, _editorConfigurationParser); + + /// + /// Create a custom value editor + /// + /// + protected override IDataValueEditor CreateValueEditor() => + DataValueEditorFactory.Create(Attribute!); } diff --git a/src/Umbraco.Core/Security/IMarkdownSanitizer.cs b/src/Umbraco.Core/Security/IMarkdownSanitizer.cs new file mode 100644 index 0000000000..ed1fed2a2c --- /dev/null +++ b/src/Umbraco.Core/Security/IMarkdownSanitizer.cs @@ -0,0 +1,14 @@ +namespace Umbraco.Cms.Core.Security; + +/// +/// Sanitizer service for the markdown editor. +/// +public interface IMarkdownSanitizer +{ + /// + /// Sanitizes Markdown + /// + /// Markdown to be sanitized + /// Sanitized Markdown + string Sanitize(string markdown); +} diff --git a/src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs b/src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs new file mode 100644 index 0000000000..3da03b0e63 --- /dev/null +++ b/src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs @@ -0,0 +1,8 @@ +namespace Umbraco.Cms.Core.Security; + +/// +public class NoopMarkdownSanitizer : IMarkdownSanitizer +{ + /// + public string Sanitize(string markdown) => markdown; +} From 1b712fe6ec52aa4e71b3acf63e393c8e6ab85385 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Tue, 6 Feb 2024 09:53:40 +0100 Subject: [PATCH 10/10] Merge pull request from GHSA-gvpc-3pj6-4m9w * Add MarkDownPropertyValueEditor with html sanitizer * Implement IMarkdownSanitizer. --- .../DependencyInjection/UmbracoBuilder.cs | 3 +- .../MarkDownPropertyValueEditor.cs | 39 +++++++++++++++++++ .../PropertyEditors/MarkdownPropertyEditor.cs | 8 ++++ .../Security/IMarkdownSanitizer.cs | 14 +++++++ .../Security/NoopMarkdownSanitizer.cs | 8 ++++ 5 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs create mode 100644 src/Umbraco.Core/Security/IMarkdownSanitizer.cs create mode 100644 src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 098f770ebc..cf44114c19 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -317,8 +317,9 @@ namespace Umbraco.Cms.Core.DependencyInjection Services.AddSingleton(); Services.AddSingleton(); - // Register a noop IHtmlSanitizer to be replaced + // Register a noop IHtmlSanitizer & IMarkdownSanitizer to be replaced Services.AddUnique(); + Services.AddUnique(); Services.AddUnique(); Services.AddUnique(); diff --git a/src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs b/src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs new file mode 100644 index 0000000000..637a971d68 --- /dev/null +++ b/src/Umbraco.Core/PropertyEditors/MarkDownPropertyValueEditor.cs @@ -0,0 +1,39 @@ +using Umbraco.Cms.Core.IO; +using Umbraco.Cms.Core.Models.Editors; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.PropertyEditors; + +/// +/// A custom value editor to ensure that macro syntax is parsed when being persisted and formatted correctly for +/// display in the editor +/// +internal class MarkDownPropertyValueEditor : DataValueEditor +{ + private readonly IMarkdownSanitizer _markdownSanitizer; + + public MarkDownPropertyValueEditor( + ILocalizedTextService localizedTextService, + IShortStringHelper shortStringHelper, + IJsonSerializer jsonSerializer, + IIOHelper ioHelper, + DataEditorAttribute attribute, + IMarkdownSanitizer markdownSanitizer) + : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) => _markdownSanitizer = markdownSanitizer; + + public override object? FromEditor(ContentPropertyData editorValue, object? currentValue) + { + if (string.IsNullOrWhiteSpace(editorValue.Value?.ToString())) + { + return null; + } + + var sanitized = _markdownSanitizer.Sanitize(editorValue.Value.ToString()!); + + return sanitized.NullOrWhiteSpaceAsNull(); + } +} diff --git a/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs b/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs index 8b1b181c8c..531262f0b2 100644 --- a/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/MarkdownPropertyEditor.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Core.PropertyEditors; @@ -50,4 +51,11 @@ public class MarkdownPropertyEditor : DataEditor /// protected override IConfigurationEditor CreateConfigurationEditor() => new MarkdownConfigurationEditor(_ioHelper, _editorConfigurationParser); + + /// + /// Create a custom value editor + /// + /// + protected override IDataValueEditor CreateValueEditor() => + DataValueEditorFactory.Create(Attribute!); } diff --git a/src/Umbraco.Core/Security/IMarkdownSanitizer.cs b/src/Umbraco.Core/Security/IMarkdownSanitizer.cs new file mode 100644 index 0000000000..ed1fed2a2c --- /dev/null +++ b/src/Umbraco.Core/Security/IMarkdownSanitizer.cs @@ -0,0 +1,14 @@ +namespace Umbraco.Cms.Core.Security; + +/// +/// Sanitizer service for the markdown editor. +/// +public interface IMarkdownSanitizer +{ + /// + /// Sanitizes Markdown + /// + /// Markdown to be sanitized + /// Sanitized Markdown + string Sanitize(string markdown); +} diff --git a/src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs b/src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs new file mode 100644 index 0000000000..3da03b0e63 --- /dev/null +++ b/src/Umbraco.Core/Security/NoopMarkdownSanitizer.cs @@ -0,0 +1,8 @@ +namespace Umbraco.Cms.Core.Security; + +/// +public class NoopMarkdownSanitizer : IMarkdownSanitizer +{ + /// + public string Sanitize(string markdown) => markdown; +}