Skip to content

Commit 59ffce9

Browse files
JVerwolfJVerwolf
committed
iter
1 parent 4e19254 commit 59ffce9

File tree

2 files changed

+101
-46
lines changed

2 files changed

+101
-46
lines changed

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

Lines changed: 75 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
package org.elasticsearch.gradle.internal.transport;
1111

12+
import org.elasticsearch.gradle.VersionProperties;
13+
import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.MajorMinor;
1214
import org.gradle.api.DefaultTask;
1315
import org.gradle.api.file.RegularFileProperty;
1416
import org.gradle.api.provider.ListProperty;
@@ -17,17 +19,18 @@
1719
import org.gradle.api.tasks.InputDirectory;
1820
import org.gradle.api.tasks.Optional;
1921
import org.gradle.api.tasks.TaskAction;
22+
import org.gradle.api.tasks.options.Option;
2023

2124
import java.io.IOException;
2225
import java.nio.file.Path;
2326
import java.util.ArrayList;
27+
import java.util.Collections;
2428
import java.util.Comparator;
25-
import java.util.HashSet;
2629
import java.util.List;
2730
import java.util.Objects;
28-
import java.util.function.Function;
2931

30-
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.IdIncrement;
32+
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.IdIncrement.PATCH;
33+
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.IdIncrement.SERVER;
3134
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinedFile;
3235
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getLatestId;
3336
import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getPriorLatestId;
@@ -56,16 +59,14 @@ public abstract class GenerateTransportVersionDataTask extends DefaultTask {
5659
*
5760
* @return
5861
*/
59-
@Optional
6062
@InputDirectory
61-
public abstract RegularFileProperty getDataFileDirectory();
63+
public abstract RegularFileProperty getDataFileDirectory(); // The plugin should always set this, not optional
6264

6365
/**
6466
* Used to set the name of the TransportVersionSet for which a data file will be generated.
6567
*/
66-
@Optional
6768
@Input
68-
public abstract Property<String> getTVName();
69+
public abstract Property<String> getTVName(); // The plugin should always set this, not optional
6970

7071
/**
7172
* Used to set the `major.minor` release version for which the specific TransportVersion ID will be generated.
@@ -75,52 +76,85 @@ public abstract class GenerateTransportVersionDataTask extends DefaultTask {
7576
@Input
7677
public abstract ListProperty<String> getMinorVersionsForTV();
7778

79+
// TODO can the Option annotation be on getMinorVersionsForTV() so that we don't have a separate getter?
7880
@Optional
7981
@Input
80-
public abstract Property<Function<String, IdIncrement>> getIdIncrementSupplier();
82+
@Option(option = "versions", description = "The minor version(s) for which to generate IDs, e.g. -Pversions=\"9.2,9.1\"")
83+
public abstract ListProperty<String> getMinorVersionsForTVCmdLine();
84+
85+
// @Optional
86+
// @Input
87+
// public abstract Property<Function<String, IdIncrement>> getIdIncrementSupplier();
8188

8289
@TaskAction
8390
public void generateTransportVersionData() throws IOException {
8491
final Path tvDataDir = Objects.requireNonNull(getDataFileDirectory().getAsFile().get()).toPath();
85-
final var tvName = Objects.requireNonNull(getTVName().get());
86-
final var forMinorVersions = Objects.requireNonNull(getMinorVersionsForTV().get());
87-
final var idIncrementSupplier = Objects.requireNonNull(getIdIncrementSupplier().get());
92+
final String tvName = Objects.requireNonNull(getTVName().get());
93+
final var forMinorVersions = getMinorVersionsForTV().getOrElse(
94+
Objects.requireNonNull(getMinorVersionsForTVCmdLine().get())
95+
);
96+
// final var idIncrementSupplier = Objects.requireNonNull(getIdIncrementSupplier().get());
8897

8998
// TODO
90-
// - do we need to also validate that the minorVersions don't contain duplicates here? How do we enforce idempotency if we don't?
91-
// - is there an order we need to apply? ( I don't think so)
92-
// - Do we need to run this iteratively for backport construction, rather than accepting a list like this? (I don't think so)
93-
// - also parse args if run alone
94-
// - check that duplicate tags don't come in too
99+
// - [x] do we need to also validate that the minorVersions don't contain duplicates here? How do we enforce idempotency if we don't?
100+
// - is there an order we need to apply? ( I don't think so)
101+
// - Do we need to run this iteratively for backport construction, rather than accepting a list like this? (I don't think so)
102+
// - [x] parse args if run alone
103+
// - check that duplicate versions don't come in?
104+
// - Check that we don't have duplicate names (elsewhere, not here)
105+
// - Do we need to allow creating only patch versions?
106+
// - Must also keep data in sync for removal.
107+
// - We could remove any TVs not associated with a version arg. We then either generate or keep any tvs
108+
// for each version arg, and discard the rest
109+
// - How will this work for follow-up backport PRs that will not have all the version labels?
110+
// - The follow up PR somehow needs to know original IDs. Look at git? Need a new task?
111+
// -
95112

96113
// Load the tvSetData for the specified name, if it exists
97114
final var tvDefinition = getDefinedFile(tvDataDir, tvName);
98115
boolean tvDefinitionExists = tvDefinition != null;
99-
final List<Integer> definitionIds = tvDefinitionExists ? tvDefinition.ids() : List.of();
116+
final List<Integer> preexistingIds = tvDefinitionExists ? Collections.unmodifiableList(tvDefinition.ids()) : List.of();
100117

101-
var seenIds = new HashSet<Integer>();
102-
var ids = new ArrayList<>(definitionIds);
103-
for (var forMinorVersion : forMinorVersions) {
118+
List<Integer> ids = new ArrayList<>();
119+
for (var forVersion : forMinorVersions.stream().map(MajorMinor::of).toList()) {
104120
// Get the latest transport version data for the specified minor version.
105-
final int latestTV = getLatestId(tvDataDir, forMinorVersion);
106-
107-
// Create the new version
108-
final int newVersion = idIncrementSupplier.apply(forMinorVersion).bumpVersionNumber(latestTV);
109-
110-
// Check that we don't already have an ID for this minor version
111-
var priorLatestID = getPriorLatestId(tvDataDir, forMinorVersion);
112-
if (containsValueInRange(priorLatestID, newVersion, ids)) {
121+
final int latestTV = getLatestId(tvDataDir, forVersion.toString());
122+
123+
// Create the new version id
124+
// final int newID = idIncrementSupplier.apply(forVersion).bumpTransportVersion(latestTV);
125+
final int newID = incrementTVId(latestTV, forVersion);
126+
127+
// Check that if we already have a TV ID for this minor version
128+
Integer preexistingTVId = retrieveValueInRange(
129+
getPriorLatestId(tvDataDir, forVersion.toString()),
130+
newID, preexistingIds
131+
);
132+
if (preexistingTVId != null) {
133+
ids.add(preexistingTVId);
113134
// TODO: Should we log something here?
114-
continue;
135+
} else {
136+
ids.add(newID);
137+
// Update the LATEST file.
138+
// TODO need to revert the latest files for anything that has been removed.
139+
updateLatestFile(tvDataDir, forVersion.toString(), tvName, newID);
115140
}
116-
117-
// Update the LATEST file.
118-
updateLatestFile(tvDataDir, forMinorVersion, tvName, newVersion);
119141
}
120142

121143
writeDefinitionFile(tvDataDir, tvName, ids.stream().sorted(Comparator.reverseOrder()).toList());
122144
}
123145

146+
private int incrementTVId(int tvID, MajorMinor version) {
147+
// We can only run this task on main, so the ElasticsearchVersion will be for main.
148+
final var mainVersion = MajorMinor.of(VersionProperties.getElasticsearchVersion());
149+
final var isMain = version.equals(mainVersion);
150+
if (isMain) {
151+
return SERVER.bumpTransportVersion(tvID);
152+
} else {
153+
return PATCH.bumpTransportVersion(tvID);
154+
}
155+
// TODO add serverless check
156+
}
157+
124158
private boolean containsValueInRange(int lowerExclusive, int upperInclusive, List<Integer> ids) {
125159
for (var id : ids) {
126160
if (lowerExclusive < id && id <= upperInclusive) {
@@ -129,4 +163,13 @@ private boolean containsValueInRange(int lowerExclusive, int upperInclusive, Lis
129163
}
130164
return false;
131165
}
166+
167+
private Integer retrieveValueInRange(int lowerExclusive, int upperInclusive, List<Integer> ids) {
168+
for (var id : ids) {
169+
if (lowerExclusive < id && id <= upperInclusive) {
170+
return id;
171+
}
172+
}
173+
return null;
174+
}
132175
}

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

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@
1010
package org.elasticsearch.gradle.internal.transport;
1111

1212
import com.google.common.collect.Comparators;
13-
13+
import org.elasticsearch.gradle.Version;
1414
import org.gradle.api.GradleException;
1515
import org.gradle.api.Project;
1616
import org.gradle.api.attributes.Attribute;
1717
import org.gradle.api.attributes.AttributeContainer;
1818
import org.gradle.api.file.Directory;
19+
import org.jetbrains.annotations.NotNull;
1920

2021
import java.io.IOException;
2122
import java.nio.charset.StandardCharsets;
@@ -154,7 +155,7 @@ public enum IdIncrement {
154155
this.max = (int) Math.pow(10, numDigits);
155156
}
156157

157-
public int bumpVersionNumber(int tvIDToBump) {
158+
public int bumpTransportVersion(int tvIDToBump) {
158159
int zeroesCleared = (tvIDToBump / value) * value;
159160
int newId = zeroesCleared + value;
160161
if ((newId / value) % max == 0) {
@@ -166,24 +167,35 @@ public int bumpVersionNumber(int tvIDToBump) {
166167
}
167168
}
168169

169-
public static int getPriorLatestId(Path dataDir, String majorMinor) throws IOException {
170-
var pattern = Pattern.compile("^(\\d+)\\.(\\d+)\\.csv$");
170+
public record MajorMinor(int major, int minor) {
171+
public static MajorMinor of(String majorMinor) {
172+
String[] versionParts = majorMinor.split("\\.");
173+
assert versionParts.length == 2;
174+
return new MajorMinor(Integer.parseInt(versionParts[0]), Integer.parseInt(versionParts[1]));
175+
}
171176

172-
var matcher = pattern.matcher(majorMinor);
173-
var major = Integer.parseInt(matcher.group(1));
174-
var minor = Integer.parseInt(matcher.group(2));
175-
if (minor > 0) {
176-
return getLatestId(dataDir, major + "." + (minor - 1));
177+
public static MajorMinor of(Version version) {
178+
return new MajorMinor(version.getMajor(), version.getMinor());
177179
}
178180

181+
@Override
182+
public @NotNull String toString() {
183+
return major + "." + minor;
184+
}
185+
}
186+
187+
public static int getPriorLatestId(Path dataDir, String majorMinor) throws IOException {
188+
var version = MajorMinor.of(majorMinor);
189+
if (version.minor() > 0) {
190+
return getLatestId(dataDir, version.major + "." + (version.minor - 1));
191+
}
179192
try (var pathStream = Files.list(Objects.requireNonNull(dataDir.resolve(LATEST_DIR)))) {
180193
var highestMinorOfPrevMajor = pathStream.flatMap(path -> {
181-
var m = pattern.matcher(path.getFileName().toString());
182-
var fileMajor = Integer.parseInt(m.group(1));
183-
var fileMinor = Integer.parseInt(m.group(2));
184-
return fileMajor == major - 1 ? Stream.of(fileMinor) : Stream.empty();
194+
var fileMajorMinor = path.getFileName().toString().replace(".csv", "");
195+
var fileVersion = MajorMinor.of(fileMajorMinor);
196+
return fileVersion.major == version.major - 1 ? Stream.of(fileVersion.minor) : Stream.empty();
185197
}).sorted().toList().getLast();
186-
return getLatestId(dataDir, (major - 1) + "." + highestMinorOfPrevMajor);
198+
return getLatestId(dataDir, (version.major - 1) + "." + highestMinorOfPrevMajor);
187199
}
188200
}
189201

0 commit comments

Comments
 (0)