fix(index): clean up indicies DB rows on all physical delete paths

[fabrizzio-dotCMS]

Summary

Fixes #35306 — ContentletIndexAPIImpl.delete() physically removed the index from the search provider but never deleted the corresponding row from the indicies table.

After a successful physical delete, a stale DB entry would remain pointing to a non-existent index, which could cause startup failures or silent inconsistencies on the next boot.

Root cause

The bug only occurs when delete() is called on an active index without a prior deactivateIndex() call (e.g., direct REST call to DELETE /api/v1/esindex/{name}). In the normal UI workflow, deactivateIndex() clears the DB row via point() before delete() is called, so no orphan is left.

Fix

After a successful physical delete, versionedIndicesAPI.removeByIndexName() is called to clean up the indicies row.

Phase safety:

• Physical delete: indexAPI resolves to IndexAPIImpl (the phase-aware router) — fans out to ES, OS, or both depending on the active migration phase.
• DB cleanup: versionedIndicesAPI.removeByIndexName() issues DELETE FROM indicies WHERE index_name = ? OR index_name = ? (bare + os:: tagged forms), covering both legacy non-versioned rows (ES, phases 0/1/2) and versioned OS rows (phase 3) in a single call. The deprecated legacyIndiciesAPI is not involved.

Test plan

• DELETE /api/v1/esindex/{name} on an active index → row removed from indicies table
• DELETE /api/v1/esindex/{name} on an already-deactivated index → no error (row already gone, removeByIndexName is a no-op)
• Verify across all phases (0, 1, 2, 3)

🤖 Generated with Claude Code

[claude]

Claude finished @fabrizzio-dotCMS's task in 2m 23s —— View job

---

PR Review

• Read changed files
• Analyze logic and edge cases
• Post review findings

---

Three issues worth addressing:

---

1. Test assertion passes vacuously — the actual bug path is not exercised

createContentIndexAndDelete() (test file, lines 379–385) asserts that no orphan rows exist in indicies after deleting the two indices. But createContentIndex() only creates the physical index in the search engine — it never inserts a row into indicies. Rows are only written by activateIndex() → point() / saveIndices(). So the COUNT query was always going to return 0 whether or not the fix exists.

The bug path described in the PR — create → activate → delete without deactivating — is not tested. A correct test would:

1. Create the index
2. Call activateIndex() (which writes the indicies row)
3. Call delete() directly (skipping deactivateIndex())
4. Assert the indicies row is gone

Additionally, the assertion only checks bare names (workingIndex, liveIndex). In OS migration phases 1–3 the row is stored as os::workingIndex / os::liveIndex, so an orphaned OS-tagged row would not be caught by this query.

Fix this →

---

2. Silent DB failure returns true — orphan row can persist without caller knowing

ContentletIndexAPIImpl.delete() (line 1287–1295):

if (deleted) {
    try {
        versionedIndicesAPI.removeByIndexName(indexName);
    } catch (DotDataException e) {
        Logger.error(this, "Index [" + indexName + "] was physically deleted but the " +
                "indices DB row could not be removed: " + e.getMessage(), e);
    }
}
return deleted;

If removeByIndexName throws, the method still returns true. The REST caller sees success, but the orphan row — the very problem this PR fixes — may still be present. This is the same bad state as before the fix, silently. A log line won't prevent the startup failure described in the bug report.

At minimum this deserves a comment documenting the intentional tradeoff. Alternatively, consider returning a wrapper result or adding a metric/alert hook so ops teams can detect the partial failure.

---

3. @WrapInTransaction on removeByIndexName provides no safety net

VersionedIndicesAPIImpl.removeByIndexName() is @WrapInTransaction, but the single DELETE statement it wraps is already atomic. More importantly, the transaction's rollback behavior is irrelevant here because the caller (point 2 above) swallows the exception. If the DELETE fails and rolls back, the exception is caught above and true is still returned — so the annotation adds no safety. It's harmless but misleading.

---

Not flagged (clean): SQL covers both bare and os::-prefixed row forms correctly for all phases; IndexTag.strip() + IndexTag.OS.tag() logic is sound; cache invalidation sequence is correct; the legacyIndiciesAPI path (phases 0/1/2) is handled because the DELETE SQL does not filter by index_version.