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

[Backport] [2.x] Fixing InnerHits storedFields ("stored_fields") generated Json name. (#781) #794

Merged
merged 1 commit into from
Jan 2, 2024
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 @@ -42,6 +42,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### Fixed
- Fixed Hit response when search request has storedFields as null ([#698](https://github.com/opensearch-project/opensearch-java/pull/698))
- Fix InnerHits storedFields deserialization/serialization ([#781](https://github.com/opensearch-project/opensearch-java/pull/781)

### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public class InnerHits implements JsonpSerializable {
@Nullable
private final SourceConfig source;

private final List<String> storedField;
private final List<String> storedFields;

@Nullable
private final Boolean trackScores;
Expand All @@ -114,7 +114,7 @@ private InnerHits(Builder builder) {
this.fields = ApiTypeHelper.unmodifiable(builder.fields);
this.sort = ApiTypeHelper.unmodifiable(builder.sort);
this.source = builder.source;
this.storedField = ApiTypeHelper.unmodifiable(builder.storedField);
this.storedFields = ApiTypeHelper.unmodifiable(builder.storedFields);
this.trackScores = builder.trackScores;
this.version = builder.version;

Expand Down Expand Up @@ -225,10 +225,10 @@ public final SourceConfig source() {
}

/**
* API name: {@code stored_field}
* API name: {@code stored_fields}
*/
public final List<String> storedField() {
return this.storedField;
public final List<String> storedFields() {
return this.storedFields;
}

/**
Expand Down Expand Up @@ -344,10 +344,10 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {
this.source.serialize(generator, mapper);

}
if (ApiTypeHelper.isDefined(this.storedField)) {
generator.writeKey("stored_field");
if (ApiTypeHelper.isDefined(this.storedFields)) {
generator.writeKey("stored_fields");
generator.writeStartArray();
for (String item0 : this.storedField) {
for (String item0 : this.storedFields) {
generator.write(item0);

}
Expand Down Expand Up @@ -414,7 +414,7 @@ public static class Builder extends ObjectBuilderBase implements ObjectBuilder<I
private SourceConfig source;

@Nullable
private List<String> storedField;
private List<String> storedFields;

@Nullable
private Boolean trackScores;
Expand Down Expand Up @@ -623,22 +623,22 @@ public final Builder source(Function<SourceConfig.Builder, ObjectBuilder<SourceC
}

/**
* API name: {@code stored_field}
* API name: {@code stored_fields}
* <p>
* Adds all elements of <code>list</code> to <code>storedField</code>.
* Adds all elements of <code>list</code> to <code>storedFields</code>.
*/
public final Builder storedField(List<String> list) {
this.storedField = _listAddAll(this.storedField, list);
public final Builder storedFields(List<String> list) {
this.storedFields = _listAddAll(this.storedFields, list);
return this;
}

/**
* API name: {@code stored_field}
* API name: {@code stored_fields}
* <p>
* Adds one or more values to <code>storedField</code>.
* Adds one or more values to <code>storedFields</code>.
*/
public final Builder storedField(String value, String... values) {
this.storedField = _listAdd(this.storedField, value, values);
public final Builder storedFields(String value, String... values) {
this.storedFields = _listAdd(this.storedFields, value, values);
return this;
}

Expand Down Expand Up @@ -696,7 +696,7 @@ protected static void setupInnerHitsDeserializer(ObjectDeserializer<InnerHits.Bu
op.add(Builder::fields, JsonpDeserializer.arrayDeserializer(JsonpDeserializer.stringDeserializer()), "fields");
op.add(Builder::sort, JsonpDeserializer.arrayDeserializer(SortOptions._DESERIALIZER), "sort");
op.add(Builder::source, SourceConfig._DESERIALIZER, "_source");
op.add(Builder::storedField, JsonpDeserializer.arrayDeserializer(JsonpDeserializer.stringDeserializer()), "stored_field");
op.add(Builder::storedFields, JsonpDeserializer.arrayDeserializer(JsonpDeserializer.stringDeserializer()), "stored_fields");
op.add(Builder::trackScores, JsonpDeserializer.booleanDeserializer(), "track_scores");
op.add(Builder::version, JsonpDeserializer.booleanDeserializer(), "version");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.json.stream.JsonParser;
import java.io.StringReader;
import java.util.List;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.Assert;
import org.junit.Test;
Expand All @@ -16,15 +18,19 @@ public class PutIndexTemplateRequestTest extends Assert {
@Test
public void deserialize_validFieldsIncluded_RequestIsBuilt() throws JsonProcessingException {
final JsonpMapper mapper = new JsonbJsonpMapper();
final Map<String, Object> indexTemplateMap = Map.of("name", "test", "index_patterns", "*", "create", true, "priority", 1);
final Map<String, Object> indexTemplateMap = new HashMap<>();
indexTemplateMap.put("name", "test");
indexTemplateMap.put("index_patterns", "*");
indexTemplateMap.put("create", true);
indexTemplateMap.put("priority", 1);

final String indexTemplate = new ObjectMapper().writeValueAsString(indexTemplateMap);
final var parser = mapper.jsonProvider().createParser(new StringReader(indexTemplate));
final JsonParser parser = mapper.jsonProvider().createParser(new StringReader(indexTemplate));

final PutIndexTemplateRequest putIndexTemplateRequest = PutIndexTemplateRequest._DESERIALIZER.deserialize(parser, mapper);

assertEquals(putIndexTemplateRequest.name(), "test");
assertEquals(putIndexTemplateRequest.indexPatterns(), List.of("*"));
assertEquals(putIndexTemplateRequest.indexPatterns(), Collections.singletonList("*"));
assertEquals((int) putIndexTemplateRequest.priority(), 1);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.opensearch.client.opensearch.core.search;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import jakarta.json.stream.JsonGenerator;
import jakarta.json.stream.JsonParser;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.List;
import org.junit.Test;
import org.opensearch.client.json.JsonpMapper;
import org.opensearch.client.json.JsonpSerializable;
import org.opensearch.client.json.jsonb.JsonbJsonpMapper;
import org.opensearch.client.opensearch.core.SearchRequest;

public class InnerHitsTest {
private final JsonpMapper mapper = new JsonbJsonpMapper();
private final String storedSalary = "details.salary";
private final String storedJobId = "details.jobId";

/**
* test if the json field for storedFields is generated with the correct name "stored_fields"
*/
@Test
public void testInnerHitStoredFields() {
InnerHits hits = InnerHits.of((it) -> it.storedFields(List.of("field1", "field2")));
assertTrue(toJson(hits).contains("stored_fields"));
}

/**
* test if the field "stored_fields" is present after deserialization/serialization
* of InnerHits
*/
@Test
public void testInnerHitFromParsed() {
JsonParser parser = mapper.jsonProvider().createParser(new StringReader(innerHitsJson));
InnerHits innerHits = InnerHits._DESERIALIZER.deserialize(parser, mapper);
assertThat(innerHits.storedFields(), containsInAnyOrder(storedJobId, storedSalary));
String actualJson = toJson(innerHits);
assertEquals(innerHitsJson, actualJson);

}

/**
* We test if the field "stored_fields" is present in the InnerHits after deserialization/serialization
* of a SearchRequest
*/
@Test
public void testRequestWithInnerHitFromParsed() {
JsonParser parser = mapper.jsonProvider().createParser(new StringReader(searchRequestJson));
SearchRequest searchRequest = SearchRequest._DESERIALIZER.deserialize(parser, mapper);
InnerHits innerHits = searchRequest.query().bool().must().get(1).nested().innerHits();
assertThat(innerHits.storedFields(), containsInAnyOrder(storedJobId, storedSalary));
String actualJson = toJson(searchRequest);
assertEquals(searchRequestJson, actualJson);
}

private String toJson(JsonpSerializable obj) {
StringWriter stringWriter = new StringWriter();
try (JsonGenerator generator = mapper.jsonProvider().createGenerator(stringWriter)) {
mapper.serialize(obj, generator);
}
return stringWriter.toString();
}

private final String innerHitsJson = String.format("{\"_source\":false,\"stored_fields\":[\"%s\",\"%s\"]}", storedJobId, storedSalary);
private final String searchRequestJson = String.format(
"{\"_source\":false,\"query\":{\"bool\":{\"must\":[{\"match_all\":{}},{\"nested\":{\"inner_hits\":%s,\"path\":\"details\","
+ "\"query\":{\"match_all\":{}}}}]}},\"stored_fields\":[\"title\",\"companyName\"]}",
innerHitsJson
);
}
Loading