feat(sitesearch): vendor-neutral Site Search — neutral aggregation + OpenSearch impl + phase-aware router (#35786)

[fabrizzio-dotCMS]

What & why

Closes the Site Search portion of the ES → OpenSearch migration (#35786). Site Search is decoupled from Elasticsearch types and given a working OpenSearch backend plus a phase-aware router, so it dual-writes and reads correctly across all migration phases.

Two commits:

1. Vendor-neutral aggregation abstraction — removes org.elasticsearch.* from the SiteSearchAPI contract and SiteSearchWebAPI, reusing the existing com.dotcms.content.index.domain.Aggregation / AggregationBucket DTOs (from Aggregation return-type change breaks existing VTL templates accessing $results.aggregations #36026) with histogram support, and introduces DotSearchException.
2. OpenSearch implementation + router — adds OSSiteSearchAPI, the SiteSearchAPIImpl phase router, and an integration test.

Changes

Area	Change
SiteSearchAPI / SiteSearchWebAPI	No vendor imports; getAggregations/getFacets return neutral Aggregation; DotSearchException added
OSSiteSearchAPI (new)	@ApplicationScoped @Default OpenSearch impl. Search/aggregations via the generic client → ContentSearchResponse (mirrors OSSearchAPIImpl); doc put/delete via _doc PUT/DELETE; get via typed client.get(...). Default index resolved from VersionedIndicesAPI (not the deprecated IndiciesAPI)
SiteSearchAPIImpl (new, router)	PhaseRouter<SiteSearchAPI> mirroring IndexAPIImpl; the single fan-out point. Reads → read provider; doc/index writes → write fan-out; listIndices/listClosedIndices merge in dual-write; Quartz task methods route to a single provider (fan-out would double-schedule jobs)
ESSiteSearchAPI	Uses raw ESIndexAPI instead of the IndexAPI router so the SiteSearch router is the only fan-out point (avoids double dual-write of OS indices)
APILocator	SITESEARCH_API now returns SiteSearchAPIImpl

Design notes

• Placement: OS impl + router live next to ESSiteSearchAPI in the enterprise package (license-gated feature). The single annotated beans.xml covers the merged target/classes, so CDI still discovers the @Default bean.
• OS index naming: site-search indices use untagged logical names. VersionedIndicesAPI force-tags .os on store/load, so the default is IndexTag.strip(...)-ed on read. deactivateIndex calls removeVersion(...) when removing the slot would leave the version empty (saveIndices rejects empty).
• Highlights: the neutral SearchHit DTO carries no highlights, so OS search() returns empty highlight arrays (the ES path is best-effort too) — marked TODO OS.

Testing

• ./mvnw compile -pl :dotcms-core → BUILD SUCCESS (Java 25)
• ./mvnw test-compile -pl :dotcms-integration -am → BUILD SUCCESS
• New OSSiteSearchAPIIntegrationTest (registered in OpenSearchUpgradeSuite) covers lifecycle, doc round-trip, aggregations, and default-index activation. Requires the opensearch-upgrade container:

  ./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false -Dopensearch.upgrade.test=true -Dit.test=OSSiteSearchAPIIntegrationTest
  

🤖 Generated with Claude Code

[github-actions]

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated, risking silent data loss in publishing pipelines
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex ignores HTTP 404 as benign but does not validate index existence before attempting delete; if index is missing, the operation silently succeeds despite invalid state
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:227 — putToIndex fan-out copies mutable SiteSearchResult per provider, but copyOf() only shallow-copies the map; if any downstream mutation modifies nested objects (e.g., Map<String, Object> values), concurrent writes corrupt state
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex logs raw exception message without context (index, docId, status) — obscures debugging in production
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex uses physicalName(idx) without validating idx is non-null or non-empty — NPE risk if called with malformed input
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex calls ESMappingAPIImpl.toJsonString(res.getMap()) — uses ES-specific JSON serializer on OpenSearch path; may produce incompatible field types (e.g., Date → string format mismatch)
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex logs "deleting from : " + idx + " url:" + docId — misleading; docId is not a URL
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex does not validate idx or res.getId() before constructing endpoint — risk of malformed OpenSearch request
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex uses ESMappingAPIImpl for JSON serialization — violates vendor isolation; should use OpenSearch-compatible mapper
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex does not check if index is closed or non-existent before DELETE — may trigger unexpected 400/404 from OpenSearch without recovery
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex does not use @WrapInTransaction for index write — if this is part of a larger transactional flow (e.g., content publish), failure to rollback on OpenSearch write leaves inconsistent state
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex calls res.getMap() directly — if SiteSearchResult.getMap() returns a mutable reference, concurrent access from fan-out may race
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex uses refresh=true — performance anti-pattern under high load; should be configurable or batched
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex uses docId as endpoint ID — assumes doc ID is URL-safe; no encoding or validation — risk of malformed OpenSearch request
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex does not validate res.getId() is non-null — NPE risk when constructing endpoint
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex logs "writing from : " + idx + " type: " + resultType + " url:" + res.getUrl() — logs sensitive URL path without masking; may expose internal paths or PII
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex uses res.getMap() — if SiteSearchResult is reused across threads (e.g., in batch), concurrent modification may occur
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex uses ESMappingAPIImpl.toJsonString — violates vendor neutrality; should use ObjectMapper from OSClientProvider or JsonpMapper
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex does not validate idx is a valid site-search index — may delete from unrelated index if alias or typo provided
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex does not check if index exists before PUT — may fail with 404 if index was deleted between listIndices and putToIndex

Existing

• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:357 — Prior finding still present: PUT document failures are logged but not propagated, risking silent data loss when OpenSearch write operations fail

Resolved

• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:88 — Replaced direct new ESIndexAPI() with APILocator usage — fixed constructor bypass
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:351 — Replaced ElasticsearchException with DotSearchException — vendor-neutral exception contract enforced
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:385 — mappingAPI.putMapping now uses IndexTag.ES — prevents dual-write to OS index
• ✅ dotCMS/src/enterprise/java/com/dotmarketing/business/APILocator.java:1484 — SITESEARCH_API now returns SiteSearchAPIImpl — router is now the single entry point
• ✅ dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java:36 — createSiteSearchIndex now throws DotSearchException — consistent with new contract
• ✅ dotCMS/src/main/java/com/dotcms/content/index/domain/Aggregation.java:88 — Added histogram bucket support — fixes aggregation deserialization
• ✅ dotCMS/src/main/java/com/dotcms/content/index/domain/AggregationBucket.java:92 — Added fromHistogram and histogramKey — fixes numeric/date histogram key handling
• ✅ dotCMS/src/main/java/com/dotcms/content/index/domain/DotSearchException.java — New vendor-neutral exception — replaces ElasticsearchException across API surface
• ✅ dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:173 — Replaced ES-specific InternalDateHistogram/StringTerms with vendor-neutral Aggregation/AggregationBucket — fixes type coupling
• ✅ dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:189 — isHistogram now checks type string — avoids class-cast exceptions
• ✅ dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:244 — InternalWrapperCountDateHistogramFacet now uses AggregationBucket.getKeyAsNumber() — fixes date histogram key conversion
• ✅ dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:280 — InternalWrapperStringTermsFacet now uses AggregationBucket.getKey() — fixes string key handling
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex now logs HTTP status — improves observability (though still swallows failure)

---

_{Run: #28107343243 · tokens: in: 19596 · out: 3443 · total: 23039}

[github-actions]

❌ Bedrock Review failed — qwen.qwen3-next-80b-a3b

The review job failed before producing output. See the run for details.

_{Run: #28114998972}

[fabrizzio-dotCMS]

Decision — neutral Aggregation vs ES Aggregation: equivalence scope & deliberate gaps

This records, as a reviewed decision, how far the vendor-neutral com.dotcms.content.index.domain.Aggregation / AggregationBucket are meant to be equivalent to the Elasticsearch types they replace on the VTL-facing surface ($sitesearch.getAggregations(...) / getFacets(...)), and which parts of the ES surface are intentionally not reproduced.

What "equivalence" means here

The ES org.elasticsearch.search.aggregations.Aggregation interface is minimal — getName(), getType(), getMetadata(). Templates never reached buckets through that interface (it has no bucket accessors); Velocity duck-types against the concrete runtime object (InternalTerms, InternalDateHistogram, TopHits, Terms.Bucket…). So the contract we preserve is the runtime accessor surface a template could realistically have walked, not the 3-method interface.

Covered (the common VTL path — what templates actually use)

ES runtime accessor	Neutral equivalent
agg.getName() / agg.getType()	getName() / getType()
(Terms/Histogram).getBuckets()	getBuckets()
bucket.getKeyAsString()	getKeyAsString()
bucket.getKeyAsNumber()	getKeyAsNumber() (lenient parse; date-histogram key normalized to epoch-millis)
bucket.getDocCount()	getDocCount()
bucket.getAggregations() (nested)	getAggregations()
topHits.getHits()… (id / source / score)	getHits() → SearchHits / SearchHit

This is verified end-to-end (SiteSearchWebAPITest, ES Phase 0), on the OpenSearch path (OSSiteSearchAPIIntegrationTest, fromOS), and at the factory level deterministically (AggregationDomainTest, incl. the date-histogram ZonedDateTime→epoch-millis conversion).

Deliberate gaps (NOT reproduced — accepted)

These ES members are intentionally not exposed on the neutral types. They are analytics/lookup methods with no observed use in Site Search VTL, and reproducing them would re-introduce vendor coupling or carry no OpenSearch counterpart:

1. Aggregation.getMetadata() — the per-aggregation meta map. Not surfaced.
2. Terms.getBucketByKey(String) — random bucket lookup. Templates iterate buckets instead; a template doing $agg.getBucketByKey("x") would break.
3. Terms.getSumOfOtherDocCounts() / Terms.Bucket.getDocCountError() / Terms.getDocCountError() — approximate-count error/overflow accounting. Not surfaced.
4. bucket.getKey() type narrowed Object → String — for a date histogram, ES getKey() returned a ZonedDateTime; the neutral getKey() returns the epoch-millis as a String. A template calling date methods on the raw key ($bucket.key.toInstant()) would break. The supported idioms are getKeyAsString() / getKeyAsNumber(), which are preserved.

Risk: a customer/site VTL template relying on (2), (3), or the Object shape of (4) would fail after this change. We assess this as low likelihood — these are not part of the documented Site Search facet/aggregation idiom — and accept it knowingly rather than re-couple the neutral API to vendor types.

Interaction with the H-8 rollback label

The deliberate gaps are a forward-compatibility consideration (a template adopting the neutral shape on N), distinct from the rollback (H-8) concern already flagged on this PR (a template adopting the new accessors getBuckets()/getHits()/iterator() and then being rolled back to N-1, whose getAggregations() returns the ES type). No template was co-migrated in this PR, so the H-8 risk window stays theoretical until one is. If/when a template is migrated to the neutral accessors, it must be tracked against the N+1 retirement of the old ES return type per H-8's two-phase guidance.

If a gap proves needed

Add the specific accessor to the neutral type (e.g. a metadata() map, or a getBucketByKey helper on Aggregation) backed by both from(...) and fromOS(...) factories, with a unit test in AggregationDomainTest — rather than leaking the vendor type back onto the API.

[fabrizzio-dotCMS]

Update — getMetadata() equivalence gap is now CLOSED

Following the equivalence-gaps decision above, one of the four gaps has been implemented: getMetadata().

Why this one, and not the others. It is the only gap that is rollback-safe: the ES org.elasticsearch.search.aggregations.Aggregation interface already exposes getMetadata(), so a template that adopts $agg.metadata resolves on both N (neutral Aggregation) and N-1 (ES Aggregation) after a rollback. Adding it therefore does not create a new H-8 vector — unlike getBuckets() / getHits(), which the ES interface lacks. It also completes equivalence with the exact interface the review anchored on: getName / getType / getMetadata (the third was the one missing).

Implementation (Aggregation.java):

• New 5th record component Map<String, Object> getMetadata (bean-named so $agg.metadata resolves via getMetadata()), with a null-safe empty-map default in the canonical constructor + a Builder.metadata(...).
• ES factory: .metadata(esAgg.getMetadata()) — already Map<String, Object>.
• OS factory: a fromOSMeta(...) helper unwraps each JsonData.to(Object.class) (falling back to the raw JSON string if it can't be mapped) from each variant's meta() (AggregateBase.meta(), inherited by the sterms/lterms/dterms/top_hits aggregates) — so the OpenSearch path is not left with a fresh divergence from ES.

Tests: AggregationDomainTest → 11/11 (added esFactory_metadata_isPreserved and aggregation_metadata_defaultsToEmptyWhenUnset). No existing test broke (unstubbed getMetadata() mocks return null → default empty map). dotcms-core install + dotcms-integration test-compile both BUILD SUCCESS (Java 25).

Still deliberately deferred (unchanged from the decision above): getBucketByKey(String), getSumOfOtherDocCounts(), getDocCountError(), and the getKey() Object→String narrowing — no observed consumer, and adding accessors the ES interface lacks would widen the H-8 rollback surface for no benefit. They remain "add when a consumer is proven, backed by both factories + a unit test."

[fabrizzio-dotCMS]

Review feedback addressed — pushed in ba8e9645cf

Summary of the changes that resolve the open review findings on this PR, plus the aggregation test gaps.

Findings resolved

Sev	Finding	Resolution
🔴	OSSiteSearchAPI.putToIndex swallows write failures (silent data loss)	Now throws DotSearchException on non-2xx / caught exceptions. It must propagate, not log-and-swallow: PhaseRouter is already phase-aware — it re-throws the primary provider's failure and swallows the shadow's (WARN). Swallowing inside the impl defeated that, so a Phase-3 (OS-primary) write failure was lost. Now publishing observes it in Phase 3; OS shadow drift in phases 1/2 is logged WARN by the router.
🔴	deleteFromIndex swallows failures	Same fix; HTTP 404 stays benign (idempotent delete).
🟠	setAlias returns false on success	Returns true (createAlias is void + throws on failure → reaching the return = success).
🟠	Missing indexName validation → NPE / invalid chars reach OpenSearch	New requireValidIndexName(idx) guard (null/blank, non-lowercase, OS-forbidden chars) fails fast with IllegalArgumentException before the name is interpolated into the REST endpoint.
🟠	Mapping/settings read via new File(url.getPath()) — NPEs if missing, fails inside a JAR	Read via getResourceAsStream (readResource helper, UTF-8, clear error if absent). JAR-safe.
🟡	Per-doc Logger.info in put/delete (log spam + URL at info)	Lowered to Logger.debug(..., Supplier) (no concatenation when off). Bulk delete log stays at info.
🟡	setAlias missing alias null/empty check	Already present (the `UtilMethods.isNotSet(indexName)

Deliberate scope note: ESSiteSearchAPI.putToIndex/deleteFromIndex also swallow failures (legacy, pre-existing). Left unchanged — in Phase 3 OS is the sole writer so the OS fix covers the critical case, and making the legacy ES path throw would surface previously-hidden failures to existing Phase-0/1 deployments (riskier, out of scope here).

Aggregation test coverage + equivalence

• AggregationDomainTest — added deterministic ES-factory unit tests (Mockito, no container), incl. the previously-untested date_histogram ZonedDateTime→epoch-millis key conversion. 11/11 pass.
• SiteSearchWebAPITest — added date_histogram cases (both getAggregations and the legacy getFacets wrapper).
• OSSiteSearchAPIIntegrationTest — strengthened the terms-agg assertion (bucket content/keys/counts, was only containsKey), added a nested top_hits case on the fromOS path, and index-name validation cases.
• getMetadata() equivalence gap closed — see the dedicated comment above; it's the one gap that's rollback-safe (the ES Aggregation interface exposes it too), so it completes interface equivalence without adding H-8 surface. getBucketByKey/getSumOfOtherDocCounts/getDocCountError remain deliberately deferred (no consumer; verified no shipped template uses $sitesearch aggregations).

Verification

./mvnw install -pl :dotcms-core and test-compile -pl :dotcms-integration → BUILD SUCCESS (Java 25). AggregationDomainTest 11/11. The OpenSearch ITs require the opensearch-upgrade container to run.

[github-actions]

❌ Bedrock Review failed — qwen.qwen3-next-80b-a3b

The review job failed before producing output. See the run for details.

_{Run: #28126292794}

[claude]

Claude finished @fabrizzio-dotCMS's task in 5m 32s —— View job

---

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get the full PR diff
• Analyze diff against all unsafe categories
• Apply appropriate label

---

Verdict: ✅ Safe To Rollback

I analyzed the diff against every category in the rollback-unsafe reference. No unsafe category is triggered. Here is the category-by-category reasoning:

Category	Finding
C-1 Structural data model change	No DB table/column changes. No runonce task. ✅
C-2 ES mapping change	ESSiteSearchAPI now calls mappingAPI.putMapping(List.of(indexName), mapping, IndexTag.ES) instead of the two-arg overload, but this is the same logical mapping pinned to the ES index — no field type, analyzer, or nesting change in the mapping content. The new os-sitesearch-mapping.json is a new OpenSearch resource, not a change to any live ES index. ✅
C-3 contentlet_as_json model version bump	Not touched. ✅
C-4 DROP TABLE / DROP COLUMN	None. ✅
H-1 One-way data migration / backfill	No data transformation tasks. ✅
H-2 RENAME TABLE / RENAME COLUMN	None. ✅
H-3 PK / unique constraint restructuring	None. ✅
H-4 New ContentType field type	None. ✅
H-5 Binary storage provider change	None. ✅
H-6 DROP PROCEDURE / DROP FUNCTION	None. ✅
H-7 NOT NULL column without default	No schema changes. ✅
H-8 VTL viewtool contract change	See detailed note below. ✅
M-1 Non-broadening column type change	None. ✅
M-2 Push publishing bundle format change	No bundle XML changes. ✅
M-3 REST / GraphQL API contract change	No REST endpoint signatures changed. ✅
M-4 OSGi interface change	SiteSearchAPI interface changes createSiteSearchIndex to throw DotSearchException instead of ElasticsearchException — see note below. ✅

---

H-8 — VTL Viewtool contract — detailed assessment

SiteSearchWebAPI is a registered viewtool ($sitesearch). This PR changes:

• The constructor signatures of the inner classes InternalWrapperCountDateHistogramFacet and InternalWrapperStringTermsFacet from List<InternalDateHistogram.Bucket> / List<Bucket> to List<AggregationBucket>. These constructors are called internally by getFacets() — they are never exposed to VTL directly.
• The public method getAggregations(String, String) returns Map<String, Aggregation> where Aggregation is now com.dotcms.content.index.domain.Aggregation instead of org.elasticsearch.search.aggregations.Aggregation. The H-8 category notes that this is only risky if a VTL template was co-migrated in this same PR to call accessors like getBuckets() / getHits() that the ES type lacked. No VTL templates are modified in this PR (per the PR description: "no template was co-migrated"), so no template currently uses the new neutral Aggregation's bucket accessors. The risk window stays theoretical until a template is migrated — at which point H-8's two-phase guidance applies.
• The getMetadata() gap-closure: Aggregation.getMetadata() is also exposed on org.elasticsearch.search.aggregations.Aggregation, so a template calling $agg.metadata resolves on both N and N-1 — this is rollback-safe by design (as documented in the PR).

M-4 — OSGi interface note

SiteSearchAPI.createSiteSearchIndex now throws DotSearchException (a RuntimeException-derived neutral exception) instead of ElasticsearchException. This is a broadening change — DotSearchException is an unchecked exception that any N-1 OSGi plugin already handles (or ignores) via the Exception catch hierarchy — not a narrowing or signature incompatibility that would cause NoSuchMethodError. ✅

[github-actions]

❌ Bedrock Review failed — qwen.qwen3-next-80b-a3b

The review job failed before producing output. See the run for details.

_{Run: #28127615180}

[github-actions]

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated, risking silent data loss in publishing pipelines
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex ignores HTTP 404 as benign but does not validate index existence before attempting delete; if index is missing, the operation silently succeeds despite invalid state
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:227 — putToIndex fan-out copies mutable SiteSearchResult per provider, but copyOf() only shallow-copies the map; if any downstream mutation modifies nested objects (e.g., Map<String, Object> values), concurrent writes corrupt state
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex logs raw exception message without context (index, docId, status) — obscures debugging in production
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex uses physicalName(idx) without validating idx is non-null or non-empty — NPE risk if called with malformed input
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — putToIndex calls ESMappingAPIImpl.toJsonString(res.getMap()) — uses ES-specific JSON serializer on OpenSearch path; may produce incompatible field types (e.g., Date → string format mismatch)
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex does not use @WrapInTransaction for index write; if underlying DB state (e.g., versioned indices) is mutated in same transaction, failure may leave index inconsistent with metadata — assumption: putToIndex is called within a transactional context that includes VersionedIndicesAPI writes; what to verify: whether SiteSearchAPIImpl.putToIndex is invoked from a @WrapInTransaction method
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex writes to OpenSearch without idempotency guard; retry or double-click may cause duplicate documents — assumption: OpenSearch _id is stable and derived from res.getId(); what to verify: whether res.getId() is guaranteed unique and stable across retries
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex does not validate docId is non-null or non-empty before constructing endpoint — risk of HTTP 400 or malformed URL — assumption: docId is always non-null from caller; what to verify: whether SiteSearchResult.getId() or upstream caller ever passes null/blank docId
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex uses ESMappingAPIImpl.toJsonString() — vendor-specific serializer used in OS path; may break if ES and OS JSON semantics diverge — assumption: ESMappingAPIImpl is compatible with OpenSearch JSON; what to verify: whether ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types (e.g., dates, booleans)

Existing

• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — prior finding still present: putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated, risking silent data loss in publishing pipelines
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — prior finding still present: deleteFromIndex ignores HTTP 404 as benign but does not validate index existence before attempting delete; if index is missing, the operation silently succeeds despite invalid state
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:227 — prior finding still present: putToIndex fan-out copies mutable SiteSearchResult per provider, but copyOf() only shallow-copies the map; if any downstream mutation modifies nested objects (e.g., Map<String, Object> values), concurrent writes corrupt state
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — prior finding still present: putToIndex logs raw exception message without context (index, docId, status) — obscures debugging in production
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — prior finding still present: deleteFromIndex uses physicalName(idx) without validating idx is non-null or non-empty — NPE risk if called with malformed input
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1046 — prior finding still present: putToIndex calls ESMappingAPIImpl.toJsonString(res.getMap()) — uses ES-specific JSON serializer on OpenSearch path; may produce incompatible field types (e.g., Date → string format mismatch)

Resolved

• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:379 — ElasticsearchException replaced with DotSearchException — now uses neutral exception type
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:634 — ElasticsearchException replaced with DotSearchException — now uses neutral exception type
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:648 — ElasticsearchException replaced with DotSearchException — now uses neutral exception type
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:669 — ElasticsearchException replaced with DotSearchException — now uses neutral exception type
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:683 — ElasticsearchException replaced with DotSearchException — now uses neutral exception type
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:1046 — mappingAPI.putMapping(indexName, mapping) replaced with mappingAPI.putMapping(List.of(indexName), mapping, IndexTag.ES) — now explicitly pins to ES index, avoiding dual-write to OS
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:64 — this(APILocator.getESIndexAPI(), ...) replaced with this(new ESIndexAPI(), ...) — now bypasses neutral router to avoid dual-write
• ✅ dotCMS/src/main/java/com/dotmarketing/business/APILocator.java:1484 — SITESEARCH_API now returns SiteSearchAPIImpl — router is now the single entry point, replacing direct ESSiteSearchAPI

---

_{Run: #28129849961 · tokens: in: 19522 · out: 3149 · total: 22671}

[fabrizzio-dotCMS]

Note for reviewers — the latest qwen Bedrock review is re-reporting already-resolved findings

The most recent automated review (model qwen.qwen3-next-80b-a3b, against 86bf0fbb44) lists several "New Issues 🔴/🟠" that are already fixed. It appears to be regurgitating the carried-forward prior findings block rather than re-reading the current code — the tell is that it cites OSSiteSearchAPI.java:1082 / :1136 / :1046, but the file is only 910 lines long, so those line numbers cannot exist.

Verified status of each, against the pushed code:

Reported (stale)	Actual current state
putToIndex silently swallows write failures	Fixed — now throw new DotSearchException(...) on non-2xx / caught exceptions (OSSiteSearchAPI.java ~595/606); PhaseRouter applies the per-phase primary/shadow policy.
deleteFromIndex no index/idx validation, NPE risk	Fixed — requireValidIndexName(idx) guard before the REST call (~553/647); 404 stays benign by design.
putToIndex logs raw exception without context	Fixed — the thrown message includes docId + index.
putToIndex uses ESMappingAPIImpl.toJsonString on the OS path	Intentional — faithful ES-format doc JSON is the documented design for the OS site-search doc body, not a regression.
SiteSearchAPIImpl:227 — copyOf() shallow-copy "concurrent writes corrupt state"	False positive — the fan-out is sequential, not concurrent; each provider gets an independent map copy; every putToIndex mutation is a top-level map.put(...) (entry replacement, not nested-object mutation), so a shallow copy is sufficient. This reasoning is already documented in the method's javadoc (SiteSearchAPIImpl.java ~240-245).

A prior review pass (deepseek r1) had already marked these Resolved; the later qwen pass simply didn't re-diff. The substantive review feedback was addressed in ba8e9645cf (see the "Review feedback addressed" comment) and 86bf0fbb44, and the rollback-safety check now labels the PR Safe To Rollback.

[github-actions]

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex uses ESMappingAPIImpl.toJsonString(res.getMap()) to serialize JSON for OpenSearch; this is vendor-specific ES logic that may produce incompatible field types (e.g., Date → string format mismatch), breaking OpenSearch indexing and search behavior.
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated beyond DotSearchException wrapping — risks silent data loss in publishing pipelines.
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:227 — putToIndex fan-out copies mutable SiteSearchResult per provider, but copyOf() only shallow-copies the map; if any downstream mutation modifies nested objects (e.g., Map<String, Object> values), concurrent writes corrupt state — same flaw as prior finding, now replicated in new code.
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex logs raw exception message without context (index, docId, status) — obscures debugging in production.
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex writes to OpenSearch without idempotency guard; retry or double-click may cause duplicate documents — assumption: OpenSearch _id is stable and derived from res.getId(); what to verify: whether res.getId() is guaranteed unique and stable across retries.
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex uses ESMappingAPIImpl.toJsonString() — vendor-specific serializer used in OS path; may break if ES and OS JSON semantics diverge — assumption: ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types (e.g., dates, booleans); what to verify: whether ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types.
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex does not use @WrapInTransaction for index write; if underlying DB state (e.g., versioned indices) is mutated in same transaction, failure may leave index inconsistent with metadata — assumption: putToIndex is called within a transactional context that includes VersionedIndicesAPI writes; what to verify: whether SiteSearchAPIImpl.putToIndex is invoked from a @WrapInTransaction method.
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex uses ESMappingAPIImpl.toJsonString() — vendor-specific serializer used in OS path; may break if ES and OS JSON semantics diverge — assumption: ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types (e.g., dates, booleans); what to verify: whether ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types.
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated, risking silent data loss in publishing pipelines
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex writes to OpenSearch without idempotency guard; retry or double-click may cause duplicate documents — assumption: OpenSearch _id is stable and derived from res.getId(); what to verify: whether res.getId() is guaranteed unique and stable across retries
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex does not use @WrapInTransaction for index write; if underlying DB state (e.g., versioned indices) is mutated in same transaction, failure may leave index inconsistent with metadata — assumption: putToIndex is called within a transactional context that includes VersionedIndicesAPI writes; what to verify: whether SiteSearchAPIImpl.putToIndex is invoked from a @WrapInTransaction method

Existing

• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated, risking silent data loss in publishing pipelines
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex uses ESMappingAPIImpl.toJsonString(res.getMap()) — uses ES-specific JSON serializer on OpenSearch path; may produce incompatible field types (e.g., Date → string format mismatch)
• 🔴 Critical: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:227 — putToIndex fan-out copies mutable SiteSearchResult per provider, but copyOf() only shallow-copies the map; if any downstream mutation modifies nested objects (e.g., Map<String, Object> values), concurrent writes corrupt state
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex logs raw exception message without context (index, docId, status) — obscures debugging in production
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex writes to OpenSearch without idempotency guard; retry or double-click may cause duplicate documents — assumption: OpenSearch _id is stable and derived from res.getId(); what to verify: whether res.getId() is guaranteed unique and stable across retries
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex uses ESMappingAPIImpl.toJsonString() — vendor-specific serializer used in OS path; may break if ES and OS JSON semantics diverge — assumption: ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types (e.g., dates, booleans); what to verify: whether ESMappingAPIImpl.toJsonString() outputs OpenSearch-compatible field types
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1819 — putToIndex does not use @WrapInTransaction for index write; if underlying DB state (e.g., versioned indices) is mutated in same transaction, failure may leave index inconsistent with metadata — assumption: putToIndex is called within a transactional context that includes VersionedIndicesAPI writes; what to verify: whether SiteSearchAPIImpl.putToIndex is invoked from a @WrapInTransaction method

Resolved

• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex silently swallows all exceptions during document write; failure to write to OpenSearch is logged but not propagated, risking silent data loss in publishing pipelines — now wrapped in DotSearchException and re-thrown, but still not propagated to caller in PhaseRouter
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex ignores HTTP 404 as benign but does not validate index existence before attempting delete — now validates index via requireValidIndexName and throws on malformed input
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1136 — deleteFromIndex uses physicalName(idx) without validating idx is non-null or non-empty — now validated by requireValidIndexName
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex uses ESMappingAPIImpl.toJsonString() — vendor-specific serializer used in OS path — still present, not resolved
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:1082 — putToIndex logs raw exception message without context — still present, not resolved
• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:227 — putToIndex fan-out copies mutable SiteSearchResult per provider, but copyOf() only shallow-copies the map — still present, not resolved

---

_{Run: #28176419832 · tokens: in: 21525 · out: 2752 · total: 24277}