Skip to content

Simplify Environment#scanJar() #5940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Simplify Environment#scanJar() #5940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions rewrite-core/src/main/java/org/openrewrite/config/ClasspathScanningLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.nio.file.Paths;
import java.util.*;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;

public class ClasspathScanningLoader implements ResourceLoader {
Expand Down Expand Up @@ -69,7 +68,8 @@ public ClasspathScanningLoader(Properties properties, String[] acceptPackages) {
scanClasses(new ClassGraph().acceptPackages(acceptPackages), getClass().getClassLoader());
scanYaml(new ClassGraph().acceptPaths("META-INF/rewrite"),
properties,
emptyList(),
null,
null,
null);
};
}
Expand All @@ -93,12 +93,13 @@ public ClasspathScanningLoader(Properties properties, ClassLoader classLoader) {
.overrideClassLoaders(classLoader)
.acceptPaths("META-INF/rewrite"),
properties,
emptyList(),
classLoader);
null,
classLoader,
null);
};
}

public ClasspathScanningLoader(Path jar, Properties properties, Collection<? extends ResourceLoader> dependencyResourceLoaders, ClassLoader classLoader) {
public ClasspathScanningLoader(Path jar, Properties properties, @Nullable ResourceLoader dependencyResourceLoader, ClassLoader classLoader) {
this.classLoader = classLoader;
this.recipeLoader = new RecipeLoader(classLoader);

Expand All @@ -109,17 +110,16 @@ public ClasspathScanningLoader(Path jar, Properties properties, Collection<? ext
.overrideClassLoaders(classLoader), classLoader, jar);

scanYaml(new ClassGraph()
.acceptJars(jar.toFile().getName())
.ignoreParentClassLoaders()
.overrideClassLoaders(classLoader)
.acceptPaths("META-INF/rewrite"), properties, dependencyResourceLoaders, classLoader);
.acceptPaths("META-INF/rewrite"), properties, dependencyResourceLoader, classLoader, jar);
};
}

public static ClasspathScanningLoader onlyYaml(Properties properties) {
ClasspathScanningLoader classpathScanningLoader = new ClasspathScanningLoader();
classpathScanningLoader.scanYaml(new ClassGraph().acceptPaths("META-INF/rewrite"),
properties, emptyList(), null);
properties, null, null, null);
return classpathScanningLoader;
}

Expand All @@ -132,14 +132,20 @@ private ClasspathScanningLoader() {
* This must be called _after_ scanClasses or the descriptors of declarative recipes will be missing any
* non-declarative recipes they depend on that would be discovered by scanClasses
*/
private void scanYaml(ClassGraph classGraph, Properties properties, Collection<? extends ResourceLoader> dependencyResourceLoaders, @Nullable ClassLoader classLoader) {
private void scanYaml(ClassGraph classGraph, Properties properties, @Nullable ResourceLoader dependencyResourceLoader, @Nullable ClassLoader classLoader, @Nullable Path targetJar) {
try (ScanResult scanResult = classGraph.scan()) {
List<YamlResourceLoader> yamlResourceLoaders = new ArrayList<>();

scanResult.getResourcesWithExtension("yml").forEachInputStreamIgnoringIOException((res, input) ->
yamlResourceLoaders.add(new YamlResourceLoader(input, res.getURI(), properties, classLoader, dependencyResourceLoaders)));
scanResult.getResourcesWithExtension("yaml").forEachInputStreamIgnoringIOException((res, input) ->
yamlResourceLoaders.add(new YamlResourceLoader(input, res.getURI(), properties, classLoader, dependencyResourceLoaders)));
scanResult.getResourcesWithExtension("yml").forEachInputStreamIgnoringIOException((res, input) -> {
if (targetJar == null || isFromJar(res.getClasspathElementURI(), targetJar)) {
yamlResourceLoaders.add(new YamlResourceLoader(input, res.getURI(), properties, classLoader, dependencyResourceLoader));
}
});
scanResult.getResourcesWithExtension("yaml").forEachInputStreamIgnoringIOException((res, input) -> {
if (targetJar == null || isFromJar(res.getClasspathElementURI(), targetJar)) {
yamlResourceLoaders.add(new YamlResourceLoader(input, res.getURI(), properties, classLoader, dependencyResourceLoader));
}
});
// Extract in two passes so that the full list of recipes from all sources are known when computing recipe descriptors
// Otherwise recipes which include recipes from other sources in their recipeList will have incomplete descriptors
for (YamlResourceLoader resourceLoader : yamlResourceLoaders) {
Expand Down
33 changes: 17 additions & 16 deletions rewrite-core/src/main/java/org/openrewrite/config/Environment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,16 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Path;
import java.util.*;
import java.util.function.Function;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static java.util.Comparator.comparingInt;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toList;
Expand Down Expand Up @@ -347,23 +352,19 @@ public Builder scanYamlResources() {
*/
@SuppressWarnings("unused")
public Builder scanJar(Path jar, Collection<Path> dependencies, ClassLoader classLoader) {
List<ClasspathScanningLoader> firstPassLoaderList = new ArrayList<>();
for (Path dep : dependencies) {
firstPassLoaderList.add(new ClasspathScanningLoader(dep, properties, emptyList(), classLoader));
}

/*
* Second loader creation pass where the firstPassLoaderList is passed as the
* dependencyResourceLoaders list to ensure that we can resolve transitive
* dependencies using the loaders we just created. This is necessary because
* the first pass may have missing recipes since the full list of loaders was
* not provided.
*/
List<ClasspathScanningLoader> secondPassLoaderList = new ArrayList<>();
for (Path dep : dependencies) {
secondPassLoaderList.add(new ClasspathScanningLoader(dep, properties, firstPassLoaderList, classLoader));
List<URL> list = new ArrayList<>();
for (Path dependency : dependencies) {
URI uri = dependency.toUri();
try {
list.add(uri.toURL());
} catch (MalformedURLException e) {
throw new RuntimeException(e);
}
}
return load(new ClasspathScanningLoader(jar, properties, secondPassLoaderList, classLoader), secondPassLoaderList);
// Single `ClassLoader` representing all dependencies
URLClassLoader dependencyClassLoader = new URLClassLoader(list.toArray(new URL[0]), classLoader);
ClasspathScanningLoader dependencyLoader = new ClasspathScanningLoader(properties, dependencyClassLoader);
return load(new ClasspathScanningLoader(jar, properties, dependencyLoader, classLoader), singletonList(dependencyLoader));
}

@SuppressWarnings("unused")
Expand Down
18 changes: 9 additions & 9 deletions rewrite-core/src/main/java/org/openrewrite/config/YamlResourceLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class YamlResourceLoader implements ResourceLoader {

private final ObjectMapper mapper;

private final Collection<? extends ResourceLoader> dependencyResourceLoaders;
private final @Nullable ResourceLoader dependencyResourceLoader;

@Nullable
private Map<String, List<Contributor>> contributors;
Expand Down Expand Up @@ -115,7 +115,7 @@ public YamlResourceLoader(InputStream yamlInput,
URI source,
Properties properties,
@Nullable ClassLoader classLoader) throws UncheckedIOException {
this(yamlInput, source, properties, classLoader, emptyList());
this(yamlInput, source, properties, classLoader, null);
}

/**
Expand All @@ -126,15 +126,15 @@ public YamlResourceLoader(InputStream yamlInput,
* @param source Declarative recipe source
* @param properties Placeholder properties
* @param classLoader Optional classloader to use with jackson. If not specified, the runtime classloader will be used.
* @param dependencyResourceLoaders Optional resource loaders for recipes from dependencies
* @param dependencyResourceLoader Optional resource loader for recipes from dependencies
* @throws UncheckedIOException On unexpected IOException
*/
public YamlResourceLoader(InputStream yamlInput,
URI source,
Properties properties,
@Nullable ClassLoader classLoader,
Collection<? extends ResourceLoader> dependencyResourceLoaders) throws UncheckedIOException {
this(yamlInput, source, properties, classLoader, dependencyResourceLoaders, jsonMapper -> {
@Nullable ResourceLoader dependencyResourceLoader) throws UncheckedIOException {
this(yamlInput, source, properties, classLoader, dependencyResourceLoader, jsonMapper -> {
});
}

Expand All @@ -146,16 +146,16 @@ public YamlResourceLoader(InputStream yamlInput,
* @param source Declarative recipe source
* @param properties Placeholder properties
* @param classLoader Optional classloader to use with jackson. If not specified, the runtime classloader will be used.
* @param dependencyResourceLoaders Optional resource loaders for recipes from dependencies
* @param dependencyResourceLoader Optional resource loader for recipes from dependencies
* @param mapperCustomizer Customizer for the ObjectMapper
* @throws UncheckedIOException On unexpected IOException
*/
public YamlResourceLoader(InputStream yamlInput, URI source, Properties properties,
@Nullable ClassLoader classLoader,
Collection<? extends ResourceLoader> dependencyResourceLoaders,
@Nullable ResourceLoader dependencyResourceLoader,
Consumer<ObjectMapper> mapperCustomizer) {
this.source = source;
this.dependencyResourceLoaders = dependencyResourceLoaders;
this.dependencyResourceLoader = dependencyResourceLoader;
this.mapper = ObjectMappers.propertyBasedMapper(classLoader);
this.recipeLoader = new RecipeLoader(classLoader);

Expand Down Expand Up @@ -409,7 +409,7 @@ public Collection<RecipeDescriptor> listRecipeDescriptors(Collection<Recipe> ext
externalRecipes.stream(),
internalRecipes.stream()
),
dependencyResourceLoaders.stream().flatMap(rl -> rl.listRecipes().stream())
dependencyResourceLoader != null ? dependencyResourceLoader.listRecipes().stream() : Stream.empty()
).collect(toList());

List<RecipeDescriptor> recipeDescriptors = new ArrayList<>();
Expand Down
3 changes: 2 additions & 1 deletion rewrite-core/src/test/java/org/openrewrite/config/ClasspathScanningLoaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.jar.JarOutputStream;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.assertj.core.api.Assertions.assertThat;

class ClasspathScanningLoaderTest {
Expand All @@ -61,7 +62,7 @@ void testInheritanceAcrossJars() throws IOException {
);

Environment fullEnv = Environment.builder()
.scanJar(concreteJar, emptyList(), fullClassLoader)
.scanJar(concreteJar, singletonList(coreJar), fullClassLoader)
.build();

assertThat(fullEnv.listRecipes()).hasSize(1);
Expand Down
3 changes: 1 addition & 2 deletions rewrite-test/src/main/java/org/openrewrite/test/RecipeSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.util.function.Function;
import java.util.function.Supplier;

import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.joining;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
Expand Down Expand Up @@ -150,7 +149,7 @@ public RecipeSpec recipeFromResources(String... activeRecipes) {

private static Recipe recipeFromInputStream(InputStream yaml, String... activeRecipes) {
return Environment.builder()
.load(new YamlResourceLoader(yaml, URI.create("rewrite.yml"), new Properties(), null, emptyList(),
.load(new YamlResourceLoader(yaml, URI.create("rewrite.yml"), new Properties(), null, null,
mapper -> mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)))
.build()
.activateRecipes(activeRecipes);
Expand Down