Fix a bug in FieldCapabilitiesRequest#equals and hashCode. (#30181)

Also update its unit test to AbstractStreamableTestCase for better coverage.
elastic · Apr 26, 2018 · 0d8aed8 · 0d8aed8
1 parent 804c383
commit 0d8aed8
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 56 deletions.
diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java
@@ -162,17 +162,17 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) return false;
 
         FieldCapabilitiesRequest that = (FieldCapabilitiesRequest) o;
-
-        if (!Arrays.equals(indices, that.indices)) return false;
-        if (!indicesOptions.equals(that.indicesOptions)) return false;
-        return Arrays.equals(fields, that.fields);
+        return  Arrays.equals(indices, that.indices) &&
+            Objects.equals(indicesOptions, that.indicesOptions) &&
+            Arrays.equals(fields, that.fields) &&
+            Objects.equals(mergeResults, that.mergeResults);
     }
 
     @Override
     public int hashCode() {
-        int result = Arrays.hashCode(indices);
-        result = 31 * result + indicesOptions.hashCode();
-        result = 31 * result + Arrays.hashCode(fields);
-        return result;
+        return Objects.hash(Arrays.hashCode(indices),
+            indicesOptions,
+            Arrays.hashCode(fields),
+            mergeResults);
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java
@@ -21,15 +21,18 @@
 
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.IndicesOptions;
-import org.elasticsearch.common.ValidationException;
-import org.elasticsearch.common.io.stream.BytesStreamOutput;
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.common.util.ArrayUtils;
+import org.elasticsearch.test.AbstractStreamableTestCase;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
 
-public class FieldCapabilitiesRequestTests extends ESTestCase {
-    private FieldCapabilitiesRequest randomRequest() {
+public class FieldCapabilitiesRequestTests extends AbstractStreamableTestCase<FieldCapabilitiesRequest> {
+
+    @Override
+    protected FieldCapabilitiesRequest createTestInstance() {
         FieldCapabilitiesRequest request =  new FieldCapabilitiesRequest();
         int size = randomIntBetween(1, 20);
         String[] randomFields = new String[size];
@@ -50,50 +53,33 @@ private FieldCapabilitiesRequest randomRequest() {
         return request;
     }
 
-    public void testEqualsAndHashcode() {
-        FieldCapabilitiesRequest request = new FieldCapabilitiesRequest();
-        request.indices("foo");
-        request.indicesOptions(IndicesOptions.lenientExpandOpen());
-        request.fields("bar");
-
-        FieldCapabilitiesRequest other = new FieldCapabilitiesRequest();
-        other.indices("foo");
-        other.indicesOptions(IndicesOptions.lenientExpandOpen());
-        other.fields("bar");
-        assertEquals(request, request);
-        assertEquals(request, other);
-        assertEquals(request.hashCode(), other.hashCode());
-
-        // change indices
-        other.indices("foo", "bar");
-        assertNotEquals(request, other);
-        other.indices("foo");
-        assertEquals(request, other);
-
-        // change fields
-        other.fields("foo", "bar");
-        assertNotEquals(request, other);
-        other.fields("bar");
-        assertEquals(request, request);
-
-        // change indices options
-        other.indicesOptions(IndicesOptions.strictExpand());
-        assertNotEquals(request, other);
-
+    @Override
+    protected FieldCapabilitiesRequest createBlankInstance() {
+        return new FieldCapabilitiesRequest();
     }
 
-    public void testSerialization() throws IOException {
-        for (int i = 0; i < 20; i++) {
-            FieldCapabilitiesRequest request = randomRequest();
-            BytesStreamOutput output = new BytesStreamOutput();
-            request.writeTo(output);
-            output.flush();
-            StreamInput input = output.bytes().streamInput();
-            FieldCapabilitiesRequest deserialized = new FieldCapabilitiesRequest();
-            deserialized.readFrom(input);
-            assertEquals(deserialized, request);
-            assertEquals(deserialized.hashCode(), request.hashCode());
-        }
+    @Override
+    protected FieldCapabilitiesRequest mutateInstance(FieldCapabilitiesRequest instance) throws IOException {
+        List<Consumer<FieldCapabilitiesRequest>> mutators = new ArrayList<>();
+        mutators.add(request -> {
+            String[] fields = ArrayUtils.concat(request.fields(), new String[] {randomAlphaOfLength(10)});
+            request.fields(fields);
+        });
+        mutators.add(request -> {
+            String[] indices = ArrayUtils.concat(instance.indices(), generateRandomStringArray(5, 10, false, false));
+            request.indices(indices);
+        });
+        mutators.add(request -> {
+            IndicesOptions indicesOptions = randomValueOtherThan(request.indicesOptions(),
+                () -> IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()));
+            request.indicesOptions(indicesOptions);
+        });
+        mutators.add(request -> request.setMergeResults(!request.isMergeResults()));
+
+        FieldCapabilitiesRequest mutatedInstance = copyInstance(instance);
+        Consumer<FieldCapabilitiesRequest> mutator = randomFrom(mutators);
+        mutator.accept(mutatedInstance);
+        return mutatedInstance;
     }
 
     public void testValidation() {