TransportVersionUtils#randomVersionBetween does not work with version…

… extensions (106119) (elastic#116198)

Introduces a new extension method to VersionExtension enabling extensions to provide additional versions and creates method TransportVersion.getAllVersions returning all transport versions defined by Elasticsearch and the extension. This ensures that TransportVersion.current() always returns the correct current (latest) transport version even if it is defined by an extension.

Fixes elastic#106119

server/src/main/java/org/elasticsearch/TransportVersion.java

@@ -16,7 +16,14 @@
 import org.elasticsearch.plugins.ExtensionLoader;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 import java.util.ServiceLoader;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Represents the version of the wire protocol used to communicate between a pair of ES nodes.
@@ -56,8 +63,14 @@ public static TransportVersion readVersion(StreamInput in) throws IOException {
         return fromId(in.readVInt());
     }
 
+    /**
+     * Finds a {@code TransportVersion} by its id.
+     * If a transport version with the specified ID does not exist,
+     * this method creates and returns a new instance of {@code TransportVersion} with the specified ID.
+     * The new instance is not registered in {@code TransportVersion.getAllVersions}.
+     */
     public static TransportVersion fromId(int id) {
-        TransportVersion known = TransportVersions.VERSION_IDS.get(id);
+        TransportVersion known = VersionsHolder.ALL_VERSIONS_MAP.get(id);
         if (known != null) {
             return known;
         }
@@ -95,7 +108,14 @@ public static boolean isCompatible(TransportVersion version) {
      * This should be the transport version with the highest id.
      */
     public static TransportVersion current() {
-        return CurrentHolder.CURRENT;
+        return VersionsHolder.CURRENT;
+    }
+
+    /**
+     * Sorted list of all defined transport versions
+     */
+    public static List<TransportVersion> getAllVersions() {
+        return VersionsHolder.ALL_VERSIONS;
     }
 
     public static TransportVersion fromString(String str) {
@@ -139,16 +159,25 @@ public String toString() {
         return Integer.toString(id);
     }
 
-    private static class CurrentHolder {
-        private static final TransportVersion CURRENT = findCurrent();
+    private static class VersionsHolder {
+        private static final List<TransportVersion> ALL_VERSIONS;
+        private static final Map<Integer, TransportVersion> ALL_VERSIONS_MAP;
+        private static final TransportVersion CURRENT;
+
+        static {
+            Collection<TransportVersion> extendedVersions = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
+                .map(VersionExtension::getTransportVersions)
+                .orElse(Collections.emptyList());
+
+            if (extendedVersions.isEmpty()) {
+                ALL_VERSIONS = TransportVersions.DEFINED_VERSIONS;
+            } else {
+                ALL_VERSIONS = Stream.concat(TransportVersions.DEFINED_VERSIONS.stream(), extendedVersions.stream()).sorted().toList();
+            }
+
+            ALL_VERSIONS_MAP = ALL_VERSIONS.stream().collect(Collectors.toUnmodifiableMap(TransportVersion::id, Function.identity()));
 
-        // finds the pluggable current version
-        private static TransportVersion findCurrent() {
-            var version = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
-                .map(e -> e.getCurrentTransportVersion(TransportVersions.LATEST_DEFINED))
-                .orElse(TransportVersions.LATEST_DEFINED);
-            assert version.onOrAfter(TransportVersions.LATEST_DEFINED);
-            return version;
+            CURRENT = ALL_VERSIONS.getLast();
         }
     }
 }

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -13,13 +13,12 @@
 import org.elasticsearch.core.UpdateForV9;
 
 import java.lang.reflect.Field;
-import java.util.Collection;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
-import java.util.NavigableMap;
 import java.util.Set;
-import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.function.IntFunction;
 
@@ -209,21 +208,24 @@ static TransportVersion def(int id) {
      */
     public static final TransportVersion MINIMUM_CCS_VERSION = V_8_15_2;
 
-    static final NavigableMap<Integer, TransportVersion> VERSION_IDS = getAllVersionIds(TransportVersions.class);
+    /**
+     * Sorted list of all versions defined in this class
+     */
+    static final List<TransportVersion> DEFINED_VERSIONS = collectAllVersionIdsDefinedInClass(TransportVersions.class);
 
-    // the highest transport version constant defined in this file, used as a fallback for TransportVersion.current()
+    // the highest transport version constant defined
     static final TransportVersion LATEST_DEFINED;
     static {
-        LATEST_DEFINED = VERSION_IDS.lastEntry().getValue();
+        LATEST_DEFINED = DEFINED_VERSIONS.getLast();
 
         // see comment on IDS field
         // now we're registered all the transport versions, we can clear the map
         IDS = null;
     }
 
-    public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?> cls) {
+    public static List<TransportVersion> collectAllVersionIdsDefinedInClass(Class<?> cls) {
         Map<Integer, String> versionIdFields = new HashMap<>();
-        NavigableMap<Integer, TransportVersion> builder = new TreeMap<>();
+        List<TransportVersion> definedTransportVersions = new ArrayList<>();
 
         Set<String> ignore = Set.of("ZERO", "CURRENT", "MINIMUM_COMPATIBLE", "MINIMUM_CCS_VERSION");
 
@@ -240,7 +242,7 @@ public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?>
                 } catch (IllegalAccessException e) {
                     throw new AssertionError(e);
                 }
-                builder.put(version.id(), version);
+                definedTransportVersions.add(version);
 
                 if (Assertions.ENABLED) {
                     // check the version number is unique
@@ -257,11 +259,9 @@ public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?>
             }
         }
 
-        return Collections.unmodifiableNavigableMap(builder);
-    }
+        Collections.sort(definedTransportVersions);
 
-    static Collection<TransportVersion> getAllVersions() {
-        return VERSION_IDS.values();
+        return List.copyOf(definedTransportVersions);
     }
 
     static final IntFunction<String> VERSION_LOOKUP = ReleaseVersions.generateVersionsLookup(TransportVersions.class, LATEST_DEFINED.id());

server/src/main/java/org/elasticsearch/internal/VersionExtension.java

@@ -12,17 +12,16 @@
 import org.elasticsearch.TransportVersion;
 import org.elasticsearch.index.IndexVersion;
 
+import java.util.List;
+
 /**
  * Allows plugging in current version elements.
  */
 public interface VersionExtension {
     /**
-     * Returns the {@link TransportVersion} that Elasticsearch should use.
-     * <p>
-     * This must be at least as high as the given fallback.
-     * @param fallback The latest transport version from server
+     * Returns list of {@link TransportVersion} defined by extension
      */
-    TransportVersion getCurrentTransportVersion(TransportVersion fallback);
+    List<TransportVersion> getTransportVersions();
 
     /**
      * Returns the {@link IndexVersion} that Elasticsearch should use.

server/src/test/java/org/elasticsearch/TransportVersionTests.java

@@ -14,7 +14,7 @@
 
 import java.lang.reflect.Modifier;
 import java.util.Collections;
-import java.util.Map;
+import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.regex.Matcher;
@@ -69,21 +69,20 @@ public static class DuplicatedIdFakeVersion {
 
     public void testStaticTransportVersionChecks() {
         assertThat(
-            TransportVersions.getAllVersionIds(CorrectFakeVersion.class),
+            TransportVersions.collectAllVersionIdsDefinedInClass(CorrectFakeVersion.class),
             equalTo(
-                Map.of(
-                    199,
-                    CorrectFakeVersion.V_0_00_01,
-                    2,
+                List.of(
                     CorrectFakeVersion.V_0_000_002,
-                    3,
                     CorrectFakeVersion.V_0_000_003,
-                    4,
-                    CorrectFakeVersion.V_0_000_004
+                    CorrectFakeVersion.V_0_000_004,
+                    CorrectFakeVersion.V_0_00_01
                 )
             )
         );
-        AssertionError e = expectThrows(AssertionError.class, () -> TransportVersions.getAllVersionIds(DuplicatedIdFakeVersion.class));
+        AssertionError e = expectThrows(
+            AssertionError.class,
+            () -> TransportVersions.collectAllVersionIdsDefinedInClass(DuplicatedIdFakeVersion.class)
+        );
         assertThat(e.getMessage(), containsString("have the same version number"));
     }
 
@@ -186,7 +185,7 @@ public void testVersionConstantPresent() {
     }
 
     public void testCURRENTIsLatest() {
-        assertThat(Collections.max(TransportVersions.getAllVersions()), is(TransportVersion.current()));
+        assertThat(Collections.max(TransportVersion.getAllVersions()), is(TransportVersion.current()));
     }
 
     public void testToReleaseVersion() {
@@ -210,7 +209,7 @@ public void testToString() {
     public void testDenseTransportVersions() {
         Set<Integer> missingVersions = new TreeSet<>();
         TransportVersion previous = null;
-        for (var tv : TransportVersions.getAllVersions()) {
+        for (var tv : TransportVersion.getAllVersions()) {
             if (tv.before(TransportVersions.V_8_16_0)) {
                 continue;
             }

test/framework/src/main/java/org/elasticsearch/test/BWCVersions.java

@@ -15,14 +15,13 @@
 import java.util.Collections;
 import java.util.List;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
-
 public final class BWCVersions {
     private BWCVersions() {}
 
     public static List<TransportVersion> getAllBWCVersions() {
-        int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
-        return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
+        List<TransportVersion> allVersions = TransportVersion.getAllVersions();
+        int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
+        return allVersions.subList(minCompatVersion, allVersions.size());
     }
 
     public static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();

test/framework/src/main/java/org/elasticsearch/test/TransportVersionUtils.java

@@ -19,32 +19,30 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
-
 public class TransportVersionUtils {
     /** Returns all released versions */
     public static List<TransportVersion> allReleasedVersions() {
-        return ALL_VERSIONS;
+        return TransportVersion.getAllVersions();
     }
 
     /** Returns the oldest known {@link TransportVersion} */
     public static TransportVersion getFirstVersion() {
-        return ALL_VERSIONS.get(0);
+        return allReleasedVersions().getFirst();
     }
 
     /** Returns a random {@link TransportVersion} from all available versions. */
     public static TransportVersion randomVersion() {
-        return ESTestCase.randomFrom(ALL_VERSIONS);
+        return ESTestCase.randomFrom(allReleasedVersions());
     }
 
     /** Returns a random {@link TransportVersion} from all available versions without the ignore set */
     public static TransportVersion randomVersion(Set<TransportVersion> ignore) {
-        return ESTestCase.randomFrom(ALL_VERSIONS.stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
+        return ESTestCase.randomFrom(allReleasedVersions().stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
     }
 
     /** Returns a random {@link TransportVersion} from all available versions. */
     public static TransportVersion randomVersion(Random random) {
-        return ALL_VERSIONS.get(random.nextInt(ALL_VERSIONS.size()));
+        return allReleasedVersions().get(random.nextInt(allReleasedVersions().size()));
     }
 
     /** Returns a random {@link TransportVersion} between <code>minVersion</code> and <code>maxVersion</code> (inclusive). */
@@ -58,12 +56,13 @@ public static TransportVersion randomVersionBetween(
         }
 
         int minVersionIndex = 0;
+        List<TransportVersion> allReleasedVersions = allReleasedVersions();
         if (minVersion != null) {
-            minVersionIndex = Collections.binarySearch(ALL_VERSIONS, minVersion);
+            minVersionIndex = Collections.binarySearch(allReleasedVersions, minVersion);
         }
-        int maxVersionIndex = ALL_VERSIONS.size() - 1;
+        int maxVersionIndex = allReleasedVersions.size() - 1;
         if (maxVersion != null) {
-            maxVersionIndex = Collections.binarySearch(ALL_VERSIONS, maxVersion);
+            maxVersionIndex = Collections.binarySearch(allReleasedVersions, maxVersion);
         }
         if (minVersionIndex < 0) {
             throw new IllegalArgumentException("minVersion [" + minVersion + "] does not exist.");
@@ -72,7 +71,7 @@ public static TransportVersion randomVersionBetween(
         } else {
             // minVersionIndex is inclusive so need to add 1 to this index
             int range = maxVersionIndex + 1 - minVersionIndex;
-            return ALL_VERSIONS.get(minVersionIndex + random.nextInt(range));
+            return allReleasedVersions.get(minVersionIndex + random.nextInt(range));
         }
     }
 
@@ -83,7 +82,7 @@ public static TransportVersion getPreviousVersion() {
     }
 
     public static TransportVersion getPreviousVersion(TransportVersion version) {
-        int place = Collections.binarySearch(ALL_VERSIONS, version);
+        int place = Collections.binarySearch(allReleasedVersions(), version);
         if (place < 0) {
             // version does not exist - need the item before the index this version should be inserted
             place = -(place + 1);
@@ -92,15 +91,16 @@ public static TransportVersion getPreviousVersion(TransportVersion version) {
         if (place < 1) {
             throw new IllegalArgumentException("couldn't find any released versions before [" + version + "]");
         }
-        return ALL_VERSIONS.get(place - 1);
+        return allReleasedVersions().get(place - 1);
     }
 
     public static TransportVersion getNextVersion(TransportVersion version) {
         return getNextVersion(version, false);
     }
 
     public static TransportVersion getNextVersion(TransportVersion version, boolean createIfNecessary) {
-        int place = Collections.binarySearch(ALL_VERSIONS, version);
+        List<TransportVersion> allReleasedVersions = allReleasedVersions();
+        int place = Collections.binarySearch(allReleasedVersions, version);
         if (place < 0) {
             // version does not exist - need the item at the index this version should be inserted
             place = -(place + 1);
@@ -109,15 +109,15 @@ public static TransportVersion getNextVersion(TransportVersion version, boolean
             place++;
         }
 
-        if (place < 0 || place >= ALL_VERSIONS.size()) {
+        if (place < 0 || place >= allReleasedVersions.size()) {
             if (createIfNecessary) {
                 // create a new transport version one greater than specified
                 return new TransportVersion(version.id() + 1);
             } else {
                 throw new IllegalArgumentException("couldn't find any released versions after [" + version + "]");
             }
         }
-        return ALL_VERSIONS.get(place);
+        return allReleasedVersions.get(place);
     }
 
     /** Returns a random {@code TransportVersion} that is compatible with {@link TransportVersion#current()} */

x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/AbstractBWCSerializationTestCase.java

@@ -16,14 +16,14 @@
 import java.util.Collections;
 import java.util.List;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
 import static org.hamcrest.Matchers.equalTo;
 
 public abstract class AbstractBWCSerializationTestCase<T extends Writeable & ToXContent> extends AbstractXContentSerializingTestCase<T> {
 
     private static List<TransportVersion> getAllBWCVersions() {
-        int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
-        return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
+        List<TransportVersion> allVersions = TransportVersion.getAllVersions();
+        int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
+        return allVersions.subList(minCompatVersion, allVersions.size());
     }
 
     private static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();