From 9baf7528f8867a9ec9e24b634de64de8d6fb05e3 Mon Sep 17 00:00:00 2001 From: Vacha Shah Date: Tue, 31 Oct 2023 12:33:35 -0700 Subject: [PATCH] Fixing Hit responses when search request has storedFields as none (#698) * Making Hit.id nullable when storedFields is none Signed-off-by: Vacha Shah * Adding test to cover storedFields none and replacing deprecated assertThat Signed-off-by: Vacha Shah * Adding changelog Signed-off-by: Vacha Shah * spotlessApply Signed-off-by: Vacha Shah --------- Signed-off-by: Vacha Shah --- 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 c25467baf1..80878af55c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ This section is for maintaining a changelog for all breaking changes for the cli ### 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 {