From a387fb27966123ec66ff34598a6acfea6882039f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 20 Aug 2025 11:34:48 -0700 Subject: [PATCH] Fix transport version validation task name (#133186) The task class validates resources, but the task name still had the old "definitions" suffix. --- .../AbstractTransportVersionFuncTest.groovy | 19 ++-- .../TransportVersionValidationFuncTest.groovy | 92 +++++++++---------- .../TransportVersionResourcesPlugin.java | 15 +-- 3 files changed, 57 insertions(+), 69 deletions(-) diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy index 257da1b64fd8e..901a395fd5649 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy @@ -41,11 +41,11 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { javaResource("myserver", "transport/definitions/unreferenced/" + name + ".csv", id) } - def definedAndUsedTransportVersion(String name, String ids) { - return definedAndUsedTransportVersion(name, ids, "Test${name.capitalize()}") + def namedAndReferencedTransportVersion(String name, String ids) { + return namedAndReferencedTransportVersion(name, ids, "Test${name.capitalize()}") } - def definedAndUsedTransportVersion(String name, String ids, String classname) { + def namedAndReferencedTransportVersion(String name, String ids, String classname) { javaSource("myserver", "org.elasticsearch", classname, "", """ static final TransportVersion usage = TransportVersion.fromName("${name}"); """) @@ -60,17 +60,17 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { return gradleRunner(":${project}:validateTransportVersionReferences").buildAndFail() } - def validateDefinitionsFails() { - return gradleRunner(":myserver:validateTransportVersionDefinitions").buildAndFail() + def validateResourcesFails() { + return gradleRunner(":myserver:validateTransportVersionResources").buildAndFail() } - def assertReferencesFailure(BuildResult result, String project, String expectedOutput) { + def assertValidateReferencesFailure(BuildResult result, String project, String expectedOutput) { result.task(":${project}:validateTransportVersionReferences").outcome == TaskOutcome.FAILED assertOutputContains(result.output, expectedOutput) } - def assertDefinitionsFailure(BuildResult result, String expectedOutput) { - result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.FAILED + def assertValidateResourcesFailure(BuildResult result, String expectedOutput) { + result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.FAILED assertOutputContains(result.output, expectedOutput) } @@ -81,9 +81,6 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest { include ':myserver' include ':myplugin' """ - file("gradle.properties") << """ - org.elasticsearch.transport.definitionsProject=:myserver - """ file("myserver/build.gradle") << """ apply plugin: 'java-library' diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy index 52fab5a4f1625..031d98ea27aed 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy @@ -16,9 +16,9 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes def "test setup works"() { when: - def result = gradleRunner("validateTransportVersionDefinitions", "validateTransportVersionReferences").build() + def result = gradleRunner("validateTransportVersionResources", "validateTransportVersionReferences").build() then: - result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS + result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS result.task(":myserver:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS result.task(":myplugin:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS } @@ -32,7 +32,7 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes when: def result = validateReferencesFails("myplugin") then: - assertReferencesFailure(result, "myplugin", "TransportVersion.fromName(\"dne\") was used at " + + assertValidateReferencesFailure(result, "myplugin", "TransportVersion.fromName(\"dne\") was used at " + "org.elasticsearch.plugin.MyPlugin line 6, but lacks a transport version definition.") } @@ -40,19 +40,19 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes given: namedTransportVersion("not_used", "1000000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/not_used.csv] is not referenced") } def "names must be lowercase alphanum or underscore"() { given: - definedAndUsedTransportVersion("${name}", "8100000", "TestNames") + namedAndReferencedTransportVersion("${name}", "8100000", "TestNames") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/${name}.csv] does not have a valid name, " + "must be lowercase alphanumeric and underscore") @@ -62,42 +62,42 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes def "definitions contain at least one id"() { given: - definedAndUsedTransportVersion("empty", "") + namedAndReferencedTransportVersion("empty", "") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/empty.csv] does not contain any ids") } def "definitions have ids in descending order"() { given: - definedAndUsedTransportVersion("out_of_order", "8100000,8200000") + namedAndReferencedTransportVersion("out_of_order", "8100000,8200000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/out_of_order.csv] does not have ordered ids") } def "definition ids are unique"() { given: - definedAndUsedTransportVersion("duplicate", "8123000") + namedAndReferencedTransportVersion("duplicate", "8123000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[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"() { given: - definedAndUsedTransportVersion("patched", "8200000,8100000") + namedAndReferencedTransportVersion("patched", "8200000,8100000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/patched.csv] contains bwc id [8100000] with a patch part of 0") } @@ -105,9 +105,9 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes given: namedTransportVersion("existing_92", "8500000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/existing_92.csv] has modified primary id from 8123000 to 8500000") } @@ -115,9 +115,9 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes given: namedTransportVersion("existing_92", "8123000,8012002") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/existing_92.csv] modifies existing patch id from 8012001 to 8012002") } @@ -125,9 +125,9 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes given: latestTransportVersion("9.2", "dne", "8123000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Latest transport version file " + + assertValidateResourcesFailure(result, "Latest transport version file " + "[myserver/src/main/resources/transport/latest/9.2.csv] contains transport version name [dne] which is not defined") } @@ -135,9 +135,9 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes given: latestTransportVersion("9.2", "existing_92", "8124000") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Latest transport version file " + + assertValidateResourcesFailure(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/definitions/named/existing_92.csv]") } @@ -145,47 +145,47 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes def "latest files have latest id within base"() { given: latestTransportVersion("9.0", "seemingly_latest", "8110001") - definedAndUsedTransportVersion("original", "8110000") - definedAndUsedTransportVersion("seemingly_latest", "8111000,8110001") - definedAndUsedTransportVersion("actual_latest", "8112000,8110002") + namedAndReferencedTransportVersion("original", "8110000") + namedAndReferencedTransportVersion("seemingly_latest", "8111000,8110001") + namedAndReferencedTransportVersion("actual_latest", "8112000,8110002") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Latest transport version file " + + assertValidateResourcesFailure(result, "Latest transport version file " + "[myserver/src/main/resources/transport/latest/9.0.csv] has id 8110001 from [seemingly_latest] with base 8110000 " + "but another id 8110002 from [actual_latest] is later for that base") } def "latest files cannot change base id"() { given: - definedAndUsedTransportVersion("original", "8013000") - definedAndUsedTransportVersion("patch", "8015000,8013001") + namedAndReferencedTransportVersion("original", "8013000") + namedAndReferencedTransportVersion("patch", "8015000,8013001") latestTransportVersion("9.1", "patch", "8013001") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Latest transport version file " + + assertValidateResourcesFailure(result, "Latest transport version file " + "[myserver/src/main/resources/transport/latest/9.1.csv] modifies base id from 8012000 to 8013000") } def "ids must be dense"() { given: - definedAndUsedTransportVersion("original", "8013000") - definedAndUsedTransportVersion("patch1", "8015000,8013002") + namedAndReferencedTransportVersion("original", "8013000") + namedAndReferencedTransportVersion("patch1", "8015000,8013002") latestTransportVersion("9.0", "patch1", "8013002") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version base id 8013000 is missing patch ids between 8013000 and 8013002") + assertValidateResourcesFailure(result, "Transport version base id 8013000 is missing patch ids between 8013000 and 8013002") } def "primary id must not be patch version"() { given: - definedAndUsedTransportVersion("patch", "8015001") + namedAndReferencedTransportVersion("patch", "8015001") when: - def result = validateDefinitionsFails() + def result = validateResourcesFails() then: - assertDefinitionsFailure(result, "Transport version definition file " + + assertValidateResourcesFailure(result, "Transport version definition file " + "[myserver/src/main/resources/transport/definitions/named/patch.csv] has patch version 8015001 as primary id") } @@ -194,8 +194,8 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes file("myserver/src/main/resources/transport/unreferenced/initial_9_0_0.csv").delete() file("myserver/src/main/resources/transport/unreferenced").deleteDir() when: - def result = gradleRunner(":myserver:validateTransportVersionDefinitions").build() + def result = gradleRunner(":myserver:validateTransportVersionResources").build() then: - result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS + result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java index 699cd4294ce9e..1edf95c080762 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java @@ -13,16 +13,12 @@ import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.dsl.DependencyHandler; -import org.gradle.api.file.Directory; import org.gradle.api.plugins.JavaPlugin; import org.gradle.api.tasks.Copy; import org.gradle.language.base.plugins.LifecycleBasePlugin; import java.util.Map; -import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinitionsDirectory; -import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getResourcesDirectory; - public class TransportVersionResourcesPlugin implements Plugin { @Override @@ -43,13 +39,9 @@ public void apply(Project project) { } var validateTask = project.getTasks() - .register("validateTransportVersionDefinitions", ValidateTransportVersionResourcesTask.class, t -> { + .register("validateTransportVersionResources", ValidateTransportVersionResourcesTask.class, t -> { t.setGroup("Transport Versions"); - t.setDescription("Validates that all defined TransportVersion constants are used in at least one project"); - Directory resourcesDir = getResourcesDirectory(project); - if (resourcesDir.getAsFile().exists()) { - t.getResourcesDirectory().set(resourcesDir); - } + t.setDescription("Validates that all transport version resources are internally consistent with each other"); t.getReferencesFiles().setFrom(tvReferencesConfig); }); project.getTasks().named(LifecycleBasePlugin.CHECK_TASK_NAME).configure(t -> t.dependsOn(validateTask)); @@ -57,8 +49,7 @@ public void apply(Project project) { var generateManifestTask = project.getTasks() .register("generateTransportVersionManifest", GenerateTransportVersionManifestTask.class, t -> { t.setGroup("Transport Versions"); - t.setDescription("Generate a manifest resource for all the known transport version definitions"); - t.getDefinitionsDirectory().set(getDefinitionsDirectory(getResourcesDirectory(project))); + t.setDescription("Generate a manifest resource for all transport version definitions"); t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt")); }); project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> {