Revert "Gjoranv/allow duplicate bundles"

8 files changed, 64 insertions, 181 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java
index 4b8f21469d1..2b3a272fadc 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleLoader.java
@@ -10,6 +10,7 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.wiring.BundleRevision;
 
 import java.io.File;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -25,18 +26,11 @@ import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX
  * Manages the set of installed 3rd-party component bundles.
  *
  * @author Tony Vaagenes
- * @author gjoranv
  */
 public class BundleLoader {
 
-    /* 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).
-     *
-     * 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 List<Bundle> initialBundles;
+
     private final Map<FileReference, List<Bundle>> reference2Bundles = new LinkedHashMap<>();
 
     private final Logger log = Logger.getLogger(BundleLoader.class.getName());
@@ -44,6 +38,7 @@ public class BundleLoader {
 
     public BundleLoader(Osgi osgi) {
         this.osgi = osgi;
+        initialBundles = Arrays.asList(osgi.getBundles());
     }
 
     private List<Bundle> obtainBundles(FileReference reference, FileAcquirer fileAcquirer) throws InterruptedException {
@@ -51,19 +46,17 @@ public class BundleLoader {
         return osgi.install(file.getAbsolutePath());
     }
 
-    private void install(List<FileReference> references) {
+    /** Returns the number of bundles installed by this call. */
+    private int install(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.
-        // It's ok that fileRefs pending uninstall are removed from the map, because they are never in the new set of bundles..
         bundlesToInstall.removeAll(reference2Bundles.keySet());
 
         PredicateSplit<FileReference> bundlesToInstall_isDisk = partition(bundlesToInstall, BundleLoader::isDiskBundle);
         installBundlesFromDisk(bundlesToInstall_isDisk.trueValues);
         installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues);
 
-        // TODO: Remove. Bundles are also started in use()
         startBundles();
+        return bundlesToInstall.size();
     }
 
     private static boolean isDiskBundle(FileReference fileReference) {
@@ -104,7 +97,6 @@ public class BundleLoader {
         }
 
         List<Bundle> bundles = osgi.install(file.getAbsolutePath());
-
         reference2Bundles.put(reference, bundles);
     }
 
@@ -121,16 +113,13 @@ public class BundleLoader {
         }
     }
 
-    /**
-     * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place
-     * after all bundles are installed to ensure that the framework can resolve dependencies between bundles.
-     */
+    // All bundles must have been started first to ensure correct package resolution.
     private void startBundles() {
         for (List<Bundle> bundles : reference2Bundles.values()) {
             for (Bundle bundle : bundles) {
                 try {
                     if ( ! isFragment(bundle))
-                        bundle.start();  // NOP for already ACTIVE bundles
+                        bundle.start();
                 } catch(Exception e) {
                     throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e);
                 }
@@ -146,44 +135,38 @@ public class BundleLoader {
         return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0;
     }
 
-    /**
-     * Returns the bundles to schedule for uninstall after their components have been deconstructed
-     * and removes the same bundles from the map of active bundles.
-     */
-    private Set<Bundle> getBundlesToUninstall(List<FileReference> newReferences) {
-        Set<Bundle> bundlesToRemove = new HashSet<>(osgi.getCurrentBundles());
+    /** Returns the number of uninstalled bundles */
+    private int retainOnly(List<FileReference> newReferences) {
+        Set<Bundle> bundlesToRemove = new HashSet<>(Arrays.asList(osgi.getBundles()));
 
         for (FileReference fileReferenceToKeep: newReferences) {
             if (reference2Bundles.containsKey(fileReferenceToKeep))
                 bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep));
         }
 
-        bundlesToRemove.removeAll(osgi.getInitialBundles());
-        removeInactiveFileReferences(newReferences);
-
-        return bundlesToRemove;
-    }
+        bundlesToRemove.removeAll(initialBundles);
+        for (Bundle bundle : bundlesToRemove) {
+            log.info("Removing bundle '" + bundle.toString() + "'");
+            osgi.uninstall(bundle);
+        }
 
-    private void removeInactiveFileReferences(List<FileReference> newReferences) {
-        // Clean up the map of active bundles
         Set<FileReference> fileReferencesToRemove = new HashSet<>(reference2Bundles.keySet());
         fileReferencesToRemove.removeAll(newReferences);
-        fileReferencesToRemove.forEach(reference2Bundles::remove);
+
+        for (FileReference fileReferenceToRemove : fileReferencesToRemove) {
+            reference2Bundles.remove(fileReferenceToRemove);
+        }
+        return bundlesToRemove.size();
     }
 
-    /**
-     * Installs the given set of bundles and returns the set of bundles that is no longer used
-     * by the application, and should therefore be scheduled for uninstall.
-     */
-    public synchronized Set<Bundle> use(List<FileReference> newBundles) {
-        Set<Bundle> bundlesToUninstall = getBundlesToUninstall(newBundles);
-        osgi.allowDuplicateBundles(bundlesToUninstall);
-        log.info(() -> bundlesToUninstall.isEmpty() ? "Adding bundles to allowed duplicates: " + bundlesToUninstall : "");
-        install(newBundles);
+    public synchronized int use(List<FileReference> bundles) {
+        int removedBundles = retainOnly(bundles);
+        int installedBundles = install(bundles);
         startBundles();
 
+        log.info(removedBundles + " bundles were removed, and " + installedBundles + " bundles were installed.");
         log.info(installedBundlesMessage());
-        return bundlesToUninstall;
+        return removedBundles + installedBundles;
     }
 
     private String installedBundlesMessage() {
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
index ef132694e10..dd8b2605820 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java
@@ -9,7 +9,6 @@ import com.yahoo.component.ComponentSpecification;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.config.FileReference;
-import com.yahoo.container.core.config.testutil.MockOsgiWrapper;
 import com.yahoo.container.di.ComponentDeconstructor;
 import com.yahoo.container.di.Container;
 import com.yahoo.container.di.componentgraph.core.ComponentGraph;
@@ -21,9 +20,10 @@ import com.yahoo.jdisc.application.OsgiFramework;
 import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.service.ClientProvider;
 import com.yahoo.jdisc.service.ServerProvider;
+import com.yahoo.language.Linguistics;
+import com.yahoo.language.simple.SimpleLinguistics;
 import com.yahoo.log.LogLevel;
 import com.yahoo.osgi.OsgiImpl;
-import com.yahoo.osgi.OsgiWrapper;
 import com.yahoo.statistics.Statistics;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.wiring.BundleWiring;
@@ -81,30 +81,19 @@ public class HandlersConfigurerDi {
                                 Injector discInjector,
                                 OsgiFramework osgiFramework) {
 
-        this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector,
-             new ContainerAndDiOsgi(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework))));
-    }
-
-    // Only public for testing
-    public HandlersConfigurerDi(SubscriberFactory subscriberFactory,
-                                com.yahoo.container.Container vespaContainer,
-                                String configId,
-                                ComponentDeconstructor deconstructor,
-                                Injector discInjector,
-                                OsgiWrapper osgiWrapper) {
-
         this.vespaContainer = vespaContainer;
-        this.osgiWrapper = osgiWrapper;
+        osgiWrapper = new OsgiWrapper(osgiFramework, new BundleLoader(new OsgiImpl(osgiFramework)));
+
         container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper);
         getNewComponentGraph(discInjector, false);
     }
 
-    private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper {
+    private static class OsgiWrapper extends OsgiImpl implements com.yahoo.container.di.Osgi {
 
         private final OsgiFramework osgiFramework;
         private final BundleLoader bundleLoader;
 
-        public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleLoader bundleLoader) {
+        public OsgiWrapper(OsgiFramework osgiFramework, BundleLoader bundleLoader) {
             super(osgiFramework);
             this.osgiFramework = osgiFramework;
             this.bundleLoader = bundleLoader;
@@ -132,9 +121,14 @@ public class HandlersConfigurerDi {
         }
 
         @Override
-        public Set<Bundle> useBundles(Collection<FileReference> bundles) {
+        public void useBundles(Collection<FileReference> bundles) {
             log.info("Installing bundles from the latest application");
-            return bundleLoader.use(new ArrayList<>(bundles));
+
+            int bundlesRemovedOrInstalled = bundleLoader.use(new ArrayList<>(bundles));
+
+            if (bundlesRemovedOrInstalled > 0) {
+                refreshPackages();
+            }
         }
     }
 
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java
index 9f49b016b68..684a45aeac1 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java
@@ -19,6 +19,7 @@ import com.yahoo.osgi.MockOsgi;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.Random;
 import java.util.Set;
@@ -89,7 +90,7 @@ public class HandlersConfigurerTestWrapper {
 
     public HandlersConfigurerTestWrapper(Container container, String configId) {
         createFiles(configId);
-        MockOsgiWrapper mockOsgiWrapper = new MockOsgiWrapper();
+        MockOsgi mockOsgi = new MockOsgi();
         ComponentDeconstructor testDeconstructor = getTestDeconstructor();
         configurer = new HandlersConfigurerDi(
                 new CloudSubscriberFactory(configSources),
@@ -97,17 +98,16 @@ public class HandlersConfigurerTestWrapper {
                 configId,
                 testDeconstructor,
                 guiceInjector(),
-                mockOsgiWrapper);
+                mockOsgi);
         this.container = container;
     }
 
     private ComponentDeconstructor getTestDeconstructor() {
-        return (components, bundles) -> components.forEach(component -> {
+        return components -> components.forEach(component -> {
             if (component instanceof AbstractComponent) {
                 AbstractComponent abstractComponent = (AbstractComponent) component;
                 if (abstractComponent.isDeconstructable()) abstractComponent.deconstruct();
             }
-            if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles");
         });
     }
 
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java
deleted file mode 100644
index a1f564f5682..00000000000
--- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java
+++ /dev/null
@@ -1,44 +0,0 @@
-package com.yahoo.container.core.config.testutil;
-
-import com.yahoo.component.ComponentSpecification;
-import com.yahoo.osgi.OsgiWrapper;
-import org.osgi.framework.Bundle;
-
-import java.util.Collection;
-import java.util.List;
-
-import static java.util.Collections.emptyList;
-
-/**
- * @author gjoranv
- */
-public class MockOsgiWrapper implements OsgiWrapper {
-
-    @Override
-    public List<Bundle> getInitialBundles() {
-        return emptyList();
-    }
-
-    @Override
-    public Bundle[] getBundles() {
-        return new Bundle[0];
-    }
-
-    @Override
-    public List<Bundle> getCurrentBundles() {
-        return emptyList();
-    }
-
-    @Override
-    public Bundle getBundle(ComponentSpecification bundleId) {
-        return null;
-    }
-
-    @Override
-    public List<Bundle> install(String absolutePath) {
-        return emptyList();
-    }
-
-    @Override
-    public void allowDuplicateBundles(Collection<Bundle> bundles) {  }
-}
diff --git a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java
index d809c493565..45ad02d2cef 100644
--- a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java
+++ b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java
@@ -16,28 +16,26 @@ import java.util.List;
 public class MockOsgi extends NonWorkingOsgiFramework implements Osgi {
 
     @Override
-    public List<Bundle> getInitialBundles() {
-        return Collections.emptyList();
+    public Bundle[] getBundles() {
+        return new Bundle[0];
     }
 
     @Override
-    public Bundle[] getBundles() {
-        return new Bundle[0];
+    public Bundle getBundle(ComponentSpecification bundleId) {
+        return null;
     }
 
     @Override
-    public List<Bundle> getCurrentBundles() {
+    public List<Bundle> install(String absolutePath) {
         return Collections.emptyList();
     }
 
     @Override
-    public Bundle getBundle(ComponentSpecification bundleId) {
-        return null;
+    public void uninstall(Bundle bundle) {
     }
 
     @Override
-    public List<Bundle> install(String absolutePath) {
-        return Collections.emptyList();
+    public void refreshPackages() {
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/Osgi.java b/container-core/src/main/java/com/yahoo/osgi/Osgi.java
index 8f0acf41f30..c94eaf43deb 100644
--- a/container-core/src/main/java/com/yahoo/osgi/Osgi.java
+++ b/container-core/src/main/java/com/yahoo/osgi/Osgi.java
@@ -4,7 +4,6 @@ package com.yahoo.osgi;
 import com.yahoo.component.ComponentSpecification;
 import org.osgi.framework.Bundle;
 
-import java.util.Collection;
 import java.util.List;
 
 /**
@@ -12,17 +11,14 @@ import java.util.List;
  */
 public interface Osgi {
 
-    List<Bundle> getInitialBundles();
-
     Bundle[] getBundles();
 
-    /** Returns all bundles that have not been scheduled for uninstall. */
-    List<Bundle> getCurrentBundles();
-
     Bundle getBundle(ComponentSpecification bundleId);
 
     List<Bundle> install(String absolutePath);
 
-    void allowDuplicateBundles(Collection<Bundle> bundles);
+    void uninstall(Bundle bundle);
+
+    void refreshPackages();
 
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java
index ed93d15c975..8b2f20a1c13 100644
--- a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java
+++ b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java
@@ -7,43 +7,19 @@ import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.jdisc.application.OsgiFramework;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
-import org.osgi.framework.launch.Framework;
 
-import java.util.Collection;
 import java.util.List;
-import java.util.logging.Logger;
 
 /**
  * @author Tony Vaagenes
  * @author bratseth
  */
 public class OsgiImpl implements Osgi {
-    private static final Logger log = Logger.getLogger(OsgiImpl.class.getName());
 
     private final OsgiFramework jdiscOsgi;
 
-    // The initial bundles are never scheduled for uninstall
-    private final List<Bundle> initialBundles;
-
-    // An initial bundle that is not the framework, and can hence be used to look up current bundles
-    private final Bundle alwaysCurrentBundle;
-
     public OsgiImpl(OsgiFramework jdiscOsgi) {
         this.jdiscOsgi = jdiscOsgi;
-
-        this.initialBundles = jdiscOsgi.bundles();
-        if (initialBundles.isEmpty())
-            throw new IllegalStateException("No initial bundles!");
-
-        alwaysCurrentBundle = firstNonFrameworkBundle(initialBundles);
-        if (alwaysCurrentBundle == null)
-            throw new IllegalStateException("The initial bundles only contained the framework bundle!");
-        log.info("Using " + alwaysCurrentBundle + " to lookup current bundles.");
-    }
-
-    @Override
-    public List<Bundle> getInitialBundles() {
-        return initialBundles;
     }
 
     @Override
@@ -52,10 +28,6 @@ public class OsgiImpl implements Osgi {
         return bundles.toArray(new Bundle[bundles.size()]);
     }
 
-    @Override
-    public List<Bundle> getCurrentBundles() {
-        return jdiscOsgi.getBundles(alwaysCurrentBundle);
-    }
 
     public Class<Object> resolveClass(BundleInstantiationSpecification spec) {
         Bundle bundle = getBundle(spec.bundle);
@@ -114,9 +86,8 @@ public class OsgiImpl implements Osgi {
      * @return the bundle match having the highest version, or null if there was no matches
      */
     public Bundle getBundle(ComponentSpecification id) {
-        log.fine(() -> "Getting bundle for component " + id + ". Set of current bundles: " + getCurrentBundles());
         Bundle highestMatch = null;
-        for (Bundle bundle : getCurrentBundles()) {
+        for (Bundle bundle : getBundles()) {
             assert bundle.getSymbolicName() != null : "ensureHasBundleSymbolicName not called during installation";
 
             if ( ! bundle.getSymbolicName().equals(id.getName())) continue;
@@ -151,16 +122,17 @@ public class OsgiImpl implements Osgi {
     }
 
     @Override
-    public void allowDuplicateBundles(Collection<Bundle> bundles) {
-        jdiscOsgi.allowDuplicateBundles(bundles);
+    public void uninstall(Bundle bundle) {
+        try {
+            bundle.uninstall();
+        } catch (BundleException e) {
+            throw new RuntimeException(e);
+        }
     }
 
-    private static Bundle firstNonFrameworkBundle(List<Bundle> bundles) {
-        for (Bundle b : bundles) {
-            if (! (b instanceof Framework))
-                return b;
-        }
-        return null;
+    @Override
+    public void refreshPackages() {
+        jdiscOsgi.refreshPackages();
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java b/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java
deleted file mode 100644
index 58e19e52ee3..00000000000
--- a/container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java
+++ /dev/null
@@ -1,16 +0,0 @@
-package com.yahoo.osgi;
-
-import com.yahoo.component.ComponentSpecification;
-import org.osgi.framework.Bundle;
-
-/**
- * @author gjoranv
- */
-public interface OsgiWrapper extends Osgi, com.yahoo.container.di.Osgi {
-
-    @Override
-    default Bundle getBundle(ComponentSpecification bundleId) {
-        return null;
-    }
-
-}