diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs index ac3ac7d455..7b7403d737 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs @@ -213,22 +213,24 @@ namespace Umbraco.Tests.PublishedContent var level = path.Count(x => x == ','); var now = DateTime.Now; + var contentData = new ContentData + { + Name = "N" + id, + Published = true, + TemplateId = 0, + VersionId = 1, + VersionDate = now, + WriterId = 0, + Properties = new Dictionary(), + CultureInfos = new Dictionary() + }; + return new ContentNodeKit { ContentTypeId = _contentTypeInvariant.Id, Node = new ContentNode(id, Guid.NewGuid(), level, path, sortOrder, parentId, DateTime.Now, 0), DraftData = null, - PublishedData = new ContentData - { - Name = "N" + id, - Published = true, - TemplateId = 0, - VersionId = 1, - VersionDate = now, - WriterId = 0, - Properties = new Dictionary(), - CultureInfos = new Dictionary() - } + PublishedData = contentData }; } @@ -333,7 +335,7 @@ namespace Umbraco.Tests.PublishedContent CultureInfos = GetCultureInfos(id, now) }; - var withDraft = id%2==0; + var withDraft = id % 2 == 0; var withPublished = !withDraft; return new ContentNodeKit @@ -1063,6 +1065,153 @@ namespace Umbraco.Tests.PublishedContent AssertLinkedNode(child3.contentNode, 1, 2, -1, -1, -1); } + [Test] + public void Refresh_Node_Ensures_Linked_list() + { + // NOTE: these tests are not using real scopes, in which case a Scope does not control + // how the snapshots generations work. We are forcing new snapshot generations manually. + + IEnumerable GetKits() + { + var paths = new Dictionary { { -1, "-1" } }; + + //root + yield return CreateInvariantKit(100, -1, 1, paths); + + //site + yield return CreateInvariantKit(2, 100, 1, paths); + yield return CreateInvariantKit(1, 100, 2, paths); //middle child + yield return CreateInvariantKit(3, 100, 3, paths); + + //children of 1 + yield return CreateInvariantKit(20, 1, 1, paths); + yield return CreateInvariantKit(30, 1, 2, paths); + yield return CreateInvariantKit(40, 1, 3, paths); + } + + Init(GetKits()); + + var snapshotService = (PublishedSnapshotService)_snapshotService; + var contentStore = snapshotService.GetContentStore(); + + Assert.AreEqual(1, contentStore.Test.LiveGen); + Assert.IsTrue(contentStore.Test.NextGen); + + var middleNode = contentStore.Test.GetValues(1)[0]; + Assert.AreEqual(1, middleNode.gen); + AssertLinkedNode(middleNode.contentNode, 100, 2, 3, 20, 40); + + //This will set a flag to force creating a new Gen next time the store is locked (i.e. In Notify) + contentStore.CreateSnapshot(); + + Assert.IsFalse(contentStore.Test.NextGen); + + _snapshotService.Notify(new[] + { + new ContentCacheRefresher.JsonPayload(1, TreeChangeTypes.RefreshNode) + }, out _, out _); + + Assert.AreEqual(2, contentStore.Test.LiveGen); + Assert.IsTrue(contentStore.Test.NextGen); + + middleNode = contentStore.Test.GetValues(1)[0]; + Assert.AreEqual(2, middleNode.gen); + AssertLinkedNode(middleNode.contentNode, 100, 2, 3, 20, 40); + } + + /// + /// This addresses issue: https://github.com/umbraco/Umbraco-CMS/issues/6698 + /// + /// + /// This test mimics if someone were to: + /// 1) Unpublish a "middle child" + /// 2) Save and publish it + /// 3) Publish it with descendants + /// 4) Repeat steps 2 and 3 + /// + /// Which has caused an exception. To replicate this test: + /// 1) RefreshBranch with kits for a branch where the top most node is unpublished + /// 2) RefreshBranch with kits for the branch where the top most node is published + /// 3) RefreshBranch with kits for the branch where the top most node is published + /// 4) RefreshNode + /// 5) RefreshBranch with kits for the branch where the top most node is published + /// + [Test] + public void Refresh_Branch_With_Alternating_Publish_Flags() + { + // NOTE: these tests are not using real scopes, in which case a Scope does not control + // how the snapshots generations work. We are forcing new snapshot generations manually. + + IEnumerable GetKits() + { + var paths = new Dictionary { { -1, "-1" } }; + + //root + yield return CreateInvariantKit(100, -1, 1, paths); + + //site + yield return CreateInvariantKit(2, 100, 1, paths); + yield return CreateInvariantKit(1, 100, 2, paths); //middle child + yield return CreateInvariantKit(3, 100, 3, paths); + + //children of 1 + yield return CreateInvariantKit(20, 1, 1, paths); + yield return CreateInvariantKit(30, 1, 2, paths); + yield return CreateInvariantKit(40, 1, 3, paths); + } + + //init with all published + Init(GetKits()); + + var snapshotService = (PublishedSnapshotService)_snapshotService; + var contentStore = snapshotService.GetContentStore(); + + var rootKit = _source.Kits[1].Clone(); + + void ChangePublishFlagOfRoot(bool published, int assertGen, TreeChangeTypes changeType) + { + //This will set a flag to force creating a new Gen next time the store is locked (i.e. In Notify) + contentStore.CreateSnapshot(); + + Assert.IsFalse(contentStore.Test.NextGen); + + //Change the root publish flag + var kit = rootKit.Clone(); + kit.DraftData = published ? null : kit.PublishedData; + kit.PublishedData = published? kit.PublishedData : null; + _source.Kits[1] = kit; + + _snapshotService.Notify(new[] + { + new ContentCacheRefresher.JsonPayload(1, changeType) + }, out _, out _); + + Assert.AreEqual(assertGen, contentStore.Test.LiveGen); + Assert.IsTrue(contentStore.Test.NextGen); + + //get the latest gen for content Id 1 + var (gen, contentNode) = contentStore.Test.GetValues(1)[0]; + Assert.AreEqual(assertGen, gen); + //even when unpublishing/re-publishing/etc... the linked list is always maintained + AssertLinkedNode(contentNode, 100, 2, 3, 20, 40); + } + + //unpublish the root + ChangePublishFlagOfRoot(false, 2, TreeChangeTypes.RefreshBranch); + + //publish the root (since it's not published, it will cause a RefreshBranch) + ChangePublishFlagOfRoot(true, 3, TreeChangeTypes.RefreshBranch); + + //publish root + descendants + ChangePublishFlagOfRoot(true, 4, TreeChangeTypes.RefreshBranch); + + //save/publish the root (since it's already published, it will just cause a RefreshNode + ChangePublishFlagOfRoot(true, 5, TreeChangeTypes.RefreshNode); + + //publish root + descendants + ChangePublishFlagOfRoot(true, 6, TreeChangeTypes.RefreshBranch); + } + [Test] public void Refresh_Branch_Ensures_Linked_List() { diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index ceeb2d076a..f1ae2010db 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -344,6 +344,9 @@ 8300 / http://localhost:8300/ + 8210 + / + http://localhost:8210/ False False @@ -425,4 +428,4 @@ - \ No newline at end of file + diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Rte.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Rte.cshtml index b7e293eed8..34bb744596 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Rte.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Rte.cshtml @@ -1,5 +1,9 @@ @model dynamic @using Umbraco.Web.Composing @using Umbraco.Web.Templates - -@Html.Raw(TemplateUtilities.ParseInternalLinks(Model.value.ToString(), Current.UmbracoContext.UrlProvider)) +@{ + var value = TemplateUtilities.ParseInternalLinks(Model.value.ToString(), Current.UmbracoContext.UrlProvider); + value = TemplateUtilities.ResolveUrlsFromTextString(value); + value = TemplateUtilities.ResolveMediaFromTextString(value); +} +@Html.Raw(value) diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index f782f09289..24e2fc29a5 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -89,7 +89,7 @@ namespace Umbraco.Web.PropertyEditors var grid = DeserializeGridValue(rawJson, out var rtes); - var userId = _umbracoContextAccessor.UmbracoContext?.Security.CurrentUser.Id ?? Constants.Security.SuperUserId; + var userId = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser?.Id ?? Constants.Security.SuperUserId; // Process the rte values foreach (var rte in rtes) diff --git a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs index 3eed40c8bf..5743e9c1d5 100644 --- a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs @@ -114,7 +114,7 @@ namespace Umbraco.Web.PropertyEditors if (editorValue.Value == null) return null; - var userId = _umbracoContextAccessor.UmbracoContext?.Security.CurrentUser.Id ?? Constants.Security.SuperUserId; + var userId = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser?.Id ?? Constants.Security.SuperUserId; var config = editorValue.DataTypeConfiguration as RichTextConfiguration; var mediaParent = config?.MediaParentId; diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs index 5f8e81fd31..3b4859432d 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using Umbraco.Core.Models.PublishedContent; using Umbraco.Web.PublishedCache.NuCache.DataSource; @@ -6,6 +7,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { // represents a content "node" ie a pair of draft + published versions // internal, never exposed, to be accessed from ContentStore (only!) + [DebuggerDisplay("Id: {Id}, Path: {Path}")] internal class ContentNode { // special ctor for root pseudo node diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index f0d695f090..f71abd6aa7 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -549,7 +549,10 @@ namespace Umbraco.Web.PublishedCache.NuCache // manage children if (existing != null) + { kit.Node.FirstChildContentId = existing.FirstChildContentId; + kit.Node.LastChildContentId = existing.LastChildContentId; + } // set SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); @@ -571,6 +574,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { // replacing existing, handle siblings kit.Node.NextSiblingContentId = existing.NextSiblingContentId; + kit.Node.PreviousSiblingContentId = existing.PreviousSiblingContentId; } _xmap[kit.Node.Uid] = kit.Node.Id; @@ -729,7 +733,9 @@ namespace Umbraco.Web.PublishedCache.NuCache // clear if (existing != null) { + //this zero's out the branch (recursively), if we're in a new gen this will add a NULL placeholder for the gen ClearBranchLocked(existing); + //TODO: This removes the current GEN from the tree - do we really want to do that? RemoveTreeNodeLocked(existing); } @@ -1002,9 +1008,9 @@ namespace Umbraco.Web.PublishedCache.NuCache } // else it's going somewhere in the middle, - // and this is bad, perfs-wise - we only do it when moving - // inserting in linked list is slow, optimizing would require trees - // but... that should not happen very often - and not on large amount of data + // TODO: There was a note about performance when this occurs and that this only happens when moving and not very often, but that is not true, + // this also happens anytime a middle node is unpublished or republished (which causes a branch update), i'm unsure if this has perf impacts, + // i think this used to but it doesn't seem bad anymore that I can see... while (child.NextSiblingContentId > 0) { // get next child