From e60b39181a066450e0b7435510fa2a2e7f24f55b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 3 Aug 2025 07:07:31 -0700 Subject: [PATCH 01/14] Add more transport version files validation This commit adds additional validation cases to the new transport version resource files. It also adds gradle tests for the validation cases. --- ...portVersionManagementPluginFuncTest.groovy | 290 ++++++++++++++++++ ...CollectTransportVersionReferencesTask.java | 6 +- ...lobalTransportVersionManagementPlugin.java | 6 +- .../transport/TransportVersionUtils.java | 100 ++++-- ...lidateTransportVersionDefinitionsTask.java | 255 ++++++++++++++- 5 files changed, 621 insertions(+), 36 deletions(-) create mode 100644 build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy 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 new file mode 100644 index 0000000000000..cacf799263fbb --- /dev/null +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy @@ -0,0 +1,290 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.gradle.internal.transport + + +import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest +import org.gradle.testkit.runner.BuildResult +import org.gradle.testkit.runner.TaskOutcome + +class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest { + + // collection task + // + + def javaResource(String project, String path, String content) { + file("${project}/src/main/resources/${path}").withWriter { writer -> + writer << content + } + } + + def javaSource(String project, String packageName, String className, String imports, String content) { + String packageSlashes = packageName.replace('.', '/') + file("${project}/src/main/java/${packageSlashes}/${className}.java").withWriter { writer -> + writer << """ + package ${packageName}; + ${imports} + public class ${className} { + ${content} + } + """ + } + } + + def definedTransportVersion(String name, String ids) { + javaResource("myserver", "transport/defined/" + name + ".csv", ids) + } + + def definedAndUsedTransportVersion(String name, String ids) { + javaSource("myserver", "org.elasticsearch", "Test${name.capitalize()}", "", """ + static final TransportVersion usage = TransportVersion.fromName("${name}"); + """) + definedTransportVersion(name, ids) + } + + def latestTransportVersion(String branch, String name, String id) { + javaResource("myserver", "transport/latest/" + branch + ".csv","${name},${id}") + } + + def validateReferencesFails(String project) { + return gradleRunner(":${project}:validateTransportVersionReferences").buildAndFail() + } + + def validateDefinitionsFails() { + return gradleRunner(":myserver:validateTransportVersionDefinitions").buildAndFail() + } + + def assertReferencesFailure(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 + assertOutputContains(result.output, expectedOutput) + } + + def setup() { + configurationCacheCompatible = false + internalBuild() + settingsFile << """ + include ':myserver' + include ':myplugin' + """ + file("gradle.properties") << """ + org.elasticsearch.transport.definitionsProject=:myserver + """ + + file("myserver/build.gradle") << """ + apply plugin: 'java-library' + apply plugin: 'elasticsearch.transport-version-management' + apply plugin: 'elasticsearch.global-transport-version-management' + """ + definedTransportVersion("existing_91", "8012000") + definedTransportVersion("existing_92", "8123000,8012001") + 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 + javaSource("myserver", "org.elasticsearch", "TransportVersion", "", """ + public static TransportVersion fromName(String name) { + return null; + } + """) + javaSource("myserver", "org.elasticsearch", "Dummy", "", """ + static final TransportVersion existing91 = TransportVersion.fromName("existing_91"); + static final TransportVersion existing92 = TransportVersion.fromName("existing_92"); + """) + + file("myplugin/build.gradle") << """ + apply plugin: 'java-library' + apply plugin: 'elasticsearch.transport-version-management' + + dependencies { + implementation project(":myserver") + } + """ + + setupLocalGitRepo() + execute("git checkout -b main") + execute("git checkout -b test") + } + + def "test setup works"() { + when: + def result = gradleRunner("validateTransportVersionDefinitions", "validateTransportVersionReferences").build() + then: + result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS + result.task(":myserver:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS + result.task(":myplugin:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS + } + + def "definitions must be referenced"() { + given: + javaSource("myplugin", "org.elasticsearch.plugin", "MyPlugin", + "import org.elasticsearch.TransportVersion;", """ + static final TransportVersion dne = TransportVersion.fromName("dne"); + """) + when: + def result = validateReferencesFails("myplugin") + then: + assertReferencesFailure(result, "myplugin", "TransportVersion.fromName(\"dne\") was used at " + + "org.elasticsearch.plugin.MyPlugin line 6, but lacks a transport version definition.") + } + + def "references must be defined"() { + given: + definedTransportVersion("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") + } + + def "names must be lowercase"() { + given: + definedAndUsedTransportVersion("CapitalTV", "8100000") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(result, "Transport version definition file " + + "[myserver/src/main/resources/transport/defined/CapitalTV.csv] does not have a valid name, " + + "must be lowercase alphanumeric and underscore") + } + + def "definitions contain at least one id"() { + given: + definedAndUsedTransportVersion("empty", "") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(result, "Transport version definition file " + + "[myserver/src/main/resources/transport/defined/empty.csv] does not contain any ids") + } + + def "definitions have ids in descending order"() { + given: + definedAndUsedTransportVersion("out_of_order", "8100000,8200000") + when: + 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") + } + + def "definition ids are unique"() { + given: + definedAndUsedTransportVersion("duplicate", "8123000") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(result, "Transport version definition file " + + "[myserver/src/main/resources/transport/defined/duplicate.csv] contains id already defined in " + + "[myserver/src/main/resources/transport/defined/existing_92.csv]") + } + + def "definitions have bwc ids with non-zero patch part"() { + given: + definedAndUsedTransportVersion("patched", "8200000,8100000") + when: + 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") + } + + def "definitions have primary ids which cannot change"() { + given: + definedTransportVersion("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") + } + + def "cannot change committed ids to a branch"() { + given: + definedTransportVersion("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") + } + + def "latest files must reference defined name"() { + given: + latestTransportVersion("9.2", "dne", "8123000") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(result, "Latest transport version file " + + "[myserver/src/main/resources/transport/latest/9.2.csv] contains transport version name [dne] which is not defined") + } + + def "latest files id must exist in definition"() { + given: + latestTransportVersion("9.2", "existing_92", "8124000") + when: + def result = validateDefinitionsFails() + 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]") + } + + 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") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(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") + latestTransportVersion("9.1", "patch", "8013001") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(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") + latestTransportVersion("9.0", "patch1", "8013002") + when: + def result = validateDefinitionsFails() + then: + assertDefinitionsFailure(result, "Transport version base version 8013000 is missing patch version 8013001") + } + + def "primary id must not be patch version"() { + given: + definedAndUsedTransportVersion("patch", "8015001") + when: + 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") + } +} diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java index 76d0d48f0db1f..5d6382d5c5b1a 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java @@ -74,14 +74,14 @@ public void checkTransportVersion() throws IOException { Files.writeString(outputFile, String.join("\n", results.stream().map(Object::toString).sorted().toList())); } - private void addNamesFromClassesDirectory(Set results, Path file) throws IOException { - Files.walkFileTree(file, new SimpleFileVisitor<>() { + private void addNamesFromClassesDirectory(Set results, Path basePath) throws IOException { + Files.walkFileTree(basePath, new SimpleFileVisitor<>() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { String filename = file.getFileName().toString(); if (filename.endsWith(CLASS_EXTENSION) && filename.endsWith(MODULE_INFO) == false) { try (var inputStream = Files.newInputStream(file)) { - addNamesFromClass(results, inputStream, classname(file.toString())); + addNamesFromClass(results, inputStream, classname(basePath.relativize(file).toString())); } } 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 c4e4c7d033e79..43192bb8ac3d0 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 @@ -43,9 +43,9 @@ public void apply(Project project) { .register("validateTransportVersionDefinitions", ValidateTransportVersionDefinitionsTask.class, t -> { t.setGroup("Transport Versions"); t.setDescription("Validates that all defined TransportVersion constants are used in at least one project"); - Directory definitionsDir = TransportVersionUtils.getDefinitionsDirectory(project); - if (definitionsDir.getAsFile().exists()) { - t.getDefinitionsDirectory().set(definitionsDir); + Directory resourcesDir = TransportVersionUtils.getResourcesDirectory(project); + if (resourcesDir.getAsFile().exists()) { + t.getResourcesDirectory().set(resourcesDir); } t.getReferencesFiles().setFrom(tvReferencesConfig); }); 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 ca932adadae25..2e1fdedd752aa 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 @@ -9,8 +9,6 @@ package org.elasticsearch.gradle.internal.transport; -import com.google.common.collect.Comparators; - import org.gradle.api.Project; import org.gradle.api.attributes.Attribute; import org.gradle.api.attributes.AttributeContainer; @@ -21,7 +19,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Comparator; import java.util.List; import static org.gradle.api.artifacts.type.ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE; @@ -30,8 +27,6 @@ class TransportVersionUtils { static final Attribute TRANSPORT_VERSION_REFERENCES_ATTRIBUTE = Attribute.of("transport-version-references", Boolean.class); - record TransportVersionConstant(String name, List ids) {} - record TransportVersionReference(String name, String location) { @Override public String toString() { @@ -39,24 +34,72 @@ public String toString() { } } - static TransportVersionConstant readDefinitionFile(Path file) throws IOException { - assert file.endsWith(".csv"); - String rawName = file.getFileName().toString(); - String name = rawName.substring(0, rawName.length() - 4); - List ids = new ArrayList<>(); - - for (String rawId : Files.readString(file, StandardCharsets.UTF_8).split(",")) { - try { - ids.add(Integer.parseInt(rawId.strip())); - } catch (NumberFormatException e) { - throw new IOException("Failed to parse id " + rawId + " in " + file, e); + record TransportVersionDefinition(String name, List ids) { + public static TransportVersionDefinition fromString(String filename, String contents) { + assert filename.endsWith(".csv"); + String name = filename.substring(0, filename.length() - 4); + List ids = new ArrayList<>(); + + if (contents.isEmpty() == false) { + for (String rawId : contents.split(",")) { + try { + ids.add(parseId(rawId)); + } catch (NumberFormatException e) { + throw new IllegalStateException("Failed to parse id " + rawId + " in " + filename, e); + } + } + } + + return new TransportVersionDefinition(name, ids); + } + } + + record TransportVersionLatest(String branch, String name, TransportVersionId id) { + public static TransportVersionLatest fromString(String filename, String contents) { + assert filename.endsWith(".csv"); + String branch = filename.substring(0, filename.length() - 4); + + String[] parts = contents.split(","); + if (parts.length != 2) { + throw new IllegalStateException("Invalid transport version latest file [" + filename + "]: " + contents); } + + return new TransportVersionLatest(branch, parts[0], parseId(parts[1])); + } + } + + record TransportVersionId(int complete, int major, int server, int subsidiary, int patch) implements Comparable { + @Override + public int compareTo(TransportVersionId o) { + return Integer.compare(complete, o.complete); + } + + @Override + public String toString() { + return Integer.toString(complete); } - if (Comparators.isInOrder(ids, Comparator.reverseOrder()) == false) { - throw new IOException("invalid transport version data file [" + file + "], ids are not in sorted"); + public int base() { + return (complete / 1000) * 1000; } - return new TransportVersionConstant(name, ids); + } + + static Path definitionFilePath(Project project, String name) { + return getDefinitionsDirectory(project).getAsFile().toPath().resolve(name + ".csv"); + } + + static Path latestFilePath(Project project, String name) { + return getLatestDirectory(project).getAsFile().toPath().resolve(name + ".csv"); + } + + static TransportVersionDefinition readDefinitionFile(Path file) throws IOException { + String contents = Files.readString(file, StandardCharsets.UTF_8).strip(); + return TransportVersionDefinition.fromString(file.getFileName().toString(), contents); + } + + static TransportVersionLatest readLatestFile(Path file) throws IOException { + String contents = Files.readString(file, StandardCharsets.UTF_8).strip(); + return TransportVersionLatest.fromString(file.getFileName().toString(), contents); } static List readReferencesFile(Path file) throws IOException { @@ -72,13 +115,30 @@ static List readReferencesFile(Path file) throws IOEx return results; } + private static TransportVersionId parseId(String rawId) { + int complete = Integer.parseInt(rawId); + int patch = complete % 100; + int subsidiary = (complete / 100) % 10; + int server = (complete / 1000) % 1000; + int major = complete / 1000000; + return new TransportVersionId(complete, major, server, subsidiary, patch); + } + static Directory getDefinitionsDirectory(Project project) { + return getResourcesDirectory(project).dir("defined"); + } + + static Directory getLatestDirectory(Project project) { + return getResourcesDirectory(project).dir("latest"); + } + + static Directory getResourcesDirectory(Project project) { var projectName = project.findProperty("org.elasticsearch.transport.definitionsProject"); if (projectName == null) { projectName = ":server"; } Directory projectDir = project.project(projectName.toString()).getLayout().getProjectDirectory(); - return projectDir.dir("src/main/resources/transport/defined"); + return projectDir.dir("src/main/resources/transport"); } static void addTransportVersionReferencesAttribute(AttributeContainer attributes) { 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 1c55bacb31e8f..02145abfd3ebf 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 @@ -9,6 +9,11 @@ package org.elasticsearch.gradle.internal.transport; +import com.google.common.collect.Comparators; + +import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionDefinition; +import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionId; +import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionLatest; import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionReference; import org.gradle.api.DefaultTask; import org.gradle.api.file.ConfigurableFileCollection; @@ -20,14 +25,32 @@ import org.gradle.api.tasks.PathSensitive; import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; +import org.gradle.process.ExecOperations; +import org.gradle.process.ExecResult; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.regex.Pattern; + +import javax.inject.Inject; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.definitionFilePath; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.latestFilePath; import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.readDefinitionFile; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.readLatestFile; import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.readReferencesFile; /** @@ -39,28 +62,240 @@ public abstract class ValidateTransportVersionDefinitionsTask extends DefaultTas @InputDirectory @Optional @PathSensitive(PathSensitivity.RELATIVE) - public abstract DirectoryProperty getDefinitionsDirectory(); + public abstract DirectoryProperty getResourcesDirectory(); @InputFiles @PathSensitive(PathSensitivity.RELATIVE) public abstract ConfigurableFileCollection getReferencesFiles(); + private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {} + + private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+"); + + private final ExecOperations execOperations; + private final String rootPath; + + // all transport version names referenced + private final Set allNames = new HashSet<>(); + // direct lookup of definition by name + private final Map definitions = new HashMap<>(); + // which resource files already existed + private final Set existingResources = new HashSet<>(); + // reverse lookup of ids back to name + private final Map definedIds = new HashMap<>(); + // lookup of base ids back to definition + private final Map> idsByBase = new HashMap<>(); + // direct lookup of latest for each branch + Map latestByBranch = new HashMap<>(); + + @Inject + public ValidateTransportVersionDefinitionsTask(ExecOperations execOperations) { + this.execOperations = execOperations; + this.rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().getAbsolutePath(); + } + @TaskAction public void validateTransportVersions() throws IOException { - Path constantsDir = getDefinitionsDirectory().getAsFile().get().toPath(); + Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath(); + Path definitionsDir = resourcesDir.resolve("defined"); + Path latestDir = resourcesDir.resolve("latest"); + + // first check which resource files already exist in main + recordExistingResources(); + + // then collect all names referenced in the codebase + for (var referencesFile : getReferencesFiles()) { + readReferencesFile(referencesFile.toPath()).stream().map(TransportVersionReference::name).forEach(allNames::add); + } + + // 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 + try (var definitionsStream = Files.list(definitionsDir)) { + for (var definitionFile : definitionsStream.toList()) { + recordAndValidateDefinition(readDefinitionFile(definitionFile)); + } + } + + // cleanup base lookup so we can check ids + // NOTE: this must run after definition recording + for (var entry : idsByBase.entrySet()) { + cleanupAndValidateBase(entry.getKey(), entry.getValue()); + } + + // now load all latest versions and do validation + // NOTE: this must run after definition recording and idsByBase cleanup + try (var latestStream = Files.list(latestDir)) { + for (var latestFile : latestStream.toList()) { + recordAndValidateLatest(readLatestFile(latestFile)); + } + } + } + + private String gitCommand(String... args) { + final ByteArrayOutputStream stdout = new ByteArrayOutputStream(); + + List command = new ArrayList<>(); + Collections.addAll(command, "git", "-C", rootPath); + Collections.addAll(command, args); + + ExecResult result = execOperations.exec(spec -> { + spec.setCommandLine(command); + spec.setStandardOutput(stdout); + spec.setErrorOutput(stdout); + spec.setIgnoreExitValue(true); + }); + + if (result.getExitValue() != 0) { + throw new RuntimeException("git command failed with exit code " + result.getExitValue() + System.lineSeparator() + + "command: " + String.join(" ", command) + System.lineSeparator() + + "output:" + System.lineSeparator() + + stdout.toString(StandardCharsets.UTF_8)); + } + + return stdout.toString(StandardCharsets.UTF_8); + } + + private void recordExistingResources() { + String resourcesPath = relativePath(getResourcesDirectory().getAsFile().get().toPath()); + String output = gitCommand("ls-tree", "--name-only", "-r", "main", resourcesPath); + Collections.addAll(existingResources, output.split(System.lineSeparator())); + } - Set allTvNames = new HashSet<>(); - for (var tvReferencesFile : getReferencesFiles()) { - readReferencesFile(tvReferencesFile.toPath()).stream().map(TransportVersionReference::name).forEach(allTvNames::add); + private void recordAndValidateDefinition(TransportVersionDefinition definition) { + definitions.put(definition.name(), definition); + // record the ids for each base id so we can ensure compactness later + for (TransportVersionId id : definition.ids()) { + idsByBase.computeIfAbsent(id.base(), k -> new ArrayList<>()).add(new IdAndDefinition(id, definition)); + } + + // validate any modifications + Map existingIdsByBase = new HashMap<>(); + TransportVersionDefinition originalDefinition = readExistingDefinition(definition.name()); + if (originalDefinition != null) { + + int primaryId = definition.ids().get(0).complete(); + int originalPrimaryId = originalDefinition.ids().get(0).complete(); + if (primaryId != originalPrimaryId) { + throwDefinitionFailure(definition.name(), "has modified primary id from " + originalPrimaryId + " to " + primaryId); + } + + originalDefinition.ids().forEach(id -> existingIdsByBase.put(id.base(), id)); + } + + if (allNames.contains(definition.name()) == false) { + throwDefinitionFailure(definition.name(), "is not referenced"); + } + if (NAME_FORMAT.matcher(definition.name()).matches() == false) { + throwDefinitionFailure(definition.name(), "does not have a valid name, must be lowercase alphanumeric and underscore"); + } + if (definition.ids().isEmpty()) { + throwDefinitionFailure(definition.name(), "does not contain any ids"); } + if (Comparators.isInOrder(definition.ids(), Comparator.reverseOrder()) == false) { + throwDefinitionFailure(definition.name(), "does not have ordered ids"); + } + for (int ndx = 0; ndx < definition.ids().size(); ++ndx) { + TransportVersionId id = definition.ids().get(ndx); + + String existing = definedIds.put(id.complete(), definition.name()); + if (existing != null) { + throwDefinitionFailure(definition.name(), "contains id already defined in [" + definitionRelativePath(existing) + "]"); + } - try (var constantsStream = Files.list(constantsDir)) { - for (var constantsFile : constantsStream.toList()) { - var tv = readDefinitionFile(constantsFile); - if (allTvNames.contains(tv.name()) == false) { - throw new IllegalStateException("Transport version constant " + tv.name() + " is not referenced"); + if (ndx == 0) { + if (id.patch() != 0) { + throwDefinitionFailure(definition.name(), "has patch version " + id.complete() + " as primary id"); } + } else { + if (id.patch() == 0) { + throwDefinitionFailure(definition.name(), "contains bwc id [" + id + "] with a patch part of 0"); + } + } + + // check modifications of ids on same branch, ie sharing same base + TransportVersionId maybeModifiedId = existingIdsByBase.get(id.base()); + if (maybeModifiedId != null && maybeModifiedId.complete() != id.complete()) { + throwDefinitionFailure(definition.name(), "modifies existing patch id from " + maybeModifiedId + " to " + id); + } + } + } + + private TransportVersionDefinition readExistingDefinition(String name) { + return readExistingFile(name, this::definitionRelativePath, TransportVersionDefinition::fromString); + } + + private TransportVersionLatest readExistingLatest(String branch) { + return readExistingFile(branch, this::latestRelativePath, TransportVersionLatest::fromString); + } + + private T readExistingFile(String name, Function pathFunction, BiFunction parser) { + String relativePath = pathFunction.apply(name); + if (existingResources.contains(relativePath) == false) { + return null; + } + String content = gitCommand("show", "main:" + relativePath); + return parser.apply(relativePath, content); + } + + private void recordAndValidateLatest(TransportVersionLatest latest) { + latestByBranch.put(latest.branch(), latest); + + TransportVersionDefinition latestDefinition = definitions.get(latest.name()); + if (latestDefinition == null) { + throwLatestFailure(latest.branch(), "contains transport version name [" + latest.name() + "] which is not defined"); + } + if (latestDefinition.ids().contains(latest.id()) == false) { + throwLatestFailure(latest.branch(), "has id " + latest.id() + + " which is not in definition [" + definitionRelativePath(latest.name()) + "]"); + } + + List baseIds = idsByBase.get(latest.id().base()); + IdAndDefinition lastId = baseIds.getLast(); + if (lastId.id().complete() != latest.id().complete()) { + throwLatestFailure(latest.branch(), "has id " + latest.id() + " from [" + latest.name() + + "] with base " + latest.id().base() + " but another id " + lastId.id().complete() + " from [" + + lastId.definition().name() + "] is later for that base"); + } + + TransportVersionLatest existingLatest = readExistingLatest(latest.branch()); + if (existingLatest != null) { + if (latest.id().patch() != 0 && latest.id().base() != existingLatest.id().base()) { + throwLatestFailure(latest.branch(), "modifies base id from " + existingLatest.id().base() + " to " + latest.id().base()); + } + } + } + + private void cleanupAndValidateBase(int base, List ids) { + // first sort the ids list so we can check compactness and quickly lookup the highest id later + ids.sort(Comparator.comparingInt(a -> a.id().complete())); + + for (int ndx = 0; ndx < ids.size(); ++ndx) { + IdAndDefinition idAndDefinition = ids.get(ndx); + if (idAndDefinition.id().patch() != ndx) { + throw new IllegalStateException("Transport version base version " + base + + " is missing patch version " + (base + ndx)); } } } + + private void throwDefinitionFailure(String name, String message) { + throw new IllegalStateException("Transport version definition file [" + definitionRelativePath(name) + "] " + message); + } + + private void throwLatestFailure(String branch, String message) { + throw new IllegalStateException("Latest transport version file [" + latestRelativePath(branch) + "] " + message); + } + + private String definitionRelativePath(String name) { + return relativePath(definitionFilePath(getProject(), name)); + } + + private String latestRelativePath(String branch) { + return relativePath(latestFilePath(getProject(), branch)); + } + + private String relativePath(Path file) { + Path rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().toPath(); + return rootPath.relativize(file).toString(); + } } From c0bbefe9a6bea38a25bdbfe7ba9d530e1eee06a2 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Sun, 3 Aug 2025 14:18:34 +0000 Subject: [PATCH 02/14] [CI] Auto commit changes from spotless --- ...CollectTransportVersionReferencesTask.java | 3 +- ...lidateTransportVersionDefinitionsTask.java | 45 +++++++++++++------ 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java index 5d6382d5c5b1a..92b8487ec742d 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java @@ -74,7 +74,8 @@ public void checkTransportVersion() throws IOException { Files.writeString(outputFile, String.join("\n", results.stream().map(Object::toString).sorted().toList())); } - private void addNamesFromClassesDirectory(Set results, Path basePath) throws IOException { + private void addNamesFromClassesDirectory(Set results, Path basePath) + throws IOException { Files.walkFileTree(basePath, new SimpleFileVisitor<>() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { 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 02145abfd3ebf..ffddee3d3147f 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 @@ -78,11 +78,11 @@ private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition // all transport version names referenced private final Set allNames = new HashSet<>(); // direct lookup of definition by name - private final Map definitions = new HashMap<>(); + private final Map definitions = new HashMap<>(); // which resource files already existed private final Set existingResources = new HashSet<>(); // reverse lookup of ids back to name - private final Map definedIds = new HashMap<>(); + private final Map definedIds = new HashMap<>(); // lookup of base ids back to definition private final Map> idsByBase = new HashMap<>(); // direct lookup of latest for each branch @@ -146,10 +146,17 @@ private String gitCommand(String... args) { }); if (result.getExitValue() != 0) { - throw new RuntimeException("git command failed with exit code " + result.getExitValue() + System.lineSeparator() + - "command: " + String.join(" ", command) + System.lineSeparator() + - "output:" + System.lineSeparator() - + stdout.toString(StandardCharsets.UTF_8)); + throw new RuntimeException( + "git command failed with exit code " + + result.getExitValue() + + System.lineSeparator() + + "command: " + + String.join(" ", command) + + System.lineSeparator() + + "output:" + + System.lineSeparator() + + stdout.toString(StandardCharsets.UTF_8) + ); } return stdout.toString(StandardCharsets.UTF_8); @@ -245,16 +252,29 @@ private void recordAndValidateLatest(TransportVersionLatest latest) { throwLatestFailure(latest.branch(), "contains transport version name [" + latest.name() + "] which is not defined"); } if (latestDefinition.ids().contains(latest.id()) == false) { - throwLatestFailure(latest.branch(), "has id " + latest.id() + - " which is not in definition [" + definitionRelativePath(latest.name()) + "]"); + throwLatestFailure( + latest.branch(), + "has id " + latest.id() + " which is not in definition [" + definitionRelativePath(latest.name()) + "]" + ); } List baseIds = idsByBase.get(latest.id().base()); IdAndDefinition lastId = baseIds.getLast(); if (lastId.id().complete() != latest.id().complete()) { - throwLatestFailure(latest.branch(), "has id " + latest.id() + " from [" + latest.name() + - "] with base " + latest.id().base() + " but another id " + lastId.id().complete() + " from [" + - lastId.definition().name() + "] is later for that base"); + throwLatestFailure( + latest.branch(), + "has id " + + latest.id() + + " from [" + + latest.name() + + "] with base " + + latest.id().base() + + " but another id " + + lastId.id().complete() + + " from [" + + lastId.definition().name() + + "] is later for that base" + ); } TransportVersionLatest existingLatest = readExistingLatest(latest.branch()); @@ -272,8 +292,7 @@ private void cleanupAndValidateBase(int base, List ids) { for (int ndx = 0; ndx < ids.size(); ++ndx) { IdAndDefinition idAndDefinition = ids.get(ndx); if (idAndDefinition.id().patch() != ndx) { - throw new IllegalStateException("Transport version base version " + base + - " is missing patch version " + (base + ndx)); + throw new IllegalStateException("Transport version base version " + base + " is missing patch version " + (base + ndx)); } } } From 23b27d405726854170a1674f27b35b887f57dd0a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 3 Aug 2025 10:38:28 -0700 Subject: [PATCH 03/14] sort definition loading --- .../TransportVersionManagementPluginFuncTest.groovy | 4 ++-- .../ValidateTransportVersionDefinitionsTask.java | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) 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 cacf799263fbb..77f91035b285d 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 @@ -186,8 +186,8 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest { def result = validateDefinitionsFails() then: assertDefinitionsFailure(result, "Transport version definition file " + - "[myserver/src/main/resources/transport/defined/duplicate.csv] contains id already defined in " + - "[myserver/src/main/resources/transport/defined/existing_92.csv]") + "[myserver/src/main/resources/transport/defined/existing_92.csv] contains id 8123000 already defined in " + + "[myserver/src/main/resources/transport/defined/duplicate.csv]") } def "definitions have bwc ids with non-zero patch part"() { 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 ffddee3d3147f..3e2707081b7f5 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 @@ -110,7 +110,8 @@ 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 - try (var definitionsStream = Files.list(definitionsDir)) { + // 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)); } @@ -206,7 +207,10 @@ private void recordAndValidateDefinition(TransportVersionDefinition definition) String existing = definedIds.put(id.complete(), definition.name()); if (existing != null) { - throwDefinitionFailure(definition.name(), "contains id already defined in [" + definitionRelativePath(existing) + "]"); + throwDefinitionFailure( + definition.name(), + "contains id " + id + " already defined in [" + definitionRelativePath(existing) + "]" + ); } if (ndx == 0) { From cf6fcc8f140f80ca9b6e0ebb29876e339821164f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 4 Aug 2025 06:48:05 -0700 Subject: [PATCH 04/14] strip existing read of whitespace --- .../transport/ValidateTransportVersionDefinitionsTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3e2707081b7f5..03562a92ac451 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 @@ -244,7 +244,7 @@ private T readExistingFile(String name, Function pathFunctio if (existingResources.contains(relativePath) == false) { return null; } - String content = gitCommand("show", "main:" + relativePath); + String content = gitCommand("show", "main:" + relativePath).strip(); return parser.apply(relativePath, content); } From 32503fa82a630864f44e7eaa0e60959c89157108 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 4 Aug 2025 10:17:11 -0700 Subject: [PATCH 05/14] better validation before migrating --- .../TransportVersionManagementPluginFuncTest.groovy | 2 +- .../ValidateTransportVersionDefinitionsTask.java | 12 ++++++++---- .../transport/defined/mappings_in_data_streams.csv | 1 + 3 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 server/src/main/resources/transport/defined/mappings_in_data_streams.csv 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 77f91035b285d..2c9fba39cf625 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 @@ -275,7 +275,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest { when: def result = validateDefinitionsFails() then: - assertDefinitionsFailure(result, "Transport version base version 8013000 is missing patch version 8013001") + assertDefinitionsFailure(result, "Transport version base id 8013000 is missing patch ids between 8013000 and 8013002") } def "primary id must not be patch version"() { 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 03562a92ac451..721a6157ffa97 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 @@ -293,10 +293,14 @@ private void cleanupAndValidateBase(int base, List ids) { // first sort the ids list so we can check compactness and quickly lookup the highest id later ids.sort(Comparator.comparingInt(a -> a.id().complete())); - for (int ndx = 0; ndx < ids.size(); ++ndx) { - IdAndDefinition idAndDefinition = ids.get(ndx); - if (idAndDefinition.id().patch() != ndx) { - throw new IllegalStateException("Transport version base version " + base + " is missing patch version " + (base + ndx)); + // TODO: switch this to a fully dense check once all existing transport versions have been migrated + IdAndDefinition previous = ids.getLast(); + for (int ndx = ids.size() - 2; ndx >= 0; --ndx) { + IdAndDefinition next = ids.get(ndx); + // note that next and previous are reversed here because we are iterating in reverse order + if (previous.id().complete() - 1 != next.id().complete()) { + throw new IllegalStateException("Transport version base id " + base + + " is missing patch ids between " + next.id() + " and " + previous.id()); } } } diff --git a/server/src/main/resources/transport/defined/mappings_in_data_streams.csv b/server/src/main/resources/transport/defined/mappings_in_data_streams.csv new file mode 100644 index 0000000000000..2c15e7beaa6d3 --- /dev/null +++ b/server/src/main/resources/transport/defined/mappings_in_data_streams.csv @@ -0,0 +1 @@ +9112000 From a13711cb54bd6a9cd81cfda138851074aa634833 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Mon, 4 Aug 2025 17:26:09 +0000 Subject: [PATCH 06/14] [CI] Auto commit changes from spotless --- .../transport/ValidateTransportVersionDefinitionsTask.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 721a6157ffa97..da97895cfc080 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 @@ -299,8 +299,9 @@ private void cleanupAndValidateBase(int base, List ids) { IdAndDefinition next = ids.get(ndx); // note that next and previous are reversed here because we are iterating in reverse order if (previous.id().complete() - 1 != next.id().complete()) { - throw new IllegalStateException("Transport version base id " + base + - " is missing patch ids between " + next.id() + " and " + previous.id()); + throw new IllegalStateException( + "Transport version base id " + base + " is missing patch ids between " + next.id() + " and " + previous.id() + ); } } } From 6b271ee2257e89561db0849cbc02c951944f66a6 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 11 Aug 2025 10:13:01 -0700 Subject: [PATCH 07/14] make validation pass for server --- ...lidateTransportVersionDefinitionsTask.java | 20 +++++++++++-------- .../org/elasticsearch/TransportVersions.java | 4 ++-- .../defined/initial_elasticsearch_8_18_5.csv | 1 + .../defined/initial_elasticsearch_9_0_5.csv | 1 + .../defined/mappings_in_data_streams.csv | 1 - .../main/resources/transport/latest/8.18.csv | 2 +- .../main/resources/transport/latest/8.19.csv | 2 +- .../main/resources/transport/latest/9.0.csv | 2 +- .../main/resources/transport/latest/9.1.csv | 2 +- .../main/resources/transport/latest/9.2.csv | 2 +- 10 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv create mode 100644 server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv delete mode 100644 server/src/main/resources/transport/defined/mappings_in_data_streams.csv 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 da97895cfc080..31cca154c4df7 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 @@ -16,6 +16,7 @@ import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionLatest; import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionReference; import org.gradle.api.DefaultTask; +import org.gradle.api.Project; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.DirectoryProperty; import org.gradle.api.tasks.CacheableTask; @@ -72,8 +73,9 @@ private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+"); + private final Project project; + private final Path rootPath; private final ExecOperations execOperations; - private final String rootPath; // all transport version names referenced private final Set allNames = new HashSet<>(); @@ -89,9 +91,10 @@ private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition Map latestByBranch = new HashMap<>(); @Inject - public ValidateTransportVersionDefinitionsTask(ExecOperations execOperations) { + public ValidateTransportVersionDefinitionsTask(Project project, ExecOperations execOperations) { + this.project = project; this.execOperations = execOperations; - this.rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().getAbsolutePath(); + this.rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().toPath(); } @TaskAction @@ -136,7 +139,7 @@ private String gitCommand(String... args) { final ByteArrayOutputStream stdout = new ByteArrayOutputStream(); List command = new ArrayList<>(); - Collections.addAll(command, "git", "-C", rootPath); + Collections.addAll(command, "git", "-C", rootPath.toAbsolutePath().toString()); Collections.addAll(command, args); ExecResult result = execOperations.exec(spec -> { @@ -214,7 +217,9 @@ private void recordAndValidateDefinition(TransportVersionDefinition definition) } if (ndx == 0) { - if (id.patch() != 0) { + // TODO: initial versions will only be applicable to a release branch, so they won't have an associated + // main version. They will also be loaded differently in the future, but until they are separate, we ignore them here. + if (id.patch() != 0 && definition.name().startsWith("initial_") == false) { throwDefinitionFailure(definition.name(), "has patch version " + id.complete() + " as primary id"); } } else { @@ -315,15 +320,14 @@ private void throwLatestFailure(String branch, String message) { } private String definitionRelativePath(String name) { - return relativePath(definitionFilePath(getProject(), name)); + return relativePath(definitionFilePath(project, name)); } private String latestRelativePath(String branch) { - return relativePath(latestFilePath(getProject(), branch)); + return relativePath(latestFilePath(project, branch)); } private String relativePath(Path file) { - Path rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().toPath(); return rootPath.relativize(file).toString(); } } diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index c5562f187b4ab..3b9b4a2de1b57 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -148,7 +148,7 @@ static TransportVersion def(int id) { public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_2 = def(8_840_0_04); public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_3 = def(8_840_0_05); public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_4 = def(8_840_0_06); - public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_5 = def(8_840_0_07); + public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_5 = TransportVersion.fromName("initial_elasticsearch_8_18_5"); public static final TransportVersion INITIAL_ELASTICSEARCH_8_19 = def(8_841_0_00); public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED_BACKPORT_8_X = def(8_841_0_01); public static final TransportVersion REMOVE_ALL_APPLICABLE_SELECTOR_BACKPORT_8_19 = def(8_841_0_02); @@ -219,7 +219,7 @@ static TransportVersion def(int id) { public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_2 = def(9_000_0_11); public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_3 = def(9_000_0_12); public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_4 = def(9_000_0_13); - public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_5 = def(9_000_0_14); + public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_5 = TransportVersion.fromName("initial_elasticsearch_9_0_5"); public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED = def(9_001_0_00); public static final TransportVersion REMOVE_SNAPSHOT_FAILURES = def(9_002_0_00); public static final TransportVersion TRANSPORT_STATS_HANDLING_TIME_REQUIRED = def(9_003_0_00); diff --git a/server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv b/server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv new file mode 100644 index 0000000000000..a22b09457dfb3 --- /dev/null +++ b/server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv @@ -0,0 +1 @@ +8840007 diff --git a/server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv b/server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv new file mode 100644 index 0000000000000..4615ee4f02e0e --- /dev/null +++ b/server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv @@ -0,0 +1 @@ +9000014 diff --git a/server/src/main/resources/transport/defined/mappings_in_data_streams.csv b/server/src/main/resources/transport/defined/mappings_in_data_streams.csv deleted file mode 100644 index 2c15e7beaa6d3..0000000000000 --- a/server/src/main/resources/transport/defined/mappings_in_data_streams.csv +++ /dev/null @@ -1 +0,0 @@ -9112000 diff --git a/server/src/main/resources/transport/latest/8.18.csv b/server/src/main/resources/transport/latest/8.18.csv index 987d72e2aaeae..2f86b67145538 100644 --- a/server/src/main/resources/transport/latest/8.18.csv +++ b/server/src/main/resources/transport/latest/8.18.csv @@ -1 +1 @@ -placeholder,8840007 +initial_elasticsearch_8_18_5,8840007 diff --git a/server/src/main/resources/transport/latest/8.19.csv b/server/src/main/resources/transport/latest/8.19.csv index 2480f207cc6e4..7f608c0ced635 100644 --- a/server/src/main/resources/transport/latest/8.19.csv +++ b/server/src/main/resources/transport/latest/8.19.csv @@ -1 +1 @@ -placeholder,8841064 +esql_split_on_big_values,8841063 diff --git a/server/src/main/resources/transport/latest/9.0.csv b/server/src/main/resources/transport/latest/9.0.csv index 478f07788af87..e0a322af52ba5 100644 --- a/server/src/main/resources/transport/latest/9.0.csv +++ b/server/src/main/resources/transport/latest/9.0.csv @@ -1 +1 @@ -placeholder,9000014 +initial_elasticsearch_9_0_5,9000014 diff --git a/server/src/main/resources/transport/latest/9.1.csv b/server/src/main/resources/transport/latest/9.1.csv index 21304ce07f713..19c743726530c 100644 --- a/server/src/main/resources/transport/latest/9.1.csv +++ b/server/src/main/resources/transport/latest/9.1.csv @@ -1 +1 @@ -placeholder,9112003 +esql_split_on_big_values,9112001 diff --git a/server/src/main/resources/transport/latest/9.2.csv b/server/src/main/resources/transport/latest/9.2.csv index 5db8e8fb48f39..a7e0269c3deaa 100644 --- a/server/src/main/resources/transport/latest/9.2.csv +++ b/server/src/main/resources/transport/latest/9.2.csv @@ -1 +1 @@ -placeholder,9130000 +esql_split_on_big_values,9116000 From 31c1c7f02d1c69586472193e82014180de688e5d Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 11 Aug 2025 10:27:38 -0700 Subject: [PATCH 08/14] fix oops --- server/src/main/java/org/elasticsearch/TransportVersions.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 3b9b4a2de1b57..46e1d8b7b8f54 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -148,7 +148,6 @@ static TransportVersion def(int id) { public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_2 = def(8_840_0_04); public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_3 = def(8_840_0_05); public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_4 = def(8_840_0_06); - public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_5 = TransportVersion.fromName("initial_elasticsearch_8_18_5"); public static final TransportVersion INITIAL_ELASTICSEARCH_8_19 = def(8_841_0_00); public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED_BACKPORT_8_X = def(8_841_0_01); public static final TransportVersion REMOVE_ALL_APPLICABLE_SELECTOR_BACKPORT_8_19 = def(8_841_0_02); @@ -219,7 +218,6 @@ static TransportVersion def(int id) { public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_2 = def(9_000_0_11); public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_3 = def(9_000_0_12); public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_4 = def(9_000_0_13); - public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_5 = TransportVersion.fromName("initial_elasticsearch_9_0_5"); public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED = def(9_001_0_00); public static final TransportVersion REMOVE_SNAPSHOT_FAILURES = def(9_002_0_00); public static final TransportVersion TRANSPORT_STATS_HANDLING_TIME_REQUIRED = def(9_003_0_00); From d2327a8383718939c957d0b242a5696d57de064a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 12 Aug 2025 10:31:35 -0700 Subject: [PATCH 09/14] address feedback --- ...portVersionManagementPluginFuncTest.groovy | 25 +++++++++++++------ .../transport/TransportVersionUtils.java | 4 +-- 2 files changed, 20 insertions(+), 9 deletions(-) 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 2c9fba39cf625..0fcd2d0ae68c3 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 @@ -16,9 +16,13 @@ import org.gradle.testkit.runner.TaskOutcome class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest { - // collection task - // - + /** + * + * @param project + * @param path + * @param content + * @return + */ def javaResource(String project, String path, String content) { file("${project}/src/main/resources/${path}").withWriter { writer -> writer << content @@ -43,7 +47,11 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest { } def definedAndUsedTransportVersion(String name, String ids) { - javaSource("myserver", "org.elasticsearch", "Test${name.capitalize()}", "", """ + return definedAndUsedTransportVersion(name, ids, "Test${name.capitalize()}") + } + + def definedAndUsedTransportVersion(String name, String ids, String classname) { + javaSource("myserver", "org.elasticsearch", classname, "", """ static final TransportVersion usage = TransportVersion.fromName("${name}"); """) definedTransportVersion(name, ids) @@ -148,15 +156,18 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest { "[myserver/src/main/resources/transport/defined/not_used.csv] is not referenced") } - def "names must be lowercase"() { + def "names must be lowercase alphanum or underscore"() { given: - definedAndUsedTransportVersion("CapitalTV", "8100000") + definedAndUsedTransportVersion("${name}", "8100000", "TestNames") when: def result = validateDefinitionsFails() then: assertDefinitionsFailure(result, "Transport version definition file " + - "[myserver/src/main/resources/transport/defined/CapitalTV.csv] does not have a valid name, " + + "[myserver/src/main/resources/transport/defined/${name}.csv] does not have a valid name, " + "must be lowercase alphanumeric and underscore") + + where: + name << ["CapitalTV", "spaces tv", "trailing_spaces_tv ", "hyphen-tv", "period.tv"] } def "definitions contain at least one id"() { 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 2e1fdedd752aa..689aeb3268e11 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 @@ -30,7 +30,7 @@ class TransportVersionUtils { record TransportVersionReference(String name, String location) { @Override public String toString() { - return name + " " + location; + return name + "," + location; } } @@ -106,7 +106,7 @@ static List readReferencesFile(Path file) throws IOEx assert file.endsWith(".txt"); List results = new ArrayList<>(); for (String line : Files.readAllLines(file, StandardCharsets.UTF_8)) { - String[] parts = line.split(" ", 2); + String[] parts = line.split(",", 2); if (parts.length != 2) { throw new IOException("Invalid transport version data file [" + file + "]: " + line); } From 42a37b7f2e9084bdf0ce28aa1762b531be5de3bb Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 12 Aug 2025 10:56:04 -0700 Subject: [PATCH 10/14] feedback --- .../GenerateTransportVersionManifestTask.java | 7 ++++--- .../GlobalTransportVersionManagementPlugin.java | 2 +- .../TransportVersionManagementPlugin.java | 6 +++--- .../transport/TransportVersionUtils.java | 16 ++++++++-------- .../ValidateTransportVersionDefinitionsTask.java | 9 +++++++-- .../ValidateTransportVersionReferencesTask.java | 7 ++++--- 6 files changed, 27 insertions(+), 20 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 47ab03e80c31c..dbb3b4f8db45d 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 @@ -22,17 +22,18 @@ public abstract class GenerateTransportVersionManifestTask extends DefaultTask { @InputDirectory - public abstract DirectoryProperty getDefinitionsDirectory(); + public abstract DirectoryProperty getResourcesDirectory(); @OutputFile public abstract RegularFileProperty getManifestFile(); @TaskAction public void generateTransportVersionManifest() throws IOException { - Path constantsDir = getDefinitionsDirectory().get().getAsFile().toPath(); + Path resourcesDir = getResourcesDirectory().get().getAsFile().toPath(); + Path definitionsDir = TransportVersionUtils.getDefinitionsDirectory(resourcesDir); Path manifestFile = getManifestFile().get().getAsFile().toPath(); try (var writer = Files.newBufferedWriter(manifestFile)) { - try (var stream = Files.list(constantsDir)) { + 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 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 43192bb8ac3d0..37e35192e665f 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 @@ -55,7 +55,7 @@ public void apply(Project project) { .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(TransportVersionUtils.getDefinitionsDirectory(project)); + t.getResourcesDirectory().set(TransportVersionUtils.getResourcesDirectory(project)); t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt")); }); project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java index 47af8f288f958..89f64049d4631 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java @@ -43,9 +43,9 @@ public void apply(Project project) { .register("validateTransportVersionReferences", ValidateTransportVersionReferencesTask.class, t -> { t.setGroup("Transport Versions"); t.setDescription("Validates that all TransportVersion references used in the project have an associated definition file"); - Directory definitionsDir = TransportVersionUtils.getDefinitionsDirectory(project); - if (definitionsDir.getAsFile().exists()) { - t.getDefinitionsDirectory().set(definitionsDir); + Directory resourcesDir = TransportVersionUtils.getResourcesDirectory(project); + if (resourcesDir.getAsFile().exists()) { + t.getResourcesDirectory().set(resourcesDir); } t.getReferencesFile().set(collectTask.get().getOutputFile()); }); 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 689aeb3268e11..6d37d5db704f2 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 @@ -84,12 +84,12 @@ public int base() { } } - static Path definitionFilePath(Project project, String name) { - return getDefinitionsDirectory(project).getAsFile().toPath().resolve(name + ".csv"); + static Path definitionFilePath(Path resourcesDir, String name) { + return getDefinitionsDirectory(resourcesDir).resolve(name + ".csv"); } - static Path latestFilePath(Project project, String name) { - return getLatestDirectory(project).getAsFile().toPath().resolve(name + ".csv"); + static Path latestFilePath(Path resourcesDir, String name) { + return getLatestDirectory(resourcesDir).resolve(name + ".csv"); } static TransportVersionDefinition readDefinitionFile(Path file) throws IOException { @@ -124,12 +124,12 @@ private static TransportVersionId parseId(String rawId) { return new TransportVersionId(complete, major, server, subsidiary, patch); } - static Directory getDefinitionsDirectory(Project project) { - return getResourcesDirectory(project).dir("defined"); + static Path getDefinitionsDirectory(Path resourcesDir) { + return resourcesDir.resolve("defined"); } - static Directory getLatestDirectory(Project project) { - return getResourcesDirectory(project).dir("latest"); + static Path getLatestDirectory(Path resourcesDir) { + return resourcesDir.resolve("latest"); } static Directory getResourcesDirectory(Project project) { 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 31cca154c4df7..893b1344643d0 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 @@ -308,6 +308,7 @@ private void cleanupAndValidateBase(int base, List ids) { "Transport version base id " + base + " is missing patch ids between " + next.id() + " and " + previous.id() ); } + previous = next; } } @@ -319,12 +320,16 @@ private void throwLatestFailure(String branch, String message) { throw new IllegalStateException("Latest transport version file [" + latestRelativePath(branch) + "] " + message); } + private Path resourcesDirPath() { + return getResourcesDirectory().get().getAsFile().toPath(); + } + private String definitionRelativePath(String name) { - return relativePath(definitionFilePath(project, name)); + return relativePath(definitionFilePath(resourcesDirPath(), name)); } private String latestRelativePath(String branch) { - return relativePath(latestFilePath(project, branch)); + return relativePath(latestFilePath(resourcesDirPath(), branch)); } private String relativePath(Path file) { 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 79050f1b171a5..77d332f4cd177 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 @@ -34,7 +34,7 @@ public abstract class ValidateTransportVersionReferencesTask extends DefaultTask @InputDirectory @Optional @PathSensitive(PathSensitivity.RELATIVE) - public abstract DirectoryProperty getDefinitionsDirectory(); + public abstract DirectoryProperty getResourcesDirectory(); @InputFile @PathSensitive(PathSensitivity.RELATIVE) @@ -43,8 +43,9 @@ public abstract class ValidateTransportVersionReferencesTask extends DefaultTask @TaskAction public void validateTransportVersions() throws IOException { final Predicate referenceChecker; - if (getDefinitionsDirectory().isPresent()) { - Path definitionsDir = getDefinitionsDirectory().getAsFile().get().toPath(); + if (getResourcesDirectory().isPresent()) { + Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath(); + Path definitionsDir = TransportVersionUtils.getDefinitionsDirectory(resourcesDir); referenceChecker = (name) -> Files.exists(definitionsDir.resolve(name + ".csv")); } else { referenceChecker = (name) -> false; From 02ebfaa58dd9dc3e3cd5ee2cb32133d55557747d Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 12 Aug 2025 10:59:05 -0700 Subject: [PATCH 11/14] remove project ref --- .../transport/ValidateTransportVersionDefinitionsTask.java | 4 +--- 1 file changed, 1 insertion(+), 3 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 893b1344643d0..750bb7f5d3686 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 @@ -73,7 +73,6 @@ private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+"); - private final Project project; private final Path rootPath; private final ExecOperations execOperations; @@ -91,8 +90,7 @@ private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition Map latestByBranch = new HashMap<>(); @Inject - public ValidateTransportVersionDefinitionsTask(Project project, ExecOperations execOperations) { - this.project = project; + public ValidateTransportVersionDefinitionsTask(ExecOperations execOperations) { this.execOperations = execOperations; this.rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().toPath(); } From 1dc253e8300c696bad95c4bfab4220dce3fd057c Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 12 Aug 2025 18:05:40 +0000 Subject: [PATCH 12/14] [CI] Auto commit changes from spotless --- .../transport/ValidateTransportVersionDefinitionsTask.java | 1 - 1 file changed, 1 deletion(-) 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 750bb7f5d3686..593515c15e882 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 @@ -16,7 +16,6 @@ import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionLatest; import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionReference; import org.gradle.api.DefaultTask; -import org.gradle.api.Project; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.DirectoryProperty; import org.gradle.api.tasks.CacheableTask; From 3336d27f0a1ca630eeee1f745ef95d1bf06bd3ee Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 12 Aug 2025 13:40:45 -0700 Subject: [PATCH 13/14] cleanup --- .../GenerateTransportVersionManifestTask.java | 5 ++-- ...lobalTransportVersionManagementPlugin.java | 7 +++-- .../TransportVersionManagementPlugin.java | 9 ++++--- .../transport/TransportVersionUtils.java | 26 +++++++++++++------ ...lidateTransportVersionDefinitionsTask.java | 8 ++---- ...alidateTransportVersionReferencesTask.java | 7 +++-- 6 files changed, 36 insertions(+), 26 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 dbb3b4f8db45d..13164dd8f457b 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 @@ -22,15 +22,14 @@ public abstract class GenerateTransportVersionManifestTask extends DefaultTask { @InputDirectory - public abstract DirectoryProperty getResourcesDirectory(); + public abstract DirectoryProperty getDefinitionsDirectory(); @OutputFile public abstract RegularFileProperty getManifestFile(); @TaskAction public void generateTransportVersionManifest() throws IOException { - Path resourcesDir = getResourcesDirectory().get().getAsFile().toPath(); - Path definitionsDir = TransportVersionUtils.getDefinitionsDirectory(resourcesDir); + Path definitionsDir = getDefinitionsDirectory().get().getAsFile().toPath(); Path manifestFile = getManifestFile().get().getAsFile().toPath(); try (var writer = Files.newBufferedWriter(manifestFile)) { try (var stream = Files.list(definitionsDir)) { 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 37e35192e665f..19732f855b05b 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 @@ -20,6 +20,9 @@ import java.util.Map; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinitionsDirectory; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getResourcesDirectory; + public class GlobalTransportVersionManagementPlugin implements Plugin { @Override @@ -43,7 +46,7 @@ public void apply(Project project) { .register("validateTransportVersionDefinitions", ValidateTransportVersionDefinitionsTask.class, t -> { t.setGroup("Transport Versions"); t.setDescription("Validates that all defined TransportVersion constants are used in at least one project"); - Directory resourcesDir = TransportVersionUtils.getResourcesDirectory(project); + Directory resourcesDir = getResourcesDirectory(project); if (resourcesDir.getAsFile().exists()) { t.getResourcesDirectory().set(resourcesDir); } @@ -55,7 +58,7 @@ public void apply(Project project) { .register("generateTransportVersionManifest", GenerateTransportVersionManifestTask.class, t -> { t.setGroup("Transport Versions"); t.setDescription("Generate a manifest resource for all the known transport version definitions"); - t.getResourcesDirectory().set(TransportVersionUtils.getResourcesDirectory(project)); + t.getDefinitionsDirectory().set(getDefinitionsDirectory(getResourcesDirectory(project))); t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt")); }); project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java index 89f64049d4631..88d419c598bea 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java @@ -17,6 +17,9 @@ import org.gradle.api.tasks.SourceSet; import org.gradle.language.base.plugins.LifecycleBasePlugin; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinitionsDirectory; +import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getResourcesDirectory; + public class TransportVersionManagementPlugin implements Plugin { @Override @@ -43,9 +46,9 @@ public void apply(Project project) { .register("validateTransportVersionReferences", ValidateTransportVersionReferencesTask.class, t -> { t.setGroup("Transport Versions"); t.setDescription("Validates that all TransportVersion references used in the project have an associated definition file"); - Directory resourcesDir = TransportVersionUtils.getResourcesDirectory(project); - if (resourcesDir.getAsFile().exists()) { - t.getResourcesDirectory().set(resourcesDir); + Directory definitionsDir = getDefinitionsDirectory(getResourcesDirectory(project)); + if (definitionsDir.getAsFile().exists()) { + t.getDefinitionsDirectory().set(definitionsDir); } t.getReferencesFile().set(collectTask.get().getOutputFile()); }); 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 6d37d5db704f2..4e1cb86276634 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 @@ -69,6 +69,16 @@ public static TransportVersionLatest fromString(String filename, String contents } record TransportVersionId(int complete, int major, int server, int subsidiary, int patch) implements Comparable { + + static TransportVersionId fromString(String s) { + int complete = Integer.parseInt(s); + int patch = complete % 100; + int subsidiary = (complete / 100) % 10; + int server = (complete / 1000) % 1000; + int major = complete / 1000000; + return new TransportVersionId(complete, major, server, subsidiary, patch); + } + @Override public int compareTo(TransportVersionId o) { return Integer.compare(complete, o.complete); @@ -84,12 +94,12 @@ public int base() { } } - static Path definitionFilePath(Path resourcesDir, String name) { - return getDefinitionsDirectory(resourcesDir).resolve(name + ".csv"); + static Path definitionFilePath(Directory resourcesDirectory, String name) { + return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve(name + ".csv"); } - static Path latestFilePath(Path resourcesDir, String name) { - return getLatestDirectory(resourcesDir).resolve(name + ".csv"); + static Path latestFilePath(Directory resourcesDirectory, String name) { + return getLatestDirectory(resourcesDirectory).getAsFile().toPath().resolve(name + ".csv"); } static TransportVersionDefinition readDefinitionFile(Path file) throws IOException { @@ -124,12 +134,12 @@ private static TransportVersionId parseId(String rawId) { return new TransportVersionId(complete, major, server, subsidiary, patch); } - static Path getDefinitionsDirectory(Path resourcesDir) { - return resourcesDir.resolve("defined"); + static Directory getDefinitionsDirectory(Directory resourcesDirectory) { + return resourcesDirectory.dir("defined"); } - static Path getLatestDirectory(Path resourcesDir) { - return resourcesDir.resolve("latest"); + static Directory getLatestDirectory(Directory resourcesDirectory) { + return resourcesDirectory.dir("latest"); } static Directory getResourcesDirectory(Project project) { 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 593515c15e882..4a5cfbf654c4d 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 @@ -317,16 +317,12 @@ private void throwLatestFailure(String branch, String message) { throw new IllegalStateException("Latest transport version file [" + latestRelativePath(branch) + "] " + message); } - private Path resourcesDirPath() { - return getResourcesDirectory().get().getAsFile().toPath(); - } - private String definitionRelativePath(String name) { - return relativePath(definitionFilePath(resourcesDirPath(), name)); + return relativePath(definitionFilePath(getResourcesDirectory().get(), name)); } private String latestRelativePath(String branch) { - return relativePath(latestFilePath(resourcesDirPath(), branch)); + return relativePath(latestFilePath(getResourcesDirectory().get(), branch)); } private String relativePath(Path file) { 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 77d332f4cd177..79050f1b171a5 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 @@ -34,7 +34,7 @@ public abstract class ValidateTransportVersionReferencesTask extends DefaultTask @InputDirectory @Optional @PathSensitive(PathSensitivity.RELATIVE) - public abstract DirectoryProperty getResourcesDirectory(); + public abstract DirectoryProperty getDefinitionsDirectory(); @InputFile @PathSensitive(PathSensitivity.RELATIVE) @@ -43,9 +43,8 @@ public abstract class ValidateTransportVersionReferencesTask extends DefaultTask @TaskAction public void validateTransportVersions() throws IOException { final Predicate referenceChecker; - if (getResourcesDirectory().isPresent()) { - Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath(); - Path definitionsDir = TransportVersionUtils.getDefinitionsDirectory(resourcesDir); + if (getDefinitionsDirectory().isPresent()) { + Path definitionsDir = getDefinitionsDirectory().getAsFile().get().toPath(); referenceChecker = (name) -> Files.exists(definitionsDir.resolve(name + ".csv")); } else { referenceChecker = (name) -> false; From 88d2a6bc9b24d7638ea75e8256a2cb8c4b69cb6e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 12 Aug 2025 14:01:09 -0700 Subject: [PATCH 14/14] ignore initial for now --- .../transport/ValidateTransportVersionDefinitionsTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4a5cfbf654c4d..34837dcbbeeac 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 @@ -190,7 +190,7 @@ private void recordAndValidateDefinition(TransportVersionDefinition definition) originalDefinition.ids().forEach(id -> existingIdsByBase.put(id.base(), id)); } - if (allNames.contains(definition.name()) == false) { + if (allNames.contains(definition.name()) == false && definition.name().startsWith("initial_") == false) { throwDefinitionFailure(definition.name(), "is not referenced"); } if (NAME_FORMAT.matcher(definition.name()).matches() == false) {