Skip to content

Commit

Permalink
Quarkus code-gen (Gradle): Fix behavior to filter unavailable services
Browse files Browse the repository at this point in the history
Java Services (those in `META-INF/services/*` files) that are defined in the (Gradle) project that uses the Quarkus Gradle plugin, are not available when Quarkus code generation runs. This is simply a task-dependency requirement in Gradle, because Java compilation happens after code generation. If a Java service, for example a Smallrye config sources, is present and the (Smallrye) configuration is needed by a Quarkus extension, the build fails during Quarkus code generation with an error message like this:
```
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':quarkusGenerateCode'.
...
Caused by: org.gradle.workers.internal.DefaultWorkerExecutor$WorkExecutionException: A failure occurred while executing io.quarkus.gradle.tasks.worker.CodeGenWorker
...
Caused by: java.util.ServiceConfigurationError: io.smallrye.config.ConfigSourceFactory: Provider xyz not found
```

`io.quarkus.deployment.CodeGenerator` has a mechanism to filter out unavailable services via `getUnavailableConfigServices()`. However the callback passed to `io.quarkus.paths.PathTree.apply()` can stop before all roots/trees (`io.quarkus.paths.PathTree.getRoots()`) have been "asked" (`MultiRootPathTree`), because the callback can return a non-`null` value. This "early stop" happens before the root/tree containing the source with the `META-INF/services/*` has been processed.

The bug is only triggered, if a Java service is defined in the Gradle project using the Quarkus Gradle plugin and if a Quarkus extension using the configuration is a dependency of that project.

This change updates the callback implementation to collect all unavailable services from all `PathTree` roots/trees.

An integration test using the Gradle plugin is included as well.

Two logging/spelling mistakes have been fixed as well.

Fixes #36716
  • Loading branch information
snazy committed Oct 31, 2023
1 parent 3149ece commit 6c895d0
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -361,39 +361,36 @@ public static <T> T readConfig(ApplicationModel appModel, LaunchMode launchMode,
private static Map<String, List<String>> getUnavailableConfigServices(ResolvedDependency dep, ClassLoader classLoader)
throws CodeGenException {
try (OpenPathTree openTree = dep.getContentTree().open()) {
return openTree.apply(META_INF_SERVICES, visit -> {
var unavailableServices = new HashMap<String, List<String>>();
openTree.apply(META_INF_SERVICES, visit -> {
if (visit == null) {
// the application module does not include META-INF/services entry
return Map.of();
// the application module does not include META-INF/services entry. Return `null` here, to let
// MultiRootPathTree.apply() look into all roots.
return null;
}
Map<String, List<String>> unavailableServices = Map.of();
var servicesDir = visit.getPath();
for (String serviceClass : CONFIG_SERVICES) {
var serviceFile = servicesDir.resolve(serviceClass);
if (!Files.exists(serviceFile)) {
continue;
}
final List<String> implList;
var unavailableList = unavailableServices.computeIfAbsent(META_INF_SERVICES + serviceClass,
k -> new ArrayList<>());
try {
implList = Files.readAllLines(serviceFile);
Files.readAllLines(serviceFile).stream()
.map(String::trim)
// skip comments and empty lines
.filter(line -> !line.startsWith("#") && !line.isEmpty())
.filter(className -> classLoader.getResource(className.replace('.', '/') + ".class") == null)
.forEach(unavailableList::add);
} catch (IOException e) {
throw new UncheckedIOException("Failed to read " + serviceFile, e);
}
final List<String> unavailableList = new ArrayList<>(implList.size());
for (String impl : implList) {
if (classLoader.getResource(impl.replace('.', '/') + ".class") == null) {
unavailableList.add(impl);
}
}
if (!unavailableList.isEmpty()) {
if (unavailableServices.isEmpty()) {
unavailableServices = new HashMap<>();
}
unavailableServices.put(META_INF_SERVICES + serviceClass, unavailableList);
}
}
return unavailableServices;
// Always return null to let MultiRootPathTree.apply() look into all roots.
return null;
});
return unavailableServices;
} catch (IOException e) {
throw new CodeGenException("Failed to read " + dep.getResolvedPaths(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void generateCode() {

File outputPath = getGeneratedOutputDirectory().get().getAsFile();

getLogger().debug("Will trigger preparing sources for source directory: {} buildDir: {}",
getLogger().debug("Will trigger preparing sources for source directories: {} buildDir: {}",
sourcesDirectories, buildDir.getAbsolutePath());

WorkQueue workQueue = workQueue(configMap, () -> extension().codeGenForkOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public void execute() {
} catch (BootstrapException | IllegalAccessException | InvocationTargetException | ClassNotFoundException e) {
// Gradle "abbreviates" the stacktrace to something human-readable, but here the underlying cause might
// get lost in the error output, so add 'e' to the message.
throw new GradleException("Failed to generate sources in the QuarkusPrepare task for " + gav + " due to " + e, e);
throw new GradleException("Failed to generate sources in the QuarkusGenerateCode task for " + gav + " due to " + e,
e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
plugins {
id 'java'
id 'io.quarkus'
}

repositories {
mavenLocal {
content {
includeGroupByRegex 'io.quarkus.*'
}
}
mavenCentral()
}

dependencies {
implementation enforcedPlatform("${quarkusPlatformGroupId}:${quarkusPlatformArtifactId}:${quarkusPlatformVersion}")
implementation "io.quarkus:quarkus-grpc" // Need a `CodeGenProvider` on the class path for this test!
compileOnly "io.smallrye.config:smallrye-config-core"
}

compileJava {
options.compilerArgs << '-parameters'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
quarkusPlatformArtifactId=quarkus-bom
quarkusPlatformGroupId=io.quarkus
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
pluginManagement {
repositories {
mavenLocal {
content {
includeGroupByRegex 'io.quarkus.*'
}
}
mavenCentral()
gradlePluginPortal()
}
plugins {
id 'io.quarkus' version "${quarkusPluginVersion}"
}
}
rootProject.name='custom-config-sources'
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package org.acme;

import org.eclipse.microprofile.config.spi.ConfigSource;

import java.util.HashMap;
import java.util.Map;
import java.util.Set;

public class InMemoryConfigSource implements ConfigSource {
private static final Map<String, String> configuration = new HashMap<>();

static {
configuration.put("my.prop", "1234");
}

@Override
public int getOrdinal() {
return 275;
}

@Override
public Set<String> getPropertyNames() {
return configuration.keySet();
}

@Override
public String getValue(final String propertyName) {
return configuration.get(propertyName);
}

@Override
public String getName() {
return InMemoryConfigSource.class.getSimpleName();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.acme;

import io.smallrye.config.ConfigSourceContext;
import io.smallrye.config.ConfigSourceFactory;
import org.eclipse.microprofile.config.spi.ConfigSource;

import java.util.List;

public class MyConfigSourceFactory implements ConfigSourceFactory {
@Override
public Iterable<ConfigSource> getConfigSources(ConfigSourceContext context) {
return List.of(new InMemoryConfigSource());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.acme.MyConfigSourceFactory
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.acme.InMemoryConfigSource
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Need an empty application.properties to trigger config loading mandatory for CustomConfigSourcesTest
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package io.quarkus.gradle;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;

public class CustomConfigSourcesTest extends QuarkusGradleWrapperTestBase {

@Test
public void testCustomConfigSources() throws Exception {
var projectDir = getProjectDir("custom-config-sources");

// The test is successful, if the build works, see https://github.com/quarkusio/quarkus/issues/36716
runGradleWrapper(projectDir, "clean", "build", "--no-build-cache");

var p = projectDir.toPath().resolve("build").resolve("quarkus-app").resolve("quarkus-run.jar");
assertThat(p).exists();
}
}

0 comments on commit 6c895d0

Please sign in to comment.