[Versioning] Fix Version.fromString logic for legacy version (#604)

This commit fixes the Version.fromString logic to identify legacy versions. It
also adds an optional "distribution" field to the MainRespose for OpenSearch
version 1.0.0+. Any preceeding versions that do not contain the distribution
label will be handeled as legacy versions appropriately.

Signed-off-by: Nicholas Walter Knize <[email protected]>

client/rest-high-level/src/test/java/org/opensearch/client/core/MainResponseTests.java

@@ -56,7 +56,7 @@ protected org.opensearch.action.main.MainResponse createServerTestInstance(XCont
         Version version = VersionUtils.randomVersionBetween(random(), LegacyESVersion.V_6_0_1, Version.CURRENT);
         Build build = new Build(
             Build.Type.UNKNOWN, randomAlphaOfLength(8), date, randomBoolean(),
-            version.toString()
+            version.toString(), version.before(Version.V_1_0_0) ? null : "opensearch"
         );
         return new org.opensearch.action.main.MainResponse(nodeName, version, clusterName, clusterUuid , build);
     }

modules/reindex/src/main/java/org/opensearch/index/reindex/remote/RemoteResponseParsers.java

@@ -33,6 +33,7 @@
 package org.opensearch.index.reindex.remote;
 
 import org.apache.lucene.search.TotalHits;
+import org.opensearch.LegacyESVersion;
 import org.opensearch.Version;
 import org.opensearch.common.ParseField;
 import org.opensearch.common.ParsingException;
@@ -292,11 +293,17 @@ public void setCausedBy(Throwable causedBy) {
             "/", true, a -> (Version) a[0]);
     static {
         ConstructingObjectParser<Version, XContentType> versionParser = new ConstructingObjectParser<>(
-                "version", true, a -> Version.fromString(
-                    ((String) a[0])
+                "version", true, a -> a[0] == null ?
+            LegacyESVersion.fromString(
+                    ((String) a[1])
                         .replace("-SNAPSHOT", "")
-                        .replaceFirst("-(alpha\\d+|beta\\d+|rc\\d+)", "")
-        ));
+                        .replaceFirst("-(alpha\\d+|beta\\d+|rc\\d+)", "")) :
+            Version.fromString(
+                ((String) a[1])
+                    .replace("-SNAPSHOT", "")
+                    .replaceFirst("-(alpha\\d+|beta\\d+|rc\\d+)", ""))
+            );
+        versionParser.declareStringOrNull(optionalConstructorArg(), new ParseField("distribution"));
         versionParser.declareString(constructorArg(), new ParseField("number"));
         MAIN_ACTION_PARSER.declareObject(constructorArg(), versionParser, new ParseField("version"));
     }

modules/reindex/src/test/java/org/opensearch/index/reindex/remote/RemoteScrollableHitSourceTests.java

@@ -50,6 +50,7 @@
 import org.apache.http.message.BasicStatusLine;
 import org.apache.http.nio.protocol.HttpAsyncRequestProducer;
 import org.apache.http.nio.protocol.HttpAsyncResponseConsumer;
+import org.opensearch.LegacyESVersion;
 import org.opensearch.OpenSearchStatusException;
 import org.opensearch.Version;
 import org.opensearch.action.bulk.BackoffPolicy;
@@ -149,14 +150,15 @@ public void validateAllConsumed() {
     }
 
     public void testLookupRemoteVersion() throws Exception {
-        assertLookupRemoteVersion(Version.fromString("0.20.5"), "main/0_20_5.json");
-        assertLookupRemoteVersion(Version.fromString("0.90.13"), "main/0_90_13.json");
-        assertLookupRemoteVersion(Version.fromString("1.7.5"), "main/1_7_5.json");
-        assertLookupRemoteVersion(Version.fromId(2030399), "main/2_3_3.json");
+        assertLookupRemoteVersion(LegacyESVersion.fromString("0.20.5"), "main/0_20_5.json");
+        assertLookupRemoteVersion(LegacyESVersion.fromString("0.90.13"), "main/0_90_13.json");
+        assertLookupRemoteVersion(LegacyESVersion.fromString("1.7.5"), "main/1_7_5.json");
+        assertLookupRemoteVersion(LegacyESVersion.fromId(2030399), "main/2_3_3.json");
         // assert for V_5_0_0 (no qualifier) since we no longer consider qualifier in Version since 7
-        assertLookupRemoteVersion(Version.fromId(5000099), "main/5_0_0_alpha_3.json");
+        assertLookupRemoteVersion(LegacyESVersion.fromId(5000099), "main/5_0_0_alpha_3.json");
         // V_5_0_0 since we no longer consider qualifier in Version
-        assertLookupRemoteVersion(Version.fromId(5000099), "main/with_unknown_fields.json");
+        assertLookupRemoteVersion(LegacyESVersion.fromId(5000099), "main/with_unknown_fields.json");
+        assertLookupRemoteVersion(Version.fromId(1000099 ^ Version.MASK), "main/OpenSearch_1_0_0.json");
     }
 
     private void assertLookupRemoteVersion(Version expected, String s) throws Exception {

modules/reindex/src/test/resources/responses/main/OpenSearch_1_0_0.json

@@ -0,0 +1,13 @@
+{
+  "name" : "Ruphos",
+  "cluster_name" : "distribution_run",
+  "version" : {
+    "distribution" : "opensearch",
+    "number" : "1.0.0",
+    "build_hash" : "42e092f",
+    "build_date" : "2016-05-26T16:55:45.405Z",
+    "build_snapshot" : true,
+    "lucene_version" : "8.8.2"
+  },
+  "tagline" : "You Know, for Better Search"
+}

server/src/main/java/org/opensearch/Build.java

@@ -104,11 +104,12 @@ public static Type fromDisplayName(final String displayName, final boolean stric
         final String date;
         final boolean isSnapshot;
         final String version;
+        final String distribution = "opensearch";
 
         // these are parsed at startup, and we require that we are able to recognize the values passed in by the startup scripts
         type = Type.fromDisplayName(System.getProperty("opensearch.distribution.type", "unknown"), true);
 
-        final String opensearchPrefix = "opensearch-" + Version.CURRENT;
+        final String opensearchPrefix = distribution + "-" + Version.CURRENT;
         final URL url = getOpenSearchCodeSourceLocation();
         final String urlStr = url == null ? "" : url.toString();
         if (urlStr.startsWith("file:/") && (
@@ -155,7 +156,7 @@ public static Type fromDisplayName(final String displayName, final boolean stric
                 "Stopping OpenSearch now so it doesn't run in subtly broken ways. This is likely a build bug.");
         }
 
-        CURRENT = new Build(type, hash, date, isSnapshot, version);
+        CURRENT = new Build(type, hash, date, isSnapshot, version, distribution);
     }
 
     private final boolean isSnapshot;
@@ -174,16 +175,18 @@ static URL getOpenSearchCodeSourceLocation() {
     private final String hash;
     private final String date;
     private final String version;
+    private final String distribution;
 
     public Build(
         final Type type, final String hash, final String date, boolean isSnapshot,
-        String version
-    ) {
+        String version,
+        String distribution) {
         this.type = type;
         this.hash = hash;
         this.date = date;
         this.isSnapshot = isSnapshot;
         this.version = version;
+        this.distribution = distribution;
     }
 
     public String hash() {
@@ -195,8 +198,16 @@ public String date() {
     }
 
     public static Build readBuild(StreamInput in) throws IOException {
+        final String distribution;
         final String flavor;
         final Type type;
+        // the following is new for opensearch: we write the distribution to support any "forks"
+        if (in.getVersion().onOrAfter(Version.V_1_0_0)) {
+            distribution = in.readString();
+        } else {
+            distribution = "other";
+        }
+
         // The following block is kept for existing BWS tests to pass.
         // TODO - clean up this code when we remove all v6 bwc tests.
         // TODO - clean this up when OSS flavor is removed in all of the code base
@@ -221,10 +232,15 @@ public static Build readBuild(StreamInput in) throws IOException {
         } else {
             version = in.getVersion().toString();
         }
-        return new Build(type, hash, date, snapshot, version);
+        return new Build(type, hash, date, snapshot, version, distribution);
     }
 
     public static void writeBuild(Build build, StreamOutput out) throws IOException {
+        // the following is new for opensearch: we write the distribution name to support any "forks" of the code
+        if (out.getVersion().onOrAfter(Version.V_1_0_0)) {
+            out.writeString(build.distribution);
+        }
+
         // The following block is kept for existing BWS tests to pass.
         // TODO - clean up this code when we remove all v6 bwc tests.
         // TODO - clean this up when OSS flavor is removed in all of the code base
@@ -249,6 +265,14 @@ public static void writeBuild(Build build, StreamOutput out) throws IOException
         }
     }
 
+    /**
+     * Get the distribution name (expected to be OpenSearch; empty if legacy; something else if forked)
+     * @return distribution name as a string
+     */
+    public String getDistribution() {
+        return distribution;
+    }
+
     /**
      * Get the version as considered at build time
      *

server/src/main/java/org/opensearch/LegacyESVersion.java

@@ -32,6 +32,12 @@
 
 package org.opensearch;
 
+import org.opensearch.common.Strings;
+import org.opensearch.common.collect.ImmutableOpenIntMap;
+import org.opensearch.common.collect.ImmutableOpenMap;
+
+import java.lang.reflect.Field;
+
 /**
  * The Contents of this file were originally moved from {@link Version}.
  *
@@ -135,6 +141,60 @@ public class LegacyESVersion extends Version {
     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;
+    protected static final ImmutableOpenMap<String, Version> stringToVersion;
+    static {
+        final ImmutableOpenIntMap.Builder<Version> builder = ImmutableOpenIntMap.builder();
+        final ImmutableOpenMap.Builder<String, Version> builderByString = ImmutableOpenMap.builder();
+
+        for (final Field declaredField : LegacyESVersion.class.getFields()) {
+            if (declaredField.getType().equals(Version.class) || declaredField.getType().equals(LegacyESVersion.class)) {
+                final String fieldName = declaredField.getName();
+                if (fieldName.equals("CURRENT") || fieldName.equals("V_EMPTY")) {
+                    continue;
+                }
+                assert fieldName.matches("V_\\d+_\\d+_\\d+(_alpha[1,2]|_beta[1,2]|_rc[1,2])?")
+                    : "expected Version field [" + fieldName + "] to match V_\\d+_\\d+_\\d+";
+                try {
+                    final Version version = (Version) declaredField.get(null);
+                    if (Assertions.ENABLED) {
+                        final String[] fields = fieldName.split("_");
+                        if (fields.length == 5) {
+                            assert (fields[1].equals("1") || fields[1].equals("6")) && fields[2].equals("0") :
+                                "field " + fieldName + " should not have a build qualifier";
+                        } else {
+                            final int major = Integer.valueOf(fields[1]) * 1000000;
+                            final int minor = Integer.valueOf(fields[2]) * 10000;
+                            final int revision = Integer.valueOf(fields[3]) * 100;
+                            final int expectedId;
+                            if (fields[1].equals("1")) {
+                                expectedId = 0x08000000 ^ (major + minor + revision + 99);
+                            } else {
+                                expectedId = (major + minor + revision + 99);
+                            }
+                            assert version.id == expectedId :
+                                "expected version [" + fieldName + "] to have id [" + expectedId + "] but was [" + version.id + "]";
+                        }
+                    }
+                    final Version maybePrevious = builder.put(version.id, version);
+                    builderByString.put(version.toString(), version);
+                    assert maybePrevious == null :
+                        "expected [" + version.id + "] to be uniquely mapped but saw [" + maybePrevious + "] and [" + version + "]";
+                } catch (final IllegalAccessException e) {
+                    assert false : "Version field [" + fieldName + "] should be public";
+                }
+            }
+        }
+        assert CURRENT.luceneVersion.equals(org.apache.lucene.util.Version.LATEST) : "Version must be upgraded to ["
+            + org.apache.lucene.util.Version.LATEST + "] is still set to [" + CURRENT.luceneVersion + "]";
+
+        builder.put(V_EMPTY_ID, V_EMPTY);
+        builderByString.put(V_EMPTY.toString(), V_EMPTY);
+        idToVersion = builder.build();
+        stringToVersion = builderByString.build();
+    }
+
     protected LegacyESVersion(int id, org.apache.lucene.util.Version luceneVersion) {
         // flip the 28th bit of the version id
         // this will be flipped back in the parent class to correctly identify as a legacy version;
@@ -156,4 +216,75 @@ public boolean isBeta() {
     public boolean isAlpha() {
         return major < 5 ? false :  build < 25;
     }
+
+    /**
+     * Returns the version given its string representation, current version if the argument is null or empty
+     */
+    public static Version fromString(String version) {
+        if (!Strings.hasLength(version)) {
+            return Version.CURRENT;
+        }
+        final Version cached = stringToVersion.get(version);
+        if (cached != null) {
+            return cached;
+        }
+        return fromStringSlow(version);
+    }
+
+    private static Version fromStringSlow(String version) {
+        final boolean snapshot; // this is some BWC for 2.x and before indices
+        if (snapshot = version.endsWith("-SNAPSHOT")) {
+            version = version.substring(0, version.length() - 9);
+        }
+        String[] parts = version.split("[.-]");
+        if (parts.length < 3 || parts.length > 4) {
+            throw new IllegalArgumentException(
+                "the version needs to contain major, minor, and revision, and optionally the build: " + version);
+        }
+
+        try {
+            final int rawMajor = Integer.parseInt(parts[0]);
+            if (rawMajor >= 5 && snapshot) { // we don't support snapshot as part of the version here anymore
+                throw new IllegalArgumentException("illegal version format - snapshots are only supported until version 2.x");
+            }
+            if (rawMajor >=7 && parts.length == 4) { // we don't support qualifier as part of the version anymore
+                throw new IllegalArgumentException("illegal version format - qualifiers are only supported until version 6.x");
+            }
+            final int betaOffset = rawMajor < 5 ? 0 : 25;
+            //we reverse the version id calculation based on some assumption as we can't reliably reverse the modulo
+            final int major = rawMajor * 1000000;
+            final int minor = Integer.parseInt(parts[1]) * 10000;
+            final int revision = Integer.parseInt(parts[2]) * 100;
+
+
+            int build = 99;
+            if (parts.length == 4) {
+                String buildStr = parts[3];
+                if (buildStr.startsWith("alpha")) {
+                    assert rawMajor >= 5 : "major must be >= 5 but was " + major;
+                    build = Integer.parseInt(buildStr.substring(5));
+                    assert build < 25 : "expected a alpha build but " + build + " >= 25";
+                } else if (buildStr.startsWith("Beta") || buildStr.startsWith("beta")) {
+                    build = betaOffset + Integer.parseInt(buildStr.substring(4));
+                    assert build < 50 : "expected a beta build but " + build + " >= 50";
+                } else if (buildStr.startsWith("RC") || buildStr.startsWith("rc")) {
+                    build = Integer.parseInt(buildStr.substring(2)) + 50;
+                } else {
+                    throw new IllegalArgumentException("unable to parse version " + version);
+                }
+            }
+
+            return Version.fromId(major + minor + revision + build);
+
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("unable to parse version " + version, e);
+        }
+    }
+
+    /**
+     * this is used to ensure the version id for new versions of OpenSearch are always less than the predecessor versions
+     */
+    protected int maskId(final int id) {
+        return id;
+    }
 }