Merge pull request #10556 from aloubyansky/10484

Dev mode: fixed discovery of local dependencies inherited from parents
quarkusio · Jul 10, 2020 · 160bdf3 · 160bdf3
2 parents 9fe0d58 + b58301d
commit 160bdf3
Show file tree

Hide file tree

Showing 38 changed files with 1,161 additions and 92 deletions.
diff --git a/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java b/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java
@@ -13,7 +13,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -25,7 +24,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
-import java.util.Queue;
 import java.util.Set;
 import java.util.jar.Attributes;
 import java.util.jar.Manifest;
@@ -54,6 +52,9 @@
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.artifact.DefaultArtifact;
 import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.graph.DependencyVisitor;
 import org.eclipse.aether.repository.RemoteRepository;
 import org.eclipse.aether.repository.WorkspaceReader;
 import org.eclipse.aether.resolution.ArtifactRequest;
@@ -68,6 +69,7 @@
 import io.quarkus.bootstrap.model.AppArtifactKey;
 import io.quarkus.bootstrap.resolver.maven.options.BootstrapMavenOptions;
 import io.quarkus.bootstrap.resolver.maven.workspace.LocalProject;
+import io.quarkus.bootstrap.resolver.maven.workspace.LocalWorkspace;
 import io.quarkus.deployment.dev.DevModeContext;
 import io.quarkus.deployment.dev.DevModeMain;
 import io.quarkus.maven.components.MavenVersionEnforcer;
@@ -873,54 +875,78 @@ protected void modifyDevModeContext(DevModeContext devModeContext) {
     }
 
     private List<LocalProject> filterExtensionDependencies(LocalProject localProject) {
+        final LocalWorkspace workspace = localProject.getWorkspace();
+        if (workspace == null) {
+            return Collections.singletonList(localProject);
+        }
+
         List<LocalProject> ret = new ArrayList<>();
-        Queue<LocalProject> toRemove = new ArrayDeque<>();
         Set<AppArtifactKey> extensionsAndDeps = new HashSet<>();
-        Set<AppArtifactKey> inProject = new HashSet<>();
 
-        for (LocalProject project : localProject.getSelfWithLocalDeps()) {
-            inProject.add(project.getKey());
+        ret.add(localProject);
+        for (Artifact a : project.getArtifacts()) {
+            final AppArtifactKey depKey = new AppArtifactKey(a.getGroupId(), a.getArtifactId());
+            final LocalProject project = workspace.getProject(depKey);
+            if (project == null) {
+                continue;
+            }
             if (project.getClassesDir() != null &&
             //if this project also contains Quarkus extensions we do no want to include these in the discovery
             //a bit of an edge case, but if you try and include a sample project with your extension you will
             //run into problems without this
                     (Files.exists(project.getClassesDir().resolve("META-INF/quarkus-extension.properties")) ||
                             Files.exists(project.getClassesDir().resolve("META-INF/quarkus-build-steps.list")))) {
-                toRemove.add(project);
-                extensionsAndDeps.add(project.getKey());
+                // TODO add the deployment deps
+                extensionDepWarning(depKey);
+                try {
+                    final DependencyNode depRoot = repoSystem.collectDependencies(repoSession, new CollectRequest()
+                            .setRoot(new org.eclipse.aether.graph.Dependency(
+                                    new DefaultArtifact(a.getGroupId(), a.getArtifactId(),
+                                            a.getClassifier(), a.getArtifactHandler().getExtension(), a.getVersion()),
+                                    JavaScopes.RUNTIME))
+                            .setRepositories(repos)).getRoot();
+                    depRoot.accept(new DependencyVisitor() {
+                        @Override
+                        public boolean visitEnter(DependencyNode node) {
+                            final org.eclipse.aether.artifact.Artifact artifact = node.getArtifact();
+                            if ("jar".equals(artifact.getExtension())) {
+                                extensionsAndDeps.add(new AppArtifactKey(artifact.getGroupId(), artifact.getArtifactId()));
+                            }
+                            return true;
+                        }
+
+                        @Override
+                        public boolean visitLeave(DependencyNode node) {
+                            return true;
+                        }
+                    });
+                } catch (DependencyCollectionException e) {
+                    throw new RuntimeException("Failed to collect dependencies for " + a, e);
+                }
             } else {
                 ret.add(project);
             }
         }
-        if (toRemove.isEmpty()) {
+
+        if (extensionsAndDeps.isEmpty()) {
             return ret;
         }
-        //we also remove transitive deps of the extensions
-        //this is common in projects that provide a library, and a quarkus extension for that library
-        //all in the same project
-        while (!toRemove.isEmpty()) {
-            LocalProject dep = toRemove.poll();
-            //we don't need to resolve deps properly, this is all in the same project
-            //so we have all the info we need locally
-            for (Dependency i : dep.getRawModel().getDependencies()) {
-                AppArtifactKey key = new AppArtifactKey(i.getGroupId(), i.getArtifactId());
-                if (inProject.contains(key) && !extensionsAndDeps.contains(key)) {
-                    extensionsAndDeps.add(key);
-                    toRemove.add(localProject.getWorkspace().getProject(key));
-                }
-            }
-        }
+
         Iterator<LocalProject> iterator = ret.iterator();
         while (iterator.hasNext()) {
-            LocalProject obj = iterator.next();
-            if (extensionsAndDeps.contains(obj.getKey())) {
-                getLog().warn("Local Quarkus extension dependency " + obj.getKey() + " will not be hot-reloadable");
+            final LocalProject localDep = iterator.next();
+            if (extensionsAndDeps.contains(localDep.getKey())) {
+                extensionDepWarning(localDep.getKey());
                 iterator.remove();
             }
         }
         return ret;
     }
 
+    private void extensionDepWarning(AppArtifactKey key) {
+        getLog().warn("Local Quarkus extension dependency " + key + " will not be hot-reloadable");
+    }
+
     private Optional<Xpp3Dom> findCompilerPluginConfiguration() {
         for (final Plugin plugin : project.getBuildPlugins()) {
             if (!plugin.getKey().equals("org.apache.maven.plugins:maven-compiler-plugin")) {

diff --git a/...en-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/LocalProject.java b/...en-resolver/src/main/java/io/quarkus/bootstrap/resolver/maven/workspace/LocalProject.java
@@ -10,11 +10,8 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.Parent;
 import org.apache.maven.model.Resource;
@@ -331,43 +328,6 @@ public AppArtifact getAppArtifact(String extension) {
         return new AppArtifact(groupId, artifactId, "", extension, version);
     }
 
-    public List<LocalProject> getSelfWithLocalDeps() {
-        if (workspace == null) {
-            return Collections.singletonList(this);
-        }
-        final List<LocalProject> ordered = new ArrayList<>();
-        collectSelfWithLocalDeps(this, new HashSet<>(), ordered);
-        return ordered;
-    }
-
-    private static void collectSelfWithLocalDeps(LocalProject project, Set<AppArtifactKey> addedDeps,
-            List<LocalProject> ordered) {
-        if (!project.modules.isEmpty()) {
-            for (LocalProject module : project.modules) {
-                collectSelfWithLocalDeps(module, addedDeps, ordered);
-            }
-        }
-        for (Dependency dep : project.getRawModel().getDependencies()) {
-            if ("test".equals(dep.getScope())) {
-                continue;
-            }
-            final AppArtifactKey depKey = project.getKey(dep);
-            final LocalProject localDep = project.workspace.getProject(depKey);
-            if (localDep == null || addedDeps.contains(depKey)) {
-                continue;
-            }
-            collectSelfWithLocalDeps(localDep, addedDeps, ordered);
-        }
-        if (addedDeps.add(project.getKey())) {
-            ordered.add(project);
-        }
-    }
-
-    private AppArtifactKey getKey(Dependency dep) {
-        return new AppArtifactKey(PROJECT_GROUPID.equals(dep.getGroupId()) ? getGroupId() : dep.getGroupId(),
-                dep.getArtifactId());
-    }
-
     private Path resolveRelativeToBaseDir(String path, String defaultPath) {
         return dir.resolve(path == null ? defaultPath : stripProjectBasedirPrefix(path));
     }

diff --git a/...solver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java b/...solver/src/test/java/io/quarkus/bootstrap/workspace/test/LocalWorkspaceDiscoveryTest.java
@@ -17,7 +17,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import org.apache.maven.model.Dependency;
@@ -200,8 +199,6 @@ public void loadIndependentProjectInTheWorkspaceTree() throws Exception {
         final Map<AppArtifactKey, LocalProject> projects = project.getWorkspace().getProjects();
         assertEquals(6, projects.size());
         assertTrue(projects.containsKey(new AppArtifactKey(MvnProjectBuilder.DEFAULT_GROUP_ID, "independent")));
-
-        assertLocalDeps(project, "root-module-not-direct-child", "root-no-parent-module", "root-module-with-parent");
     }
 
     @Test
@@ -213,7 +210,6 @@ public void loadModuleProjectWithoutParent() throws Exception {
         assertEquals(MvnProjectBuilder.DEFAULT_GROUP_ID, project.getGroupId());
         assertEquals("root-no-parent-module", project.getArtifactId());
         assertEquals(MvnProjectBuilder.DEFAULT_VERSION, project.getVersion());
-        assertLocalDeps(project);
     }
 
     @Test
@@ -228,7 +224,6 @@ public void loadWorkspaceForModuleWithoutParent() throws Exception {
         final Map<AppArtifactKey, LocalProject> projects = project.getWorkspace().getProjects();
         assertEquals(5, projects.size());
         assertTrue(projects.containsKey(new AppArtifactKey(MvnProjectBuilder.DEFAULT_GROUP_ID, "root-no-parent-module")));
-        assertLocalDeps(project, "root-module-not-direct-child");
     }
 
     @Test
@@ -240,7 +235,6 @@ public void loadModuleProjectWithParent() throws Exception {
         assertEquals(MvnProjectBuilder.DEFAULT_GROUP_ID, project.getGroupId());
         assertEquals("root-module-with-parent", project.getArtifactId());
         assertEquals(MvnProjectBuilder.DEFAULT_VERSION, project.getVersion());
-        assertLocalDeps(project);
     }
 
     @Test
@@ -254,7 +248,6 @@ public void loadWorkspaceForModuleWithParent() throws Exception {
         assertEquals(MvnProjectBuilder.DEFAULT_VERSION, project.getVersion());
 
         assertCompleteWorkspace(project);
-        assertLocalDeps(project, "root-module-not-direct-child", "root-no-parent-module");
     }
 
     @Test
@@ -268,7 +261,6 @@ public void loadWorkspaceForModuleWithNotDirectParentPath() throws Exception {
         assertEquals(MvnProjectBuilder.DEFAULT_VERSION, project.getVersion());
 
         assertCompleteWorkspace(project);
-        assertLocalDeps(project);
     }
 
     @Test
@@ -289,7 +281,6 @@ public void loadNonModuleChildProject() throws Exception {
         assertTrue(projects.containsKey(new AppArtifactKey(MvnProjectBuilder.DEFAULT_GROUP_ID, "another-child")));
         assertTrue(projects
                 .containsKey(new AppArtifactKey(MvnProjectBuilder.DEFAULT_GROUP_ID, "empty-parent-relative-path-module")));
-        assertLocalDeps(project, "another-child");
     }
 
     /**
@@ -306,7 +297,6 @@ public void loadWorkspaceForModuleWithEmptyRelativePathParent() throws Exception
         assertEquals(MvnProjectBuilder.DEFAULT_VERSION, project.getVersion());
 
         assertCompleteWorkspace(project);
-        assertLocalDeps(project);
     }
 
     @Test
@@ -402,19 +392,4 @@ private void assertCompleteWorkspace(final LocalProject project) {
                 .containsKey(new AppArtifactKey(MvnProjectBuilder.DEFAULT_GROUP_ID, "empty-parent-relative-path-module")));
         assertTrue(projects.containsKey(new AppArtifactKey(MvnProjectBuilder.DEFAULT_GROUP_ID, "root")));
     }
-
-    private static void assertLocalDeps(LocalProject project, String... deps) {
-        final List<LocalProject> list = project.getSelfWithLocalDeps();
-        assertEquals(deps.length + 1, list.size());
-        int i = 0;
-        while (i < deps.length) {
-            final LocalProject dep = list.get(i);
-            assertEquals(deps[i++], dep.getArtifactId());
-            assertEquals(project.getGroupId(), dep.getGroupId());
-        }
-        final LocalProject self = list.get(i);
-        assertEquals(project.getGroupId(), self.getGroupId());
-        assertEquals(project.getArtifactId(), self.getArtifactId());
-        assertEquals(project.getVersion(), self.getVersion());
-    }
 }
diff --git a/integration-tests/maven/src/test/java/io/quarkus/maven/it/DevMojoIT.java b/integration-tests/maven/src/test/java/io/quarkus/maven/it/DevMojoIT.java
@@ -4,17 +4,21 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.nio.charset.Charset;
+import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Properties;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
@@ -236,6 +240,23 @@ public void testThatTheApplicationIsReloadedOnPomChange() throws MavenInvocation
                 .atMost(1, TimeUnit.MINUTES).until(() -> DevModeTestUtils.getHttpResponse("/openapi").contains("hello"));
     }
 
+    @Test
+    public void testProjectWithExtension() throws MavenInvocationException, IOException {
+        testDir = getTargetDir("projects/project-with-extension");
+        runAndCheck();
+
+        final List<String> extDepWarnings = Files.readAllLines(testDir.toPath().resolve("build-project-with-extension.log"))
+                .stream().filter(s -> s.startsWith("[WARNING] Local Quarkus extension dependency "))
+                .collect(Collectors.toList());
+        assertTrue(extDepWarnings
+                .contains("[WARNING] Local Quarkus extension dependency org.acme:acme-quarkus-ext will not be hot-reloadable"));
+        assertTrue(extDepWarnings
+                .contains("[WARNING] Local Quarkus extension dependency org.acme:acme-common will not be hot-reloadable"));
+        assertTrue(extDepWarnings.contains(
+                "[WARNING] Local Quarkus extension dependency org.acme:acme-common-transitive will not be hot-reloadable"));
+        assertEquals(3, extDepWarnings.size());
+    }
+
     @Test
     public void testThatTheApplicationIsReloadedMultiModule() throws MavenInvocationException, IOException {
         testDir = initProject("projects/multimodule", "projects/multimodule-with-deps");
@@ -817,4 +838,10 @@ public void testResourcesFromClasspath() throws MavenInvocationException, IOExce
         // test that we don't get multiple instances of a resource when loading from the ClassLoader
         assertThat(DevModeTestUtils.getHttpResponse("/cp/resourcesCount")).isEqualTo("1");
     }
+
+    @Test
+    public void testThatDependencyInParentIsEvaluated() throws IOException, MavenInvocationException {
+        testDir = initProject("projects/multimodule-parent-dep");
+        runAndCheck();
+    }
 }
diff --git a/integration-tests/maven/src/test/resources/projects/multimodule-parent-dep/level0/pom.xml b/integration-tests/maven/src/test/resources/projects/multimodule-parent-dep/level0/pom.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project>
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.acme</groupId>
+        <artifactId>quarkus-quickstart-multimodule-parent</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>quarkus-quickstart-multimodule-level0</artifactId>
+
+</project>
diff --git a/...s/projects/multimodule-parent-dep/level0/src/main/java/org/acme/level0/Level0Service.java b/...s/projects/multimodule-parent-dep/level0/src/main/java/org/acme/level0/Level0Service.java
@@ -0,0 +1,16 @@
+package org.acme.level0;
+
+import javax.enterprise.context.ApplicationScoped;
+
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+
+@ApplicationScoped
+public class Level0Service {
+
+    @ConfigProperty(name = "greeting")
+    String greeting;
+
+    public String getGreeting() {
+        return greeting;
+    }
+}
diff --git a/...st/resources/projects/multimodule-parent-dep/level0/src/main/resources/META-INF/beans.xml b/...st/resources/projects/multimodule-parent-dep/level0/src/main/resources/META-INF/beans.xml
diff --git a/integration-tests/maven/src/test/resources/projects/multimodule-parent-dep/level1/pom.xml b/integration-tests/maven/src/test/resources/projects/multimodule-parent-dep/level1/pom.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project>
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.acme</groupId>
+        <artifactId>quarkus-quickstart-multimodule-parent</artifactId>
+        <version>1.0-SNAPSHOT</version>
+        <relativePath>../</relativePath>
+    </parent>
+    <artifactId>quarkus-quickstart-multimodule-level1</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.acme</groupId>
+            <artifactId>quarkus-quickstart-multimodule-level0</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+    </dependencies>
+</project>