Skip to content

Commit

Permalink
Fixing Hit responses when search request has storedFields as none (#698
Browse files Browse the repository at this point in the history
…) (#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 9baf752)

Signed-off-by: Vacha Shah <vachshah@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 89a37b0 commit 3491c33
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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 {
Expand Down

0 comments on commit 3491c33

Please sign in to comment.