From bd9ca7cce55cf31dfdfab4ed65f7c719136963c0 Mon Sep 17 00:00:00 2001 From: "Daniel Doubrovkine (dB.)" Date: Wed, 2 Jun 2021 16:03:50 -0400 Subject: [PATCH] Version checks are incorrectly returning versions < 1.0.0. (#797) * Version checks are incorrectly returning versions < 1.0.0. Signed-off-by: dblock * Removed V_7_10_3 which has not been released as of time of the fork. Signed-off-by: dblock * 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 Co-authored-by: Rabi Panda --- .../org/opensearch/gradle/BwcVersions.java | 9 +- .../gradle/BwcOpenSearchVersionsTests.java | 91 +++++++++++++++++++ .../java/org/opensearch/LegacyESVersion.java | 1 - .../org/opensearch/test/VersionUtils.java | 23 +++-- 4 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 buildSrc/src/test/java/org/opensearch/gradle/BwcOpenSearchVersionsTests.java diff --git a/buildSrc/src/main/java/org/opensearch/gradle/BwcVersions.java b/buildSrc/src/main/java/org/opensearch/gradle/BwcVersions.java index 6c22c324916..d8b8167f256 100644 --- 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 class BwcVersions { // 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 class BwcVersions { int currentMajor = currentVersion.getMajor(); int lastMajor = currentMajor == 1 ? 6 : currentMajor - 1; List 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 class BwcVersions { 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 new file mode 100644 index 00000000000..28f116710e4 --- /dev/null +++ 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> 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 expected, List actual) { + assertEquals(expected.stream().map(Version::fromString).collect(Collectors.toList()), actual); + } + + private BwcVersions getVersionCollection(String versionString) { + List 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 index 0157b76866c..e535d209e59 100644 --- 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 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 index d4ebe0e5c56..b447a4e0bc9 100644 --- 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 @@ public class VersionUtils { } } - // 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); + } } }