From 05f25bdcda66a7d5d2902c16ff70b4c3e1631f3e Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 10:37:49 -0600 Subject: [PATCH 01/11] =?UTF-8?q?feat(migration):=20role-gated=20migration?= =?UTF-8?q?-readiness=20endpoint=20=E2=80=94=20framework=20+=20Site=20Sear?= =?UTF-8?q?ch=20half=20(#36360)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Internal, non-public pre-phase-change readiness report for support: - GET /api/v1/index/migration/readiness — @Hidden (absent from the OpenAPI / API playground) and gated to CMS admins or the migration support role (OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY, default os_migration_qa); 403 otherwise. Read-only, stateless — every field derived from live index state at request time. - MigrationReadiness (report DTO): current phase + read/write engines + evaluable flag; overall verdict (safeToAdvance / safeToRollback / outOfSyncCount / summary / per-index blockers); and the per-index ES↔OS mirror diff for both mirrored families. - MirrorStatus (shared per-index diff: kind, es/os existence + exact counts, verdict IN_SYNC/MISSING_TWIN/COUNT_DRIFT, recommendation). - SiteSearchMirrorReconciler (recreated from the lost PR3 work) now uses the exact SiteSearchAPI.documentCount (not a 10k-capped search total), so drift on large indices is reported. ContentIndexMirrorReconciler is stubbed for step b. - MigrationReadinessService composes phase + both reconcilers into the verdict: advance is gated on zero out-of-sync in dual-write phases; rollback is unsafe when any index's ES copy is behind its OS twin (a downgrade would drop that delta) — derived from live counts, no persisted state. Unit test (mocked reconcilers, phase via Config): 7/7 — advance/rollback verdicts across phases 0–3, missing-twin and >10k count-drift blocking advance. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ContentIndexMirrorReconciler.java | 24 +++ .../index/migration/MigrationReadiness.java | 53 ++++++ .../migration/MigrationReadinessService.java | 111 +++++++++++++ .../content/index/migration/MirrorStatus.java | 68 ++++++++ .../migration/SiteSearchMirrorReconciler.java | 91 +++++++++++ .../v1/index/MigrationReadinessResource.java | 108 +++++++++++++ .../MigrationReadinessServiceTest.java | 152 ++++++++++++++++++ 7 files changed, 607 insertions(+) create mode 100644 dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java create mode 100644 dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java create mode 100644 dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java create mode 100644 dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java create mode 100644 dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java create mode 100644 dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java create mode 100644 dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java new file mode 100644 index 000000000000..5277c2b95595 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java @@ -0,0 +1,24 @@ +package com.dotcms.content.index.migration; + +import java.util.Collections; +import java.util.List; + +/** + * Content-index half of the migration-readiness report (issue #36360): compares the versioned + * content indices (working/live) against their {@code .os} twins across both engines, mirroring + * {@link SiteSearchMirrorReconciler} but for the content store. + * + *

Work in progress (PR3, step b). The framework and the Site Search half are in + * place; this half is stubbed to return no rows so {@link MigrationReadinessService} composes both + * sections without special-casing. It will enumerate working/live from {@code IndiciesInfo} and read + * exact per-engine document counts (index stats, not a capped search total) for each twin — + * the same drift/missing-twin verdicts as the Site Search half, emitted as + * {@link MirrorStatus.IndexKind#CONTENT_WORKING} / {@link MirrorStatus.IndexKind#CONTENT_LIVE}.

+ */ +public class ContentIndexMirrorReconciler { + + /** Per-index mirror status for the working/live content indices. TODO(#36360 PR3 step b). */ + public List statuses() { + return Collections.emptyList(); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java new file mode 100644 index 000000000000..b289a4fc5c92 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -0,0 +1,53 @@ +package com.dotcms.content.index.migration; + +import java.util.List; + +/** + * Support-facing ES→OS migration-readiness report (issue #36360): a point-in-time snapshot that a + * role-gated support technician reads before changing the migration phase, to see whether it + * is safe and, if not, what to do. Read-only and stateless — every field is derived from live index + * state at request time, nothing is persisted. + * + * @param phase the current migration phase and which engine it reads/writes + * @param verdict the overall go/no-go for advancing and rolling back, with reasons + * @param contentIndices per-index mirror status for the versioned content indices (working/live) + * @param siteSearchIndices per-index mirror status for the Site Search indices + */ +public record MigrationReadiness( + PhaseInfo phase, + Verdict verdict, + List contentIndices, + List siteSearchIndices) { + + /** + * @param current the current phase ordinal (0–3) + * @param name the phase enum name (e.g. {@code PHASE_2_DUAL_WRITE_OS_READS}) + * @param readEngine which engine currently serves reads ("Elasticsearch" or "OpenSearch") + * @param writeEngines which engines currently receive writes + * @param evaluable whether a cross-engine comparison is meaningful for a forward phase change + * (only the dual-write phases 1/2); when false the mirror lists are advisory + * context, not a forward go/no-go + */ + public record PhaseInfo( + int current, + String name, + String readEngine, + List writeEngines, + boolean evaluable) {} + + /** + * @param safeToAdvance whether it is safe to promote toward the OpenSearch-only phase + * @param safeToRollback whether it is safe to downgrade — false when any index's Elasticsearch + * copy is behind its OpenSearch twin, because a downgrade routes reads back + * to Elasticsearch and would silently drop that delta until a reindex + * @param outOfSyncCount how many indices need attention (missing twin or count drift) + * @param summary one human-readable sentence describing the overall state + * @param blockers per-index reasons that make advancing unsafe (empty when safe) + */ + public record Verdict( + boolean safeToAdvance, + boolean safeToRollback, + int outOfSyncCount, + String summary, + List blockers) {} +} diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java new file mode 100644 index 000000000000..e555e8258049 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -0,0 +1,111 @@ +package com.dotcms.content.index.migration; + +import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Composes the ES→OS migration-readiness report (issue #36360): current phase + the per-index mirror + * status of both mirrored index families (content and Site Search) + an overall go/no-go verdict for + * changing the phase. Read-only and stateless — the verdict is derived from live index state at + * request time, nothing is persisted (see {@link MigrationReadiness}). + * + *

Verdict semantics

+ *
    + *
  • Advance (toward OpenSearch-only): meaningful in the dual-write phases (1/2), where it + * is safe only when no index needs attention. In Phase 0 there is nothing to reconcile yet + * (twins are built during dual-write) and in Phase 3 there is no further phase — both report + * safe with an explanatory summary.
    • + *
  • Rollback (downgrade): a downgrade ultimately routes reads back to Elasticsearch + * (Phases 0/1), so it is unsafe when any index's ES copy is behind its OpenSearch twin — that + * delta (typically content written while OpenSearch served reads) would be silently missing + * until a full reindex. Derived from the same live counts, so no historical state is needed.
    • + *
+ */ +public class MigrationReadinessService { + + private final SiteSearchMirrorReconciler siteSearchReconciler; + private final ContentIndexMirrorReconciler contentReconciler; + + public MigrationReadinessService() { + this(new SiteSearchMirrorReconciler(), new ContentIndexMirrorReconciler()); + } + + @VisibleForTesting + MigrationReadinessService(final SiteSearchMirrorReconciler siteSearchReconciler, + final ContentIndexMirrorReconciler contentReconciler) { + this.siteSearchReconciler = siteSearchReconciler; + this.contentReconciler = contentReconciler; + } + + /** Builds the readiness report for the current phase. */ + public MigrationReadiness evaluate() { + final MigrationPhase phase = MigrationPhase.current(); + final List content = new ArrayList<>(contentReconciler.statuses()); + final List siteSearch = new ArrayList<>(siteSearchReconciler.statuses()); + + final List all = new ArrayList<>(content.size() + siteSearch.size()); + all.addAll(content); + all.addAll(siteSearch); + + final List outOfSync = all.stream() + .filter(MirrorStatus::needsAttention) + .collect(Collectors.toList()); + // A downgrade routes reads back to Elasticsearch; any index whose ES copy is missing or behind + // its OpenSearch twin would lose that delta after the downgrade (a failed ES count is -1, which + // is < any real OS count → flagged, fail-safe). + final boolean esBehindAnywhere = all.stream() + .anyMatch(s -> !s.esExists() || s.esDocCount() < s.osDocCount()); + + final boolean safeToAdvance; + final String summary; + final List blockers = new ArrayList<>(); + + if (phase.isMigrationNotStarted()) { + safeToAdvance = true; + summary = "Phase 0 (Elasticsearch only). OpenSearch twins are built during the dual-write " + + "phases, so there is nothing to reconcile yet. Safe to advance to Phase 1."; + } else if (phase.isMigrationComplete()) { + safeToAdvance = true; // no phase beyond 3 + summary = "Phase 3 (OpenSearch only) — the final phase, nothing to advance to. " + + (esBehindAnywhere + ? "WARNING: OpenSearch holds content Elasticsearch does not; a downgrade would " + + "hide it until a full reindex." + : "No index shows Elasticsearch behind OpenSearch; still verify before any " + + "downgrade."); + } else { + safeToAdvance = outOfSync.isEmpty(); + for (final MirrorStatus s : outOfSync) { + blockers.add(String.format("%s '%s': %s", s.kind(), s.indexName(), s.recommendation())); + } + summary = safeToAdvance + ? "All mirrors are in sync. Safe to advance toward the OpenSearch-only phase." + : String.format("%d index(es) out of sync. Re-crawl/reindex them before promoting " + + "the phase — Phase 3 reads OpenSearch with no Elasticsearch fallback.", + outOfSync.size()); + } + + final MigrationReadiness.PhaseInfo phaseInfo = new MigrationReadiness.PhaseInfo( + phase.ordinal(), phase.name(), readEngine(phase), writeEngines(phase), + phase.isDualWrite()); + final MigrationReadiness.Verdict verdict = new MigrationReadiness.Verdict( + safeToAdvance, !esBehindAnywhere, outOfSync.size(), summary, blockers); + return new MigrationReadiness(phaseInfo, verdict, content, siteSearch); + } + + private static String readEngine(final MigrationPhase phase) { + return phase.isReadEnabled() ? "OpenSearch" : "Elasticsearch"; + } + + private static List writeEngines(final MigrationPhase phase) { + if (phase.isMigrationNotStarted()) { + return List.of("Elasticsearch"); + } + if (phase.isMigrationComplete()) { + return List.of("OpenSearch"); + } + return List.of("Elasticsearch", "OpenSearch"); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java new file mode 100644 index 000000000000..9f5d7db360b5 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -0,0 +1,68 @@ +package com.dotcms.content.index.migration; + +/** + * Per-index ES↔OS mirror status for the migration-readiness report (issue #36360): how one logical + * index compares against its twin across the two engines. Purely factual — the phase-aware + * "is this a blocker for changing phase" interpretation is layered on top by + * {@link MigrationReadinessService}. + * + *

Shared by every index family that is mirrored during the migration: the versioned content + * indices (working/live) and the Site Search indices. The {@link IndexKind} says which family this + * row belongs to.

+ * + * @param indexName the logical index name (no {@code .os} tag) + * @param kind which mirrored index family this row belongs to + * @param esExists whether the Elasticsearch copy exists + * @param esDocCount exact document count in the Elasticsearch copy (0 when absent, -1 when the + * count query failed) + * @param osExists whether the OpenSearch ({@code .os}) twin exists + * @param osDocCount exact document count in the OpenSearch twin (0 when absent, -1 when the count + * query failed) + * @param verdict the diff verdict between the two copies + * @param recommendation human-readable, action-oriented advice for a support technician + */ +public record MirrorStatus( + String indexName, + IndexKind kind, + boolean esExists, + long esDocCount, + boolean osExists, + long osDocCount, + Verdict verdict, + String recommendation) { + + /** Which mirrored index family a status row belongs to. */ + public enum IndexKind { CONTENT_WORKING, CONTENT_LIVE, SITE_SEARCH } + + /** The diff outcome between an index and its twin. */ + public enum Verdict { + /** Both copies exist with the same document count. */ + IN_SYNC, + /** The index exists on one engine but its twin is missing on the other. */ + MISSING_TWIN, + /** Both copies exist but hold a different number of documents. */ + COUNT_DRIFT + } + + /** Whether this index needs operator action (a re-crawl / reindex) before the phase change. */ + public boolean needsAttention() { + return verdict != Verdict.IN_SYNC; + } + + /** + * Classifies a mirror from raw existence + exact counts: a missing copy on either engine is + * {@link Verdict#MISSING_TWIN}; both present with unequal counts is {@link Verdict#COUNT_DRIFT} + * (a failed count is reported as {@code -1}, which compares unequal and so surfaces as drift — + * fail-safe); otherwise {@link Verdict#IN_SYNC}. + */ + public static Verdict verdictFor(final boolean esExists, final boolean osExists, + final long esDocCount, final long osDocCount) { + if (!esExists || !osExists) { + return Verdict.MISSING_TWIN; + } + if (esDocCount != osDocCount) { + return Verdict.COUNT_DRIFT; + } + return Verdict.IN_SYNC; + } +} diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java new file mode 100644 index 000000000000..aa39f848f4a8 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java @@ -0,0 +1,91 @@ +package com.dotcms.content.index.migration; + +import com.dotcms.cdi.CDIUtils; +import com.dotcms.content.index.IndexConfigHelper; +import com.dotcms.content.index.migration.MirrorStatus.IndexKind; +import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotcms.enterprise.publishing.sitesearch.ESSiteSearchAPI; +import com.dotcms.enterprise.publishing.sitesearch.OSSiteSearchAPI; +import com.dotmarketing.sitesearch.business.SiteSearchAPI; +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.List; +import java.util.TreeSet; + +/** + * Site Search half of the migration-readiness report (issue #36360): compares every logical + * site-search index against its {@code .os} twin across both engines and produces a factual + * {@link MirrorStatus} per index — does the ES copy exist, does the OpenSearch twin exist, do their + * exact document counts match — plus a re-crawl recommendation. Never mutates anything. + * + *

It queries the two engine leaves directly (ES the plain name, OpenSearch the {@code .os} twin) + * rather than the phase-aware router, so the report shows both sides regardless of which + * engine the current phase reads from. Counts come from {@link SiteSearchAPI#documentCount(String)} + * — an exact total, not a search hit-count capped at 10,000 — so content drift on large indices is + * detected (issue #36360).

+ */ +public class SiteSearchMirrorReconciler { + + private final SiteSearchAPI esImpl; + private final SiteSearchAPI osImpl; + + public SiteSearchMirrorReconciler() { + this(new ESSiteSearchAPI(), CDIUtils.getBeanThrows(OSSiteSearchAPI.class)); + } + + @VisibleForTesting + SiteSearchMirrorReconciler(final SiteSearchAPI esImpl, final SiteSearchAPI osImpl) { + this.esImpl = esImpl; + this.osImpl = osImpl; + } + + /** + * Whether a cross-engine mirror comparison is meaningful for a forward phase change in the + * current phase. Only the dual-write phases (1 and 2) keep both engines populated as write + * providers; Phase 0 (ES only) and Phase 3 (OS only) have a single write engine, so a "missing + * twin" there is either expected (0) or unfixable in-phase (3). + */ + public boolean canEvaluate() { + return IndexConfigHelper.MigrationPhase.current().isDualWrite(); + } + + /** + * The per-index mirror status for every logical site-search index that exists on either + * engine (so a twin missing on one side still appears). Purely factual and phase-independent. + */ + public List statuses() { + final TreeSet names = new TreeSet<>(esImpl.listIndices()); + names.addAll(osImpl.listIndices()); + final List statuses = new ArrayList<>(names.size()); + for (final String name : names) { + statuses.add(statusFor(name)); + } + return statuses; + } + + private MirrorStatus statusFor(final String name) { + final boolean esExists = esImpl.existsOnAllWriteEngines(name); + final boolean osExists = osImpl.existsOnAllWriteEngines(name); + final long esCount = esExists ? esImpl.documentCount(name) : 0L; + final long osCount = osExists ? osImpl.documentCount(name) : 0L; + final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); + return new MirrorStatus(name, IndexKind.SITE_SEARCH, esExists, esCount, osExists, osCount, + verdict, recommend(name, verdict)); + } + + private static String recommend(final String name, final Verdict verdict) { + switch (verdict) { + case IN_SYNC: + return "In sync — no action needed."; + case MISSING_TWIN: + return String.format("A copy of site-search index '%s' is missing on one engine. " + + "Re-crawl it (Site Search → Run now) to rebuild the twin before " + + "promoting to the OpenSearch-only phase.", name); + case COUNT_DRIFT: + default: + return String.format("The two copies of site-search index '%s' hold a different " + + "number of documents. Re-crawl it (Site Search → Run now) to rebuild " + + "the twin before promoting the phase.", name); + } + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java new file mode 100644 index 000000000000..c1eca10b9838 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java @@ -0,0 +1,108 @@ +package com.dotcms.rest.api.v1.index; + +import com.dotcms.content.index.MigrationIndexVisibility; +import com.dotcms.content.index.migration.MigrationReadinessService; +import com.dotcms.rest.InitDataObject; +import com.dotcms.rest.ResponseEntityView; +import com.dotcms.rest.WebResource; +import com.dotcms.rest.annotation.NoCache; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.util.Config; +import com.dotmarketing.util.UtilMethods; +import com.google.common.annotations.VisibleForTesting; +import com.liferay.portal.model.User; +import io.swagger.v3.oas.annotations.Hidden; +import io.vavr.control.Try; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.ForbiddenException; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.glassfish.jersey.server.JSONP; + +/** + * Internal, role-gated ES→OS migration-readiness endpoint (issue #36360). It condenses the migration + * status a support technician needs before changing the phase: the current phase and its + * read/write engines, the per-index ES↔OS mirror diff for both mirrored families (content and Site + * Search) with missing-twin / count-drift verdicts and re-crawl/reindex recommendations, and an + * overall safe-to-advance / safe-to-rollback verdict. Read-only; it never mutates any index. + * + *

Not public. The class is {@link Hidden} so it never appears in the OpenAPI / + * API-playground schema, and every method requires a backend user who is a CMS administrator or a + * member of the migration support role + * ({@value com.dotcms.content.index.MigrationIndexVisibility#VISIBILITY_ROLE_KEY}, default + * {@value com.dotcms.content.index.MigrationIndexVisibility#DEFAULT_VISIBILITY_ROLE_KEY}); anyone + * else gets a 403.

+ */ +@Path("/v1/index/migration") +@Hidden +public class MigrationReadinessResource { + + private final MigrationReadinessService readinessService; + + public MigrationReadinessResource() { + this(new MigrationReadinessService()); + } + + @VisibleForTesting + MigrationReadinessResource(final MigrationReadinessService readinessService) { + this.readinessService = readinessService; + } + + /** + * Returns the migration-readiness report for the current phase. Requires a CMS administrator or a + * member of the configured migration support role. + */ + @GET + @JSONP + @NoCache + @Hidden + @Path("/readiness") + @Produces({MediaType.APPLICATION_JSON, "application/javascript"}) + public Response readiness(@Context final HttpServletRequest request, + @Context final HttpServletResponse response) { + + final InitDataObject initData = new WebResource.InitBuilder(request, response) + .requiredBackendUser(true) + .init(); + + final User user = initData.getUser(); + if (!isMigrationSupportUser(user)) { + throw new ForbiddenException( + "Migration readiness is restricted to CMS administrators and the migration " + + "support role."); + } + + return Response.ok(new ResponseEntityView<>(readinessService.evaluate())).build(); + } + + /** + * Whether {@code user} may read the migration-readiness report: a CMS administrator, or a member + * of the configured support role (same config key as the {@code .os} visibility policy). Unlike + * {@link MigrationIndexVisibility#canSeeMigrationIndices(User)} this does not open up to + * everyone in Phase 3 — the report is an internal support tool in every phase. + */ + private static boolean isMigrationSupportUser(final User user) { + if (user == null) { + return false; + } + return Try.of(() -> { + if (APILocator.getUserAPI().isCMSAdmin(user)) { + return true; + } + final String roleKey = Config.getStringProperty( + MigrationIndexVisibility.VISIBILITY_ROLE_KEY, + MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY); + if (!UtilMethods.isSet(roleKey)) { + return false; + } + final Role role = APILocator.getRoleAPI().loadRoleByKey(roleKey); + return role != null && APILocator.getRoleAPI().doesUserHaveRole(user, role); + }).getOrElse(false); + } +} diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java new file mode 100644 index 000000000000..b943088acf30 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -0,0 +1,152 @@ +package com.dotcms.content.index.migration; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.dotcms.UnitTestBase; +import com.dotcms.content.index.IndexConfigHelper; +import com.dotcms.content.index.migration.MirrorStatus.IndexKind; +import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotmarketing.util.Config; +import java.util.List; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit tests for {@link MigrationReadinessService} — the phase-aware verdict composed from the two + * mirror reconcilers (issue #36360). Both reconcilers are mocked, so no live cluster is needed; the + * phase is driven through {@code Config}. + */ +public class MigrationReadinessServiceTest extends UnitTestBase { + + private static final int PHASE_0 = 0; + private static final int PHASE_1 = 1; + private static final int PHASE_2 = 2; + private static final int PHASE_3 = 3; + + private String previousPhase; + private SiteSearchMirrorReconciler siteSearch; + private ContentIndexMirrorReconciler content; + private MigrationReadinessService service; + + @Before + public void setUp() { + previousPhase = Config.getStringProperty(IndexConfigHelper.MigrationPhase.FLAG_KEY, null); + siteSearch = mock(SiteSearchMirrorReconciler.class); + content = mock(ContentIndexMirrorReconciler.class); + when(content.statuses()).thenReturn(List.of()); + service = new MigrationReadinessService(siteSearch, content); + } + + @After + public void tearDown() { + Config.setProperty(IndexConfigHelper.MigrationPhase.FLAG_KEY, previousPhase); + } + + private static void setPhase(final int ordinal) { + Config.setProperty(IndexConfigHelper.MigrationPhase.FLAG_KEY, String.valueOf(ordinal)); + } + + private static MirrorStatus ss(final String name, final boolean esExists, final long esCount, + final boolean osExists, final long osCount) { + final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); + return new MirrorStatus(name, IndexKind.SITE_SEARCH, esExists, esCount, osExists, osCount, + verdict, "advice"); + } + + /** Dual-write phase with every mirror in sync → safe to advance, nothing out of sync. */ + @Test + public void dualWrite_allInSync_safeToAdvance() { + setPhase(PHASE_1); + when(siteSearch.statuses()).thenReturn(List.of(ss("a", true, 100, true, 100))); + + final MigrationReadiness r = service.evaluate(); + + assertTrue(r.phase().evaluable()); + assertEquals("Elasticsearch", r.phase().readEngine()); + assertTrue(r.verdict().safeToAdvance()); + assertEquals(0, r.verdict().outOfSyncCount()); + assertTrue(r.verdict().blockers().isEmpty()); + } + + /** Dual-write with a missing twin → NOT safe to advance, one blocker, count reported. */ + @Test + public void dualWrite_missingTwin_blocksAdvance() { + setPhase(PHASE_2); + when(siteSearch.statuses()).thenReturn(List.of( + ss("a", true, 100, true, 100), + ss("b", true, 50, false, 0))); // OS twin missing + + final MigrationReadiness r = service.evaluate(); + + assertEquals("OpenSearch", r.phase().readEngine()); + assertFalse(r.verdict().safeToAdvance()); + assertEquals(1, r.verdict().outOfSyncCount()); + assertEquals(1, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().get(0).contains("'b'")); + } + + /** Count drift above 10k is caught (the reconciler feeds exact counts) → blocks advance. */ + @Test + public void dualWrite_countDriftAbove10k_blocksAdvance() { + setPhase(PHASE_2); + when(siteSearch.statuses()).thenReturn(List.of(ss("big", true, 15_000, true, 12_000))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertEquals(1, r.verdict().outOfSyncCount()); + } + + /** OpenSearch ahead of Elasticsearch → a downgrade would lose that delta → not safe to rollback. */ + @Test + public void osAheadOfEs_notSafeToRollback() { + setPhase(PHASE_2); + when(siteSearch.statuses()).thenReturn(List.of(ss("a", true, 80, true, 100))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToRollback()); + } + + /** Mirrors even → safe to rollback. */ + @Test + public void mirrorsEven_safeToRollback() { + setPhase(PHASE_2); + when(siteSearch.statuses()).thenReturn(List.of(ss("a", true, 100, true, 100))); + + final MigrationReadiness r = service.evaluate(); + + assertTrue(r.verdict().safeToRollback()); + } + + /** Phase 0: not evaluable for a forward comparison, but advancing to dual-write is safe. */ + @Test + public void phase0_notEvaluable_safeToAdvance() { + setPhase(PHASE_0); + when(siteSearch.statuses()).thenReturn(List.of()); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.phase().evaluable()); + assertEquals(List.of("Elasticsearch"), r.phase().writeEngines()); + assertTrue(r.verdict().safeToAdvance()); + } + + /** Phase 3: not evaluable; write engine is OpenSearch only. */ + @Test + public void phase3_notEvaluable_openSearchOnly() { + setPhase(PHASE_3); + when(siteSearch.statuses()).thenReturn(List.of(ss("a", true, 100, true, 100))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.phase().evaluable()); + assertEquals("OpenSearch", r.phase().readEngine()); + assertEquals(List.of("OpenSearch"), r.phase().writeEngines()); + } +} From 35fbb4372029a7defb4343da1c5303b8022e348d Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 10:53:14 -0600 Subject: [PATCH 02/11] feat(migration): content-index readiness half + drop "twin" wording (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step b of the migration-readiness endpoint (#36360): - ContentIndexMirrorReconciler now real (was a stub): compares the active working and live content indices against their .os counterparts across both engines, phase-independently. IndiciesInfo holds the cluster-prefixed, un-tagged ES name; exact per-engine counts come from each leaf's getIndicesStats() (_stats primaries.docs.count, not the 10k-capped search total), keyed by the cluster-stripped name (ES bare, OS with .os) — strip-then-tag to match. Emits CONTENT_WORKING / CONTENT_LIVE rows with the same missing-counterpart / count-drift verdicts and a reindex recommendation. Reads leaves directly, never the router. - Terminology: renamed "twin" -> "counterpart" across the new readiness code (verdict MISSING_TWIN -> MISSING_COUNTERPART); "mirror" (the feature name) kept. Unit tests: ContentIndexMirrorReconcilerTest (mocked leaves + injected IndiciesInfo) 5/5 — in-sync, missing OS counterpart, count drift, null/absent slots; service test still 7/7. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ContentIndexMirrorReconciler.java | 117 +++++++++++++-- .../index/migration/MigrationReadiness.java | 4 +- .../migration/MigrationReadinessService.java | 8 +- .../content/index/migration/MirrorStatus.java | 16 +- .../migration/SiteSearchMirrorReconciler.java | 16 +- .../v1/index/MigrationReadinessResource.java | 2 +- .../ContentIndexMirrorReconcilerTest.java | 138 ++++++++++++++++++ .../MigrationReadinessServiceTest.java | 6 +- 8 files changed, 269 insertions(+), 38 deletions(-) create mode 100644 dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java index 5277c2b95595..75e60ac9d2cb 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java @@ -1,24 +1,117 @@ package com.dotcms.content.index.migration; -import java.util.Collections; +import com.dotcms.cdi.CDIUtils; +import com.dotcms.content.elasticsearch.business.ESIndexAPI; +import com.dotcms.content.elasticsearch.business.IndiciesInfo; +import com.dotcms.content.index.IndexAPI; +import com.dotcms.content.index.IndexTag; +import com.dotcms.content.index.domain.IndexStats; +import com.dotcms.content.index.migration.MirrorStatus.IndexKind; +import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotcms.content.index.opensearch.OSIndexAPIImpl; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.util.Logger; +import com.dotmarketing.util.UtilMethods; +import com.google.common.annotations.VisibleForTesting; +import io.vavr.control.Try; +import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.function.Supplier; /** - * Content-index half of the migration-readiness report (issue #36360): compares the versioned - * content indices (working/live) against their {@code .os} twins across both engines, mirroring - * {@link SiteSearchMirrorReconciler} but for the content store. + * Content-index half of the migration-readiness report (issue #36360): compares the active versioned + * content indices (working and live) against their {@code .os} counterparts across both engines, + * mirroring {@link SiteSearchMirrorReconciler} but for the content store. Never mutates anything. * - *

Work in progress (PR3, step b). The framework and the Site Search half are in - * place; this half is stubbed to return no rows so {@link MigrationReadinessService} composes both - * sections without special-casing. It will enumerate working/live from {@code IndiciesInfo} and read - * exact per-engine document counts (index stats, not a capped search total) for each twin — - * the same drift/missing-twin verdicts as the Site Search half, emitted as - * {@link MirrorStatus.IndexKind#CONTENT_WORKING} / {@link MirrorStatus.IndexKind#CONTENT_LIVE}.

+ *

How the counts are read (phase-independently)

+ *

{@code IndiciesInfo} always holds the cluster-prefixed, un-tagged Elasticsearch name for + * working/live (its backing {@code indicies} table owns only the ES rows — {@code index_version IS + * NULL}); the OpenSearch counterpart is that name with the {@code .os} tag. Exact per-engine document + * counts come from each engine leaf's {@code getIndicesStats()} — the index {@code _stats} + * {@code primaries.docs.count}, an exact total not subject to the 10,000 search hit-count cap. Those + * stats maps are keyed by the cluster-stripped name (Elasticsearch un-tagged, OpenSearch + * carrying {@code .os}), so each raw name is stripped of the cluster prefix and then, for the + * OpenSearch lookup, tagged — the same strip-then-tag order the maintenance JSP uses.

+ * + *

It queries the two engine leaves directly (never the phase-aware router) so the report shows both + * sides regardless of which engine the current phase reads from. Scope is the active working/live + * pair; reindex slots are out of scope for this report.

*/ public class ContentIndexMirrorReconciler { - /** Per-index mirror status for the working/live content indices. TODO(#36360 PR3 step b). */ + private final IndexAPI esImpl; + private final IndexAPI osImpl; + private final Supplier indiciesSupplier; + + public ContentIndexMirrorReconciler() { + this(new ESIndexAPI(), CDIUtils.getBeanThrows(OSIndexAPIImpl.class), + ContentIndexMirrorReconciler::loadIndiciesQuietly); + } + + @VisibleForTesting + ContentIndexMirrorReconciler(final IndexAPI esImpl, final IndexAPI osImpl, + final Supplier indiciesSupplier) { + this.esImpl = esImpl; + this.osImpl = osImpl; + this.indiciesSupplier = indiciesSupplier; + } + + /** Per-index mirror status for the active working and live content indices. */ public List statuses() { - return Collections.emptyList(); + final IndiciesInfo info = indiciesSupplier.get(); + if (info == null) { + return List.of(); + } + final Map esStats = esImpl.getIndicesStats(); + final Map osStats = osImpl.getIndicesStats(); + final List out = new ArrayList<>(2); + addStatus(out, IndexKind.CONTENT_WORKING, info.getWorking(), esStats, osStats); + addStatus(out, IndexKind.CONTENT_LIVE, info.getLive(), esStats, osStats); + return out; + } + + private void addStatus(final List out, final IndexKind kind, final String rawName, + final Map esStats, final Map osStats) { + if (!UtilMethods.isSet(rawName)) { + return; + } + // IndiciesInfo holds the cluster-prefixed, un-tagged ES name; the stats maps are keyed by the + // cluster-stripped name (ES un-tagged, OS carrying .os). Strip first, then tag for the OS key. + final String bare = esImpl.removeClusterIdFromName(rawName); + final String osKey = IndexTag.OS.tag(bare); + + final boolean esExists = esStats.containsKey(bare); + final long esCount = esExists ? esStats.get(bare).documentCount() : 0L; + final boolean osExists = osStats.containsKey(osKey); + final long osCount = osExists ? osStats.get(osKey).documentCount() : 0L; + + final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); + out.add(new MirrorStatus(bare, kind, esExists, esCount, osExists, osCount, verdict, + recommend(bare, verdict, osExists))); + } + + private static String recommend(final String name, final Verdict verdict, final boolean osExists) { + switch (verdict) { + case IN_SYNC: + return "In sync — no action needed."; + case MISSING_COUNTERPART: + final String missing = osExists ? "Elasticsearch" : "OpenSearch"; + return String.format("The %s copy of content index '%s' is missing. Run a full " + + "reindex to rebuild it before promoting to the OpenSearch-only phase.", + missing, name); + case COUNT_DRIFT: + default: + return String.format("The two copies of content index '%s' hold a different number " + + "of documents. Run a full reindex to rebuild the OpenSearch copy before " + + "promoting the phase.", name); + } + } + + private static IndiciesInfo loadIndiciesQuietly() { + return Try.of(() -> APILocator.getIndiciesAPI().loadIndicies()) + .onFailure(e -> Logger.warn(ContentIndexMirrorReconciler.class, + "Could not load content indices for migration readiness: " + e.getMessage())) + .getOrNull(); } } diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index b289a4fc5c92..861bc3c5ffbe 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -38,9 +38,9 @@ public record PhaseInfo( /** * @param safeToAdvance whether it is safe to promote toward the OpenSearch-only phase * @param safeToRollback whether it is safe to downgrade — false when any index's Elasticsearch - * copy is behind its OpenSearch twin, because a downgrade routes reads back + * copy is behind its OpenSearch counterpart, because a downgrade routes reads back * to Elasticsearch and would silently drop that delta until a reindex - * @param outOfSyncCount how many indices need attention (missing twin or count drift) + * @param outOfSyncCount how many indices need attention (missing counterpart or count drift) * @param summary one human-readable sentence describing the overall state * @param blockers per-index reasons that make advancing unsafe (empty when safe) */ diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index e555e8258049..ce01ad345835 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -16,10 +16,10 @@ *
    *
  • Advance (toward OpenSearch-only): meaningful in the dual-write phases (1/2), where it * is safe only when no index needs attention. In Phase 0 there is nothing to reconcile yet - * (twins are built during dual-write) and in Phase 3 there is no further phase — both report + * (counterparts are built during dual-write) and in Phase 3 there is no further phase — both report * safe with an explanatory summary.
    • *
  • Rollback (downgrade): a downgrade ultimately routes reads back to Elasticsearch - * (Phases 0/1), so it is unsafe when any index's ES copy is behind its OpenSearch twin — that + * (Phases 0/1), so it is unsafe when any index's ES copy is behind its OpenSearch counterpart — that * delta (typically content written while OpenSearch served reads) would be silently missing * until a full reindex. Derived from the same live counts, so no historical state is needed.
    • *
@@ -54,7 +54,7 @@ public MigrationReadiness evaluate() { .filter(MirrorStatus::needsAttention) .collect(Collectors.toList()); // A downgrade routes reads back to Elasticsearch; any index whose ES copy is missing or behind - // its OpenSearch twin would lose that delta after the downgrade (a failed ES count is -1, which + // its OpenSearch counterpart would lose that delta after the downgrade (a failed ES count is -1, which // is < any real OS count → flagged, fail-safe). final boolean esBehindAnywhere = all.stream() .anyMatch(s -> !s.esExists() || s.esDocCount() < s.osDocCount()); @@ -65,7 +65,7 @@ public MigrationReadiness evaluate() { if (phase.isMigrationNotStarted()) { safeToAdvance = true; - summary = "Phase 0 (Elasticsearch only). OpenSearch twins are built during the dual-write " + summary = "Phase 0 (Elasticsearch only). OpenSearch counterparts are built during the dual-write " + "phases, so there is nothing to reconcile yet. Safe to advance to Phase 1."; } else if (phase.isMigrationComplete()) { safeToAdvance = true; // no phase beyond 3 diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java index 9f5d7db360b5..c2c680a2fccd 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -2,7 +2,7 @@ /** * Per-index ES↔OS mirror status for the migration-readiness report (issue #36360): how one logical - * index compares against its twin across the two engines. Purely factual — the phase-aware + * index compares against its counterpart across the two engines. Purely factual — the phase-aware * "is this a blocker for changing phase" interpretation is layered on top by * {@link MigrationReadinessService}. * @@ -15,8 +15,8 @@ * @param esExists whether the Elasticsearch copy exists * @param esDocCount exact document count in the Elasticsearch copy (0 when absent, -1 when the * count query failed) - * @param osExists whether the OpenSearch ({@code .os}) twin exists - * @param osDocCount exact document count in the OpenSearch twin (0 when absent, -1 when the count + * @param osExists whether the OpenSearch ({@code .os}) counterpart exists + * @param osDocCount exact document count in the OpenSearch counterpart (0 when absent, -1 when the count * query failed) * @param verdict the diff verdict between the two copies * @param recommendation human-readable, action-oriented advice for a support technician @@ -34,12 +34,12 @@ public record MirrorStatus( /** Which mirrored index family a status row belongs to. */ public enum IndexKind { CONTENT_WORKING, CONTENT_LIVE, SITE_SEARCH } - /** The diff outcome between an index and its twin. */ + /** The diff outcome between an index and its counterpart. */ public enum Verdict { /** Both copies exist with the same document count. */ IN_SYNC, - /** The index exists on one engine but its twin is missing on the other. */ - MISSING_TWIN, + /** The index exists on one engine but its counterpart is missing on the other. */ + MISSING_COUNTERPART, /** Both copies exist but hold a different number of documents. */ COUNT_DRIFT } @@ -51,14 +51,14 @@ public boolean needsAttention() { /** * Classifies a mirror from raw existence + exact counts: a missing copy on either engine is - * {@link Verdict#MISSING_TWIN}; both present with unequal counts is {@link Verdict#COUNT_DRIFT} + * {@link Verdict#MISSING_COUNTERPART}; both present with unequal counts is {@link Verdict#COUNT_DRIFT} * (a failed count is reported as {@code -1}, which compares unequal and so surfaces as drift — * fail-safe); otherwise {@link Verdict#IN_SYNC}. */ public static Verdict verdictFor(final boolean esExists, final boolean osExists, final long esDocCount, final long osDocCount) { if (!esExists || !osExists) { - return Verdict.MISSING_TWIN; + return Verdict.MISSING_COUNTERPART; } if (esDocCount != osDocCount) { return Verdict.COUNT_DRIFT; diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java index aa39f848f4a8..9f0e57f09bda 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java @@ -14,11 +14,11 @@ /** * Site Search half of the migration-readiness report (issue #36360): compares every logical - * site-search index against its {@code .os} twin across both engines and produces a factual - * {@link MirrorStatus} per index — does the ES copy exist, does the OpenSearch twin exist, do their + * site-search index against its {@code .os} counterpart across both engines and produces a factual + * {@link MirrorStatus} per index — does the ES copy exist, does the OpenSearch counterpart exist, do their * exact document counts match — plus a re-crawl recommendation. Never mutates anything. * - *

It queries the two engine leaves directly (ES the plain name, OpenSearch the {@code .os} twin) + *

It queries the two engine leaves directly (ES the plain name, OpenSearch the {@code .os} counterpart) * rather than the phase-aware router, so the report shows both sides regardless of which * engine the current phase reads from. Counts come from {@link SiteSearchAPI#documentCount(String)} * — an exact total, not a search hit-count capped at 10,000 — so content drift on large indices is @@ -43,7 +43,7 @@ public SiteSearchMirrorReconciler() { * Whether a cross-engine mirror comparison is meaningful for a forward phase change in the * current phase. Only the dual-write phases (1 and 2) keep both engines populated as write * providers; Phase 0 (ES only) and Phase 3 (OS only) have a single write engine, so a "missing - * twin" there is either expected (0) or unfixable in-phase (3). + * counterpart" there is either expected (0) or unfixable in-phase (3). */ public boolean canEvaluate() { return IndexConfigHelper.MigrationPhase.current().isDualWrite(); @@ -51,7 +51,7 @@ public boolean canEvaluate() { /** * The per-index mirror status for every logical site-search index that exists on either - * engine (so a twin missing on one side still appears). Purely factual and phase-independent. + * engine (so a counterpart missing on one side still appears). Purely factual and phase-independent. */ public List statuses() { final TreeSet names = new TreeSet<>(esImpl.listIndices()); @@ -77,15 +77,15 @@ private static String recommend(final String name, final Verdict verdict) { switch (verdict) { case IN_SYNC: return "In sync — no action needed."; - case MISSING_TWIN: + case MISSING_COUNTERPART: return String.format("A copy of site-search index '%s' is missing on one engine. " - + "Re-crawl it (Site Search → Run now) to rebuild the twin before " + + "Re-crawl it (Site Search → Run now) to rebuild the counterpart before " + "promoting to the OpenSearch-only phase.", name); case COUNT_DRIFT: default: return String.format("The two copies of site-search index '%s' hold a different " + "number of documents. Re-crawl it (Site Search → Run now) to rebuild " - + "the twin before promoting the phase.", name); + + "the counterpart before promoting the phase.", name); } } } diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java index c1eca10b9838..2a3a4b4044df 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java @@ -29,7 +29,7 @@ * Internal, role-gated ES→OS migration-readiness endpoint (issue #36360). It condenses the migration * status a support technician needs before changing the phase: the current phase and its * read/write engines, the per-index ES↔OS mirror diff for both mirrored families (content and Site - * Search) with missing-twin / count-drift verdicts and re-crawl/reindex recommendations, and an + * Search) with missing-counterpart / count-drift verdicts and re-crawl/reindex recommendations, and an * overall safe-to-advance / safe-to-rollback verdict. Read-only; it never mutates any index. * *

Not public. The class is {@link Hidden} so it never appears in the OpenAPI / diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java new file mode 100644 index 000000000000..0243e492eb7c --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java @@ -0,0 +1,138 @@ +package com.dotcms.content.index.migration; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.dotcms.UnitTestBase; +import com.dotcms.content.elasticsearch.business.IndiciesInfo; +import com.dotcms.content.index.IndexAPI; +import com.dotcms.content.index.domain.IndexStats; +import com.dotcms.content.index.migration.MirrorStatus.IndexKind; +import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import java.util.List; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit tests for {@link ContentIndexMirrorReconciler} — the content (working/live) half of the + * migration-readiness report (issue #36360). Both engine leaves are mocked and the index names are + * fed through an injected {@code IndiciesInfo}, so no live cluster is needed. The mocked ES leaf + * strips a fixed {@code cluster_x.} prefix, matching {@code removeClusterIdFromName}. + */ +public class ContentIndexMirrorReconcilerTest extends UnitTestBase { + + private static final String PREFIX = "cluster_x."; + + private IndexAPI es; + private IndexAPI os; + + @Before + public void setUp() { + es = mock(IndexAPI.class); + os = mock(IndexAPI.class); + when(es.removeClusterIdFromName(anyString())).thenAnswer(inv -> { + final String n = inv.getArgument(0); + return n.startsWith(PREFIX) ? n.substring(PREFIX.length()) : n; + }); + } + + private static IndexStats stats(final long count) { + final IndexStats s = mock(IndexStats.class); + when(s.documentCount()).thenReturn(count); + return s; + } + + private static IndiciesInfo indicies(final String working, final String live) { + return new IndiciesInfo.Builder().setWorking(working).setLive(live).build(); + } + + private ContentIndexMirrorReconciler reconciler(final IndiciesInfo info) { + return new ContentIndexMirrorReconciler(es, os, () -> info); + } + + /** Both content indices present on both engines with equal counts → two IN_SYNC rows. */ + @Test + public void workingAndLive_inSync() { + // Build the stats maps first: nesting stats() (a when()) inside a when().thenReturn(...) would + // trip Mockito's UnfinishedStubbingException. + final Map esStats = Map.of("working_1", stats(100), "live_1", stats(50)); + final Map osStats = Map.of("working_1.os", stats(100), "live_1.os", stats(50)); + when(es.getIndicesStats()).thenReturn(esStats); + when(os.getIndicesStats()).thenReturn(osStats); + + final List statuses = + reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).statuses(); + + assertEquals(2, statuses.size()); + final MirrorStatus working = statuses.get(0); + assertEquals(IndexKind.CONTENT_WORKING, working.kind()); + assertEquals("working_1", working.indexName()); // logical: cluster prefix stripped, no .os + assertEquals(Verdict.IN_SYNC, working.verdict()); + assertEquals(100, working.esDocCount()); + assertEquals(100, working.osDocCount()); + assertEquals(IndexKind.CONTENT_LIVE, statuses.get(1).kind()); + assertEquals(Verdict.IN_SYNC, statuses.get(1).verdict()); + } + + /** The OpenSearch counterpart of the working index is missing → MISSING_COUNTERPART. */ + @Test + public void missingOsCounterpart_onWorking() { + final Map esStats = Map.of("working_1", stats(100), "live_1", stats(50)); + final Map osStats = Map.of("live_1.os", stats(50)); // working_1.os absent + when(es.getIndicesStats()).thenReturn(esStats); + when(os.getIndicesStats()).thenReturn(osStats); + + final List statuses = + reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).statuses(); + + final MirrorStatus working = statuses.get(0); + assertEquals(Verdict.MISSING_COUNTERPART, working.verdict()); + assertTrue(working.esExists()); + assertFalse(working.osExists()); + assertTrue(working.needsAttention()); + assertTrue(working.recommendation().contains("OpenSearch")); + } + + /** Counts diverge on the live index (exact stats, no cap) → COUNT_DRIFT. */ + @Test + public void countDrift_onLive() { + final Map esStats = Map.of("working_1", stats(100), "live_1", stats(50)); + final Map osStats = Map.of("working_1.os", stats(100), "live_1.os", stats(40)); + when(es.getIndicesStats()).thenReturn(esStats); + when(os.getIndicesStats()).thenReturn(osStats); + + final List statuses = + reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).statuses(); + + final MirrorStatus live = statuses.get(1); + assertEquals(IndexKind.CONTENT_LIVE, live.kind()); + assertEquals(Verdict.COUNT_DRIFT, live.verdict()); + assertEquals(50, live.esDocCount()); + assertEquals(40, live.osDocCount()); + } + + /** A null IndiciesInfo (could not be loaded) yields no rows rather than throwing. */ + @Test + public void nullIndicies_emptyList() { + assertTrue(reconciler(null).statuses().isEmpty()); + } + + /** An unset working/live slot is skipped (no row, no NPE). */ + @Test + public void unsetSlot_skipped() { + final Map esStats = Map.of("live_1", stats(50)); + final Map osStats = Map.of("live_1.os", stats(50)); + when(es.getIndicesStats()).thenReturn(esStats); + when(os.getIndicesStats()).thenReturn(osStats); + + final List statuses = reconciler(indicies(null, PREFIX + "live_1")).statuses(); + + assertEquals(1, statuses.size()); + assertEquals(IndexKind.CONTENT_LIVE, statuses.get(0).kind()); + } +} diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index b943088acf30..8db4414a9da1 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -73,13 +73,13 @@ public void dualWrite_allInSync_safeToAdvance() { assertTrue(r.verdict().blockers().isEmpty()); } - /** Dual-write with a missing twin → NOT safe to advance, one blocker, count reported. */ + /** Dual-write with a missing counterpart → NOT safe to advance, one blocker, count reported. */ @Test - public void dualWrite_missingTwin_blocksAdvance() { + public void dualWrite_missingCounterpart_blocksAdvance() { setPhase(PHASE_2); when(siteSearch.statuses()).thenReturn(List.of( ss("a", true, 100, true, 100), - ss("b", true, 50, false, 0))); // OS twin missing + ss("b", true, 50, false, 0))); // OS counterpart missing final MigrationReadiness r = service.evaluate(); From 065b2efd27f2f1f53d5f5c51e485df741d09603e Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 11:06:49 -0600 Subject: [PATCH 03/11] feat(migration): phase-only index portlet visibility + readiness gate test + docs (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step c of the migration-readiness endpoint (#36360): - Teardown of the role-gated .os reveal in the index portlets (reverts I-4). MigrationIndexVisibility is now purely phase-based: .os indices are hidden in Phases 0/1/2 and shown in Phase 3, for EVERYONE — regular admins never learn a migration is running. The role key is retained only to gate the readiness endpoint, which is now the single source of truth for migration/QA. Both display sinks (IndexResourceHelper.indexStatsList, cmsmaintenance/index_stats.jsp) drop the user argument; ESIndexResource updated accordingly. - Gate coverage: MigrationReadinessResource.isMigrationSupportUser made package-private and unit-tested (CMS admin / role member allowed, non-admin-without-role denied, null user denied, access-lookup failure fails closed) — 5/5. - MigrationIndexVisibilityTest rewritten to the phase-only contract — 5/5. - OPENSEARCH_MIGRATION.md: new "Migration-readiness endpoint" subsection (route, @Hidden + role gate, what it reports, exact counts, stateless rollback verdict) and note that the portlets no longer reveal .os by role. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 38 +++- .../index/MigrationIndexVisibility.java | 71 +++---- .../rest/api/v1/index/ESIndexResource.java | 2 +- .../v1/index/MigrationReadinessResource.java | 9 +- .../common/reindex/IndexResourceHelper.java | 13 +- .../ext/cmsmaintenance/index_stats.jsp | 9 +- .../index/MigrationIndexVisibilityTest.java | 193 +++--------------- .../index/MigrationReadinessResourceTest.java | 94 +++++++++ 8 files changed, 198 insertions(+), 231 deletions(-) create mode 100644 dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index 366aeecad2dc..3a861a75cce5 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -353,10 +353,40 @@ closed when a crawl actually runs in a dual-write phase (1 or 2). In the window twin that was never rebuilt, and never crawling, is a hard gap. **Operational rule (pairs with the code):** before promoting the phase — especially into Phase 3 — -ensure every Site Search index has been crawled at least once so its OS twin exists and is in sync. A -targeted verify/repair job (detect twins that are missing **or** whose counts diverge → rebuild full) -that closes the no-crawl window without depending on a crawl is deferred as a follow-up under the same -issue. +ensure every Site Search index has been crawled at least once so its OS counterpart exists and is in +sync. The migration-readiness endpoint below is what tells the operator *which* indices still need +that crawl, before they change the phase. + +#### Migration-readiness endpoint (pre-phase-change advisory) + +`GET /api/v1/index/migration/readiness` is an internal, read-only report a support technician runs +**before changing the migration phase** to see whether it is safe and, if not, what to do. It never +mutates anything — the fix is always the operator re-running the crawl / reindex, which self-heals +through the write-path gate above. + +- **Not public.** The resource is `@Hidden` (absent from the OpenAPI / API-playground schema) and + gated to CMS administrators or members of the migration support role + (`OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY`, default `os_migration_qa`); anyone else gets a 403. +- **What it reports.** The current phase with its read/write engines and an `evaluable` flag; an + overall verdict — `safeToAdvance` (toward OpenSearch-only) and `safeToRollback` (downgrade) with an + `outOfSyncCount`, a human `summary`, and per-index `blockers`; and the per-index ES↔OS mirror diff + for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site + Search indices. Each row carries `{esExists, esDocCount, osExists, osDocCount, verdict, + recommendation}` with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. +- **Stateless, from live counts.** Every field is derived at request time. Counts are **exact** — the + Site Search half uses `SiteSearchAPI.documentCount` and the content half reads each engine leaf's + `getIndicesStats()` (index `_stats` `primaries.docs.count`), never a search total (which the ES/OS + clients cap at 10,000 and would hide drift on large indices). Both reconcilers query the two engine + leaves directly, not the phase-aware router, so the report shows both sides in every phase. +- **`safeToRollback` needs no history.** A downgrade routes reads back to Elasticsearch, so it is + unsafe when any index's ES copy is behind its OpenSearch counterpart (`esDocCount < osDocCount`, or + the ES copy missing) — that delta, typically content written while OpenSearch served reads, would be + silently absent after the downgrade until a full reindex. That is derivable from the same snapshot, + so no per-phase state is persisted. + +Because this endpoint is the source of truth for migration/QA, the index portlets no longer reveal +`.os` indices by role: `MigrationIndexVisibility` is now purely phase-based (hidden in Phases 0/1/2, +shown in Phase 3, for everyone). The role key is retained only to gate this endpoint. #### Tag manipulation is the sole responsibility of `IndexTag` diff --git a/dotCMS/src/main/java/com/dotcms/content/index/MigrationIndexVisibility.java b/dotCMS/src/main/java/com/dotcms/content/index/MigrationIndexVisibility.java index 1d2672c2b6a1..d806e5a1b353 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/MigrationIndexVisibility.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/MigrationIndexVisibility.java @@ -1,12 +1,6 @@ package com.dotcms.content.index; import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; -import com.dotmarketing.business.APILocator; -import com.dotmarketing.business.Role; -import com.dotmarketing.util.Config; -import com.dotmarketing.util.UtilMethods; -import com.liferay.portal.model.User; -import io.vavr.control.Try; import java.util.List; import java.util.stream.Collectors; @@ -21,20 +15,20 @@ * (optimize-all, flush-all, {@code indexExists} validation, bulk fix). Filtering inside those * methods would silently skip OS indices for those operations in phases 1/2 — a behavioural * change disguised as a UI tweak. The complete, phase-correct set must stay intact at the API; - * only the two display sinks (the maintenance JSP and {@code IndexResourceHelper.indexStatsList}) + * only the display sinks (the maintenance JSP and {@code IndexResourceHelper.indexStatsList}) * apply this filter, and only those — see {@code docs/backend/OPENSEARCH_MIGRATION.md}.

* - *

Rule

+ *

Rule — phase-based, for everyone

*
    *
  • Phase 3 (OS-only): OS is the live store, so {@code .os} indices are always visible.
    • - *
  • Phases 0/1/2: {@code .os} indices are a migration/uniqueness artifact and are hidden, - * unless the acting user holds the configured QA/preview role - * ({@value #VISIBILITY_ROLE_KEY}, default {@value #DEFAULT_VISIBILITY_ROLE_KEY}).
    • + *
  • Phases 0/1/2: {@code .os} indices are a migration/uniqueness artifact and are hidden + * from every user — regular admins never learn a migration is running.
    • *
* - *

The acting {@link User} is supplied explicitly by each display sink (both are authenticated - * admin requests where the user is always available) — never resolved from a thread-local inside - * this policy, so it is safe to unit-test and free of request-context coupling.

+ *

The role-gated preview of {@code .os} indices was removed in issue #36360: support and QA now + * get migration detail from the dedicated, role-gated migration-readiness endpoint + * ({@code /api/v1/index/migration/readiness}), which is the single source of truth. This display + * policy is therefore purely phase-based and consults no user or role.

* *

OS-origin detection always goes through {@link IndexTag#isTagged(String)}, never * {@code name.endsWith(".os")}, per the {@link IndexTag} contract.

@@ -42,13 +36,14 @@ public final class MigrationIndexVisibility { /** - * Config key holding the {@link Role#getRoleKey() role key} whose members may preview - * OS-tagged ({@code .os}) indices before Phase 3. Defaults to - * {@value #DEFAULT_VISIBILITY_ROLE_KEY}. + * Config key holding the {@link com.dotmarketing.business.Role#getRoleKey() role key} whose + * members may read the role-gated migration-readiness endpoint + * ({@code /api/v1/index/migration/readiness}). Defaults to {@value #DEFAULT_VISIBILITY_ROLE_KEY}. + * It no longer governs the index portlet display (which is purely phase-based since issue #36360). */ public static final String VISIBILITY_ROLE_KEY = "OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY"; - /** Default role key allowed to preview migration ({@code .os}) indices. */ + /** Default role key allowed to read the migration-readiness endpoint. */ public static final String DEFAULT_VISIBILITY_ROLE_KEY = "os_migration_qa"; private MigrationIndexVisibility() { @@ -56,44 +51,28 @@ private MigrationIndexVisibility() { } /** - * Whether {@code user} may see OS-tagged ({@code .os}) indices in the current phase. - * - * @param user the acting user; {@code null} is treated as "not allowed" outside Phase 3 - * @return {@code true} in Phase 3, or when {@code user} holds the configured QA role + * Whether OS-tagged ({@code .os}) indices are shown in the current phase — only in Phase 3, + * where OpenSearch is the live store. Before Phase 3 they are a migration artifact and stay + * hidden from everyone. */ - public static boolean canSeeMigrationIndices(final User user) { - if (MigrationPhase.current().isMigrationComplete()) { - return true; - } - if (user == null) { - return false; - } - final String roleKey = Config.getStringProperty(VISIBILITY_ROLE_KEY, - DEFAULT_VISIBILITY_ROLE_KEY); - if (!UtilMethods.isSet(roleKey)) { - return false; - } - return Try.of(() -> { - final Role role = APILocator.getRoleAPI().loadRoleByKey(roleKey); - return role != null && APILocator.getRoleAPI().doesUserHaveRole(user, role); - }).getOrElse(false); + public static boolean showMigrationIndices() { + return MigrationPhase.current().isMigrationComplete(); } /** - * Returns {@code indexNames} with OS-tagged ({@code .os}) entries removed when {@code user} - * is not allowed to see them; otherwise returns the list unchanged. + * Returns {@code indexNames} with OS-tagged ({@code .os}) entries removed outside Phase 3; + * in Phase 3 (or for a null/empty input) the list is returned unchanged. * - * @param indexNames the full, phase-correct list of index names; {@code null}/empty is - * returned as-is - * @param user the acting user + * @param indexNames the full, phase-correct list of index names; {@code null}/empty is returned + * as-is * @return a filtered copy, or the original list when no filtering applies */ - public static List filter(final List indexNames, final User user) { - if (indexNames == null || indexNames.isEmpty() || canSeeMigrationIndices(user)) { + public static List filter(final List indexNames) { + if (indexNames == null || indexNames.isEmpty() || showMigrationIndices()) { return indexNames; } return indexNames.stream() .filter(name -> !IndexTag.OS.isTagged(name)) .collect(Collectors.toList()); } -} \ No newline at end of file +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java index 35700822adb7..c9bf0b08db87 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java @@ -644,7 +644,7 @@ public Response getIndexStatus(@Context final HttpServletRequest request, final InitDataObject init = auth(request, response); return Response.ok(new ResponseEntityView<>( - IndexResourceHelper.getInstance().indexStatsList(init.getUser()))).build(); + IndexResourceHelper.getInstance().indexStatsList())).build(); } diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java index 2a3a4b4044df..024749550b63 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java @@ -83,11 +83,12 @@ public Response readiness(@Context final HttpServletRequest request, /** * Whether {@code user} may read the migration-readiness report: a CMS administrator, or a member - * of the configured support role (same config key as the {@code .os} visibility policy). Unlike - * {@link MigrationIndexVisibility#canSeeMigrationIndices(User)} this does not open up to - * everyone in Phase 3 — the report is an internal support tool in every phase. + * of the configured support role ({@link MigrationIndexVisibility#VISIBILITY_ROLE_KEY}, default + * {@link MigrationIndexVisibility#DEFAULT_VISIBILITY_ROLE_KEY}). This is an internal support tool + * in every phase — it never opens up to everyone, unlike the phase-based index portlet display. */ - private static boolean isMigrationSupportUser(final User user) { + @VisibleForTesting + static boolean isMigrationSupportUser(final User user) { if (user == null) { return false; } diff --git a/dotCMS/src/main/java/com/dotmarketing/common/reindex/IndexResourceHelper.java b/dotCMS/src/main/java/com/dotmarketing/common/reindex/IndexResourceHelper.java index 40f8117bdbaa..7c4af7c4e075 100644 --- a/dotCMS/src/main/java/com/dotmarketing/common/reindex/IndexResourceHelper.java +++ b/dotCMS/src/main/java/com/dotmarketing/common/reindex/IndexResourceHelper.java @@ -13,7 +13,6 @@ import com.dotcms.content.index.domain.IndexStats; import com.dotmarketing.business.APILocator; import com.google.common.collect.ImmutableList; -import com.liferay.portal.model.User; import io.vavr.control.Try; @@ -37,15 +36,15 @@ public static IndexResourceHelper getInstance() { - public List> indexStatsList(final User user) { + public List> indexStatsList() { Map clusterHealth = esapi.getClusterHealth(); - // Hide OS-tagged (.os) migration indices from the maintenance dashboard outside Phase 3, - // unless the acting user holds the configured QA/preview role. Operational paths keep the - // full set; only this display sink filters — see MigrationIndexVisibility. - List openIndices=MigrationIndexVisibility.filter(idxApi.listDotCMSIndices(), user); - List closedIndices=MigrationIndexVisibility.filter(idxApi.listDotCMSClosedIndices(), user); + // Hide OS-tagged (.os) migration indices from the maintenance dashboard outside Phase 3 + // (phase-based, for everyone). Operational paths keep the full set; only this display sink + // filters — see MigrationIndexVisibility. + List openIndices=MigrationIndexVisibility.filter(idxApi.listDotCMSIndices()); + List closedIndices=MigrationIndexVisibility.filter(idxApi.listDotCMSClosedIndices()); List currentIdx = Try.of(()->idxApi.getCurrentIndex()).getOrElse(ImmutableList.of()); List newIdx =Try.of(()->idxApi.getNewIndex()).getOrElse(ImmutableList.of()); Map indexInfo = esapi.getIndicesStats(); diff --git a/dotCMS/src/main/webapp/html/portlet/ext/cmsmaintenance/index_stats.jsp b/dotCMS/src/main/webapp/html/portlet/ext/cmsmaintenance/index_stats.jsp index c82bce90ac40..d82b0f902995 100644 --- a/dotCMS/src/main/webapp/html/portlet/ext/cmsmaintenance/index_stats.jsp +++ b/dotCMS/src/main/webapp/html/portlet/ext/cmsmaintenance/index_stats.jsp @@ -45,10 +45,11 @@ try { List currentIdx =idxApi.getCurrentIndex(); List newIdx =idxApi.getNewIndex(); -// Hide OS-tagged (.os) migration indices outside Phase 3 unless the user holds the configured -// QA/preview role. Only this display sink filters; operational paths keep the full set. -List indices=MigrationIndexVisibility.filter(idxApi.listDotCMSIndices(), user); -List closedIndices=MigrationIndexVisibility.filter(idxApi.listDotCMSClosedIndices(), user); +// Hide OS-tagged (.os) migration indices outside Phase 3 (phase-based, for everyone). Only this +// display sink filters; operational paths keep the full set. Support/QA see migration detail via the +// role-gated readiness endpoint, not this portlet (issue #36360). +List indices=MigrationIndexVisibility.filter(idxApi.listDotCMSIndices()); +List closedIndices=MigrationIndexVisibility.filter(idxApi.listDotCMSClosedIndices()); Map indexInfo = esapi.getIndicesStats(); SimpleDateFormat dater = new SimpleDateFormat("yyyyMMddHHmmss"); diff --git a/dotCMS/src/test/java/com/dotcms/content/index/MigrationIndexVisibilityTest.java b/dotCMS/src/test/java/com/dotcms/content/index/MigrationIndexVisibilityTest.java index 10b334dc68d6..4c863d1c7d50 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/MigrationIndexVisibilityTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/MigrationIndexVisibilityTest.java @@ -4,31 +4,22 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; -import com.dotmarketing.business.APILocator; -import com.dotmarketing.business.Role; -import com.dotmarketing.business.RoleAPI; -import com.dotmarketing.exception.DotDataException; import com.dotmarketing.util.Config; -import com.liferay.portal.model.User; import java.util.Arrays; import java.util.Collections; import java.util.List; import org.junit.After; import org.junit.Test; -import org.mockito.MockedStatic; -import org.mockito.Mockito; /** * Unit tests for {@link MigrationIndexVisibility}. * - *

Verifies the phase + role visibility rule for OS-tagged ({@code .os}) indices: - * always visible in Phase 3; otherwise hidden unless the acting user holds the - * configured QA/preview role. The policy must fail closed when the role is absent or the - * lookup errors, and must never resolve the user from a thread-local.

+ *

The policy is purely phase-based (issue #36360 removed the role-gated preview — migration + * detail now comes from the role-gated readiness endpoint): OS-tagged ({@code .os}) indices are + * visible to everyone only in Phase 3; before it they are hidden from the display sinks. No + * user, role, or thread-local is consulted.

*/ public class MigrationIndexVisibilityTest { @@ -44,7 +35,6 @@ private static List mixedList() { @After public void clearConfig() { Config.setProperty(MigrationPhase.FLAG_KEY, null); - Config.setProperty(MigrationIndexVisibility.VISIBILITY_ROLE_KEY, null); } private static void setPhase(final int ordinal) { @@ -52,192 +42,65 @@ private static void setPhase(final int ordinal) { } // ========================================================================= - // Phase 3 — OS is the live store, everything is visible to everyone + // Phase 3 — OS is the live store, .os is visible to everyone // ========================================================================= - /** - * Given Scenario: Phase 3 (OS-only), no user supplied. - * Expected Result: canSeeMigrationIndices is true and filter returns the list unchanged - * (the .os entries stay), without consulting the role API. - */ + /** Phase 3: showMigrationIndices is true and filter returns the list unchanged. */ @Test - public void test_phase3_showsAllIncludingOsTagged_evenForNullUser() { + public void test_phase3_showsAllIncludingOsTagged() { setPhase(3); - assertTrue(MigrationIndexVisibility.canSeeMigrationIndices(null)); + assertTrue(MigrationIndexVisibility.showMigrationIndices()); final List list = mixedList(); assertSame("Phase 3 must return the same list instance untouched", - list, MigrationIndexVisibility.filter(list, null)); + list, MigrationIndexVisibility.filter(list)); } // ========================================================================= - // Phases 0/1/2 — .os hidden unless the user holds the QA role + // Phases 0/1/2 — .os hidden from everyone // ========================================================================= - /** - * Given Scenario: Phase 1 (dual-write), no user (e.g. could not be resolved). - * Expected Result: fail closed — OS-tagged entries are stripped, ES entries remain. - */ + /** Phase 0: .os entries are stripped, ES entries remain. */ @Test - public void test_phase1_nullUser_hidesOsTagged() { - setPhase(1); + public void test_phase0_hidesOsTagged() { + setPhase(0); - assertFalse(MigrationIndexVisibility.canSeeMigrationIndices(null)); + assertFalse(MigrationIndexVisibility.showMigrationIndices()); assertEquals(Arrays.asList(ES_OPEN, ES_CLOSED), - MigrationIndexVisibility.filter(mixedList(), null)); + MigrationIndexVisibility.filter(mixedList())); } - /** - * Given Scenario: Phase 1, an admin user who does NOT hold the QA role. - * Expected Result: OS-tagged entries are hidden. - */ + /** Phase 1 (dual-write): .os hidden. */ @Test - public void test_phase1_userWithoutRole_hidesOsTagged() throws DotDataException { + public void test_phase1_hidesOsTagged() { setPhase(1); - final User user = mock(User.class); - final Role role = mock(Role.class); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenReturn(role); - when(roleAPI.doesUserHaveRole(user, role)).thenReturn(false); - - try (MockedStatic apiLocator = Mockito.mockStatic(APILocator.class)) { - apiLocator.when(APILocator::getRoleAPI).thenReturn(roleAPI); - - assertFalse(MigrationIndexVisibility.canSeeMigrationIndices(user)); - assertEquals(Arrays.asList(ES_OPEN, ES_CLOSED), - MigrationIndexVisibility.filter(mixedList(), user)); - } - } - /** - * Given Scenario: Phase 1, a user who holds the configured QA role. - * Expected Result: the full list (including .os) is returned. - */ - @Test - public void test_phase1_userWithRole_showsAll() throws DotDataException { - setPhase(1); - final User user = mock(User.class); - final Role role = mock(Role.class); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenReturn(role); - when(roleAPI.doesUserHaveRole(user, role)).thenReturn(true); - - try (MockedStatic apiLocator = Mockito.mockStatic(APILocator.class)) { - apiLocator.when(APILocator::getRoleAPI).thenReturn(roleAPI); - - assertTrue(MigrationIndexVisibility.canSeeMigrationIndices(user)); - assertEquals(mixedList(), MigrationIndexVisibility.filter(mixedList(), user)); - } + assertFalse(MigrationIndexVisibility.showMigrationIndices()); + assertEquals(Arrays.asList(ES_OPEN, ES_CLOSED), + MigrationIndexVisibility.filter(mixedList())); } - /** - * Given Scenario: Phase 2 behaves identically to Phase 1 (still pre-complete). - * Expected Result: .os hidden for a user without the role. - */ + /** Phase 2 behaves identically to Phase 1 (still pre-complete). */ @Test - public void test_phase2_userWithoutRole_hidesOsTagged() throws DotDataException { + public void test_phase2_hidesOsTagged() { setPhase(2); - final User user = mock(User.class); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenReturn(mock(Role.class)); - when(roleAPI.doesUserHaveRole(Mockito.eq(user), Mockito.any(Role.class))) - .thenReturn(false); - - try (MockedStatic apiLocator = Mockito.mockStatic(APILocator.class)) { - apiLocator.when(APILocator::getRoleAPI).thenReturn(roleAPI); - - assertEquals(Arrays.asList(ES_OPEN, ES_CLOSED), - MigrationIndexVisibility.filter(mixedList(), user)); - } - } - - // ========================================================================= - // Fail-closed behaviour - // ========================================================================= - - /** - * Given Scenario: Phase 1 and the configured QA role does not exist (loadRoleByKey null). - * Expected Result: fail closed — user cannot see .os indices. - */ - @Test - public void test_phase1_roleMissing_failsClosed() throws DotDataException { - setPhase(1); - final User user = mock(User.class); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenReturn(null); - - try (MockedStatic apiLocator = Mockito.mockStatic(APILocator.class)) { - apiLocator.when(APILocator::getRoleAPI).thenReturn(roleAPI); - - assertFalse(MigrationIndexVisibility.canSeeMigrationIndices(user)); - } - } - /** - * Given Scenario: Phase 1 and the role lookup throws. - * Expected Result: the exception is swallowed and the policy fails closed. - */ - @Test - public void test_phase1_roleLookupThrows_failsClosed() throws DotDataException { - setPhase(1); - final User user = mock(User.class); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenThrow(new DotDataException("boom")); - - try (MockedStatic apiLocator = Mockito.mockStatic(APILocator.class)) { - apiLocator.when(APILocator::getRoleAPI).thenReturn(roleAPI); - - assertFalse(MigrationIndexVisibility.canSeeMigrationIndices(user)); - } - } - - // ========================================================================= - // Configurable role key - // ========================================================================= - - /** - * Given Scenario: a custom role key is configured via {@code OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY}. - * Expected Result: the policy looks up that key, not the default, when deciding visibility. - */ - @Test - public void test_customRoleKey_isHonored() throws DotDataException { - setPhase(1); - final String customKey = "my_custom_qa_role"; - Config.setProperty(MigrationIndexVisibility.VISIBILITY_ROLE_KEY, customKey); - - final User user = mock(User.class); - final Role role = mock(Role.class); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(customKey)).thenReturn(role); - when(roleAPI.doesUserHaveRole(user, role)).thenReturn(true); - - try (MockedStatic apiLocator = Mockito.mockStatic(APILocator.class)) { - apiLocator.when(APILocator::getRoleAPI).thenReturn(roleAPI); - - assertTrue(MigrationIndexVisibility.canSeeMigrationIndices(user)); - } + assertEquals(Arrays.asList(ES_OPEN, ES_CLOSED), + MigrationIndexVisibility.filter(mixedList())); } // ========================================================================= // Null / empty list handling // ========================================================================= - /** - * Given Scenario: filter is called with null or empty input in a hiding phase. - * Expected Result: the input is returned as-is, no NPE, no role lookup. - */ + /** filter with null or empty input in a hiding phase is returned as-is, no NPE. */ @Test public void test_filter_nullOrEmptyList_returnedAsIs() { setPhase(1); - assertSame(null, MigrationIndexVisibility.filter(null, null)); + assertSame(null, MigrationIndexVisibility.filter(null)); final List empty = Collections.emptyList(); - assertSame(empty, MigrationIndexVisibility.filter(empty, null)); + assertSame(empty, MigrationIndexVisibility.filter(empty)); } -} \ No newline at end of file +} diff --git a/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java b/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java new file mode 100644 index 000000000000..d1747338ce68 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java @@ -0,0 +1,94 @@ +package com.dotcms.rest.api.v1.index; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.dotcms.content.index.MigrationIndexVisibility; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.business.RoleAPI; +import com.dotmarketing.business.UserAPI; +import com.dotmarketing.exception.DotDataException; +import com.liferay.portal.model.User; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +/** + * Unit tests for the role gate of {@link MigrationReadinessResource#isMigrationSupportUser(User)} — + * the readiness endpoint is restricted to CMS administrators and members of the migration support + * role, and fails closed otherwise (issue #36360). All access APIs are mocked; no container needed. + */ +public class MigrationReadinessResourceTest { + + /** No authenticated user → denied. */ + @Test + public void nullUser_denied() { + assertFalse(MigrationReadinessResource.isMigrationSupportUser(null)); + } + + /** A CMS administrator is allowed without any role lookup. */ + @Test + public void cmsAdmin_allowed() throws DotDataException { + final User user = mock(User.class); + final UserAPI userAPI = mock(UserAPI.class); + when(userAPI.isCMSAdmin(user)).thenReturn(true); + + try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { + api.when(APILocator::getUserAPI).thenReturn(userAPI); + assertTrue(MigrationReadinessResource.isMigrationSupportUser(user)); + } + } + + /** A non-admin who holds the configured support role is allowed. */ + @Test + public void roleMember_allowed() throws DotDataException { + final User user = mock(User.class); + final UserAPI userAPI = mock(UserAPI.class); + when(userAPI.isCMSAdmin(user)).thenReturn(false); + final Role role = mock(Role.class); + final RoleAPI roleAPI = mock(RoleAPI.class); + when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) + .thenReturn(role); + when(roleAPI.doesUserHaveRole(user, role)).thenReturn(true); + + try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { + api.when(APILocator::getUserAPI).thenReturn(userAPI); + api.when(APILocator::getRoleAPI).thenReturn(roleAPI); + assertTrue(MigrationReadinessResource.isMigrationSupportUser(user)); + } + } + + /** A non-admin without the role is denied. */ + @Test + public void nonAdminWithoutRole_denied() throws DotDataException { + final User user = mock(User.class); + final UserAPI userAPI = mock(UserAPI.class); + when(userAPI.isCMSAdmin(user)).thenReturn(false); + final RoleAPI roleAPI = mock(RoleAPI.class); + when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) + .thenReturn(mock(Role.class)); + when(roleAPI.doesUserHaveRole(Mockito.eq(user), Mockito.any(Role.class))).thenReturn(false); + + try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { + api.when(APILocator::getUserAPI).thenReturn(userAPI); + api.when(APILocator::getRoleAPI).thenReturn(roleAPI); + assertFalse(MigrationReadinessResource.isMigrationSupportUser(user)); + } + } + + /** An access-layer failure fails closed (denied), never open. */ + @Test + public void accessLookupThrows_failsClosed() throws DotDataException { + final User user = mock(User.class); + final UserAPI userAPI = mock(UserAPI.class); + when(userAPI.isCMSAdmin(user)).thenThrow(new DotDataException("boom")); + + try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { + api.when(APILocator::getUserAPI).thenReturn(userAPI); + assertFalse(MigrationReadinessResource.isMigrationSupportUser(user)); + } + } +} From 2e3e32fac267e80d29e3de40116a26c1074a6c3a Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 11:22:53 -0600 Subject: [PATCH 04/11] refactor(migration): nest per-index status as es/os {exists,docCount} (#36360) Readiness JSON now reads es:{exists,docCount} / os:{exists,docCount} per index instead of flat esExists/esDocCount/osExists/osDocCount. MirrorStatus gains a nested EngineCopy record; reconcilers, service verdict, and unit tests updated. 22/22 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 2 +- .../ContentIndexMirrorReconciler.java | 4 +-- .../migration/MigrationReadinessService.java | 2 +- .../content/index/migration/MirrorStatus.java | 25 +++++++++++-------- .../migration/SiteSearchMirrorReconciler.java | 5 ++-- .../ContentIndexMirrorReconcilerTest.java | 12 ++++----- .../MigrationReadinessServiceTest.java | 5 ++-- 7 files changed, 30 insertions(+), 25 deletions(-) diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index 3a861a75cce5..ddfcf3b4045e 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -371,7 +371,7 @@ through the write-path gate above. overall verdict — `safeToAdvance` (toward OpenSearch-only) and `safeToRollback` (downgrade) with an `outOfSyncCount`, a human `summary`, and per-index `blockers`; and the per-index ES↔OS mirror diff for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site - Search indices. Each row carries `{esExists, esDocCount, osExists, osDocCount, verdict, + Search indices. Each row carries `{es:{exists,docCount}, os:{exists,docCount}, verdict, recommendation}` with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. - **Stateless, from live counts.** Every field is derived at request time. Counts are **exact** — the Site Search half uses `SiteSearchAPI.documentCount` and the content half reads each engine leaf's diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java index 75e60ac9d2cb..57dea1c59d67 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java @@ -87,8 +87,8 @@ private void addStatus(final List out, final IndexKind kind, final final long osCount = osExists ? osStats.get(osKey).documentCount() : 0L; final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); - out.add(new MirrorStatus(bare, kind, esExists, esCount, osExists, osCount, verdict, - recommend(bare, verdict, osExists))); + out.add(new MirrorStatus(bare, kind, new MirrorStatus.EngineCopy(esExists, esCount), + new MirrorStatus.EngineCopy(osExists, osCount), verdict, recommend(bare, verdict, osExists))); } private static String recommend(final String name, final Verdict verdict, final boolean osExists) { diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index ce01ad345835..7e2aa2651702 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -57,7 +57,7 @@ public MigrationReadiness evaluate() { // its OpenSearch counterpart would lose that delta after the downgrade (a failed ES count is -1, which // is < any real OS count → flagged, fail-safe). final boolean esBehindAnywhere = all.stream() - .anyMatch(s -> !s.esExists() || s.esDocCount() < s.osDocCount()); + .anyMatch(s -> !s.es().exists() || s.es().docCount() < s.os().docCount()); final boolean safeToAdvance; final String summary; diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java index c2c680a2fccd..2042d124a90c 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -8,26 +8,21 @@ * *

Shared by every index family that is mirrored during the migration: the versioned content * indices (working/live) and the Site Search indices. The {@link IndexKind} says which family this - * row belongs to.

+ * row belongs to. Each engine's side is a nested {@link EngineCopy} so the report reads as + * {@code es:{exists,docCount}} / {@code os:{exists,docCount}}.

* * @param indexName the logical index name (no {@code .os} tag) * @param kind which mirrored index family this row belongs to - * @param esExists whether the Elasticsearch copy exists - * @param esDocCount exact document count in the Elasticsearch copy (0 when absent, -1 when the - * count query failed) - * @param osExists whether the OpenSearch ({@code .os}) counterpart exists - * @param osDocCount exact document count in the OpenSearch counterpart (0 when absent, -1 when the count - * query failed) + * @param es the Elasticsearch copy (existence + exact document count) + * @param os the OpenSearch ({@code .os}) copy (existence + exact document count) * @param verdict the diff verdict between the two copies * @param recommendation human-readable, action-oriented advice for a support technician */ public record MirrorStatus( String indexName, IndexKind kind, - boolean esExists, - long esDocCount, - boolean osExists, - long osDocCount, + EngineCopy es, + EngineCopy os, Verdict verdict, String recommendation) { @@ -44,6 +39,14 @@ public enum Verdict { COUNT_DRIFT } + /** + * One engine's copy of the index. + * + * @param exists whether this engine holds the index + * @param docCount exact document count (0 when absent, -1 when the count query failed) + */ + public record EngineCopy(boolean exists, long docCount) {} + /** Whether this index needs operator action (a re-crawl / reindex) before the phase change. */ public boolean needsAttention() { return verdict != Verdict.IN_SYNC; diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java index 9f0e57f09bda..2b2ea8920c7c 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java @@ -69,8 +69,9 @@ private MirrorStatus statusFor(final String name) { final long esCount = esExists ? esImpl.documentCount(name) : 0L; final long osCount = osExists ? osImpl.documentCount(name) : 0L; final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); - return new MirrorStatus(name, IndexKind.SITE_SEARCH, esExists, esCount, osExists, osCount, - verdict, recommend(name, verdict)); + return new MirrorStatus(name, IndexKind.SITE_SEARCH, + new MirrorStatus.EngineCopy(esExists, esCount), + new MirrorStatus.EngineCopy(osExists, osCount), verdict, recommend(name, verdict)); } private static String recommend(final String name, final Verdict verdict) { diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java index 0243e492eb7c..31702ee16751 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java @@ -73,8 +73,8 @@ public void workingAndLive_inSync() { assertEquals(IndexKind.CONTENT_WORKING, working.kind()); assertEquals("working_1", working.indexName()); // logical: cluster prefix stripped, no .os assertEquals(Verdict.IN_SYNC, working.verdict()); - assertEquals(100, working.esDocCount()); - assertEquals(100, working.osDocCount()); + assertEquals(100, working.es().docCount()); + assertEquals(100, working.os().docCount()); assertEquals(IndexKind.CONTENT_LIVE, statuses.get(1).kind()); assertEquals(Verdict.IN_SYNC, statuses.get(1).verdict()); } @@ -92,8 +92,8 @@ public void missingOsCounterpart_onWorking() { final MirrorStatus working = statuses.get(0); assertEquals(Verdict.MISSING_COUNTERPART, working.verdict()); - assertTrue(working.esExists()); - assertFalse(working.osExists()); + assertTrue(working.es().exists()); + assertFalse(working.os().exists()); assertTrue(working.needsAttention()); assertTrue(working.recommendation().contains("OpenSearch")); } @@ -112,8 +112,8 @@ public void countDrift_onLive() { final MirrorStatus live = statuses.get(1); assertEquals(IndexKind.CONTENT_LIVE, live.kind()); assertEquals(Verdict.COUNT_DRIFT, live.verdict()); - assertEquals(50, live.esDocCount()); - assertEquals(40, live.osDocCount()); + assertEquals(50, live.es().docCount()); + assertEquals(40, live.os().docCount()); } /** A null IndiciesInfo (could not be loaded) yields no rows rather than throwing. */ diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index 8db4414a9da1..fbf13aae9828 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -54,8 +54,9 @@ private static void setPhase(final int ordinal) { private static MirrorStatus ss(final String name, final boolean esExists, final long esCount, final boolean osExists, final long osCount) { final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); - return new MirrorStatus(name, IndexKind.SITE_SEARCH, esExists, esCount, osExists, osCount, - verdict, "advice"); + return new MirrorStatus(name, IndexKind.SITE_SEARCH, + new MirrorStatus.EngineCopy(esExists, esCount), + new MirrorStatus.EngineCopy(osExists, osCount), verdict, "advice"); } /** Dual-write phase with every mirror in sync → safe to advance, nothing out of sync. */ From 71081f0e8eb408f34b61d5c1867b7c076be6ffb1 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 13:30:54 -0600 Subject: [PATCH 05/11] feat(migration): report full physical index name per engine + clusterId (#36360) Each per-index row now carries the full name as stored on the server in each engine's EngineCopy.physicalName (ES cluster-prefixed, OS additionally .os-tagged), and the report carries the top-level clusterId embedded in those names. Cluster prefix/id are injected (suppliers) so unit tests stay isolated. 22/22 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 7 +++++-- .../ContentIndexMirrorReconciler.java | 13 ++++++++---- .../index/migration/MigrationReadiness.java | 4 ++++ .../migration/MigrationReadinessService.java | 12 ++++++++--- .../content/index/migration/MirrorStatus.java | 10 +++++++--- .../migration/SiteSearchMirrorReconciler.java | 20 +++++++++++++++---- .../ContentIndexMirrorReconcilerTest.java | 3 +++ .../MigrationReadinessServiceTest.java | 8 +++++--- 8 files changed, 58 insertions(+), 19 deletions(-) diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index ddfcf3b4045e..7d083c3988d5 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -371,8 +371,11 @@ through the write-path gate above. overall verdict — `safeToAdvance` (toward OpenSearch-only) and `safeToRollback` (downgrade) with an `outOfSyncCount`, a human `summary`, and per-index `blockers`; and the per-index ES↔OS mirror diff for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site - Search indices. Each row carries `{es:{exists,docCount}, os:{exists,docCount}, verdict, - recommendation}` with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. + Search indices. Each row carries `{indexName (logical), es:{exists,docCount,physicalName}, + os:{exists,docCount,physicalName}, verdict, recommendation}` — `physicalName` is the full name as + stored on each server (cluster-prefixed; `.os`-tagged on OpenSearch) — with verdict `IN_SYNC` / + `MISSING_COUNTERPART` / `COUNT_DRIFT`. The top level also carries the `clusterId` embedded in every + physical name. - **Stateless, from live counts.** Every field is derived at request time. Counts are **exact** — the Site Search half uses `SiteSearchAPI.documentCount` and the content half reads each engine leaf's `getIndicesStats()` (index `_stats` `primaries.docs.count`), never a search total (which the ES/OS diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java index 57dea1c59d67..15390b1113d6 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java @@ -76,8 +76,11 @@ private void addStatus(final List out, final IndexKind kind, final if (!UtilMethods.isSet(rawName)) { return; } - // IndiciesInfo holds the cluster-prefixed, un-tagged ES name; the stats maps are keyed by the - // cluster-stripped name (ES un-tagged, OS carrying .os). Strip first, then tag for the OS key. + // IndiciesInfo holds the cluster-prefixed, un-tagged ES name — which IS the full ES physical + // name; the OS physical name is that + .os. The stats maps are keyed by the cluster-stripped + // name (ES un-tagged, OS carrying .os), so strip for the count lookup, tag for the OS key. + final String esPhysical = rawName; + final String osPhysical = IndexTag.OS.tag(rawName); final String bare = esImpl.removeClusterIdFromName(rawName); final String osKey = IndexTag.OS.tag(bare); @@ -87,8 +90,10 @@ private void addStatus(final List out, final IndexKind kind, final final long osCount = osExists ? osStats.get(osKey).documentCount() : 0L; final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); - out.add(new MirrorStatus(bare, kind, new MirrorStatus.EngineCopy(esExists, esCount), - new MirrorStatus.EngineCopy(osExists, osCount), verdict, recommend(bare, verdict, osExists))); + out.add(new MirrorStatus(bare, kind, + new MirrorStatus.EngineCopy(esExists, esCount, esPhysical), + new MirrorStatus.EngineCopy(osExists, osCount, osPhysical), + verdict, recommend(bare, verdict, osExists))); } private static String recommend(final String name, final Verdict verdict, final boolean osExists) { diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index 861bc3c5ffbe..04cd4ec10eb0 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -8,12 +8,16 @@ * is safe and, if not, what to do. Read-only and stateless — every field is derived from live index * state at request time, nothing is persisted. * + * @param clusterId the dotCMS cluster id embedded in every physical index name + * (the {@code } of the {@code cluster_.} prefix); identical for the + * Elasticsearch and OpenSearch backends * @param phase the current migration phase and which engine it reads/writes * @param verdict the overall go/no-go for advancing and rolling back, with reasons * @param contentIndices per-index mirror status for the versioned content indices (working/live) * @param siteSearchIndices per-index mirror status for the Site Search indices */ public record MigrationReadiness( + String clusterId, PhaseInfo phase, Verdict verdict, List contentIndices, diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index 7e2aa2651702..c1e7a981bbf2 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -1,9 +1,11 @@ package com.dotcms.content.index.migration; import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; +import com.dotcms.enterprise.cluster.ClusterFactory; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -28,16 +30,20 @@ public class MigrationReadinessService { private final SiteSearchMirrorReconciler siteSearchReconciler; private final ContentIndexMirrorReconciler contentReconciler; + private final Supplier clusterIdSupplier; public MigrationReadinessService() { - this(new SiteSearchMirrorReconciler(), new ContentIndexMirrorReconciler()); + this(new SiteSearchMirrorReconciler(), new ContentIndexMirrorReconciler(), + ClusterFactory::getClusterId); } @VisibleForTesting MigrationReadinessService(final SiteSearchMirrorReconciler siteSearchReconciler, - final ContentIndexMirrorReconciler contentReconciler) { + final ContentIndexMirrorReconciler contentReconciler, + final Supplier clusterIdSupplier) { this.siteSearchReconciler = siteSearchReconciler; this.contentReconciler = contentReconciler; + this.clusterIdSupplier = clusterIdSupplier; } /** Builds the readiness report for the current phase. */ @@ -92,7 +98,7 @@ public MigrationReadiness evaluate() { phase.isDualWrite()); final MigrationReadiness.Verdict verdict = new MigrationReadiness.Verdict( safeToAdvance, !esBehindAnywhere, outOfSync.size(), summary, blockers); - return new MigrationReadiness(phaseInfo, verdict, content, siteSearch); + return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, verdict, content, siteSearch); } private static String readEngine(final MigrationPhase phase) { diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java index 2042d124a90c..83f82dba00b1 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -42,10 +42,14 @@ public enum Verdict { /** * One engine's copy of the index. * - * @param exists whether this engine holds the index - * @param docCount exact document count (0 when absent, -1 when the count query failed) + * @param exists whether this engine holds the index + * @param docCount exact document count (0 when absent, -1 when the count query failed) + * @param physicalName the full index name as stored on that engine's server — cluster-prefixed and, + * for OpenSearch, {@code .os}-tagged (e.g. {@code cluster_08abc3.live_20260406} + * on ES, {@code cluster_08abc3.live_20260406.os} on OS). Reported whether or not + * the copy exists, so a missing copy shows the name to look for. */ - public record EngineCopy(boolean exists, long docCount) {} + public record EngineCopy(boolean exists, long docCount, String physicalName) {} /** Whether this index needs operator action (a re-crawl / reindex) before the phase change. */ public boolean needsAttention() { diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java index 2b2ea8920c7c..d8f7526deaec 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java @@ -2,15 +2,18 @@ import com.dotcms.cdi.CDIUtils; import com.dotcms.content.index.IndexConfigHelper; +import com.dotcms.content.index.IndexTag; import com.dotcms.content.index.migration.MirrorStatus.IndexKind; import com.dotcms.content.index.migration.MirrorStatus.Verdict; import com.dotcms.enterprise.publishing.sitesearch.ESSiteSearchAPI; import com.dotcms.enterprise.publishing.sitesearch.OSSiteSearchAPI; +import com.dotmarketing.business.APILocator; import com.dotmarketing.sitesearch.business.SiteSearchAPI; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.List; import java.util.TreeSet; +import java.util.function.Supplier; /** * Site Search half of the migration-readiness report (issue #36360): compares every logical @@ -28,15 +31,19 @@ public class SiteSearchMirrorReconciler { private final SiteSearchAPI esImpl; private final SiteSearchAPI osImpl; + private final Supplier clusterPrefixSupplier; public SiteSearchMirrorReconciler() { - this(new ESSiteSearchAPI(), CDIUtils.getBeanThrows(OSSiteSearchAPI.class)); + this(new ESSiteSearchAPI(), CDIUtils.getBeanThrows(OSSiteSearchAPI.class), + () -> APILocator.getESIndexAPI().getClusterPrefix()); } @VisibleForTesting - SiteSearchMirrorReconciler(final SiteSearchAPI esImpl, final SiteSearchAPI osImpl) { + SiteSearchMirrorReconciler(final SiteSearchAPI esImpl, final SiteSearchAPI osImpl, + final Supplier clusterPrefixSupplier) { this.esImpl = esImpl; this.osImpl = osImpl; + this.clusterPrefixSupplier = clusterPrefixSupplier; } /** @@ -68,10 +75,15 @@ private MirrorStatus statusFor(final String name) { final boolean osExists = osImpl.existsOnAllWriteEngines(name); final long esCount = esExists ? esImpl.documentCount(name) : 0L; final long osCount = osExists ? osImpl.documentCount(name) : 0L; + // Physical names as stored: ES is the cluster-prefixed logical name; OS is that + the .os tag + // (applied via IndexTag, the sole owner of the marker). + final String esPhysical = clusterPrefixSupplier.get() + name; + final String osPhysical = IndexTag.OS.tag(esPhysical); final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); return new MirrorStatus(name, IndexKind.SITE_SEARCH, - new MirrorStatus.EngineCopy(esExists, esCount), - new MirrorStatus.EngineCopy(osExists, osCount), verdict, recommend(name, verdict)); + new MirrorStatus.EngineCopy(esExists, esCount, esPhysical), + new MirrorStatus.EngineCopy(osExists, osCount, osPhysical), + verdict, recommend(name, verdict)); } private static String recommend(final String name, final Verdict verdict) { diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java index 31702ee16751..30d7f40d7f5d 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java @@ -75,6 +75,9 @@ public void workingAndLive_inSync() { assertEquals(Verdict.IN_SYNC, working.verdict()); assertEquals(100, working.es().docCount()); assertEquals(100, working.os().docCount()); + // full physical names as stored: ES cluster-prefixed, OS additionally .os-tagged + assertEquals("cluster_x.working_1", working.es().physicalName()); + assertEquals("cluster_x.working_1.os", working.os().physicalName()); assertEquals(IndexKind.CONTENT_LIVE, statuses.get(1).kind()); assertEquals(Verdict.IN_SYNC, statuses.get(1).verdict()); } diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index fbf13aae9828..cc858a5d6702 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -39,7 +39,7 @@ public void setUp() { siteSearch = mock(SiteSearchMirrorReconciler.class); content = mock(ContentIndexMirrorReconciler.class); when(content.statuses()).thenReturn(List.of()); - service = new MigrationReadinessService(siteSearch, content); + service = new MigrationReadinessService(siteSearch, content, () -> "cluster_x"); } @After @@ -55,8 +55,9 @@ private static MirrorStatus ss(final String name, final boolean esExists, final final boolean osExists, final long osCount) { final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); return new MirrorStatus(name, IndexKind.SITE_SEARCH, - new MirrorStatus.EngineCopy(esExists, esCount), - new MirrorStatus.EngineCopy(osExists, osCount), verdict, "advice"); + new MirrorStatus.EngineCopy(esExists, esCount, "cluster_x." + name), + new MirrorStatus.EngineCopy(osExists, osCount, "cluster_x." + name + ".os"), + verdict, "advice"); } /** Dual-write phase with every mirror in sync → safe to advance, nothing out of sync. */ @@ -67,6 +68,7 @@ public void dualWrite_allInSync_safeToAdvance() { final MigrationReadiness r = service.evaluate(); + assertEquals("cluster_x", r.clusterId()); assertTrue(r.phase().evaluable()); assertEquals("Elasticsearch", r.phase().readEngine()); assertTrue(r.verdict().safeToAdvance()); From cf7d5426c8ed143a3ea1adacb88fbb56038ad568 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 13:50:00 -0600 Subject: [PATCH 06/11] refactor(migration): drop response envelope, reorder + rename readiness fields (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Return the readiness model directly (Response.ok(model)) instead of wrapping it in ResponseEntityView — the internal endpoint has no use for the errors/messages/ pagination/permissions envelope. - Field order is now clusterId, phase, content, siteSearch, verdict. - Renamed contentIndices -> content, siteSearchIndices -> siteSearch. 22/22 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../index/migration/MigrationReadiness.java | 14 +++++++------- .../index/migration/MigrationReadinessService.java | 2 +- .../api/v1/index/MigrationReadinessResource.java | 5 +++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index 04cd4ec10eb0..b7db35bfcd5e 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -11,17 +11,17 @@ * @param clusterId the dotCMS cluster id embedded in every physical index name * (the {@code } of the {@code cluster_.} prefix); identical for the * Elasticsearch and OpenSearch backends - * @param phase the current migration phase and which engine it reads/writes - * @param verdict the overall go/no-go for advancing and rolling back, with reasons - * @param contentIndices per-index mirror status for the versioned content indices (working/live) - * @param siteSearchIndices per-index mirror status for the Site Search indices + * @param phase the current migration phase and which engine it reads/writes + * @param content per-index mirror status for the versioned content indices (working/live) + * @param siteSearch per-index mirror status for the Site Search indices + * @param verdict the overall go/no-go for advancing and rolling back, with reasons */ public record MigrationReadiness( String clusterId, PhaseInfo phase, - Verdict verdict, - List contentIndices, - List siteSearchIndices) { + List content, + List siteSearch, + Verdict verdict) { /** * @param current the current phase ordinal (0–3) diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index c1e7a981bbf2..d96d0b0608da 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -98,7 +98,7 @@ public MigrationReadiness evaluate() { phase.isDualWrite()); final MigrationReadiness.Verdict verdict = new MigrationReadiness.Verdict( safeToAdvance, !esBehindAnywhere, outOfSync.size(), summary, blockers); - return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, verdict, content, siteSearch); + return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, content, siteSearch, verdict); } private static String readEngine(final MigrationPhase phase) { diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java index 024749550b63..540e86c79db5 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java @@ -3,7 +3,6 @@ import com.dotcms.content.index.MigrationIndexVisibility; import com.dotcms.content.index.migration.MigrationReadinessService; import com.dotcms.rest.InitDataObject; -import com.dotcms.rest.ResponseEntityView; import com.dotcms.rest.WebResource; import com.dotcms.rest.annotation.NoCache; import com.dotmarketing.business.APILocator; @@ -78,7 +77,9 @@ public Response readiness(@Context final HttpServletRequest request, + "support role."); } - return Response.ok(new ResponseEntityView<>(readinessService.evaluate())).build(); + // Return the readiness model directly (no ResponseEntityView envelope) — this internal endpoint + // has no use for the errors/messages/pagination/permissions wrapper. + return Response.ok(readinessService.evaluate()).build(); } /** From 24bc76c3d3663f40f4aa701fbc6cd6cb9dd7bd4d Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 14:17:12 -0600 Subject: [PATCH 07/11] refactor(migration): key content/siteSearch by label, drop kind from JSON (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - content and siteSearch are now JSON objects, not arrays: content keyed by slot (WORKING/LIVE), siteSearch keyed by logical index name — self-documenting and directly addressable. - kind is excluded from the payload (@JsonIgnoreProperties on MirrorStatus); it is kept internally only to derive the content slot key and the blocker labels. 23/23 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 5 ++-- .../index/migration/MigrationReadiness.java | 9 +++---- .../migration/MigrationReadinessService.java | 21 +++++++++++++++- .../content/index/migration/MirrorStatus.java | 3 +++ .../MigrationReadinessServiceTest.java | 24 +++++++++++++++++++ 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index 7d083c3988d5..291ed14fbcd2 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -371,11 +371,12 @@ through the write-path gate above. overall verdict — `safeToAdvance` (toward OpenSearch-only) and `safeToRollback` (downgrade) with an `outOfSyncCount`, a human `summary`, and per-index `blockers`; and the per-index ES↔OS mirror diff for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site - Search indices. Each row carries `{indexName (logical), es:{exists,docCount,physicalName}, + Search indices. `content` is keyed by slot (`WORKING` / `LIVE`); `siteSearch` is keyed by the + logical index name. Each entry carries `{indexName, es:{exists,docCount,physicalName}, os:{exists,docCount,physicalName}, verdict, recommendation}` — `physicalName` is the full name as stored on each server (cluster-prefixed; `.os`-tagged on OpenSearch) — with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. The top level also carries the `clusterId` embedded in every - physical name. + physical name. The response is the model itself (no `ResponseEntityView` envelope). - **Stateless, from live counts.** Every field is derived at request time. Counts are **exact** — the Site Search half uses `SiteSearchAPI.documentCount` and the content half reads each engine leaf's `getIndicesStats()` (index `_stats` `primaries.docs.count`), never a search total (which the ES/OS diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index b7db35bfcd5e..aef672bdd223 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -1,6 +1,7 @@ package com.dotcms.content.index.migration; import java.util.List; +import java.util.Map; /** * Support-facing ES→OS migration-readiness report (issue #36360): a point-in-time snapshot that a @@ -12,15 +13,15 @@ * (the {@code } of the {@code cluster_.} prefix); identical for the * Elasticsearch and OpenSearch backends * @param phase the current migration phase and which engine it reads/writes - * @param content per-index mirror status for the versioned content indices (working/live) - * @param siteSearch per-index mirror status for the Site Search indices + * @param content the versioned content indices keyed by slot ({@code WORKING} / {@code LIVE}) + * @param siteSearch the Site Search indices keyed by their logical index name * @param verdict the overall go/no-go for advancing and rolling back, with reasons */ public record MigrationReadiness( String clusterId, PhaseInfo phase, - List content, - List siteSearch, + Map content, + Map siteSearch, Verdict verdict) { /** diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index d96d0b0608da..c060aaf7d904 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -2,9 +2,12 @@ import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; import com.dotcms.enterprise.cluster.ClusterFactory; +import com.dotcms.content.index.migration.MirrorStatus.IndexKind; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -98,7 +101,23 @@ public MigrationReadiness evaluate() { phase.isDualWrite()); final MigrationReadiness.Verdict verdict = new MigrationReadiness.Verdict( safeToAdvance, !esBehindAnywhere, outOfSync.size(), summary, blockers); - return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, content, siteSearch, verdict); + + // Content is keyed by slot (WORKING/LIVE — a fixed pair); Site Search by its logical index + // name (an open set). LinkedHashMap keeps the reconcilers' order for a stable response. + final Map contentBySlot = new LinkedHashMap<>(); + for (final MirrorStatus s : content) { + contentBySlot.put(contentSlot(s.kind()), s); + } + final Map siteSearchByName = new LinkedHashMap<>(); + for (final MirrorStatus s : siteSearch) { + siteSearchByName.put(s.indexName(), s); + } + return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, contentBySlot, + siteSearchByName, verdict); + } + + private static String contentSlot(final IndexKind kind) { + return kind == IndexKind.CONTENT_WORKING ? "WORKING" : "LIVE"; } private static String readEngine(final MigrationPhase phase) { diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java index 83f82dba00b1..7760a3f5cd18 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -1,5 +1,7 @@ package com.dotcms.content.index.migration; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + /** * Per-index ES↔OS mirror status for the migration-readiness report (issue #36360): how one logical * index compares against its counterpart across the two engines. Purely factual — the phase-aware @@ -18,6 +20,7 @@ * @param verdict the diff verdict between the two copies * @param recommendation human-readable, action-oriented advice for a support technician */ +@JsonIgnoreProperties("kind") // internal grouping/label only — the report keys rows by it, never emits it public record MirrorStatus( String indexName, IndexKind kind, diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index cc858a5d6702..d2ea0ff921bc 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -152,4 +152,28 @@ public void phase3_notEvaluable_openSearchOnly() { assertEquals("OpenSearch", r.phase().readEngine()); assertEquals(List.of("OpenSearch"), r.phase().writeEngines()); } + + /** Content is keyed by slot (WORKING/LIVE); Site Search by logical index name. */ + @Test + public void indices_keyedBySlotAndName() { + setPhase(PHASE_2); + when(content.statuses()).thenReturn(List.of( + cc(IndexKind.CONTENT_WORKING, "working_1", 10), + cc(IndexKind.CONTENT_LIVE, "live_1", 5))); + when(siteSearch.statuses()).thenReturn(List.of(ss("sitesearch_a", true, 3, true, 3))); + + final MigrationReadiness r = service.evaluate(); + + assertTrue(r.content().containsKey("WORKING")); + assertTrue(r.content().containsKey("LIVE")); + assertEquals("working_1", r.content().get("WORKING").indexName()); + assertTrue(r.siteSearch().containsKey("sitesearch_a")); + } + + private static MirrorStatus cc(final IndexKind kind, final String name, final long count) { + return new MirrorStatus(name, kind, + new MirrorStatus.EngineCopy(true, count, "cluster_x." + name), + new MirrorStatus.EngineCopy(true, count, "cluster_x." + name + ".os"), + Verdict.IN_SYNC, "advice"); + } } From 114654ef79f7bd00e23eeeeb96bb89f9cccf2f8b Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 31 Jul 2026 14:43:34 -0600 Subject: [PATCH 08/11] refactor(migration): siteSearch back to a list, content stays keyed by slot (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Site Search is an open set with no natural key, so keying it by index name only duplicated indexName. It reverts to a list; content stays a keyed object (WORKING/ LIVE — a fixed pair). The asymmetry mirrors the semantics. 23/23 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 4 ++-- .../content/index/migration/MigrationReadiness.java | 8 +++++--- .../index/migration/MigrationReadinessService.java | 11 ++++------- .../migration/MigrationReadinessServiceTest.java | 7 ++++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index 291ed14fbcd2..ce782f711265 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -371,8 +371,8 @@ through the write-path gate above. overall verdict — `safeToAdvance` (toward OpenSearch-only) and `safeToRollback` (downgrade) with an `outOfSyncCount`, a human `summary`, and per-index `blockers`; and the per-index ES↔OS mirror diff for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site - Search indices. `content` is keyed by slot (`WORKING` / `LIVE`); `siteSearch` is keyed by the - logical index name. Each entry carries `{indexName, es:{exists,docCount,physicalName}, + Search indices. `content` is a keyed object by slot (`WORKING` / `LIVE` — a fixed pair); + `siteSearch` is a list (an open set). Each entry carries `{indexName, es:{exists,docCount,physicalName}, os:{exists,docCount,physicalName}, verdict, recommendation}` — `physicalName` is the full name as stored on each server (cluster-prefixed; `.os`-tagged on OpenSearch) — with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. The top level also carries the `clusterId` embedded in every diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index aef672bdd223..a1890a3d9670 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -13,15 +13,17 @@ * (the {@code } of the {@code cluster_.} prefix); identical for the * Elasticsearch and OpenSearch backends * @param phase the current migration phase and which engine it reads/writes - * @param content the versioned content indices keyed by slot ({@code WORKING} / {@code LIVE}) - * @param siteSearch the Site Search indices keyed by their logical index name + * @param content the versioned content indices keyed by slot ({@code WORKING} / {@code LIVE}) — a + * fixed pair, so a keyed object reads naturally + * @param siteSearch the Site Search indices as a list — an open set with no natural key, so a list + * (each entry carries its own {@code indexName}) * @param verdict the overall go/no-go for advancing and rolling back, with reasons */ public record MigrationReadiness( String clusterId, PhaseInfo phase, Map content, - Map siteSearch, + List siteSearch, Verdict verdict) { /** diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index c060aaf7d904..89b58e8b19b3 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -102,18 +102,15 @@ public MigrationReadiness evaluate() { final MigrationReadiness.Verdict verdict = new MigrationReadiness.Verdict( safeToAdvance, !esBehindAnywhere, outOfSync.size(), summary, blockers); - // Content is keyed by slot (WORKING/LIVE — a fixed pair); Site Search by its logical index - // name (an open set). LinkedHashMap keeps the reconcilers' order for a stable response. + // Content is keyed by slot (WORKING/LIVE — a fixed pair, so a keyed object reads naturally); + // Site Search stays a list (an open set with no natural key). LinkedHashMap keeps the + // reconciler's order for a stable response. final Map contentBySlot = new LinkedHashMap<>(); for (final MirrorStatus s : content) { contentBySlot.put(contentSlot(s.kind()), s); } - final Map siteSearchByName = new LinkedHashMap<>(); - for (final MirrorStatus s : siteSearch) { - siteSearchByName.put(s.indexName(), s); - } return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, contentBySlot, - siteSearchByName, verdict); + siteSearch, verdict); } private static String contentSlot(final IndexKind kind) { diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index d2ea0ff921bc..ce6a6086d320 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -153,9 +153,9 @@ public void phase3_notEvaluable_openSearchOnly() { assertEquals(List.of("OpenSearch"), r.phase().writeEngines()); } - /** Content is keyed by slot (WORKING/LIVE); Site Search by logical index name. */ + /** Content is keyed by slot (WORKING/LIVE); Site Search stays an ordered list. */ @Test - public void indices_keyedBySlotAndName() { + public void content_keyedBySlot_siteSearchAsList() { setPhase(PHASE_2); when(content.statuses()).thenReturn(List.of( cc(IndexKind.CONTENT_WORKING, "working_1", 10), @@ -167,7 +167,8 @@ public void indices_keyedBySlotAndName() { assertTrue(r.content().containsKey("WORKING")); assertTrue(r.content().containsKey("LIVE")); assertEquals("working_1", r.content().get("WORKING").indexName()); - assertTrue(r.siteSearch().containsKey("sitesearch_a")); + assertEquals(1, r.siteSearch().size()); + assertEquals("sitesearch_a", r.siteSearch().get(0).indexName()); } private static MirrorStatus cc(final IndexKind kind, final String name, final long count) { From f7253c808f30356513d6134a4bf8e1dcf17cc31a Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Mon, 3 Aug 2026 09:48:39 -0600 Subject: [PATCH 09/11] fix(migration): readiness no longer greenlights advance with no active content indices (#36360) An empty content-index set made the readiness verdict pass vacuously (outOfSync empty -> safeToAdvance true). Treat a missing/ES-copy-less WORKING or LIVE slot as a hard blocker in phases 0/1/2; Site Search stays optional. Also rename PhaseInfo.evaluable -> dualWrite for clarity. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../index/migration/MigrationReadiness.java | 8 +- .../migration/MigrationReadinessService.java | 77 +++++++++++++--- .../MigrationReadinessServiceTest.java | 92 ++++++++++++++++--- 3 files changed, 150 insertions(+), 27 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index a1890a3d9670..9dae7f4092c8 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -31,16 +31,16 @@ public record MigrationReadiness( * @param name the phase enum name (e.g. {@code PHASE_2_DUAL_WRITE_OS_READS}) * @param readEngine which engine currently serves reads ("Elasticsearch" or "OpenSearch") * @param writeEngines which engines currently receive writes - * @param evaluable whether a cross-engine comparison is meaningful for a forward phase change - * (only the dual-write phases 1/2); when false the mirror lists are advisory - * context, not a forward go/no-go + * @param dualWrite whether this is a dual-write phase (1/2), where both engines are populated so a + * cross-engine mirror comparison is meaningful as a forward go/no-go; when false + * (phases 0 and 3) the mirror lists are advisory context, not a forward go/no-go */ public record PhaseInfo( int current, String name, String readEngine, List writeEngines, - boolean evaluable) {} + boolean dualWrite) {} /** * @param safeToAdvance whether it is safe to promote toward the OpenSearch-only phase diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index 89b58e8b19b3..5323816110f5 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -68,15 +68,18 @@ public MigrationReadiness evaluate() { final boolean esBehindAnywhere = all.stream() .anyMatch(s -> !s.es().exists() || s.es().docCount() < s.os().docCount()); + // Content WORKING/LIVE are mandatory in every pre-OpenSearch-only phase: they are the source + // that gets mirrored to OpenSearch, so a missing slot (pointer unset, or its Elasticsearch copy + // gone) means there is nothing to migrate — a hard no-go, independent of the sync check, which + // would otherwise pass vacuously when there are no active indices at all. Site Search is an open + // set that may legitimately be empty, so it is not required here. + final List missingContent = requiredContentBlockers(content); + final boolean safeToAdvance; final String summary; final List blockers = new ArrayList<>(); - if (phase.isMigrationNotStarted()) { - safeToAdvance = true; - summary = "Phase 0 (Elasticsearch only). OpenSearch counterparts are built during the dual-write " - + "phases, so there is nothing to reconcile yet. Safe to advance to Phase 1."; - } else if (phase.isMigrationComplete()) { + if (phase.isMigrationComplete()) { safeToAdvance = true; // no phase beyond 3 summary = "Phase 3 (OpenSearch only) — the final phase, nothing to advance to. " + (esBehindAnywhere @@ -84,16 +87,33 @@ public MigrationReadiness evaluate() { + "hide it until a full reindex." : "No index shows Elasticsearch behind OpenSearch; still verify before any " + "downgrade."); + } else if (phase.isMigrationNotStarted()) { + // Phase 0: OpenSearch counterparts are built later (during dual-write), so their absence is + // expected and NOT a blocker; only the mandatory Elasticsearch content pair is required. + blockers.addAll(missingContent); + safeToAdvance = blockers.isEmpty(); + summary = safeToAdvance + ? "Phase 0 (Elasticsearch only). OpenSearch counterparts are built during the " + + "dual-write phases, so there is nothing to reconcile yet. Safe to advance " + + "to Phase 1." + : String.format("Not safe to advance from Phase 0: %s to resolve first (see the " + + "blockers list). Dual-write needs an active Elasticsearch content index " + + "to mirror from.", plural(blockers.size(), "blocker")); } else { - safeToAdvance = outOfSync.isEmpty(); - for (final MirrorStatus s : outOfSync) { - blockers.add(String.format("%s '%s': %s", s.kind(), s.indexName(), s.recommendation())); - } + // Phases 1/2 (dual-write): require the mandatory content pair AND every mirror in sync. Drop + // out-of-sync rows already reported as missing content (an ES-missing content slot surfaces + // both as a missing-source blocker and as MISSING_COUNTERPART) so it is not reported twice. + blockers.addAll(missingContent); + outOfSync.stream() + .filter(s -> !(isContent(s.kind()) && !s.es().exists())) + .forEach(s -> blockers.add(String.format("%s '%s': %s", s.kind(), s.indexName(), + s.recommendation()))); + safeToAdvance = blockers.isEmpty(); summary = safeToAdvance ? "All mirrors are in sync. Safe to advance toward the OpenSearch-only phase." - : String.format("%d index(es) out of sync. Re-crawl/reindex them before promoting " - + "the phase — Phase 3 reads OpenSearch with no Elasticsearch fallback.", - outOfSync.size()); + : String.format("Not safe to advance: %s to resolve first (see the blockers list). " + + "Phase 3 serves reads from OpenSearch only, so every index must be present " + + "and in sync before promoting.", plural(blockers.size(), "blocker")); } final MigrationReadiness.PhaseInfo phaseInfo = new MigrationReadiness.PhaseInfo( @@ -117,6 +137,39 @@ private static String contentSlot(final IndexKind kind) { return kind == IndexKind.CONTENT_WORKING ? "WORKING" : "LIVE"; } + /** + * Blockers for the mandatory content pair: WORKING and LIVE must each have a set pointer and an + * existing Elasticsearch copy (the migration source). Returns one message per missing/empty slot; + * an empty list means both are present. This is what stops a "no active content indices" state from + * passing the readiness check vacuously (an empty status list would otherwise leave nothing to flag). + */ + private static List requiredContentBlockers(final List content) { + final List out = new ArrayList<>(2); + for (final IndexKind kind : List.of(IndexKind.CONTENT_WORKING, IndexKind.CONTENT_LIVE)) { + final String slot = contentSlot(kind); + final MirrorStatus status = content.stream() + .filter(s -> s.kind() == kind).findFirst().orElse(null); + if (status == null) { + out.add(String.format("No active %s content index — Elasticsearch has no %s index to " + + "migrate. Reindex to (re)create it before changing the phase.", + slot, slot.toLowerCase())); + } else if (!status.es().exists()) { + out.add(String.format("The active %s content index '%s' has no Elasticsearch copy — " + + "reindex to rebuild it before changing the phase.", slot, status.indexName())); + } + } + return out; + } + + private static boolean isContent(final IndexKind kind) { + return kind == IndexKind.CONTENT_WORKING || kind == IndexKind.CONTENT_LIVE; + } + + /** {@code "1 blocker"} / {@code "2 blockers"} — count with a correctly pluralized noun. */ + private static String plural(final int count, final String noun) { + return count + " " + noun + (count == 1 ? "" : "s"); + } + private static String readEngine(final MigrationPhase phase) { return phase.isReadEnabled() ? "OpenSearch" : "Elasticsearch"; } diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index ce6a6086d320..3aeb37aa9569 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -38,7 +38,9 @@ public void setUp() { previousPhase = Config.getStringProperty(IndexConfigHelper.MigrationPhase.FLAG_KEY, null); siteSearch = mock(SiteSearchMirrorReconciler.class); content = mock(ContentIndexMirrorReconciler.class); - when(content.statuses()).thenReturn(List.of()); + // Default to a healthy WORKING/LIVE pair so the mandatory-content precondition is satisfied; + // tests that exercise missing content override this explicitly. + when(content.statuses()).thenReturn(healthyContentPair()); service = new MigrationReadinessService(siteSearch, content, () -> "cluster_x"); } @@ -69,7 +71,7 @@ public void dualWrite_allInSync_safeToAdvance() { final MigrationReadiness r = service.evaluate(); assertEquals("cluster_x", r.clusterId()); - assertTrue(r.phase().evaluable()); + assertTrue(r.phase().dualWrite()); assertEquals("Elasticsearch", r.phase().readEngine()); assertTrue(r.verdict().safeToAdvance()); assertEquals(0, r.verdict().outOfSyncCount()); @@ -127,28 +129,28 @@ public void mirrorsEven_safeToRollback() { assertTrue(r.verdict().safeToRollback()); } - /** Phase 0: not evaluable for a forward comparison, but advancing to dual-write is safe. */ + /** Phase 0: not a dual-write phase, but advancing to dual-write is safe. */ @Test - public void phase0_notEvaluable_safeToAdvance() { + public void phase0_notDualWrite_safeToAdvance() { setPhase(PHASE_0); when(siteSearch.statuses()).thenReturn(List.of()); final MigrationReadiness r = service.evaluate(); - assertFalse(r.phase().evaluable()); + assertFalse(r.phase().dualWrite()); assertEquals(List.of("Elasticsearch"), r.phase().writeEngines()); assertTrue(r.verdict().safeToAdvance()); } - /** Phase 3: not evaluable; write engine is OpenSearch only. */ + /** Phase 3: not a dual-write phase; write engine is OpenSearch only. */ @Test - public void phase3_notEvaluable_openSearchOnly() { + public void phase3_notDualWrite_openSearchOnly() { setPhase(PHASE_3); when(siteSearch.statuses()).thenReturn(List.of(ss("a", true, 100, true, 100))); final MigrationReadiness r = service.evaluate(); - assertFalse(r.phase().evaluable()); + assertFalse(r.phase().dualWrite()); assertEquals("OpenSearch", r.phase().readEngine()); assertEquals(List.of("OpenSearch"), r.phase().writeEngines()); } @@ -171,10 +173,78 @@ public void content_keyedBySlot_siteSearchAsList() { assertEquals("sitesearch_a", r.siteSearch().get(0).indexName()); } + /** No active content indices at all → must NOT pass vacuously; both mandatory slots are blockers. */ + @Test + public void dualWrite_noContentIndices_blocksAdvance() { + setPhase(PHASE_1); + when(content.statuses()).thenReturn(List.of()); + when(siteSearch.statuses()).thenReturn(List.of(ss("a", true, 100, true, 100))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertEquals(2, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().stream().anyMatch(b -> b.contains("WORKING"))); + assertTrue(r.verdict().blockers().stream().anyMatch(b -> b.contains("LIVE"))); + } + + /** Phase 0 with no source content indices → cannot even start the migration. */ + @Test + public void phase0_noContentIndices_blocksAdvance() { + setPhase(PHASE_0); + when(content.statuses()).thenReturn(List.of()); + when(siteSearch.statuses()).thenReturn(List.of()); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertEquals(2, r.verdict().blockers().size()); + } + + /** One content slot present, the other missing → single blocker for the missing slot. */ + @Test + public void dualWrite_oneContentSlotMissing_blocksAdvance() { + setPhase(PHASE_2); + when(content.statuses()).thenReturn(List.of(cc(IndexKind.CONTENT_WORKING, "working_1", 10))); + when(siteSearch.statuses()).thenReturn(List.of()); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertEquals(1, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().get(0).contains("LIVE")); + } + + /** A content slot whose ES copy is gone is one blocker, not double-reported as MISSING_COUNTERPART. */ + @Test + public void dualWrite_contentEsCopyMissing_singleBlockerNoDuplicate() { + setPhase(PHASE_2); + when(content.statuses()).thenReturn(List.of( + cc(IndexKind.CONTENT_WORKING, "working_1", true, 10, true, 10), + cc(IndexKind.CONTENT_LIVE, "live_1", false, 0, true, 5))); // ES copy gone + when(siteSearch.statuses()).thenReturn(List.of()); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertEquals(1, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().get(0).contains("no Elasticsearch copy")); + } + + private static List healthyContentPair() { + return List.of(cc(IndexKind.CONTENT_WORKING, "working_1", 10), + cc(IndexKind.CONTENT_LIVE, "live_1", 5)); + } + private static MirrorStatus cc(final IndexKind kind, final String name, final long count) { + return cc(kind, name, true, count, true, count); + } + + private static MirrorStatus cc(final IndexKind kind, final String name, final boolean esExists, + final long esCount, final boolean osExists, final long osCount) { return new MirrorStatus(name, kind, - new MirrorStatus.EngineCopy(true, count, "cluster_x." + name), - new MirrorStatus.EngineCopy(true, count, "cluster_x." + name + ".os"), - Verdict.IN_SYNC, "advice"); + new MirrorStatus.EngineCopy(esExists, esCount, "cluster_x." + name), + new MirrorStatus.EngineCopy(osExists, osCount, "cluster_x." + name + ".os"), + MirrorStatus.verdictFor(esExists, osExists, esCount, osCount), "advice"); } } From 1359e749ffd599402d56d03e366ad4592339539f Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Mon, 3 Aug 2026 16:56:51 -0600 Subject: [PATCH 10/11] fix(migration): require CMS admin AND support role for the readiness endpoint (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate was admin OR role; a plain CMS admin could read the migration report. Change to admin AND the migration support role (OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY) — a plain admin without the role is now denied, so regular/other admin users never learn a migration is running. Fail-closed on any access-lookup error. Gate test updated to the AND semantics (5/5); docs aligned (also fixes a stale evaluable->dualWrite mention). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 7 ++-- .../v1/index/MigrationReadinessResource.java | 34 ++++++++++------- .../index/MigrationReadinessResourceTest.java | 38 ++++++++++--------- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index ce782f711265..24d12d7dd100 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -365,9 +365,10 @@ mutates anything — the fix is always the operator re-running the crawl / reind through the write-path gate above. - **Not public.** The resource is `@Hidden` (absent from the OpenAPI / API-playground schema) and - gated to CMS administrators or members of the migration support role - (`OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY`, default `os_migration_qa`); anyone else gets a 403. -- **What it reports.** The current phase with its read/write engines and an `evaluable` flag; an + gated to CMS administrators who **also** hold the migration support role + (`OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY`, default `os_migration_qa`) — a plain admin without the + role is not enough. Anyone else gets a 403, so regular users never learn a migration is running. +- **What it reports.** The current phase with its read/write engines and a `dualWrite` flag; an overall verdict — `safeToAdvance` (toward OpenSearch-only) and `safeToRollback` (downgrade) with an `outOfSyncCount`, a human `summary`, and per-index `blockers`; and the per-index ES↔OS mirror diff for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java index 540e86c79db5..27272d6e5c64 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java @@ -32,11 +32,12 @@ * overall safe-to-advance / safe-to-rollback verdict. Read-only; it never mutates any index. * *

Not public. The class is {@link Hidden} so it never appears in the OpenAPI / - * API-playground schema, and every method requires a backend user who is a CMS administrator or a - * member of the migration support role + * API-playground schema, and every method requires a backend user who is a CMS administrator + * and a member of the migration support role * ({@value com.dotcms.content.index.MigrationIndexVisibility#VISIBILITY_ROLE_KEY}, default - * {@value com.dotcms.content.index.MigrationIndexVisibility#DEFAULT_VISIBILITY_ROLE_KEY}); anyone - * else gets a 403.

+ * {@value com.dotcms.content.index.MigrationIndexVisibility#DEFAULT_VISIBILITY_ROLE_KEY}) — a plain + * admin without the role is not enough. Anyone else gets a 403, so regular users never learn a + * migration is running.

*/ @Path("/v1/index/migration") @Hidden @@ -54,8 +55,8 @@ public MigrationReadinessResource() { } /** - * Returns the migration-readiness report for the current phase. Requires a CMS administrator or a - * member of the configured migration support role. + * Returns the migration-readiness report for the current phase. Requires a CMS administrator who + * also holds the configured migration support role. */ @GET @JSONP @@ -73,8 +74,8 @@ public Response readiness(@Context final HttpServletRequest request, final User user = initData.getUser(); if (!isMigrationSupportUser(user)) { throw new ForbiddenException( - "Migration readiness is restricted to CMS administrators and the migration " - + "support role."); + "Migration readiness is restricted to CMS administrators who also hold the " + + "migration support role."); } // Return the readiness model directly (no ResponseEntityView envelope) — this internal endpoint @@ -83,10 +84,12 @@ public Response readiness(@Context final HttpServletRequest request, } /** - * Whether {@code user} may read the migration-readiness report: a CMS administrator, or a member - * of the configured support role ({@link MigrationIndexVisibility#VISIBILITY_ROLE_KEY}, default - * {@link MigrationIndexVisibility#DEFAULT_VISIBILITY_ROLE_KEY}). This is an internal support tool - * in every phase — it never opens up to everyone, unlike the phase-based index portlet display. + * Whether {@code user} may read the migration-readiness report: a CMS administrator who + * also holds the configured support role + * ({@link MigrationIndexVisibility#VISIBILITY_ROLE_KEY}, default + * {@link MigrationIndexVisibility#DEFAULT_VISIBILITY_ROLE_KEY}) — both are required. This is an + * internal support tool in every phase; it never opens up to everyone (a plain admin is not + * enough), unlike the phase-based index portlet display. */ @VisibleForTesting static boolean isMigrationSupportUser(final User user) { @@ -94,8 +97,11 @@ static boolean isMigrationSupportUser(final User user) { return false; } return Try.of(() -> { - if (APILocator.getUserAPI().isCMSAdmin(user)) { - return true; + // Must be BOTH a CMS administrator AND a member of the migration support role — a plain + // admin without the role is not enough, and the role without admin is not enough, so a + // regular user never accesses or learns of the migration (issue #36360). + if (!APILocator.getUserAPI().isCMSAdmin(user)) { + return false; } final String roleKey = Config.getStringProperty( MigrationIndexVisibility.VISIBILITY_ROLE_KEY, diff --git a/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java b/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java index d1747338ce68..cc98088c47d5 100644 --- a/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java +++ b/dotCMS/src/test/java/com/dotcms/rest/api/v1/index/MigrationReadinessResourceTest.java @@ -18,8 +18,10 @@ /** * Unit tests for the role gate of {@link MigrationReadinessResource#isMigrationSupportUser(User)} — - * the readiness endpoint is restricted to CMS administrators and members of the migration support - * role, and fails closed otherwise (issue #36360). All access APIs are mocked; no container needed. + * the readiness endpoint is restricted to users who are BOTH a CMS administrator AND a member of the + * migration support role, and fails closed otherwise (issue #36360). A plain admin without the role, + * and the role without admin, are both denied — so regular users never learn a migration is running. + * All access APIs are mocked; no container needed. */ public class MigrationReadinessResourceTest { @@ -29,52 +31,52 @@ public void nullUser_denied() { assertFalse(MigrationReadinessResource.isMigrationSupportUser(null)); } - /** A CMS administrator is allowed without any role lookup. */ + /** A CMS administrator who also holds the support role → allowed. */ @Test - public void cmsAdmin_allowed() throws DotDataException { + public void cmsAdminWithRole_allowed() throws DotDataException { final User user = mock(User.class); final UserAPI userAPI = mock(UserAPI.class); when(userAPI.isCMSAdmin(user)).thenReturn(true); + final Role role = mock(Role.class); + final RoleAPI roleAPI = mock(RoleAPI.class); + when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) + .thenReturn(role); + when(roleAPI.doesUserHaveRole(user, role)).thenReturn(true); try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { api.when(APILocator::getUserAPI).thenReturn(userAPI); + api.when(APILocator::getRoleAPI).thenReturn(roleAPI); assertTrue(MigrationReadinessResource.isMigrationSupportUser(user)); } } - /** A non-admin who holds the configured support role is allowed. */ + /** A CMS administrator WITHOUT the support role → denied (admin alone is not enough). */ @Test - public void roleMember_allowed() throws DotDataException { + public void cmsAdminWithoutRole_denied() throws DotDataException { final User user = mock(User.class); final UserAPI userAPI = mock(UserAPI.class); - when(userAPI.isCMSAdmin(user)).thenReturn(false); - final Role role = mock(Role.class); + when(userAPI.isCMSAdmin(user)).thenReturn(true); final RoleAPI roleAPI = mock(RoleAPI.class); when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenReturn(role); - when(roleAPI.doesUserHaveRole(user, role)).thenReturn(true); + .thenReturn(mock(Role.class)); + when(roleAPI.doesUserHaveRole(Mockito.eq(user), Mockito.any(Role.class))).thenReturn(false); try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { api.when(APILocator::getUserAPI).thenReturn(userAPI); api.when(APILocator::getRoleAPI).thenReturn(roleAPI); - assertTrue(MigrationReadinessResource.isMigrationSupportUser(user)); + assertFalse(MigrationReadinessResource.isMigrationSupportUser(user)); } } - /** A non-admin without the role is denied. */ + /** A support-role member who is NOT a CMS administrator → denied (role alone is not enough). */ @Test - public void nonAdminWithoutRole_denied() throws DotDataException { + public void roleWithoutAdmin_denied() throws DotDataException { final User user = mock(User.class); final UserAPI userAPI = mock(UserAPI.class); when(userAPI.isCMSAdmin(user)).thenReturn(false); - final RoleAPI roleAPI = mock(RoleAPI.class); - when(roleAPI.loadRoleByKey(MigrationIndexVisibility.DEFAULT_VISIBILITY_ROLE_KEY)) - .thenReturn(mock(Role.class)); - when(roleAPI.doesUserHaveRole(Mockito.eq(user), Mockito.any(Role.class))).thenReturn(false); try (MockedStatic api = Mockito.mockStatic(APILocator.class)) { api.when(APILocator::getUserAPI).thenReturn(userAPI); - api.when(APILocator::getRoleAPI).thenReturn(roleAPI); assertFalse(MigrationReadinessResource.isMigrationSupportUser(user)); } } From f04216cef9c4c3f06c7ed6d199acfc483d83ca20 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Mon, 3 Aug 2026 20:42:21 -0600 Subject: [PATCH 11/11] feat(migration): add driftPercent per index to the readiness report (#36360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each index now carries driftPercent: the signed % the OpenSearch (mirror) doc count deviates from the Elasticsearch (original) — 0 when equal, negative when the mirror is behind, positive when ahead, -100 for a missing mirror, null when a count is unknown. Derived on MirrorStatus and serialized via @JsonProperty. Tests assert -20% drift and -100% missing. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backend/OPENSEARCH_MIGRATION.md | 6 +++-- .../content/index/migration/MirrorStatus.java | 23 +++++++++++++++++++ .../ContentIndexMirrorReconcilerTest.java | 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index 24d12d7dd100..872f3b4c6238 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -374,8 +374,10 @@ through the write-path gate above. for **both** mirrored families — the versioned content indices (`working`/`live`) and the Site Search indices. `content` is a keyed object by slot (`WORKING` / `LIVE` — a fixed pair); `siteSearch` is a list (an open set). Each entry carries `{indexName, es:{exists,docCount,physicalName}, - os:{exists,docCount,physicalName}, verdict, recommendation}` — `physicalName` is the full name as - stored on each server (cluster-prefixed; `.os`-tagged on OpenSearch) — with verdict `IN_SYNC` / + os:{exists,docCount,physicalName}, driftPercent, verdict, recommendation}` — `physicalName` is the + full name as stored on each server (cluster-prefixed; `.os`-tagged on OpenSearch), and + `driftPercent` is the signed % the OpenSearch (mirror) count deviates from the Elasticsearch + (original) — negative = behind, positive = ahead, `null` when a count is unknown — with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. The top level also carries the `clusterId` embedded in every physical name. The response is the model itself (no `ResponseEntityView` envelope). - **Stateless, from live counts.** Every field is derived at request time. Counts are **exact** — the diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java index 7760a3f5cd18..8a26a410ca54 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -1,6 +1,7 @@ package com.dotcms.content.index.migration; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; /** * Per-index ES↔OS mirror status for the migration-readiness report (issue #36360): how one logical @@ -59,6 +60,28 @@ public boolean needsAttention() { return verdict != Verdict.IN_SYNC; } + /** + * Signed percentage by which the OpenSearch (mirror) document count deviates from the + * Elasticsearch (original), relative to the original: {@code 0.0} when equal, negative when the + * mirror is behind, positive when it is ahead (e.g. ES=1000/OS=900 → {@code -10.0}; a missing OS + * copy → {@code -100.0}). When the original is empty a non-empty mirror reads as {@code 100.0}. + * {@code null} when either count is unknown (a failed count, reported as -1). Rounded to two + * decimals. + */ + @JsonProperty("driftPercent") + public Double driftPercent() { + final long esCount = es.docCount(); + final long osCount = os.docCount(); + if (esCount < 0 || osCount < 0) { + return null; + } + if (esCount == osCount) { + return 0.0; + } + final double pct = esCount == 0 ? 100.0 : (osCount - esCount) * 100.0 / esCount; + return Math.round(pct * 100.0) / 100.0; + } + /** * Classifies a mirror from raw existence + exact counts: a missing copy on either engine is * {@link Verdict#MISSING_COUNTERPART}; both present with unequal counts is {@link Verdict#COUNT_DRIFT} diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java index 30d7f40d7f5d..f0dfebf818a1 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java @@ -98,6 +98,7 @@ public void missingOsCounterpart_onWorking() { assertTrue(working.es().exists()); assertFalse(working.os().exists()); assertTrue(working.needsAttention()); + assertEquals(-100.0, working.driftPercent(), 0.001); // mirror empty vs original of 100 → -100% assertTrue(working.recommendation().contains("OpenSearch")); } @@ -117,6 +118,8 @@ public void countDrift_onLive() { assertEquals(Verdict.COUNT_DRIFT, live.verdict()); assertEquals(50, live.es().docCount()); assertEquals(40, live.os().docCount()); + // mirror 10 docs behind the original of 50 → -20% + assertEquals(-20.0, live.driftPercent(), 0.001); } /** A null IndiciesInfo (could not be loaded) yields no rows rather than throwing. */