From f8647b85f5d9729fd16ebdd6d9425e6679fbf213 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 20 Aug 2025 15:44:54 -0700 Subject: [PATCH] Backport several transport version fixes This commit backports several changes to the new transport version system relates #133034 relates #133185 relates #133186 relates #133189 --- .../AbstractTransportVersionFuncTest.groovy | 19 +- .../TransportVersionValidationFuncTest.groovy | 123 +++++--- .../GenerateTransportVersionManifestTask.java | 19 +- .../transport/TransportVersionId.java | 3 +- .../transport/TransportVersionReference.java | 11 + .../TransportVersionReferencesPlugin.java | 8 - .../TransportVersionResourcesPlugin.java | 32 +- .../TransportVersionResourcesService.java | 254 +++++++++++++++ .../transport/TransportVersionUtils.java | 56 ---- ...alidateTransportVersionReferencesTask.java | 22 +- ...ValidateTransportVersionResourcesTask.java | 296 +++++++----------- 11 files changed, 512 insertions(+), 331 deletions(-) create mode 100644 build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java delete mode 100644 build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java 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..56c260677d76c 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,39 @@ 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 + } + + def "latest can refer to an unreferenced definition"() { + given: + unreferencedTransportVersion("initial_10.0.0", "10000000") + latestTransportVersion("10.0", "initial_10.0.0", "10000000") + when: + def result = gradleRunner(":myserver:validateTransportVersionResources").build() + then: + result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS + } + + def "named and unreferenced definitions cannot have the same name"() { + given: + unreferencedTransportVersion("existing_92", "10000000") + when: + def result = validateResourcesFails() + then: + assertValidateResourcesFailure(result, "Transport version definition file " + + "[myserver/src/main/resources/transport/definitions/named/existing_92.csv] " + + "has same name as unreferenced definition " + + "[myserver/src/main/resources/transport/definitions/unreferenced/existing_92.csv]") + } + + def "unreferenced definitions can have primary ids that are patches"() { + given: + unreferencedTransportVersion("initial_10.0.1", "10000001") + when: + def result = gradleRunner(":myserver:validateTransportVersionResources").build() + then: + result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS } } 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 cb39a08a6aa44..153fab4723ac0 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 @@ -10,10 +10,14 @@ package org.elasticsearch.gradle.internal.transport; import org.gradle.api.DefaultTask; -import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.RegularFileProperty; +import org.gradle.api.provider.Property; +import org.gradle.api.services.ServiceReference; import org.gradle.api.tasks.InputDirectory; +import org.gradle.api.tasks.Optional; import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.PathSensitive; +import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; import java.io.IOException; @@ -24,15 +28,24 @@ import java.nio.file.attribute.BasicFileAttributes; public abstract class GenerateTransportVersionManifestTask extends DefaultTask { + + @ServiceReference("transportVersionResources") + abstract Property getTransportResources(); + @InputDirectory - public abstract DirectoryProperty getDefinitionsDirectory(); + @Optional + @PathSensitive(PathSensitivity.RELATIVE) + public Path getDefinitionsDirectory() { + return getTransportResources().get().getDefinitionsDir(); + } @OutputFile public abstract RegularFileProperty getManifestFile(); @TaskAction public void generateTransportVersionManifest() throws IOException { - Path definitionsDir = getDefinitionsDirectory().get().getAsFile().toPath(); + + Path definitionsDir = getDefinitionsDirectory(); Path manifestFile = getManifestFile().get().getAsFile().toPath(); try (var writer = Files.newBufferedWriter(manifestFile)) { Files.walkFileTree(definitionsDir, new SimpleFileVisitor<>() { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java index 1fa0d8f46eea8..407c3bd511f09 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java @@ -22,7 +22,8 @@ static TransportVersionId fromString(String s) { @Override public int compareTo(TransportVersionId o) { - return Integer.compare(complete, o.complete); + // note: this is descending order so the arguments are reversed + return Integer.compare(o.complete, complete); } @Override diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java index 5c89b41db799d..f94f4fc6d9b6b 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReference.java @@ -12,12 +12,15 @@ import org.gradle.api.attributes.Attribute; import org.gradle.api.attributes.AttributeContainer; +import java.io.File; 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.HashSet; import java.util.List; +import java.util.Set; import static org.gradle.api.artifacts.type.ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE; @@ -43,6 +46,14 @@ static void addArtifactAttribute(AttributeContainer attributes) { attributes.attribute(REFERENCES_ATTRIBUTE, true); } + static Set collectNames(Iterable referencesFiles) throws IOException { + Set names = new HashSet<>(); + for (var referencesFile : referencesFiles) { + listFromFile(referencesFile.toPath()).stream().map(TransportVersionReference::name).forEach(names::add); + } + return names; + } + @Override public String toString() { return name + "," + location; diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java index 60012feac5da3..da3f056825aeb 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionReferencesPlugin.java @@ -13,13 +13,9 @@ import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.file.Directory; 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 TransportVersionReferencesPlugin implements Plugin { @Override @@ -46,10 +42,6 @@ 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 = getDefinitionsDirectory(getResourcesDirectory(project)); - if (definitionsDir.getAsFile().exists()) { - t.getDefinitionsDirectory().set(definitionsDir); - } t.getReferencesFile().set(collectTask.get().getOutputFile()); }); project.getTasks().named(LifecycleBasePlugin.CHECK_TASK_NAME).configure(t -> t.dependsOn(validateTask)); 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..763d50e6e1267 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 @@ -20,15 +20,22 @@ 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 public void apply(Project project) { project.getPluginManager().apply(LifecycleBasePlugin.class); + String resourceRoot = getResourceRoot(project); + + project.getGradle() + .getSharedServices() + .registerIfAbsent("transportVersionResources", TransportVersionResourcesService.class, spec -> { + Directory transportResources = project.getLayout().getProjectDirectory().dir("src/main/resources/" + resourceRoot); + spec.getParameters().getTransportResourcesDirectory().set(transportResources); + spec.getParameters().getRootDirectory().set(project.getRootProject().getRootDir()); + }); + DependencyHandler depsHandler = project.getDependencies(); Configuration tvReferencesConfig = project.getConfigurations().create("globalTvReferences"); tvReferencesConfig.setCanBeConsumed(false); @@ -43,13 +50,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,12 +60,19 @@ 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 -> { t.into("transport/definitions", c -> c.from(generateManifestTask)); }); } + + private static String getResourceRoot(Project project) { + var resourceRoot = project.findProperty("org.elasticsearch.transport.resourceRoot"); + if (resourceRoot == null) { + resourceRoot = "transport"; + } + return resourceRoot.toString(); + } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java new file mode 100644 index 0000000000000..b495bcff6f4b2 --- /dev/null +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java @@ -0,0 +1,254 @@ +/* + * 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.gradle.api.file.DirectoryProperty; +import org.gradle.api.services.BuildService; +import org.gradle.api.services.BuildServiceParameters; +import org.gradle.process.ExecOperations; +import org.gradle.process.ExecResult; + +import java.io.ByteArrayOutputStream; +import java.io.File; +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.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiFunction; + +import javax.inject.Inject; + +/** + * An encapsulation of operations on transport version resources. + * + *

These are resource files to describe transport versions that will be loaded at Elasticsearch runtime. They exist + * as jar resource files at runtime, and as a directory of resources at build time. + * + *

The layout of the transport version resources are as follows: + *

    + *
  • /transport/definitions/named/ + * - Definitions that can be looked up by name. The name is the filename before the .csv suffix.
    • + *
  • /transport/definitions/unreferenced/ + * - Definitions which contain ids that are known at runtime, but cannot be looked up by name.
    • + *
  • /transport/latest/ + * - The latest transport version definition for each release branch.
    • + *
+ */ +public abstract class TransportVersionResourcesService implements BuildService { + + public interface Parameters extends BuildServiceParameters { + DirectoryProperty getTransportResourcesDirectory(); + + DirectoryProperty getRootDirectory(); + } + + @Inject + public abstract ExecOperations getExecOperations(); + + private static final Path DEFINITIONS_DIR = Path.of("definitions"); + private static final Path NAMED_DIR = DEFINITIONS_DIR.resolve("named"); + private static final Path UNREFERENCED_DIR = DEFINITIONS_DIR.resolve("unreferenced"); + private static final Path LATEST_DIR = Path.of("latest"); + + private final Path transportResourcesDir; + private final Path rootDir; + private final AtomicReference> mainResources = new AtomicReference<>(null); + private final AtomicReference> changedResources = new AtomicReference<>(null); + + @Inject + public TransportVersionResourcesService(Parameters params) { + this.transportResourcesDir = params.getTransportResourcesDirectory().get().getAsFile().toPath(); + this.rootDir = params.getRootDirectory().get().getAsFile().toPath(); + } + + /** + * Return the directory for this repository which contains transport version resources. + * This should be an input to any tasks reading resources from this service. + */ + Path getTransportResourcesDir() { + return transportResourcesDir; + } + + /** + * Return the transport version definitions directory for this repository. + * This should be an input to any tasks that only read definitions from this service. + */ + Path getDefinitionsDir() { + return transportResourcesDir.resolve(DEFINITIONS_DIR); + } + + // return the path, relative to the resources dir, of a named definition + private Path getNamedDefinitionRelativePath(String name) { + return NAMED_DIR.resolve(name + ".csv"); + } + + /** Return all named definitions, mapped by their name. */ + Map getNamedDefinitions() throws IOException { + return readDefinitions(transportResourcesDir.resolve(NAMED_DIR)); + } + + /** Get a named definition from main if it exists there, or null otherwise */ + TransportVersionDefinition getNamedDefinitionFromMain(String name) { + String resourcePath = getNamedDefinitionRelativePath(name).toString(); + return getMainFile(resourcePath, TransportVersionDefinition::fromString); + } + + /** Test whether the given named definition exists */ + boolean namedDefinitionExists(String name) { + return Files.exists(transportResourcesDir.resolve(getNamedDefinitionRelativePath(name))); + } + + /** Return the path within the repository of the given named definition */ + Path getNamedDefinitionRepositoryPath(TransportVersionDefinition definition) { + return rootDir.relativize(transportResourcesDir.resolve(getNamedDefinitionRelativePath(definition.name()))); + } + + // return the path, relative to the resources dir, of an unreferenced definition + private Path getUnreferencedDefinitionRelativePath(String name) { + return UNREFERENCED_DIR.resolve(name + ".csv"); + } + + /** Return all unreferenced definitions, mapped by their name. */ + Map getUnreferencedDefinitions() throws IOException { + return readDefinitions(transportResourcesDir.resolve(UNREFERENCED_DIR)); + } + + /** Get a named definition from main if it exists there, or null otherwise */ + TransportVersionDefinition getUnreferencedDefinitionFromMain(String name) { + String resourcePath = getUnreferencedDefinitionRelativePath(name).toString(); + return getMainFile(resourcePath, TransportVersionDefinition::fromString); + } + + /** Return the path within the repository of the given named definition */ + Path getUnreferencedDefinitionRepositoryPath(TransportVersionDefinition definition) { + return rootDir.relativize(transportResourcesDir.resolve(getUnreferencedDefinitionRelativePath(definition.name()))); + } + + /** Read all latest files and return them mapped by their release branch */ + Map getLatestByReleaseBranch() throws IOException { + Map latests = new HashMap<>(); + try (var stream = Files.list(transportResourcesDir.resolve(LATEST_DIR))) { + for (var latestFile : stream.toList()) { + String contents = Files.readString(latestFile, StandardCharsets.UTF_8).strip(); + var latest = TransportVersionLatest.fromString(latestFile.getFileName().toString(), contents); + latests.put(latest.name(), latest); + } + } + return latests; + } + + /** Retrieve the latest transport version for the given release branch on main */ + TransportVersionLatest getLatestFromMain(String releaseBranch) { + String resourcePath = getLatestRelativePath(releaseBranch).toString(); + return getMainFile(resourcePath, TransportVersionLatest::fromString); + } + + /** Return the path within the repository of the given latest */ + Path getLatestRepositoryPath(TransportVersionLatest latest) { + return rootDir.relativize(transportResourcesDir.resolve(getLatestRelativePath(latest.branch()))); + } + + private Path getLatestRelativePath(String releaseBranch) { + return LATEST_DIR.resolve(releaseBranch + ".csv"); + } + + // Return the transport version resources paths that exist in main + private Set getMainResources() { + if (mainResources.get() == null) { + synchronized (mainResources) { + String output = gitCommand("ls-tree", "--name-only", "-r", "main", "."); + + HashSet resources = new HashSet<>(); + Collections.addAll(resources, output.split(System.lineSeparator())); + mainResources.set(resources); + } + } + return mainResources.get(); + } + + // Return the transport version resources paths that have been changed relative to main + private Set getChangedResources() { + if (changedResources.get() == null) { + synchronized (changedResources) { + String output = gitCommand("diff", "--name-only", "main", "."); + + HashSet resources = new HashSet<>(); + Collections.addAll(resources, output.split(System.lineSeparator())); + changedResources.set(resources); + } + } + return changedResources.get(); + } + + // Read a transport version resource from the main branch, or return null if it doesn't exist on main + private T getMainFile(String resourcePath, BiFunction parser) { + if (getMainResources().contains(resourcePath) == false) { + return null; + } + + String content = gitCommand("show", "main:." + File.separator + resourcePath).strip(); + return parser.apply(resourcePath, content); + } + + private static Map readDefinitions(Path dir) throws IOException { + if (Files.isDirectory(dir) == false) { + return Map.of(); + } + Map definitions = new HashMap<>(); + try (var definitionsStream = Files.list(dir)) { + for (var definitionFile : definitionsStream.toList()) { + String contents = Files.readString(definitionFile, StandardCharsets.UTF_8).strip(); + var definition = TransportVersionDefinition.fromString(definitionFile.getFileName().toString(), contents); + definitions.put(definition.name(), definition); + } + } + return definitions; + } + + // run a git command, relative to the transport version resources directory + private String gitCommand(String... args) { + ByteArrayOutputStream stdout = new ByteArrayOutputStream(); + + List command = new ArrayList<>(); + Collections.addAll(command, "git", "-C", getTransportResourcesDir().toString()); + Collections.addAll(command, args); + + ExecResult result = getExecOperations().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); + } +} 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 deleted file mode 100644 index 0aa2b173d2466..0000000000000 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * 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.gradle.api.Project; -import org.gradle.api.file.Directory; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; - -class TransportVersionUtils { - - static Path definitionFilePath(Directory resourcesDirectory, String name) { - return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve("named/" + name + ".csv"); - } - - static Path latestFilePath(Directory resourcesDirectory, String name) { - return getLatestDirectory(resourcesDirectory).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 Directory getDefinitionsDirectory(Directory resourcesDirectory) { - return resourcesDirectory.dir("definitions"); - } - - static Directory getLatestDirectory(Directory resourcesDirectory) { - return resourcesDirectory.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"); - } -} 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 2a19900076ec7..2ddfeb2f4d060 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 @@ -10,8 +10,9 @@ package org.elasticsearch.gradle.internal.transport; import org.gradle.api.DefaultTask; -import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.RegularFileProperty; +import org.gradle.api.provider.Property; +import org.gradle.api.services.ServiceReference; import org.gradle.api.tasks.CacheableTask; import org.gradle.api.tasks.InputDirectory; import org.gradle.api.tasks.InputFile; @@ -21,9 +22,7 @@ import org.gradle.api.tasks.TaskAction; import java.io.IOException; -import java.nio.file.Files; import java.nio.file.Path; -import java.util.function.Predicate; /** * Validates that each transport version named reference has a constant definition. @@ -31,10 +30,15 @@ @CacheableTask public abstract class ValidateTransportVersionReferencesTask extends DefaultTask { + @ServiceReference("transportVersionResources") + abstract Property getTransportResources(); + @InputDirectory @Optional @PathSensitive(PathSensitivity.RELATIVE) - public abstract DirectoryProperty getDefinitionsDirectory(); + public Path getDefinitionsDir() { + return getTransportResources().get().getDefinitionsDir(); + } @InputFile @PathSensitive(PathSensitivity.RELATIVE) @@ -42,17 +46,11 @@ public abstract class ValidateTransportVersionReferencesTask extends DefaultTask @TaskAction public void validateTransportVersions() throws IOException { - final Predicate referenceChecker; - if (getDefinitionsDirectory().isPresent()) { - Path definitionsDir = getDefinitionsDirectory().getAsFile().get().toPath(); - referenceChecker = (name) -> Files.exists(definitionsDir.resolve("named/" + name + ".csv")); - } else { - referenceChecker = (name) -> false; - } Path namesFile = getReferencesFile().get().getAsFile().toPath(); + TransportVersionResourcesService resources = getTransportResources().get(); for (var tvReference : TransportVersionReference.listFromFile(namesFile)) { - if (referenceChecker.test(tvReference.name()) == false) { + if (resources.namedDefinitionExists(tvReference.name()) == false) { throw new RuntimeException( "TransportVersion.fromName(\"" + tvReference.name() diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java index 5fadf524ee355..19667405e8dbd 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java @@ -13,7 +13,8 @@ import org.gradle.api.DefaultTask; import org.gradle.api.file.ConfigurableFileCollection; -import org.gradle.api.file.DirectoryProperty; +import org.gradle.api.provider.Property; +import org.gradle.api.services.ServiceReference; import org.gradle.api.tasks.CacheableTask; import org.gradle.api.tasks.InputDirectory; import org.gradle.api.tasks.InputFiles; @@ -21,34 +22,18 @@ 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.Collection; 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.TransportVersionReference.listFromFile; -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; - /** * Validates that each defined transport version constant is referenced by at least one project. */ @@ -58,7 +43,9 @@ public abstract class ValidateTransportVersionResourcesTask extends DefaultTask @InputDirectory @Optional @PathSensitive(PathSensitivity.RELATIVE) - public abstract DirectoryProperty getResourcesDirectory(); + public Path getResourcesDir() { + return getResources().get().getTransportResourcesDir(); + } @InputFiles @PathSensitive(PathSensitivity.RELATIVE) @@ -68,208 +55,156 @@ private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+"); - private final Path rootPath; - private final ExecOperations execOperations; - - // 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 ValidateTransportVersionResourcesTask(ExecOperations execOperations) { - this.execOperations = execOperations; - this.rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().toPath(); - } + @ServiceReference("transportVersionResources") + abstract Property getResources(); @TaskAction public void validateTransportVersions() throws IOException { - Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath(); - Path definitionsDir = resourcesDir.resolve("definitions"); - 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()) { - listFromFile(referencesFile.toPath()).stream().map(TransportVersionReference::name).forEach(allNames::add); + TransportVersionResourcesService resources = getResources().get(); + Set referencedNames = TransportVersionReference.collectNames(getReferencesFiles()); + Map namedDefinitions = resources.getNamedDefinitions(); + Map unreferencedDefinitions = resources.getUnreferencedDefinitions(); + Map allDefinitions = collectAllDefinitions(namedDefinitions, unreferencedDefinitions); + Map> idsByBase = collectIdsByBase(allDefinitions.values()); + Map latestByReleaseBranch = resources.getLatestByReleaseBranch(); + + for (var definition : namedDefinitions.values()) { + validateNamedDefinition(definition, referencedNames); } - // now load all definitions, do some validation and record them by various keys for later quick lookup - // NOTE: this must run after loading referenced names and existing definitions - // NOTE: this is sorted so that the order of cross validation is deterministic - for (String subDirName : List.of("unreferenced", "named")) { - Path subDir = definitionsDir.resolve(subDirName); - if (Files.isDirectory(subDir)) { - try (var definitionsStream = Files.list(subDir).sorted()) { - for (var definitionFile : definitionsStream.toList()) { - recordAndValidateDefinition(readDefinitionFile(definitionFile)); - } - } - } + for (var definition : unreferencedDefinitions.values()) { + validateUnreferencedDefinition(definition); } - // 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()); + validateBase(entry.getKey(), entry.getValue()); + } + + for (var latest : latestByReleaseBranch.values()) { + validateLatest(latest, allDefinitions, idsByBase); } + } - // 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 Map collectAllDefinitions( + Map namedDefinitions, + Map unreferencedDefinitions + ) { + Map allDefinitions = new HashMap<>(namedDefinitions); + for (var entry : unreferencedDefinitions.entrySet()) { + TransportVersionDefinition existing = allDefinitions.put(entry.getKey(), entry.getValue()); + if (existing != null) { + Path unreferencedPath = getResources().get().getUnreferencedDefinitionRepositoryPath(entry.getValue()); + throwDefinitionFailure(existing, "has same name as unreferenced definition [" + unreferencedPath + "]"); } } + return allDefinitions; } - private String gitCommand(String... args) { - final ByteArrayOutputStream stdout = new ByteArrayOutputStream(); - - List command = new ArrayList<>(); - Collections.addAll(command, "git", "-C", rootPath.toAbsolutePath().toString()); - 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) - ); + private Map> collectIdsByBase(Collection definitions) { + Map> idsByBase = new HashMap<>(); + + // first collect all ids, organized by base + for (TransportVersionDefinition definition : definitions) { + for (TransportVersionId id : definition.ids()) { + idsByBase.computeIfAbsent(id.base(), k -> new ArrayList<>()).add(new IdAndDefinition(id, definition)); + } } - return stdout.toString(StandardCharsets.UTF_8); - } + // now sort the ids within each base so we can check density later + for (var ids : idsByBase.values()) { + // 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())); + } - 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())); + return idsByBase; } - 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)); - } + private void validateNamedDefinition(TransportVersionDefinition definition, Set referencedNames) { // validate any modifications Map existingIdsByBase = new HashMap<>(); - TransportVersionDefinition originalDefinition = readExistingDefinition(definition.name()); + TransportVersionDefinition originalDefinition = getResources().get().getNamedDefinitionFromMain(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); - } - + validateIdenticalPrimaryId(definition, originalDefinition); originalDefinition.ids().forEach(id -> existingIdsByBase.put(id.base(), id)); } - if (allNames.contains(definition.name()) == false && definition.name().startsWith("initial_") == false) { - throwDefinitionFailure(definition.name(), "is not referenced"); + if (referencedNames.contains(definition.name()) == false) { + throwDefinitionFailure(definition, "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"); + throwDefinitionFailure(definition, "does not have a valid name, must be lowercase alphanumeric and underscore"); } if (definition.ids().isEmpty()) { - throwDefinitionFailure(definition.name(), "does not contain any ids"); + throwDefinitionFailure(definition, "does not contain any ids"); } - if (Comparators.isInOrder(definition.ids(), Comparator.reverseOrder()) == false) { - throwDefinitionFailure(definition.name(), "does not have ordered ids"); + if (Comparators.isInOrder(definition.ids(), Comparator.naturalOrder()) == false) { + throwDefinitionFailure(definition, "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 " + id + " already defined in [" + definitionRelativePath(existing) + "]" - ); - } - if (ndx == 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"); + if (id.patch() != 0) { + throwDefinitionFailure(definition, "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"); + throwDefinitionFailure(definition, "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); + throwDefinitionFailure(definition, "modifies existing patch id from " + maybeModifiedId + " to " + id); } } } - private TransportVersionDefinition readExistingDefinition(String name) { - return readExistingFile(name, this::definitionRelativePath, TransportVersionDefinition::fromString); + private void validateUnreferencedDefinition(TransportVersionDefinition definition) { + TransportVersionDefinition originalDefinition = getResources().get().getUnreferencedDefinitionFromMain(definition.name()); + if (originalDefinition != null) { + validateIdenticalPrimaryId(definition, originalDefinition); + } + if (definition.ids().isEmpty()) { + throwDefinitionFailure(definition, "does not contain any ids"); + } + if (definition.ids().size() > 1) { + throwDefinitionFailure(definition, " contains more than one id"); + } + // note: no name validation, anything that is a valid filename is ok, this allows eg initial_8.9.1 } - private TransportVersionLatest readExistingLatest(String branch) { - return readExistingFile(branch, this::latestRelativePath, TransportVersionLatest::fromString); - } + private void validateIdenticalPrimaryId(TransportVersionDefinition definition, TransportVersionDefinition originalDefinition) { + assert definition.name().equals(originalDefinition.name()); - private T readExistingFile(String name, Function pathFunction, BiFunction parser) { - String relativePath = pathFunction.apply(name); - if (existingResources.contains(relativePath) == false) { - return null; + int primaryId = definition.ids().get(0).complete(); + int originalPrimaryId = originalDefinition.ids().get(0).complete(); + if (primaryId != originalPrimaryId) { + throwDefinitionFailure(definition, "has modified primary id from " + originalPrimaryId + " to " + primaryId); } - String content = gitCommand("show", "main:" + relativePath).strip(); - return parser.apply(relativePath, content); } - private void recordAndValidateLatest(TransportVersionLatest latest) { - latestByBranch.put(latest.branch(), latest); - + private void validateLatest( + TransportVersionLatest latest, + Map definitions, + Map> idsByBase + ) { TransportVersionDefinition latestDefinition = definitions.get(latest.name()); if (latestDefinition == null) { - throwLatestFailure(latest.branch(), "contains transport version name [" + latest.name() + "] which is not defined"); + throwLatestFailure(latest, "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()) + "]" - ); + Path relativePath = getResources().get().getNamedDefinitionRepositoryPath(latestDefinition); + throwLatestFailure(latest, "has id " + latest.id() + " which is not in definition [" + relativePath + "]"); } List baseIds = idsByBase.get(latest.id().base()); IdAndDefinition lastId = baseIds.get(baseIds.size() - 1); if (lastId.id().complete() != latest.id().complete()) { throwLatestFailure( - latest.branch(), + latest, "has id " + latest.id() + " from [" @@ -284,49 +219,44 @@ private void recordAndValidateLatest(TransportVersionLatest latest) { ); } - TransportVersionLatest existingLatest = readExistingLatest(latest.branch()); + TransportVersionLatest existingLatest = getResources().get().getLatestFromMain(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()); + throwLatestFailure(latest, "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())); - + private void validateBase(int base, List ids) { // TODO: switch this to a fully dense check once all existing transport versions have been migrated IdAndDefinition previous = ids.get(ids.size() - 1); 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()) { + IdAndDefinition current = ids.get(ndx); + + if (previous.id().equals(current.id())) { + Path existingDefinitionPath = getResources().get().getNamedDefinitionRepositoryPath(previous.definition); + throwDefinitionFailure( + current.definition(), + "contains id " + current.id + " already defined in [" + existingDefinitionPath + "]" + ); + } + + if (previous.id().complete() - 1 != current.id().complete()) { throw new IllegalStateException( - "Transport version base id " + base + " is missing patch ids between " + next.id() + " and " + previous.id() + "Transport version base id " + base + " is missing patch ids between " + current.id() + " and " + previous.id() ); } - previous = next; + previous = current; } } - 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(getResourcesDirectory().get(), name)); - } - - private String latestRelativePath(String branch) { - return relativePath(latestFilePath(getResourcesDirectory().get(), branch)); + private void throwDefinitionFailure(TransportVersionDefinition definition, String message) { + Path relativePath = getResources().get().getNamedDefinitionRepositoryPath(definition); + throw new IllegalStateException("Transport version definition file [" + relativePath + "] " + message); } - private String relativePath(Path file) { - return rootPath.relativize(file).toString(); + private void throwLatestFailure(TransportVersionLatest latest, String message) { + Path relativePath = getResources().get().getLatestRepositoryPath(latest); + throw new IllegalStateException("Latest transport version file [" + relativePath + "] " + message); } }