From d26334ad398c980159cd8ad583b3886debd9391b Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Mon, 13 Jan 2020 17:16:55 +1100
Subject: [PATCH] Fixes the issue of manipulating data for an existing Gen,
 adds notes, makes the Gen comparison operator consistent.

---
 .../PublishedCache/NuCache/ContentNode.cs     |  6 ++++++
 .../PublishedCache/NuCache/ContentStore.cs    | 19 +++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs
index 3b4859432d..a724e78b72 100644
--- a/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs
+++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentNode.cs
@@ -109,6 +109,8 @@ namespace Umbraco.Web.PublishedCache.NuCache
 
         // everything that is common to both draft and published versions
         // keep this as small as possible
+
+        
         public readonly int Id;
         public readonly Guid Uid;
         public IPublishedContentType ContentType;
@@ -116,10 +118,14 @@ namespace Umbraco.Web.PublishedCache.NuCache
         public readonly string Path;
         public readonly int SortOrder;
         public readonly int ParentContentId;
+
+        // TODO: Can we make everything readonly?? This would make it easier to debug and be less error prone especially for new developers.
+        // Once a Node is created and exists in the cache it is readonly so we should be able to make that happen at the API level too.
         public int FirstChildContentId;
         public int LastChildContentId;
         public int NextSiblingContentId;
         public int PreviousSiblingContentId;
+
         public readonly DateTime CreateDate;
         public readonly int CreatorId;
 
diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs
index 550fd507d5..3857a49a28 100644
--- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs
+++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs
@@ -602,7 +602,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
 
         private void ClearRootLocked()
         {
-            if (_root.Gen < _liveGen)
+            if (_root.Gen != _liveGen)
                 _root = new LinkedNode<ContentNode>(new ContentNode(), _liveGen, _root);
             else
                 _root.Value.FirstChildContentId = -1;
@@ -899,8 +899,18 @@ namespace Umbraco.Web.PublishedCache.NuCache
             return link;
         }
 
+        /// <summary>
+        /// This removes this current node from the tree hiearchy by removing it from it's parent's linked list
+        /// </summary>
+        /// <param name="content"></param>
+        /// <remarks>
+        /// This is called within a lock which means a new Gen is being created therefore this will not modify any existing content in a Gen.
+        /// </remarks>
         private void RemoveTreeNodeLocked(ContentNode content)
         {
+            // NOTE: DO NOT modify `content` here, this would modify data for an existing Gen, all modifications are done to clones
+            // which would be targeting the new Gen.
+
             var parentLink = content.ParentContentId < 0
                 ? _root
                 : GetRequiredLinkedNode(content.ParentContentId, "parent", null);
@@ -937,9 +947,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
                 var prev = GenCloneLocked(prevLink);
                 prev.NextSiblingContentId = content.NextSiblingContentId;
             }
-
-            content.NextSiblingContentId = -1;
-            content.PreviousSiblingContentId = -1;
         }
 
         private bool ParentPublishedLocked(ContentNodeKit kit)
@@ -955,7 +962,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
         {
             var node = link.Value;
 
-            if (node != null && link.Gen < _liveGen)
+            if (node != null && link.Gen != _liveGen)
             {
                 node = new ContentNode(link.Value);
                 if (link == _root)
@@ -1109,7 +1116,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
             // this is safe only because we're write-locked
             foreach (var kvp in dict.Where(x => x.Value != null))
             {
-                if (kvp.Value.Gen < _liveGen)
+                if (kvp.Value.Gen != _liveGen)
                 {
                     var link = new LinkedNode<TValue>(null, _liveGen, kvp.Value);
                     dict.TryUpdate(kvp.Key, link, kvp.Value);