Update transport version directory structure (#132891) (#132990)

This changes the directory structure for transport version files to the following:

/transport/latest
/transport/definitions/named
/transport/definitions/initial
/transport/definitions/manifest.txt

This allows manifest to be placed in a location for named and initial 
transport versions without adding confusion to latest.

ES-12401

.gitignore

@@ -69,7 +69,7 @@ testfixtures_shared/
 # Generated
 checkstyle_ide.xml
 x-pack/plugin/esql/src/main/generated-src/generated/
-server/src/main/resources/transport/defined/manifest.txt
+server/src/main/resources/transport/definitions/manifest.txt
 
 # JEnv
 .java-version

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy

@@ -42,8 +42,12 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         }
     }
 
-    def definedTransportVersion(String name, String ids) {
-        javaResource("myserver", "transport/defined/" + name + ".csv", ids)
+    def namedTransportVersion(String name, String ids) {
+        javaResource("myserver", "transport/definitions/named/" + name + ".csv", ids)
+    }
+
+    def initialTransportVersion(String name, String id) {
+        javaResource("myserver", "transport/definitions/initial/" + name + ".csv", id)
     }
 
     def definedAndUsedTransportVersion(String name, String ids) {
@@ -54,7 +58,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         javaSource("myserver", "org.elasticsearch", classname, "", """
             static final TransportVersion usage = TransportVersion.fromName("${name}");
         """)
-        definedTransportVersion(name, ids)
+        namedTransportVersion(name, ids)
     }
 
     def latestTransportVersion(String branch, String name, String id) {
@@ -95,8 +99,9 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
             apply plugin: 'elasticsearch.transport-version-references'
             apply plugin: 'elasticsearch.transport-version-resources'
         """
-        definedTransportVersion("existing_91", "8012000")
-        definedTransportVersion("existing_92", "8123000,8012001")
+        namedTransportVersion("existing_91", "8012000")
+        namedTransportVersion("existing_92", "8123000,8012001")
+        initialTransportVersion("initial_9_0_0", "8000000")
         latestTransportVersion("9.2", "existing_92", "8123000")
         latestTransportVersion("9.1", "existing_92", "8012001")
         // a mock version of TransportVersion, just here so we can compile Dummy.java et al
@@ -148,12 +153,12 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
 
     def "references must be defined"() {
         given:
-        definedTransportVersion("not_used", "1000000")
+        namedTransportVersion("not_used", "1000000")
         when:
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/not_used.csv] is not referenced")
+            "[myserver/src/main/resources/transport/definitions/named/not_used.csv] is not referenced")
     }
 
     def "names must be lowercase alphanum or underscore"() {
@@ -163,7 +168,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/${name}.csv] does not have a valid name, " +
+            "[myserver/src/main/resources/transport/definitions/named/${name}.csv] does not have a valid name, " +
             "must be lowercase alphanumeric and underscore")
 
         where:
@@ -177,7 +182,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/empty.csv] does not contain any ids")
+            "[myserver/src/main/resources/transport/definitions/named/empty.csv] does not contain any ids")
     }
 
     def "definitions have ids in descending order"() {
@@ -187,7 +192,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/out_of_order.csv] does not have ordered ids")
+            "[myserver/src/main/resources/transport/definitions/named/out_of_order.csv] does not have ordered ids")
     }
 
     def "definition ids are unique"() {
@@ -197,8 +202,8 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/existing_92.csv] contains id 8123000 already defined in " +
-            "[myserver/src/main/resources/transport/defined/duplicate.csv]")
+            "[myserver/src/main/resources/transport/definitions/named/existing_92.csv] contains id 8123000 already defined in " +
+            "[myserver/src/main/resources/transport/definitions/named/duplicate.csv]")
     }
 
     def "definitions have bwc ids with non-zero patch part"() {
@@ -208,27 +213,27 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/patched.csv] contains bwc id [8100000] with a patch part of 0")
+            "[myserver/src/main/resources/transport/definitions/named/patched.csv] contains bwc id [8100000] with a patch part of 0")
     }
 
     def "definitions have primary ids which cannot change"() {
         given:
-        definedTransportVersion("existing_92", "8500000")
+        namedTransportVersion("existing_92", "8500000")
         when:
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/existing_92.csv] has modified primary id from 8123000 to 8500000")
+            "[myserver/src/main/resources/transport/definitions/named/existing_92.csv] has modified primary id from 8123000 to 8500000")
     }
 
     def "cannot change committed ids to a branch"() {
         given:
-        definedTransportVersion("existing_92", "8123000,8012002")
+        namedTransportVersion("existing_92", "8123000,8012002")
         when:
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/existing_92.csv] modifies existing patch id from 8012001 to 8012002")
+            "[myserver/src/main/resources/transport/definitions/named/existing_92.csv] modifies existing patch id from 8012001 to 8012002")
     }
 
     def "latest files must reference defined name"() {
@@ -249,7 +254,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         then:
         assertDefinitionsFailure(result, "Latest transport version file " +
             "[myserver/src/main/resources/transport/latest/9.2.csv] has id 8124000 which is not in definition " +
-            "[myserver/src/main/resources/transport/defined/existing_92.csv]")
+            "[myserver/src/main/resources/transport/definitions/named/existing_92.csv]")
     }
 
     def "latest files have latest id within base"() {
@@ -296,6 +301,6 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
         def result = validateDefinitionsFails()
         then:
         assertDefinitionsFailure(result, "Transport version definition file " +
-            "[myserver/src/main/resources/transport/defined/patch.csv] has patch version 8015001 as primary id")
+            "[myserver/src/main/resources/transport/definitions/named/patch.csv] has patch version 8015001 as primary id")
     }
 }

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java

@@ -17,8 +17,11 @@ import org.gradle.api.tasks.OutputFile;
 import org.gradle.api.tasks.TaskAction;
 
 import java.io.IOException;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 
 public abstract class GenerateTransportVersionManifestTask extends DefaultTask {
     @InputDirectory
@@ -32,15 +35,14 @@ public abstract class GenerateTransportVersionManifestTask extends DefaultTask {
         Path definitionsDir = getDefinitionsDirectory().get().getAsFile().toPath();
         Path manifestFile = getManifestFile().get().getAsFile().toPath();
         try (var writer = Files.newBufferedWriter(manifestFile)) {
-            try (var stream = Files.list(definitionsDir)) {
-                for (String filename : stream.map(p -> p.getFileName().toString()).toList()) {
-                    if (filename.equals(manifestFile.getFileName().toString())) {
-                        // don't list self
-                        continue;
-                    }
-                    writer.write(filename + "\n");
+            Files.walkFileTree(definitionsDir, new SimpleFileVisitor<>() {
+                @Override
+                public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException {
+                    String subPath = definitionsDir.relativize(path).toString().replace('\\', '/');
+                    writer.write(subPath + "\n");
+                    return FileVisitResult.CONTINUE;
                 }
-            }
+            });
         }
     }
 }

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java

@@ -62,7 +62,7 @@ public class TransportVersionResourcesPlugin implements Plugin<Project> {
                 t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt"));
             });
         project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> {
-            t.into("transport/defined", c -> c.from(generateManifestTask));
+            t.into("transport/definitions", c -> c.from(generateManifestTask));
         });
     }
 }

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java

@@ -20,7 +20,7 @@ import java.nio.file.Path;
 class TransportVersionUtils {
 
     static Path definitionFilePath(Directory resourcesDirectory, String name) {
-        return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve(name + ".csv");
+        return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve("named/" + name + ".csv");
     }
 
     static Path latestFilePath(Directory resourcesDirectory, String name) {
@@ -38,7 +38,7 @@ class TransportVersionUtils {
     }
 
     static Directory getDefinitionsDirectory(Directory resourcesDirectory) {
-        return resourcesDirectory.dir("defined");
+        return resourcesDirectory.dir("definitions");
     }
 
     static Directory getLatestDirectory(Directory resourcesDirectory) {

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java

@@ -45,7 +45,7 @@ public abstract class ValidateTransportVersionReferencesTask extends DefaultTask
         final Predicate<String> referenceChecker;
         if (getDefinitionsDirectory().isPresent()) {
             Path definitionsDir = getDefinitionsDirectory().getAsFile().get().toPath();
-            referenceChecker = (name) -> Files.exists(definitionsDir.resolve(name + ".csv"));
+            referenceChecker = (name) -> Files.exists(definitionsDir.resolve("named/" + name + ".csv"));
         } else {
             referenceChecker = (name) -> false;
         }

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java

@@ -93,7 +93,7 @@ public abstract class ValidateTransportVersionResourcesTask extends DefaultTask
     @TaskAction
     public void validateTransportVersions() throws IOException {
         Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath();
-        Path definitionsDir = resourcesDir.resolve("defined");
+        Path definitionsDir = resourcesDir.resolve("definitions");
         Path latestDir = resourcesDir.resolve("latest");
 
         // first check which resource files already exist in main
@@ -107,9 +107,11 @@ public abstract class ValidateTransportVersionResourcesTask extends DefaultTask
         // now load all definitions, do some validation and record them by various keys for later quick lookup
         // NOTE: this must run after loading referenced names and existing definitions
         // NOTE: this is sorted so that the order of cross validation is deterministic
-        try (var definitionsStream = Files.list(definitionsDir).sorted()) {
-            for (var definitionFile : definitionsStream.toList()) {
-                recordAndValidateDefinition(readDefinitionFile(definitionFile));
+        for (String subDir : List.of("initial", "named")) {
+            try (var definitionsStream = Files.list(definitionsDir.resolve(subDir)).sorted()) {
+                for (var definitionFile : definitionsStream.toList()) {
+                    recordAndValidateDefinition(readDefinitionFile(definitionFile));
+                }
             }
         }
 

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

@@ -24,7 +24,6 @@ import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -113,6 +112,7 @@ public record TransportVersion(String name, int id, TransportVersion nextPatchVe
         String component,
         String path,
         boolean nameInFile,
+        boolean isNamed,
         BufferedReader bufferedReader,
         Integer latest
     ) {
@@ -128,7 +128,14 @@ public record TransportVersion(String name, int id, TransportVersion nextPatchVe
             if (parts.length < (nameInFile ? 2 : 1)) {
                 throw new IllegalStateException("invalid transport version file format [" + toComponentPath(component, path) + "]");
             }
-            String name = nameInFile ? parts[0] : path.substring(path.lastIndexOf('/') + 1, path.length() - 4);
+            String name = null;
+            if (isNamed) {
+                if (nameInFile) {
+                    name = parts[0];
+                } else {
+                    name = path.substring(path.lastIndexOf('/') + 1, path.length() - 4);
+                }
+            }
             List<Integer> ids = new ArrayList<>();
             for (int i = nameInFile ? 1 : 0; i < parts.length; ++i) {
                 try {
@@ -156,7 +163,7 @@ public record TransportVersion(String name, int id, TransportVersion nextPatchVe
         }
     }
 
-    public static Map<String, TransportVersion> collectFromInputStreams(
+    public static List<TransportVersion> collectFromInputStreams(
         String component,
         Function<String, InputStream> nameToStream,
         String latestFileName
@@ -165,32 +172,32 @@ public record TransportVersion(String name, int id, TransportVersion nextPatchVe
             component,
             "/transport/latest/" + latestFileName,
             nameToStream,
-            (c, p, br) -> fromBufferedReader(c, p, true, br, Integer.MAX_VALUE)
+            (c, p, br) -> fromBufferedReader(c, p, true, false, br, Integer.MAX_VALUE)
         );
         if (latest != null) {
-            List<String> versionFilesNames = parseFromBufferedReader(
+            List<String> versionRelativePaths = parseFromBufferedReader(
                 component,
-                "/transport/defined/manifest.txt",
+                "/transport/definitions/manifest.txt",
                 nameToStream,
                 (c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList()
             );
-            if (versionFilesNames != null) {
-                Map<String, TransportVersion> transportVersions = new HashMap<>();
-                for (String versionFileName : versionFilesNames) {
+            if (versionRelativePaths != null) {
+                List<TransportVersion> transportVersions = new ArrayList<>();
+                for (String versionRelativePath : versionRelativePaths) {
                     TransportVersion transportVersion = parseFromBufferedReader(
                         component,
-                        "/transport/defined/" + versionFileName,
+                        "/transport/definitions/" + versionRelativePath,
                         nameToStream,
-                        (c, p, br) -> fromBufferedReader(c, p, false, br, latest.id())
+                        (c, p, br) -> fromBufferedReader(c, p, false, versionRelativePath.startsWith("named/"), br, latest.id())
                     );
                     if (transportVersion != null) {
-                        transportVersions.put(versionFileName.substring(0, versionFileName.length() - 4), transportVersion);
+                        transportVersions.add(transportVersion);
                     }
                 }
                 return transportVersions;
             }
         }
-        return Map.of();
+        return List.of();
     }
 
     private static String toComponentPath(String component, String path) {
@@ -419,12 +426,15 @@ public record TransportVersion(String name, int id, TransportVersion nextPatchVe
         static {
             // collect all the transport versions from server and es modules/plugins (defined in server)
             List<TransportVersion> allVersions = new ArrayList<>(TransportVersions.DEFINED_VERSIONS);
-            Map<String, TransportVersion> allVersionsByName = collectFromInputStreams(
+            List<TransportVersion> streamVersions = collectFromInputStreams(
                 "<server>",
                 TransportVersion.class::getResourceAsStream,
                 Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv"
             );
-            addTransportVersions(allVersionsByName.values(), allVersions).sort(TransportVersion::compareTo);
+            Map<String, TransportVersion> allVersionsByName = streamVersions.stream()
+                .filter(tv -> tv.name() != null)
+                .collect(Collectors.toMap(TransportVersion::name, v -> v));
+            addTransportVersions(streamVersions, allVersions).sort(TransportVersion::compareTo);
 
             // set version lookup by release before adding serverless versions
             // serverless versions should not affect release version

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

@@ -228,9 +228,9 @@ public class TransportVersionTests extends ESTestCase {
     public void testLatest() {
         TransportVersion latest = TransportVersion.parseFromBufferedReader(
             "<test>",
-            "/transport/defined/" + Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv",
+            "/transport/definitions/" + Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv",
             TransportVersion.class::getResourceAsStream,
-            (c, p, br) -> TransportVersion.fromBufferedReader(c, p, true, br, Integer.MAX_VALUE)
+            (c, p, br) -> TransportVersion.fromBufferedReader(c, p, true, false, br, Integer.MAX_VALUE)
         );
         // TODO: once placeholder is removed, test the latest known version can be found fromName
         // assertThat(latest, is(TransportVersion.fromName(latest.name())));
@@ -242,6 +242,7 @@ public class TransportVersionTests extends ESTestCase {
             "<test>",
             "testSupports0",
             false,
+            true,
             new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data0), StandardCharsets.UTF_8)),
             5000000
         );
@@ -254,6 +255,7 @@ public class TransportVersionTests extends ESTestCase {
             "<test>",
             "testSupports1",
             false,
+            true,
             new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data1), StandardCharsets.UTF_8)),
             5000000
         );
@@ -269,6 +271,7 @@ public class TransportVersionTests extends ESTestCase {
             "<test>",
             "testSupports2",
             false,
+            true,
             new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data2), StandardCharsets.UTF_8)),
             5000000
         );
@@ -296,6 +299,7 @@ public class TransportVersionTests extends ESTestCase {
             "<test>",
             "testSupports3",
             false,
+            true,
             new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data3), StandardCharsets.UTF_8)),
             5000000
         );
@@ -324,6 +328,7 @@ public class TransportVersionTests extends ESTestCase {
             "<test>",
             "testSupports3",
             false,
+            true,
             new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data4), StandardCharsets.UTF_8)),
             5000000
         );