dotCMS / dotCMS/core

Folder delete leaves orphaned search documents: the orphan sweep is not covered by #37276's journal

Open
#37,599 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

OKR : Customer Support Team : Scout Type : Defect
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Problem Statement

#37276 made content deletes durable: destroyContentlets now journals the index removal in the same transaction that deletes the rows, and ReindexThread retries it until the index acknowledges. A repo-wide search confirms ESContentletAPIImpl:3118 is the only production caller of ReindexQueueAPI.addIdentifierDelete.

Folder deletion does not go through that path, and so is not covered by that fix.

FolderAPIImpl._deleteChildrenAssetsFromFolder deletes a folder's contents in two passes. The first uses ContentletAPI.destroy, which is journalled and fine. The second is a sweep that removes whatever the first could not see, straight from the database by path:

// FolderAPIImpl.java:583-587
Identifier ident = APILocator.getIdentifierAPI().find(folder.getIdentifier());
List<Identifier> orphanList = APILocator.getIdentifierAPI()
        .findByParentPath(folder.getHostId(), ident.getURI());
for (Identifier orphan : orphanList) {
    APILocator.getIdentifierAPI().delete(orphan);
}

findByParentPath is a plain select * from identifier where parent_path = ? and host_inode = ? (IdentifierFactoryImpl.java:193), and IdentifierFactoryImpl.deleteIdentifier is a raw SQL cascade across permission, permission_reference, tree, multi_tree, the version-info table, workflow_task, the asset table, inode and identifier — then it clears the identifier cache only. Nothing records that the search index still owes a removal.

So every folder delete that reaches this sweep leaves search documents with no content behind them, permanently, until someone runs a full reindex by hand. That is precisely the defect #37276 was raised to fix, in a path its fix does not reach.

Why the sweep is reached routinely, not exceptionally

The first pass is findContentletsByFolder, which is a search-index query filtered by READ permission (ESContentletAPIImpl.java:993-996). Anything the acting user cannot read, and anything not currently indexed, is invisible to it and falls through to the sweep. It is not an edge case.

Why now

Content Drive bulk folder delete (#37063) multiplies how often folder deletion runs. The defect is pre-existing and reachable today through the shipped single-folder delete (#35161), so this is not caused by that feature — but it is why it was found, and why leaving it is less comfortable than it was.

Steps to Reproduce

  1. As an administrator, create a folder on a site and put a piece of content in it.
  2. Make that content invisible to the first pass — the simplest lever is to act as a user with edit and permission-editing rights on the folder but no read on the contentlet, so the permission-filtered lookup does not return it.
  3. Delete the folder as that user. It succeeds.
  4. Confirm the database rows are gone: select * from identifier where id = '<contentlet identifier>' returns nothing.
  5. Query the live index for that identifier.

Expected: the document is gone, or a journal row exists and ReindexThread removes it.
Actual: the document is still in the index with no content behind it, and nothing will ever remove it.

Contrast: destroy the same contentlet directly rather than through its folder, and the removal is journalled and retried. Only the folder path leaks.

Acceptance Criteria

  • An integration test reproduces the orphan through a folder delete and fails on today's code
  • The sweep records a durable index removal for the content it deletes, in the same transaction, reusing ReindexQueueAPI.addIdentifierDelete rather than a second mechanism
  • Only identifiers that actually belong in the content index are journalled — Identifier.getAssetType() distinguishes contentlet from folder and the others, and journalling a folder identifier would be noise
  • #37276's scoping caveat is respected: addIdentifierDelete is identifier-wide, which is correct here because the sweep destroys every version and language of what it removes — the same condition that makes it correct in destroyContentlets. It must not be extended to any path where some languages stay live
  • A failure to record the removal fails the delete rather than committing silently, matching journalContentDeletes
  • Other callers of IdentifierAPI.delete / IdentifierFactory.deleteIdentifier are checked for the same shape, and either fixed or recorded as not affected
  • New test classes are registered in a MainSuite* / Junit5Suite* @SuiteClasses list — an unregistered class compiles, passes locally, and is never run in CI
  • Integration tests run with -Dmaven.build.cache.enabled=false, and Tests run: N is confirmed in target/failsafe-reports/*.txt rather than trusting the exit code

dotCMS Version

main — the sweep predates #37276 and is unchanged by it.

Severity

Medium. Silent and permanent index drift, with the same user-visible symptom #37276 documented: the index reports N results, the database resolves N−k, and the missing k are dropped without an error. It needs a folder delete to occur, which bounds it below #37276's own frequency.

Links

  • #37276 — the fix this path was not covered by
  • #37063 — Content Drive bulk folder delete, where this was found; its backend spec records it as FR-009d
  • #35161 — single-folder delete, shipped, reaches the same sweep

Related but deliberately separate: the same sweep also deletes content without checking permissions or honouring locks, so an author who cannot see content destroys it with less scrutiny than one who can. That is a behaviour question with its own blast radius, not an index-consistency one, and folding the two together would make neither reviewable. Recorded in #37063's backend spec as FR-009b / D-014.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with FolderAPIImpl._deleteChildrenAssetsFromFolder around lines 583-587, then read IdentifierFactoryImpl.findByParentPath and deleteIdentifier and the existing journaling path in ESContentletAPIImpl around lines 993-996 and 3118. Add an integration test for the folder-delete orphan case, register any new test in the relevant MainSuite* or Junit5Suite*, and run it with the stated cache setting while checking the failsafe report count. Done means the sweep's contentlet removals are durably handled without journaling folder identifiers, and failures prevent a silent commit.

Written by the indexing model from the issue text.

Assessment

Tech stack
elasticsearch, java
Domain
backend, databases, search, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.