Skip to content

Commit e57cd37

Browse files
prdoyleprdoyle
committed
First pass addressing PR comments
1 parent 9949f91 commit e57cd37

File tree

4 files changed

+87
-83
lines changed

4 files changed

+87
-83
lines changed

‎build-tools/src/main/java/org/elasticsearch/gradle/plugin/GenerateTestBuildInfoTask.java‎

Lines changed: 84 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,18 @@
99

1010
package org.elasticsearch.gradle.plugin;
1111

12+
import com.fasterxml.jackson.databind.ObjectMapper;
13+
import com.fasterxml.jackson.databind.SerializationFeature;
14+
1215
import org.gradle.api.DefaultTask;
13-
import org.gradle.api.file.DirectoryProperty;
1416
import org.gradle.api.file.FileCollection;
17+
import org.gradle.api.file.RegularFileProperty;
1518
import org.gradle.api.provider.Property;
19+
import org.gradle.api.tasks.CacheableTask;
20+
import org.gradle.api.tasks.Classpath;
1621
import org.gradle.api.tasks.Input;
17-
import org.gradle.api.tasks.InputFiles;
1822
import org.gradle.api.tasks.Optional;
19-
import org.gradle.api.tasks.OutputDirectory;
23+
import org.gradle.api.tasks.OutputFile;
2024
import org.gradle.api.tasks.TaskAction;
2125
import org.objectweb.asm.ClassReader;
2226
import org.objectweb.asm.ClassVisitor;
@@ -32,9 +36,7 @@
3236
import java.nio.file.Path;
3337
import java.util.ArrayList;
3438
import java.util.Arrays;
35-
import java.util.HashMap;
3639
import java.util.List;
37-
import java.util.Map;
3840
import java.util.jar.JarEntry;
3941
import java.util.jar.JarFile;
4042
import java.util.jar.Manifest;
@@ -47,9 +49,11 @@
4749
* used to imitate modular behavior during unit tests so
4850
* entitlements can lookup correct policies.
4951
*/
52+
@CacheableTask
5053
public abstract class GenerateTestBuildInfoTask extends DefaultTask {
5154

5255
public static final String DESCRIPTION = "generates plugin test dependencies file";
56+
public static final String META_INF_VERSIONS_PREFIX = "META-INF/versions/";
5357

5458
public GenerateTestBuildInfoTask() {
5559
setDescription(DESCRIPTION);
@@ -62,82 +66,71 @@ public GenerateTestBuildInfoTask() {
6266
@Input
6367
public abstract Property<String> getComponentName();
6468

65-
@InputFiles
69+
@Classpath
6670
public abstract Property<FileCollection> getCodeLocations();
6771

68-
@Input
69-
public abstract Property<String> getOutputFileName();
70-
71-
@OutputDirectory
72-
public abstract DirectoryProperty getOutputDirectory();
72+
@OutputFile
73+
public abstract RegularFileProperty getOutputFile();
7374

7475
@TaskAction
7576
public void generatePropertiesFile() throws IOException {
76-
Map<String, String> classesToModules = buildClassesToModules();
77-
78-
Path outputDirectory = getOutputDirectory().get().getAsFile().toPath();
79-
Files.createDirectories(outputDirectory);
80-
Path outputFile = outputDirectory.resolve(getOutputFileName().get());
77+
Path outputFile = getOutputFile().get().getAsFile().toPath();
78+
Files.createDirectories(outputFile.getParent());
8179

8280
try (var writer = Files.newBufferedWriter(outputFile, StandardCharsets.UTF_8)) {
83-
writer.write("{\n");
84-
85-
writer.write(" \"name\": \"");
86-
writer.write(getComponentName().get());
87-
writer.write("\",\n");
88-
89-
writer.write(" \"locations\": [\n");
90-
if (classesToModules.isEmpty() == false) {
91-
StringBuilder sb = new StringBuilder();
92-
for (Map.Entry<String, String> entry : classesToModules.entrySet()) {
93-
sb.append(" {\n");
94-
sb.append(" \"class\": \"");
95-
sb.append(entry.getKey());
96-
sb.append("\",\n \"module\": \"");
97-
sb.append(entry.getValue());
98-
sb.append("\"\n },\n");
99-
}
100-
writer.write(sb.substring(0, sb.length() - 2));
101-
}
102-
writer.write("\n ]\n}\n");
81+
ObjectMapper mapper = new ObjectMapper().configure(SerializationFeature.INDENT_OUTPUT, true);
82+
mapper.writeValue(writer, new OutputFileContents(getComponentName().get(), buildLocationList()));
10383
}
10484
}
10585

106-
// build the association for a class to a module;
107-
// there are different methods for finding these depending on if the
108-
// classpath entry is a jar or a directory
109-
private Map<String, String> buildClassesToModules() throws IOException {
110-
Map<String, String> classesToModules = new HashMap<>();
86+
/**
87+
* The output of this task is a JSON file formatted according to this record
88+
*/
89+
record OutputFileContents(String component, List<Location> locations) {}
90+
91+
record Location(String representativeClass, String module) {}
92+
93+
/**
94+
* Build the list of {@link Location}s for all {@link #getCodeLocations() code locations}.
95+
* There are different methods for finding these depending on if the
96+
* classpath entry is a jar or a directory
97+
*/
98+
private List<Location> buildLocationList() throws IOException {
99+
List<Location> locations = new ArrayList<>();
111100
for (File file : getCodeLocations().get().getFiles()) {
112101
if (file.exists()) {
113102
if (file.getName().endsWith(".jar")) {
114-
extractFromJar(file, classesToModules);
103+
extractLocationsFromJar(file, locations);
115104
} else if (file.isDirectory()) {
116-
extractFromDirectory(file, classesToModules);
105+
extractLocationsFromDirectory(file, locations);
117106
} else {
118107
throw new IllegalArgumentException("unrecognized classpath entry: " + file);
119108
}
120109
}
121110
}
122-
return classesToModules;
111+
return List.copyOf(locations);
123112
}
124113

125-
// find the first class and module when the class path entry is a jar
126-
private void extractFromJar(File file, Map<String, String> classesToModules) throws IOException {
114+
/**
115+
* find the first class and module when the class path entry is a jar
116+
*/
117+
private void extractLocationsFromJar(File file, List<Location> locations) throws IOException {
127118
try (JarFile jarFile = new JarFile(file)) {
128-
String className = extractClassNameFromJar(jarFile);
129-
String moduleName = extractModuleNameFromJar(file, jarFile);
119+
var className = extractClassNameFromJar(jarFile);
130120

131-
if (className != null) {
132-
classesToModules.put(className, moduleName);
121+
if (className.isPresent()) {
122+
String moduleName = extractModuleNameFromJar(file, jarFile);
123+
locations.add(new Location(className.get(), moduleName));
133124
}
134125
}
135126
}
136127

137-
// look through the jar to find the first unique class that isn't
138-
// in META-INF (those may not be unique) and isn't module-info.class
139-
// (which is also not unique) and avoid anonymous classes
140-
private String extractClassNameFromJar(JarFile jarFile) {
128+
/**
129+
* look through the jar to find the first unique class that isn't
130+
* in META-INF (those may not be unique) and isn't module-info.class
131+
* (which is also not unique) and avoid anonymous classes
132+
*/
133+
private java.util.Optional<String> extractClassNameFromJar(JarFile jarFile) {
141134
return jarFile.stream()
142135
.filter(
143136
je -> je.getName().startsWith("META-INF") == false
@@ -146,12 +139,13 @@ private String extractClassNameFromJar(JarFile jarFile) {
146139
&& je.getName().endsWith(".class")
147140
)
148141
.findFirst()
149-
.map(ZipEntry::getName)
150-
.orElse(null);
142+
.map(ZipEntry::getName);
151143
}
152144

153-
// look through the jar for the module name with
154-
// each step commented inline
145+
/**
146+
* look through the jar for the module name with
147+
* each step commented inline
148+
*/
155149
private String extractModuleNameFromJar(File file, JarFile jarFile) throws IOException {
156150
String moduleName = null;
157151

@@ -162,8 +156,12 @@ private String extractModuleNameFromJar(File file, JarFile jarFile) throws IOExc
162156
// fewer than or equal to the current JVM version
163157
if (jarFile.isMultiRelease()) {
164158
List<Integer> versions = jarFile.stream()
165-
.filter(je -> je.getName().startsWith("META-INF/versions/") && je.getName().endsWith("/module-info.class"))
166-
.map(je -> Integer.parseInt(je.getName().substring(18, je.getName().length() - 18)))
159+
.filter(je -> je.getName().startsWith(META_INF_VERSIONS_PREFIX) && je.getName().endsWith("/module-info.class"))
160+
.map(
161+
je -> Integer.parseInt(
162+
je.getName().substring(META_INF_VERSIONS_PREFIX.length(), je.getName().length() - META_INF_VERSIONS_PREFIX.length())
163+
)
164+
)
167165
.toList();
168166
versions = new ArrayList<>(versions);
169167
versions.sort(Integer::compareTo);
@@ -176,7 +174,7 @@ private String extractModuleNameFromJar(File file, JarFile jarFile) throws IOExc
176174
break;
177175
}
178176
}
179-
if (path.length() > 18) {
177+
if (path.length() > META_INF_VERSIONS_PREFIX.length()) {
180178
path.append("/module-info.class");
181179
JarEntry moduleEntry = jarFile.getJarEntry(path.toString());
182180
if (moduleEntry != null) {
@@ -216,7 +214,7 @@ private String extractModuleNameFromJar(File file, JarFile jarFile) throws IOExc
216214
// if the jar does not have module-info.class and no module name in the manifest
217215
// default to the jar name without .jar and no versioning
218216
if (moduleName == null) {
219-
String jn = file.getName().substring(0, file.getName().length() - 4);
217+
String jn = file.getName().substring(0, file.getName().length() - ".jar".length());
220218
Matcher matcher = Pattern.compile("-(\\d+(\\.|$))").matcher(jn);
221219
if (matcher.find()) {
222220
jn = jn.substring(0, matcher.start());
@@ -228,27 +226,31 @@ private String extractModuleNameFromJar(File file, JarFile jarFile) throws IOExc
228226
return moduleName;
229227
}
230228

231-
// find the first class and module when the class path entry is a directory
232-
private void extractFromDirectory(File file, Map<String, String> classesToModules) throws IOException {
233-
String className = extractClassNameFromDirectory(file);
234-
String moduleName = extractModuleNameFromDirectory(file);
229+
/**
230+
* find the first class and module when the class path entry is a directory
231+
*/
232+
private void extractLocationsFromDirectory(File dir, List<Location> locations) throws IOException {
233+
String className = extractClassNameFromDirectory(dir);
234+
String moduleName = extractModuleNameFromDirectory(dir);
235235

236236
if (className != null && moduleName != null) {
237-
classesToModules.put(className, moduleName);
237+
locations.add(new Location(className, moduleName));
238238
}
239239
}
240240

241-
// look through the directory to find the first unique class that isn't
242-
// module-info.class (which may not be unique) and avoid anonymous classes
243-
private String extractClassNameFromDirectory(File file) {
244-
List<File> files = new ArrayList<>(List.of(file));
241+
/**
242+
* look through the directory to find the first unique class that isn't
243+
* module-info.class (which may not be unique) and avoid anonymous classes
244+
*/
245+
private String extractClassNameFromDirectory(File dir) {
246+
List<File> files = new ArrayList<>(List.of(dir));
245247
while (files.isEmpty() == false) {
246248
File find = files.removeFirst();
247249
if (find.exists()) {
248250
if (find.getName().endsWith(".class")
249251
&& find.getName().equals("module-info.class") == false
250252
&& find.getName().contains("$") == false) {
251-
return find.getAbsolutePath().substring(file.getAbsolutePath().length() + 1);
253+
return find.getAbsolutePath().substring(dir.getAbsolutePath().length() + 1);
252254
} else if (find.isDirectory()) {
253255
files.addAll(Arrays.asList(find.listFiles()));
254256
}
@@ -258,10 +260,12 @@ private String extractClassNameFromDirectory(File file) {
258260
return null;
259261
}
260262

261-
// look through the directory to find the module name in either module-info.class
262-
// if it exists or the preset one derived from the jar task
263-
private String extractModuleNameFromDirectory(File file) throws IOException {
264-
List<File> files = new ArrayList<>(List.of(file));
263+
/**
264+
* look through the directory to find the module name in either module-info.class
265+
* if it exists or the preset one derived from the jar task
266+
*/
267+
private String extractModuleNameFromDirectory(File dir) throws IOException {
268+
List<File> files = new ArrayList<>(List.of(dir));
265269
while (files.isEmpty() == false) {
266270
File find = files.removeFirst();
267271
if (find.exists()) {
@@ -277,8 +281,10 @@ private String extractModuleNameFromDirectory(File file) throws IOException {
277281
return getModuleName().isPresent() ? getModuleName().get() : null;
278282
}
279283

280-
// a helper method to extract the module name from module-info.class
281-
// using an ASM ClassVisitor
284+
/**
285+
* a helper method to extract the module name from module-info.class
286+
* using an ASM ClassVisitor
287+
*/
282288
private String extractModuleNameFromModuleInfo(InputStream inputStream) throws IOException {
283289
String[] moduleName = new String[1];
284290
ClassReader cr = new ClassReader(inputStream);

‎build-tools/src/main/java/org/elasticsearch/gradle/plugin/PluginBuildPlugin.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ public void apply(final Project project) {
6666
}
6767
var propertiesExtension = project.getExtensions().getByType(PluginPropertiesExtension.class);
6868
task.getComponentName().set(providerFactory.provider(propertiesExtension::getName));
69-
task.getOutputFileName().set("plugin-test-build-info.json");
7069
});
7170

7271
project.getTasks().withType(ProcessResources.class).named("processResources").configure(task -> {

‎build-tools/src/main/java/org/elasticsearch/gradle/test/TestBuildInfoPlugin.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import org.elasticsearch.gradle.plugin.GenerateTestBuildInfoTask;
1414
import org.gradle.api.Plugin;
1515
import org.gradle.api.Project;
16-
import org.gradle.api.file.Directory;
16+
import org.gradle.api.file.RegularFile;
1717
import org.gradle.api.provider.Provider;
1818
import org.gradle.api.provider.ProviderFactory;
1919
import org.gradle.api.tasks.SourceSet;
@@ -47,8 +47,8 @@ public void apply(Project project) {
4747
.minus(project.getConfigurations().getByName(CompileOnlyResolvePlugin.RESOLVEABLE_COMPILE_ONLY_CONFIGURATION_NAME))
4848
.plus(sourceSets.getByName(SourceSet.MAIN_SOURCE_SET_NAME).getOutput().getClassesDirs())
4949
);
50-
Provider<Directory> directory = project.getLayout().getBuildDirectory().dir("generated-build-info");
51-
task.getOutputDirectory().set(directory);
50+
Provider<RegularFile> directory = project.getLayout().getBuildDirectory().file("generated-build.info/test-build-info.json");
51+
task.getOutputFile().set(directory);
5252
});
5353

5454
project.getTasks().withType(ProcessResources.class).named("processResources").configure(task -> {

‎server/build.gradle‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,5 @@ tasks.withType(Checkstyle.class).configureEach { t -> t.getMaxHeapSize().set("2g
288288
tasks.named("generateTestBuildInfo").configure {
289289
t -> {
290290
t.getComponentName().set("server")
291-
t.getOutputFileName().set("server-test-build-info.json")
292291
}
293292
}

0 commit comments

Comments
 (0)