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

[claude]

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

---

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get full PR diff
• Analyze diff against all unsafe categories
• Post results and apply labels

---

Pull Request Unsafe to Rollback!!!

• Category: H-8 — VTL Viewtool Contract Change

• Risk Level: 🟠 HIGH

• Why it's unsafe: SiteSearchWebAPI is registered in WEB-INF/toolbox.xml under the $sitesearch key (request scope). This PR changes getAggregations(String, String) return type from Map<String, org.elasticsearch.search.aggregations.Aggregation> (the ES interface) to Map<String, com.dotcms.content.index.domain.Aggregation> (a neutral record). These are entirely different types with different accessor sets. The new neutral Aggregation record exposes getBuckets(), getHits(), and iterator() — accessors that do not exist on the old ES Aggregation interface. If any Velocity template written during the N release cycle calls $agg.buckets, $agg.hits, or iterates the aggregation directly, those templates will fail on rollback to N-1. N-1's getAggregations() still returns the ES Aggregation interface which lacks those methods. No templates were co-migrated in this PR, but the risk window is open for the entire release cycle: any template that adopts the new contract will break on N-1.

• Code that makes it unsafe:

  // dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java
  // Return type changed on the viewtool-exposed method:
  - Map<String, org.elasticsearch.search.aggregations.Aggregation> getAggregations(...);
  + Map<String, com.dotcms.content.index.domain.Aggregation> getAggregations(...);
  
  // dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:148
  // Viewtool method now returns the neutral record type via:
  public Map<String, Aggregation> getAggregations(final String indexName, final String query)
          throws DotDataException {
      return siteSearchAPI.getAggregations(indexName, query);
  }
  
  // The neutral Aggregation record adds new accessors not on the old ES interface:
  // dotCMS/src/main/java/com/dotcms/content/index/domain/Aggregation.java
  + List<AggregationBucket> getBuckets()   // new accessor (vector 2b)
  + SearchHits getHits()                   // new accessor (vector 2b)
  + Iterator<AggregationBucket> iterator() // new accessor (vector 2b)

• Alternative (if possible): Apply the two-phase contract change from H-8:

  • Release N (this PR): Ship the new neutral Aggregation record, but ensure it either (a) also exposes any method a template might chain from the old ES Aggregation type, or (b) document that no templates should be migrated to use getBuckets()/getHits() until N+1.
  • Release N+1: After N-1 is fully retired, any compat shims can be removed.
  • The key requirement: do not co-migrate templates to the new accessors until the rollback window has closed. Document $sitesearch.getAggregations() as the affected viewtool key in release notes.

[github-actions]

🤖 Bedrock Review — deepseek.v3.2

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:328 — setAlias method incorrectly returns false on success. The method calls indexApi.createAlias(indexName, alias) but then returns false, which contradicts the expected behavior (should return true on success). This could cause callers to misinterpret the operation's outcome.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:330 — Missing null/empty check for alias parameter in setAlias. The method validates both indexName and alias but only throws an IllegalArgumentException if both are not set. It should also handle the case where alias is empty or null individually, as the error message suggests.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:336 — Potential NullPointerException when reading resource files. The code assumes url.getPath() is non-null, but classLoader.getResource(...) could return null if the resource is not found. This would cause an NPE when calling new File(url.getPath()).

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:336-338 — Hardcoded resource paths may not be portable. Using File with url.getPath() assumes a file-system URL, which may fail in certain deployment environments (e.g., JAR packaging). Consider using InputStream instead.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:445 — Missing validation for resultType parameter in putToIndex. The method logs resultType but does not validate it; while not critical, unvalidated input in logs could be a minor security concern if it contains malicious characters.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:447 — Potential NullPointerException when res.getMap() returns null. The code accesses res.getMap().put(...) and res.getMap().get(...) without checking if the map is null. This could happen if SiteSearchResult is improperly constructed.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:455 — Inconsistent logging level: Logger.info for each document write in putToIndex could produce excessive log output in production. Consider lowering to debug or making it conditional.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:462 — Hardcoded endpoint path "/" + physicalName(idx) + "/_doc/" + res.getId() assumes OpenSearch's _doc type. While OpenSearch 3.x uses _doc, this is a vendor-specific detail that should be centralized or configurable.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:468 — Error handling in putToIndex only logs the HTTP status; it does not throw an exception or propagate failure. This could silently ignore indexing failures.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:486 — Suppressed warnings @SuppressWarnings({"unchecked", "rawtypes"}) in getFromIndex indicate unsafe type casting. While likely safe due to OpenSearch's JSON response, it's a code smell.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:505 — In deleteFromIndex, logging includes the docId which could be sensitive (e.g., containing PII). Ensure docId is sanitized or avoid logging it at info level.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:511 — Error handling in deleteFromIndex treats HTTP 404 as benign, which is correct, but other error statuses are only logged. Consider throwing an exception for non-404 errors to signal failure.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:752 — In rawSearch, the method catches IOException and wraps it in DotSearchException, but other exceptions (e.g., JSONException from new JSONObject(query)) are caught as Exception and may lose context. Ensure all relevant exceptions are properly handled.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:145 — In isDefaultIndex, the method catches Exception and wraps it in DotDataException, which could mask the original exception type. It should preserve the original exception if possible.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:177 — In createSiteSearchIndex, the method catches Exception and wraps it in IOException, which may not be appropriate for all exception types (e.g., DotSearchException). It should rethrow the original exception if it's already a DotSearchException or IOException.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:195 — In activateIndex, the method catches Exception and wraps it in DotDataException, which could obscure the root cause. It should rethrow DotDataException as is.

🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:204 — In deactivateIndex, the method catches Exception and wraps it in DotDataException, but IOException should be rethrown as is. The catch block should handle DotDataException and IOException separately.

🟡 Medium dotCMS/src/main/java/com/dotcms/content/index/domain/AggregationBucket.java:115 — In histogramKey, the method converts ZonedDateTime to epoch millis, but the conversion may lose sub-millisecond precision. Ensure this is acceptable for downstream use.

🟡 Medium dotCMS/src/main/java/com/dotcms/content/index/domain/AggregationBucket.java:118 — In histogramKey, converting Number to longValue() may truncate floating-point values. Consider preserving the original number type if needed.

🟡 Medium dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:190 — In isHistogram, the check type.contains("histogram") is fragile; it could match unrelated aggregation types. Consider a more precise check (e.g., type.endsWith("_histogram")).

🟡 Medium dotCMS/src/main/java/com/dotmarketing/sitesearch/viewtool/SiteSearchWebAPI.java:213 — In InternalWrapperCountDateHistogramFacet, the key may be null if entry.getKeyAsNumber() returns null. The code assigns time = 0L in that case, which may not be correct. Handle null appropriately.

🟠 High dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:336 — Security: File path constructed from classpath resource URL (url.getPath()) could be vulnerable to path traversal if the resource URL is manipulated. Ensure the resource is trusted (it's internal, so low risk).

🟠 High dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:336-338 — Security: Reading files via File with paths derived from classloader resources could expose the server's file system if the resource path is attacker-controlled. Since resources are internal, risk is low, but consider using getResourceAsStream instead.

🟡 Medium dotcms-integration/src/test/java/com/dotcms/content/index/opensearch/OSSiteSearchAPIIntegrationTest.java:300 — Test truncation: The diff ends abruptly, indicating the test file may be incomplete. Ensure the full test suite is included and properly structured.

Note: The PR introduces a new OpenSearch implementation for Site Search with a phase-aware router, which is a significant architectural change. The code generally follows dotCMS conventions and includes comprehensive

---

_{Run: #28054257576 · tokens: in: 19743 · out: 2048 · total: 21791}

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-8 — VTL Viewtool Contract Change
• Risk Level: 🟠 HIGH
• Why it's unsafe: SiteSearchWebAPI is registered in WEB-INF/toolbox.xml under $sitesearch (request scope). Its getAggregations(String, String) method's return type was swapped from Map<String, org.elasticsearch.search.aggregations.Aggregation> (ES interface) to Map<String, com.dotcms.content.index.domain.Aggregation> (neutral record). These are different types with different accessor sets. The new neutral Aggregation record exposes getBuckets(), getHits(), and iterator() — accessors that do not exist on the old ES Aggregation interface. If any Velocity template during the N release cycle calls $agg.buckets, $agg.hits, or iterates the aggregation directly (all new accessors exclusive to the neutral record), rolling back to N-1 breaks those templates at render time: N-1's getAggregations() returns the ES Aggregation interface which lacks those methods. No templates were co-migrated in this PR, but the risk window is open for the entire N release cycle: any template written or modified to adopt the new contract will fail on N-1 rollback. Per the H-8 reference, absent proof that no templates will use these new accessors, the risk is rated HIGH.
• Code that makes it unsafe:

  // dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java
  // Return type changed on the viewtool-exposed method:
  - Map<String, org.elasticsearch.search.aggregations.Aggregation> getAggregations(...);
  + Map<String, com.dotcms.content.index.domain.Aggregation> getAggregations(...);
  
  // dotCMS/src/main/java/com/dotcms/content/index/domain/Aggregation.java
  // New accessors not on the old ES Aggregation interface:
  + List<AggregationBucket> getBuckets()   // Vector 2b: new accessor
  + SearchHits getHits()                   // Vector 2b: new accessor
  + Iterator<AggregationBucket> iterator() // Vector 2b: new accessor
  
  // toolbox.xml registration (unchanged, but confirms VTL exposure):
  <key>sitesearch</key>
  <class>com.dotmarketing.sitesearch.viewtool.SiteSearchWebAPI</class>
  

• Alternative (if possible): Apply the two-phase contract change from H-8:
  • Release N (this PR): Ship the new neutral Aggregation record with the new accessors, but also keep getAggregations() returning the old ES Aggregation type (or an intermediate adapter that implements both the ES interface and exposes the new accessors). This allows templates on N and N-1 to both resolve correctly.
  • Release N+1: Remove the old ES Aggregation from the return type once N-1 is outside the rollback window.
  • Alternatively, since no templates were co-migrated in this PR, document this as rollback-unsafe in the release notes and identify $sitesearch.getAggregations() as the affected viewtool key before any templates adopt the new getBuckets()/getHits() accessors.

[github-actions]

🤖 Bedrock Review — us.deepseek.r1-v1:0

New Issues

• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:93 — Constructor directly instantiates ESIndexAPI() instead of using APILocator, potentially bypassing required initialization and causing NPEs in dependent services
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:189 — Dual-write operations lack error recovery — partial failures could leave ES/OS indices inconsistent with no rollback
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:483 — Missing input validation on indexName parameter allows invalid characters through to OpenSearch client

Existing

• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:106 — Merged index lists in dual-write phase could return duplicates if same logical name exists in both ES and OS clusters

---

_{Run: #28056199569 · tokens: in: 20424 · out: 1336 · total: 21760}

[github-actions]

🤖 Bedrock Review — us.deepseek.r1-v1:0

New Issues

• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:189 — Dual-write operations lack error recovery — partial failures could leave ES/OS indices inconsistent
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:482 — Missing input validation on indexName parameter allows invalid characters
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:357 — PUT document failures are logged but not propagated, risking silent data loss

Existing

• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:93 — Constructor directly instantiates ESIndexAPI() instead of using APILocator, potentially bypassing required initialization
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:189 — Dual-write operations lack error recovery — partial failures could leave ES/OS indices inconsistent
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:483 — Missing input validation on indexName parameter allows invalid characters

Resolved

• ✅ dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:106 — Merged index lists use LinkedHashSet to deduplicate entries

---

_{Run: #28060964611 · tokens: in: 20687 · out: 1621 · total: 22308}

[github-actions]

🤖 Bedrock Review — us.deepseek.r1-v1:0

New Issues

• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:357 — PUT document failures are logged but not propagated, risking silent data loss when OpenSearch write operations fail
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:189 — Dual-write operations lack transactional error handling — partial failures leave ES/OS indices inconsistent
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:482 — Missing validation on indexName parameter allows invalid OpenSearch index names (special chars, uppercase)

Existing

• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java:93 — Constructor directly instantiates ESIndexAPI() instead of using APILocator, potentially bypassing required initialization
• 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:189 — Dual-write operations lack error recovery — partial failures could leave ES/OS indices inconsistent
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:482 — Missing input validation on indexName parameter allows invalid characters
• 🟡 Medium: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:357 — PUT document failures are logged but not propagated, risking silent data loss

---

_{Run: #28064221231 · tokens: in: 20669 · out: 2540 · total: 23209}