diff options
author | gjoranv <gv@verizonmedia.com> | 2022-09-01 08:44:33 +0200 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2022-09-01 08:44:33 +0200 |
commit | 7973264e277d7ddffe69a4e5ac611c11b0693595 (patch) | |
tree | 50edfa0e2e34fd20d7b4f7d0f3a0faab3adea088 /container-core | |
parent | 624e1d285ae4fbc5cf94b10b691c3e3b44d9553b (diff) |
Reapply "Make it possible to test the Container with synthetic bundles"
This reverts commit cbed8a40c2a72ca09b7b8e97f83c3acde3479ab3.
Diffstat (limited to 'container-core')
7 files changed, 183 insertions, 120 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java index e45a5bf2ac5..db463edecc0 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Container.java +++ b/container-core/src/main/java/com/yahoo/container/di/Container.java @@ -67,11 +67,6 @@ public class Container { this.retriever = new ConfigRetriever(bootstrapKeys, subscriberFactory); } - public Container(SubscriberFactory subscriberFactory, String configId, ComponentDeconstructor destructor) { - this(subscriberFactory, configId, destructor, new Osgi() { - }); - } - // TODO: try to simplify by returning the result even when the graph failed, instead of throwing here. public ComponentGraphResult waitForNextGraphGeneration(ComponentGraph oldGraph, Injector fallbackInjector, boolean isInitializing) { try { 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 04ed52db53b..bc5b6d6069b 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 @@ -1,19 +1,19 @@ // Copyright Yahoo. 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.filedistribution.fileacquirer.FileAcquirer; -import com.yahoo.filedistribution.fileacquirer.MockFileAcquirer; -import com.yahoo.osgi.Osgi; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.osgi.framework.Bundle; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.Set; +import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_1; +import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_1_REF; +import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_2; +import static com.yahoo.container.core.config.BundleTestUtil.BUNDLE_2_REF; +import static com.yahoo.container.core.config.BundleTestUtil.testBundles; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -22,20 +22,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue; */ public class ApplicationBundleLoaderTest { - private static final FileReference BUNDLE_1_REF = new FileReference("bundle-1"); - private static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value()); - private static final FileReference BUNDLE_2_REF = new FileReference("bundle-2"); - private static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value()); - private ApplicationBundleLoader bundleLoader; private TestOsgi osgi; @BeforeEach public void setup() { osgi = new TestOsgi(testBundles()); - var bundleInstaller = new TestBundleInstaller(MockFileAcquirer.returnFile(null)); - - bundleLoader = new ApplicationBundleLoader(osgi, bundleInstaller); + bundleLoader = osgi.bundleLoader(); } @Test @@ -122,22 +115,4 @@ public class ApplicationBundleLoaderTest { assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); } - private static Map<String, Bundle> testBundles() { - return Map.of(BUNDLE_1_REF.value(), BUNDLE_1, - 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/BundleTestUtil.java b/container-core/src/test/java/com/yahoo/container/core/config/BundleTestUtil.java new file mode 100644 index 00000000000..fddbeaf42be --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/BundleTestUtil.java @@ -0,0 +1,39 @@ +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.filedistribution.fileacquirer.FileAcquirer; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.util.List; +import java.util.Map; + +/** + * @author gjoranv + */ +public class BundleTestUtil { + + public static final FileReference BUNDLE_1_REF = new FileReference("bundle-1"); + public static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value()); + public static final FileReference BUNDLE_2_REF = new FileReference("bundle-2"); + public static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value()); + + public static Map<String, Bundle> testBundles() { + return Map.of(BUNDLE_1_REF.value(), BUNDLE_1, + BUNDLE_2_REF.value(), BUNDLE_2); + } + + public 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/TestBundle.java b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java index 4a607520b2a..17709844f99 100644 --- a/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestBundle.java @@ -16,7 +16,7 @@ import java.util.List; /** * @author gjoranv */ -class TestBundle extends MockBundle { +public class TestBundle extends MockBundle { private static final BundleRevision revision = new TestBundleRevision(); @@ -24,7 +24,7 @@ class TestBundle extends MockBundle { boolean started = false; - TestBundle(String symbolicName) { + public TestBundle(String symbolicName) { this.symbolicName = symbolicName; } diff --git a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java index cf3d1cef5f8..0df338c2144 100644 --- a/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java +++ b/container-core/src/test/java/com/yahoo/container/core/config/TestOsgi.java @@ -1,6 +1,8 @@ // Copyright Yahoo. 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.filedistribution.fileacquirer.MockFileAcquirer; import com.yahoo.osgi.MockOsgi; import org.osgi.framework.Bundle; @@ -8,21 +10,28 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Set; /** * @author gjoranv */ -class TestOsgi extends MockOsgi { +public class TestOsgi extends MockOsgi implements com.yahoo.container.di.Osgi { + private final ApplicationBundleLoader bundleLoader; private final Map<String, Bundle> availableBundles; private final List<Bundle> installedBundles = new ArrayList<>(); private final List<Bundle> allowedDuplicates = new ArrayList<>(); - TestOsgi(Map<String, Bundle> availableBundles) { + public TestOsgi(Map<String, Bundle> availableBundles) { this.availableBundles = availableBundles; + + var bundleInstaller = new BundleTestUtil.TestBundleInstaller(MockFileAcquirer.returnFile(null)); + bundleLoader = new ApplicationBundleLoader(this, bundleInstaller); } + public ApplicationBundleLoader bundleLoader() { return bundleLoader; } + @Override public List<Bundle> install(String fileReferenceValue) { if (! availableBundles.containsKey(fileReferenceValue)) @@ -55,4 +64,17 @@ class TestOsgi extends MockOsgi { allowedDuplicates.addAll(bundles); } + @Override + public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles) { + return bundleLoader.useBundles(new ArrayList<>(bundles)); + } + + @Override + public Collection<Bundle> revertApplicationBundles() { + return bundleLoader.revertToPreviousGeneration(); + } + + public void removeBundle(Bundle bundle) { + installedBundles.remove(bundle); + } } diff --git a/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java index 04091c25ad6..ccd60390129 100644 --- a/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java +++ b/container-core/src/test/java/com/yahoo/container/di/ContainerTest.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.di; -import com.google.inject.Guice; import com.yahoo.component.AbstractComponent; import com.yahoo.config.di.IntConfig; import com.yahoo.config.test.TestConfig; @@ -11,16 +10,20 @@ import com.yahoo.container.di.componentgraph.core.ComponentGraphTest.SimpleCompo import com.yahoo.container.di.componentgraph.core.ComponentNode.ComponentConstructorException; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.osgi.framework.Bundle; -import java.util.Collection; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * @author Tony Vaagenes @@ -88,6 +91,28 @@ public class ContainerTest extends ContainerTestBase { container.shutdownConfigRetriever(); } + @Test + void bundle_from_previous_generation_is_uninstalled_when_not_used_in_the_new_generation() { + ComponentEntry component1 = new ComponentEntry("component1", SimpleComponent.class); + ComponentEntry component2 = new ComponentEntry("component2", SimpleComponent.class); + + writeBootstrapConfigsWithBundles(List.of("bundle-1"), List.of(component1)); + Container container = newContainer(dirConfigSource); + ComponentGraph graph = getNewComponentGraph(container); + + // bundle-1 is installed + assertEquals(1, osgi.getBundles().length); + assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName()); + + writeBootstrapConfigsWithBundles(List.of("bundle-2"), List.of(component2)); + container.reloadConfig(2); + getNewComponentGraph(container, graph); + + // bundle-1 is kept, bundle-2 has been uninstalled + assertEquals(1, osgi.getBundles().length); + assertEquals("bundle-2", osgi.getBundles()[0].getSymbolicName()); + } + //@Test TODO public void deconstructor_is_given_guice_components() { } @@ -179,6 +204,34 @@ public class ContainerTest extends ContainerTestBase { } @Test + void bundle_from_failing_generation_is_uninstalled() { + ComponentEntry simpleComponentEntry = new ComponentEntry("simpleComponent", SimpleComponent.class); + ComponentEntry throwingComponentEntry = new ComponentEntry("throwingComponent", ComponentThrowingExceptionInConstructor.class); + + writeBootstrapConfigsWithBundles(List.of("bundle-1"), List.of(simpleComponentEntry)); + Container container = newContainer(dirConfigSource); + ComponentGraph currentGraph = getNewComponentGraph(container); + + // bundle-1 is installed + assertEquals(1, osgi.getBundles().length); + assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName()); + + writeBootstrapConfigsWithBundles(List.of("bundle-2"), List.of(throwingComponentEntry)); + container.reloadConfig(2); + try { + currentGraph = getNewComponentGraph(container, currentGraph); + fail("Expected exception"); + } catch (ComponentConstructorException ignored) { + // Expected, do nothing + } + assertEquals(1, currentGraph.generation()); + + // bundle-1 is kept, bundle-2 has been uninstalled + assertEquals(1, osgi.getBundles().length); + assertEquals("bundle-1", osgi.getBundles()[0].getSymbolicName()); + } + + @Test void getNewComponentGraph_hangs_waiting_for_valid_config_after_invalid_config() throws Exception { dirConfigSource.writeConfig("test", "stringVal \"original\""); writeBootstrapConfigs("myId", ComponentTakingConfig.class); @@ -303,38 +356,6 @@ public class ContainerTest extends ContainerTestBase { } } - public static class TestDeconstructor implements ComponentDeconstructor { - @Override - public void deconstruct(long generation, List<Object> components, Collection<Bundle> bundles) { - 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"); - } - } - - private static Container newContainer(DirConfigSource dirConfigSource, - ComponentDeconstructor deconstructor) { - return new Container(new CloudSubscriberFactory(dirConfigSource.configSource), dirConfigSource.configId(), deconstructor); - } - - private static Container newContainer(DirConfigSource dirConfigSource) { - return newContainer(dirConfigSource, new TestDeconstructor()); - } - - ComponentGraph getNewComponentGraph(Container container, ComponentGraph oldGraph) { - Container.ComponentGraphResult result = container.waitForNextGraphGeneration(oldGraph, Guice.createInjector(), true); - result.oldComponentsCleanupTask().run(); - return result.newGraph(); - } - - ComponentGraph getNewComponentGraph(Container container) { - return container.waitForNextGraphGeneration(new ComponentGraph(), Guice.createInjector(), true).newGraph(); - } - private ComponentTakingConfig createComponentTakingConfig(ComponentGraph componentGraph) { return componentGraph.getInstance(ComponentTakingConfig.class); } diff --git a/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java index f2faed650a8..7442eb2068d 100644 --- a/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java +++ b/container-core/src/test/java/com/yahoo/container/di/ContainerTestBase.java @@ -2,9 +2,8 @@ package com.yahoo.container.di; 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.core.config.BundleTestUtil; +import com.yahoo.container.core.config.TestOsgi; import com.yahoo.container.di.ContainerTest.ComponentTakingConfig; import com.yahoo.container.di.componentgraph.core.ComponentGraph; import org.junit.jupiter.api.AfterEach; @@ -14,9 +13,7 @@ import org.osgi.framework.Bundle; import java.io.File; import java.util.Collection; -import java.util.Set; - -import static java.util.Collections.emptySet; +import java.util.List; /** * @author Tony Vaagenes @@ -27,6 +24,7 @@ public class ContainerTestBase { private ComponentGraph componentGraph; protected DirConfigSource dirConfigSource = null; + protected TestOsgi osgi; @TempDir File tmpDir; @@ -34,6 +32,8 @@ public class ContainerTestBase { @BeforeEach public void setup() { dirConfigSource = new DirConfigSource(tmpDir, "ContainerTest-"); + componentGraph = new ComponentGraph(0); + osgi = new TestOsgi(BundleTestUtil.testBundles()); } @AfterEach @@ -41,52 +41,43 @@ public class ContainerTestBase { dirConfigSource.cleanup(); } - @BeforeEach - public void createGraph() { - componentGraph = new ComponentGraph(0); + + protected Container newContainer(DirConfigSource dirConfigSource, + ComponentDeconstructor deconstructor) { + return new Container(new CloudSubscriberFactory(dirConfigSource.configSource), + dirConfigSource.configId(), + deconstructor, + osgi); } - public void complete() { - try { - Container container = new Container(new CloudSubscriberFactory(dirConfigSource.configSource()), dirConfigSource.configId(), - new ContainerTest.TestDeconstructor(), new Osgi() { - @SuppressWarnings("unchecked") - @Override - public Class<Object> resolveClass(BundleInstantiationSpecification spec) { - try { - return (Class<Object>) Class.forName(spec.classId.getName()); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); - } - } - - @Override - public Set<Bundle> useApplicationBundles(Collection<FileReference> bundles) { - return emptySet(); - } - - @Override - public Collection<Bundle> revertApplicationBundles() { - return emptySet(); - } - - @Override - public Bundle getBundle(ComponentSpecification spec) { - throw new UnsupportedOperationException("getBundle not supported."); - } - }); - Container.ComponentGraphResult result = container.waitForNextGraphGeneration(this.componentGraph, Guice.createInjector(), true); - result.oldComponentsCleanupTask().run(); - this.componentGraph = result.newGraph(); - } catch (Exception e) { - throw new RuntimeException(e); - } + protected Container newContainer(DirConfigSource dirConfigSource) { + return newContainer(dirConfigSource, new TestDeconstructor(osgi)); } + ComponentGraph getNewComponentGraph(Container container, ComponentGraph oldGraph) { + Container.ComponentGraphResult result = container.waitForNextGraphGeneration(oldGraph, Guice.createInjector(), true); + result.oldComponentsCleanupTask().run(); + return result.newGraph(); + } + + ComponentGraph getNewComponentGraph(Container container) { + return container.waitForNextGraphGeneration(new ComponentGraph(), Guice.createInjector(), true).newGraph(); + } + + public <T> T getInstance(Class<T> componentClass) { return componentGraph.getInstance(componentClass); } + protected void writeBootstrapConfigsWithBundles(List<String> applicationBundles, List<ComponentEntry> components) { + writeBootstrapConfigs(components.toArray(new ComponentEntry[0])); + StringBuilder bundles = new StringBuilder(); + for (int i = 0; i < applicationBundles.size(); i++) { + bundles.append("bundles[" + i + "] \"" + applicationBundles.get(i) + "\"\n"); + } + dirConfigSource.writeConfig("application-bundles", String.format("bundles[%s]\n%s", applicationBundles.size(), bundles)); + } + protected void writeBootstrapConfigs(ComponentEntry... componentEntries) { dirConfigSource.writeConfig("platform-bundles", ""); dirConfigSource.writeConfig("application-bundles", ""); @@ -126,4 +117,24 @@ public class ContainerTestBase { } } + + public static class TestDeconstructor implements ComponentDeconstructor { + + final TestOsgi osgi; + + public TestDeconstructor(TestOsgi osgi) { + this.osgi = osgi; + } + + @Override + public void deconstruct(long generation, List<Object> components, Collection<Bundle> bundles) { + components.forEach(component -> { + if (component instanceof ContainerTest.DestructableComponent vespaComponent) { + vespaComponent.deconstruct(); + } + }); + bundles.forEach(osgi::removeBundle); + } + } + } |