From ed0058a503564499f0012da76c2ad5435d00fc4e Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Tue, 3 Dec 2024 08:00:18 +0100 Subject: [PATCH] Fix `PanicException: failed to get child with id=` after updating content types (#17702) * Remove tree node from hierarchy after clearing branch * Refactor ClearBranchLocked to split clearing child and siblings --- .../ContentStore.cs | 75 ++++++++++--------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs index d706301ff8..1a463420b8 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs @@ -1276,48 +1276,42 @@ public class ContentStore // try to find the content // if it is not there, nothing to do - _contentNodes.TryGetValue(id, out LinkedNode? link); // else null - if (link?.Value == null) + if (_contentNodes.TryGetValue(id, out LinkedNode? link) && + link.Value is ContentNode content) { - return false; + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug("Clear content ID: {ContentId}", content.Id); + } + + // clear the entire branch + ClearBranchLocked(content); + + // manage the tree + RemoveTreeNodeLocked(content); + + return true; } - ContentNode? content = link.Value; - - if (_logger.IsEnabled(LogLevel.Debug)) - { - _logger.LogDebug("Clear content ID: {ContentId}", content.Id); - } - - // clear the entire branch - ClearBranchLocked(content); - - // manage the tree - RemoveTreeNodeLocked(content); - - return true; + return false; } private void ClearBranchLocked(int id) { - _contentNodes.TryGetValue(id, out LinkedNode? link); - if (link?.Value == null) + if (_contentNodes.TryGetValue(id, out LinkedNode? link) && + link.Value is ContentNode content) { - return; - } + // clear the entire branch + ClearBranchLocked(content); - ClearBranchLocked(link.Value); + // manage the tree + RemoveTreeNodeLocked(content); + } } - private void ClearBranchLocked(ContentNode? content) + private void ClearBranchLocked(ContentNode content) { - // This should never be null, all code that calls this method is null checking but we've seen - // issues of null ref exceptions in issue reports so we'll double check here - if (content == null) - { - throw new ArgumentNullException(nameof(content)); - } - + // Clear content node SetValueLocked(_contentNodes, content.Id, null); if (_localDb != null) { @@ -1326,14 +1320,21 @@ public class ContentStore _contentKeyToIdMap.TryRemove(content.Uid, out _); - var id = content.FirstChildContentId; - while (id > 0) + // Clear children + int childId = content.FirstChildContentId; + if (childId > 0) { - // get the required link node, this ensures that both `link` and `link.Value` are not null - LinkedNode link = GetRequiredLinkedNode(id, "child", null); - ContentNode? linkValue = link.Value; // capture local since clearing in recurse can clear it - ClearBranchLocked(linkValue); // recurse - id = linkValue?.NextSiblingContentId ?? 0; + ContentNode childContent = GetRequiredLinkedNode(childId, "first child", null).Value!; + ClearBranchLocked(childContent); // recurse + + // Clear all siblings of child + int siblingId = childContent.NextSiblingContentId; + while (siblingId > 0) + { + ContentNode siblingContent = GetRequiredLinkedNode(siblingId, "next sibling", null).Value!; + ClearBranchLocked(siblingContent); // recurse + siblingId = siblingContent.NextSiblingContentId; + } } }