Skip to content

Commit 2ec1317

Browse files
jdconradjdconrad
committed
Update transport version directory structure (elastic#132891)
This changes the directory structure for transport version files to the following: /transport/latest /transport/definitions/named /transport/definitions/initial /transport/definitions/manifest.txt This allows manifest to be placed in a location for named and initial transport versions without adding confusion to latest. ES-12401
1 parent 3a70119 commit 2ec1317

File tree

10 files changed

+77
-53
lines changed

10 files changed

+77
-53
lines changed

‎.gitignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,4 @@ testfixtures_shared/
6969
# Generated
7070
checkstyle_ide.xml
7171
x-pack/plugin/esql/src/main/generated-src/generated/
72-
server/src/main/resources/transport/defined/manifest.txt
72+
server/src/main/resources/transport/definitions/manifest.txt

‎build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,12 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
4242
}
4343
}
4444

45-
def definedTransportVersion(String name, String ids) {
46-
javaResource("myserver", "transport/defined/" + name + ".csv", ids)
45+
def namedTransportVersion(String name, String ids) {
46+
javaResource("myserver", "transport/definitions/named/" + name + ".csv", ids)
47+
}
48+
49+
def initialTransportVersion(String name, String id) {
50+
javaResource("myserver", "transport/definitions/initial/" + name + ".csv", id)
4751
}
4852

4953
def definedAndUsedTransportVersion(String name, String ids) {
@@ -54,7 +58,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
5458
javaSource("myserver", "org.elasticsearch", classname, "", """
5559
static final TransportVersion usage = TransportVersion.fromName("${name}");
5660
""")
57-
definedTransportVersion(name, ids)
61+
namedTransportVersion(name, ids)
5862
}
5963

6064
def latestTransportVersion(String branch, String name, String id) {
@@ -95,8 +99,9 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
9599
apply plugin: 'elasticsearch.transport-version-references'
96100
apply plugin: 'elasticsearch.transport-version-resources'
97101
"""
98-
definedTransportVersion("existing_91", "8012000")
99-
definedTransportVersion("existing_92", "8123000,8012001")
102+
namedTransportVersion("existing_91", "8012000")
103+
namedTransportVersion("existing_92", "8123000,8012001")
104+
initialTransportVersion("initial_9_0_0", "8000000")
100105
latestTransportVersion("9.2", "existing_92", "8123000")
101106
latestTransportVersion("9.1", "existing_92", "8012001")
102107
// a mock version of TransportVersion, just here so we can compile Dummy.java et al
@@ -148,12 +153,12 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
148153

149154
def "references must be defined"() {
150155
given:
151-
definedTransportVersion("not_used", "1000000")
156+
namedTransportVersion("not_used", "1000000")
152157
when:
153158
def result = validateDefinitionsFails()
154159
then:
155160
assertDefinitionsFailure(result, "Transport version definition file " +
156-
"[myserver/src/main/resources/transport/defined/not_used.csv] is not referenced")
161+
"[myserver/src/main/resources/transport/definitions/named/not_used.csv] is not referenced")
157162
}
158163

159164
def "names must be lowercase alphanum or underscore"() {
@@ -163,7 +168,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
163168
def result = validateDefinitionsFails()
164169
then:
165170
assertDefinitionsFailure(result, "Transport version definition file " +
166-
"[myserver/src/main/resources/transport/defined/${name}.csv] does not have a valid name, " +
171+
"[myserver/src/main/resources/transport/definitions/named/${name}.csv] does not have a valid name, " +
167172
"must be lowercase alphanumeric and underscore")
168173

169174
where:
@@ -177,7 +182,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
177182
def result = validateDefinitionsFails()
178183
then:
179184
assertDefinitionsFailure(result, "Transport version definition file " +
180-
"[myserver/src/main/resources/transport/defined/empty.csv] does not contain any ids")
185+
"[myserver/src/main/resources/transport/definitions/named/empty.csv] does not contain any ids")
181186
}
182187

183188
def "definitions have ids in descending order"() {
@@ -187,7 +192,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
187192
def result = validateDefinitionsFails()
188193
then:
189194
assertDefinitionsFailure(result, "Transport version definition file " +
190-
"[myserver/src/main/resources/transport/defined/out_of_order.csv] does not have ordered ids")
195+
"[myserver/src/main/resources/transport/definitions/named/out_of_order.csv] does not have ordered ids")
191196
}
192197

193198
def "definition ids are unique"() {
@@ -197,8 +202,8 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
197202
def result = validateDefinitionsFails()
198203
then:
199204
assertDefinitionsFailure(result, "Transport version definition file " +
200-
"[myserver/src/main/resources/transport/defined/existing_92.csv] contains id 8123000 already defined in " +
201-
"[myserver/src/main/resources/transport/defined/duplicate.csv]")
205+
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv] contains id 8123000 already defined in " +
206+
"[myserver/src/main/resources/transport/definitions/named/duplicate.csv]")
202207
}
203208

204209
def "definitions have bwc ids with non-zero patch part"() {
@@ -208,27 +213,27 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
208213
def result = validateDefinitionsFails()
209214
then:
210215
assertDefinitionsFailure(result, "Transport version definition file " +
211-
"[myserver/src/main/resources/transport/defined/patched.csv] contains bwc id [8100000] with a patch part of 0")
216+
"[myserver/src/main/resources/transport/definitions/named/patched.csv] contains bwc id [8100000] with a patch part of 0")
212217
}
213218

214219
def "definitions have primary ids which cannot change"() {
215220
given:
216-
definedTransportVersion("existing_92", "8500000")
221+
namedTransportVersion("existing_92", "8500000")
217222
when:
218223
def result = validateDefinitionsFails()
219224
then:
220225
assertDefinitionsFailure(result, "Transport version definition file " +
221-
"[myserver/src/main/resources/transport/defined/existing_92.csv] has modified primary id from 8123000 to 8500000")
226+
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv] has modified primary id from 8123000 to 8500000")
222227
}
223228

224229
def "cannot change committed ids to a branch"() {
225230
given:
226-
definedTransportVersion("existing_92", "8123000,8012002")
231+
namedTransportVersion("existing_92", "8123000,8012002")
227232
when:
228233
def result = validateDefinitionsFails()
229234
then:
230235
assertDefinitionsFailure(result, "Transport version definition file " +
231-
"[myserver/src/main/resources/transport/defined/existing_92.csv] modifies existing patch id from 8012001 to 8012002")
236+
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv] modifies existing patch id from 8012001 to 8012002")
232237
}
233238

234239
def "latest files must reference defined name"() {
@@ -249,7 +254,7 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
249254
then:
250255
assertDefinitionsFailure(result, "Latest transport version file " +
251256
"[myserver/src/main/resources/transport/latest/9.2.csv] has id 8124000 which is not in definition " +
252-
"[myserver/src/main/resources/transport/defined/existing_92.csv]")
257+
"[myserver/src/main/resources/transport/definitions/named/existing_92.csv]")
253258
}
254259

255260
def "latest files have latest id within base"() {
@@ -296,6 +301,6 @@ class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
296301
def result = validateDefinitionsFails()
297302
then:
298303
assertDefinitionsFailure(result, "Transport version definition file " +
299-
"[myserver/src/main/resources/transport/defined/patch.csv] has patch version 8015001 as primary id")
304+
"[myserver/src/main/resources/transport/definitions/named/patch.csv] has patch version 8015001 as primary id")
300305
}
301306
}

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@
2121

2222
import java.io.IOException;
2323
import java.nio.charset.StandardCharsets;
24+
import java.nio.file.FileVisitResult;
2425
import java.nio.file.Files;
2526
import java.nio.file.Path;
27+
import java.nio.file.SimpleFileVisitor;
28+
import java.nio.file.attribute.BasicFileAttributes;
2629

2730
public abstract class GenerateTransportVersionManifestTask extends DefaultTask {
2831
@InputDirectory
@@ -44,15 +47,14 @@ public void generateTransportVersionManifest() throws IOException {
4447
}
4548

4649
try (var writer = Files.newBufferedWriter(manifestFile)) {
47-
try (var stream = Files.list(definitionsDir)) {
48-
for (String filename : stream.map(p -> p.getFileName().toString()).toList()) {
49-
if (filename.equals(manifestFile.getFileName().toString())) {
50-
// don't list self
51-
continue;
52-
}
53-
writer.write(filename + "\n");
50+
Files.walkFileTree(definitionsDir, new SimpleFileVisitor<>() {
51+
@Override
52+
public FileVisitResult visitFile(Path path, BasicFileAttributes attrs) throws IOException {
53+
String subPath = definitionsDir.relativize(path).toString().replace('\\', '/');
54+
writer.write(subPath + "\n");
55+
return FileVisitResult.CONTINUE;
5456
}
55-
}
57+
});
5658
}
5759
}
5860
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void apply(Project project) {
6464
t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt"));
6565
});
6666
project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> {
67-
t.into("transport/defined", c -> c.from(generateManifestTask));
67+
t.into("transport/definitions", c -> c.from(generateManifestTask));
6868
});
6969
}
7070
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
class TransportVersionUtils {
2121

2222
static Path definitionFilePath(Directory resourcesDirectory, String name) {
23-
return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve(name + ".csv");
23+
return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve("named/" + name + ".csv");
2424
}
2525

2626
static Path latestFilePath(Directory resourcesDirectory, String name) {
@@ -38,7 +38,7 @@ static TransportVersionLatest readLatestFile(Path file) throws IOException {
3838
}
3939

4040
static Directory getDefinitionsDirectory(Directory resourcesDirectory) {
41-
return resourcesDirectory.dir("defined");
41+
return resourcesDirectory.dir("definitions");
4242
}
4343

4444
static Directory getLatestDirectory(Directory resourcesDirectory) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public void validateTransportVersions() throws IOException {
4545
final Predicate<String> referenceChecker;
4646
if (getDefinitionsDirectory().isPresent()) {
4747
Path definitionsDir = getDefinitionsDirectory().getAsFile().get().toPath();
48-
referenceChecker = (name) -> Files.exists(definitionsDir.resolve(name + ".csv"));
48+
referenceChecker = (name) -> Files.exists(definitionsDir.resolve("named/" + name + ".csv"));
4949
} else {
5050
referenceChecker = (name) -> false;
5151
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public void validateTransportVersions() throws IOException {
9696
return; // no definitions to validate, remove this leniency once all branches have at least one version
9797
}
9898
Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath();
99-
Path definitionsDir = resourcesDir.resolve("defined");
99+
Path definitionsDir = resourcesDir.resolve("definitions");
100100
Path latestDir = resourcesDir.resolve("latest");
101101

102102
// first check which resource files already exist in main
@@ -110,9 +110,11 @@ public void validateTransportVersions() throws IOException {
110110
// now load all definitions, do some validation and record them by various keys for later quick lookup
111111
// NOTE: this must run after loading referenced names and existing definitions
112112
// NOTE: this is sorted so that the order of cross validation is deterministic
113-
try (var definitionsStream = Files.list(definitionsDir).sorted()) {
114-
for (var definitionFile : definitionsStream.toList()) {
115-
recordAndValidateDefinition(readDefinitionFile(definitionFile));
113+
for (String subDir : List.of("initial", "named")) {
114+
try (var definitionsStream = Files.list(definitionsDir.resolve(subDir)).sorted()) {
115+
for (var definitionFile : definitionsStream.toList()) {
116+
recordAndValidateDefinition(readDefinitionFile(definitionFile));
117+
}
116118
}
117119
}
118120

‎server/src/main/java/org/elasticsearch/TransportVersion.java‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import java.util.ArrayList;
2323
import java.util.Collection;
2424
import java.util.Collections;
25-
import java.util.HashMap;
2625
import java.util.List;
2726
import java.util.Map;
2827
import java.util.Objects;
@@ -110,6 +109,7 @@ public static TransportVersion fromBufferedReader(
110109
String component,
111110
String path,
112111
boolean nameInFile,
112+
boolean isNamed,
113113
BufferedReader bufferedReader,
114114
Integer latest
115115
) {
@@ -125,7 +125,14 @@ public static TransportVersion fromBufferedReader(
125125
if (parts.length < (nameInFile ? 2 : 1)) {
126126
throw new IllegalStateException("invalid transport version file format [" + toComponentPath(component, path) + "]");
127127
}
128-
String name = nameInFile ? parts[0] : path.substring(path.lastIndexOf('/') + 1, path.length() - 4);
128+
String name = null;
129+
if (isNamed) {
130+
if (nameInFile) {
131+
name = parts[0];
132+
} else {
133+
name = path.substring(path.lastIndexOf('/') + 1, path.length() - 4);
134+
}
135+
}
129136
List<Integer> ids = new ArrayList<>();
130137
for (int i = nameInFile ? 1 : 0; i < parts.length; ++i) {
131138
try {
@@ -153,7 +160,7 @@ public static TransportVersion fromBufferedReader(
153160
}
154161
}
155162

156-
public static Map<String, TransportVersion> collectFromInputStreams(
163+
public static List<TransportVersion> collectFromInputStreams(
157164
String component,
158165
Function<String, InputStream> nameToStream,
159166
String latestFileName
@@ -162,32 +169,32 @@ public static Map<String, TransportVersion> collectFromInputStreams(
162169
component,
163170
"/transport/latest/" + latestFileName,
164171
nameToStream,
165-
(c, p, br) -> fromBufferedReader(c, p, true, br, Integer.MAX_VALUE)
172+
(c, p, br) -> fromBufferedReader(c, p, true, false, br, Integer.MAX_VALUE)
166173
);
167174
if (latest != null) {
168-
List<String> versionFilesNames = parseFromBufferedReader(
175+
List<String> versionRelativePaths = parseFromBufferedReader(
169176
component,
170-
"/transport/defined/manifest.txt",
177+
"/transport/definitions/manifest.txt",
171178
nameToStream,
172179
(c, p, br) -> br.lines().filter(line -> line.isBlank() == false).toList()
173180
);
174-
if (versionFilesNames != null) {
175-
Map<String, TransportVersion> transportVersions = new HashMap<>();
176-
for (String versionFileName : versionFilesNames) {
181+
if (versionRelativePaths != null) {
182+
List<TransportVersion> transportVersions = new ArrayList<>();
183+
for (String versionRelativePath : versionRelativePaths) {
177184
TransportVersion transportVersion = parseFromBufferedReader(
178185
component,
179-
"/transport/defined/" + versionFileName,
186+
"/transport/definitions/" + versionRelativePath,
180187
nameToStream,
181-
(c, p, br) -> fromBufferedReader(c, p, false, br, latest.id())
188+
(c, p, br) -> fromBufferedReader(c, p, false, versionRelativePath.startsWith("named/"), br, latest.id())
182189
);
183190
if (transportVersion != null) {
184-
transportVersions.put(versionFileName.substring(0, versionFileName.length() - 4), transportVersion);
191+
transportVersions.add(transportVersion);
185192
}
186193
}
187194
return transportVersions;
188195
}
189196
}
190-
return Map.of();
197+
return List.of();
191198
}
192199

193200
private static String toComponentPath(String component, String path) {
@@ -411,12 +418,15 @@ private static class VersionsHolder {
411418
static {
412419
// collect all the transport versions from server and es modules/plugins (defined in server)
413420
List<TransportVersion> allVersions = new ArrayList<>(TransportVersions.DEFINED_VERSIONS);
414-
Map<String, TransportVersion> allVersionsByName = collectFromInputStreams(
421+
List<TransportVersion> streamVersions = collectFromInputStreams(
415422
"<server>",
416423
TransportVersion.class::getResourceAsStream,
417424
Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv"
418425
);
419-
addTransportVersions(allVersionsByName.values(), allVersions).sort(TransportVersion::compareTo);
426+
Map<String, TransportVersion> allVersionsByName = streamVersions.stream()
427+
.filter(tv -> tv.name() != null)
428+
.collect(Collectors.toMap(TransportVersion::name, v -> v));
429+
addTransportVersions(streamVersions, allVersions).sort(TransportVersion::compareTo);
420430

421431
// set the transport version lookups
422432
ALL_VERSIONS = Collections.unmodifiableList(allVersions);

‎server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv‎ renamed to ‎server/src/main/resources/transport/definitions/initial/initial_elasticsearch_8_18_5.csv‎

File renamed without changes.

‎server/src/test/java/org/elasticsearch/TransportVersionTests.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,9 @@ public void testDuplicateConstants() {
273273
public void testLatest() {
274274
TransportVersion latest = TransportVersion.parseFromBufferedReader(
275275
"<test>",
276-
"/transport/defined/" + Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv",
276+
"/transport/definitions/" + Version.CURRENT.major + "." + Version.CURRENT.minor + ".csv",
277277
TransportVersion.class::getResourceAsStream,
278-
(c, p, br) -> TransportVersion.fromBufferedReader(c, p, true, br, Integer.MAX_VALUE)
278+
(c, p, br) -> TransportVersion.fromBufferedReader(c, p, true, false, br, Integer.MAX_VALUE)
279279
);
280280
// TODO: once placeholder is removed, test the latest known version can be found fromName
281281
// assertThat(latest, is(TransportVersion.fromName(latest.name())));
@@ -287,6 +287,7 @@ public void testSupports() {
287287
"<test>",
288288
"testSupports0",
289289
false,
290+
true,
290291
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data0), StandardCharsets.UTF_8)),
291292
5000000
292293
);
@@ -299,6 +300,7 @@ public void testSupports() {
299300
"<test>",
300301
"testSupports1",
301302
false,
303+
true,
302304
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data1), StandardCharsets.UTF_8)),
303305
5000000
304306
);
@@ -314,6 +316,7 @@ public void testSupports() {
314316
"<test>",
315317
"testSupports2",
316318
false,
319+
true,
317320
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data2), StandardCharsets.UTF_8)),
318321
5000000
319322
);
@@ -341,6 +344,7 @@ public void testSupports() {
341344
"<test>",
342345
"testSupports3",
343346
false,
347+
true,
344348
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data3), StandardCharsets.UTF_8)),
345349
5000000
346350
);
@@ -369,6 +373,7 @@ public void testSupports() {
369373
"<test>",
370374
"testSupports3",
371375
false,
376+
true,
372377
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(data4), StandardCharsets.UTF_8)),
373378
5000000
374379
);

0 commit comments

Comments
 (0)