From 6c895d0c2dfadb24a74b48e8d4ba4494ecb6a227 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Tue, 31 Oct 2023 15:12:39 +0100 Subject: [PATCH] Quarkus code-gen (Gradle): Fix behavior to filter unavailable services 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 --- .../io/quarkus/deployment/CodeGenerator.java | 35 +++++++++---------- .../gradle/tasks/QuarkusGenerateCode.java | 2 +- .../gradle/tasks/worker/CodeGenWorker.java | 3 +- .../custom-config-sources/build.gradle | 23 ++++++++++++ .../custom-config-sources/gradle.properties | 2 ++ .../custom-config-sources/settings.gradle | 15 ++++++++ .../java/org/acme/InMemoryConfigSource.java | 35 +++++++++++++++++++ .../java/org/acme/MyConfigSourceFactory.java | 14 ++++++++ .../io.smallrye.config.ConfigSourceFactory | 1 + ...lipse.microprofile.config.spi.ConfigSource | 1 + .../src/main/resources/application.properties | 1 + .../gradle/CustomConfigSourcesTest.java | 19 ++++++++++ 12 files changed, 130 insertions(+), 21 deletions(-) create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/build.gradle create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/gradle.properties create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/settings.gradle create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/InMemoryConfigSource.java create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/MyConfigSourceFactory.java create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/io.smallrye.config.ConfigSourceFactory create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSource create mode 100644 integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/application.properties create mode 100644 integration-tests/gradle/src/test/java/io/quarkus/gradle/CustomConfigSourcesTest.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/CodeGenerator.java b/core/deployment/src/main/java/io/quarkus/deployment/CodeGenerator.java index 3e0ea9bc12f29..71852e90e5c86 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/CodeGenerator.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/CodeGenerator.java @@ -361,39 +361,36 @@ public static T readConfig(ApplicationModel appModel, LaunchMode launchMode, private static Map> getUnavailableConfigServices(ResolvedDependency dep, ClassLoader classLoader) throws CodeGenException { try (OpenPathTree openTree = dep.getContentTree().open()) { - return openTree.apply(META_INF_SERVICES, visit -> { + var unavailableServices = new HashMap>(); + 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> unavailableServices = Map.of(); var servicesDir = visit.getPath(); for (String serviceClass : CONFIG_SERVICES) { var serviceFile = servicesDir.resolve(serviceClass); if (!Files.exists(serviceFile)) { continue; } - final List 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 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); } diff --git a/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/QuarkusGenerateCode.java b/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/QuarkusGenerateCode.java index 3e9c6c02cf304..b01ffdb6732f0 100644 --- a/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/QuarkusGenerateCode.java +++ b/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/QuarkusGenerateCode.java @@ -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); diff --git a/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/worker/CodeGenWorker.java b/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/worker/CodeGenWorker.java index efa9bc1fd7668..fa79bbbe36928 100644 --- a/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/worker/CodeGenWorker.java +++ b/devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/worker/CodeGenWorker.java @@ -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); } } } diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/build.gradle b/integration-tests/gradle/src/main/resources/custom-config-sources/build.gradle new file mode 100644 index 0000000000000..2dbbb90a158e7 --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/build.gradle @@ -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' +} diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/gradle.properties b/integration-tests/gradle/src/main/resources/custom-config-sources/gradle.properties new file mode 100644 index 0000000000000..ec2b6ef199c2c --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/gradle.properties @@ -0,0 +1,2 @@ +quarkusPlatformArtifactId=quarkus-bom +quarkusPlatformGroupId=io.quarkus diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/settings.gradle b/integration-tests/gradle/src/main/resources/custom-config-sources/settings.gradle new file mode 100644 index 0000000000000..171b346e0e750 --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/settings.gradle @@ -0,0 +1,15 @@ +pluginManagement { + repositories { + mavenLocal { + content { + includeGroupByRegex 'io.quarkus.*' + } + } + mavenCentral() + gradlePluginPortal() + } + plugins { + id 'io.quarkus' version "${quarkusPluginVersion}" + } +} +rootProject.name='custom-config-sources' diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/InMemoryConfigSource.java b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/InMemoryConfigSource.java new file mode 100644 index 0000000000000..b63ec196a074e --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/InMemoryConfigSource.java @@ -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 configuration = new HashMap<>(); + + static { + configuration.put("my.prop", "1234"); + } + + @Override + public int getOrdinal() { + return 275; + } + + @Override + public Set getPropertyNames() { + return configuration.keySet(); + } + + @Override + public String getValue(final String propertyName) { + return configuration.get(propertyName); + } + + @Override + public String getName() { + return InMemoryConfigSource.class.getSimpleName(); + } +} diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/MyConfigSourceFactory.java b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/MyConfigSourceFactory.java new file mode 100644 index 0000000000000..e8d1e05f517fe --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/java/org/acme/MyConfigSourceFactory.java @@ -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 getConfigSources(ConfigSourceContext context) { + return List.of(new InMemoryConfigSource()); + } +} diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/io.smallrye.config.ConfigSourceFactory b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/io.smallrye.config.ConfigSourceFactory new file mode 100644 index 0000000000000..d8f8a6fb31a63 --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/io.smallrye.config.ConfigSourceFactory @@ -0,0 +1 @@ +org.acme.MyConfigSourceFactory \ No newline at end of file diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSource b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSource new file mode 100644 index 0000000000000..934c1492c8bd5 --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSource @@ -0,0 +1 @@ +org.acme.InMemoryConfigSource \ No newline at end of file diff --git a/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/application.properties b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/application.properties new file mode 100644 index 0000000000000..c2480799c946c --- /dev/null +++ b/integration-tests/gradle/src/main/resources/custom-config-sources/src/main/resources/application.properties @@ -0,0 +1 @@ +# Need an empty application.properties to trigger config loading mandatory for CustomConfigSourcesTest \ No newline at end of file diff --git a/integration-tests/gradle/src/test/java/io/quarkus/gradle/CustomConfigSourcesTest.java b/integration-tests/gradle/src/test/java/io/quarkus/gradle/CustomConfigSourcesTest.java new file mode 100644 index 0000000000000..74dcd95fa3960 --- /dev/null +++ b/integration-tests/gradle/src/test/java/io/quarkus/gradle/CustomConfigSourcesTest.java @@ -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(); + } +}