Make TransportVersion final (#95781)

Remove static checks duplicated by tests

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

@@ -15,7 +15,6 @@ import org.elasticsearch.core.Assertions;
 
 import java.io.IOException;
 import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -23,8 +22,6 @@ import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Set;
 import java.util.TreeMap;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 /**
  * Represents the version of the wire protocol used to communicate between ES nodes.
@@ -60,7 +57,7 @@ import java.util.regex.Pattern;
  * If you revert a commit with a transport version change, you <em>must</em> ensure there is a <em>new</em> transport version
  * representing the reverted change. <em>Do not</em> let the transport version go backwards, it must <em>always</em> be incremented.
  */
-public class TransportVersion implements Comparable<TransportVersion> {
+public final class TransportVersion implements Comparable<TransportVersion> {
     public static final TransportVersion ZERO = new TransportVersion(0, "00000000-0000-0000-0000-000000000000");
     public static final TransportVersion V_7_0_0 = new TransportVersion(7_00_00_99, "7505fd05-d982-43ce-a63f-ff4c6c8bdeec");
     public static final TransportVersion V_7_0_1 = new TransportVersion(7_00_01_99, "ae772780-e6f9-46a1-b0a0-20ed0cae37f7");
@@ -183,8 +180,6 @@ public class TransportVersion implements Comparable<TransportVersion> {
         NavigableMap<Integer, TransportVersion> builder = new TreeMap<>();
 
         Set<String> ignore = Set.of("ZERO", "CURRENT", "MINIMUM_COMPATIBLE", "MINIMUM_CCS_VERSION");
-        Pattern bwcVersionField = Pattern.compile("^V_(\\d_\\d{1,2}_\\d{1,2})$");
-        Pattern transportVersionField = Pattern.compile("^V_(\\d+_\\d{3}_\\d{3})$");
 
         for (Field declaredField : cls.getFields()) {
             if (declaredField.getType().equals(TransportVersion.class)) {
@@ -193,12 +188,6 @@ public class TransportVersion implements Comparable<TransportVersion> {
                     continue;
                 }
 
-                // check the field modifiers
-                if (declaredField.getModifiers() != (Modifier.PUBLIC | Modifier.STATIC | Modifier.FINAL)) {
-                    assert false : "Version field [" + fieldName + "] should be public static final";
-                    continue;
-                }
-
                 TransportVersion version;
                 try {
                     version = (TransportVersion) declaredField.get(null);
@@ -220,28 +209,6 @@ public class TransportVersion implements Comparable<TransportVersion> {
                             + version.id
                             + "]. Each TransportVersion should have a different version number";
 
-                    // check the name matches the version number
-                    try {
-                        int fieldNumber;
-                        int idNumber = version.id;
-                        Matcher matcher = bwcVersionField.matcher(fieldName);
-                        if (matcher.matches()) {
-                            // match single digits _\d_ or _\d$ to put a 0 in front, but do not actually capture the _ or $
-                            fieldNumber = Integer.parseInt(matcher.group(1).replaceAll("_(\\d)(?=_|$)", "_0$1").replace("_", ""));
-                            idNumber /= 100;    // remove the extra '99'
-                        } else if ((matcher = transportVersionField.matcher(fieldName)).matches()) {
-                            fieldNumber = Integer.parseInt(matcher.group(1).replace("_", ""));
-                        } else {
-                            assert false : "Version [" + fieldName + "] does not have the correct name format";
-                            continue;
-                        }
-
-                        assert fieldNumber == idNumber : "Version [" + fieldName + "] does not match its version number [" + idNumber + "]";
-                    } catch (NumberFormatException e) {
-                        assert false : "Version [" + fieldName + "] does not have the correct name format";
-                        continue;
-                    }
-
                     // check the id is unique
                     var sameUniqueId = uniqueIdFields.put(version.uniqueId, fieldName);
                     assert sameUniqueId == null

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

@@ -51,10 +51,6 @@ public class TransportVersionTests extends ESTestCase {
         assertThat(V_8_0_0, is(greaterThan(V_7_2_0)));
     }
 
-    private static String padNumber(String number) {
-        return number.length() == 1 ? "0" + number : number;
-    }
-
     public static class CorrectFakeVersion {
         public static final TransportVersion V_0_00_01 = new TransportVersion(199, "1");
         public static final TransportVersion V_0_000_002 = new TransportVersion(2, "2");
@@ -62,10 +58,6 @@ public class TransportVersionTests extends ESTestCase {
         public static final TransportVersion V_0_000_004 = new TransportVersion(4, "4");
     }
 
-    public static class IncorrectFormatVersion {
-        public static final TransportVersion V_1 = new TransportVersion(1, "1");
-    }
-
     public static class DuplicatedIdFakeVersion {
         public static final TransportVersion V_0_000_001 = new TransportVersion(1, "1");
         public static final TransportVersion V_0_000_002 = new TransportVersion(2, "2");
@@ -94,23 +86,31 @@ public class TransportVersionTests extends ESTestCase {
                 )
             )
         );
-        AssertionError e = expectThrows(AssertionError.class, () -> TransportVersion.getAllVersionIds(IncorrectFormatVersion.class));
-        assertThat(e.getMessage(), containsString("does not have the correct name format"));
-        e = expectThrows(AssertionError.class, () -> TransportVersion.getAllVersionIds(DuplicatedIdFakeVersion.class));
+        AssertionError e = expectThrows(AssertionError.class, () -> TransportVersion.getAllVersionIds(DuplicatedIdFakeVersion.class));
         assertThat(e.getMessage(), containsString("have the same version number"));
         e = expectThrows(AssertionError.class, () -> TransportVersion.getAllVersionIds(DuplicatedStringIdFakeVersion.class));
         assertThat(e.getMessage(), containsString("have the same unique id"));
     }
 
+    private static String padNumber(String number) {
+        return number.length() == 1 ? "0" + number : number;
+    }
+
     public void testDefinedConstants() throws IllegalAccessException {
         Pattern historicalVersion = Pattern.compile("^V_(\\d{1,2})_(\\d{1,2})_(\\d{1,2})$");
-        Pattern transportVersion = Pattern.compile("^V_(\\d{2,})_(\\d{3})_(\\d{3})$");
+        Pattern transportVersion = Pattern.compile("^V_(\\d+)_(\\d{3})_(\\d{3})$");
         Set<String> ignore = Set.of("ZERO", "CURRENT", "MINIMUM_COMPATIBLE", "MINIMUM_CCS_VERSION");
 
         for (java.lang.reflect.Field field : TransportVersion.class.getFields()) {
-            if (Modifier.isStatic(field.getModifiers())
-                && field.getType() == TransportVersion.class
-                && ignore.contains(field.getName()) == false) {
+            if (field.getType() == TransportVersion.class && ignore.contains(field.getName()) == false) {
+
+                // check the field modifiers
+                assertEquals(
+                    "Field " + field.getName() + " should be public static final",
+                    Modifier.PUBLIC | Modifier.STATIC | Modifier.FINAL,
+                    field.getModifiers()
+                );
+
                 Matcher historical = historicalVersion.matcher(field.getName());
                 Matcher transport;
                 if (historical.matches()) {