address feedback

Author: rjernst

…t/UpdateTransportVersionsFuncTest.groovy …pdateTransportVersionsCSVFuncTest.groovybuild-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsFuncTest.groovy renamed to build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsCSVFuncTest.groovy build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsFuncTest.groovy renamed to build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsCSVFuncTest.groovy

@@ -12,20 +12,20 @@ package org.elasticsearch.gradle.internal.transport
 import org.gradle.testkit.runner.BuildResult
 import org.gradle.testkit.runner.TaskOutcome
 
-class UpdateTransportVersionsFuncTest extends AbstractTransportVersionFuncTest {
+class UpdateTransportVersionsCSVFuncTest extends AbstractTransportVersionFuncTest {
     def runUpdateTask(String... additionalArgs) {
         List<String> args = new ArrayList<>()
-        args.add(":myserver:updateTransportVersions")
+        args.add(":myserver:updateTransportVersionsCSV")
         args.addAll(additionalArgs);
         return gradleRunner(args.toArray())
     }
 
     void assertUpdateSuccess(BuildResult result) {
-        assert result.task(":myserver:updateTransportVersions").outcome == TaskOutcome.SUCCESS
+        assert result.task(":myserver:updateTransportVersionsCSV").outcome == TaskOutcome.SUCCESS
     }
 
     void assertUpdateFailure(BuildResult result, String expectedOutput) {
-        assert result.task(":myserver:updateTransportVersions").outcome == TaskOutcome.FAILED
+        assert result.task(":myserver:updateTransportVersionsCSV").outcome == TaskOutcome.FAILED
         assertOutputContains(result.output, expectedOutput)
     }
 
@@ -87,12 +87,28 @@ class UpdateTransportVersionsFuncTest extends AbstractTransportVersionFuncTest {
         then:
         assertUpdateSuccess(result1)
         assertUpdateSuccess(result2)
-        assertOutputContains(result2.output, "Version 9.2.0 already exists in TransportVersions.csv, skipping")
+        assertOutputContains(result2.output, "Version 9.2.0 already exists in TransportVersions.csv with correct transport version ID, skipping")
         // Should only have one entry for 9.2.0
         assertTransportVersionsCsv("""
             9.0.0,8000000
             9.1.0,8012001
             9.2.0,8123000
         """)
     }
+
+    def "fails when existing version has wrong transport version ID"() {
+        given:
+        // Manually add an entry with wrong transport version ID
+        javaResource("myserver", "org/elasticsearch/TransportVersions.csv", """
+            9.0.0,8000000
+            9.1.0,8012001
+            9.2.0,9999999
+        """)
+
+        when:
+        def result = runUpdateTask("--stack-version", "9.2.0").buildAndFail()
+
+        then:
+        assertUpdateFailure(result, "Version 9.2.0 already exists in TransportVersions.csv with transport version ID 9999999, but expected 8123000")
+    }
 }

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

@@ -125,12 +125,13 @@ public void apply(Project project) {
             });
         validateTask.configure(t -> t.mustRunAfter(generateInitialTask));
 
-        var updateTransportVersionsTask = project.getTasks().register("updateTransportVersions", UpdateTransportVersionsTask.class, t -> {
-            t.setGroup(taskGroup);
-            t.setDescription("Updates TransportVersions.csv with a new stack version entry");
-            t.getTransportVersionsFile()
-                .set(project.getLayout().getProjectDirectory().file("src/main/resources/org/elasticsearch/TransportVersions.csv"));
-        });
+        var updateTransportVersionsTask = project.getTasks()
+            .register("updateTransportVersionsCSV", UpdateTransportVersionsCSVTask.class, t -> {
+                t.setGroup(taskGroup);
+                t.setDescription("Updates TransportVersions.csv with a new stack version entry");
+                t.getTransportVersionsFile()
+                    .set(project.getLayout().getProjectDirectory().file("src/main/resources/org/elasticsearch/TransportVersions.csv"));
+            });
         validateTask.configure(t -> t.mustRunAfter(updateTransportVersionsTask));
     }
 

…ansport/UpdateTransportVersionsTask.java …port/UpdateTransportVersionsCSVTask.javabuild-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsTask.java renamed to build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsCSVTask.java build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsTask.java renamed to build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/UpdateTransportVersionsCSVTask.java

@@ -23,7 +23,7 @@
 import java.nio.file.Files;
 import java.nio.file.StandardOpenOption;
 
-public abstract class UpdateTransportVersionsTask extends DefaultTask {
+public abstract class UpdateTransportVersionsCSVTask extends DefaultTask {
 
     @ServiceReference("transportVersionResources")
     abstract Property<TransportVersionResourcesService> getResourceService();
@@ -38,13 +38,6 @@ public abstract class UpdateTransportVersionsTask extends DefaultTask {
     @TaskAction
     public void run() throws IOException {
         Version stackVersion = Version.fromString(getStackVersion().get());
-
-        // Check if this version is already in the CSV file (idempotency check)
-        if (isVersionAlreadyRecorded(stackVersion)) {
-            getLogger().lifecycle("Version {} already exists in TransportVersions.csv, skipping", stackVersion);
-            return;
-        }
-
         String upperBoundName = getUpperBoundName(stackVersion);
         TransportVersionResourcesService resources = getResourceService().get();
         TransportVersionUpperBound upperBound = resources.getUpperBoundFromGitBase(upperBoundName);
@@ -53,15 +46,48 @@ public void run() throws IOException {
             throw new RuntimeException("Missing upper bound " + upperBoundName + " for stack version " + stackVersion);
         }
 
-        int transportVersionId = upperBound.definitionId().complete();
-        addTransportVersionRecord(stackVersion, transportVersionId);
+        int expectedTransportVersionId = upperBound.definitionId().complete();
+
+        // Check if this version is already in the CSV file (idempotency check)
+        Integer existingTransportVersionId = getExistingTransportVersionId(stackVersion);
+        if (existingTransportVersionId != null) {
+            if (existingTransportVersionId != expectedTransportVersionId) {
+                throw new RuntimeException(
+                    "Version "
+                        + stackVersion
+                        + " already exists in TransportVersions.csv with transport version ID "
+                        + existingTransportVersionId
+                        + ", but expected "
+                        + expectedTransportVersionId
+                );
+            }
+            getLogger().lifecycle(
+                "Version {} already exists in TransportVersions.csv with correct transport version ID, skipping",
+                stackVersion
+            );
+            return;
+        }
+
+        addTransportVersionRecord(stackVersion, expectedTransportVersionId);
     }
 
-    private boolean isVersionAlreadyRecorded(Version stackVersion) throws IOException {
+    private Integer getExistingTransportVersionId(Version stackVersion) throws IOException {
         var csvFile = getTransportVersionsFile().getAsFile().get().toPath();
-
         String versionPrefix = stackVersion.toString() + ",";
-        return Files.readAllLines(csvFile).stream().anyMatch(line -> line.trim().startsWith(versionPrefix));
+
+        return Files.readAllLines(csvFile)
+            .stream()
+            .map(String::trim)
+            .filter(line -> line.startsWith(versionPrefix))
+            .findFirst()
+            .map(line -> {
+                String[] parts = line.split(",");
+                if (parts.length >= 2) {
+                    return Integer.parseInt(parts[1]);
+                }
+                return null;
+            })
+            .orElse(null);
     }
 
     private void addTransportVersionRecord(Version stackVersion, int transportVersionId) throws IOException {