Completely disallow setting negative size in search (#70209)

We used to treat setting size to -1 in search request bodies or as a rest
parameter as a no-op, using the default search size of 10 in this case. This
lenient behaviour was deprecated in #69548 and is removed with this PR in 8.0.

Relates to #69548

Author: Christoph Büscher

rest-api-spec/build.gradle

@@ -340,6 +340,7 @@ tasks.named("yamlRestCompatTest").configure {
     'search/160_exists_query/Test exists query on _type field',
     'search/171_terms_query_with_types/Terms Query with No.of terms exceeding index.max_terms_count should FAIL',
     'search/20_default_values/Basic search',
+    'search/260_parameter_validation/test size=-1 is deprecated',
     'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception',
     'search/340_type_query/type query',
     'search/40_indices_boost/Indices boost using object',

rest-api-spec/src/main/resources/rest-api-spec/test/search/260_parameter_validation.yml

@@ -18,23 +18,6 @@ setup:
         - '{ "index" : { "_index" : "index1", "_id" : "1" } }'
         - '{ "foo": "bar"}'
 
----
-"test size=-1 is deprecated":
-  - skip:
-      version: " - 7.99.99"
-      reason: deprecation so far only in 8.0, lower when backported
-      features: allowed_warnings
-
-  - do:
-      allowed_warnings:
-        - "Using search size of -1 is deprecated and will be removed in future versions. Instead, don't use the `size` parameter if you don't want to set it explicitely."
-      search:
-        rest_total_hits_as_int: true
-        index: index1
-        size: -1
-
-  - length: { hits.hits: 1 }
-
 ---
 "test negative size throws IAE":
 

server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

@@ -19,8 +19,6 @@
 import org.elasticsearch.common.Booleans;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
-import org.elasticsearch.common.logging.DeprecationCategory;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.rest.BaseRestHandler;
@@ -29,6 +27,7 @@
 import org.elasticsearch.rest.action.RestCancellableNodeClient;
 import org.elasticsearch.rest.action.RestStatusToXContentListener;
 import org.elasticsearch.search.Scroll;
+import org.elasticsearch.search.SearchService;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.fetch.StoredFieldsContext;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
@@ -60,8 +59,6 @@ public class RestSearchAction extends BaseRestHandler {
     public static final String TYPED_KEYS_PARAM = "typed_keys";
     private static final Set<String> RESPONSE_PARAMS;
 
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestSearchAction.class);
-
     static {
         final Set<String> responseParams = new HashSet<>(Arrays.asList(TYPED_KEYS_PARAM, TOTAL_HITS_AS_INT_PARAM));
         RESPONSE_PARAMS = Collections.unmodifiableSet(responseParams);
@@ -195,17 +192,8 @@ private static void parseSearchSource(final SearchSourceBuilder searchSourceBuil
             searchSourceBuilder.from(request.paramAsInt("from", 0));
         }
         if (request.hasParam("size")) {
-            int size = request.paramAsInt("size", -1);
-            if (size != -1) {
-                setSize.accept(size);
-            } else {
-                deprecationLogger.deprecate(
-                    DeprecationCategory.API,
-                    "search-api-size-1",
-                    "Using search size of -1 is deprecated and will be removed in future versions. Instead, don't use the `size` parameter "
-                        + "if you don't want to set it explicitely."
-                );
-            }
+            int size = request.paramAsInt("size", SearchService.DEFAULT_SIZE);
+            setSize.accept(size);
         }
 
         if (request.hasParam("explain")) {

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

@@ -18,8 +18,6 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
-import org.elasticsearch.common.logging.DeprecationCategory;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.ToXContentObject;
@@ -105,8 +103,6 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R
     public static final ParseField POINT_IN_TIME = new ParseField("pit");
     public static final ParseField RUNTIME_MAPPINGS_FIELD = new ParseField("runtime_mappings");
 
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(SearchSourceBuilder.class);
-
     public static SearchSourceBuilder fromXContent(XContentParser parser) throws IOException {
         return fromXContent(parser, true);
     }
@@ -1121,19 +1117,7 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th
                 if (FROM_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
                     from(parser.intValue());
                 } else if (SIZE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    int parsedSize = parser.intValue();
-                    // we treat -1 as not-set, but deprecate it to be able to later remove this funny extra treatment
-                    if (parsedSize != -1) {
-                        size(parsedSize);
-                    } else {
-                        deprecationLogger.deprecate(
-                            DeprecationCategory.API,
-                            "search-api-size-1",
-                            "Using search size of -1 is deprecated and will be removed in future versions. "
-                            + "Instead, don't use the `size` parameter if you don't want to set it explicitely."
-                        );
-                    }
-
+                    size(parser.intValue());
                 } else if (TIMEOUT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
                     timeout = TimeValue.parseTimeValue(parser.text(), null, TIMEOUT_FIELD.getPreferredName());
                 } else if (TERMINATE_AFTER_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {

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

@@ -460,23 +460,14 @@ public void testNegativeFromErrors() {
     }
 
     public void testNegativeSizeErrors() throws IOException {
-        int randomSize = randomIntBetween(-100000, -2);
+        int randomSize = randomIntBetween(-100000, -1);
         IllegalArgumentException expected = expectThrows(IllegalArgumentException.class,
                 () -> new SearchSourceBuilder().size(randomSize));
         assertEquals("[size] parameter cannot be negative, found [" + randomSize + "]", expected.getMessage());
         expected = expectThrows(IllegalArgumentException.class, () -> new SearchSourceBuilder().size(-1));
         assertEquals("[size] parameter cannot be negative, found [-1]", expected.getMessage());
 
-        // we don't want to error on -1 for bwc reasons, this is treated as if the value is unset later
-        String restContent = "{\"size\" : -1}";
-        try (XContentParser parser = createParser(JsonXContent.jsonXContent, restContent)) {
-            SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(parser);
-            assertEquals(-1, searchSourceBuilder.size());
-        }
-        assertWarnings("Using search size of -1 is deprecated and will be removed in future versions. Instead, don't use the `size` "
-            + "parameter if you don't want to set it explicitely.");
-
-        restContent = "{\"size\" : " + randomSize + "}";
+        String restContent = "{\"size\" : " + randomSize + "}";
         try (XContentParser parser = createParser(JsonXContent.jsonXContent, restContent)) {
             IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> SearchSourceBuilder.fromXContent(parser));
             assertThat(ex.getMessage(), containsString(Integer.toString(randomSize)));