Skip to content

Elements: Invalidate the id/key map when an element container is deleted (closes #23072)#23074

Open
AndyButland wants to merge 4 commits into
release/18.0from
v18/bugfix/23072-refresh-element-cache-on-container-delete
Open

Elements: Invalidate the id/key map when an element container is deleted (closes #23072)#23074
AndyButland wants to merge 4 commits into
release/18.0from
v18/bugfix/23072-refresh-element-cache-on-container-delete

Conversation

@AndyButland
Copy link
Copy Markdown
Contributor

@AndyButland AndyButland commented Jun 4, 2026

Description

#23072 reports a scenario found in testing with Umbraco Deploy, where the same container and element are transferred from one environment to another, deleted and then transferred again.

This leads to the an element container being deleted and recreated under the same fixed GUID, but it will be allocated a new integer Id.

In this situation the backoffice element tree's children endpoint (GET /umbraco/management/api/v1/tree/element/children?parentId=<containerGuid>) returns no items for elements nested under an element container, even though the root endpoint reports the container with hasChildren: true. The tree only starts showing the children after the application is restarted.

Root cause

The children endpoint resolves the container GUID to its integer id via IIdKeyMap, then queries WHERE ParentId == <resolvedId>. The failure happens because nothing has evicted the stale GUID → old-id entry from IIdKeyMap when the container was deleted.

So the children query keeps resolving the container GUID to the old (now non-existent) id and returns nothing, until an app restart rebuilds the in-memory map.

Fix

A new notification handler, ElementContainerDeletedDistributedCacheNotificationHandler, has been created and registered. It handles EntityContainerDeletedNotification and, for element containers, evicts the container's stale IIdKeyMap entry, via the distributed cache so it clears on every server in a load-balanced setup.

The eviction runs through a dedicated ElementContainerCacheRefresher whose only job is to clear the container's id/key mapping. A container deletion changes no element data, so this deliberately avoids the broader invalidation in
ElementCacheRefresher (which clears the entire elements cache on every payload).

Fixes #23072.

Testing

Automated

ElementContainerDeletedDistributedCacheNotificationHandlerTests (integration test) reproduces the
exact mechanism:

  1. Create an element type and a container under a fixed GUID.
  2. Resolve the container's children once, to warm the GUID → id mapping in IIdKeyMap.
  3. Delete the container, then recreate it under the same GUID (asserting it gets a new id).
  4. Create an element under the recreated container.
  5. Assert the container GUID now resolves to the new id, and that the element-tree children query
    returns the element.

Confirmed red before the fix (container GUID resolves to the old id; total = 0) and green after.

Manual

A small debug controller was used to reproduce and verify this end-to-end in a running site:

DebugElementTreeChildrenController.cs
using System.Text;
using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Entities;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Cms.Core.Strings;

namespace Umbraco.Cms.Web.UI.Controllers;

public class DebugElementTreeChildrenController : Controller
{
    private const string ElementTypeAlias = "debugElementTreeItem";

    private readonly IContentTypeService _contentTypeService;
    private readonly IElementService _elementService;
    private readonly IElementContainerService _elementContainerService;
    private readonly IEntityService _entityService;
    private readonly IIdKeyMap _idKeyMap;
    private readonly IShortStringHelper _shortStringHelper;

    public DebugElementTreeChildrenController(
        IContentTypeService contentTypeService,
        IElementService elementService,
        IElementContainerService elementContainerService,
        IEntityService entityService,
        IIdKeyMap idKeyMap,
        IShortStringHelper shortStringHelper)
    {
        _contentTypeService = contentTypeService;
        _elementService = elementService;
        _elementContainerService = elementContainerService;
        _entityService = entityService;
        _idKeyMap = idKeyMap;
        _shortStringHelper = shortStringHelper;
    }

    [HttpGet("/debug/element-tree-children")]
    public async Task<IActionResult> Run()
    {
        IContentType elementType = await GetOrCreateElementTypeAsync();

        // Use a fixed key so the container can be deleted and re-created under the same key
        // (the node id changes because ids are never reused).
        var containerKey = Guid.Parse("d0d0d0d0-2307-2072-0000-000000000001");

        // 1. Create the container, then resolve its children once so the key->id mapping is cached.
        await CreateContainerAsync(containerKey, "Debug Element Container (v1)");
        _ = _entityService.GetPagedChildren(
            containerKey,
            [UmbracoObjectTypes.ElementContainer, UmbracoObjectTypes.Element],
            [UmbracoObjectTypes.ElementContainer, UmbracoObjectTypes.Element],
            0,
            100,
            false,
            out _);

        // 2. Delete and re-create under the same key - the new container gets a different id, but without
        //    the fix the stale key->id mapping is never evicted (no element-container cache invalidation).
        await _elementContainerService.DeleteAsync(containerKey, Constants.Security.SuperUserKey);
        EntityContainer container = await CreateContainerAsync(containerKey, "Debug Element Container (v2)");

        // Create an element under the recreated container via the low-level service.
        var element = new Element($"Debug Element {Guid.NewGuid():N}", container.Id, elementType);
        OperationResult saveResult = _elementService.Save(element);

        // Show what the tree's resolution path actually returns right now.
        Attempt<int> resolvedParentId = _idKeyMap.GetIdForKey(containerKey, UmbracoObjectTypes.ElementContainer);
        IEntitySlim[] childrenViaTreePath = _entityService.GetPagedChildren(
            containerKey,
            [UmbracoObjectTypes.ElementContainer, UmbracoObjectTypes.Element],
            [UmbracoObjectTypes.ElementContainer, UmbracoObjectTypes.Element],
            0,
            100,
            false,
            out var total).ToArray();

        var sb = new StringBuilder();
        sb.AppendLine($"Element type       : {ElementTypeAlias} (key {elementType.Key})");
        sb.AppendLine($"Container key      : {containerKey}");
        sb.AppendLine($"Container real id  : {container.Id}");
        sb.AppendLine($"Element saved      : {saveResult.Success} (key {element.Key}, parentId {element.ParentId})");
        sb.AppendLine();
        sb.AppendLine($"IdKeyMap resolves container key -> id : {(resolvedParentId.Success ? resolvedParentId.Result.ToString() : "FAILED")}");
        sb.AppendLine($"  (stale if this != container real id {container.Id})");
        sb.AppendLine($"GetPagedChildren via tree path        : total={total}, returned={childrenViaTreePath.Length}");
        sb.AppendLine();
        sb.AppendLine("Now expand the container in the backoffice Library tree, or call:");
        sb.AppendLine($"  GET /umbraco/management/api/v1/tree/element/children?parentId={containerKey}");
        sb.AppendLine();
        sb.AppendLine(total == 0
            ? "BUG REPRODUCED: the element is in the DB but the tree path returns nothing (until restart)."
            : "Tree path returns the element - bug not reproduced in this run.");

        return Content(sb.ToString(), "text/plain");
    }

    private async Task<IContentType> GetOrCreateElementTypeAsync()
    {
        IContentType? elementType = _contentTypeService.Get(ElementTypeAlias);
        if (elementType is not null)
        {
            return elementType;
        }

        elementType = new ContentType(_shortStringHelper, Constants.System.Root)
        {
            Alias = ElementTypeAlias,
            Name = "Debug Element Tree Item",
            IsElement = true,
            AllowedInLibrary = true,
        };
        await _contentTypeService.CreateAsync(elementType, Constants.Security.SuperUserKey);
        return elementType;
    }

    private async Task<EntityContainer> CreateContainerAsync(Guid key, string name)
    {
        Attempt<EntityContainer?, EntityContainerOperationStatus> result =
            await _elementContainerService.CreateAsync(key, name, null, Constants.Security.SuperUserKey);
        if (result.Success is false || result.Result is null)
        {
            throw new InvalidOperationException($"Failed to create element container: {result.Status}");
        }

        return result.Result;
    }
}

Before the fix — the container GUID resolves to the old id and the tree returns nothing:

Element type       : debugElementTreeItem (key b36df702-32bf-4eb3-975a-4c3f5e4f6102)
Container key      : d0d0d0d0-2307-2072-0000-000000000001
Container real id  : 5874
Element saved      : True (key de76e20e-6e34-49d3-92bb-a587655baac3, parentId 5874)

IdKeyMap resolves container key -> id : 5873
  (stale if this != container real id 5874)
GetPagedChildren via tree path        : total=0, returned=0

Now expand the container in the backoffice Library tree, or call:
  GET /umbraco/management/api/v1/tree/element/children?parentId=d0d0d0d0-2307-2072-0000-000000000001

BUG REPRODUCED: the element is in the DB but the tree path returns nothing (until restart).

The container shows no children in the Library tree until the site is restarted.

After the fix — the GUID resolves to the recreated container's id and the element is returned:

Element type       : debugElementTreeItem (key b36df702-32bf-4eb3-975a-4c3f5e4f6102)
Container key      : d0d0d0d0-2307-2072-0000-000000000001
Container real id  : 5871
Element saved      : True (key 7cf7ada7-eb4a-48fc-917b-fa7021bc50dc, parentId 5871)

IdKeyMap resolves container key -> id : 5871
  (stale if this != container real id 5871)
GetPagedChildren via tree path        : total=1, returned=1

Now expand the container in the backoffice Library tree, or call:
  GET /umbraco/management/api/v1/tree/element/children?parentId=d0d0d0d0-2307-2072-0000-000000000001

Tree path returns the element - bug not reproduced in this run.

The element appears in the Library tree without a restart ✅.

Copilot AI review requested due to automatic review settings June 4, 2026 16:35
@claude
Copy link
Copy Markdown

claude Bot commented Jun 4, 2026

Claude finished @AndyButland's task in 3m 33s —— View job


PR Review

Target: origin/release/18.0 · Based on commit: a4a53d8a

Fixes a stale IIdKeyMap entry that caused the element tree's children endpoint to return nothing after a container was deleted and recreated under the same GUID. Adds a new EntityContainerDeletedNotification handler that routes a Remove payload through the ElementCacheRefresher on every server, and an integration test that reproduces the exact failure mode.

  • Modified public API: New DistributedCacheExtensions.RefreshElementCache(IEnumerable<EntityContainer>) overload; new ElementContainerDeletedDistributedCacheNotificationHandler public class.
  • Affected implementations (outside this PR): None — purely additive.
  • Breaking changes: None.

Suggestions

  • tests/.../ElementContainerDeletedDistributedCacheNotificationHandlerTests.cs:52: Test method name Child_Element_Is_Returned_After_Container_Recreated_Under_Same_Key doesn't follow the project's Can_/Cannot_ naming convention. Consider Can_Resolve_Children_After_Container_Recreated_Under_Same_Key.

Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

The implementation is clean and follows established patterns exactly (DeletedDistributedCacheNotificationHandlerBase, sibling handler structure, DistributedCacheExtensions overload). The ContainedObjectType == Constants.ObjectTypes.Element guard correctly scopes the handler to element containers only, which is important since EntityContainerDeletedNotification is also published for document/media type container deletions. The test design — warming the cache, deleting, recreating under the same key with a LocalServerMessenger to deliver the cache refresh in-process — is a solid regression test for the exact failure mode described.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a stale IIdKeyMap entry scenario for element containers (folders) by ensuring container deletions trigger distributed cache invalidation, so a container recreated under the same GUID resolves to the new integer id without requiring an app restart.

Changes:

  • Added a distributed-cache notification handler for EntityContainerDeletedNotification to invalidate element-container mappings on delete.
  • Added a DistributedCacheExtensions.RefreshElementCache(IEnumerable<EntityContainer>) overload to emit ElementCacheRefresher remove payloads for deleted element containers.
  • Added an integration regression test covering delete + recreate-under-same-key + children query behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
tests/Umbraco.Tests.Integration/Umbraco.Core/Cache/ElementContainerDeletedDistributedCacheNotificationHandlerTests.cs Integration regression test reproducing and validating the cache eviction behavior for deleted/recreated element containers.
src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs Registers the new distributed-cache notification handler in core notifications.
src/Umbraco.Core/Cache/NotificationHandlers/Implement/ElementContainerDeletedDistributedCacheNotificationHandler.cs New handler that routes element-container deletions into element cache refresh (remove) to evict IIdKeyMap mappings across servers.
src/Umbraco.Core/Cache/DistributedCacheExtensions.cs Adds an overload to build/remove payloads for deleted element containers via ElementCacheRefresher.

Comment thread src/Umbraco.Core/Cache/DistributedCacheExtensions.cs Outdated
AndyButland and others added 2 commits June 4, 2026 18:51
Routing container-delete invalidation through ElementCacheRefresher cleared
the entire elements cache on every payload, so deleting a container triggered
a full clear even though no element data changed (and a second clear on top of
the ElementTreeChangeNotification refresh when the container held elements).

Add a dedicated ElementContainerCacheRefresher whose only job is to evict the
container's IIdKeyMap entry, and route EntityContainerDeletedNotification
through it instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/Umbraco.Core/Cache/DistributedCacheExtensions.cs
@AndyButland AndyButland requested a review from lauraneto June 5, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants