From 3491c3336c6fff2a25df6d4a7388024fa0340c90 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:45:25 -0400 Subject: [PATCH] Fixing Hit responses when search request has storedFields as none (#698) (#699) * Making Hit.id nullable when storedFields is none * Adding test to cover storedFields none and replacing deprecated assertThat * Adding changelog * spotlessApply --------- (cherry picked from commit 9baf7528f8867a9ec9e24b634de64de8d6fb05e3) Signed-off-by: Vacha Shah Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- CHANGELOG.md | 1 + .../client/opensearch/core/search/Hit.java | 16 +++-- .../integTest/AbstractSearchRequestIT.java | 65 +++++++++++++++---- 3 files changed, 63 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15b2f77ac0..879cae3d7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Removed ### Fixed +- Fixed Hit response when search request has storedFields as null ([#698](https://github.com/opensearch-project/opensearch-java/pull/698)) ### Security diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java index 0050f9da69..dab34ab94a 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java @@ -108,7 +108,7 @@ public class Hit implements JsonpSerializable { private Hit(Builder builder) { this.index = ApiTypeHelper.requireNonNull(builder.index, this, "index"); - this.id = ApiTypeHelper.requireNonNull(builder.id, this, "id"); + this.id = builder.id; this.score = builder.score; this.explanation = builder.explanation; this.fields = ApiTypeHelper.unmodifiable(builder.fields); @@ -141,8 +141,9 @@ public final String index() { } /** - * Required - API name: {@code _id} + * API name: {@code _id} */ + @Nullable public final String id() { return this.id; } @@ -283,8 +284,10 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { generator.writeKey("_index"); generator.write(this.index); - generator.writeKey("_id"); - generator.write(this.id); + if (this.id != null) { + generator.writeKey("_id"); + generator.write(this.id); + } if (this.score != null) { generator.writeKey("_score"); @@ -418,6 +421,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { public static class Builder extends ObjectBuilderBase implements ObjectBuilder> { private String index; + @Nullable private String id; @Nullable @@ -480,9 +484,9 @@ public final Builder index(String value) { } /** - * Required - API name: {@code _id} + * API name: {@code _id} */ - public final Builder id(String value) { + public final Builder id(@Nullable String value) { this.id = value; return this; } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractSearchRequestIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractSearchRequestIT.java index 96ee498894..19f3af617f 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractSearchRequestIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractSearchRequestIT.java @@ -8,12 +8,9 @@ package org.opensearch.client.opensearch.integTest; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.arrayContaining; -import static org.hamcrest.Matchers.hasSize; - import java.io.IOException; +import java.util.Arrays; +import java.util.stream.Collectors; import org.junit.Test; import org.opensearch.client.opensearch._types.FieldValue; import org.opensearch.client.opensearch._types.SortOrder; @@ -27,9 +24,9 @@ public abstract class AbstractSearchRequestIT extends OpenSearchJavaClientTestCase { @Test - public void shouldReturnSearcheResults() throws Exception { - final String index = "searches_request"; - assertThat( + public void shouldReturnSearchResults() throws Exception { + final String index = "search_request"; + assertTrue( javaClient().indices() .create( b -> b.index(index) @@ -39,8 +36,7 @@ public void shouldReturnSearcheResults() throws Exception { ) .settings(settings -> settings.sort(s -> s.field("name").order(SegmentSortOrder.Asc))) ) - .acknowledged(), - equalTo(true) + .acknowledged() ); createTestDocuments(index); @@ -61,10 +57,53 @@ public void shouldReturnSearcheResults() throws Exception { ); final SearchResponse response = javaClient().search(request, ShopItem.class); - assertThat(response.hits().hits(), hasSize(2)); + assertEquals(response.hits().hits().size(), 2); + + assertTrue( + Arrays.stream(response.hits().hits().get(0).fields().get("name").to(String[].class)) + .collect(Collectors.toList()) + .contains("hummer") + ); + assertTrue( + Arrays.stream(response.hits().hits().get(1).fields().get("name").to(String[].class)) + .collect(Collectors.toList()) + .contains("jammer") + ); + } + + @Test + public void shouldReturnSearchResultsWithoutStoredFields() throws Exception { + final String index = "search_request"; + assertTrue( + javaClient().indices() + .create( + b -> b.index(index) + .mappings( + m -> m.properties("name", Property.of(p -> p.keyword(v -> v.docValues(true)))) + .properties("size", Property.of(p -> p.keyword(v -> v.docValues(true)))) + ) + .settings(settings -> settings.sort(s -> s.field("name").order(SegmentSortOrder.Asc))) + ) + .acknowledged() + ); + + createTestDocuments(index); + javaClient().indices().refresh(); - assertThat(response.hits().hits().get(0).fields().get("name").to(String[].class), arrayContaining("hummer")); - assertThat(response.hits().hits().get(1).fields().get("name").to(String[].class), arrayContaining("jammer")); + final Query query = Query.of( + q -> q.bool( + builder -> builder.filter(filter -> filter.term(TermQuery.of(term -> term.field("size").value(FieldValue.of("huge"))))) + ) + ); + + final SearchRequest request = SearchRequest.of( + r -> r.index(index).sort(s -> s.field(f -> f.field("name").order(SortOrder.Asc))).query(query).storedFields("_none_") + ); + + final SearchResponse response = javaClient().search(request, ShopItem.class); + assertEquals(response.hits().hits().size(), 2); + assertNull(response.hits().hits().get(0).id()); + assertNull(response.hits().hits().get(1).id()); } private void createTestDocuments(String index) throws IOException {