Restore assignment of time value when deserializing a scroll instance (#19977)

* Assign scroll keepAlive when deserializing

The scroll time value was never assign when deserializing from the transport layer, meaning that it would always be null when received from another node, although the originating search request might have it set to some value.

* add tests for SearchRequest serialization and fail fast with illegal arguments

To ease testing, also introduced equals, hashcode and toString methods in SearchRequest and Scroll.

The serialization test brought up a few wrong assumptions about non null instance members, for which some null checks were needed to avoid NPEs when serializing.

* make Scroll implement Writeable rather than Streamable

* [TEST] add serialization test for ShardSearchTransportRequest

This also covers ShardSearchLocalRequest implicitly as most of the serialization code is in it.

core/src/main/java/org/elasticsearch/action/search/SearchRequest.java

@@ -33,8 +33,8 @@ import org.elasticsearch.search.Scroll;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 
 import java.io.IOException;
-
-import static org.elasticsearch.search.Scroll.readScroll;
+import java.util.Arrays;
+import java.util.Objects;
 
 /**
  * A request to execute search against one or more indices (or all). Best created using
@@ -47,11 +47,11 @@ import static org.elasticsearch.search.Scroll.readScroll;
  * @see org.elasticsearch.client.Client#search(SearchRequest)
  * @see SearchResponse
  */
-public class SearchRequest extends ActionRequest<SearchRequest> implements IndicesRequest.Replaceable {
+public final class SearchRequest extends ActionRequest<SearchRequest> implements IndicesRequest.Replaceable {
 
     private SearchType searchType = SearchType.DEFAULT;
 
-    private String[] indices;
+    private String[] indices = Strings.EMPTY_ARRAY;
 
     @Nullable
     private String routing;
@@ -94,12 +94,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
 
     @Override
     public ActionRequestValidationException validate() {
-        ActionRequestValidationException validationException = null;
-        // no need to check, we resolve to match all query
-//        if (source == null && extraSource == null) {
-//            validationException = addValidationError("search source is missing", validationException);
-//        }
-        return validationException;
+        return null;
     }
 
     /**
@@ -107,14 +102,9 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
      */
     @Override
     public SearchRequest indices(String... indices) {
-        if (indices == null) {
-            throw new IllegalArgumentException("indices must not be null");
-        } else {
-            for (int i = 0; i < indices.length; i++) {
-                if (indices[i] == null) {
-                    throw new IllegalArgumentException("indices[" + i + "] must not be null");
-                }
-            }
+        Objects.requireNonNull(indices, "indices must not be null");
+        for (String index : indices) {
+            Objects.requireNonNull(index, "index must not be null");
         }
         this.indices = indices;
         return this;
@@ -126,7 +116,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
     }
 
     public SearchRequest indicesOptions(IndicesOptions indicesOptions) {
-        this.indicesOptions = indicesOptions;
+        this.indicesOptions = Objects.requireNonNull(indicesOptions, "indicesOptions must not be null");
         return this;
     }
 
@@ -143,6 +133,10 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
      * all types.
      */
     public SearchRequest types(String... types) {
+        Objects.requireNonNull(types, "types must not be null");
+        for (String type : types) {
+            Objects.requireNonNull(type, "type must not be null");
+        }
         this.types = types;
         return this;
     }
@@ -188,7 +182,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
      * The search type to execute, defaults to {@link SearchType#DEFAULT}.
      */
     public SearchRequest searchType(SearchType searchType) {
-        this.searchType = searchType;
+        this.searchType = Objects.requireNonNull(searchType, "searchType must not be null");
         return this;
     }
 
@@ -205,10 +199,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
      * The source of the search request.
      */
     public SearchRequest source(SearchSourceBuilder sourceBuilder) {
-        if (sourceBuilder == null) {
-            throw new IllegalArgumentException("source must not be null");
-        }
-        this.source = sourceBuilder;
+        this.source = Objects.requireNonNull(sourceBuilder, "source must not be null");
         return this;
     }
 
@@ -288,25 +279,16 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
     public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
         searchType = SearchType.fromId(in.readByte());
-
         indices = new String[in.readVInt()];
         for (int i = 0; i < indices.length; i++) {
             indices[i] = in.readString();
         }
-
         routing = in.readOptionalString();
         preference = in.readOptionalString();
-
-        if (in.readBoolean()) {
-            scroll = readScroll(in);
-        }
-        if (in.readBoolean()) {
-            source = new SearchSourceBuilder(in);
-        }
-
+        scroll = in.readOptionalWriteable(Scroll::new);
+        source = in.readOptionalWriteable(SearchSourceBuilder::new);
         types = in.readStringArray();
         indicesOptions = IndicesOptions.readIndicesOptions(in);
-
         requestCache = in.readOptionalBoolean();
     }
 
@@ -314,29 +296,56 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeByte(searchType.id());
-
         out.writeVInt(indices.length);
         for (String index : indices) {
             out.writeString(index);
         }
-
         out.writeOptionalString(routing);
         out.writeOptionalString(preference);
-
-        if (scroll == null) {
-            out.writeBoolean(false);
-        } else {
-            out.writeBoolean(true);
-            scroll.writeTo(out);
-        }
-        if (source == null) {
-            out.writeBoolean(false);
-        } else {
-            out.writeBoolean(true);
-            source.writeTo(out);
-        }
+        out.writeOptionalWriteable(scroll);
+        out.writeOptionalWriteable(source);
         out.writeStringArray(types);
         indicesOptions.writeIndicesOptions(out);
         out.writeOptionalBoolean(requestCache);
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        SearchRequest that = (SearchRequest) o;
+        return searchType == that.searchType &&
+                Arrays.equals(indices, that.indices) &&
+                Objects.equals(routing, that.routing) &&
+                Objects.equals(preference, that.preference) &&
+                Objects.equals(source, that.source) &&
+                Objects.equals(requestCache, that.requestCache)  &&
+                Objects.equals(scroll, that.scroll) &&
+                Arrays.equals(types, that.types) &&
+                Objects.equals(indicesOptions, that.indicesOptions);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(searchType, Arrays.hashCode(indices), routing, preference, source, requestCache,
+                scroll, Arrays.hashCode(types), indicesOptions);
+    }
+
+    @Override
+    public String toString() {
+        return "SearchRequest{" +
+                "searchType=" + searchType +
+                ", indices=" + Arrays.toString(indices) +
+                ", indicesOptions=" + indicesOptions +
+                ", types=" + Arrays.toString(types) +
+                ", routing='" + routing + '\'' +
+                ", preference='" + preference + '\'' +
+                ", requestCache=" + requestCache +
+                ", scroll=" + scroll +
+                ", source=" + source + '}';
+    }
 }

core/src/main/java/org/elasticsearch/action/search/SearchScrollRequest.java

@@ -27,9 +27,9 @@ import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.search.Scroll;
 
 import java.io.IOException;
+import java.util.Objects;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
-import static org.elasticsearch.search.Scroll.readScroll;
 
 /**
  *
@@ -100,20 +100,39 @@ public class SearchScrollRequest extends ActionRequest<SearchScrollRequest> {
     public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
         scrollId = in.readString();
-        if (in.readBoolean()) {
-            scroll = readScroll(in);
-        }
+        scroll = in.readOptionalWriteable(Scroll::new);
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeString(scrollId);
-        if (scroll == null) {
-            out.writeBoolean(false);
-        } else {
-            out.writeBoolean(true);
-            scroll.writeTo(out);
+        out.writeOptionalWriteable(scroll);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
         }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        SearchScrollRequest that = (SearchScrollRequest) o;
+        return Objects.equals(scrollId, that.scrollId) &&
+                Objects.equals(scroll, that.scroll);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(scrollId, scroll);
+    }
+
+    @Override
+    public String toString() {
+        return "SearchScrollRequest{" +
+                "scrollId='" + scrollId + '\'' +
+                ", scroll=" + scroll +
+                '}';
     }
 }

core/src/main/java/org/elasticsearch/search/Scroll.java

@@ -21,10 +21,11 @@ package org.elasticsearch.search;
 
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.unit.TimeValue;
 
 import java.io.IOException;
+import java.util.Objects;
 
 /**
  * A scroll enables scrolling of search request. It holds a {@link #keepAlive()} time that
@@ -32,19 +33,19 @@ import java.io.IOException;
  *
  *
  */
-public class Scroll implements Streamable {
+public final class Scroll implements Writeable {
 
-    private TimeValue keepAlive;
-
-    private Scroll() {
+    private final TimeValue keepAlive;
 
+    public Scroll(StreamInput in) throws IOException {
+        this.keepAlive = new TimeValue(in);
     }
 
     /**
      * Constructs a new scroll of the provided keep alive.
      */
     public Scroll(TimeValue keepAlive) {
-        this.keepAlive = keepAlive;
+        this.keepAlive = Objects.requireNonNull(keepAlive, "keepAlive must not be null");
     }
 
     /**
@@ -54,19 +55,30 @@ public class Scroll implements Streamable {
         return keepAlive;
     }
 
-    public static Scroll readScroll(StreamInput in) throws IOException {
-        Scroll scroll = new Scroll();
-        scroll.readFrom(in);
-        return scroll;
-    }
-
-    @Override
-    public void readFrom(StreamInput in) throws IOException {
-        in.readOptionalWriteable(TimeValue::new);
-    }
-
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        out.writeOptionalWriteable(keepAlive);
+        keepAlive.writeTo(out);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        Scroll scroll = (Scroll) o;
+        return Objects.equals(keepAlive, scroll.keepAlive);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(keepAlive);
+    }
+
+    @Override
+    public String toString() {
+        return "Scroll{keepAlive=" + keepAlive + '}';
     }
 }

core/src/main/java/org/elasticsearch/search/internal/InternalScrollSearchRequest.java

@@ -27,11 +27,6 @@ import org.elasticsearch.transport.TransportRequest;
 
 import java.io.IOException;
 
-import static org.elasticsearch.search.Scroll.readScroll;
-
-/**
- *
- */
 public class InternalScrollSearchRequest extends TransportRequest {
 
     private long id;
@@ -63,20 +58,13 @@ public class InternalScrollSearchRequest extends TransportRequest {
     public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
         id = in.readLong();
-        if (in.readBoolean()) {
-            scroll = readScroll(in);
-        }
+        scroll = in.readOptionalWriteable(Scroll::new);
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeLong(id);
-        if (scroll == null) {
-            out.writeBoolean(false);
-        } else {
-            out.writeBoolean(true);
-            scroll.writeTo(out);
-        }
+        out.writeOptionalWriteable(scroll);
     }
 }

core/src/main/java/org/elasticsearch/search/internal/ShardSearchLocalRequest.java

@@ -35,8 +35,6 @@ import org.elasticsearch.search.builder.SearchSourceBuilder;
 
 import java.io.IOException;
 
-import static org.elasticsearch.search.Scroll.readScroll;
-
 /**
  * Shard level search request that gets created and consumed on the local node.
  * Used by warmers and by api that need to create a search context within their execution.
@@ -168,12 +166,8 @@ public class ShardSearchLocalRequest implements ShardSearchRequest {
         shardId = ShardId.readShardId(in);
         searchType = SearchType.fromId(in.readByte());
         numberOfShards = in.readVInt();
-        if (in.readBoolean()) {
-            scroll = readScroll(in);
-        }
-        if (in.readBoolean()) {
-            source = new SearchSourceBuilder(in);
-        }
+        scroll = in.readOptionalWriteable(Scroll::new);
+        source = in.readOptionalWriteable(SearchSourceBuilder::new);
         types = in.readStringArray();
         filteringAliases = in.readStringArray();
         nowInMillis = in.readVLong();
@@ -186,19 +180,8 @@ public class ShardSearchLocalRequest implements ShardSearchRequest {
         if (!asKey) {
             out.writeVInt(numberOfShards);
         }
-        if (scroll == null) {
-            out.writeBoolean(false);
-        } else {
-            out.writeBoolean(true);
-            scroll.writeTo(out);
-        }
-        if (source == null) {
-            out.writeBoolean(false);
-        } else {
-            out.writeBoolean(true);
-            source.writeTo(out);
-
-        }
+        out.writeOptionalWriteable(scroll);
+        out.writeOptionalWriteable(source);
         out.writeStringArray(types);
         out.writeStringArrayNullable(filteringAliases);
         if (!asKey) {

core/src/test/java/org/elasticsearch/action/search/SearchScrollRequestTests.java

@@ -0,0 +1,121 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.action.search;
+
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.search.internal.InternalScrollSearchRequest;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+
+public class SearchScrollRequestTests extends ESTestCase {
+
+    public void testSerialization() throws Exception {
+        SearchScrollRequest searchScrollRequest = createSearchScrollRequest();
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            searchScrollRequest.writeTo(output);
+            try (StreamInput in = output.bytes().streamInput()) {
+                SearchScrollRequest deserializedRequest = new SearchScrollRequest();
+                deserializedRequest.readFrom(in);
+                assertEquals(deserializedRequest, searchScrollRequest);
+                assertEquals(deserializedRequest.hashCode(), searchScrollRequest.hashCode());
+                assertNotSame(deserializedRequest, searchScrollRequest);
+            }
+        }
+    }
+
+    public void testInternalScrollSearchRequestSerialization() throws IOException {
+        SearchScrollRequest searchScrollRequest = createSearchScrollRequest();
+        InternalScrollSearchRequest internalScrollSearchRequest = new InternalScrollSearchRequest(searchScrollRequest, randomLong());
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            internalScrollSearchRequest.writeTo(output);
+            try (StreamInput in = output.bytes().streamInput()) {
+                InternalScrollSearchRequest deserializedRequest = new InternalScrollSearchRequest();
+                deserializedRequest.readFrom(in);
+                assertEquals(deserializedRequest.id(), internalScrollSearchRequest.id());
+                assertEquals(deserializedRequest.scroll(), internalScrollSearchRequest.scroll());
+                assertNotSame(deserializedRequest, internalScrollSearchRequest);
+            }
+        }
+    }
+
+    public void testEqualsAndHashcode() {
+        SearchScrollRequest firstSearchScrollRequest = createSearchScrollRequest();
+        assertNotNull("search scroll request is equal to null", firstSearchScrollRequest);
+        assertNotEquals("search scroll request  is equal to incompatible type", firstSearchScrollRequest, "");
+        assertEquals("search scroll request is not equal to self", firstSearchScrollRequest, firstSearchScrollRequest);
+        assertEquals("same source builder's hashcode returns different values if called multiple times",
+                firstSearchScrollRequest.hashCode(), firstSearchScrollRequest.hashCode());
+
+        SearchScrollRequest secondSearchScrollRequest = copyRequest(firstSearchScrollRequest);
+        assertEquals("search scroll request  is not equal to self", secondSearchScrollRequest, secondSearchScrollRequest);
+        assertEquals("search scroll request is not equal to its copy", firstSearchScrollRequest, secondSearchScrollRequest);
+        assertEquals("search scroll request is not symmetric", secondSearchScrollRequest, firstSearchScrollRequest);
+        assertEquals("search scroll request copy's hashcode is different from original hashcode",
+                firstSearchScrollRequest.hashCode(), secondSearchScrollRequest.hashCode());
+
+        SearchScrollRequest thirdSearchScrollRequest = copyRequest(secondSearchScrollRequest);
+        assertEquals("search scroll request is not equal to self", thirdSearchScrollRequest, thirdSearchScrollRequest);
+        assertEquals("search scroll request is not equal to its copy", secondSearchScrollRequest, thirdSearchScrollRequest);
+        assertEquals("search scroll request copy's hashcode is different from original hashcode",
+                secondSearchScrollRequest.hashCode(), thirdSearchScrollRequest.hashCode());
+        assertEquals("equals is not transitive", firstSearchScrollRequest, thirdSearchScrollRequest);
+        assertEquals("search scroll request copy's hashcode is different from original hashcode",
+                firstSearchScrollRequest.hashCode(), thirdSearchScrollRequest.hashCode());
+        assertEquals("equals is not symmetric", thirdSearchScrollRequest, secondSearchScrollRequest);
+        assertEquals("equals is not symmetric", thirdSearchScrollRequest, firstSearchScrollRequest);
+
+        boolean changed = false;
+        if (randomBoolean()) {
+            secondSearchScrollRequest.scrollId(randomAsciiOfLengthBetween(3, 10));
+            if (secondSearchScrollRequest.scrollId().equals(firstSearchScrollRequest.scrollId()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchScrollRequest.scroll(randomPositiveTimeValue());
+            if (secondSearchScrollRequest.scroll().equals(firstSearchScrollRequest.scroll()) == false) {
+                changed = true;
+            }
+        }
+        
+        if (changed) {
+            assertNotEquals(firstSearchScrollRequest, secondSearchScrollRequest);
+            assertNotEquals(firstSearchScrollRequest.hashCode(), secondSearchScrollRequest.hashCode());
+        } else {
+            assertEquals(firstSearchScrollRequest, secondSearchScrollRequest);
+            assertEquals(firstSearchScrollRequest.hashCode(), secondSearchScrollRequest.hashCode());
+        }
+    }
+    
+    public static SearchScrollRequest createSearchScrollRequest() {
+        SearchScrollRequest searchScrollRequest = new SearchScrollRequest(randomAsciiOfLengthBetween(3, 10));
+        searchScrollRequest.scroll(randomPositiveTimeValue());
+        return searchScrollRequest;
+    }
+
+    private static SearchScrollRequest copyRequest(SearchScrollRequest searchScrollRequest) {
+        SearchScrollRequest result = new SearchScrollRequest();
+        result.scrollId(searchScrollRequest.scrollId());
+        result.scroll(searchScrollRequest.scroll());
+        return result;
+    }
+}

core/src/test/java/org/elasticsearch/search/SearchRequestTests.java

@@ -0,0 +1,250 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.search;
+
+import org.elasticsearch.action.search.SearchRequest;
+import org.elasticsearch.action.search.SearchType;
+import org.elasticsearch.action.support.IndicesOptions;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.indices.IndicesModule;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.BeforeClass;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import static java.util.Collections.emptyList;
+import static org.elasticsearch.search.builder.SearchSourceBuilderTests.createSearchSourceBuilder;
+
+public class SearchRequestTests extends ESTestCase {
+
+    private static NamedWriteableRegistry namedWriteableRegistry;
+
+    @BeforeClass
+    public static void beforeClass() {
+        IndicesModule indicesModule = new IndicesModule(emptyList()) {
+            @Override
+            protected void configure() {
+                bindMapperExtension();
+            }
+        };
+        SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList()) {
+            @Override
+            protected void configureSearch() {
+                // Skip me
+            }
+        };
+        List<NamedWriteableRegistry.Entry> entries = new ArrayList<>();
+        entries.addAll(indicesModule.getNamedWriteables());
+        entries.addAll(searchModule.getNamedWriteables());
+        namedWriteableRegistry = new NamedWriteableRegistry(entries);
+    }
+
+    public void testSerialization() throws Exception {
+        SearchRequest searchRequest = createSearchRequest();
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            searchRequest.writeTo(output);
+            try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) {
+                SearchRequest deserializedRequest = new SearchRequest();
+                deserializedRequest.readFrom(in);
+                assertEquals(deserializedRequest, searchRequest);
+                assertEquals(deserializedRequest.hashCode(), searchRequest.hashCode());
+                assertNotSame(deserializedRequest, searchRequest);
+            }
+        }
+    }
+
+    public void testIllegalArguments() {
+        SearchRequest searchRequest = new SearchRequest();
+        assertNotNull(searchRequest.indices());
+        assertNotNull(searchRequest.indicesOptions());
+        assertNotNull(searchRequest.types());
+        assertNotNull(searchRequest.searchType());
+
+        NullPointerException e = expectThrows(NullPointerException.class, () -> searchRequest.indices((String[]) null));
+        assertEquals("indices must not be null", e.getMessage());
+        e = expectThrows(NullPointerException.class, () -> searchRequest.indices((String) null));
+        assertEquals("index must not be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> searchRequest.indicesOptions(null));
+        assertEquals("indicesOptions must not be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> searchRequest.types((String[]) null));
+        assertEquals("types must not be null", e.getMessage());
+        e = expectThrows(NullPointerException.class, () -> searchRequest.types((String) null));
+        assertEquals("type must not be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> searchRequest.searchType((SearchType)null));
+        assertEquals("searchType must not be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> searchRequest.source(null));
+        assertEquals("source must not be null", e.getMessage());
+
+        e = expectThrows(NullPointerException.class, () -> searchRequest.scroll((TimeValue)null));
+        assertEquals("keepAlive must not be null", e.getMessage());
+    }
+
+    public void testEqualsAndHashcode() throws IOException {
+        SearchRequest firstSearchRequest = createSearchRequest();
+        assertNotNull("search request is equal to null", firstSearchRequest);
+        assertNotEquals("search request  is equal to incompatible type", firstSearchRequest, "");
+        assertEquals("search request is not equal to self", firstSearchRequest, firstSearchRequest);
+        assertEquals("same source builder's hashcode returns different values if called multiple times",
+                firstSearchRequest.hashCode(), firstSearchRequest.hashCode());
+
+        SearchRequest secondSearchRequest = copyRequest(firstSearchRequest);
+        assertEquals("search request  is not equal to self", secondSearchRequest, secondSearchRequest);
+        assertEquals("search request is not equal to its copy", firstSearchRequest, secondSearchRequest);
+        assertEquals("search request is not symmetric", secondSearchRequest, firstSearchRequest);
+        assertEquals("search request copy's hashcode is different from original hashcode",
+                firstSearchRequest.hashCode(), secondSearchRequest.hashCode());
+
+        SearchRequest thirdSearchRequest = copyRequest(secondSearchRequest);
+        assertEquals("search request is not equal to self", thirdSearchRequest, thirdSearchRequest);
+        assertEquals("search request is not equal to its copy", secondSearchRequest, thirdSearchRequest);
+        assertEquals("search request copy's hashcode is different from original hashcode",
+                secondSearchRequest.hashCode(), thirdSearchRequest.hashCode());
+        assertEquals("equals is not transitive", firstSearchRequest, thirdSearchRequest);
+        assertEquals("search request copy's hashcode is different from original hashcode",
+                firstSearchRequest.hashCode(), thirdSearchRequest.hashCode());
+        assertEquals("equals is not symmetric", thirdSearchRequest, secondSearchRequest);
+        assertEquals("equals is not symmetric", thirdSearchRequest, firstSearchRequest);
+
+        boolean changed = false;
+        if (randomBoolean()) {
+            secondSearchRequest.indices(generateRandomStringArray(10, 10, false, false));
+            if (Arrays.equals(secondSearchRequest.indices(), firstSearchRequest.indices()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.indicesOptions(
+                    IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()));
+            if (secondSearchRequest.indicesOptions().equals(firstSearchRequest.indicesOptions()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.types(generateRandomStringArray(10, 10, false, false));
+            if (Arrays.equals(secondSearchRequest.types(), firstSearchRequest.types()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.preference(randomAsciiOfLengthBetween(3, 10));
+            if (secondSearchRequest.preference().equals(firstSearchRequest.preference()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.routing(randomAsciiOfLengthBetween(3, 10));
+            if (secondSearchRequest.routing().equals(firstSearchRequest.routing()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.requestCache(randomBoolean());
+            if (secondSearchRequest.requestCache().equals(firstSearchRequest.requestCache()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.scroll(randomPositiveTimeValue());
+            if (secondSearchRequest.scroll().equals(firstSearchRequest.scroll()) == false) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.searchType(randomFrom(SearchType.values()));
+            if (secondSearchRequest.searchType() != firstSearchRequest.searchType()) {
+                changed = true;
+            }
+        }
+        if (randomBoolean()) {
+            secondSearchRequest.source(createSearchSourceBuilder());
+            if (secondSearchRequest.source().equals(firstSearchRequest.source()) == false) {
+                changed = true;
+            }
+        }
+
+        if (changed) {
+            assertNotEquals(firstSearchRequest, secondSearchRequest);
+            assertNotEquals(firstSearchRequest.hashCode(), secondSearchRequest.hashCode());
+        } else {
+            assertEquals(firstSearchRequest, secondSearchRequest);
+            assertEquals(firstSearchRequest.hashCode(), secondSearchRequest.hashCode());
+        }
+    }
+
+    public static SearchRequest createSearchRequest() throws IOException {
+        SearchRequest searchRequest = new SearchRequest();
+        if (randomBoolean()) {
+            searchRequest.indices(generateRandomStringArray(10, 10, false, false));
+        }
+        if (randomBoolean()) {
+            searchRequest.indicesOptions(IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()));
+        }
+        if (randomBoolean()) {
+            searchRequest.types(generateRandomStringArray(10, 10, false, false));
+        }
+        if (randomBoolean()) {
+            searchRequest.preference(randomAsciiOfLengthBetween(3, 10));
+        }
+        if (randomBoolean()) {
+            searchRequest.requestCache(randomBoolean());
+        }
+        if (randomBoolean()) {
+            searchRequest.routing(randomAsciiOfLengthBetween(3, 10));
+        }
+        if (randomBoolean()) {
+            searchRequest.scroll(randomPositiveTimeValue());
+        }
+        if (randomBoolean()) {
+            searchRequest.searchType(randomFrom(SearchType.values()));
+        }
+        if (randomBoolean()) {
+            searchRequest.source(createSearchSourceBuilder());
+        }
+        return searchRequest;
+    }
+
+    private static SearchRequest copyRequest(SearchRequest searchRequest) throws IOException {
+        SearchRequest result = new SearchRequest();
+        result.indices(searchRequest.indices());
+        result.indicesOptions(searchRequest.indicesOptions());
+        result.types(searchRequest.types());
+        result.searchType(searchRequest.searchType());
+        result.preference(searchRequest.preference());
+        result.routing(searchRequest.routing());
+        result.requestCache(searchRequest.requestCache());
+        result.scroll(searchRequest.scroll());
+        if (searchRequest.source() != null) {
+            result.source(searchRequest.source());
+        }
+        return result;
+    }
+}

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

@@ -190,7 +190,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
         namedWriteableRegistry = null;
     }
 
-    protected final SearchSourceBuilder createSearchSourceBuilder() throws IOException {
+    public static SearchSourceBuilder createSearchSourceBuilder() throws IOException {
         SearchSourceBuilder builder = new SearchSourceBuilder();
         if (randomBoolean()) {
             builder.from(randomIntBetween(0, 10000));
@@ -484,8 +484,8 @@ public class SearchSourceBuilderTests extends ESTestCase {
         assertTrue("equals is not symmetric", thirdBuilder.equals(firstBuilder));
     }
 
-    //we use the streaming infra to create a copy of the query provided as argument
-    protected SearchSourceBuilder copyBuilder(SearchSourceBuilder builder) throws IOException {
+    //we use the streaming infra to create a copy of the builder provided as argument
+    protected static SearchSourceBuilder copyBuilder(SearchSourceBuilder builder) throws IOException {
         try (BytesStreamOutput output = new BytesStreamOutput()) {
             builder.writeTo(output);
             try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) {

core/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java

@@ -0,0 +1,112 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.search.internal;
+
+import org.elasticsearch.action.search.SearchRequest;
+import org.elasticsearch.cluster.routing.RestoreSource;
+import org.elasticsearch.cluster.routing.ShardRouting;
+import org.elasticsearch.cluster.routing.UnassignedInfo;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.indices.IndicesModule;
+import org.elasticsearch.search.SearchModule;
+import org.elasticsearch.search.SearchRequestTests;
+import org.elasticsearch.snapshots.Snapshot;
+import org.elasticsearch.snapshots.SnapshotId;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.VersionUtils;
+import org.junit.BeforeClass;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import static java.util.Collections.emptyList;
+
+public class ShardSearchTransportRequestTests extends ESTestCase {
+
+    private static NamedWriteableRegistry namedWriteableRegistry;
+
+    @BeforeClass
+    public static void beforeClass() {
+        IndicesModule indicesModule = new IndicesModule(emptyList()) {
+            @Override
+            protected void configure() {
+                bindMapperExtension();
+            }
+        };
+        SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList()) {
+            @Override
+            protected void configureSearch() {
+                // Skip me
+            }
+        };
+        List<NamedWriteableRegistry.Entry> entries = new ArrayList<>();
+        entries.addAll(indicesModule.getNamedWriteables());
+        entries.addAll(searchModule.getNamedWriteables());
+        namedWriteableRegistry = new NamedWriteableRegistry(entries);
+    }
+
+    public void testSerialization() throws Exception {
+        ShardSearchTransportRequest shardSearchTransportRequest = createShardSearchTransportRequest();
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            shardSearchTransportRequest.writeTo(output);
+            try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) {
+                ShardSearchTransportRequest deserializedRequest = new ShardSearchTransportRequest();
+                deserializedRequest.readFrom(in);
+                assertEquals(deserializedRequest.scroll(), shardSearchTransportRequest.scroll());
+                assertArrayEquals(deserializedRequest.filteringAliases(), shardSearchTransportRequest.filteringAliases());
+                assertArrayEquals(deserializedRequest.indices(), shardSearchTransportRequest.indices());
+                assertArrayEquals(deserializedRequest.types(), shardSearchTransportRequest.types());
+                assertEquals(deserializedRequest.indicesOptions(), shardSearchTransportRequest.indicesOptions());
+                assertEquals(deserializedRequest.isProfile(), shardSearchTransportRequest.isProfile());
+                assertEquals(deserializedRequest.nowInMillis(), shardSearchTransportRequest.nowInMillis());
+                assertEquals(deserializedRequest.source(), shardSearchTransportRequest.source());
+                assertEquals(deserializedRequest.searchType(), shardSearchTransportRequest.searchType());
+                assertEquals(deserializedRequest.shardId(), shardSearchTransportRequest.shardId());
+                assertEquals(deserializedRequest.numberOfShards(), shardSearchTransportRequest.numberOfShards());
+                assertNotSame(deserializedRequest, shardSearchTransportRequest);
+            }
+        }
+    }
+
+    private static ShardSearchTransportRequest createShardSearchTransportRequest() throws IOException {
+        SearchRequest searchRequest = SearchRequestTests.createSearchRequest();
+        ShardId shardId = new ShardId(randomAsciiOfLengthBetween(2, 10), randomAsciiOfLengthBetween(2, 10), randomInt());
+        Snapshot snapshot = new Snapshot(randomAsciiOfLengthBetween(3, 10),
+                new SnapshotId(randomAsciiOfLengthBetween(3, 10), randomAsciiOfLengthBetween(3, 10)));
+        RestoreSource restoreSource = new RestoreSource(snapshot, VersionUtils.randomVersion(random()), randomAsciiOfLengthBetween(3, 10));
+        ShardRouting shardRouting = ShardRouting.newUnassigned(shardId, restoreSource, randomBoolean(),
+                new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()), "reason"));
+        String[] filteringAliases;
+        if (randomBoolean()) {
+            filteringAliases = generateRandomStringArray(10, 10, false, false);
+        } else {
+            filteringAliases = Strings.EMPTY_ARRAY;
+        }
+        return new ShardSearchTransportRequest(searchRequest, shardRouting,
+                randomIntBetween(1, 100), filteringAliases, Math.abs(randomLong()));
+    }
+}

core/src/test/java/org/elasticsearch/search/simple/SimpleSearchIT.java

@@ -19,10 +19,6 @@
 
 package org.elasticsearch.search.simple;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-
 import org.apache.lucene.util.Constants;
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
@@ -36,6 +32,11 @@ import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.search.rescore.QueryRescorerBuilder;
 import org.elasticsearch.test.ESIntegTestCase;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+
+import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
 import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
@@ -50,23 +51,14 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitC
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
 import static org.hamcrest.Matchers.containsString;
 
-import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
-
 public class SimpleSearchIT extends ESIntegTestCase {
     public void testSearchNullIndex() {
-        try {
-            client().prepareSearch((String) null).setQuery(QueryBuilders.termQuery("_id", "XXX1")).execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
+        expectThrows(NullPointerException.class,
+                () -> client().prepareSearch((String) null).setQuery(QueryBuilders.termQuery("_id", "XXX1")).get());
 
-        }
+        expectThrows(NullPointerException.class,
+                () -> client().prepareSearch((String[]) null).setQuery(QueryBuilders.termQuery("_id", "XXX1")).get());
 
-        try {
-            client().prepareSearch((String[]) null).setQuery(QueryBuilders.termQuery("_id", "XXX1")).execute().actionGet();
-            fail();
-        } catch (IllegalArgumentException e) {
-
-        }
     }
 
     public void testSearchRandomPreference() throws InterruptedException, ExecutionException {