fix(search): guard active-index deletion and gate ES/OS delete cascade (#35640)

[fabrizzio-dotCMS]

Problem

QA on #35640 (epic #35476, TC-016–020) surfaced defects in the index delete path (DELETE /api/v1/esindex/{name} → ContentletIndexAPIImpl.delete()):

• TC-018 (High): deleting the currently active index was possible via REST/AJAX. The maintenance UI only hides the Delete option for active indices (a client-side guard), so a direct DELETE /api/v1/esindex/{name} bypassed it and could leave the site with zero indices.
• TC-016: the open design question of whether delete should cascade to the .os twin — undocumented and not configurable.
• TC-016/017: the endpoint returned a false 404 when given the full physical name (with cluster prefix), returned an empty 404 body, and had an inverted-sense helper (indexExists returned true when the index did not exist).

Changes

TC-018 — active-index guard (ContentletIndexAPIImpl.delete)

• Rejects deletion of any index reported active/building by the phase-aware getCurrentIndex() ∪ getNewIndex() — the same sources the maintenance UI uses to flag active/building — via DotStateException, mapped to HTTP 400 in ESIndexResource.
• Fail-closed if the active set can't be resolved.
• Bypass: FEATURE_FLAG_ALLOW_ACTIVE_INDEX_DELETE (default false).
• Reuses the phase-aware getters rather than VersionedIndices directly, because VersionedIndices is empty in Phase 0 (rows are only written once migration starts), which would leave the most common prod state unprotected.

TC-016 — bidirectional cascade (ContentletIndexAPIImpl.delete)

• Per the transparent-mirror principle (the operator sees a single index), delete broadcasts the logical (untagged) name to every write provider, so deleting by either the ES (bare) or OS (.os) name removes the index in every engine that holds it — the mirror is never left half-deleted. Unconditional (no feature flag).

TC-016/017 — cleanups (ESIndexResource)

• Normalize the incoming name with removeClusterIdFromName in both deleteIndex and modIndex, so the endpoints accept both the short name and the full physical name (no more false 404).
• Readable 404 body instead of an empty response.
• Rename indexExists → indexDoesNotExist (matches its actual meaning).

Mirror the rest of the fan-out family (IndexAPIImpl)

• flushCaches / optimize partition the index list by vendor tag so each engine flushes/optimizes only the names it owns (no cross-engine index_not_found).
• clearIndex / openIndex / closeIndex / updateReplicas resolve a per-engine physical name via providerName(impl, name) (OS → .os, others → bare) so the lifecycle op reaches the real index on both engines instead of silently hitting only ES. Site-search is carved out (its OS copy is not .os-tagged).
• Doc OPENSEARCH_MIGRATION.md: adds the transparent-mirror guiding principle, flips the ops table so user-triggered lifecycle replicates to OS (reindex stays the sole feasibility exception), and marks the fan-out routing open-issue resolved.

Testing

• ContentletIndexAPIImplTest#delete_activeIndex_isRejected_unlessFeatureFlagOverrides — active index survives a rejected delete; FF bypass then allows it.
• ContentletIndexAPIImplMigrationIntegrationTest — Phase 1: delete by bare name and by .os name both remove both twins; flushCaches targets each engine cleanly.
• ContentletIndexAPIImplTest#delete_clearsDbPointer — delete clears the indicies DB pointer.
• ContentletIndexAPIImplMigrationIntegrationTest#test_closeAndOpen_phase1_mirrorToBothEngines — close/open by the bare name closes and reopens both twins (validates the shared providerName routing for clear/open/close/updateReplicas).
• Core compiles clean; integration module test-compiles clean; no openapi.yaml diff.

Release Notes (as-merged behavior)

Hardening of the search-index delete/manage endpoints (/api/v1/esindex/*).

Behavior changes

• Active/building indexes can no longer be deleted. Deleting the active working/live index (or one being rebuilt) returns HTTP 400 with a clear message instead of proceeding — a direct REST/AJAX call previously bypassed the UI guard and could leave the instance with zero indexes (TC-018). Deactivate first, then delete.
• Index delete cascades between engines (ES ↔ OpenSearch). During dual-write, deleting by either the ES name or the OpenSearch (.os) name removes the copy in both engines, so no orphan copy is left behind (TC-016).
• The database pointer is always cleaned up. Delete removes the index's row from the indicies table, so no stale pointer to a deleted index remains.
• Delete is resilient. A failure deleting from one engine no longer aborts the operation — the remaining engine and the DB cleanup still run.
• Endpoints accept the full physical index name. delete and activate/deactivate/clear/open/close accept both the short name and the full physical name (with cluster prefix); the full name previously returned a spurious 404 (TC-016/017).
• Cleaner error responses. 404 and 400 return the message in the standard errors array (codes INDEX_NOT_FOUND / INDEX_NOT_DELETABLE) with a readable body.
• Index cache flush (/api/v1/esindex/cache) targets each engine correctly. The flush now routes each index to its own engine instead of sending every index to both, so the OpenSearch cache is reliably flushed and no index_not_found errors are raised during a migration.
• Index lifecycle ops mirror to both engines. Clear / open / close / replica-count changes now apply to the index in both engines during a migration, not just Elasticsearch — the operator sees a single index and the action applies to the whole mirror (transparent-mirror principle).

New configuration flags

Property	Default	Effect
FEATURE_FLAG_ALLOW_ACTIVE_INDEX_DELETE	false	When false, deleting an active/building index is rejected. Set true only for emergency/scripted maintenance to bypass the guard.

Out of scope (follow-ups)

• Site-search index delete — a separate PR under the same issue ([QA-G6] OpenSearch Migration — Index Delete: ES vs OS Behavior (TC-016–TC-020) #35640). Site-search is already OS-aware and phase-routed, but its maintenance delete path isn't wired into that subsystem (needs a new SiteSearchAPI.deleteIndex).
• Expected-miss log-noise refinement on the shadow leg for divergent names (exists-check + shadow-write log policy).
• UI confirmation dialog on delete (requested by @jcastro-dotcms).

🤖 Generated with Claude Code

This PR fixes: #35640

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[claude]

Claude finished @fabrizzio-dotCMS's task in 13m 18s —— View job

---

Code Review — PR #36399

I reviewed the diff against origin/main. The active-index guard, phase-aware union of protected names, bidirectional cascade, per-engine flush/lifecycle routing, and the AJAX reroute all look correct and are well-tested. One real bug in the new ES-store pointer cleanup, plus a couple of minor notes.

New Issues

• 🟠 High: dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java:1855 (clearEsStorePointer) — silently drops the reindexLive pointer.
  The method rebuilds the ES store via IndiciesInfo.Builder.copy(info), but IndiciesInfo.Builder.copy (IndiciesInfo.java:67-75) copies working, live, reindexWorking, and siteSearch — it does not copy reindexLive. So whenever a delete matches any slot (changed = true) and legacyIndiciesAPI.point(builder.build()) runs, an in-progress reindexLive pointer is persisted as null, corrupting reindex state even though that index was not the delete target.
  Reachability: matching working/live/reindexWorking requires the FEATURE_FLAG_ALLOW_ACTIVE_INDEX_DELETE bypass (those are in the protected set), so the trigger is a bypassed delete of an active index while a full reindex is running — exactly the "emergency/scripted maintenance" scenario the bypass exists for. The result is a half-cleared ES store with the live reindex pointer wiped.
  Note the OS counterpart (clearOsStorePointer, same file) rebuilds explicitly slot-by-slot and correctly preserves everything — the ES path is the inconsistent one. Fix: either add builder.setReindexLive(info.getReindexLive()) to Builder.copy, or (matching the OS path) rebuild clearEsStorePointer explicitly instead of via copy. Fix this →

Notes (non-blocking)

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/content/index/IndexAPIImpl.java:459 (clearIndex) — the active-index guard is enforced, but there is no integration test asserting the OS twin is also cleared/guarded by physical name through providerName. test_clear_phase1_activeIndex_isRejectedUnlessBypass covers the reject + bypass path; the bypassed-clear assertion only checks both twins still exist (recreated), not that the OS leg was actually cleared vs. left untouched. Minor coverage gap, not a defect.

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/content/index/IndexAPIImpl.java:370 (flushCaches) — Collectors.groupingBy(IndexTag::resolve) will throw NPE if the input list ever contains a null element (resolve(null)). The public callers (listDotCMSIndices()) won't produce nulls, so this is defensive-only; worth a filter(Objects::nonNull) if you want it fully robust. Assumption: no caller passes a list with null entries — verified for the current call sites.

Resolved (from prior review threads)

• ✅ ContentletIndexAPIImpl.java — divergent-name active OS index now protected via collectProtectedLogicalNames() (ES ∪ OS union in Phases 1/2). Confirmed in the diff + test_delete_phase2_divergentOsName_isProtectedByGuard.
• ✅ IndexAPIImpl.clearIndex — ?action=clear hole closed; guard hoisted into assertIndexNotActive and consulted before dispatch.
• ✅ IndexAjaxAction.deleteIndex:150 — now routes through ContentletIndexAPI.delete, inheriting guard + cascade + pointer cleanup.
• ✅ clearOsStorePointer last-slot case — falls back to removeVersion(OPENSEARCH_3X) instead of a contractually-rejected empty saveIndices. Covered by test_delete_lastOsSlot_....
• ✅ ESIndexResource modIndex/deleteIndex — removeClusterIdFromName normalization + structured ErrorEntity 404/400 bodies; the "unhandled → 500" flags are by-design (mapper-sanitized 500 for genuine backend failures).
• ✅ flushCaches cross-contamination — per-vendor tag partitioning confirmed.

The 🟠 High reindexLive drop is the only item I'd consider blocking; the rest is optional polish.

· Branch: issue-35640-index-delete-guard-and-cascade-ff

[github-actions]

🤖 dotBot Review (Bedrock)

Reviewed 6 file(s); 7 candidate(s) → 6 confirmed, 0 uncertain (unverified, kept for review).

Confirmed findings

• 🟠 High dotCMS/src/main/java/com/dotcms/content/index/IndexAPIImpl.java — Unhandled exceptions in flushCaches lead to partial execution
  The flushCaches method in IndexAPIImpl.java does not handle exceptions thrown by individual provider.flush() calls. If one provider's flush fails (e.g. Elasticsearch connection issue), the exception propagates immediately, preventing subsequent providers from executing their flush. This violates the transparent-mirror principle by leaving some engine caches unflushed while others succeed. The code should wrap each provider.flush() in try-catch blocks to ensure all providers attempt their flush regardless of individual failures.
• 🟠 High dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java:576 — Unhandled exceptions in modIndex operations leak 500 errors
  The modIndex method in ESIndexResource.java catches DotStateException and DotSecurityException but allows other exceptions (e.g., runtime errors from indexAPI.activate/deactivate) to propagate, resulting in unhandled 500 errors. This violates the PR's goal of clean error responses and leaves potential error paths unmanaged.
• 🟡 Medium dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java:312 — Unhandled exceptions in deleteIndex lead to 500 errors
  The deleteIndex method in ESIndexResource.java catches only DotStateException, allowing other exceptions (e.g., ElasticsearchClientException) to propagate. This results in 500 Internal Server Error responses instead of proper error handling. Evidence: The try-catch block around indexAPI.delete() only handles DotStateException, leaving other exceptions uncaught.
• 🟡 Medium dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java:312 — Test gap: modIndex error cases lack coverage
  The modIndex method in ESIndexResource.java handles error cases like activating a non-existent index or an already active index, but the test search revealed no existing tests for modIndex in the test directory. This leaves potential error paths untested, risking undetected regressions.
• 🟡 Medium dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplMigrationIntegrationTest.java:495 — Potential NPE in test assertion on missing map key
  The test asserts result.get("failedShards") equals 0 using isEqualTo(), which would throw a NullPointerException if the key is absent. AssertJ's containsEntry handles missing keys safely by checking both presence and value.
• 🟡 Medium dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplTest.java:500 — Assertion may produce false negatives due to substring match
  The test uses endsWith to check if an index was deleted, which could incorrectly pass if another index exists with a matching suffix. For example, deleting 'index' would still match 'myindex'. The assertion should use exact equality (equals) to ensure only the deleted index is checked.

---

_{us.deepseek.r1-v1:0 · Run: #28821384283 · tokens: in: 64457 · out: 19422 · total: 83879 · calls: 17 · est. ~$0.192}

[swicken]

Changes look good, feedback correctly resolved. Approved!