Skip to content

Commit

Permalink
Making sure deployment modules excluded in POM files aren't pulled in…
Browse files Browse the repository at this point in the history
… by the Gradle plugin

This makes sure that exclusions in POM files of deployment modules are respects.

Once case where this is critical is when using quarkus-hibernate-reactive. Its deployment module takes care of pulling in quarkus-hibernate-orm-deployment where it excludes Agroal and Narayana.

If that exclusion isn't taken into account by the Gradle plugin, it pulls in quarkus-hibernate-orm-deployment on its own, where those other modules aren't excluded, which leads to a failure at runtime because Agroal tries to initialize and looks for a JDBC driver which isn't typically available in reactive DB scenarios.

Fixes quarkusio#38533
  • Loading branch information
vemilyus committed Feb 8, 2024
1 parent ad904bc commit 0093e7c
Show file tree
Hide file tree
Showing 16 changed files with 561 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,22 @@ public void exportDeploymentClasspath(String configurationName) {
project.getConfigurations().getByName(configurationName));
Set<ExtensionDependency<?>> extensionDependencies = collectFirstMetQuarkusExtensions(configuration);

final Set<String> seenDeploymentDependencies = new HashSet<>();

DependencyHandler dependencies = project.getDependencies();

for (ExtensionDependency<?> extension : extensionDependencies) {
if (!alreadyProcessed.add(extension.getExtensionId())) {
continue;
}

if (!seenDeploymentDependencies.add(DependencyUtils.getDeploymentDependencyIdentifier(extension))) {
continue;
}

dependencies.add(deploymentConfigurationName,
DependencyUtils.createDeploymentDependency(dependencies, extension));
DependencyUtils.createDeploymentDependency(project, dependencies, extension,
seenDeploymentDependencies));
}
});
}
Expand Down
3 changes: 3 additions & 0 deletions devtools/gradle/gradle-model/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ plugins {
}

dependencies {
implementation(libs.maven.model)

compileOnly(libs.kotlin.gradle.plugin.api)
gradleApi()
}

group = "io.quarkus"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,26 +205,46 @@ private void setUpDeploymentConfiguration() {
ConditionalDependenciesEnabler cdEnabler = new ConditionalDependenciesEnabler(project, mode,
enforcedPlatforms);
final Collection<ExtensionDependency<?>> allExtensions = cdEnabler.getAllExtensions();
Set<ExtensionDependency<?>> extensions = collectFirstMetQuarkusExtensions(getRawRuntimeConfiguration(),
allExtensions);

// linked hash set is very important here! we need to keep the first met dependencies
// at the start of the collection so dependency based exclusions stick!
// Example: this prevents pulling in hibernate-orm-deployment here if it has already been
// pulled in by hibernate-reactive where it excludes Agroal and Narayana.
// otherwise those get pulled in as well and lead to failures when trying to run
// an application with reactive DB drivers without JDBC drivers on the class path
Set<ExtensionDependency<?>> extensions = new LinkedHashSet<>(
collectFirstMetQuarkusExtensions(getRawRuntimeConfiguration(),
allExtensions));
// Add conditional extensions
for (ExtensionDependency<?> knownExtension : allExtensions) {
if (knownExtension.isConditional()) {
extensions.add(knownExtension);
}
}

final Set<String> seenDeploymentDependencies = new HashSet<>();

final Set<ModuleVersionIdentifier> alreadyProcessed = new HashSet<>(extensions.size());
final DependencyHandler dependencies = project.getDependencies();
final Set<Dependency> deploymentDependencies = new HashSet<>();

for (ExtensionDependency<?> extension : extensions) {
if (!alreadyProcessed.add(extension.getExtensionId())) {
continue;
}

if (!seenDeploymentDependencies.add(DependencyUtils.getDeploymentDependencyIdentifier(extension))) {
continue;
}

deploymentDependencies.add(
DependencyUtils.createDeploymentDependency(dependencies, extension));
DependencyUtils.createDeploymentDependency(
project,
dependencies,
extension,
seenDeploymentDependencies));
}

return deploymentDependencies;
})));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import io.quarkus.gradle.extension.ConfigurationUtils;
import io.quarkus.gradle.extension.ExtensionConstants;
import io.quarkus.gradle.tooling.ToolingUtils;
import io.quarkus.gradle.tooling.pom.PomUtils;
import io.quarkus.maven.dependency.ArtifactCoords;
import io.quarkus.maven.dependency.ArtifactKey;
import io.quarkus.maven.dependency.GACT;
Expand Down Expand Up @@ -349,15 +350,16 @@ public static Dependency create(DependencyHandler dependencies, String condition
dependencyCoords.getVersion()));
}

public static Dependency createDeploymentDependency(
public static Dependency createDeploymentDependency(Project project,
DependencyHandler dependencyHandler,
ExtensionDependency<?> dependency) {
ExtensionDependency<?> dependency,
Set<String> seenDeploymentDependencies) {
if (dependency instanceof ProjectExtensionDependency) {
ProjectExtensionDependency ped = (ProjectExtensionDependency) dependency;
return createDeploymentProjectDependency(dependencyHandler, ped);
} else if (dependency instanceof ArtifactExtensionDependency) {
ArtifactExtensionDependency aed = (ArtifactExtensionDependency) dependency;
return createArtifactDeploymentDependency(dependencyHandler, aed);
return createArtifactDeploymentDependency(project, dependencyHandler, aed, seenDeploymentDependencies);
}

throw new IllegalArgumentException("Unknown ExtensionDependency type: " + dependency.getClass().getName());
Expand All @@ -376,10 +378,28 @@ private static Dependency createDeploymentProjectDependency(DependencyHandler ha
}
}

private static Dependency createArtifactDeploymentDependency(DependencyHandler handler,
ArtifactExtensionDependency dependency) {
private static Dependency createArtifactDeploymentDependency(
Project project,
DependencyHandler handler,
ArtifactExtensionDependency dependency,
Set<String> seenDeploymentDependencies) {

seenDeploymentDependencies.addAll(PomUtils.readDeploymentDependencies(project, dependency.getDeploymentModule()));

return handler.create(dependency.getDeploymentModule().getGroupId() + ":"
+ dependency.getDeploymentModule().getArtifactId() + ":"
+ dependency.getDeploymentModule().getVersion());
}

public static String getDeploymentDependencyIdentifier(ExtensionDependency<?> extension) {
if (extension instanceof ArtifactExtensionDependency) {
ArtifactExtensionDependency aed = (ArtifactExtensionDependency) extension;
return aed.getDeploymentModule().getGroupId() + ":" + aed.getDeploymentModule().getArtifactId();
} else if (extension instanceof ProjectExtensionDependency) {
ProjectExtensionDependency ped = (ProjectExtensionDependency) extension;
return ped.getDeploymentModule().getGroup() + ":" + ped.getDeploymentModule().getName();
}

return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package io.quarkus.gradle.tooling.pom;

import java.io.FileReader;
import java.nio.charset.StandardCharsets;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;

import org.apache.maven.model.Dependency;
import org.apache.maven.model.Model;
import org.apache.maven.model.io.xpp3.MavenXpp3Reader;
import org.gradle.api.Project;
import org.gradle.api.artifacts.result.ArtifactResolutionResult;
import org.gradle.api.artifacts.result.ArtifactResult;
import org.gradle.api.artifacts.result.ComponentArtifactsResult;
import org.gradle.api.internal.artifacts.DefaultModuleIdentifier;
import org.gradle.api.internal.artifacts.result.DefaultResolvedArtifactResult;
import org.gradle.internal.component.external.model.DefaultModuleComponentIdentifier;
import org.gradle.maven.MavenModule;
import org.gradle.maven.MavenPomArtifact;

import io.quarkus.maven.dependency.ArtifactCoords;

public final class PomUtils {

This comment has been minimized.

Copy link
@aloubyansky

aloubyansky Feb 9, 2024

Oh, is this really necessary?

One point to keep in mind is that, when we resolve deployment dependencies, we need to resolve only the top level one deployment artifact. So in that issue we are fixing, it'll be quarkus-hibernate-reactive-deployment. It will already resolve its dependencies with the exclusions applied.
Then we "simply" replace the quarkus-hibernate-reactive with quarkus-hibernate-reactive-deployment and its dependencies (where quarkus-hibernate-reactive is also present) to create the Quarkus build classpath.

However, when we resolve quarkus-hibernate-reactive-deployment we should apply exclusions from the dependencies that appear higher in the hierarchy, e.g. if quarkus-hibernate-reactive was a dependency of acme-lib and my-app had a dependency on acme-lib with quarkus-hibernate-reactive pulled in as its transitive dependency. So, if my-app configured dependency exclusions on acme-lib dependency, those exclusions that should also apply when we resolve dependencies of quarkus-hibernate-reactive-deployment.

This comment has been minimized.

Copy link
@vemilyus

vemilyus Feb 9, 2024

Author Owner

Let me address your concerns one at a time:

  1. We can never guarantee that deployment modules pull in all the deployment modules of an extension's runtime dependencies. We also don't enforce the validateExtension task in the extension plugin, which can also be disabled.
  2. By only resolving the first level artifacts we lose out on extensions pulled in by normal libraries.
  3. Extensions that are dependencies of normal libs (acme-lib) wouldn't be recognized as extensions, thus their deployment module wouldn't be added to the deployment classpath thereby nullifying its functionality.
public static Set<String> readDeploymentDependencies(Project project, ArtifactCoords artifactCoords) {
@SuppressWarnings("unchecked")
ArtifactResolutionResult resolutionResult = project
.getDependencies().createArtifactResolutionQuery()
.forComponents(
new DefaultModuleComponentIdentifier(
DefaultModuleIdentifier.newId(
artifactCoords.getGroupId(),
artifactCoords.getArtifactId()),
artifactCoords.getVersion()))
.withArtifacts(MavenModule.class, MavenPomArtifact.class)
.execute();

MavenXpp3Reader pomReader = new MavenXpp3Reader();

final Set<String> results = new HashSet<>();

for (ComponentArtifactsResult resolvedComponent : resolutionResult.getResolvedComponents()) {
for (ArtifactResult artifact : resolvedComponent.getArtifacts(MavenPomArtifact.class)) {
if (artifact instanceof DefaultResolvedArtifactResult) {
DefaultResolvedArtifactResult resolvedArtifact = (DefaultResolvedArtifactResult) artifact;

Model pom;
try (FileReader fr = new FileReader(resolvedArtifact.getFile(), StandardCharsets.UTF_8)) {
pom = pomReader.read(fr);
} catch (Exception e) {
project.getLogger().warn("Failed to read pom.xml for " + resolvedArtifact, e);
continue;
}

for (Dependency pomDependency : pom.getDependencies()) {
if (!Objects.equals("test", pomDependency.getScope())) {
results.add(pomDependency.getGroupId() + ":" + pomDependency.getArtifactId());
}
}
}
}
}

return results;
}

private PomUtils() {
}
}
8 changes: 6 additions & 2 deletions devtools/gradle/gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ plugin-publish = "1.2.1"
kotlin = "1.9.22"
smallrye-config = "3.5.4"

maven = "3.9.6"

junit5 = "5.10.2"
assertj = "3.25.3"

Expand All @@ -22,8 +24,10 @@ quarkus-project-core-extension-codestarts = { module = "io.quarkus:quarkus-proje

kotlin-gradle-plugin-api = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin-api", version.ref = "kotlin" }
smallrye-config-yaml = { module = "io.smallrye.config:smallrye-config-source-yaml", version.ref = "smallrye-config" }
jackson-databind = {module="com.fasterxml.jackson.core:jackson-databind"}
jackson-dataformat-yaml = {module="com.fasterxml.jackson.dataformat:jackson-dataformat-yaml"}
jackson-databind = { module = "com.fasterxml.jackson.core:jackson-databind" }
jackson-dataformat-yaml = { module = "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml" }

maven-model = { module = "org.apache.maven:maven-model", version.ref = "maven" }

junit-bom = { module = "org.junit:junit-bom", version.ref = "junit5" }
junit-api = { module = "org.junit.jupiter:junit-jupiter-api" }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
plugins {
id 'java'
id 'io.quarkus'
}

repositories {
mavenLocal {
content {
includeGroupByRegex 'io.quarkus.*'
includeGroup 'org.hibernate.orm'
}
}
mavenCentral()
}

dependencies {
implementation enforcedPlatform("${quarkusPlatformGroupId}:${quarkusPlatformArtifactId}:${quarkusPlatformVersion}")
implementation 'io.quarkus:quarkus-resteasy-reactive-jackson'
implementation 'io.quarkus:quarkus-hibernate-reactive-panache'
implementation 'io.quarkus:quarkus-reactive-pg-client'
implementation 'io.quarkus:quarkus-arc'
implementation 'io.quarkus:quarkus-resteasy-reactive'
testImplementation 'io.quarkus:quarkus-junit5'
testImplementation 'io.rest-assured:rest-assured'
}

group 'org.acme'
version '1.0.0-SNAPSHOT'

test {
systemProperty "java.util.logging.manager", "org.jboss.logmanager.LogManager"
}
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,17 @@
pluginManagement {
repositories {
mavenLocal {
content {
includeGroupByRegex 'io.quarkus.*'
includeGroup 'org.hibernate.orm'
}
}
mavenCentral()
gradlePluginPortal()
}
plugins {
id 'io.quarkus' version "${quarkusPluginVersion}"
}
}

rootProject.name='code-with-quarkus'
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.acme;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

@Path("/hello")
public class GreetingResource {

@GET
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "Hello from RESTEasy Reactive";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

package org.acme;

import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import java.util.UUID;

@Entity
public class MyEntity {
@Id
@GeneratedValue
private UUID id;
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
quarkus.datasource.db-kind = postgresql
quarkus.datasource.username = quarkus_test
quarkus.datasource.password = quarkus_test
quarkus.datasource.reactive.url = vertx-reactive:postgresql://localhost:5432/quarkus_test
#quarkus.datasource.jdbc=false
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package org.acme;

import io.quarkus.test.junit.QuarkusIntegrationTest;

@QuarkusIntegrationTest
class GreetingResourceIT extends GreetingResourceTest {
// Execute the same tests but in packaged mode.
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.acme;

import static io.restassured.RestAssured.given;
import static org.hamcrest.CoreMatchers.is;

import org.junit.jupiter.api.Test;

import io.quarkus.test.junit.QuarkusTest;

@QuarkusTest
class GreetingResourceTest {
@Test
void testHelloEndpoint() {
given()
.when().get("/hello")
.then()
.statusCode(200)
.body(is("Hello from RESTEasy Reactive"));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.quarkus.gradle.devmode;

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

/**
* This makes sure that exclusions in POM files of deployment modules are respected.
* <p>
* One case where this is critical is when using quarkus-hibernate-reactive. Its deployment
* module takes care of pulling in quarkus-hibernate-orm-deployment where it excludes
* Agroal and Narayana.
* <p>
* If that exclusion isn't taken into account by the Gradle plugin, it pulls in
* quarkus-hibernate-orm-deployment on its own, where those other modules aren't excluded,
* which leads to a failure at runtime because Agroal tries to initialize and looks
* for a JDBC driver which isn't typically available in reactive DB scenarios.
*/
public class MavenExclusionInExtensionDependencyDevModeTest extends QuarkusDevGradleTestBase {
@Override
protected String projectDirectoryName() {
return "maven-exclusion-in-extension-dependency";
}

@Override
protected void testDevMode() throws Exception {
assertThat(getHttpResponse("/hello")).contains("Hello");
}
}

0 comments on commit 0093e7c

Please sign in to comment.