From 76c8eca3961fc9f3f5b19476ff546f985a1b1f9b Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 13 Aug 2025 15:42:42 -0700 Subject: [PATCH 1/6] update transport version directory structure --- .gitignore | 2 +- ...portVersionManagementPluginFuncTest.groovy | 43 +++++++++++-------- .../GenerateTransportVersionManifestTask.java | 18 ++++---- ...lobalTransportVersionManagementPlugin.java | 2 +- .../transport/TransportVersionUtils.java | 4 +- ...lidateTransportVersionDefinitionsTask.java | 10 +++-- ...alidateTransportVersionReferencesTask.java | 2 +- .../org/elasticsearch/TransportVersion.java | 9 ++-- .../initial}/initial_elasticsearch_8_18_5.csv | 0 .../initial}/initial_elasticsearch_9_0_5.csv | 0 .../named}/esql_split_on_big_values.csv | 0 .../elasticsearch/TransportVersionTests.java | 2 +- 12 files changed, 52 insertions(+), 40 deletions(-) rename server/src/main/resources/transport/{defined => definitions/initial}/initial_elasticsearch_8_18_5.csv (100%) rename server/src/main/resources/transport/{defined => definitions/initial}/initial_elasticsearch_9_0_5.csv (100%) rename server/src/main/resources/transport/{defined => definitions/named}/esql_split_on_big_values.csv (100%) diff --git a/.gitignore b/.gitignore index cac5a799012e1..b7e999610c149 100644 --- a/.gitignore +++ b/.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 diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy index 0fcd2d0ae68c3..ec79ed81944dc 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy +++ b/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-management' apply plugin: 'elasticsearch.global-transport-version-management' """ - 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") } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java index 13164dd8f457b..5d4ac52c3ac05 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java @@ -17,8 +17,11 @@ 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 void generateTransportVersionManifest() throws IOException { 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(); + writer.write(subPath + "\n"); + return FileVisitResult.CONTINUE; } - } + }); } } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GlobalTransportVersionManagementPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GlobalTransportVersionManagementPlugin.java index 9f14c95f3591d..02edcc1513354 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GlobalTransportVersionManagementPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GlobalTransportVersionManagementPlugin.java @@ -62,7 +62,7 @@ public void apply(Project 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)); }); } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java index d8fe1ffed147b..0aa2b173d2466 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java @@ -20,7 +20,7 @@ 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 @@ static TransportVersionLatest readLatestFile(Path file) throws IOException { } static Directory getDefinitionsDirectory(Directory resourcesDirectory) { - return resourcesDirectory.dir("defined"); + return resourcesDirectory.dir("definitions"); } static Directory getLatestDirectory(Directory resourcesDirectory) { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java index 794c4645d6184..7ffccc3fcea44 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java @@ -93,7 +93,7 @@ public ValidateTransportVersionDefinitionsTask(ExecOperations execOperations) { @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 void validateTransportVersions() throws IOException { // 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)); + } } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java index 968146c6d6759..2a19900076ec7 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionReferencesTask.java @@ -45,7 +45,7 @@ public void validateTransportVersions() throws IOException { final Predicate 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; } diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index a90274496aca6..e6a4a1bdf0648 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -170,7 +170,7 @@ public static Map collectFromInputStreams( if (latest != null) { List versionFilesNames = parseFromBufferedReader( component, - "/transport/defined/manifest.txt", + "/transport/definitions/manifest.txt", nameToStream, (c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList() ); @@ -179,12 +179,15 @@ public static Map collectFromInputStreams( for (String versionFileName : versionFilesNames) { TransportVersion transportVersion = parseFromBufferedReader( component, - "/transport/defined/" + versionFileName, + "/transport/definitions/" + versionFileName, nameToStream, (c, p, br) -> fromBufferedReader(c, p, false, br, latest.id()) ); if (transportVersion != null) { - transportVersions.put(versionFileName.substring(0, versionFileName.length() - 4), transportVersion); + transportVersions.put( + versionFileName.substring(versionFileName.lastIndexOf("/") + 1, versionFileName.length() - 4), + transportVersion + ); } } return transportVersions; diff --git a/server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv b/server/src/main/resources/transport/definitions/initial/initial_elasticsearch_8_18_5.csv similarity index 100% rename from server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv rename to server/src/main/resources/transport/definitions/initial/initial_elasticsearch_8_18_5.csv diff --git a/server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv b/server/src/main/resources/transport/definitions/initial/initial_elasticsearch_9_0_5.csv similarity index 100% rename from server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv rename to server/src/main/resources/transport/definitions/initial/initial_elasticsearch_9_0_5.csv diff --git a/server/src/main/resources/transport/defined/esql_split_on_big_values.csv b/server/src/main/resources/transport/definitions/named/esql_split_on_big_values.csv similarity index 100% rename from server/src/main/resources/transport/defined/esql_split_on_big_values.csv rename to server/src/main/resources/transport/definitions/named/esql_split_on_big_values.csv diff --git a/server/src/test/java/org/elasticsearch/TransportVersionTests.java b/server/src/test/java/org/elasticsearch/TransportVersionTests.java index 8798b0bc89f13..78089c0d7bdf5 100644 --- a/server/src/test/java/org/elasticsearch/TransportVersionTests.java +++ b/server/src/test/java/org/elasticsearch/TransportVersionTests.java @@ -228,7 +228,7 @@ public void testDuplicateConstants() { public void testLatest() { TransportVersion latest = TransportVersion.parseFromBufferedReader( "", - "/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) ); From ec3147170508eea28942ef3f8af77a83ada4ecf7 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 13 Aug 2025 16:26:16 -0700 Subject: [PATCH 2/6] switch map to list --- ...lidateTransportVersionDefinitionsTask.java | 2 +- .../org/elasticsearch/TransportVersion.java | 41 +++++++++++-------- .../elasticsearch/TransportVersionTests.java | 7 +++- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java index 7ffccc3fcea44..756daae7dba5d 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java @@ -107,7 +107,7 @@ public void validateTransportVersions() throws IOException { // 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 - for (String subDir: List.of("initial", "named")) { + for (String subDir : List.of("initial", "named")) { try (var definitionsStream = Files.list(definitionsDir.resolve(subDir)).sorted()) { for (var definitionFile : definitionsStream.toList()) { recordAndValidateDefinition(readDefinitionFile(definitionFile)); diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index e6a4a1bdf0648..6791b59f88c4f 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -24,7 +24,6 @@ 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 static TransportVersion fromBufferedReader( String component, String path, boolean nameInFile, + boolean ignoreName, BufferedReader bufferedReader, Integer latest ) { @@ -128,7 +128,14 @@ public static TransportVersion fromBufferedReader( 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 (ignoreName == false) { + if (nameInFile) { + name = path.substring(path.lastIndexOf('/') + 1, path.length() - 4); + } else { + name = parts[0]; + } + } List ids = new ArrayList<>(); for (int i = nameInFile ? 1 : 0; i < parts.length; ++i) { try { @@ -156,7 +163,7 @@ public static TransportVersion fromBufferedReader( } } - public static Map collectFromInputStreams( + public static List collectFromInputStreams( String component, Function nameToStream, String latestFileName @@ -165,35 +172,32 @@ public static Map collectFromInputStreams( 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 versionFilesNames = parseFromBufferedReader( + List versionRelativePaths = parseFromBufferedReader( component, "/transport/definitions/manifest.txt", nameToStream, (c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList() ); - if (versionFilesNames != null) { - Map transportVersions = new HashMap<>(); - for (String versionFileName : versionFilesNames) { + if (versionRelativePaths != null) { + List transportVersions = new ArrayList<>(); + for (String versionRelativePath : versionRelativePaths) { TransportVersion transportVersion = parseFromBufferedReader( component, - "/transport/definitions/" + versionFileName, + "/transport/definitions/" + versionRelativePath, nameToStream, - (c, p, br) -> fromBufferedReader(c, p, false, br, latest.id()) + (c, p, br) -> fromBufferedReader(c, p, false, versionRelativePath.startsWith("initial/"), br, latest.id()) ); if (transportVersion != null) { - transportVersions.put( - versionFileName.substring(versionFileName.lastIndexOf("/") + 1, versionFileName.length() - 4), - transportVersion - ); + transportVersions.add(transportVersion); } } return transportVersions; } } - return Map.of(); + return List.of(); } private static String toComponentPath(String component, String path) { @@ -422,12 +426,15 @@ private static class VersionsHolder { static { // collect all the transport versions from server and es modules/plugins (defined in server) List allVersions = new ArrayList<>(TransportVersions.DEFINED_VERSIONS); - Map allVersionsByName = collectFromInputStreams( + List streamVersions = collectFromInputStreams( "", TransportVersion.class::getResourceAsStream, Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv" ); - addTransportVersions(allVersionsByName.values(), allVersions).sort(TransportVersion::compareTo); + Map 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 diff --git a/server/src/test/java/org/elasticsearch/TransportVersionTests.java b/server/src/test/java/org/elasticsearch/TransportVersionTests.java index 78089c0d7bdf5..748c4f57dfeb5 100644 --- a/server/src/test/java/org/elasticsearch/TransportVersionTests.java +++ b/server/src/test/java/org/elasticsearch/TransportVersionTests.java @@ -230,7 +230,7 @@ public void testLatest() { "", "/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 void testSupports() { "", "testSupports0", false, + false, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data0), StandardCharsets.UTF_8)), 5000000 ); @@ -254,6 +255,7 @@ public void testSupports() { "", "testSupports1", false, + false, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data1), StandardCharsets.UTF_8)), 5000000 ); @@ -269,6 +271,7 @@ public void testSupports() { "", "testSupports2", false, + false, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data2), StandardCharsets.UTF_8)), 5000000 ); @@ -296,6 +299,7 @@ public void testSupports() { "", "testSupports3", false, + false, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data3), StandardCharsets.UTF_8)), 5000000 ); @@ -324,6 +328,7 @@ public void testSupports() { "", "testSupports3", false, + false, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data4), StandardCharsets.UTF_8)), 5000000 ); From 698223095661279774a53ee57c09fccc2a3c073d Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 13 Aug 2025 16:45:47 -0700 Subject: [PATCH 3/6] fix name --- server/src/main/java/org/elasticsearch/TransportVersion.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index 6791b59f88c4f..47fa3cd076db4 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -131,9 +131,9 @@ public static TransportVersion fromBufferedReader( String name = null; if (ignoreName == false) { if (nameInFile) { - name = path.substring(path.lastIndexOf('/') + 1, path.length() - 4); - } else { name = parts[0]; + } else { + name = path.substring(path.lastIndexOf('/') + 1, path.length() - 4); } } List ids = new ArrayList<>(); From ed7a2bbb1e7d6d54e806d1391afd271347c568a6 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Thu, 14 Aug 2025 09:39:41 -0700 Subject: [PATCH 4/6] add additional info to fromName exception --- .../org/elasticsearch/TransportVersion.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index 47fa3cd076db4..dfa87b15fe788 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -235,7 +235,23 @@ public static TransportVersion fromId(int id) { public static TransportVersion fromName(String name) { TransportVersion known = VersionsHolder.ALL_VERSIONS_BY_NAME.get(name); if (known == null) { - throw new IllegalStateException("unknown transport version [" + name + "]"); + List versionRelativePaths = parseFromBufferedReader( + "", + "/transport/definitions/manifest.txt", + TransportVersion.class::getResourceAsStream, + (c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList() + ); + throw new IllegalStateException( + "unknown transport version [" + + name + + "];\n" + + "known names [" + + VersionsHolder.ALL_VERSIONS_BY_NAME + + "]\n" + + "version relative paths [" + + versionRelativePaths + + "]\n" + ); } return known; } From 33c9aae92897c67183d8acb72a3f50d97a88c05d Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Thu, 14 Aug 2025 10:13:02 -0700 Subject: [PATCH 5/6] fix windows slash --- .../GenerateTransportVersionManifestTask.java | 2 +- .../java/org/elasticsearch/TransportVersion.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java index 5d4ac52c3ac05..cb39a08a6aa44 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java @@ -38,7 +38,7 @@ public void generateTransportVersionManifest() throws IOException { Files.walkFileTree(definitionsDir, new SimpleFileVisitor<>() { @Override public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException { - String subPath = definitionsDir.relativize(path).toString(); + String subPath = definitionsDir.relativize(path).toString().replace('\\', '/'); writer.write(subPath + "\n"); return FileVisitResult.CONTINUE; } diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index dfa87b15fe788..f781f73a69309 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -242,15 +242,15 @@ public static TransportVersion fromName(String name) { (c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList() ); throw new IllegalStateException( - "unknown transport version [" + "\nunknown transport version [" + name - + "];\n" - + "known names [" + + "];" + + "\nknown names [" + VersionsHolder.ALL_VERSIONS_BY_NAME - + "]\n" - + "version relative paths [" + + "]" + + "\nrelative paths" + versionRelativePaths - + "]\n" + + "\n" ); } return known; From f3581d68e054d5d9b43b04603849a65e5070017d Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Thu, 14 Aug 2025 11:29:51 -0700 Subject: [PATCH 6/6] response to pr comments --- .../org/elasticsearch/TransportVersion.java | 24 ++++--------------- .../elasticsearch/TransportVersionTests.java | 10 ++++---- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index f781f73a69309..3cfebd440d2e3 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -112,7 +112,7 @@ public static TransportVersion fromBufferedReader( String component, String path, boolean nameInFile, - boolean ignoreName, + boolean isNamed, BufferedReader bufferedReader, Integer latest ) { @@ -129,7 +129,7 @@ public static TransportVersion fromBufferedReader( throw new IllegalStateException("invalid transport version file format [" + toComponentPath(component, path) + "]"); } String name = null; - if (ignoreName == false) { + if (isNamed) { if (nameInFile) { name = parts[0]; } else { @@ -188,7 +188,7 @@ public static List collectFromInputStreams( component, "/transport/definitions/" + versionRelativePath, nameToStream, - (c, p, br) -> fromBufferedReader(c, p, false, versionRelativePath.startsWith("initial/"), br, latest.id()) + (c, p, br) -> fromBufferedReader(c, p, false, versionRelativePath.startsWith("named/"), br, latest.id()) ); if (transportVersion != null) { transportVersions.add(transportVersion); @@ -235,23 +235,7 @@ public static TransportVersion fromId(int id) { public static TransportVersion fromName(String name) { TransportVersion known = VersionsHolder.ALL_VERSIONS_BY_NAME.get(name); if (known == null) { - List versionRelativePaths = parseFromBufferedReader( - "", - "/transport/definitions/manifest.txt", - TransportVersion.class::getResourceAsStream, - (c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList() - ); - throw new IllegalStateException( - "\nunknown transport version [" - + name - + "];" - + "\nknown names [" - + VersionsHolder.ALL_VERSIONS_BY_NAME - + "]" - + "\nrelative paths" - + versionRelativePaths - + "\n" - ); + throw new IllegalStateException("unknown transport version [" + name + "]"); } return known; } diff --git a/server/src/test/java/org/elasticsearch/TransportVersionTests.java b/server/src/test/java/org/elasticsearch/TransportVersionTests.java index 748c4f57dfeb5..9ee08fc74a702 100644 --- a/server/src/test/java/org/elasticsearch/TransportVersionTests.java +++ b/server/src/test/java/org/elasticsearch/TransportVersionTests.java @@ -242,7 +242,7 @@ public void testSupports() { "", "testSupports0", false, - false, + true, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data0), StandardCharsets.UTF_8)), 5000000 ); @@ -255,7 +255,7 @@ public void testSupports() { "", "testSupports1", false, - false, + true, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data1), StandardCharsets.UTF_8)), 5000000 ); @@ -271,7 +271,7 @@ public void testSupports() { "", "testSupports2", false, - false, + true, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data2), StandardCharsets.UTF_8)), 5000000 ); @@ -299,7 +299,7 @@ public void testSupports() { "", "testSupports3", false, - false, + true, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data3), StandardCharsets.UTF_8)), 5000000 ); @@ -328,7 +328,7 @@ public void testSupports() { "", "testSupports3", false, - false, + true, new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data4), StandardCharsets.UTF_8)), 5000000 );