Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 31, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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

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
Loading