refactor(search): split SearchHit into a lean content shape and a Site Search one (#36360)

[fabrizzio-dotCMS]

Summary

Follow-up to @wezell's review on #36886: content search results should stay small and fast, and
highlight fragments are a Site Search concern no other caller reads. Instead of every hit carrying
a field only one caller uses, SearchHit becomes a sealed interface over two records.

Stacked on #36886 (base is issue-36360-phase-sweep-fixes, not main) — the field this splits
only exists on that branch. Retarget to main once #36886 merges.

The shape

public sealed interface SearchHit permits ContentSearchHit, SiteSearchHit {
    // ... the six neutral accessors ...
    default List<String> highlightsFor(String field) { return List.of(); }
}

• ContentSearchHit — six components, no highlight state at all. What every content query produces.
• SiteSearchHit — the same six plus the fragments, overriding highlightsFor.

The default is what makes it work: the concern is declared once on the contract, costs zero bytes
per instance, and a caller holding a plain SearchHit never has to know or ask which shape it got.

Callers never choose the shape

Builder.build() picks it from the data — the lean record unless the engine actually returned
fragments. That matters because Site Search and content search reach the neutral layer through the
same factory (ContentSearchResponse.from, since Site Search reads via rawSearch after #36398),
so a "do I want highlights" flag would have to be threaded through rawSearch →
ContentSearchResponse.from → SearchHits.from → the hit adapter. Deciding from the response keeps
it in one place. Since the only queries that request highlighting are the two Site Search ones, a
content hit is always the lean shape.

Blast radius: none outside the domain package

./mvnw compile -pl :dotcms-core passes with zero changes anywhere else — the ten
SearchHits.from / SearchHit::from call sites, ContentSearchResponse, Aggregation,
ESContentResourcePortlet and OSSiteSearchAPI all compile untouched. Verified the enterprise
consumer really does resolve through the contract:

javap OSSiteSearchAPI.class
  429: invokeinterface  // SearchHit.highlightsFor:(Ljava/lang/String;)Ljava/util/List;

Jackson

An interface is not instantiable, so deserialization goes through a @JsonCreator static factory that
rebuilds via Builder.build(). The serialized form already carries the highlights key when there were
fragments, so the same rule that picks the shape coming from the engine picks it coming from JSON — no
polymorphic @JsonTypeInfo, so the JSON shape the query caches depend on is unchanged.

The first cut of this PR instead pinned deserialization to ContentSearchHit, which made a serialized
SiteSearchHit come back without its fragments and without an error. Nothing serializes one today, but
that is a silent-loss landmine of the same class as #36026, so it is fixed rather than documented.

Side benefit: ContentSearchHit no longer serializes an empty "highlights":{}.

Velocity

No template reads highlights (zero .vtl references in the repo), and Site Search results are not
exposed through a viewtool at all — those templates consume SiteSearchResult, which this does not
touch. The at-risk surface is the content hit accessors reachable as $hit.id / $hit.sourceAsMap /
$hit.sortValues, and those are guarded by ContentSearchToolTest, which evaluates real customer VTL
through the dotCMS Velocity engine.

Testing

All green against this branch. 25 unit + 83 integration tests, covering every family that consumes
the neutral hit:

Test	Covers	Result
SearchHitTest	shape decision, OS sort() unwrap, highlights, JSON round-trip both ways, field mapped to null	9/9
AggregationDomainTest	Jackson round-trip of SearchHit / SearchHits	16/16
ContentSearchToolTest	real Velocity: $hit.id / .index / .sourceAsMap / .sortValues on search() and raw(), plus the nested top_hits walk	11/11
ESContentResourcePortletTest	the /api/es/search and /api/es/raw wire shape	14/14
ContentletIndexAPIImplTest	content indexing + search, and the Site Search highlight assertions	16/16
ESContentFactoryImplTest	searchHits on the Elasticsearch factory path	42/42 (1 pre-existing skip)

Verified the ITs ran against the refactor rather than a stale artifact — the integration module resolves
dotcms-core from ~/.m2, so core was reinstalled first and the installed jar carries SearchHit as an
interface alongside the two records.

Coverage gap, stated plainly: the runs above are phase 0, where Site Search reads are served by
ESSiteSearchAPI — which uses Elasticsearch's own SearchHit and never touches the neutral type. So
SiteSearchHit end-to-end (OSSiteSearchAPI → neutral hit → fragments on the result) is currently
covered by unit tests only. Exercising it needs a phase 2/3 run, where OpenSearch serves reads.

Breaking Changes

None at the source level. SearchHit changes from a record to a sealed interface, so anything
compiled against the concrete type would need recompiling — nothing in the repo does, and the type is
internal to the neutral search layer.

🤖 Generated with Claude Code

This PR fixes: #36360

This PR fixes: #36360

[mergify]

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

• Queue this pull request

[claude]

Claude finished @fabrizzio-dotCMS's task in 1m 27s —— View job

---

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

---

Code Review — SearchHit sealed split

• Read the three domain files + test
• Trace SearchHit / getHighlights / highlightsFor call sites repo-wide (incl. enterprise + VTL)
• Verify Jackson round-trip and shape-selection logic
• Check the record→interface binary-break blast radius

New Issues

No new bugs found. The logic is sound and well-tested:

• Builder.build() selecting shape from highlights.isEmpty() is consistent between the engine path (from(...)) and the JSON path (fromJson), so an un-highlighted hit is always ContentSearchHit and a highlighted one is always SiteSearchHit. Round-trip tests pin both directions.
• SiteSearchHit.highlightsFor correctly uses get(...) == null ? List.of() rather than getOrDefault, which preserves the prior null-safety for a field explicitly mapped to null (the {"content": null} payload case). Verified the two enterprise consumers (OSSiteSearchAPI:368, ESSiteSearchAPI) call highlightsFor(...).toArray(...), so a null there would have NPE'd — the guard matters and is kept.
• The @JsonCreator fix (routing through Builder.build() instead of @JsonDeserialize(as = ContentSearchHit.class)) resolves the silent-fragment-loss path that the earlier cut of this PR introduced. Good catch on your own review.

Notes (non-blocking)

• 🟡 SearchHit.java:69 — Binary break + removed accessor, both already tracked. Two things change on the neutral contract:
  1. SearchHit goes record→interface (source-compatible in-repo, but binary-incompatible for anything compiled against the old concrete type). This is exactly @wezell's flag about the customer plugin that leverages the base ES classes — worth confirming that plugin resolves SearchHit through the interface (as OSSiteSearchAPI does via invokeinterface) and not the old record constructor before merge.
  2. The neutral contract no longer exposes a getHighlights() component — only highlightsFor(field). The old javadoc advertised Velocity reaching $hit.highlights.content; that path no longer resolves on a SearchHit reference (only SiteSearchHit has getHighlights, and Site Search results aren't exposed via a viewtool). I confirmed zero .getHighlights() Java callers outside SiteSearchHit and zero .vtl references to highlights/highlightsFor, so this is safe in-repo — flagging only because it's a contract narrowing a customer template could theoretically touch.

Both are trade-offs already documented in the PR body and comments, not defects. No code change requested from me.
· issue-36360-searchhit-sealed-split

[fabrizzio-dotCMS]

Closing: I built this to address @wezell's cohesion point on #36886, then measured what it actually bought and concluded it should not land. Full reasoning in #36886 (comment).

Short version: the shared contract still carries highlightsFor(field), Builder.highlights(...) and the ES Text[] flattening adapter, so all this really moved is which of the two records holds the map — a net saving of one reference field per content hit that already pointed at a shared Map.of() singleton. Against that it introduces a record→interface binary break and a new silent-data-loss path (@JsonDeserialize(as = ContentSearchHit.class) means a serialized SiteSearchHit round-trips back without its fragments).

The branch issue-36360-searchhit-sealed-split stays up. If we decide the separation is still worth it, the honest version is the one without highlightsFor on the interface, which trades the saving for a type check at the Site Search call site.

The one piece worth keeping landed on #36886 instead: two ContentSearchToolTest assertions matched only a literal template prefix and would have stayed green on a broken accessor.

[fabrizzio-dotCMS]

Reopened — @wezell is good with the shape:

public sealed interface SearchHit permits ContentSearchHit, SiteSearchHit {
    // ... the six neutral accessors ...
    default List<String> highlightsFor(String field) { return List.of(); }
}

One thing changed since I closed it. My objection had two parts: that the cohesion gain was small, which is a judgement call and yours to make, and that the first cut introduced a new silent-data-loss path, which was a real defect rather than a trade-off. d74911be8e fixes the second.

Deserialization no longer pins the interface to ContentSearchHit (which would have made a serialized SiteSearchHit come back without its fragments, silently). It goes through a @JsonCreator static factory that rebuilds via Builder.build(), so the same rule that picks the shape coming from the engine picks it coming from JSON. No polymorphic type discriminator, so the JSON shape the query caches depend on is unchanged. Covered both ways — 24/24 unit tests green.

Still worth flagging, since it is a trade-off rather than a bug: SearchHit goes from a record to an interface, so anything compiled against the concrete type needs recompiling. Nothing in the repo does.

[wezell]

On the Record --> Interface change - we should check our customer plugin who leverages the base ES classes to make sure we don't break it.