fix(es-search): restore per-hit sort values in /api/es/search (#36581)

[fabrizzio-dotCMS]

Proposed Changes

Fixes #36581.

/api/es/search (and /api/es/raw) silently dropped the per-hit sort array from the response. When a query sorts by a field — e.g. a _geo_distance sort — Elasticsearch/OpenSearch return the computed sort value on each hit under hits.hits[i].sort; for a geo sort that value is the distance clients display. Since the phase-aware SearchAPI cutover (#36398, first released v26.07.04-01) the legacy ES-wire response is rebuilt manually and the sort element was lost. #36480 (the non-finite _score fix) only touched _score; it did not restore sort, and the track_scores: true workaround does not bring it back.

Root cause (two layers)

1. Model — com.dotcms.content.index.domain.SearchHit had no component for per-hit sort values, so SearchHit.from(esSearchHit) / from(osHit) discarded getSortValues() / sort() on ingestion.
2. Serialization — ESContentResourcePortlet.hitsToLegacyJson() emitted only _id/_index/_score/_source.

Fix (symmetric ES/OS)

• SearchHit: add a sortValues component, populated from esSearchHit.getSortValues() (ES Object[]) and osHit.sort() (OpenSearch List<FieldValue>, unwrapped via _get()). Defaults to an empty list, so JSON (de)serialization of cached hits stays backward-compatible.
• hitsToLegacyJson: emit a sort array per hit only when sort values are present — relevance-only queries get no sort key, matching the native engine wire format. Non-finite entries are coerced to null (new finiteOrNull(Object) overload), consistent with the _score handling.

Both /api/es/search (GET + POST) and /api/es/raw share toLegacyEsJson, so all are covered.

Testing

Integration tests added to ESContentResourcePortletTest (registered in MainSuite2a):

• test_search_geoDistanceSort_emitsPerHitSortValues — a _geo_distance-sorted query returns each hit with a sort array carrying the finite distance, in ascending order (nearest ~0 km).
• test_search_relevanceOnlyQuery_omitsPerHitSort — a relevance-only query returns hits with no sort key.

Verified end-to-end on a local build in migration Phase 0: the customer's exact _geo_distance query now returns sort: [0.0003, 11.82, 35.03, 71.57] km per hit — identical to the native Elasticsearch response — with _score still null.

	before fix	native ES	with fix
per-hit sort	❌ absent	✅ [0.0003, 11.82, 35.03, 71.57]	✅ identical

🤖 Generated with Claude Code

This PR fixes: #36581

[claude]

Claude finished @fabrizzio-dotCMS's task in 9m 12s —— View job

---

Rollback-Safety Analysis — PR #36582

• Read docs/core/ROLLBACK_UNSAFE_CATEGORIES.md
• Get full PR diff (base 328cddf8/origin/main parent → head 65cc31bd)
• Analyze diff against every category (C-1..C-4, H-1..H-8, M-1..M-4)
• Post verdict / label

Verdict: Unsafe to rollback — 🟠 HIGH, category H-8 (VTL Viewtool Contract Change)

The fix adds a new getSortValues() record component to com.dotcms.content.index.domain.SearchHit, which is a type reached transitively from the $estool viewtool (ESContentTool, registered in toolbox.xml) via $estool.search(...) and $estool.raw(...). That's H-8's "vector 2b": a new accessor on an object returned by a viewtool that templates can adopt. The PR's own new test (ContentSearchToolTest#searchVtl_rendersPerHitSortValues) exercises $hit.sortValues / $hit.getSortValues() directly from VTL, confirming the accessor is reachable. If a customer template adopts $hit.sortValues post-deploy, a rollback to N-1 removes that method — Velocity render fails for the affected page/template (blank/error output), though the app boots fine and no data is lost.

Full finding with code references posted here: #36582 (comment)

Label AI: Not Safe To Rollback added.

Other categories checked and ruled out: no DB migration/runonce task, no ES mapping change, no contentlet_as_json version bump, no DROP/RENAME, no PK change, no new ContentType field type, no storage provider change, no stored procedure change, no NOT NULL-without-default column, no column type change, no push-publishing bundle format change, no OSGi interface change. The /api/es/search JSON response gains a new optional "sort" key (M-3 REST contract territory) but per-field JSON additions are additive/backward-compatible for REST clients, so that alone wouldn't have been flagged — the H-8 viewtool exposure is the actual blocker.

[fabrizzio-dotCMS]

Thanks for the review 🙏 Addressed the two actionable items in 7c3f53f:

• 🟡 OS FieldValue NPE (SearchHit.java): added a null-element guard in SearchHit.from(osHit) — fieldValue == null || fieldValue.isNull() — so a raw null in Hit.sort() can no longer NPE (defensive; the client is not known to emit nulls).
• OS-path coverage: added SearchHitTest unit test exercising the OpenSearch sort-unwrap branch (double/string FieldValue, empty sort, and a null element), which the ES-only integration environment does not cover. 3/3 green.

Re: the finiteOrNull(Object) note on the missing-geo sentinel — leaving as-is intentionally: raw JSON cannot carry Infinity/NaN anyway, so coercing to null is the safe (and only serializable) choice, consistent with _score.

[ihoffmann-dot]

No critical or warn findings. This is a tight, well-scoped fix: null-safe defaults on the new sortValues record component, correct unwrap of the OpenSearch FieldValue tagged union, sort key omitted for relevance-only queries (matching native wire format), and solid test coverage (unit tests for the OS branch, integration test for the ES geo-distance path).

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-8 — VTL Viewtool Contract Change (vector 2b: new accessor on a type returned transitively from a viewtool)
• Risk Level: 🟠 HIGH
• Why it's unsafe: SearchHit is a domain record returned transitively from the $estool viewtool (ESContentTool, registered in WEB-INF/toolbox.xml under key estool). Both $estool.raw(query) → ContentSearchResponse.hits().getHits() and $estool.search(query) → ContentSearchResults.hits.hits hand SearchHit instances to Velocity templates. This PR adds a brand-new getSortValues() record component to SearchHit (exposed to VTL as $hit.sortValues). This is exactly the H-8 "vector (2b)" case the reference doc calls out: "the return type is the same class, but that class gained a new accessor in N that templates adopted... $result.newAccessor() fails on N-1, which lacks the method." The PR itself proves the accessor is reachable and intended for template consumption — it adds dotcms-integration/.../ContentSearchToolTest.java#searchVtl_rendersPerHitSortValues, whose SEARCH_SORT_VTL template calls $hit.sortValues and $hit.getSortValues().get(0) directly. If any customer-authored VTL template (container/template/widget stored in the DB, which survives a binary rollback) adopts $hit.sortValues after this release ships — the whole point of restoring per-hit sort/distance data — a rollback to N-1 removes that method. Velocity's dynamic method resolution then fails at render time for any page rendering that template: blank/partial/error output, even though the app itself boots fine and no data is lost. Per the doc, this is not provably confined to one scope-limited template (any template touching $estool.search()/$estool.raw() hits could adopt it), so it defaults to HIGH rather than MEDIUM.
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/content/index/domain/SearchHit.java:54 — new record component @JsonProperty("sortValues") List<Object> getSortValues, plus its population in from(esSearchHit) (line 82-90) and from(osHit) (line 121-137).
  • dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/ESContentTool.java:64-88 — the $estool.search()/$estool.raw() viewtool methods that transitively hand SearchHit objects to VTL templates.
  • dotCMS/src/main/webapp/WEB-INF/toolbox.xml:300-304 — confirms ESContentTool is registered as the live $estool viewtool key.
  • dotcms-integration/src/test/java/com/dotcms/rendering/velocity/viewtools/ContentSearchToolTest.java (new SEARCH_SORT_VTL constant and searchVtl_rendersPerHitSortValues test) — demonstrates $hit.sortValues / $hit.getSortValues() resolve today, i.e. the exact contract a customer template could adopt going forward.
• Alternative (if possible): The PR already follows the safer half of H-8's two-phase pattern by shipping the accessor without migrating any shipped default template/container to depend on it — so N-1 isn't broken by dotCMS's own templates. The remaining exposure is customer-authored VTL adopting $hit.sortValues post-deploy. Recommend documenting in release notes that a rollback after customers begin using $hit.sortValues (or the new REST "sort" array) in custom VTL will only break rendering for the specific templates/pages using it — no data loss, no reindex needed, and it self-heals once the customer template is edited or the fix is redeployed.

[mergify]

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

• Queue this pull request