Version checks are incorrectly returning versions < 1.0.0. (#797)

* Version checks are incorrectly returning versions < 1.0.0. Signed-off-by: dblock <[email protected]> * Removed V_7_10_3 which has not been released as of time of the fork. Signed-off-by: dblock <[email protected]> * Update check for current version to get unreleased versions. - no unreleased version if the current version is "1.0.0" - add unit tests for OpenSearch 1.0.0 with legacy ES versions. - update VersionUtils to include all legacy ES versions as released. Signed-off-by: Rabi Panda <[email protected]> Co-authored-by: Rabi Panda <[email protected]>
opensearch-project · Jun 2, 2021 · bd9ca7c · bd9ca7c
1 parent e7d6dcd
commit bd9ca7c
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 12 deletions.
diff --git a/buildSrc/src/main/java/org/opensearch/gradle/BwcVersions.java b/buildSrc/src/main/java/org/opensearch/gradle/BwcVersions.java
@@ -268,6 +268,11 @@ public List<Version> getUnreleased() {
         // The current version is being worked, is always unreleased
         unreleased.add(currentVersion);
 
+        // No unreleased versions for 1.0.0
+        if (currentVersion.equals(Version.fromString("1.0.0"))) {
+            return unmodifiableList(unreleased);
+        }
+
         // version 1 is the first release, there is no previous "unreleased version":
         if (currentVersion.getMajor() != 1) {
             // the tip of the previous major is unreleased for sure, be it a minor or a bugfix
@@ -380,6 +385,9 @@ public List<Version> getWireCompatible() {
         int currentMajor = currentVersion.getMajor();
         int lastMajor = currentMajor == 1 ? 6 : currentMajor - 1;
         List<Version> lastMajorList = groupByMajor.get(lastMajor);
+        if (lastMajorList == null) {
+            throw new IllegalStateException("Expected to find a list of versions for version: " + lastMajor);
+        }
         int minor = lastMajorList.get(lastMajorList.size() - 1).getMinor();
         for (int i = lastMajorList.size() - 1; i > 0 && lastMajorList.get(i).getMinor() == minor; --i) {
             wireCompat.add(lastMajorList.get(i));
@@ -395,7 +403,6 @@ public List<Version> getWireCompatible() {
         wireCompat.addAll(groupByMajor.get(currentMajor));
         wireCompat.remove(currentVersion);
         wireCompat.sort(Version::compareTo);
-
         return unmodifiableList(wireCompat);
     }
 

diff --git a/buildSrc/src/test/java/org/opensearch/gradle/BwcOpenSearchVersionsTests.java b/buildSrc/src/test/java/org/opensearch/gradle/BwcOpenSearchVersionsTests.java
@@ -0,0 +1,91 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.gradle;
+
+import org.opensearch.gradle.test.GradleUnitTestCase;
+import org.junit.Rule;
+import org.junit.rules.ExpectedException;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static java.util.Arrays.asList;
+
+/**
+ * Tests to specifically verify the OpenSearch version 1.x with Legacy ES versions.
+ * This supplements the tests in BwcVersionsTests.
+ *
+ * Currently the versioning logic doesn't work for OpenSearch 2.x as the masking
+ * is only applied specifically for 1.x.
+ */
+public class BwcOpenSearchVersionsTests extends GradleUnitTestCase {
+
+    private static final Map<String, List<String>> sampleVersions = new HashMap<>();
+
+    @Rule
+    public ExpectedException expectedEx = ExpectedException.none();
+
+    static {
+        sampleVersions.put("1.0.0", asList("5_6_13", "6_6_1", "6_8_15", "7_0_0", "7_9_1", "7_10_0", "7_10_1", "7_10_2", "1_0_0"));
+        sampleVersions.put("1.1.0", asList("5_6_13", "6_6_1", "6_8_15", "7_0_0", "7_9_1", "7_10_0", "7_10_1", "7_10_2", "1_0_0", "1_1_0"));
+    }
+
+    public void testWireCompatible() {
+        assertVersionsEquals(
+            asList("6.8.15", "7.0.0", "7.9.1", "7.10.0", "7.10.1", "7.10.2"),
+            getVersionCollection("1.0.0").getWireCompatible()
+        );
+        assertVersionsEquals(
+            asList("6.8.15", "7.0.0", "7.9.1", "7.10.0", "7.10.1", "7.10.2", "1.0.0"),
+            getVersionCollection("1.1.0").getWireCompatible()
+        );
+    }
+
+    public void testWireCompatibleUnreleased() {
+        assertVersionsEquals(Collections.emptyList(), getVersionCollection("1.0.0").getUnreleasedWireCompatible());
+    }
+
+    public void testIndexCompatible() {
+        assertVersionsEquals(
+            asList("6.6.1", "6.8.15", "7.0.0", "7.9.1", "7.10.0", "7.10.1", "7.10.2"),
+            getVersionCollection("1.0.0").getIndexCompatible()
+        );
+        assertVersionsEquals(
+            asList("6.6.1", "6.8.15", "7.0.0", "7.9.1", "7.10.0", "7.10.1", "7.10.2", "1.0.0"),
+            getVersionCollection("1.1.0").getIndexCompatible()
+        );
+    }
+
+    public void testIndexCompatibleUnreleased() {
+        assertVersionsEquals(Collections.emptyList(), getVersionCollection("1.0.0").getUnreleasedIndexCompatible());
+    }
+
+    public void testGetUnreleased() {
+        assertVersionsEquals(Collections.singletonList("1.0.0"), getVersionCollection("1.0.0").getUnreleased());
+    }
+
+    private String formatVersionToLine(final String version) {
+        return " public static final Version V_" + version.replaceAll("\\.", "_") + " ";
+    }
+
+    private void assertVersionsEquals(List<String> expected, List<Version> actual) {
+        assertEquals(expected.stream().map(Version::fromString).collect(Collectors.toList()), actual);
+    }
+
+    private BwcVersions getVersionCollection(String versionString) {
+        List<String> versionMap = sampleVersions.get(versionString);
+        assertNotNull(versionMap);
+        Version version = Version.fromString(versionString);
+        assertNotNull(version);
+        return new BwcVersions(versionMap.stream().map(this::formatVersionToLine).collect(Collectors.toList()), version);
+    }
+}
diff --git a/server/src/main/java/org/opensearch/LegacyESVersion.java b/server/src/main/java/org/opensearch/LegacyESVersion.java
@@ -139,7 +139,6 @@ public class LegacyESVersion extends Version {
     public static final LegacyESVersion V_7_10_0 = new LegacyESVersion(7100099, org.apache.lucene.util.Version.LUCENE_8_7_0);
     public static final LegacyESVersion V_7_10_1 = new LegacyESVersion(7100199, org.apache.lucene.util.Version.LUCENE_8_7_0);
     public static final LegacyESVersion V_7_10_2 = new LegacyESVersion(7100299, org.apache.lucene.util.Version.LUCENE_8_7_0);
-    public static final LegacyESVersion V_7_10_3 = new LegacyESVersion(7100399, org.apache.lucene.util.Version.LUCENE_8_7_0);
 
     // todo move back to Version.java if retiring legacy version support
     protected static final ImmutableOpenIntMap<Version> idToVersion;

diff --git a/test/framework/src/main/java/org/opensearch/test/VersionUtils.java b/test/framework/src/main/java/org/opensearch/test/VersionUtils.java
@@ -90,16 +90,19 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
             }
         }
 
-        // remove next minor
-        Version lastMinor = moveLastToUnreleased(stableVersions, unreleasedVersions);
-        if (lastMinor.revision == 0) {
-            if (stableVersions.get(stableVersions.size() - 1).size() == 1) {
-                // a minor is being staged, which is also unreleased
-                moveLastToUnreleased(stableVersions, unreleasedVersions);
-            }
-            // remove the next bugfix
-            if (stableVersions.isEmpty() == false) {
-                moveLastToUnreleased(stableVersions, unreleasedVersions);
+        // remove last minor unless the it's the first OpenSearch version.
+        // all Legacy ES versions are released, so we don't exclude any.
+        if (current.equals(Version.V_1_0_0) == false) {
+            Version lastMinor = moveLastToUnreleased(stableVersions, unreleasedVersions);
+            if (lastMinor.revision == 0) {
+                if (stableVersions.get(stableVersions.size() - 1).size() == 1) {
+                    // a minor is being staged, which is also unreleased
+                    moveLastToUnreleased(stableVersions, unreleasedVersions);
+                }
+                // remove the next bugfix
+                if (stableVersions.isEmpty() == false) {
+                    moveLastToUnreleased(stableVersions, unreleasedVersions);
+                }
             }
         }