Skip to content

Commit

Permalink
Fixing InnerHits storedFields ("stored_fields") generated Json name. (o…
Browse files Browse the repository at this point in the history
…pensearch-project#781) (opensearch-project#794)

* dummy CHANGELOG.md



* corrected the InnerHits storedFields json field name
to "stored_fields"



* adding correct PR information to CHANGELOG.md



* ran ./gradlew :java-client:spotlessApply



---------


(cherry picked from commit 0729a92)

Signed-off-by: brentam <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: brentam <[email protected]>
  • Loading branch information
reta and brentam authored Jan 2, 2024
1 parent 081a17e commit 58416a5
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 22 deletions.
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
);
}

0 comments on commit 58416a5

Please sign in to comment.