Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.dotcms.content.index.domain;

import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

/**
* Immutable domain representation of a single search result hit from any search engine.
Expand Down Expand Up @@ -35,6 +38,9 @@
* @param getScore the search relevance score for this hit
* @param getFields the document fields retrieved by the search query (additional fields beyond
* the source document that were explicitly requested), empty if none were requested
* @param getSortValues the per-hit sort values the engine returns when the query sorts by a field
* (e.g. the computed distance for a {@code _geo_distance} sort), in the order the
* {@code sort} clause declared; empty for relevance-only (unsorted) queries
* @author Fabrizio Araya
* @see SearchHits
* @see com.dotcms.content.index.ContentFactoryIndexOperations
Expand All @@ -44,15 +50,17 @@ public record SearchHit(
@JsonProperty("index") String getIndex,
@JsonProperty("sourceAsMap") Map<String, Object> getSourceAsMap,
@JsonProperty("score") float getScore,
@JsonProperty("fields") Map<String, Object> getFields) {
@JsonProperty("fields") Map<String, Object> getFields,
@JsonProperty("sortValues") List<Object> getSortValues) {

/**
* Canonical constructor. Collection components default to an empty map when {@code null} so the
* accessors never return {@code null} (mirrors the previous Immutables collection defaults).
* Canonical constructor. Collection components default to an empty map/list when {@code null} so
* the accessors never return {@code null} (mirrors the previous Immutables collection defaults).
*/
public SearchHit {
getSourceAsMap = getSourceAsMap == null ? Map.of() : getSourceAsMap;
getFields = getFields == null ? Map.of() : getFields;
getSortValues = getSortValues == null ? List.of() : getSortValues;
}

/**
Expand All @@ -71,12 +79,14 @@ public static Builder builder() {
* @return a new SearchHit instance
*/
public static SearchHit from(org.elasticsearch.search.SearchHit esSearchHit) {
final Object[] esSortValues = esSearchHit.getSortValues();
return builder()
.id(esSearchHit.getId())
.sourceAsMap(esSearchHit.getSourceAsMap())
.fields(esSearchHit.getFields())
.score(esSearchHit.getScore())
.index(esSearchHit.getIndex())
.sortValues(esSortValues == null ? null : Arrays.asList(esSortValues))
.build();
}

Expand Down Expand Up @@ -108,11 +118,22 @@ public static SearchHit from(org.opensearch.client.opensearch.core.search.Hit<?>
sourceMap = Map.of();
}

// OpenSearch returns per-hit sort values as a List<FieldValue> tagged union; unwrap each to
// its raw scalar (Double/Long/Boolean/String, or null) so the neutral hit mirrors ES's Object[].
final List<org.opensearch.client.opensearch._types.FieldValue> osSortValues = osHit.sort();
final List<Object> sortValues = (osSortValues == null || osSortValues.isEmpty())
? null
: osSortValues.stream()
.map(fieldValue -> (fieldValue == null || fieldValue.isNull())
? null : fieldValue._get())
.collect(Collectors.toList());

return builder()
.id(osHit.id())
.index(osHit.index())
.sourceAsMap(sourceMap)
.score(osHit.score() != null ? osHit.score().floatValue() : 0.0f)
.sortValues(sortValues)
.build();
}

Expand All @@ -128,6 +149,7 @@ public static final class Builder {
private Map<String, Object> sourceAsMap = Map.of();
private float score;
private Map<String, Object> fields = Map.of();
private List<Object> sortValues = List.of();

public Builder id(final String id) {
this.id = id;
Expand Down Expand Up @@ -156,8 +178,13 @@ public Builder fields(final Map<String, ?> fields) {
return this;
}

public Builder sortValues(final List<Object> sortValues) {
this.sortValues = sortValues == null ? List.of() : sortValues;
return this;
}

public SearchHit build() {
return new SearchHit(id, index, sourceAsMap, score, fields);
return new SearchHit(id, index, sourceAsMap, score, fields, sortValues);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,23 @@ private static JSONObject hitsToLegacyJson(final SearchHits hits) throws JSONExc
.put("relation", total.relation() == Relation.EQUAL_TO ? "eq" : "gte"));
final JSONArray arr = new JSONArray();
for (final SearchHit hit : hits.getHits()) {
arr.put(new JSONObject()
final JSONObject hitJson = new JSONObject()
.put("_id", hit.getId())
.put("_index", hit.getIndex())
.put("_score", finiteOrNull(hit.getScore()))
.put("_source", new JSONObject(hit.getSourceAsMap())));
.put("_source", new JSONObject(hit.getSourceAsMap()));
// Preserve the engine's native per-hit "sort" array (e.g. the _geo_distance value that
// field-sorted queries depend on). Only emitted when the query actually sorted by a
// field — relevance-only queries carry no sort values and get no "sort" key, matching
// the native ES/OS wire format. Non-finite entries are coerced to null like _score.
if (!hit.getSortValues().isEmpty()) {
final JSONArray sortArr = new JSONArray();
for (final Object sortValue : hit.getSortValues()) {
sortArr.put(finiteOrNull(sortValue));
}
hitJson.put("sort", sortArr);
}
arr.put(hitJson);
}
hitsObj.put("hits", arr);
return hitsObj;
Expand All @@ -379,6 +391,26 @@ private static Object finiteOrNull(final float value) {
return Float.isFinite(value) ? Float.valueOf(value) : JSONObject.NULL;
}

/**
* Object overload of {@link #finiteOrNull(float)} for per-hit {@code sort} values, which arrive as
* boxed scalars ({@link Double} for a {@code _geo_distance} sort, {@link Long}, {@link String}, …).
* Non-finite floating-point values are coerced to {@code null} for the same reason as {@code _score};
* every other value (including {@code null}) passes through unchanged.
*/
private static Object finiteOrNull(final Object value) {
if (value == null) {
return JSONObject.NULL;
}
if (value instanceof Double) {
final double doubleValue = (Double) value;
return Double.isFinite(doubleValue) ? value : JSONObject.NULL;
}
if (value instanceof Float) {
return finiteOrNull(((Float) value).floatValue());
}
return value;
}

/** Maps the neutral aggregation tree (keyed by aggregation name) to the ES-native {@code aggregations} JSON. */
private static JSONObject aggregationsToLegacyJson(final Map<String, Aggregation> tree)
throws JSONException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package com.dotcms.content.index.domain;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import java.util.Arrays;
import java.util.List;
import org.junit.Test;
import org.opensearch.client.opensearch._types.FieldValue;
import org.opensearch.client.opensearch.core.search.Hit;

/**
* Unit tests for {@link SearchHit} conversion from vendor types, focused on the per-hit
* {@code sortValues} mapping restored for
* <a href="https://github.com/dotCMS/core/issues/36581">#36581</a>. Exercises the OpenSearch
* {@code Hit.sort()} unwrap branch (a {@code List<FieldValue>} tagged union), which the ES-only
* integration environment does not cover.
*/
public class SearchHitTest {

/**
* Method to test: {@link SearchHit#from(Hit)}
* Given scenario: an OpenSearch hit whose {@code sort()} carries a double and a string FieldValue.
* Expected result: {@code getSortValues()} unwraps each FieldValue to its raw scalar, in order.
*/
@Test
public void from_openSearchHit_unwrapsSortValues() {
final Hit<Object> osHit = Hit.of(builder -> builder
.index("idx")
.id("1")
.sort(Arrays.asList(FieldValue.of(11.82d), FieldValue.of("abc"))));

final SearchHit hit = SearchHit.from(osHit);

final List<Object> sortValues = hit.getSortValues();
assertEquals("both sort values must survive the conversion", 2, sortValues.size());
assertEquals(11.82d, ((Number) sortValues.get(0)).doubleValue(), 0.0001d);
assertEquals("abc", sortValues.get(1));
}

/**
* Method to test: {@link SearchHit#from(Hit)}
* Given scenario: an OpenSearch hit with no {@code sort()} values (a relevance-only query).
* Expected result: {@code getSortValues()} is empty (never null), so serializers omit the
* {@code sort} key instead of emitting an empty array.
*/
@Test
public void from_openSearchHit_noSort_yieldsEmptySortValues() {
final Hit<Object> osHit = Hit.of(builder -> builder.index("idx").id("1"));

final SearchHit hit = SearchHit.from(osHit);

assertTrue("a hit without a sort clause must expose empty sort values",
hit.getSortValues().isEmpty());
}

/**
* Method to test: {@link SearchHit#from(Hit)}
* Given scenario: the {@code sort()} list contains a raw Java {@code null} element (defensive
* against a client that yields nulls).
* Expected result: the conversion does not NPE and maps the null element to a null entry.
*/
@Test
public void from_openSearchHit_nullSortElement_doesNotThrow() {
final Hit<Object> osHit = Hit.of(builder -> builder
.index("idx")
.id("1")
.sort(Arrays.asList(FieldValue.of(1.0d), null)));

final SearchHit hit = SearchHit.from(osHit);

final List<Object> sortValues = hit.getSortValues();
assertEquals(2, sortValues.size());
assertEquals(1.0d, ((Number) sortValues.get(0)).doubleValue(), 0.0001d);
assertTrue("a null FieldValue element must map to null, not throw", sortValues.get(1) == null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,25 @@ public class ContentSearchToolTest extends IntegrationTestBase {
#end
""";

/**
* A field-sorted {@code $estool.search(...)} whose per-hit {@code sort} values the template reads
* via {@code $hit.sortValues} (property → {@code getSortValues()}) and
* {@code $hit.getSortValues().get(0)}. Guards the Velocity-facing side of
* <a href="https://github.com/dotCMS/core/issues/36581">#36581</a>: the neutral {@link SearchHit}
* now carries the sort values and exposes them under a bean-style {@code get}-accessor, so VTL
* templates that read the per-hit sort value (e.g. the {@code moddate} sort key, or a
* {@code _geo_distance} distance) resolve it instead of silently getting {@code null}.
*/
private static final String SEARCH_SORT_VTL = """
#set($esQuery = '{"size":5,"query":{"bool":{"filter":[{"term":{"live":true}}]}},"sort":[{"moddate":"desc"}]}')
#set($results = $estool.search($esQuery))
#foreach($hit in $results.hits.hits)
hit id: $!{hit.id}
sortValues: $!{hit.sortValues}
firstSort: $!{hit.getSortValues().get(0)}
#end
""";

/**
* Locks the same non-aggregation accessor surface for {@code $estool.raw(...)} —
* {@link ContentSearchResponse}. Because {@code raw()} returns a <i>record</i>, its top-level
Expand Down Expand Up @@ -635,6 +654,25 @@ public void searchVtl_rendersHitFieldsAndTiming() throws Exception {
output.contains("inode"));
}

/**
* Velocity-facing regression for <a href="https://github.com/dotCMS/core/issues/36581">#36581</a>:
* a field-sorted {@code $estool.search(...)} must expose the per-hit sort value in VTL. Asserts
* {@code $hit.sortValues} renders a non-empty array and {@code $hit.getSortValues().get(0)} renders
* the sort key (the {@code moddate} epoch here) — proving the neutral {@link SearchHit} both carries
* the value and exposes it under a bean {@code get}-accessor that Velocity resolves.
*/
@Test
public void searchVtl_rendersPerHitSortValues() throws Exception {
final String output = VelocityUtil.eval(SEARCH_SORT_VTL, velocityContext());
Logger.info(this, "\n===== search sort VTL output =====\n" + output + "\n================================");

assertTrue("a field-sorted search() must expose a non-empty per-hit sort array via "
+ "$hit.sortValues (getSortValues())",
Pattern.compile("sortValues:\\s*\\[[^\\]]+\\]").matcher(output).find());
assertTrue("$hit.getSortValues().get(0) must render the moddate sort key as a number",
Pattern.compile("firstSort:\\s*\\d+").matcher(output).find());
}

/**
* Gap closer for the {@code raw()} sibling: exercises the hit &amp; timing accessor surface of
* {@code $estool.raw(...)} — a {@link ContentSearchResponse} record — through Velocity. Because
Expand Down
Loading
Loading