Simplify BundleManager so that it only handles app bundles.

- Fail if an app bundle tries to pre-install other bundles.

1 files changed, 27 insertions, 101 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java
index 406d68408e3..3727da66b63 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java
@@ -1,7 +1,6 @@
 // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.core.config;
 
-import com.yahoo.collections.PredicateSplit;
 import com.yahoo.config.FileReference;
 import com.yahoo.container.Container;
 import com.yahoo.filedistribution.fileacquirer.FileAcquirer;
@@ -18,9 +17,6 @@ import java.util.Set;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
-import static com.yahoo.collections.PredicateSplit.partition;
-import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX;
-
 /**
  * Manages the set of installed and active/inactive bundles.
  *
@@ -29,15 +25,14 @@ import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX
  */
 public class BundleManager {
 
-    /* Map of file refs of active bundles (not scheduled for uninstall) to a list of all bundles that were installed
-     * (pre-install directive) by the bundle pointed to by the file ref (including itself).
+    /* Map of file refs of active bundles (not scheduled for uninstall) to the installed bundle.
      *
      * Used to:
      * 1. Avoid installing already installed bundles. Just an optimization, installing the same bundle location is a NOP
      * 2. Start bundles (all are started every time)
      * 3. Calculate the set of bundles to uninstall
      */
-    private final Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>();
+    private final Map<FileReference, Bundle> reference2Bundle = new LinkedHashMap<>();
 
     private final Logger log = Logger.getLogger(BundleManager.class.getName());
     private final Osgi osgi;
@@ -54,101 +49,45 @@ public class BundleManager {
      * by the application, and should therefore be scheduled for uninstall.
      */
     public synchronized Set<Bundle> use(List<FileReference> newFileReferences) {
-        // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles
-        Set<Bundle> bundlesToUninstall = getObsoleteBundles(newFileReferences);
 
         Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newFileReferences);
-        allowDuplicateBundles(obsoleteReferences);
+        Set<Bundle> bundlesToUninstall = getObsoleteBundles(obsoleteReferences);
+        log.info("Bundles to schedule for uninstall: " + bundlesToUninstall);
+
+        osgi.allowDuplicateBundles(bundlesToUninstall);
         removeInactiveFileReferences(obsoleteReferences);
 
         installBundles(newFileReferences);
         startBundles();
-
-        bundlesToUninstall.removeAll(allActiveBundles());
-        log.info("Bundles to schedule for uninstall: " + bundlesToUninstall);
-
         log.info(installedBundlesMessage());
-        return bundlesToUninstall;
-    }
 
-    /**
-     * Returns the bundles that are not assumed to be retained by the new application generation.
-     * Note that at this point we don't yet know the full set of new bundles, because of the potential
-     * pre-install directives in the new bundles. However, only "disk bundles" (file:) can be listed
-     * in the pre-install directive, so we know about all the obsolete application bundles.
-     */
-    private Set<Bundle> getObsoleteBundles(List<FileReference> newReferences) {
-        Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles());
-
-        for (FileReference fileReferenceToKeep : newReferences) {
-            if (reference2Bundles.containsKey(fileReferenceToKeep)) {
-                bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep));
-            }
-        }
-        bundlesToRemove.removeAll(osgi.getInitialBundles());
-        return bundlesToRemove;
+        return bundlesToUninstall;
     }
 
-
     private Set<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) {
-        Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundles.keySet());
+        Set<FileReference> obsoleteReferences = new HashSet<>(reference2Bundle.keySet());
         obsoleteReferences.removeAll(newReferences);
         return obsoleteReferences;
     }
 
+
     /**
-     * Allow duplicates (bsn+version) for each bundle that corresponds to obsolete file references,
-     * and avoid allowing duplicates for bundles that were installed via the
-     * X-JDisc-Preinstall-Bundle directive. These bundles are always "disk bundles" (library
-     * bundles installed on the node, and not transferred via file distribution).
-     * Such bundles will never have duplicates because they always have the same location id.
+     * Returns the bundles that will not be retained by the new application generation.
      */
-    private void allowDuplicateBundles(Set<FileReference> obsoleteReferences) {
-        // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location
-        Set<Bundle> allowedDuplicates = obsoleteReferences.stream()
-                .filter(reference -> ! isDiskBundle(reference))
-                .map(reference -> reference2Bundles.get(reference).get(0))
-                .collect(Collectors.toSet());
-
-        log.info(() -> allowedDuplicates.isEmpty() ? "" : "Adding bundles to allowed duplicates: " + allowedDuplicates);
-        osgi.allowDuplicateBundles(allowedDuplicates);
+    private Set<Bundle> getObsoleteBundles(Set<FileReference> obsoleteReferences) {
+        return obsoleteReferences.stream().map(reference2Bundle::get).collect(Collectors.toSet());
     }
 
-    /**
-     * Cleans up the map of active file references
-     */
     private void removeInactiveFileReferences(Set<FileReference> fileReferencesToRemove) {
-        // Clean up the map of active bundles
-        fileReferencesToRemove.forEach(reference2Bundles::remove);
+        fileReferencesToRemove.forEach(reference2Bundle::remove);
     }
 
     private void installBundles(List<FileReference> references) {
         Set<FileReference> bundlesToInstall = new HashSet<>(references);
 
         // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle.
-        bundlesToInstall.removeAll(reference2Bundles.keySet());
-
-        PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleManager::isDiskBundle);
-        installBundlesFromDisk(bundlesToInstall_isDisk.trueValues);
-        installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues);
-    }
-
-    private static boolean isDiskBundle(FileReference fileReference) {
-        return fileReference.value().startsWith(DISK_BUNDLE_PREFIX);
-    }
+        bundlesToInstall.removeAll(reference2Bundle.keySet());
 
-    private void installBundlesFromDisk(List<FileReference> bundlesToInstall) {
-        for (FileReference reference : bundlesToInstall) {
-            try {
-                installBundleFromDisk(reference);
-            }
-            catch(Exception e) {
-                throw new RuntimeException("Could not install bundle '" + reference + "'", e);
-            }
-        }
-    }
-
-    private void installBundlesFromFileDistribution(List<FileReference> bundlesToInstall) {
         if (!bundlesToInstall.isEmpty()) {
             FileAcquirer fileAcquirer = Container.get().getFileAcquirer();
             boolean hasFileDistribution = (fileAcquirer != null);
@@ -162,23 +101,18 @@ public class BundleManager {
         }
     }
 
-    private void installBundleFromDisk(FileReference reference) {
-        log.info("Installing bundle from disk with reference '" + reference.value() + "'");
-
-        var bundleInstaller = new DiskBundleInstaller();
-        List<Bundle> bundles = bundleInstaller.installBundles(reference, osgi);
-        reference2Bundles.put(reference, bundles);
-    }
-
-    private void installWithFileDistribution(List<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) {
+    private void installWithFileDistribution(Set<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) {
         for (FileReference reference : bundlesToInstall) {
             try {
                 log.info("Installing bundle with reference '" + reference.value() + "'");
                 List<Bundle> bundles = bundleInstaller.installBundles(reference, osgi);
-                reference2Bundles.put(reference, bundles);
+                if (bundles.size() > 1) {
+                    throw new RuntimeException("Bundle '" + bundles.get(0).getSymbolicName() + "' tried to pre-install other bundles.");
+                }
+                reference2Bundle.put(reference, bundles.get(0));
             }
             catch(Exception e) {
-                throw new RuntimeException("Could not install bundle '" + reference + "'", e);
+                throw new RuntimeException("Could not install bundle with reference '" + reference + "'", e);
             }
         }
     }
@@ -188,14 +122,12 @@ public class BundleManager {
      * after all bundles are installed to ensure that the framework can resolve dependencies between bundles.
      */
     private void startBundles() {
-        for (List<Bundle> bundles : reference2Bundles.values()) {
-            for (Bundle bundle : bundles) {
-                try {
-                    if ( ! isFragment(bundle))
-                        bundle.start();  // NOP for already ACTIVE bundles
-                } catch(Exception e) {
-                    throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e);
-                }
+        for (var bundle : reference2Bundle.values()) {
+            try {
+                if ( ! isFragment(bundle))
+                    bundle.start();  // NOP for already ACTIVE bundles
+            } catch(Exception e) {
+                throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e);
             }
         }
     }
@@ -208,12 +140,6 @@ public class BundleManager {
         return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0;
     }
 
-    private Set<Bundle> allActiveBundles() {
-        return reference2Bundles.keySet().stream()
-                .flatMap(reference -> reference2Bundles.get(reference).stream())
-                .collect(Collectors.toSet());
-    }
-
     private String installedBundlesMessage() {
         StringBuilder sb = new StringBuilder("Installed bundles: {" );
         for (Bundle b : osgi.getBundles())
@@ -230,7 +156,7 @@ public class BundleManager {
 
     // Only for testing
     List<FileReference> getActiveFileReferences() {
-        return new ArrayList<>(reference2Bundles.keySet());
+        return new ArrayList<>(reference2Bundle.keySet());
     }
 
 }