Log embedded packages that are presumably unnecessary

(included in both compile scoped jars and provided jars)

* Add methods and refactor to facilitate this feature.

3 files changed, 41 insertions, 11 deletions

diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java
index ecd95dbfdba..0626c786822 100644
--- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java
+++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java
@@ -11,6 +11,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.jar.Manifest;
 import java.util.stream.Collectors;
 
@@ -26,10 +27,18 @@ public class AnalyzeBundle {
         public final List<Export> exports;
         public final List<String> globals;
 
-        public PublicPackages(List<Export> exports, List<String> globals) {
+        PublicPackages(List<Export> exports, List<String> globals) {
             this.exports = exports;
             this.globals = globals;
         }
+
+        public Set<String> exportedPackageNames() {
+            return exports.stream()
+                    .map(Export::getPackageNames)
+                    .flatMap(Collection::stream)
+                    .collect(Collectors.toSet());
+        }
+
     }
 
     public static PublicPackages publicPackagesAggregated(Collection<File> jarFiles) {
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java
index 2389849ad02..685cda66a16 100644
--- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java
+++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java
@@ -100,6 +100,9 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
             AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars = publicPackagesAggregated(
                     artifactSet.getJarArtifactsProvided().stream().map(Artifact::getFile).collect(Collectors.toList()));
 
+            // Packages from Export-Package headers in provided scoped jars
+            Set<String> exportedPackagesFromProvidedDeps = publicPackagesFromProvidedJars.exportedPackageNames();
+
             // Packaged defined in this project's code
             PackageTally projectPackages = getProjectClassesTally();
 
@@ -110,16 +113,16 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
             PackageTally includedPackages = projectPackages.combine(compileJarsPackages);
 
             warnIfPackagesDefinedOverlapsGlobalPackages(includedPackages.definedPackages(), publicPackagesFromProvidedJars.globals);
-
             logDebugPackageSets(publicPackagesFromProvidedJars, includedPackages);
 
             if (hasJdiscCoreProvided(artifactSet.getJarArtifactsProvided())) {
-                // jdisc_core being provided, guarantees that log output does not contain its exported packages
-                logMissingPackages(publicPackagesFromProvidedJars.exports, projectPackages, compileJarsPackages, includedPackages);
+                // jdisc_core being provided guarantees that log output does not contain its exported packages
+                logMissingPackages(exportedPackagesFromProvidedDeps, projectPackages, compileJarsPackages, includedPackages);
             } else {
                 getLog().warn("This project does not have jdisc_core as provided dependency, so the " +
                                       "generated 'Import-Package' OSGi header may be missing important packages.");
             }
+            logUnnecessaryPackages(compileJarsPackages, exportedPackagesFromProvidedDeps);
 
             Map<String, Import> calculatedImports = calculateImports(includedPackages.referencedPackages(),
                                                                      includedPackages.definedPackages(),
@@ -138,6 +141,20 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
         }
     }
 
+    /**
+     * This mostly detects packages re-exported via composite bundles like jdisc_core and container-disc.
+     * An artifact can only be represented once, either in compile or provided scope. So if the project
+     * adds an artifact in compile scope that we deploy as a pre-installed bundle, we won't see the same
+     * artifact as provided via container-dev and hence can't detect the duplicate packages.
+     */
+    private void logUnnecessaryPackages(PackageTally compileJarsPackages, Set<String> exportedPackagesFromProvidedDeps) {
+        Set<String> unnecessaryPackages = Sets.intersection(compileJarsPackages.definedPackages(), exportedPackagesFromProvidedDeps);
+        if (! unnecessaryPackages.isEmpty()) {
+            getLog().info("Compile scoped jars contain the following packages that are most likely " +
+                                  "available from jdisc runtime: " + unnecessaryPackages);
+        }
+    }
+
     private void logDebugPackageSets(AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars, PackageTally includedPackages) {
         if (getLog().isDebugEnabled()) {
             getLog().debug("Referenced packages = " + includedPackages.referencedPackages());
@@ -151,17 +168,12 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
         return providedArtifacts.stream().anyMatch(artifact -> artifact.getArtifactId().equals("jdisc_core"));
     }
 
-    private void logMissingPackages(List<Export> exportedPackagesFromProvidedJars,
+    private void logMissingPackages(Set<String> exportedPackagesFromProvidedJars,
                                     PackageTally projectPackages,
                                     PackageTally compileJarPackages,
                                     PackageTally includedPackages) {
-        Set<String> exportedPackagesFromProvidedDeps = exportedPackagesFromProvidedJars
-                .stream()
-                .map(Export::getPackageNames)
-                .flatMap(Collection::stream)
-                .collect(Collectors.toSet());
 
-        Set<String> definedAndExportedPackages = Sets.union(includedPackages.definedPackages(), exportedPackagesFromProvidedDeps);
+        Set<String> definedAndExportedPackages = Sets.union(includedPackages.definedPackages(), exportedPackagesFromProvidedJars);
 
         Set<String> missingProjectPackages = projectPackages.referencedPackagesMissingFrom(definedAndExportedPackages);
         if (! missingProjectPackages.isEmpty()) {
diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java
index 8cccf0598ab..a09604b842b 100644
--- a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java
+++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java
@@ -11,8 +11,10 @@ import org.junit.rules.ExpectedException;
 
 import java.io.File;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static com.yahoo.container.plugin.classanalysis.TestUtilities.throwableMessage;
 import static org.hamcrest.CoreMatchers.containsString;
@@ -28,6 +30,7 @@ import static org.junit.Assert.assertThat;
  */
 public class AnalyzeBundleTest {
     private final List<ExportPackages.Export> exports;
+    private final Set<String> exportedPackageNames;
     private final Map<String, ExportPackages.Export> exportsByPackageName;
 
     File jarDir = new File("src/test/resources/jar");
@@ -37,6 +40,7 @@ public class AnalyzeBundleTest {
         File simple = new File(jarDir, "simple1.jar");
         PublicPackages pp = AnalyzeBundle.publicPackagesAggregated(Arrays.asList(notOsgi, simple));
         this.exports = pp.exports;
+        this.exportedPackageNames = pp.exportedPackageNames();
         this.exportsByPackageName = ExportPackages.exportsByPackageName(exports);
     }
 
@@ -55,6 +59,11 @@ public class AnalyzeBundleTest {
         assertThat(exportsByPackageName.keySet(), hasItem("com.yahoo.sample.exported.package"));
     }
 
+    @Test
+    public void exported_class_names_can_be_retrieved() {
+        assertThat(exportedPackageNames, is(new HashSet<>(exports.get(0).getPackageNames())));
+    }
+
     @Rule
     public ExpectedException exception = ExpectedException.none();