fix: add new approach to rename functionality (#34655)

[gortiz-dotcms]

This PR fixes: #34655

Problem

`FolderAPIImpl.renameFolder()` failed with a PostgreSQL constraint violation when renaming a folder that contained more than ~20K contentlets:

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

Root Cause

The old implementation followed a create-new / move-all / delete-old cycle:

1. Create a new `Folder` + `Identifier` record with the new name.
2. Fetch all children via Elasticsearch (`findContentletsByFolder` → `search("+conFolder:...")`).
3. Move each child one-by-one in a Java loop (one DB round-trip per item).
4. Delete the old folder record.

Step 2 is the critical flaw: any contentlet not indexed in Elasticsearch is silently skipped (e.g. `.HEIC` files, recently uploaded content, or anything with a stale index). After moving only the ES-indexed subset, the old folder still had DB children pointing to its path. When step 4 tried to delete the folder, the PostgreSQL trigger `check_child_assets()` found those leftover children and raised a constraint violation.

This also explains why a manual re-index temporarily "fixed" the issue (#32181): the re-index brought ES and the DB back in sync, so the next rename attempt found all children.

Secondary cost: moving 20K+ items one-by-one in Java meant 20K+ individual DB round-trips, making the operation slow even when it succeeded.

Fix

Replace the create-new / move-all / delete-old cycle with a pure-SQL in-place rename:

1. Update `folder.name` for the existing folder record (via `save()`).
2. Update `identifier.asset_name` for the folder's identifier (via `IdentifierAPI.save()`). The old URI is explicitly evicted from the identifier cache before mutation so the pre-rename path key is removed.
3. Bulk-update all children's `parent_path` using a plain `UPDATE identifier SET parent_path = ? WHERE parent_path = ? AND host_inode = ?`, processed depth-first by sorting all (oldPath → newPath) pairs by ascending path length before issuing any UPDATE.
4. Flush folder, identifier, and nav-tool caches (targeted eviction using a pre-rename snapshot of sub-folder paths — no nuclear cache clear).
5. Queue async ES reindex after the transaction commits (in `FolderAPIImpl`, not inside the factory) so a transient ES failure never rolls back a successful DB rename.

No new folder is created, no old folder is deleted — so the `check_child_assets()` trigger never fires. The rename is now DB-only and ES-independent, so unindexed items are never missed.

Why depth-first ordering (not a single bulk UPDATE)?

The existing `identifier_parent_path_trigger` fires `BEFORE UPDATE` on every `identifier` row and verifies that the new `parent_path` resolves to an existing folder. Processing each folder level before its children guarantees that by the time the trigger runs for a child row, its new parent path already exists in the DB.

All (oldPath → newPath) pairs are derived from a pre-loaded snapshot of sub-folder data and sorted by ascending old-path length: a parent path is always shorter than any of its descendants, so length-order guarantees parent-before-child without recursion or an explicit stack.

New index for trigger performance

`EXPLAIN` on the trigger's validation query showed it was using `idx_identifier_perm` (`asset_type + host_inode`) and then applying `lower(parent_path||asset_name||'/')` as a row-level filter — a scan over all folders on the host per trigger invocation. With 20K rows being updated this is O(N × F) lookups.

A new functional index `idx_identifier_parent_path_trigger` on `(host_inode, asset_type, lower(parent_path||asset_name||'/'))` matches the trigger's exact expression. After adding it, `EXPLAIN` showed the expression moved from `Filter` to `Index Cond` and cost dropped from 90.34 → 8.57. The index is added to `postgres.sql` for new installs and via startup task `Task260324AddIdentifierPathTriggerIndex` for existing databases.

Performance

	Old approach	New approach
Child discovery	ES query (misses unindexed items)	None needed
Child updates	20K+ Java → DB round-trips	1 bulk `UPDATE` per folder level
Trigger validation	0 (before broken delete)	Per-row, now index-backed (O(log F))
Folder deletion + cascade	Required → constraint error	Not needed

Changes

• `FolderFactoryImpl` — in-place rename, targeted cache eviction, depth-first child path update (sorted by path length), LIKE escaping for folder names containing `%` / `_`.
• `FolderAPIImpl` — ES reindex moved here (post-transaction, guarded by try/catch); removed duplicate nav-cache evictions now handled by the factory.
• `Task260324AddIdentifierPathTriggerIndex` + `postgres.sql` — new functional index for trigger query performance.

Tests

• Updated `renameFolder()` — adjusted assertions to reflect in-place rename semantics: identifiers are preserved (same UUID), `asset_name` / `parent_path` are updated rather than creating new records.
• New `renameFolder_updatesChildrenAndSubChildrenPaths()` — creates a folder tree with a file asset, an HTML page, and a sub-folder (itself containing a file asset), renames the root, then asserts at the DB level (via `identifierAPI.loadFromDb()` and a direct `DotConnect` query — no cache, no ES) that every item's `parent_path` was updated correctly at all nesting levels and the old path no longer exists.

🤖 Generated with Claude Code

[claude]

Claude finished @gortiz-dotcms's task in 6m 39s —— View job

---

🔍 dotCMS Backend Review

• Load dotCMS conventions
• Get Java diff
• Run specialized sub-agent reviews (security, database, Java standards, REST API)
• Consolidate findings and post review

Three 🟡 Medium issues identified (all previously flagged, all still open). Full review posted in the dedicated review comment above.

[claude]

🔍 dotCMS Backend Review

[🟡 Medium] Task260324AddIdentifierPathTriggerIndex.java:48 — PostgreSQL-specific DDL without a database-type guard

CREATE INDEX IF NOT EXISTS ... lower(parent_path||asset_name||'/') is a PostgreSQL functional-index expression. The task runs unconditionally on every database type. On H2 (integration tests) or any future non-Postgres backend this DDL will throw at startup, halting the task runner. Established runonce tasks (e.g. Task05080RecreateIdentifierIndex) guard equivalent DDL with DbConnectionFactory.isPostgres().

new DotConnect().executeStatement(
    "CREATE INDEX IF NOT EXISTS " + INDEX_NAME + " ON identifier"
    + " (host_inode, asset_type, lower(parent_path||asset_name||'/')))");

💡 Wrap in if (DbConnectionFactory.isPostgres()) { ... }. This was flagged in the prior review and is still open.

---

[🟡 Medium] FolderAPIImpl.java:192-204 — folder.getPath() logged without CR/LF sanitization

newName is correctly sanitized into safeNewName, but folder.getPath() — built from user-supplied ancestor folder names stored in the DB — is passed verbatim into every Logger.error() call in the catch blocks. A folder whose path contains \r or \n can inject synthetic log lines, masking audit trails or confusing log-aggregation pipelines.

Logger.error(FolderAPIImpl.class, "Error renaming folder '"
    + folder.getPath() + "' with id: " + folder.getIdentifier() + " to name: "
    + safeNewName + ". Error: " + e.getMessage());

💡 Apply the same sanitization: final String safePath = folder.getPath().replaceAll("[\\r\\n]", " "); and substitute safePath in all Logger calls inside the catch blocks. This was flagged in the prior review and is still open.

---

[🟡 Medium] Task260324AddIdentifierPathTriggerIndex.java:33 — INDEX_NAME constant has package-private visibility

static final String INDEX_NAME is accessible package-wide but serves no purpose outside this class. No other class in the package references it; the package-private access appears to be an accidental omission of private.

static final String INDEX_NAME = "idx_identifier_parent_path_trigger";

💡 Change to private static final String INDEX_NAME = ...;. This was flagged in the prior review and is still open.

---

Next steps

• 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in <File.java>
• Every new push triggers a fresh review automatically

View job run