From a8f5da405fb7a99a63f7fcf372993a4f1107e1dc Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Mon, 30 Apr 2018 15:04:49 -0500 Subject: [PATCH] Align logic in VersionUtils to VersionCollection The BCW generation code in VersionCollection.groovy was updated a while ago to accurately grab released and unreleased versions from Version.java. The logic for VersionUtils, which is the java equivalent of VersionCollection, was never updated to fix the bugs present in VersionCollection. This commit uses the same logic present in VersionCollection in VersionUtils. While the best solution is not to duplicate logic, this commit will first fix the test, and de duping the logic is out of scope for this. Relates #30133 --- .../org/elasticsearch/test/VersionUtils.java | 226 +++++++++++++----- .../elasticsearch/test/VersionUtilsTests.java | 51 ++-- 2 files changed, 202 insertions(+), 75 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java b/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java index 766fc80ba5605..f38d171c77cf0 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java @@ -30,8 +30,12 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.Random; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.stream.Collectors; import static java.util.Collections.singletonList; @@ -39,6 +43,9 @@ /** Utilities for selecting versions in tests */ public class VersionUtils { + // this will need to be set to false once the branch has no BWC candidates (ie, its 2 major releases behind a released version) + private static final boolean isReleasableBranch = true; + /** * Sort versions that have backwards compatibility guarantees from * those that don't. Doesn't actually check whether or not the versions @@ -50,73 +57,107 @@ public class VersionUtils { * guarantees in v1 and versions without the guranteees in v2 */ static Tuple, List> resolveReleasedVersions(Version current, Class versionClass) { - List versions = Version.getDeclaredVersions(versionClass); + TreeSet releasedVersions = new TreeSet<>(Version.getDeclaredVersions(versionClass)); + List unreleasedVersions = new ArrayList<>(); - Version last = versions.remove(versions.size() - 1); - assert last.equals(current) : "The highest version must be the current one " - + "but was [" + last + "] and current was [" + current + "]"; + assert releasedVersions.last().equals(current) : "The highest version must be the current one " + + "but was [" + releasedVersions.last() + "] and current was [" + current + "]"; - if (current.revision != 0) { - /* If we are in a stable branch there should be no unreleased version constants - * because we don't expect to release any new versions in older branches. If there - * are extra constants then gradle will yell about it. */ - return new Tuple<>(unmodifiableList(versions), singletonList(current)); - } + List temporarilyRemovedReleases = new ArrayList<>(); - /* If we are on a patch release then we know that at least the version before the - * current one is unreleased. If it is released then gradle would be complaining. */ - int unreleasedIndex = versions.size() - 1; - while (true) { - if (unreleasedIndex < 0) { - throw new IllegalArgumentException("Couldn't find first non-alpha release"); - } - /* We don't support backwards compatibility for alphas, betas, and rcs. But - * they were released so we add them to the released list. Usually this doesn't - * matter to consumers, but consumers that do care should filter non-release - * versions. */ - if (versions.get(unreleasedIndex).isRelease()) { - break; + for (Version version: releasedVersions) { + if (version.isRelease() == false && isMajorReleased(version, releasedVersions)) { + // remove the Alpha/Beta/RC temporarily for already released versions + temporarilyRemovedReleases.add(version); + } else if (version.isRelease() == false + && version.major == current.major + && version.minor == current.minor + && version.revision == current.revision + && version.build != current.build) { + // remove the Alpha/Beta/RC temporarily for the current version + temporarilyRemovedReleases.add(version); } - unreleasedIndex--; } + releasedVersions.removeAll(temporarilyRemovedReleases); + releasedVersions.remove(current); - Version unreleased = versions.remove(unreleasedIndex); - if (unreleased.revision == 0) { - /* - * If the last unreleased version is itself a patch release then Gradle enforces that there is yet another unreleased version - * before that. However, we have to skip alpha/betas/RCs too (e.g., consider when the version constants are ..., 5.6.3, 5.6.4, - * 6.0.0-alpha1, ..., 6.0.0-rc1, 6.0.0-rc2, 6.0.0, 6.1.0 on the 6.x branch. In this case, we will have pruned 6.0.0 and 6.1.0 as - * unreleased versions, but we also need to prune 5.6.4. At this point though, unreleasedIndex will be pointing to 6.0.0-rc2, so - * we have to skip backwards until we find a non-alpha/beta/RC again. Then we can prune that version as an unreleased version - * too. - */ - do { - unreleasedIndex--; - } while (versions.get(unreleasedIndex).isRelease() == false); - Version earlierUnreleased = versions.remove(unreleasedIndex); - - // This earlierUnreleased is either the snapshot on the minor branch lower, or its possible its a staged release. If it is a - // staged release, remove it and return it in unreleased as well. - if (earlierUnreleased.revision == 0) { - unreleasedIndex--; - Version actualUnreleasedPreviousMinor = versions.remove(unreleasedIndex); - return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(actualUnreleasedPreviousMinor, - earlierUnreleased, unreleased, current))); - } - - return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(earlierUnreleased, unreleased, current))); - } else if (unreleased.major == current.major) { - // need to remove one more of the last major's minor set - do { - unreleasedIndex--; - } while (unreleasedIndex > 0 && versions.get(unreleasedIndex).major == current.major); - if (unreleasedIndex > 0) { - // some of the test cases return very small lists, so its possible this is just the end of the list, if so, dont include it - Version earlierMajorsMinor = versions.remove(unreleasedIndex); - return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(earlierMajorsMinor, unreleased, current))); + if (isReleasableBranch) { + if (isReleased(current)) { + // if the minor has been released then it only has a maintenance version + // go back 1 version to get the last supported snapshot version of the line, which is a maint bugfix + Version highestMinor = getHighestPreviousMinor(current.major, releasedVersions); + releasedVersions.remove(highestMinor); + unreleasedVersions.add(highestMinor); + } else { + // if our currentVersion is a X.0.0, we need to check X-1 minors to see if they are released + if (current.minor == 0) { + boolean hasFoundNextMinor = false; + boolean hasFoundStagedMinor = false; + for (Version version: getMinorTips(current.major - 1, releasedVersions)) { + if (isReleased(version) == false) { + // This should only ever contain 2 non released branches in flight. An example is 6.x is frozen, + // and 6.2 is cut but not yet released there is some simple logic to make sure that in the case of more than 2, + // it will bail. The order is that the minor snapshot is fufilled first, and then the staged minor snapshot + if (hasFoundNextMinor == false) { + hasFoundNextMinor = true; + releasedVersions.remove(version); + unreleasedVersions.add(version); + } else if (hasFoundStagedMinor == false) { + hasFoundStagedMinor = true; + releasedVersions.remove(version); + unreleasedVersions.add(version); + } else { + throw new IllegalArgumentException( + "More than 2 snapshot version existed for the next minor and staged (frozen) minors."); + } + } else { + // this is the last minor snap for this major, so replace the highest (last) one of these and break + releasedVersions.remove(version); + unreleasedVersions.add(version); + // we only care about the largest minor here, so in the case of 6.1 and 6.0, it will only get 6.1 + break; + } + } + // now dip back 2 versions to get the last supported snapshot version of the line + Version highestMinor = getHighestPreviousMinor(current.major - 1, releasedVersions); + releasedVersions.remove(highestMinor); + unreleasedVersions.add(highestMinor); + } else { + // version is not a X.0.0, so we are somewhere on a X.Y line only check till minor == 0 of the major + boolean hasFoundStagedMinor = false; + for (Version version: getMinorTips(current.major, releasedVersions)) { + if (isReleased(version) == false) { + // This should only ever contain 0 or 1 branch in flight. An example is 6.x is frozen, and 6.2 is cut + // but not yet released there is some simple logic to make sure that in the case of more than 1, it will bail + if (hasFoundStagedMinor == false) { + hasFoundStagedMinor = true; + releasedVersions.remove(version); + unreleasedVersions.add(version); + } else { + throw new IllegalArgumentException("More than 1 snapshot version existed for the staged (frozen) minors."); + } + } else { + // this is the last minor snap for this major, so replace the highest (last) one of these and break + releasedVersions.remove(version); + unreleasedVersions.add(version); + // we only care about the largest minor here, so in the case of 6.1 and 6.0, it will only get 6.1 + break; + } + } + // now dip back 1 version to get the last supported snapshot version of the line + Version highestMinor = getHighestPreviousMinor(current.major, releasedVersions); + releasedVersions.remove(highestMinor); + unreleasedVersions.add(highestMinor); + } } } - return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(unreleased, current))); + + // re-add the Alpha/Beta/RC + releasedVersions.addAll(temporarilyRemovedReleases); + unreleasedVersions.add(current); + + Collections.sort(unreleasedVersions); + return new Tuple<>(new ArrayList<>(releasedVersions), unreleasedVersions); } private static final List RELEASED_VERSIONS; @@ -245,4 +286,71 @@ public static Version maxCompatibleVersion(Version version) { return compatible.get(compatible.size() - 1); } + private static Version generateVersion(int major, int minor, int revision) { + return Version.fromString(String.format(Locale.ROOT, "%s.%s.%s", major, minor, revision)); + } + + /** + * Uses basic logic about our releases to determine if this version has been previously released + */ + private static boolean isReleased(Version version) { + return version.revision > 0; + } + + /** + * Validates that the count of non suffixed (alpha/beta/rc) versions in a given major to major+1 is greater than 1. + * This means that there is more than just a major.0.0 or major.0.0-alpha in a branch to signify it has been prevously released. + */ + private static boolean isMajorReleased(Version version, TreeSet items) { + return getMajorSet(version.major, items) + .stream() + .map(v -> v.isRelease()) + .count() > 1; + } + + /** + * Gets the largest version previous major version based on the nextMajorVersion passed in. + * If you have a list [5.0.2, 5.1.2, 6.0.1, 6.1.1] and pass in 6 for the nextMajorVersion, it will return you 5.1.2 + */ + private static Version getHighestPreviousMinor(int majorVersion, TreeSet items){ + return items.headSet(generateVersion(majorVersion, 0, 0)).last(); + } + + /** + * Gets the entire set of major.minor.* given those parameters. + */ + private static SortedSet getMinorSetForMajor(int major, int minor, TreeSet items) { + return items + .tailSet(generateVersion(major, minor, 0)) + .headSet(generateVersion(major, minor + 1, 0)); + } + + /** + * Gets the entire set of major.* to the currentVersion + */ + private static SortedSet getMajorSet(int major, TreeSet items) { + return items + .tailSet(generateVersion(major, 0, 0)) + .headSet(generateVersion(major + 1, 0, 0)); + } + + /** + * Gets the tip of each minor set and puts it in a list. + * + * examples: + * [1.0.0, 1.1.0, 1.1.1, 1.2.0, 1.3.1] will return [1.0.0, 1.1.1, 1.2.0, 1.3.1] + * [1.0.0, 1.0.1, 1.0.2, 1.0.3, 1.0.4] will return [1.0.4] + */ + private static List getMinorTips(int major, TreeSet items) { + SortedSet majorSet = getMajorSet(major, items); + List minorList = new ArrayList<>(); + for (int minor = majorSet.last().minor; minor >= 0; minor--) { + SortedSet minorSetInMajor = getMinorSetForMajor(major, minor, items); + if (minorSetInMajor.isEmpty() == false) { + minorList.add(minorSetInMajor.last()); + } + } + return minorList; + } + } diff --git a/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java b/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java index 3c8b349792b75..cb865b3be508d 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java @@ -107,11 +107,13 @@ public void testRandomVersionBetween() { } public static class TestReleaseBranch { - public static final Version V_5_3_0 = Version.fromString("5.3.0"); - public static final Version V_5_3_1 = Version.fromString("5.3.1"); - public static final Version V_5_3_2 = Version.fromString("5.3.2"); - public static final Version V_5_4_0 = Version.fromString("5.4.0"); - public static final Version V_5_4_1 = Version.fromString("5.4.1"); + public static final Version V_4_0_0 = Version.fromString("4.0.0"); + public static final Version V_4_0_1 = Version.fromString("4.0.1"); + public static final Version V_5_3_0 = Version.fromString("5.0.0"); + public static final Version V_5_3_1 = Version.fromString("5.0.1"); + public static final Version V_5_3_2 = Version.fromString("5.0.2"); + public static final Version V_5_4_0 = Version.fromString("5.1.0"); + public static final Version V_5_4_1 = Version.fromString("5.1.1"); public static final Version CURRENT = V_5_4_1; } public void testResolveReleasedVersionsForReleaseBranch() { @@ -120,19 +122,24 @@ public void testResolveReleasedVersionsForReleaseBranch() { List unreleased = t.v2(); assertThat(released, equalTo(Arrays.asList( + TestReleaseBranch.V_4_0_0, TestReleaseBranch.V_5_3_0, TestReleaseBranch.V_5_3_1, TestReleaseBranch.V_5_3_2, TestReleaseBranch.V_5_4_0))); - assertThat(unreleased, equalTo(Collections.singletonList(TestReleaseBranch.V_5_4_1))); + assertThat(unreleased, equalTo(Arrays.asList( + TestReleaseBranch.V_4_0_1, + TestReleaseBranch.V_5_4_1))); } public static class TestStableBranch { - public static final Version V_5_3_0 = Version.fromString("5.3.0"); - public static final Version V_5_3_1 = Version.fromString("5.3.1"); - public static final Version V_5_3_2 = Version.fromString("5.3.2"); - public static final Version V_5_4_0 = Version.fromString("5.4.0"); - public static final Version CURRENT = V_5_4_0; + public static final Version V_4_0_0 = Version.fromString("4.0.0"); + public static final Version V_4_0_1 = Version.fromString("4.0.1"); + public static final Version V_5_0_0 = Version.fromString("5.0.0"); + public static final Version V_5_0_1 = Version.fromString("5.0.1"); + public static final Version V_5_0_2 = Version.fromString("5.0.2"); + public static final Version V_5_1_0 = Version.fromString("5.1.0"); + public static final Version CURRENT = V_5_1_0; } public void testResolveReleasedVersionsForUnreleasedStableBranch() { Tuple, List> t = VersionUtils.resolveReleasedVersions(TestStableBranch.CURRENT, @@ -141,14 +148,18 @@ public void testResolveReleasedVersionsForUnreleasedStableBranch() { List unreleased = t.v2(); assertThat(released, equalTo(Arrays.asList( - TestStableBranch.V_5_3_0, - TestStableBranch.V_5_3_1))); + TestStableBranch.V_4_0_0, + TestStableBranch.V_5_0_0, + TestStableBranch.V_5_0_1))); assertThat(unreleased, equalTo(Arrays.asList( - TestStableBranch.V_5_3_2, - TestStableBranch.V_5_4_0))); + TestStableBranch.V_4_0_1, + TestStableBranch.V_5_0_2, + TestStableBranch.V_5_1_0))); } public static class TestStableBranchBehindStableBranch { + public static final Version V_4_0_0 = Version.fromString("4.0.0"); + public static final Version V_4_0_1 = Version.fromString("4.0.1"); public static final Version V_5_3_0 = Version.fromString("5.3.0"); public static final Version V_5_3_1 = Version.fromString("5.3.1"); public static final Version V_5_3_2 = Version.fromString("5.3.2"); @@ -163,15 +174,19 @@ public void testResolveReleasedVersionsForStableBranchBehindStableBranch() { List unreleased = t.v2(); assertThat(released, equalTo(Arrays.asList( + TestStableBranchBehindStableBranch.V_4_0_0, TestStableBranchBehindStableBranch.V_5_3_0, TestStableBranchBehindStableBranch.V_5_3_1))); assertThat(unreleased, equalTo(Arrays.asList( + TestStableBranchBehindStableBranch.V_4_0_1, TestStableBranchBehindStableBranch.V_5_3_2, TestStableBranchBehindStableBranch.V_5_4_0, TestStableBranchBehindStableBranch.V_5_5_0))); } public static class TestUnstableBranch { + public static final Version V_4_0_0 = Version.fromString("4.0.0"); + public static final Version V_4_0_1 = Version.fromString("4.0.1"); public static final Version V_5_3_0 = Version.fromString("5.3.0"); public static final Version V_5_3_1 = Version.fromString("5.3.1"); public static final Version V_5_3_2 = Version.fromString("5.3.2"); @@ -189,11 +204,13 @@ public void testResolveReleasedVersionsForUnstableBranch() { List unreleased = t.v2(); assertThat(released, equalTo(Arrays.asList( + TestUnstableBranch.V_4_0_0, TestUnstableBranch.V_5_3_0, TestUnstableBranch.V_5_3_1, TestUnstableBranch.V_6_0_0_alpha1, TestUnstableBranch.V_6_0_0_alpha2))); assertThat(unreleased, equalTo(Arrays.asList( + TestUnstableBranch.V_4_0_1, TestUnstableBranch.V_5_3_2, TestUnstableBranch.V_5_4_0, TestUnstableBranch.V_6_0_0_beta1))); @@ -221,13 +238,13 @@ public void testResolveReleasedVersionsAtNewMajorRelease() { assertThat(released, equalTo(Arrays.asList( TestNewMajorRelease.V_5_6_0, TestNewMajorRelease.V_5_6_1, - TestNewMajorRelease.V_5_6_2, TestNewMajorRelease.V_6_0_0_alpha1, TestNewMajorRelease.V_6_0_0_alpha2, TestNewMajorRelease.V_6_0_0_beta1, TestNewMajorRelease.V_6_0_0_beta2, TestNewMajorRelease.V_6_0_0))); assertThat(unreleased, equalTo(Arrays.asList( + TestNewMajorRelease.V_5_6_2, TestNewMajorRelease.V_6_0_1))); } @@ -306,6 +323,8 @@ public void testResolveReleasedVersionsAtNewMinorBranchIn6x() { } public static class TestIncorrectCurrentVersion { + public static final Version V_4_0_0 = Version.fromString("4.0.0"); + public static final Version V_4_0_1 = Version.fromString("4.0.1"); public static final Version V_5_3_0 = Version.fromString("5.3.0"); public static final Version V_5_3_1 = Version.fromString("5.3.1"); public static final Version V_5_4_0 = Version.fromString("5.4.0");