Revert "Gjoranv/allow duplicate bundles"

5 files changed, 22 insertions, 57 deletions

diff --git a/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java b/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java
index 61497cf71bc..09f72c9d86d 100644
--- a/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java
+++ b/container-di/src/main/java/com/yahoo/container/di/ComponentDeconstructor.java
@@ -1,8 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.di;
 
-import org.osgi.framework.Bundle;
-
 import java.util.Collection;
 
 /**
@@ -10,7 +8,5 @@ import java.util.Collection;
  * @author Tony Vaagenes
  */
 public interface ComponentDeconstructor {
-
-    void deconstruct(Collection<Object> components, Collection<Bundle> bundles);
-
+    void deconstruct(Collection<Object> components);
 }
diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java
index 9a9245f4ba2..f82fd22f40d 100644
--- a/container-di/src/main/java/com/yahoo/container/di/Container.java
+++ b/container-di/src/main/java/com/yahoo/container/di/Container.java
@@ -18,13 +18,9 @@ import com.yahoo.container.di.componentgraph.core.Node;
 import com.yahoo.container.di.config.RestApiContext;
 import com.yahoo.container.di.config.SubscriberFactory;
 import com.yahoo.vespa.config.ConfigKey;
-import org.osgi.framework.Bundle;
 
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.logging.Level;
@@ -69,22 +65,19 @@ public class Container {
         });
     }
 
-    private void deconstructObsoleteComponents(ComponentGraph oldGraph,
-                                               ComponentGraph newGraph,
-                                               Collection<Bundle> obsoleteBundles) {
+    private void deconstructObsoleteComponents(ComponentGraph oldGraph, ComponentGraph newGraph) {
         IdentityHashMap<Object, Object> oldComponents = new IdentityHashMap<>();
         oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null));
         newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove);
-        componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles);
+        componentDeconstructor.deconstruct(oldComponents.keySet());
     }
 
     public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) {
         try {
-            Collection<Bundle> obsoleteBundles = new HashSet<>();
-            ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy, obsoleteBundles);
+            ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy);
             newGraph.reuseNodes(oldGraph);
             constructComponents(newGraph);
-            deconstructObsoleteComponents(oldGraph, newGraph, obsoleteBundles);
+            deconstructObsoleteComponents(oldGraph, newGraph);
             return newGraph;
         } catch (Throwable t) {
             // TODO: Wrap ComponentConstructorException in an Error when generation==0 (+ unit test that Error is thrown)
@@ -129,13 +122,8 @@ public class Container {
         }
     }
 
-    private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph,
-                                                   Injector fallbackInjector,
-                                                   boolean restartOnRedeploy,
-                                                   Collection<Bundle> obsoleteBundles) // NOTE: Return value
-    {
+    private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, Injector fallbackInjector, boolean restartOnRedeploy) {
         ConfigSnapshot snapshot;
-
         while (true) {
             snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, restartOnRedeploy);
 
@@ -143,6 +131,7 @@ public class Container {
                     graph.configKeys(), graph.generation(), snapshot));
 
             if (snapshot instanceof BootstrapConfigs) {
+                // TODO: remove require when proven unnecessary
                 if (getBootstrapGeneration() <= previousConfigGeneration) {
                     throw new IllegalStateException(String.format(
                             "Got bootstrap configs out of sequence for old config generation %d.\n" + "Previous config generation is %d",
@@ -153,12 +142,8 @@ public class Container {
                                 "Got new bootstrap generation\n" + "bootstrap generation = %d\n" + "components generation: %d\n"
                                         + "previous generation: %d\n",
                                 getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration));
-
-                Collection<Bundle> bundlesToRemove = installBundles(snapshot.configs());
-                obsoleteBundles.addAll(bundlesToRemove);
-
+                installBundles(snapshot.configs());
                 graph = createComponentsGraph(snapshot.configs(), getBootstrapGeneration(), fallbackInjector);
-
                 // Continues loop
 
             } else if (snapshot instanceof ConfigRetriever.ComponentsConfigs) {
@@ -199,9 +184,9 @@ public class Container {
         }
     }
 
-    private Set<Bundle> installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) {
+    private void installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) {
         BundlesConfig bundlesConfig = getConfig(bundlesConfigKey, configsIncludingBootstrapConfigs);
-        return osgi.useBundles(bundlesConfig.bundle());
+        osgi.useBundles(bundlesConfig.bundle());
     }
 
     private ComponentGraph createComponentsGraph(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs,
@@ -259,8 +244,7 @@ public class Container {
     }
 
     private void deconstructAllComponents(ComponentGraph graph, ComponentDeconstructor deconstructor) {
-        // This is only used for shutdown, so no need to uninstall any bundles.
-        deconstructor.deconstruct(graph.allConstructedComponentsAndProviders(), Collections.emptyList());
+        deconstructor.deconstruct(graph.allConstructedComponentsAndProviders());
     }
 
     public static <T extends ConfigInstance> T getConfig(ConfigKey<T> key,
diff --git a/container-di/src/main/java/com/yahoo/container/di/Osgi.java b/container-di/src/main/java/com/yahoo/container/di/Osgi.java
index ab7da7665b6..7095180dfc5 100644
--- a/container-di/src/main/java/com/yahoo/container/di/Osgi.java
+++ b/container-di/src/main/java/com/yahoo/container/di/Osgi.java
@@ -13,8 +13,6 @@ import java.util.Collections;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import static java.util.Collections.emptySet;
-
 /**
  * @author gjoranv
  * @author Tony Vaagenes
@@ -25,13 +23,8 @@ public interface Osgi {
         return new BundleClasses(new MockBundle(), Collections.emptySet());
     }
 
-    /**
-     * Returns the set of bundles that is not used by the current application generation,
-     * and therefore should be scheduled for uninstalling.
-     */
-    default Set<Bundle> useBundles(Collection<FileReference> bundles) {
+    default void useBundles(Collection<FileReference> bundles) {
         System.out.println("useBundles " + bundles.stream().map(Object::toString).collect(Collectors.joining(", ")));
-        return emptySet();
     }
 
     default Class<?> resolveClass(BundleInstantiationSpecification spec) {
diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java
index 90bda0ef8a8..9c4891c7db2 100644
--- a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java
+++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java
@@ -14,8 +14,6 @@ import com.yahoo.container.di.componentgraph.core.Node;
 import com.yahoo.container.di.config.RestApiContext;
 import org.junit.Ignore;
 import org.junit.Test;
-import org.osgi.framework.Bundle;
-import org.osgi.framework.BundleException;
 
 import java.util.Collection;
 import java.util.concurrent.ExecutionException;
@@ -285,16 +283,13 @@ public class ContainerTest extends ContainerTestBase {
     public void providers_are_destructed() {
         writeBootstrapConfigs("id1", DestructableProvider.class);
 
-        ComponentDeconstructor deconstructor = (components, bundles) -> {
-            components.forEach(component -> {
-                if (component instanceof AbstractComponent) {
-                    ((AbstractComponent) component).deconstruct();
-                } else if (component instanceof Provider) {
-                    ((Provider<?>) component).deconstruct();
-                }
-            });
-            if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles");
-        };
+        ComponentDeconstructor deconstructor = components -> components.forEach(component -> {
+            if (component instanceof AbstractComponent) {
+                ((AbstractComponent) component).deconstruct();
+            } else if (component instanceof Provider) {
+                ((Provider<?>) component).deconstruct();
+            }
+        });
 
         Container container = newContainer(dirConfigSource, deconstructor);
 
@@ -378,14 +373,13 @@ public class ContainerTest extends ContainerTestBase {
 
     public static class TestDeconstructor implements ComponentDeconstructor {
         @Override
-        public void deconstruct(Collection<Object> components, Collection<Bundle> bundles) {
+        public void deconstruct(Collection<Object> components) {
             components.forEach(component -> {
                 if (component instanceof DestructableComponent) {
                     DestructableComponent vespaComponent = (DestructableComponent) component;
                     vespaComponent.deconstruct();
                 }
             });
-            if (! bundles.isEmpty()) throw new IllegalArgumentException("This test should not use bundles");
         }
     }
 
diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java
index 18236a6bde9..79cb080dfa4 100644
--- a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java
+++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java
@@ -5,6 +5,7 @@ import com.google.inject.Guice;
 import com.yahoo.component.ComponentSpecification;
 import com.yahoo.config.FileReference;
 import com.yahoo.container.bundle.BundleInstantiationSpecification;
+import com.yahoo.container.di.CloudSubscriberFactory;
 import com.yahoo.container.di.ContainerTest.ComponentTakingConfig;
 import com.yahoo.container.di.componentgraph.core.ComponentGraph;
 import com.yahoo.container.di.osgi.BundleClasses;
@@ -15,8 +16,6 @@ import org.osgi.framework.Bundle;
 import java.util.Collection;
 import java.util.Set;
 
-import static java.util.Collections.emptySet;
-
 /**
  * @author Tony Vaagenes
  * @author gjoranv
@@ -61,8 +60,7 @@ public class ContainerTestBase {
                         }
 
                         @Override
-                        public Set<Bundle> useBundles(Collection<FileReference> bundles) {
-                            return emptySet();
+                        public void useBundles(Collection<FileReference> bundles) {
                         }
 
                         @Override