REST high-level client: Fix parsing of script fields (#28395)

Script fields can get a bit more complicated than just stored fields. A script can return null, an object and also an array. Extended parsing to support such valid values. Also renamed util method from `parseStoredFieldsValue` to `parseFieldsValue` given that it can parse stored fields but also script fields, anything that's returned as `fields`.

Closes #28380

client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java

@@ -65,6 +65,8 @@ import org.junit.Before;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.both;
@@ -431,6 +433,47 @@ public class SearchIT extends ESRestHighLevelClientTestCase {
         }
     }
 
+    public void testSearchWithWeirdScriptFields() throws Exception {
+        HttpEntity entity = new NStringEntity("{ \"field\":\"value\"}", ContentType.APPLICATION_JSON);
+        client().performRequest("PUT", "test/type/1", Collections.emptyMap(), entity);
+        client().performRequest("POST", "/test/_refresh");
+
+        {
+            SearchRequest searchRequest = new SearchRequest("test").source(SearchSourceBuilder.searchSource()
+                    .scriptField("result", new Script("null")));
+            SearchResponse searchResponse = execute(searchRequest, highLevelClient()::search, highLevelClient()::searchAsync);
+            SearchHit searchHit = searchResponse.getHits().getAt(0);
+            List<Object> values = searchHit.getFields().get("result").getValues();
+            assertNotNull(values);
+            assertEquals(1, values.size());
+            assertNull(values.get(0));
+        }
+        {
+            SearchRequest searchRequest = new SearchRequest("test").source(SearchSourceBuilder.searchSource()
+                    .scriptField("result", new Script("new HashMap()")));
+            SearchResponse searchResponse = execute(searchRequest, highLevelClient()::search, highLevelClient()::searchAsync);
+            SearchHit searchHit = searchResponse.getHits().getAt(0);
+            List<Object> values = searchHit.getFields().get("result").getValues();
+            assertNotNull(values);
+            assertEquals(1, values.size());
+            assertThat(values.get(0), instanceOf(Map.class));
+            Map<?, ?> map = (Map<?, ?>) values.get(0);
+            assertEquals(0, map.size());
+        }
+        {
+            SearchRequest searchRequest = new SearchRequest("test").source(SearchSourceBuilder.searchSource()
+                    .scriptField("result", new Script("new String[]{}")));
+            SearchResponse searchResponse = execute(searchRequest, highLevelClient()::search, highLevelClient()::searchAsync);
+            SearchHit searchHit = searchResponse.getHits().getAt(0);
+            List<Object> values = searchHit.getFields().get("result").getValues();
+            assertNotNull(values);
+            assertEquals(1, values.size());
+            assertThat(values.get(0), instanceOf(List.class));
+            List<?> list = (List<?>) values.get(0);
+            assertEquals(0, list.size());
+        }
+    }
+
     public void testSearchScroll() throws Exception {
 
         for (int i = 0; i < 100; i++) {

server/src/main/java/org/elasticsearch/common/document/DocumentField.java

@@ -36,7 +36,7 @@ import java.util.List;
 import java.util.Objects;
 
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.elasticsearch.common.xcontent.XContentParserUtils.parseStoredFieldsValue;
+import static org.elasticsearch.common.xcontent.XContentParserUtils.parseFieldsValue;
 
 /**
  * A single field name and values part of {@link SearchHit} and {@link GetResult}.
@@ -139,7 +139,7 @@ public class DocumentField implements Streamable, ToXContentFragment, Iterable<O
         ensureExpectedToken(XContentParser.Token.START_ARRAY, token, parser::getTokenLocation);
         List<Object> values = new ArrayList<>();
         while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-            values.add(parseStoredFieldsValue(parser));
+            values.add(parseFieldsValue(parser));
         }
         return new DocumentField(fieldName, values);
     }

server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java

@@ -39,8 +39,8 @@ public final class XContentParserUtils {
     }
 
     /**
-     * Makes sure that current token is of type {@link XContentParser.Token#FIELD_NAME} and the field name is equal to the provided one
+     * Makes sure that current token is of type {@link Token#FIELD_NAME} and the field name is equal to the provided one
-     * @throws ParsingException if the token is not of type {@link XContentParser.Token#FIELD_NAME} or is not equal to the given field name
+     * @throws ParsingException if the token is not of type {@link Token#FIELD_NAME} or is not equal to the given field name
      */
     public static void ensureFieldName(XContentParser parser, Token token, String fieldName) throws IOException {
         ensureExpectedToken(Token.FIELD_NAME, token, parser::getTokenLocation);
@@ -62,7 +62,7 @@ public final class XContentParserUtils {
     /**
      * @throws ParsingException with a "unknown token found" reason
      */
-    public static void throwUnknownToken(XContentParser.Token token, XContentLocation location) {
+    public static void throwUnknownToken(Token token, XContentLocation location) {
         String message = "Failed to parse object: unexpected token [%s] found";
         throw new ParsingException(location, String.format(Locale.ROOT, message, token));
     }
@@ -83,27 +83,36 @@ public final class XContentParserUtils {
      * Parse the current token depending on its token type. The following token types will be
      * parsed by the corresponding parser methods:
      * <ul>
-     *    <li>XContentParser.Token.VALUE_STRING: parser.text()</li>
+     *    <li>{@link Token#VALUE_STRING}: {@link XContentParser#text()}</li>
-     *    <li>XContentParser.Token.VALUE_NUMBER: parser.numberValue()</li>
+     *    <li>{@link Token#VALUE_NUMBER}: {@link XContentParser#numberValue()} ()}</li>
-     *    <li>XContentParser.Token.VALUE_BOOLEAN: parser.booleanValue()</li>
+     *    <li>{@link Token#VALUE_BOOLEAN}: {@link XContentParser#booleanValue()} ()}</li>
-     *    <li>XContentParser.Token.VALUE_EMBEDDED_OBJECT: parser.binaryValue()</li>
+     *    <li>{@link Token#VALUE_EMBEDDED_OBJECT}: {@link XContentParser#binaryValue()} ()}</li>
+     *    <li>{@link Token#VALUE_NULL}: returns null</li>
+     *    <li>{@link Token#START_OBJECT}: {@link XContentParser#mapOrdered()} ()}</li>
+     *    <li>{@link Token#START_ARRAY}: {@link XContentParser#listOrderedMap()} ()}</li>
      * </ul>
      *
-     * @throws ParsingException if the token none of the allowed values
+     * @throws ParsingException if the token is none of the allowed values
      */
-    public static Object parseStoredFieldsValue(XContentParser parser) throws IOException {
+    public static Object parseFieldsValue(XContentParser parser) throws IOException {
-        XContentParser.Token token = parser.currentToken();
+        Token token = parser.currentToken();
         Object value = null;
-        if (token == XContentParser.Token.VALUE_STRING) {
+        if (token == Token.VALUE_STRING) {
             //binary values will be parsed back and returned as base64 strings when reading from json and yaml
             value = parser.text();
-        } else if (token == XContentParser.Token.VALUE_NUMBER) {
+        } else if (token == Token.VALUE_NUMBER) {
             value = parser.numberValue();
-        } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
+        } else if (token == Token.VALUE_BOOLEAN) {
             value = parser.booleanValue();
-        } else if (token == XContentParser.Token.VALUE_EMBEDDED_OBJECT) {
+        } else if (token == Token.VALUE_EMBEDDED_OBJECT) {
             //binary values will be parsed back and returned as BytesArray when reading from cbor and smile
             value = new BytesArray(parser.binaryValue());
+        } else if (token == Token.VALUE_NULL) {
+            value = null;
+        } else if (token == Token.START_OBJECT) {
+            value = parser.mapOrdered();
+        } else if (token == Token.START_ARRAY) {
+            value = parser.listOrderedMap();
         } else {
             throwUnknownToken(token, parser.getTokenLocation());
         }
@@ -132,7 +141,7 @@ public final class XContentParserUtils {
      */
     public static <T> void parseTypedKeysObject(XContentParser parser, String delimiter, Class<T> objectClass, Consumer<T> consumer)
             throws IOException {
-        if (parser.currentToken() != XContentParser.Token.START_OBJECT && parser.currentToken() != XContentParser.Token.START_ARRAY) {
+        if (parser.currentToken() != Token.START_OBJECT && parser.currentToken() != Token.START_ARRAY) {
             throwUnknownToken(parser.currentToken(), parser.getTokenLocation());
         }
         String currentFieldName = parser.currentName();

server/src/main/java/org/elasticsearch/search/SearchHit.java

@@ -69,7 +69,7 @@ import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constru
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName;
-import static org.elasticsearch.common.xcontent.XContentParserUtils.parseStoredFieldsValue;
+import static org.elasticsearch.common.xcontent.XContentParserUtils.parseFieldsValue;
 import static org.elasticsearch.search.fetch.subphase.highlight.HighlightField.readHighlightField;
 
 /**
@@ -604,7 +604,7 @@ public final class SearchHit implements Streamable, ToXContentObject, Iterable<D
                     fieldMap.put(field.getName(), field);
                 }, (p, c) -> {
                     List<Object> values = new ArrayList<>();
-                    values.add(parseStoredFieldsValue(p));
+                    values.add(parseFieldsValue(p));
                     return new DocumentField(metadatafield, values);
                 }, new ParseField(metadatafield), ValueType.VALUE);
             }
@@ -649,7 +649,7 @@ public final class SearchHit implements Streamable, ToXContentObject, Iterable<D
         String description = null;
         List<Explanation> details = new ArrayList<>();
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            ensureExpectedToken(XContentParser.Token.FIELD_NAME, token, () -> parser.getTokenLocation());
+            ensureExpectedToken(XContentParser.Token.FIELD_NAME, token, parser::getTokenLocation);
             String currentFieldName = parser.currentName();
             token = parser.nextToken();
             if (Fields.VALUE.equals(currentFieldName)) {
@@ -657,7 +657,7 @@ public final class SearchHit implements Streamable, ToXContentObject, Iterable<D
             } else if (Fields.DESCRIPTION.equals(currentFieldName)) {
                 description = parser.textOrNull();
             } else if (Fields.DETAILS.equals(currentFieldName)) {
-                ensureExpectedToken(XContentParser.Token.START_ARRAY, token, () -> parser.getTokenLocation());
+                ensureExpectedToken(XContentParser.Token.START_ARRAY, token, parser::getTokenLocation);
                 while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
                     details.add(parseExplanation(parser));
                 }

server/src/test/java/org/elasticsearch/common/xcontent/XContentParserUtilsTests.java

@@ -21,6 +21,7 @@ package org.elasticsearch.common.xcontent;
 
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.common.CheckedBiConsumer;
+import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesArray;
@@ -32,12 +33,14 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
+import java.util.Map;
 
 import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.parseTypedKeysObject;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
 
 public class XContentParserUtilsTests extends ESTestCase {
 
@@ -54,39 +57,39 @@ public class XContentParserUtilsTests extends ESTestCase {
         }
     }
 
-    public void testParseStoredFieldsValueString() throws IOException {
+    public void testStoredFieldsValueString() throws IOException {
         final String value = randomAlphaOfLengthBetween(0, 10);
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> assertEquals(value, result));
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> assertEquals(value, result));
     }
 
-    public void testParseStoredFieldsValueInt() throws IOException {
+    public void testStoredFieldsValueInt() throws IOException {
         final Integer value = randomInt();
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> assertEquals(value, result));
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> assertEquals(value, result));
     }
 
-    public void testParseStoredFieldsValueLong() throws IOException {
+    public void testStoredFieldsValueLong() throws IOException {
         final Long value = randomLong();
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> assertEquals(value, result));
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> assertEquals(value, result));
     }
 
-    public void testParseStoredFieldsValueDouble() throws IOException {
+    public void testStoredFieldsValueDouble() throws IOException {
         final Double value = randomDouble();
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> assertEquals(value, ((Number) result).doubleValue(), 0.0d));
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> assertEquals(value, ((Number) result).doubleValue(), 0.0d));
     }
 
-    public void testParseStoredFieldsValueFloat() throws IOException {
+    public void testStoredFieldsValueFloat() throws IOException {
         final Float value = randomFloat();
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> assertEquals(value, ((Number) result).floatValue(), 0.0f));
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> assertEquals(value, ((Number) result).floatValue(), 0.0f));
     }
 
-    public void testParseStoredFieldsValueBoolean() throws IOException {
+    public void testStoredFieldsValueBoolean() throws IOException {
         final Boolean value = randomBoolean();
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> assertEquals(value, result));
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> assertEquals(value, result));
     }
 
-    public void testParseStoredFieldsValueBinary() throws IOException {
+    public void testStoredFieldsValueBinary() throws IOException {
         final byte[] value = randomUnicodeOfLength(scaledRandomIntBetween(10, 1000)).getBytes("UTF-8");
-        assertParseStoredFieldsValue(value, (xcontentType, result) -> {
+        assertParseFieldsSimpleValue(value, (xcontentType, result) -> {
             if (xcontentType == XContentType.JSON || xcontentType == XContentType.YAML) {
                 //binary values will be parsed back and returned as base64 strings when reading from json and yaml
                 assertArrayEquals(value, Base64.getDecoder().decode((String) result));
@@ -97,27 +100,50 @@ public class XContentParserUtilsTests extends ESTestCase {
         });
     }
 
-    public void testParseStoredFieldsValueUnknown() throws IOException {
+    public void testStoredFieldsValueNull() throws IOException {
+        assertParseFieldsSimpleValue(null, (xcontentType, result) -> assertNull(result));
+    }
+
+    public void testStoredFieldsValueObject() throws IOException {
+        assertParseFieldsValue((builder) -> builder.startObject().endObject(),
+                (xcontentType, result) -> assertThat(result, instanceOf(Map.class)));
+    }
+
+    public void testStoredFieldsValueArray() throws IOException {
+        assertParseFieldsValue((builder) -> builder.startArray().endArray(),
+                (xcontentType, result) -> assertThat(result, instanceOf(List.class)));
+    }
+
+    public void testParseFieldsValueUnknown() {
         ParsingException e = expectThrows(ParsingException.class, () ->
-                assertParseStoredFieldsValue(null, (x, r) -> fail("Should have thrown a parsing exception")));
+                assertParseFieldsValue((builder) -> {}, (x, r) -> fail("Should have thrown a parsing exception")));
         assertThat(e.getMessage(), containsString("unexpected token"));
     }
 
-    private void assertParseStoredFieldsValue(final Object value, final CheckedBiConsumer<XContentType, Object, IOException> consumer)
+    private void assertParseFieldsSimpleValue(final Object value, final CheckedBiConsumer<XContentType, Object, IOException> assertConsumer)
             throws IOException {
+        assertParseFieldsValue((builder) -> builder.value(value), assertConsumer);
+    }
+
+    private void assertParseFieldsValue(final CheckedConsumer<XContentBuilder, IOException> fieldBuilder,
+                                        final CheckedBiConsumer<XContentType, Object, IOException> assertConsumer) throws IOException {
         final XContentType xContentType = randomFrom(XContentType.values());
         try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
             final String fieldName = randomAlphaOfLengthBetween(0, 10);
 
             builder.startObject();
-            builder.field(fieldName, value);
+            builder.startArray(fieldName);
+            fieldBuilder.accept(builder);
+            builder.endArray();
             builder.endObject();
 
             try (XContentParser parser = createParser(builder)) {
                 ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
                 ensureFieldName(parser, parser.nextToken(), fieldName);
+                ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation);
                 assertNotNull(parser.nextToken());
-                consumer.accept(xContentType, XContentParserUtils.parseStoredFieldsValue(parser));
+                assertConsumer.accept(xContentType, XContentParserUtils.parseFieldsValue(parser));
+                ensureExpectedToken(XContentParser.Token.END_ARRAY, parser.nextToken(), parser::getTokenLocation);
                 ensureExpectedToken(XContentParser.Token.END_OBJECT, parser.nextToken(), parser::getTokenLocation);
                 assertNull(parser.nextToken());
             }

server/src/test/java/org/elasticsearch/search/SearchHitTests.java

@@ -56,6 +56,7 @@ import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
 import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
@@ -258,7 +259,7 @@ public class SearchHitTests extends ESTestCase {
         assertThat(results.getAt(1).getShard(), equalTo(target));
     }
 
-    public void testNullSource() throws Exception {
+    public void testNullSource() {
         SearchHit searchHit = new SearchHit(0, "_id", new Text("_type"), null);
 
         assertThat(searchHit.getSourceAsMap(), nullValue());
@@ -277,6 +278,73 @@ public class SearchHitTests extends ESTestCase {
         assertTrue(searchHit.hasSource());
     }
 
+    public void testWeirdScriptFields() throws Exception {
+        {
+            XContentParser parser = createParser(XContentType.JSON.xContent(), "{\n" +
+                    "  \"_index\": \"twitter\",\n" +
+                    "  \"_type\": \"tweet\",\n" +
+                    "  \"_id\": \"1\",\n" +
+                    "  \"_score\": 1.0,\n" +
+                    "  \"fields\": {\n" +
+                    "    \"result\": [null]\n" +
+                    "  }\n" +
+                    "}");
+            SearchHit searchHit = SearchHit.fromXContent(parser);
+            Map<String, DocumentField> fields = searchHit.getFields();
+            assertEquals(1, fields.size());
+            DocumentField result = fields.get("result");
+            assertNotNull(result);
+            assertEquals(1, result.getValues().size());
+            assertNull(result.getValues().get(0));
+        }
+        {
+            XContentParser parser = createParser(XContentType.JSON.xContent(), "{\n" +
+                    "  \"_index\": \"twitter\",\n" +
+                    "  \"_type\": \"tweet\",\n" +
+                    "  \"_id\": \"1\",\n" +
+                    "  \"_score\": 1.0,\n" +
+                    "  \"fields\": {\n" +
+                    "    \"result\": [{}]\n" +
+                    "  }\n" +
+                    "}");
+
+            SearchHit searchHit = SearchHit.fromXContent(parser);
+            Map<String, DocumentField> fields = searchHit.getFields();
+            assertEquals(1, fields.size());
+            DocumentField result = fields.get("result");
+            assertNotNull(result);
+            assertEquals(1, result.getValues().size());
+            Object value = result.getValues().get(0);
+            assertThat(value, instanceOf(Map.class));
+            Map<?, ?> map = (Map<?, ?>) value;
+            assertEquals(0, map.size());
+        }
+        {
+            XContentParser parser = createParser(JsonXContent.jsonXContent, "{\n" +
+                    "  \"_index\": \"twitter\",\n" +
+                    "  \"_type\": \"tweet\",\n" +
+                    "  \"_id\": \"1\",\n" +
+                    "  \"_score\": 1.0,\n" +
+                    "  \"fields\": {\n" +
+                    "    \"result\": [\n" +
+                    "      []\n" +
+                    "    ]\n" +
+                    "  }\n" +
+                    "}");
+
+            SearchHit searchHit = SearchHit.fromXContent(parser);
+            Map<String, DocumentField> fields = searchHit.getFields();
+            assertEquals(1, fields.size());
+            DocumentField result = fields.get("result");
+            assertNotNull(result);
+            assertEquals(1, result.getValues().size());
+            Object value = result.getValues().get(0);
+            assertThat(value, instanceOf(List.class));
+            List<?> list = (List<?>) value;
+            assertEquals(0, list.size());
+        }
+    }
+
     private static Explanation createExplanation(int depth) {
         String description = randomAlphaOfLengthBetween(5, 20);
         float value = randomFloat();