From 2ef1e922a1d845b3cd79e9fb329925e7e9896919 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 4 Nov 2019 15:55:23 +0100 Subject: Revert "Gjoranv/allow duplicate bundles" --- .../yahoo/container/core/config/BundleLoader.java | 71 ++++++++-------------- .../core/config/HandlersConfigurerDi.java | 32 ++++------ .../testutil/HandlersConfigurerTestWrapper.java | 8 +-- .../core/config/testutil/MockOsgiWrapper.java | 44 -------------- .../src/main/java/com/yahoo/osgi/MockOsgi.java | 16 +++-- .../src/main/java/com/yahoo/osgi/Osgi.java | 10 +-- .../src/main/java/com/yahoo/osgi/OsgiImpl.java | 48 +++------------ .../src/main/java/com/yahoo/osgi/OsgiWrapper.java | 16 ----- 8 files changed, 64 insertions(+), 181 deletions(-) delete mode 100644 container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java delete mode 100644 container-core/src/main/java/com/yahoo/osgi/OsgiWrapper.java (limited to 'container-core') 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 initialBundles; + private final Map> 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 obtainBundles(FileReference reference, FileAcquirer fileAcquirer) throws InterruptedException { @@ -51,19 +46,17 @@ public class BundleLoader { return osgi.install(file.getAbsolutePath()); } - private void install(List references) { + /** Returns the number of bundles installed by this call. */ + private int install(List references) { Set 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 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 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 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 getBundlesToUninstall(List newReferences) { - Set bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); + /** Returns the number of uninstalled bundles */ + private int retainOnly(List newReferences) { + Set 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 newReferences) { - // Clean up the map of active bundles Set 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 use(List newBundles) { - Set bundlesToUninstall = getBundlesToUninstall(newBundles); - osgi.allowDuplicateBundles(bundlesToUninstall); - log.info(() -> bundlesToUninstall.isEmpty() ? "Adding bundles to allowed duplicates: " + bundlesToUninstall : ""); - install(newBundles); + public synchronized int use(List 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 useBundles(Collection bundles) { + public void useBundles(Collection 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 getInitialBundles() { - return emptyList(); - } - - @Override - public Bundle[] getBundles() { - return new Bundle[0]; - } - - @Override - public List getCurrentBundles() { - return emptyList(); - } - - @Override - public Bundle getBundle(ComponentSpecification bundleId) { - return null; - } - - @Override - public List install(String absolutePath) { - return emptyList(); - } - - @Override - public void allowDuplicateBundles(Collection 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 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 getCurrentBundles() { + public List install(String absolutePath) { return Collections.emptyList(); } @Override - public Bundle getBundle(ComponentSpecification bundleId) { - return null; + public void uninstall(Bundle bundle) { } @Override - public List 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 getInitialBundles(); - Bundle[] getBundles(); - /** Returns all bundles that have not been scheduled for uninstall. */ - List getCurrentBundles(); - Bundle getBundle(ComponentSpecification bundleId); List install(String absolutePath); - void allowDuplicateBundles(Collection 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 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 getInitialBundles() { - return initialBundles; } @Override @@ -52,10 +28,6 @@ public class OsgiImpl implements Osgi { return bundles.toArray(new Bundle[bundles.size()]); } - @Override - public List getCurrentBundles() { - return jdiscOsgi.getBundles(alwaysCurrentBundle); - } public Class 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 bundles) { - jdiscOsgi.allowDuplicateBundles(bundles); + public void uninstall(Bundle bundle) { + try { + bundle.uninstall(); + } catch (BundleException e) { + throw new RuntimeException(e); + } } - private static Bundle firstNonFrameworkBundle(List 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; - } - -} -- cgit v1.2.3