fix(folders): restore new-identifier-on-rename behavior while keeping SQL-based child update (#35176)

## Problem

PR #35086 fixed a critical bug (#34655) where renaming a folder
containing 20K+ contentlets would fail with:

```
ERROR: Cannot delete as this path has children
  Where: PL/pgSQL function check_child_assets() line 13 at RAISE
```

The root cause was that the old implementation discovered children via
**Elasticsearch** — any unindexed content was silently skipped, leaving
orphaned DB rows that caused the delete trigger to fire and fail.

PR #35086 fixed this by switching to an **in-place rename** (no
create/delete cycle), which eliminated both the ES dependency and the
delete entirely. However, this introduced an unintended behavioral
change: **the folder identifier (UUID) no longer changes when the folder
URL changes**.

This breaks the contract established by the deterministic identifier
system (`DeterministicIdentifierAPIImpl`), where a folder's UUID is a
SHA-256 hash of `assetType:hostname:parentPath:folderName`. After an
in-place rename, the UUID stays tied to the old path, decoupling
identity from URL.

## Fix

This PR restores the original create-new/delete-old identity contract
while **keeping the SQL-based child discovery** from PR #35086 — the
actual fix for the root cause.

### What changes

**`FolderFactoryImpl.renameFolder()`** — reverts the in-place mutation
back to a create+delete cycle, but replaces the ES-based child discovery
with the depth-first bulk SQL UPDATE:

1. `folder.setName(newName)` so `getNewFolderRecord()` picks up the new
name
2. `getNewFolderRecord()` creates a new folder record — new inode + new
deterministic identifier via `IdentifierAPI.createNew()` (URL change →
identity change restored)
3. `clearIdentifierCacheForSubtree(oldPath)` — evict stale cache entries
before mutation
4. `updateChildPaths(oldPath → newPath)` — depth-first bulk `UPDATE
identifier SET parent_path = ?` sorted by path length, guaranteeing
parent-before-child so the `identifier_parent_path_trigger` passes at
every level without needing deferred constraints
5. Cache evictions (identifier, folder, nav) — unchanged from PR #35086
6. `updateOtherFolderReferences(newInode, oldInode)` — restored from
pre-#35086 code; needed because the inode now changes (permissions,
content-type structure references)
7. `delete(folder)` — safe because all children are moved before this
runs, so `check_child_assets()` finds nothing
8. Mutates `folder.setInode()` + `folder.setIdentifier()` on the
caller's reference so `FolderAPIImpl.refreshContentUnderFolder()`
targets the correct new folder

### Why the delete is now safe

The original bug was caused by ES-based child discovery missing
unindexed items. With `updateChildPaths()`, child discovery is a direct
SQL query against the `identifier` table — no ES dependency, no items
skipped. Every child `parent_path` is updated before `delete()` runs, so
`check_child_assets()` finds zero children and the trigger allows the
delete.

### Why no deferred constraints are needed

Depth-first ordering (sort by ascending path length = parent before
child) ensures that by the time the `identifier_parent_path_trigger`
validates each row's new `parent_path`, the parent folder row was
already updated in the previous iteration. No `DEFERRABLE` constraint
changes required.

### Identifier semantics

| | Old (broken, pre-#35086) | PR #35086 (in-place) | This PR |
|---|---|---|---|
| URL change → new UUID | Yes (via ES, unreliable) | No | Yes (via SQL,
reliable) |
| Child discovery | Elasticsearch | Direct SQL | Direct SQL |
| 20K+ item performance | 20K round-trips | 1 UPDATE/depth level | 1
UPDATE/depth level |
| Constraint violation risk | Yes (unindexed items) | None (no delete) |
None (children moved first) |

## Tests updated

- **`renameFolder()`** — captures old identifier before rename; asserts
old identifier is deleted, sub-folder identifiers are preserved (only
`parent_path` updated), renamed folder has new UUID
- **`renameFolder_updatesChildrenAndSubChildrenPaths()`** — asserts old
parent identifier is deleted, new identifier exists with correct
`asset_name`, sub-folder identifier is unchanged, all `parent_path`
values are correct at every nesting level

Fixes #34655

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

Author: gortiz-dotcms

dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderAPIImpl.java

@@ -174,10 +174,11 @@ public boolean renameFolder(final Folder folder, final String newName,
 			renamed = folderFactory.renameFolder(folder, newName, user, respectFrontEndPermissions);
 
 			// Nav cache eviction for the folder and sub-tree is handled inside the factory.
-			// NOTE: the factory mutates the passed-in folder via folder.setName(newName), so
-			// folder.getPath() returns the new path here. refreshContentUnderFolder depends on
-			// this side-effect to queue the reindex against the renamed path. Do not refactor
-			// the factory to work on a defensive copy without updating this call site.
+			// NOTE: the factory mutates the passed-in folder: setName(newName), setInode(), and
+			// setIdentifier() are all updated to reflect the newly created folder record.
+			// refreshContentUnderFolder depends on these side-effects to target the renamed path
+			// and the correct new inode/identifier. Do not refactor the factory to work on a
+			// defensive copy without updating this call site.
 			//
 			// Queue async ES reindex. DotReindexStateException is caught here so a transient
 			// reindex-queue failure does not roll back an otherwise successful rename.
@@ -676,7 +677,10 @@ public void save(final Folder folder, final String existingId,
 		final boolean isNew = folder.getInode() == null;
 		//if the folder was renamed, we will need to create a new identifier
 		if (!folder.getName().equalsIgnoreCase(existingID.getAssetName())){
-			folderFactory.renameFolder(folder, folder.getName(), user, respectFrontEndPermissions);
+			if (!folderFactory.renameFolder(folder, folder.getName(), user, respectFrontEndPermissions)) {
+				throw new DotDataException("Could not rename folder '" + existingID.getAssetName()
+						+ "' to '" + folder.getName() + "': a folder with that name already exists.");
+			}
 			// Queue async ES reindex so content under the renamed folder is re-indexed at the new
 			// path. folderFactory.renameFolder() mutates folder.setName(newName), so
 			// folder.getPath() already returns the new path when this runs.

dotCMS/src/main/java/com/dotmarketing/portlets/folders/business/FolderFactoryImpl.java

@@ -8,6 +8,7 @@
 import static com.dotmarketing.portlets.folders.business.FolderFactorySql.GET_CONTENT_TYPE_COUNT;
 
 import com.dotcms.browser.BrowserQuery;
+import com.dotcms.business.WrapInTransaction;
 import com.dotcms.system.SimpleMapAppContext;
 import com.dotcms.util.transform.DBTransformer;
 import com.dotcms.util.transform.TransformerLocator;
@@ -52,7 +53,6 @@
 import io.vavr.control.Try;
 import java.io.IOException;
 import java.sql.Connection;
-import java.sql.SQLException;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -632,21 +632,20 @@ private boolean move(final Folder folder, final Object destination)
     return successOperation.getValue();
   }
 
-  private void updateOtherFolderReferences(final String newFolderInode, final String oldFolderInode) {
-    final DotConnect dotConnect = new DotConnect();
-    try {
-      dotConnect.executeStatement("update structure set folder = '" + newFolderInode
-          + "' where folder = '" + oldFolderInode + "'");
-      dotConnect.executeStatement("update permission set inode_id = '" + newFolderInode
-          + "' where inode_id = '" + oldFolderInode + "'");
-      dotConnect.executeStatement("update permission_reference set asset_id = '"
-          + newFolderInode + "' where asset_id = '" + oldFolderInode + "'");
-
-
-    } catch (SQLException e) {
-      Logger.error(FolderFactoryImpl.class, e.getMessage(), e);
-      throw new DotRuntimeException(e.getMessage(), e);
-    }
+  private void updateOtherFolderReferences(final String newFolderInode, final String oldFolderInode)
+      throws DotDataException {
+    final DotConnect dc = new DotConnect();
+    dc.executeUpdate(
+        "UPDATE structure SET folder = ? WHERE folder = ?", newFolderInode, oldFolderInode);
+    APILocator.getContentTypeAPI(APILocator.systemUser())
+        .search("folder='" + newFolderInode + "'", "mod_date", -1, 0)
+        .forEach(CacheLocator.getContentTypeCache2()::remove);
+    dc.executeUpdate(
+        "UPDATE permission SET inode_id = ? WHERE inode_id = ?", newFolderInode, oldFolderInode);
+    dc.executeUpdate(
+        "UPDATE permission_reference SET asset_id = ? WHERE asset_id = ?", newFolderInode, oldFolderInode);
+    APILocator.getPermissionAPI().removePermissionableFromCache(oldFolderInode);
+    APILocator.getPermissionAPI().removePermissionableFromCache(newFolderInode);
   }
 
   /**
@@ -778,6 +777,7 @@ protected boolean isChildFolder(final Folder childFolder, final Folder parentFol
 		}
   }
 
+  @WrapInTransaction
   @Override
   protected boolean renameFolder(final Folder folder, final String newName, final User user,
       final boolean respectFrontEndPermissions) throws DotDataException, DotSecurityException {
@@ -805,41 +805,20 @@ protected boolean renameFolder(final Folder folder, final String newName, final
     // Snapshot sub-folder old-path data BEFORE any update so targeted cache eviction is possible
     final List<Map<String, Object>> subFolderSnapshot = loadSubFolderSnapshot(oldPath, hostId);
 
-    // Capture the old values before mutation so they can be restored if a subsequent DB
-    // operation fails. @WrapInTransaction rolls back DB changes, but Java object state is
-    // not rolled back automatically — callers that retain a reference after failure would
-    // otherwise observe the new name permanently despite the rename not completing.
-    final String oldFolderName = folder.getName();
-    final String oldAssetName  = ident.getAssetName();
-
-    // Update the folder record in-place.
-    // save() uses upsertFolder keyed on the existing inode — the inode is NOT regenerated.
-    // Because the inode is preserved, the old updateOtherFolderReferences calls (which updated
-    // structure.folder, permission.inode_id, permission_reference.asset_id from oldInode to
-    // newInode) are pure no-ops (SET x = X WHERE x = X) and have been removed.
-    //
-    // save() internally calls APILocator.getIdentifierAPI().find() to get the identifier for
-    // FolderCache eviction. At this point the identifier cache still holds asset_name=oldName, so
-    // save() correctly evicts the old path-keyed cache entry. The identifier cache is evicted on
-    // the next line, AFTER save() reads it, preserving the correct ordering.
+    // Set the new name on the folder object so getNewFolderRecord() picks it up via
+    // initialFolder.getName(). Restore the original name if a subsequent DB operation fails,
+    // since @WrapInTransaction rolls back DB changes but Java object state is not automatically
+    // restored.
+    folder.setName(newName);
+
+    // Create a new folder record
+    // getNewFolderRecord() calls IdentifierAPI.createNew() which generates the deterministic hash
+    // based on assetType:hostname:parentPath:folderName — changing the URL changes the identity.
+    final Folder newFolder;
     try {
-      folder.setName(newName);
-      save(folder);
-
-      // Evict old identifier URI cache entry BEFORE mutating asset_name: removeFromCacheByIdentifier
-      // looks up the identifier by ID from cache (still holds old URI) and evicts by that key.
-      // After save(ident), the new URI key is stored normally.
-      CacheLocator.getIdentifierCache().removeFromCacheByIdentifier(ident.getId());
-      ident.setAssetName(newName);
-      APILocator.getIdentifierAPI().save(ident);
-
-      // Evict again AFTER the DB identifier row has been updated. This prevents a race where the
-      // identifier can be cached by another thread (e.g. reindex) after the pre-update eviction
-      // but before/while the asset_name mutation becomes visible.
-      CacheLocator.getIdentifierCache().removeFromCacheDirect(ident.getId(), hostId, null);
+      newFolder = getNewFolderRecord(folder, user, parentPath, hostId);
     } catch (final DotDataException | RuntimeException e) {
-      folder.setName(oldFolderName);
-      ident.setAssetName(oldAssetName);
+      folder.setName(ident.getAssetName()); // restore original name
       // A concurrent rename to the same destination path can race past the collision check and
       // hit the DB unique constraint on identifier(parent_path, asset_name, host_inode).
       // Treat this the same as a pre-checked collision: restore state and return false.
@@ -849,32 +828,49 @@ protected boolean renameFolder(final Folder folder, final String newName, final
       throw e;
     }
 
-    // Evict identifier cache for the entire subtree BEFORE the bulk UPDATE so the eviction can
-    // reliably find the old-path-keyed cache entries. removeFromCacheByIdentifier() uses the
-    // cached identifier's current URI (old path) to evict the path-keyed entry; if we waited
-    // until after the UPDATE, those URI-keyed entries might already have been LRU-evicted and
-    // the stale old-path keys would survive indefinitely.
+    // Evict identifier cache for the entire old subtree BEFORE the bulk UPDATE so the eviction
+    // can reliably find the old-path-keyed cache entries.
     clearIdentifierCacheForSubtree(oldPath, hostId);
 
     // Bulk-update every child identifier's parent_path, processing parent folders before children.
-    // The snapshot already contains all sub-folder paths so no extra SELECTs are needed.
+    // The new folder identifier already exists in the DB (created above), so the trigger's
+    // parent-path existence check passes at every depth level as parents are updated first.
     updateChildPaths(oldPath, newPath, hostId, subFolderSnapshot);
 
-    // Evict identifier caches rooted at the *new* parent path after the bulk update.
-    // This closes the window where other threads could have cached descendants with the old
-    // parent_path during the rename transaction.
+    // Evict identifier caches rooted at the new path after the bulk update.
     clearIdentifierCacheForSubtree(newPath, hostId);
 
-    // Targeted folder cache eviction for affected sub-folders (using old path data from snapshot)
+    // Targeted folder cache eviction for affected sub-folders (using old path data from snapshot).
     evictSubFolderCache(subFolderSnapshot, hostId);
 
-    // Nav cache cleanup — evict the renamed folder, its parent, and every sub-folder in the tree
+    // Nav cache cleanup — evict the renamed folder, its parent, and every sub-folder in the tree.
+    // Uses folder.getInode() (old inode) before it is replaced below.
     CacheLocator.getNavToolCache().removeNav(folder.getHostId(), folder.getInode());
     CacheLocator.getNavToolCache().removeNavByPath(hostId, parentPath);
     for (final Map<String, Object> row : subFolderSnapshot) {
       CacheLocator.getNavToolCache().removeNav(hostId, (String) row.get("inode"));
     }
 
+    // Transfer content-type structure and permission references from old inode to new inode.
+    updateOtherFolderReferences(newFolder.getInode(), folder.getInode());
+
+    // Delete old folder. All children have been moved to the new path by updateChildPaths(),
+    // so check_child_assets() finds no remaining children pointing to the old path and the
+    // delete succeeds without a constraint violation.
+    delete(folder);
+
+    // Update the caller's folder reference to the new inode and identifier so that
+    // FolderAPIImpl.refreshContentUnderFolder() targets the correct renamed folder.
+    // folder.getName() was already set to newName above.
+    folder.setInode(newFolder.getInode());
+    folder.setIdentifier(newFolder.getIdentifier());
+
+    // Evict the new folder's own identifier cache entry. clearIdentifierCacheForSubtree()
+    // only covers items whose parent_path starts with newPath (the folder's children), not the
+    // folder itself (whose parent_path is parentPath). Without this eviction, a prior URI-keyed
+    // entry for the same path could be served until natural LRU expiry.
+    CacheLocator.getIdentifierCache().removeFromCacheByIdentifier(newFolder.getIdentifier());
+
     return true;
   }
 

dotcms-integration/src/test/java/com/dotmarketing/portlets/folders/business/FolderAPITest.java

@@ -151,10 +151,11 @@ public static void prepare() throws Exception {
 
 
 	/**
-	 * Verifies the basic in-place rename behavior for a folder hierarchy.
+	 * Verifies the rename behavior for a folder hierarchy.
 	 * <p>
-	 * All folder identifiers are preserved (same UUIDs), while
-	 * {@code asset_name}/{@code parent_path} are updated consistently.
+	 * The renamed folder receives a new identifier (URL change = identity change). Sub-folder
+	 * identifiers are preserved but their {@code parent_path} values are updated consistently
+	 * via bulk SQL — no Elasticsearch dependency, so unindexed items are never missed.
 	 */
 	@Test
 	public void renameFolder() throws Exception {
@@ -167,26 +168,33 @@ public void renameFolder() throws Exception {
 		final Folder ftest3 = folderAPI
 				.createFolders(ftest.getPath()+"/ff1/ff2/ff3", host, user, false);
 
+		// Capture the old top-level identifier BEFORE rename — the factory mutates ftest.
+		final String oldFtestIdentifier = ftest.getIdentifier();
+
 		final String newFolderName = "folderTestXX" + System.currentTimeMillis();
 		assertTrue(folderAPI.renameFolder(ftest, newFolderName, user, false));
 
-		final Identifier ident  = identifierAPI.loadFromDb(ftest.getIdentifier());
+		// Old top-level identifier is deleted (create+delete cycle).
+		final Identifier oldIdent = identifierAPI.loadFromDb(oldFtestIdentifier);
+		assertNull(oldIdent);
+
+		// Renamed folder has a new identifier UUID.
+		assertNotEquals(oldFtestIdentifier, ftest.getIdentifier());
+
+		// Sub-folder identifiers are preserved — only parent_path is updated via bulk SQL.
 		final Identifier ident1 = identifierAPI.loadFromDb(ftest1.getIdentifier());
 		final Identifier ident2 = identifierAPI.loadFromDb(ftest2.getIdentifier());
 		final Identifier ident3 = identifierAPI.loadFromDb(ftest3.getIdentifier());
-
-		assertNotNull(ident);
 		assertNotNull(ident1);
 		assertNotNull(ident2);
 		assertNotNull(ident3);
 
-		assertEquals(newFolderName, ident.getAssetName());
-
 		final String newRootPath = StringPool.SLASH + newFolderName + StringPool.SLASH;
 		assertEquals(newRootPath, ident1.getParentPath());
 		assertEquals(newRootPath + "ff1" + StringPool.SLASH, ident2.getParentPath());
 		assertEquals(newRootPath + "ff1" + StringPool.SLASH + "ff2" + StringPool.SLASH, ident3.getParentPath());
 
+		// Folder and sub-folders are findable by new paths.
 		final Folder newFolder = folderAPI.findFolderByPath(newRootPath, host, user, false);
 		final Folder newFolder1 = folderAPI.findFolderByPath(newRootPath + "ff1/", host, user, false);
 		final Folder newFolder2 = folderAPI.findFolderByPath(newRootPath + "ff1/ff2/", host, user, false);
@@ -196,7 +204,10 @@ public void renameFolder() throws Exception {
 		assertNotNull(newFolder1);
 		assertNotNull(newFolder2);
 		assertNotNull(newFolder3);
+		// Top-level folder has a new identifier; ftest object was mutated to hold it.
 		assertEquals(ftest.getIdentifier(), newFolder.getIdentifier());
+		assertNotEquals(oldFtestIdentifier, newFolder.getIdentifier());
+		// Sub-folders retain their original identifiers.
 		assertEquals(ftest1.getIdentifier(), newFolder1.getIdentifier());
 		assertEquals(ftest2.getIdentifier(), newFolder2.getIdentifier());
 		assertEquals(ftest3.getIdentifier(), newFolder3.getIdentifier());
@@ -239,7 +250,8 @@ public void renameFolder_toExistingName_returnsFalse() throws DotDataException,
 	 *     <li><b>Expected Result:</b>
 	 *     <ul>
 	 *         <li>Rename returns {@code true}.</li>
-	 *         <li>Folder and sub-folder identifiers are unchanged (in-place rename).</li>
+	 *         <li>The renamed folder receives a new identifier UUID (URL change = identity change).</li>
+	 *         <li>Sub-folder identifier is preserved (only {@code parent_path} is updated).</li>
 	 *         <li>All direct children's {@code parent_path} in the DB equals the new folder path.</li>
 	 *         <li>All sub-children's {@code parent_path} equals the new sub-folder path.</li>
 	 *         <li>Folder is findable by new path; old path resolves to nothing.</li>
@@ -268,6 +280,7 @@ public void renameFolder_updatesChildrenAndSubChildrenPaths() throws Exception {
 		// File asset under /original/sub/
 		final Contentlet fileInSub = new FileAssetDataGen(subFolder, "content-in-sub").nextPersisted();
 
+		// Capture old identifiers BEFORE rename — factory mutates the parentFolder object.
 		final String parentIdentifierId = parentFolder.getIdentifier();
 		final String subIdentifierId    = subFolder.getIdentifier();
 
@@ -277,15 +290,17 @@ public void renameFolder_updatesChildrenAndSubChildrenPaths() throws Exception {
 
 		assertTrue("renameFolder must return true", renamed);
 
-		// Folder identifier is preserved — in-place rename, no create+delete cycle
-		final Identifier renamedParentIdent = identifierAPI.loadFromDb(parentIdentifierId);
-		assertNotNull("Parent folder identifier must still exist after rename", renamedParentIdent);
-		assertEquals("Parent folder identifier UUID must be unchanged",
-				parentIdentifierId, renamedParentIdent.getId());
-		assertEquals("Parent folder asset_name must reflect the new name",
-				newName, renamedParentIdent.getAssetName());
-
-		// Sub-folder identifier is preserved and its parent_path is updated
+		// Renamed folder gets a new identifier — create+delete cycle, URL change = identity change.
+		final Identifier oldParentIdent = identifierAPI.loadFromDb(parentIdentifierId);
+		assertNull("Old parent folder identifier must be deleted after rename", oldParentIdent);
+		assertNotEquals("Renamed folder must have a new identifier UUID",
+				parentIdentifierId, parentFolder.getIdentifier());
+		final Identifier newParentIdent = identifierAPI.loadFromDb(parentFolder.getIdentifier());
+		assertNotNull("New parent folder identifier must exist after rename", newParentIdent);
+		assertEquals("New parent folder asset_name must reflect the new name",
+				newName, newParentIdent.getAssetName());
+
+		// Sub-folder identifier is preserved and its parent_path is updated via bulk SQL.
 		final Identifier renamedSubIdent = identifierAPI.loadFromDb(subIdentifierId);
 		assertNotNull("Sub-folder identifier must still exist", renamedSubIdent);
 		assertEquals("Sub-folder identifier UUID must be unchanged",
@@ -311,11 +326,13 @@ public void renameFolder_updatesChildrenAndSubChildrenPaths() throws Exception {
 		assertEquals("File asset in sub-folder: parent_path must reflect the renamed sub-folder path",
 				"/" + newName + "/sub/", fileInSubIdent.getParentPath());
 
-		// Folder is findable by new path and returns the same identifier
+		// Folder is findable by new path and has the new identifier UUID.
 		final Folder foundByNewPath = folderAPI.findFolderByPath("/" + newName + "/", site, user, false);
 		assertNotNull("Renamed folder must be findable by new path", foundByNewPath);
-		assertEquals("Folder found by new path must have the same identifier UUID",
+		assertNotEquals("Folder found by new path must have a new identifier UUID (URL change = identity change)",
 				parentIdentifierId, foundByNewPath.getIdentifier());
+		assertEquals("Folder found by new path must match the mutated parentFolder identifier",
+				parentFolder.getIdentifier(), foundByNewPath.getIdentifier());
 
 		// Old path no longer resolves to a folder — verified via DB to avoid a stale cache hit.
 		// A DB query for asset_name=originalName under parentPath='/' must return no folder row.