Merge pull request #13925 from vespa-engine/gjoranv/container-refactoring

Gjoranv/container refactoring

7 files changed, 39 insertions, 75 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java
index 3e8ef65ade6..f87dd3f42d2 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java
@@ -2,11 +2,8 @@
 package com.yahoo.container.core.config;
 
 import com.yahoo.config.FileReference;
-import com.yahoo.container.Container;
-import com.yahoo.filedistribution.fileacquirer.FileAcquirer;
 import com.yahoo.osgi.Osgi;
 import org.osgi.framework.Bundle;
-import org.osgi.framework.wiring.BundleRevision;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -24,6 +21,7 @@ import java.util.stream.Collectors;
  * @author Tony Vaagenes
  */
 public class ApplicationBundleLoader {
+    private static final Logger log = Logger.getLogger(ApplicationBundleLoader.class.getName());
 
     /* Map of file refs of active bundles (not scheduled for uninstall) to the installed bundle.
      *
@@ -34,14 +32,12 @@ public class ApplicationBundleLoader {
      */
     private final Map<FileReference, Bundle> reference2Bundle = new LinkedHashMap<>();
 
-    private final Logger log = Logger.getLogger(ApplicationBundleLoader.class.getName());
     private final Osgi osgi;
+    private final FileAcquirerBundleInstaller bundleInstaller;
 
-    // A custom bundle installer for non-disk bundles, to be used for testing
-    private BundleInstaller customBundleInstaller = null;
-
-    public ApplicationBundleLoader(Osgi osgi) {
+    public ApplicationBundleLoader(Osgi osgi, FileAcquirerBundleInstaller bundleInstaller) {
         this.osgi = osgi;
+        this.bundleInstaller = bundleInstaller;
     }
 
     /**
@@ -70,7 +66,6 @@ public class ApplicationBundleLoader {
         return obsoleteReferences;
     }
 
-
     /**
      * Returns the bundles that will not be retained by the new application generation.
      */
@@ -89,19 +84,16 @@ public class ApplicationBundleLoader {
         bundlesToInstall.removeAll(reference2Bundle.keySet());
 
         if (!bundlesToInstall.isEmpty()) {
-            FileAcquirer fileAcquirer = Container.get().getFileAcquirer();
-            boolean hasFileDistribution = (fileAcquirer != null);
-            if (hasFileDistribution) {
-                installWithFileDistribution(bundlesToInstall, new FileAcquirerBundleInstaller(fileAcquirer));
-            } else if (customBundleInstaller != null) {
-                installWithFileDistribution(bundlesToInstall, customBundleInstaller);
+            if (bundleInstaller.hasFileDistribution()) {
+                installWithFileDistribution(bundlesToInstall, bundleInstaller);
             } else {
                 log.warning("Can't retrieve bundles since file distribution is disabled.");
             }
         }
     }
 
-    private void installWithFileDistribution(Set<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) {
+    private void installWithFileDistribution(Set<FileReference> bundlesToInstall,
+                                             FileAcquirerBundleInstaller bundleInstaller) {
         for (FileReference reference : bundlesToInstall) {
             try {
                 log.info("Installing bundle with reference '" + reference.value() + "'");
@@ -133,11 +125,6 @@ public class ApplicationBundleLoader {
     }
 
     // Only for testing
-    void useCustomBundleInstaller(BundleInstaller bundleInstaller) {
-        customBundleInstaller = bundleInstaller;
-    }
-
-    // Only for testing
     List<FileReference> getActiveFileReferences() {
         return new ArrayList<>(reference2Bundle.keySet());
     }
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java
deleted file mode 100644
index fc919571b6c..00000000000
--- a/container-core/src/main/java/com/yahoo/container/core/config/BundleInstaller.java
+++ /dev/null
@@ -1,21 +0,0 @@
-// 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.config.FileReference;
-import com.yahoo.osgi.Osgi;
-import org.osgi.framework.Bundle;
-
-import java.util.List;
-
-/**
- * @author gjoranv
- */
-public interface BundleInstaller {
-
-    /**
-     * Installs the bundle with the given file reference, plus all bundles in its X-JDisc-Preinstall-Bundle directive.
-     * Returns all bundles installed to the given OSGi framework as a result of this call.
-     */
-    List<Bundle> installBundles(FileReference reference, Osgi osgi) throws InterruptedException;
-
-}
diff --git a/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java b/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java
index 72951e67b4e..51d77462652 100644
--- a/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java
+++ b/container-core/src/main/java/com/yahoo/container/core/config/FileAcquirerBundleInstaller.java
@@ -13,9 +13,11 @@ import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
 /**
+ * Retrieves bundles with file distribution, and installs them to the OSGi framework.
+ *
  * @author gjoranv
  */
-public class FileAcquirerBundleInstaller implements BundleInstaller {
+public class FileAcquirerBundleInstaller {
     private static Logger log = Logger.getLogger(FileAcquirerBundleInstaller.class.getName());
 
     private final FileAcquirer fileAcquirer;
@@ -24,7 +26,6 @@ public class FileAcquirerBundleInstaller implements BundleInstaller {
         this.fileAcquirer = fileAcquirer;
     }
 
-    @Override
     public List<Bundle> installBundles(FileReference reference, Osgi osgi) throws InterruptedException {
         File file = fileAcquirer.waitFor(reference, 7, TimeUnit.DAYS);
 
@@ -46,6 +47,10 @@ public class FileAcquirerBundleInstaller implements BundleInstaller {
         return osgi.install(file.getAbsolutePath());
     }
 
+    public boolean hasFileDistribution() {
+        return fileAcquirer != null;
+    }
+
     private static boolean notReadable(File file) {
         return ! Files.isReadable(file.toPath());
     }
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 a58dff13f09..13d50b9b30f 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
@@ -16,6 +16,7 @@ import com.yahoo.container.di.config.SubscriberFactory;
 import com.yahoo.container.di.osgi.BundleClasses;
 import com.yahoo.container.di.osgi.OsgiUtil;
 import com.yahoo.container.logging.AccessLog;
+import com.yahoo.filedistribution.fileacquirer.FileAcquirer;
 import com.yahoo.jdisc.application.OsgiFramework;
 import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.service.ClientProvider;
@@ -68,7 +69,6 @@ public class HandlersConfigurerDi {
     }
 
     private final com.yahoo.container.Container vespaContainer;
-    private final OsgiWrapper osgiWrapper;
     private final Container container;
 
     private volatile ComponentGraph currentGraph = new ComponentGraph(0);
@@ -81,7 +81,7 @@ public class HandlersConfigurerDi {
                                 OsgiFramework osgiFramework) {
 
         this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector,
-             new ContainerAndDiOsgi(osgiFramework));
+             new ContainerAndDiOsgi(osgiFramework, vespaContainer.getFileAcquirer()));
     }
 
     // Only public for testing
@@ -93,7 +93,6 @@ public class HandlersConfigurerDi {
                                 OsgiWrapper osgiWrapper) {
 
         this.vespaContainer = vespaContainer;
-        this.osgiWrapper = osgiWrapper;
         container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper);
         getNewComponentGraph(discInjector, false);
     }
@@ -104,12 +103,12 @@ public class HandlersConfigurerDi {
         private final ApplicationBundleLoader applicationBundleLoader;
         private final PlatformBundleLoader platformBundleLoader;
 
-        public ContainerAndDiOsgi(OsgiFramework osgiFramework) {
+        public ContainerAndDiOsgi(OsgiFramework osgiFramework, FileAcquirer fileAcquirer) {
             super(osgiFramework);
             this.osgiFramework = osgiFramework;
 
             OsgiImpl osgi = new OsgiImpl(osgiFramework);
-            applicationBundleLoader = new ApplicationBundleLoader(osgi);
+            applicationBundleLoader = new ApplicationBundleLoader(osgi, new FileAcquirerBundleInstaller(fileAcquirer));
             platformBundleLoader = new PlatformBundleLoader(osgi);
         }
 
diff --git a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java
index ee71e8a8f4c..b56e5d99123 100644
--- a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java
+++ b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java
@@ -2,6 +2,9 @@
 package com.yahoo.container.core.config;
 
 import com.yahoo.config.FileReference;
+import com.yahoo.filedistribution.fileacquirer.FileAcquirer;
+import com.yahoo.filedistribution.fileacquirer.MockFileAcquirer;
+import com.yahoo.osgi.Osgi;
 import org.junit.Before;
 import org.junit.Test;
 import org.osgi.framework.Bundle;
@@ -29,9 +32,9 @@ public class ApplicationBundleLoaderTest {
     @Before
     public void setup() {
         osgi = new TestOsgi(testBundles());
-        var bundleInstaller = new TestBundleInstaller();
-        bundleLoader = new ApplicationBundleLoader(osgi);
-        bundleLoader.useCustomBundleInstaller(bundleInstaller);
+        var bundleInstaller = new TestBundleInstaller(MockFileAcquirer.returnFile(null));
+
+        bundleLoader = new ApplicationBundleLoader(osgi, bundleInstaller);
     }
 
     @Test
@@ -103,4 +106,17 @@ public class ApplicationBundleLoaderTest {
                       BUNDLE_2_REF.value(), BUNDLE_2);
     }
 
+    static class TestBundleInstaller extends FileAcquirerBundleInstaller {
+
+        TestBundleInstaller(FileAcquirer fileAcquirer) {
+            super(fileAcquirer);
+        }
+
+        @Override
+        public List<Bundle> installBundles(FileReference reference, Osgi osgi) {
+            return osgi.install(reference.value());
+        }
+
+    }
+
 }
diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java
deleted file mode 100644
index 43a5268eabf..00000000000
--- a/container-core/src/test/java/com/yahoo/container/core/config/TestBundleInstaller.java
+++ /dev/null
@@ -1,20 +0,0 @@
-// 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.config.FileReference;
-import com.yahoo.osgi.Osgi;
-import org.osgi.framework.Bundle;
-
-import java.util.List;
-
-/**
- * @author gjoranv
- */
-class TestBundleInstaller implements BundleInstaller {
-
-    @Override
-    public List<Bundle> installBundles(FileReference reference, Osgi osgi) {
-        return osgi.install(reference.value());
-    }
-
-}
diff --git a/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java
index 6ed9c2a2994..d8350ec2f6b 100644
--- a/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java
+++ b/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java
@@ -74,9 +74,7 @@ public final class ConfigRetriever {
         return getConfigs(componentConfigKeys, leastGeneration, false);
     }
 
-    /**
-     * Try to get config just once
-     */
+    // TODO: duplicate code, let getConfigs call this.
     Optional<ConfigSnapshot> getConfigsOnce(Set<ConfigKey<? extends ConfigInstance>> componentConfigKeys,
                                             long leastGeneration,
                                             boolean restartOnRedeploy) {