Require timeout units when parsing query body

Today when parsing the timeout field in a query body, if time units are
supplied the parser throws a NumberFormatException. Addtionally, the
parsing allows the timeout field to not specify units (it assumes
milliseconds). This commit fixes this behavior by not only allowing time
units to be specified but requires time units to be specified. This is
consistent with the documented behavior and the behavior in 2.x.

Relates #19077

core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

@@ -21,7 +21,6 @@ package org.elasticsearch.search.builder;
 
 import com.carrotsearch.hppc.ObjectFloatHashMap;
 import com.carrotsearch.hppc.cursors.ObjectCursor;
-
 import org.elasticsearch.action.support.ToXContentToBytes;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
@@ -42,7 +41,6 @@ import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.search.aggregations.AggregationBuilder;
-import org.elasticsearch.search.slice.SliceBuilder;
 import org.elasticsearch.search.aggregations.AggregatorFactories;
 import org.elasticsearch.search.aggregations.AggregatorParsers;
 import org.elasticsearch.search.aggregations.PipelineAggregationBuilder;
@@ -51,6 +49,7 @@ import org.elasticsearch.search.highlight.HighlightBuilder;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.search.rescore.RescoreBuilder;
 import org.elasticsearch.search.searchafter.SearchAfterBuilder;
+import org.elasticsearch.search.slice.SliceBuilder;
 import org.elasticsearch.search.sort.ScoreSortBuilder;
 import org.elasticsearch.search.sort.SortBuilder;
 import org.elasticsearch.search.sort.SortBuilders;
@@ -959,7 +958,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ
                 } else if (context.getParseFieldMatcher().match(currentFieldName, SIZE_FIELD)) {
                     size = parser.intValue();
                 } else if (context.getParseFieldMatcher().match(currentFieldName, TIMEOUT_FIELD)) {
-                    timeoutInMillis = parser.longValue();
+                    timeoutInMillis = TimeValue.parseTimeValue(parser.text(), null, TIMEOUT_FIELD.getPreferredName()).millis();
                 } else if (context.getParseFieldMatcher().match(currentFieldName, TERMINATE_AFTER_FIELD)) {
                     terminateAfter = parser.intValue();
                 } else if (context.getParseFieldMatcher().match(currentFieldName, MIN_SCORE_FIELD)) {
@@ -1105,7 +1104,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ
         }
 
         if (timeoutInMillis != -1) {
-            builder.field(TIMEOUT_FIELD.getPreferredName(), timeoutInMillis);
+            builder.field(TIMEOUT_FIELD.getPreferredName(), TimeValue.timeValueMillis(timeoutInMillis).toString());
         }
 
         if (terminateAfter != SearchContext.DEFAULT_TERMINATE_AFTER) {

core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.builder;
 
+import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -89,7 +90,9 @@ import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.test.ClusterServiceUtils.createClusterService;
 import static org.elasticsearch.test.ClusterServiceUtils.setState;
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasToString;
 
 public class SearchSourceBuilderTests extends ESTestCase {
     private static Injector injector;
@@ -593,6 +596,27 @@ public class SearchSourceBuilderTests extends ESTestCase {
         }
     }
 
+    public void testTimeoutWithUnits() throws IOException {
+        final String timeout = randomTimeValue();
+        final String query = "{ \"query\": { \"match_all\": {}}, \"timeout\": \"" + timeout + "\"}";
+        try (XContentParser parser = XContentFactory.xContent(query).createParser(query)) {
+            final SearchSourceBuilder builder = SearchSourceBuilder.fromXContent(createParseContext(parser), aggParsers, suggesters);
+            assertThat(builder.timeoutInMillis(), equalTo(TimeValue.parseTimeValue(timeout, null, "timeout").millis()));
+        }
+    }
+
+    public void testTimeoutWithoutUnits() throws IOException {
+        final int timeout = randomIntBetween(1, 1024);
+        final String query = "{ \"query\": { \"match_all\": {}}, \"timeout\": \"" + timeout + "\"}";
+        try (XContentParser parser = XContentFactory.xContent(query).createParser(query)) {
+            final ElasticsearchParseException e =
+                    expectThrows(
+                            ElasticsearchParseException.class,
+                            () -> SearchSourceBuilder.fromXContent(createParseContext(parser), aggParsers, suggesters));
+            assertThat(e, hasToString(containsString("unit is missing or unrecognized")));
+        }
+    }
+
     public void testEmptyPostFilter() throws IOException {
         SearchSourceBuilder builder = new SearchSourceBuilder();
         String query = "{ \"post_filter\": {} }";