Remove version conditionals from InternalAggregations (#40193)

* Remove version conditionals from InternalAggregations Version conditionals are no longer needed once #40177 is back-ported all the way to 6.7. * Disable bwc tests Relates to #40177 * indentation
elastic · Mar 19, 2019 · a85f893 · a85f893
1 parent 1f62a7a
commit a85f893
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 49 deletions.
diff --git a/build.gradle b/build.gradle
@@ -162,8 +162,8 @@ task verifyVersions {
  * after the backport of the backcompat code is complete.
  */
 
-boolean bwc_tests_enabled = true
-final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
+boolean bwc_tests_enabled = false
+final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/40177" /* place a PR link here when committing bwc changes */
 if (bwc_tests_enabled == false) {
   if (bwc_tests_disabled_issue.isEmpty()) {
     throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")

diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/InternalAggregations.java b/server/src/main/java/org/elasticsearch/search/aggregations/InternalAggregations.java
@@ -18,7 +18,6 @@
  */
 package org.elasticsearch.search.aggregations;
 
-import org.elasticsearch.Version;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Streamable;
@@ -150,22 +149,14 @@ public void readFrom(StreamInput in) throws IOException {
         if (aggregations.isEmpty()) {
             aggregationsAsMap = emptyMap();
         }
-        //TODO update version after backport
-        if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
-            this.topLevelPipelineAggregators = in.readList(
+        this.topLevelPipelineAggregators = in.readList(
                 stream -> (SiblingPipelineAggregator)in.readNamedWriteable(PipelineAggregator.class));
-        } else {
-            this.topLevelPipelineAggregators = Collections.emptyList();
-        }
     }
 
     @Override
     @SuppressWarnings("unchecked")
     public void writeTo(StreamOutput out) throws IOException {
         out.writeNamedWriteableList((List<InternalAggregation>)aggregations);
-        //TODO update version after backport
-        if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
-            out.writeNamedWriteableList(topLevelPipelineAggregators);
-        }
+        out.writeNamedWriteableList(topLevelPipelineAggregators);
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationsTests.java
@@ -26,23 +26,19 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.SearchModule;
-import org.elasticsearch.search.aggregations.bucket.histogram.InternalDateHistogram;
 import org.elasticsearch.search.aggregations.bucket.histogram.InternalDateHistogramTests;
 import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
 import org.elasticsearch.search.aggregations.bucket.terms.StringTermsTests;
 import org.elasticsearch.search.aggregations.pipeline.AvgBucketPipelineAggregationBuilder;
-import org.elasticsearch.search.aggregations.pipeline.InternalSimpleValue;
 import org.elasticsearch.search.aggregations.pipeline.InternalSimpleValueTests;
 import org.elasticsearch.search.aggregations.pipeline.MaxBucketPipelineAggregationBuilder;
 import org.elasticsearch.search.aggregations.pipeline.SiblingPipelineAggregator;
 import org.elasticsearch.search.aggregations.pipeline.SumBucketPipelineAggregationBuilder;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.VersionUtils;
-import org.hamcrest.Matchers;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Base64;
 import java.util.Collections;
 import java.util.List;
 
@@ -136,18 +132,13 @@ private void writeToAndReadFrom(InternalAggregations aggregations, int iteration
                 if (aggregations.getTopLevelPipelineAggregators() == null) {
                     assertEquals(0, deserialized.getTopLevelPipelineAggregators().size());
                 } else {
-                    //TODO update version after backport
-                    if (version.before(Version.V_8_0_0)) {
-                        assertEquals(0, deserialized.getTopLevelPipelineAggregators().size());
-                    } else {
-                        assertEquals(aggregations.getTopLevelPipelineAggregators().size(),
-                            deserialized.getTopLevelPipelineAggregators().size());
-                        for (int i = 0; i < aggregations.getTopLevelPipelineAggregators().size(); i++) {
-                            SiblingPipelineAggregator siblingPipelineAggregator1 = aggregations.getTopLevelPipelineAggregators().get(i);
-                            SiblingPipelineAggregator siblingPipelineAggregator2 = deserialized.getTopLevelPipelineAggregators().get(i);
-                            assertArrayEquals(siblingPipelineAggregator1.bucketsPaths(), siblingPipelineAggregator2.bucketsPaths());
-                            assertEquals(siblingPipelineAggregator1.name(), siblingPipelineAggregator2.name());
-                        }
+                    assertEquals(aggregations.getTopLevelPipelineAggregators().size(),
+                        deserialized.getTopLevelPipelineAggregators().size());
+                    for (int i = 0; i < aggregations.getTopLevelPipelineAggregators().size(); i++) {
+                        SiblingPipelineAggregator siblingPipelineAggregator1 = aggregations.getTopLevelPipelineAggregators().get(i);
+                        SiblingPipelineAggregator siblingPipelineAggregator2 = deserialized.getTopLevelPipelineAggregators().get(i);
+                        assertArrayEquals(siblingPipelineAggregator1.bucketsPaths(), siblingPipelineAggregator2.bucketsPaths());
+                        assertEquals(siblingPipelineAggregator1.name(), siblingPipelineAggregator2.name());
                     }
                 }
                 if (iteration < 2) {
@@ -157,24 +148,4 @@ private void writeToAndReadFrom(InternalAggregations aggregations, int iteration
             }
         }
     }
-
-    //TODO update version and rename after backport
-    public void testSerializationFromPre_8_0_0() throws IOException {
-        String aggsString = "AwZzdGVybXMFb0F0Q0EKCQVsZG5ncgAFeG56RWcFeUFxVmcABXBhQVVpBUtYc2VIAAVaclRESwVqUkxySAAFelp5d1AFRUREcEYABW1" +
-            "sckF0BU5wWWVFAAVJYVJmZgVURlJVbgAFT0RiU04FUWNwSVoABU1sb09HBUNzZHFlAAVWWmJHaQABAwGIDgNyYXcFAQAADmRhdGVfaGlzdG9ncmFt" +
-            "BVhHbVl4/wADAAKAurcDA1VUQwABAQAAAWmOhukAAQAAAWmR9dEAAAAAAAAAAAAAAANyYXcACAAAAWmQrDoAUQAAAAFpkRoXAEMAAAABaZGH9AAtA" +
-            "AAAAWmR9dEAJwAAAAFpkmOuAFwAAAABaZLRiwAYAAAAAWmTP2gAKgAAAAFpk61FABsADHNpbXBsZV92YWx1ZQVsWVNLVv8AB2RlY2ltYWwGIyMjLi" +
-            "MjQLZWZVy5zBYAAAAAAAAAAAAAAAAAAAAAAAAA";
-
-        byte[] aggsBytes = Base64.getDecoder().decode(aggsString);
-        try (NamedWriteableAwareStreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(aggsBytes), registry)) {
-            in.setVersion(VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(),
-                Version.max(Version.CURRENT.minimumCompatibilityVersion(), VersionUtils.getPreviousVersion(Version.CURRENT))));
-            InternalAggregations deserialized = InternalAggregations.readAggregations(in);
-            assertEquals(3, deserialized.aggregations.size());
-            assertThat(deserialized.aggregations.get(0), Matchers.instanceOf(StringTerms.class));
-            assertThat(deserialized.aggregations.get(1), Matchers.instanceOf(InternalDateHistogram.class));
-            assertThat(deserialized.aggregations.get(2), Matchers.instanceOf(InternalSimpleValue.class));
-        }
-    }
 }