Fixing Hit responses when search request has storedFields as none (#698)

* Making Hit.id nullable when storedFields is none Signed-off-by: Vacha Shah <[email protected]> * Adding test to cover storedFields none and replacing deprecated assertThat Signed-off-by: Vacha Shah <[email protected]> * Adding changelog Signed-off-by: Vacha Shah <[email protected]> * spotlessApply Signed-off-by: Vacha Shah <[email protected]> --------- Signed-off-by: Vacha Shah <[email protected]>
opensearch-project · Oct 31, 2023 · 9baf752 · 9baf752
1 parent 59bb4db
commit 9baf752
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 19 deletions.
diff --git 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
@@ -108,7 +108,7 @@ public class Hit<TDocument> implements JsonpSerializable {
     private Hit(Builder<TDocument> 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<TDocument> extends ObjectBuilderBase implements ObjectBuilder<Hit<TDocument>> {
         private String index;
 
+        @Nullable
         private String id;
 
         @Nullable
@@ -480,9 +484,9 @@ public final Builder<TDocument> index(String value) {
         }
 
         /**
-         * Required - API name: {@code _id}
+         * API name: {@code _id}
          */
-        public final Builder<TDocument> id(String value) {
+        public final Builder<TDocument> id(@Nullable String value) {
             this.id = value;
             return this;
         }

diff --git a/...ent/src/test/java/org/opensearch/client/opensearch/integTest/AbstractSearchRequestIT.java b/...ent/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<ShopItem> 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<ShopItem> 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 {