Merge pull request #6734 from umbraco/v8/bugfix/6698-nucache-panic

Fixes nucache #6698
This commit is contained in:
Claus
2019-10-18 10:36:19 +02:00
committed by GitHub
3 changed files with 172 additions and 15 deletions

View File

@@ -219,22 +219,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<string, PropertyData[]>(),
CultureInfos = new Dictionary<string, CultureVariation>()
};
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<string, PropertyData[]>(),
CultureInfos = new Dictionary<string, CultureVariation>()
}
PublishedData = contentData
};
}
@@ -339,7 +341,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
@@ -1069,6 +1071,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<ContentNodeKit> GetKits()
{
var paths = new Dictionary<int, string> { { -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, Guid.Empty, 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);
}
/// <summary>
/// This addresses issue: https://github.com/umbraco/Umbraco-CMS/issues/6698
/// </summary>
/// <remarks>
/// 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
/// </remarks>
[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<ContentNodeKit> GetKits()
{
var paths = new Dictionary<int, string> { { -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, Guid.Empty, 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()
{

View File

@@ -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

View File

@@ -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