task(Migrate ESMappingAPIImpl) Refs #34933

[fabrizzio-dotCMS]

Summary

Migrates ContentletIndexAPIImpl — the central content indexing implementation — to the vendor-neutral Phase-aware router pattern. All Elasticsearch-specific types (BulkRequest, BulkProcessor, ActionListener) are replaced with domain-layer abstractions (IndexBulkRequest, IndexBulkProcessor, IndexBulkListener), enabling dual-write routing to both ES and OS backends during the ES→OS migration without leaking vendor types to callers.

Changes

Backend — Core Indexing

• ContentletIndexAPI (interface): Removed all org.elasticsearch.* imports from method signatures; replaced with vendor-neutral IndexBulkRequest, IndexBulkProcessor, and IndexBulkListener domain types. Added @IndexLibraryIndependent annotation. Changed fullReindexStart() return type from String to IndexStartResult. Added thread-safe DateTimeFormatter alongside deprecated SimpleDateFormat.
• ContentletIndexAPIImpl: Full rewrite to a phase-aware router:
  • Holds two ContentletIndexOperations instances (operationsES, operationsOS) and delegates via PhaseRouter<ContentletIndexOperations>
  • New DualIndexBulkRequest inner class fans out synchronous bulk batches to both ES and OS in dual-write phases
  • New CompositeBulkProcessor inner class fans out async bulk processing; OS failures are fire-and-forget (shadow) in phases 1/2, propagating only in phase 3
  • ProviderIndices inner class resolves working/live/reindex index names per provider, stripping os:: vendor tags before passing to the OS client
  • Lazy-initialized ContentMappingAPI via AtomicReference to break the circular dependency chain
  • Dropped all direct org.elasticsearch.* imports from the implementation
• IndexTag: New utility for stripping vendor-specific index name prefixes (os::)
• IndexStartResult: New domain value object replacing the raw String returned by fullReindexStart()
• BulkProcessorListener: Updated to implement IndexBulkListener instead of the ES-specific listener
• ReindexThread: Minor updates to use IndexBulkProcessor/IndexBulkRequest instead of ES types
• MappingHelper: Replaces ESMappingUtilHelper as the vendor-neutral mapping utility
• ContentletIndexOperationsES / ContentletIndexOperationsOS: Updated to expose the new domain type APIs

Testing

• ContentletIndexAPIImplTest: Updated to use the new domain types and refactored test constructors
• ESSiteSearchAPITest, EMAWebInterceptorTest, CleanUpFieldReferencesJobTest: Minor compile fixes following the API changes

Testing

1. Start the integration test environment: just test-integration-ide
2. Run the primary integration test:

   ./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false -Dit.test=ContentletIndexAPIImplTest
   

3. Verify reindexing still works end-to-end by triggering a full reindex from the dotCMS Admin → System → Reindex
4. Check that ReindexThread completes without errors in both Phase 0 (ES only) and Phase 1 (dual-write) configurations

Breaking Changes

• ContentletIndexAPI.fullReindexStart() now returns IndexStartResult instead of String. Callers previously using the raw timestamp string must call .timestampSuffix() on the result.
• BulkRequest, BulkProcessor, and ActionListener<BulkResponse> removed from the ContentletIndexAPI interface. Any code directly casting or importing these types against the interface will not compile.

This PR fixes: #34933

This PR fixes: #34933

[claude]

Claude finished @fabrizzio-dotCMS's task in 4m 11s —— View job

---

PR Review

• Gather context and read changed files
• Identify issues

---

Four open issues worth addressing before merge:

---

1. Thread-unsafe SimpleDateFormat in new elapsedSinceIndexCreated — ContentletIndexAPIImpl.java:1242

final Date startTime = IndiciesInfo.timestampFormatter.parse(ts);

IndiciesInfo.timestampFormatter is a static SimpleDateFormat, which is not thread-safe. reindexTimeElapsed() (which calls this) is invoked from BulkProcessorListener.beforeBulk (BulkProcessor internal thread) and from fullReindexSwitchover (synchronized on this). These two paths can race. The PR introduced ContentletIndexAPI.threadSafeTimestampFormatter (DateTimeFormatter) for exactly this reason — use it here instead.

Suggested fix:

private long elapsedSinceIndexCreated(final String indexName) {
    try {
        final String ts = indexName.substring(indexName.lastIndexOf('_') + 1);
        final LocalDateTime startTime = LocalDateTime.parse(ts, ContentletIndexAPI.threadSafeTimestampFormatter);
        return Duration.between(startTime, LocalDateTime.now()).toMillis();
    } catch (Exception e) {
        Logger.debug(this, "unable to parse timestamp from index name '" + indexName + "': " + e, e);
        return 0;
    }
}

Fix this →

---

2. reindexTimeElapsed() calls reindexTimeElapsedInLong() twice — ContentletIndexAPIImpl.java:1255-1258

long elapsedTime = reindexTimeElapsedInLong();          // first call
if (elapsedTime > 0) {
    return Optional.of(
        DateUtil.humanReadableFormat(Duration.ofMillis(reindexTimeElapsedInLong()))  // second call — wasteful
            .toLowerCase());
}

The second call discards elapsedTime and re-queries the DB (in phases 0-2 it reads from legacyIndiciesAPI). Should be Duration.ofMillis(elapsedTime).

Fix this →

---

3. bootstrapAndPointES/bootstrapAndPointOS registers indices even on soft failure — ContentletIndexAPIImpl.java:685–692 and 725–731

if (!result) {
    Logger.error(getClass(), "Unable to Bootstrap: ...");
}
pointES(operationsES.toPhysicalName(workingName), ...);  // called regardless of result

When createContentIndex returns false (soft failure — index not created), pointES/pointOS still runs and stores the non-existent index names as the active working/live pointers. Any subsequent write to those pointers will fail silently or corrupt the reindex. Either guard the point* call behind if (result) and throw, or explicitly propagate the failure.

This is ihoffmann-dot's open comment at line ~690 — still unanswered.

Fix this →

---

4. OSGi API breakage — unresolved (flagged in all three prior Claude reviews)

ContentletIndexAPI and BulkProcessorListener are in OSGi-exported packages (com.dotcms.content.elasticsearch.business, com.dotmarketing.common.reindex — osgi-extra.conf lines 92, 573). The following breaking changes have no deprecated shim:

• checkAndInitialiazeIndex() removed (spelling fix, no default-method shim)
• fullReindexStart() return changed String → IndexStartResult
• putToIndex(BulkRequest, ActionListener<BulkResponse>) removed
• createBulkRequest() / appendBulkRequest() / appendBulkRemoveRequest() parameter types changed
• BulkProcessorListener no longer implements BulkProcessor.Listener

Any OSGi plugin compiled against the pre-PR interface will fail at activation with NoSuchMethodError or IncompatibleClassChangeError. Two-phase migration approach (add @Deprecated default-method shims now, remove in next release) is still the recommended fix.

---

Already addressed / not bugs:

• BulkProcessorListener.java:124 (ihoffmann-dot's comment): sep > 0 already guards the substring call when indexOf returns -1. Not a bug — no action needed, but a response to close the thread would help.
• DualIndexBulkRequest.size() checking isMigrationComplete(): dead code path (dual requests can't exist in Phase 3), but harmless.

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-4 — OSGi Plugin API Breakage

• Risk Level: 🟡 MEDIUM

• Why it's unsafe: The ContentletIndexAPI public interface had multiple breaking method signature changes. Any OSGi plugin compiled against the previous version of this interface will fail at activation time with NoSuchMethodError or IncompatibleClassChangeError.

  Breaking changes introduced in this PR:

  1. Method renamed: checkAndInitialiazeIndex() → checkAndInitializeIndex() in ContentletIndexAPI. Any plugin calling the old (misspelled) method receives a NoSuchMethodError.
  2. Return type changed: fullReindexStart() now returns IndexStartResult instead of String. Any plugin assigning the result to a String variable will get a ClassCastException at runtime; any plugin that compiled against the old signature will get NoSuchMethodError.
  3. Method removed: putToIndex(BulkRequest, ActionListener<BulkResponse>) was removed from the ContentletIndexAPI interface entirely and replaced with putToIndex(IndexBulkRequest). Any plugin calling the old two-argument form gets NoSuchMethodError.
  4. Listener contract changed: BulkProcessorListener changed from implementing BulkProcessor.Listener (Elasticsearch type) to implementing IndexBulkListener (domain type). Plugins that registered, subclassed, or cast to the old Elasticsearch-typed listener will fail.

• Code that makes it unsafe:

  • dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPI.java — method checkAndInitializeIndex() (line ~35, was checkAndInitialiazeIndex); method fullReindexStart() return type change (line ~52); removal of putToIndex(BulkRequest, ActionListener<BulkResponse>)
  • dotCMS/src/main/java/com/dotmarketing/common/reindex/BulkProcessorListener.java — class declaration changed from implements BulkProcessor.Listener to implements IndexBulkListener

• Alternative (if possible): Follow the two-phase OSGi migration pattern:

  • Release N (this PR): Keep the old method signatures as @Deprecated overloads that delegate to the new signatures, so N-1 plugins continue to activate cleanly.
  • Release N+1: Remove the deprecated shims once N-2 is outside the rollback window.

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-4 — OSGi Plugin API Breakage

• Risk Level: 🟡 MEDIUM

• Why it's unsafe: The ContentletIndexAPI public interface and BulkProcessorListener class both reside in OSGi-exported packages (com.dotcms.content.elasticsearch.business and com.dotmarketing.common.reindex, confirmed in osgi-extra.conf lines 92 and 573). Multiple breaking changes were made without backward-compatible deprecated shims:

  1. Method removed (no shim): checkAndInitialiazeIndex() removed from ContentletIndexAPI and replaced by checkAndInitializeIndex() (spelling fix). Any OSGi plugin calling the old spelling receives NoSuchMethodError. The only in-tree caller — InitServlet.java:94 — was updated, but external plugin callers were not.

  2. Return type changed: fullReindexStart() return changed from String to IndexStartResult. Any plugin that assigns the result to a String variable throws ClassCastException; any plugin compiled against the old signature throws NoSuchMethodError.

  3. Methods removed: putToIndex(BulkRequest, ActionListener<BulkResponse>) and the BulkRequest-typed overloads of createBulkRequest(), appendBulkRequest(), appendBulkRemoveRequest() removed from ContentletIndexAPI. Replaced with IndexBulkRequest-typed equivalents. Plugins calling any of these forms get NoSuchMethodError.

  4. Processor API changed: createBulkProcessor(BulkProcessorListener) replaced by createBulkProcessor(IndexBulkListener); appendToBulkProcessor(BulkProcessor, ...) replaced by appendToBulkProcessor(IndexBulkProcessor, ...). Plugins using async bulk processing cannot compile or activate.

  5. Listener contract changed: BulkProcessorListener changed from implements BulkProcessor.Listener (Elasticsearch type) to implements IndexBulkListener (domain type). The beforeBulk and afterBulk method signatures changed (e.g. BulkRequest → int actionCount; BulkResponse → List<IndexBulkItemResult>). Plugins that subclass, cast, or register BulkProcessorListener as a BulkProcessor.Listener fail at activation.

• Code that makes it unsafe:

  • dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPI.java — all lines; checkAndInitializeIndex at line 38 (was checkAndInitialiazeIndex); fullReindexStart return at line 52; all BulkRequest/BulkProcessor/ActionListener method signatures replaced throughout
  • dotCMS/src/main/java/com/dotmarketing/common/reindex/BulkProcessorListener.java — class declaration line 34 (implements IndexBulkListener replacing BulkProcessor.Listener); beforeBulk and afterBulk method signatures
  • dotCMS/src/main/resources/osgi/osgi-extra.conf lines 92, 573 — confirm both affected packages are exported to the OSGi container

• Alternative (if possible): Follow the two-phase OSGi migration pattern:

  • Release N (this PR): Keep each removed/changed method as a @Deprecated overload that delegates to the new signature. For example, add @Deprecated default void checkAndInitialiazeIndex() { checkAndInitializeIndex(); } as an interface default method; add a @Deprecated default String fullReindexStart() { return fullReindexStart_new().timestampSuffix(); } shim (renaming the new method accordingly). N-1 plugins continue to activate cleanly against these shims.
  • Release N+1: Remove the deprecated shims once N-2 is outside the rollback window.

[mbiuki]

All four issues from my earlier review are addressed in the latest commits — , , explicit cast, and on . The additional fixes for the misleading log message and field naming are also good improvements. Looks solid. Approving.

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-4 — OSGi Plugin API Breakage

• Risk Level: 🟡 MEDIUM

• Why it's unsafe: The ContentletIndexAPI public interface and BulkProcessorListener class both reside in OSGi-exported packages (com.dotcms.content.elasticsearch.business at osgi-extra.conf line 92; com.dotmarketing.common.reindex at osgi-extra.conf line 573). Multiple breaking changes were made without backward-compatible deprecated shims:

  1. Method removed (no shim): checkAndInitialiazeIndex() removed from ContentletIndexAPI and replaced by checkAndInitializeIndex() (spelling fix). Any OSGi plugin calling the old spelling receives NoSuchMethodError. The only in-tree caller — InitServlet.java:94 — was updated, but external plugin callers were not.

  2. Return type changed: fullReindexStart() return changed from String to IndexStartResult. Any plugin that assigns the result to a String variable throws ClassCastException; any plugin compiled against the old signature throws NoSuchMethodError.

  3. Methods removed without shims: All BulkRequest-typed overloads removed from ContentletIndexAPI — putToIndex(BulkRequest, ActionListener<BulkResponse>), putToIndex(BulkRequest), createBulkRequest() / createBulkRequest(List<Contentlet>) (return type changed), appendBulkRequest(...) (parameter and return types changed), appendBulkRemoveRequest(...) (parameter type changed). All replaced with IndexBulkRequest-typed equivalents. Any plugin calling the old forms gets NoSuchMethodError.

  4. Method removed entirely: getRidOfOldIndex() removed from both the ContentletIndexAPI interface and ContentletIndexAPIImpl with no shim. Plugins calling this method will fail with NoSuchMethodError.

  5. Processor API changed: createBulkProcessor(BulkProcessorListener) replaced by createBulkProcessor(IndexBulkListener); appendToBulkProcessor(BulkProcessor, ...) replaced by appendToBulkProcessor(IndexBulkProcessor, ...). Plugins using async bulk processing cannot compile or activate.

  6. Listener contract changed: BulkProcessorListener changed from implements BulkProcessor.Listener (Elasticsearch type) to implements IndexBulkListener (domain type). The beforeBulk and afterBulk method signatures changed. Plugins that subclass, cast, or register BulkProcessorListener as a BulkProcessor.Listener fail at activation.

• Code that makes it unsafe:

  • dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPI.java — all lines; checkAndInitializeIndex (was checkAndInitialiazeIndex); fullReindexStart return type; removal of getRidOfOldIndex(); all BulkRequest/BulkProcessor/ActionListener method signatures replaced throughout
  • dotCMS/src/main/java/com/dotmarketing/common/reindex/BulkProcessorListener.java — class declaration (implements IndexBulkListener replacing BulkProcessor.Listener); beforeBulk and afterBulk method signatures
  • dotCMS/src/main/resources/osgi/osgi-extra.conf lines 92, 573 — confirm both affected packages are exported to the OSGi container

• Alternative (if possible): Follow the two-phase OSGi migration pattern:

  • Release N (this PR): Keep each removed/changed method as a @Deprecated overload that delegates to the new signature. For example, add @Deprecated default void checkAndInitialiazeIndex() { checkAndInitializeIndex(); } as an interface default method; add a @Deprecated default String fullReindexStart() { ... } shim that calls the new method and returns .indexSuffixES(). N-1 plugins continue to activate cleanly against these shims.
  • Release N+1: Remove the deprecated shims once N-2 is outside the rollback window.

[alwaysmeticulous]

Meticulous was unable to execute a test run for this PR because the most recent commit is associated with multiple PRs. To execute a test run, please try pushing up a new commit that is only associated with this PR.

_{Last updated for commit d63b088. This comment will update as new commits are pushed.}