Fixes issue - when calling Set, we were not setting all node graph elements.

src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs

@@ -1097,8 +1097,8 @@ namespace Umbraco.Tests.PublishedContent
 
                 //site
                 yield return CreateInvariantKit(2, 100, 1, paths);
-                yield return CreateInvariantKit(1, 100, 1, paths); //middle child
-                yield return CreateInvariantKit(3, 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);
@@ -1134,11 +1134,17 @@ namespace Umbraco.Tests.PublishedContent
 
                 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);
 
@@ -1149,7 +1155,7 @@ namespace Umbraco.Tests.PublishedContent
             ChangePublishFlagOfRoot(true, 5, TreeChangeTypes.RefreshNode);
 
             //publish root + descendants
-            ChangePublishFlagOfRoot(true, 6, TreeChangeTypes.RefreshBranch); //TODO: This should fail, need to figure out what the diff is between this and a website
+            ChangePublishFlagOfRoot(true, 6, TreeChangeTypes.RefreshBranch);
         }
 
         [Test]

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

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,8 @@ namespace Umbraco.Web.PublishedCache.NuCache
                 {
                     // replacing existing, handle siblings
                     kit.Node.NextSiblingContentId = existing.NextSiblingContentId;
+                    //TODO: What about previous sibling??
+                    kit.Node.PreviousSiblingContentId = existing.PreviousSiblingContentId;
                 }
 
                 _xmap[kit.Node.Uid] = kit.Node.Id;
@@ -729,7 +734,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,11 +1009,19 @@ 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)
             {
+                if (child.NextSiblingContentId == content.Id)
+                {
+                    content.PreviousSiblingContentId = child.Id;
+                    child = content;
+                    continue;
+                }
+
+
                 // get next child
                 var nextChildLink = GetRequiredLinkedNode(child.NextSiblingContentId, "next child", null);
                 var nextChild = nextChildLink.Value;