From 3e1864b31fea345c7a43cac45e8ae26c71634e3d Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 6 Jul 2020 21:50:03 +0200 Subject: minor: rearrange methods --- .../main/java/com/yahoo/container/di/Container.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 dcdd1767c9c..17eb1abd641 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 @@ -69,15 +69,6 @@ public class Container { }); } - private void deconstructObsoleteComponents(ComponentGraph oldGraph, - ComponentGraph newGraph, - Collection obsoleteBundles) { - IdentityHashMap oldComponents = new IdentityHashMap<>(); - oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); - newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); - componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); - } - public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) { try { Collection obsoleteBundles = new HashSet<>(); @@ -101,6 +92,15 @@ public class Container { return getNewComponentGraph(new ComponentGraph(), Guice.createInjector(), false); } + private void deconstructObsoleteComponents(ComponentGraph oldGraph, + ComponentGraph newGraph, + Collection obsoleteBundles) { + IdentityHashMap oldComponents = new IdentityHashMap<>(); + oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); + newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); + componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); + } + private static String newGraphErrorMessage(long generation, Throwable cause) { String failedFirstMessage = "Failed to set up first component graph"; String failedNewMessage = "Failed to set up new component graph"; -- cgit v1.2.3 From 3a64c2cb147b06e5f54d8d12d968ee17bc1521be Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 6 Jul 2020 21:57:56 +0200 Subject: Move test methods to test. --- .../java/com/yahoo/container/di/Container.java | 8 ---- .../java/com/yahoo/container/di/ContainerTest.java | 51 +++++++++++++--------- 2 files changed, 30 insertions(+), 29 deletions(-) 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 17eb1abd641..fdbdd009536 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 @@ -84,14 +84,6 @@ public class Container { } } - ComponentGraph getNewComponentGraph(ComponentGraph oldGraph) { - return getNewComponentGraph(oldGraph, Guice.createInjector(), false); - } - - ComponentGraph getNewComponentGraph() { - return getNewComponentGraph(new ComponentGraph(), Guice.createInjector(), false); - } - private void deconstructObsoleteComponents(ComponentGraph oldGraph, ComponentGraph newGraph, Collection obsoleteBundles) { 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..c5fe7f8a82c 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 @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. 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; @@ -49,7 +50,7 @@ public class ContainerTest extends ContainerTestBase { Container container = newContainer(dirConfigSource); - ComponentTakingConfig component = createComponentTakingConfig(container.getNewComponentGraph()); + ComponentTakingConfig component = createComponentTakingConfig(getNewComponentGraph(container)); assertThat(component.config.stringVal(), is("myString")); container.shutdownConfigurer(); @@ -62,7 +63,7 @@ public class ContainerTest extends ContainerTestBase { Container container = newContainer(dirConfigSource); - ComponentGraph componentGraph = container.getNewComponentGraph(); + ComponentGraph componentGraph = getNewComponentGraph(container); ComponentTakingConfig component = createComponentTakingConfig(componentGraph); assertThat(component.config.stringVal(), is("original")); @@ -71,7 +72,7 @@ public class ContainerTest extends ContainerTestBase { dirConfigSource.writeConfig("test", "stringVal \"reconfigured\""); container.reloadConfig(2); - ComponentGraph newComponentGraph = container.getNewComponentGraph(componentGraph); + ComponentGraph newComponentGraph = getNewComponentGraph(container, componentGraph); ComponentTakingConfig component2 = createComponentTakingConfig(newComponentGraph); assertThat(component2.config.stringVal(), is("reconfigured")); @@ -85,7 +86,7 @@ public class ContainerTest extends ContainerTestBase { Container container = newContainer(dirConfigSource); - ComponentGraph graph = container.getNewComponentGraph(); + ComponentGraph graph = getNewComponentGraph(container); ComponentTakingConfig component = createComponentTakingConfig(graph); assertThat(component.getId().toString(), is("id1")); @@ -94,7 +95,7 @@ public class ContainerTest extends ContainerTestBase { new ComponentEntry("id2", ComponentTakingConfig.class)); container.reloadConfig(2); - ComponentGraph newGraph = container.getNewComponentGraph(graph); + ComponentGraph newGraph = getNewComponentGraph(container, graph); assertThat(ComponentGraph.getNode(newGraph, "id1"), notNullValue(Node.class)); assertThat(ComponentGraph.getNode(newGraph, "id2"), notNullValue(Node.class)); @@ -112,12 +113,12 @@ public class ContainerTest extends ContainerTestBase { Container container = newContainer(dirConfigSource); - ComponentGraph oldGraph = container.getNewComponentGraph(); + ComponentGraph oldGraph = getNewComponentGraph(container); DestructableComponent componentToDestruct = oldGraph.getInstance(DestructableComponent.class); writeBootstrapConfigs("id2", DestructableComponent.class); container.reloadConfig(2); - container.getNewComponentGraph(oldGraph); + getNewComponentGraph(container, oldGraph); assertTrue(componentToDestruct.deconstructed); } @@ -127,7 +128,7 @@ public class ContainerTest extends ContainerTestBase { writeBootstrapConfigs("thrower", ComponentThrowingExceptionInConstructor.class); Container container = newContainer(dirConfigSource); try { - container.getNewComponentGraph(); + getNewComponentGraph(container); fail("Expected to log and die."); } catch (Throwable t) { fail("Expected to log and die"); @@ -140,14 +141,14 @@ public class ContainerTest extends ContainerTestBase { writeBootstrapConfigs(simpleComponentEntry); Container container = newContainer(dirConfigSource); - ComponentGraph currentGraph = container.getNewComponentGraph(); + ComponentGraph currentGraph = getNewComponentGraph(container); SimpleComponent simpleComponent = currentGraph.getInstance(SimpleComponent.class); writeBootstrapConfigs("thrower", ComponentThrowingExceptionInConstructor.class); container.reloadConfig(2); try { - currentGraph = container.getNewComponentGraph(currentGraph); + currentGraph = getNewComponentGraph(container, currentGraph); fail("Expected exception"); } catch (ComponentConstructorException ignored) { // Expected, do nothing @@ -161,7 +162,7 @@ public class ContainerTest extends ContainerTestBase { dirConfigSource.writeConfig("test", "stringVal \"myString\""); writeBootstrapConfigs(simpleComponentEntry, componentTakingConfigEntry); container.reloadConfig(3); - currentGraph = container.getNewComponentGraph(currentGraph); + currentGraph = getNewComponentGraph(container, currentGraph); assertEquals(3, currentGraph.generation()); assertSame(simpleComponent, currentGraph.getInstance(SimpleComponent.class)); @@ -174,7 +175,7 @@ public class ContainerTest extends ContainerTestBase { writeBootstrapConfigs(simpleComponentEntry); Container container = newContainer(dirConfigSource); - ComponentGraph currentGraph = container.getNewComponentGraph(); + ComponentGraph currentGraph = getNewComponentGraph(container); currentGraph.getInstance(SimpleComponent.class); @@ -182,7 +183,7 @@ public class ContainerTest extends ContainerTestBase { dirConfigSource.writeConfig("test", "stringVal \"myString\""); container.reloadConfig(2); try { - currentGraph = container.getNewComponentGraph(currentGraph); + currentGraph = getNewComponentGraph(container, currentGraph); fail("Expected exception"); } catch (IllegalArgumentException ignored) { // Expected, do nothing @@ -198,18 +199,18 @@ public class ContainerTest extends ContainerTestBase { writeBootstrapConfigs("myId", ComponentTakingConfig.class); Container container = newContainer(dirConfigSource); - final ComponentGraph currentGraph = container.getNewComponentGraph(); + final ComponentGraph currentGraph = getNewComponentGraph(container); writeBootstrapConfigs("thrower", ComponentThrowingExceptionForMissingConfig.class); container.reloadConfig(2); try { - container.getNewComponentGraph(currentGraph); + getNewComponentGraph(container, currentGraph); fail("expected exception"); } catch (Exception ignored) { } ExecutorService exec = Executors.newFixedThreadPool(1); - Future newGraph = exec.submit(() -> container.getNewComponentGraph(currentGraph)); + Future newGraph = exec.submit(() -> getNewComponentGraph(container, currentGraph)); try { newGraph.get(1, TimeUnit.SECONDS); @@ -234,7 +235,7 @@ public class ContainerTest extends ContainerTestBase { dirConfigSource.writeConfig("jersey-injection", "inject[0]"); Container container = newContainer(dirConfigSource); - ComponentGraph componentGraph = container.getNewComponentGraph(); + ComponentGraph componentGraph = getNewComponentGraph(container); RestApiContext restApiContext = componentGraph.getInstance(clazz); assertNotNull(restApiContext); @@ -271,7 +272,7 @@ public class ContainerTest extends ContainerTestBase { dirConfigSource.writeConfig("jersey-injection", injectionConfig); Container container = newContainer(dirConfigSource); - ComponentGraph componentGraph = container.getNewComponentGraph(); + ComponentGraph componentGraph = getNewComponentGraph(container); RestApiContext restApiContext = componentGraph.getInstance(restApiClass); @@ -298,12 +299,12 @@ public class ContainerTest extends ContainerTestBase { Container container = newContainer(dirConfigSource, deconstructor); - ComponentGraph oldGraph = container.getNewComponentGraph(); + ComponentGraph oldGraph = getNewComponentGraph(container); DestructableEntity destructableEntity = oldGraph.getInstance(DestructableEntity.class); writeBootstrapConfigs("id2", DestructableProvider.class); container.reloadConfig(2); - container.getNewComponentGraph(oldGraph); + getNewComponentGraph(container, oldGraph); assertTrue(destructableEntity.deconstructed); } @@ -314,7 +315,7 @@ public class ContainerTest extends ContainerTestBase { Container container = newContainer(dirConfigSource); - ComponentGraph oldGraph = container.getNewComponentGraph(); + ComponentGraph oldGraph = getNewComponentGraph(container); } static class DestructableEntity { @@ -398,6 +399,14 @@ public class ContainerTest extends ContainerTestBase { return newContainer(dirConfigSource, new TestDeconstructor()); } + ComponentGraph getNewComponentGraph(Container container, ComponentGraph oldGraph) { + return container.getNewComponentGraph(oldGraph, Guice.createInjector(), false); + } + + ComponentGraph getNewComponentGraph(Container container) { + return container.getNewComponentGraph(new ComponentGraph(), Guice.createInjector(), false); + } + private ComponentTakingConfig createComponentTakingConfig(ComponentGraph componentGraph) { return componentGraph.getInstance(ComponentTakingConfig.class); } -- cgit v1.2.3 From 5be1a06e462a1689cfdca61baffa7a58efe033c4 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 6 Jul 2020 22:10:54 +0200 Subject: Fix ambiguous naming and use modern Java. --- container-di/src/main/java/com/yahoo/container/di/Container.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 fdbdd009536..027c9cdd354 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 @@ -58,10 +58,8 @@ public class Container { this.componentDeconstructor = componentDeconstructor; this.osgi = osgi; - Set> keySet = new HashSet<>(); - keySet.add(bundlesConfigKey); - keySet.add(componentsConfigKey); - this.configurer = new ConfigRetriever(keySet, subscriberFactory::getSubscriber); + var bootstrapKeys = Set.of(bundlesConfigKey, componentsConfigKey); + this.configurer = new ConfigRetriever(bootstrapKeys, subscriberFactory::getSubscriber); } public Container(SubscriberFactory subscriberFactory, String configId, ComponentDeconstructor componentDeconstructor) { -- cgit v1.2.3 From 3b7a09a474730eb24a9b26bf29a46b8778c95f7d Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 6 Jul 2020 22:22:17 +0200 Subject: Rearrange methods, no functional changes. --- .../java/com/yahoo/container/di/Container.java | 95 +++++++++++----------- 1 file changed, 47 insertions(+), 48 deletions(-) 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 027c9cdd354..3b75dc14fdb 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 @@ -82,50 +82,12 @@ public class Container { } } - private void deconstructObsoleteComponents(ComponentGraph oldGraph, - ComponentGraph newGraph, - Collection obsoleteBundles) { - IdentityHashMap oldComponents = new IdentityHashMap<>(); - oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); - newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); - componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); - } - - private static String newGraphErrorMessage(long generation, Throwable cause) { - String failedFirstMessage = "Failed to set up first component graph"; - String failedNewMessage = "Failed to set up new component graph"; - String constructMessage = " due to error when constructing one of the components"; - String retainMessage = ". Retaining previous component generation."; - - if (generation == 0) { - if (cause instanceof ComponentNode.ComponentConstructorException) { - return failedFirstMessage + constructMessage; - } else { - return failedFirstMessage; - } - } else { - if (cause instanceof ComponentNode.ComponentConstructorException) { - return failedNewMessage + constructMessage + retainMessage; - } else { - return failedNewMessage + retainMessage; - } - } - } - - private void invalidateGeneration(long generation, Throwable cause) { - leastGeneration = Math.max(configurer.getComponentsGeneration(), configurer.getBootstrapGeneration()) + 1; - if (!(cause instanceof InterruptedException) && !(cause instanceof ConfigInterruptedException)) { - log.log(Level.WARNING, newGraphErrorMessage(generation, cause), cause); - } - } - private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph, Injector fallbackInjector, boolean restartOnRedeploy, Collection obsoleteBundles) // NOTE: Return value { ConfigSnapshot snapshot; - while (true) { snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, restartOnRedeploy); @@ -178,15 +140,17 @@ public class Container { return componentGraph; } - private void injectNodes(ComponentsConfig config, ComponentGraph graph) { - for (ComponentsConfig.Components component : config.components()) { - Node componentNode = ComponentGraph.getNode(graph, component.id()); + private void constructComponents(ComponentGraph graph) { + graph.nodes().forEach(Node::constructInstance); + } - for (ComponentsConfig.Components.Inject inject : component.inject()) { - //TODO: Support inject.name() - componentNode.inject(ComponentGraph.getNode(graph, inject.id())); - } - } + private void deconstructObsoleteComponents(ComponentGraph oldGraph, + ComponentGraph newGraph, + Collection obsoleteBundles) { + IdentityHashMap oldComponents = new IdentityHashMap<>(); + oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); + newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); + componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); } private Set installBundles(Map, ConfigInstance> configsIncludingBootstrapConfigs) { @@ -228,8 +192,43 @@ public class Container { } } - private void constructComponents(ComponentGraph graph) { - graph.nodes().forEach(Node::constructInstance); + private void injectNodes(ComponentsConfig config, ComponentGraph graph) { + for (ComponentsConfig.Components component : config.components()) { + Node componentNode = ComponentGraph.getNode(graph, component.id()); + + for (ComponentsConfig.Components.Inject inject : component.inject()) { + //TODO: Support inject.name() + componentNode.inject(ComponentGraph.getNode(graph, inject.id())); + } + } + } + + private void invalidateGeneration(long generation, Throwable cause) { + leastGeneration = Math.max(configurer.getComponentsGeneration(), configurer.getBootstrapGeneration()) + 1; + if (!(cause instanceof InterruptedException) && !(cause instanceof ConfigInterruptedException)) { + log.log(Level.WARNING, newGraphErrorMessage(generation, cause), cause); + } + } + + private static String newGraphErrorMessage(long generation, Throwable cause) { + String failedFirstMessage = "Failed to set up first component graph"; + String failedNewMessage = "Failed to set up new component graph"; + String constructMessage = " due to error when constructing one of the components"; + String retainMessage = ". Retaining previous component generation."; + + if (generation == 0) { + if (cause instanceof ComponentNode.ComponentConstructorException) { + return failedFirstMessage + constructMessage; + } else { + return failedFirstMessage; + } + } else { + if (cause instanceof ComponentNode.ComponentConstructorException) { + return failedNewMessage + constructMessage + retainMessage; + } else { + return failedNewMessage + retainMessage; + } + } } public void shutdown(ComponentGraph graph, ComponentDeconstructor deconstructor) { -- cgit v1.2.3 From fdfb5124eb9be3d23e17f5b717a7bf1ae695d6b5 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Tue, 7 Jul 2020 12:49:33 +0200 Subject: Remove unused imports --- container-di/src/main/java/com/yahoo/container/di/Container.java | 2 -- 1 file changed, 2 deletions(-) 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 3b75dc14fdb..64a7c607972 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 @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. 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.google.inject.Injector; import com.yahoo.config.ConfigInstance; import com.yahoo.config.ConfigurationRuntimeException; @@ -24,7 +23,6 @@ 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; -- cgit v1.2.3 From 1f6de163d0afcd050fd5eb1318a88f2cd0fb1ce3 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 8 Jul 2020 12:54:47 +0200 Subject: Declare a field final and rearrange. No functional changes. --- container-di/src/main/java/com/yahoo/container/di/Container.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 64a7c607972..ef7813ce368 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 @@ -45,17 +45,17 @@ public class Container { private final ComponentDeconstructor componentDeconstructor; private final Osgi osgi; - private ConfigRetriever configurer; + private final ConfigRetriever configurer; private long previousConfigGeneration = -1L; private long leastGeneration = -1L; public Container(SubscriberFactory subscriberFactory, String configId, ComponentDeconstructor componentDeconstructor, Osgi osgi) { this.subscriberFactory = subscriberFactory; - this.bundlesConfigKey = new ConfigKey<>(BundlesConfig.class, configId); - this.componentsConfigKey = new ConfigKey<>(ComponentsConfig.class, configId); this.componentDeconstructor = componentDeconstructor; this.osgi = osgi; + bundlesConfigKey = new ConfigKey<>(BundlesConfig.class, configId); + componentsConfigKey = new ConfigKey<>(ComponentsConfig.class, configId); var bootstrapKeys = Set.of(bundlesConfigKey, componentsConfigKey); this.configurer = new ConfigRetriever(bootstrapKeys, subscriberFactory::getSubscriber); } -- cgit v1.2.3 From df08379870b903abcad2d24196944b64fb86be9c Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 8 Jul 2020 23:43:13 +0200 Subject: Update test names. --- .../com/yahoo/component/provider/test/ComponentRegistryTestCase.java | 1 + container-di/src/test/java/com/yahoo/container/di/ContainerTest.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/container-di/src/test/java/com/yahoo/component/provider/test/ComponentRegistryTestCase.java b/container-di/src/test/java/com/yahoo/component/provider/test/ComponentRegistryTestCase.java index 463a36af5d8..69eec95b746 100644 --- a/container-di/src/test/java/com/yahoo/component/provider/test/ComponentRegistryTestCase.java +++ b/container-di/src/test/java/com/yahoo/component/provider/test/ComponentRegistryTestCase.java @@ -14,6 +14,7 @@ import com.yahoo.component.provider.ComponentRegistry; /** * Tests that ComponentRegistry handles namespaces correctly. + * * @author Tony Vaagenes */ public class ComponentRegistryTestCase { 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 c5fe7f8a82c..eac64c20274 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 @@ -108,7 +108,7 @@ public class ContainerTest extends ContainerTestBase { } @Test - public void osgi_component_is_deconstructed_when_not_reused() { + public void component_is_deconstructed_when_not_reused() { writeBootstrapConfigs("id1", DestructableComponent.class); Container container = newContainer(dirConfigSource); @@ -194,7 +194,7 @@ public class ContainerTest extends ContainerTestBase { } @Test - public void runOnce_hangs_waiting_for_valid_config_after_invalid_config() throws InterruptedException, ExecutionException, TimeoutException { + public void getNewComponentGraph_hangs_waiting_for_valid_config_after_invalid_config() throws Exception { dirConfigSource.writeConfig("test", "stringVal \"original\""); writeBootstrapConfigs("myId", ComponentTakingConfig.class); -- cgit v1.2.3 From 6b1fe50d0c148fb5dde8fe3c9a0cdc0228c90a07 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 8 Jul 2020 23:43:40 +0200 Subject: Remove demo test cases that are covered elsewhere. --- container-di/src/test/java/demo/Base.java | 5 --- .../src/test/java/demo/ComponentConfigTest.java | 48 ---------------------- .../src/test/java/demo/ComponentRegistryTest.java | 42 ------------------- .../src/test/java/demo/DeconstructTest.java | 36 ---------------- 4 files changed, 131 deletions(-) delete mode 100644 container-di/src/test/java/demo/ComponentConfigTest.java delete mode 100644 container-di/src/test/java/demo/ComponentRegistryTest.java delete mode 100644 container-di/src/test/java/demo/DeconstructTest.java diff --git a/container-di/src/test/java/demo/Base.java b/container-di/src/test/java/demo/Base.java index 95ff2e14d53..e98d85a9193 100644 --- a/container-di/src/test/java/demo/Base.java +++ b/container-di/src/test/java/demo/Base.java @@ -56,9 +56,4 @@ public class Base { this.injector = injector; } - @SuppressWarnings("unchecked") - public void addConfig(ConfigInstance configInstance, ComponentId id) { - configs.put(new ConfigKey<>((Class)configInstance.getClass(), id.toString()), - configInstance); - } } diff --git a/container-di/src/test/java/demo/ComponentConfigTest.java b/container-di/src/test/java/demo/ComponentConfigTest.java deleted file mode 100644 index 02e98bbc325..00000000000 --- a/container-di/src/test/java/demo/ComponentConfigTest.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package demo; - -import com.yahoo.config.test.ThreadPoolConfig; -import com.yahoo.container.di.componentgraph.Provider; -import org.junit.Test; - -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; - -import static org.junit.Assert.assertNotNull; - - -/** - * @author Tony Vaagenes - * @author gjoranv - */ -public class ComponentConfigTest extends Base { - public static class ThreadPoolExecutorProvider implements Provider { - private ExecutorService executor; - - public ThreadPoolExecutorProvider(ThreadPoolConfig config) { - executor = Executors.newFixedThreadPool(config.numThreads()); - } - - @Override - public Executor get() { - return executor; - } - - @Override - public void deconstruct() { - executor.shutdown(); - } - } - - @Test - public void require_that_non_components_can_be_configured() { - register(ThreadPoolExecutorProvider.class); - addConfig(new ThreadPoolConfig(new ThreadPoolConfig.Builder().numThreads(4)), - toId(ThreadPoolExecutorProvider.class)); - complete(); - - Executor executor = getInstance(Executor.class); - assertNotNull(executor); - } -} diff --git a/container-di/src/test/java/demo/ComponentRegistryTest.java b/container-di/src/test/java/demo/ComponentRegistryTest.java deleted file mode 100644 index 26ef0a476d7..00000000000 --- a/container-di/src/test/java/demo/ComponentRegistryTest.java +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package demo; - -import com.yahoo.component.AbstractComponent; -import com.yahoo.component.provider.ComponentRegistry; -import org.junit.Test; - -import static org.junit.Assert.assertNotNull; - - -/** - * @author Tony Vaagenes - * @author gjoranv - */ -public class ComponentRegistryTest extends Base { - public static class SearchHandler extends AbstractComponent { - private final ComponentRegistry searchers; - - public SearchHandler(ComponentRegistry searchers) { - this.searchers = searchers; - } - } - - public static class Searcher extends AbstractComponent {} - - public static class FooSearcher extends Searcher {} - public static class BarSearcher extends Searcher {} - - @Test - public void require_that_component_registry_can_be_injected() { - register(SearchHandler.class); - register(FooSearcher.class); - register(BarSearcher.class); - complete(); - - SearchHandler handler = getInstance(SearchHandler.class); - - ComponentRegistry searchers = handler.searchers; - assertNotNull(searchers.getComponent(toId(FooSearcher.class))); - assertNotNull(searchers.getComponent(toId(BarSearcher.class))); - } -} diff --git a/container-di/src/test/java/demo/DeconstructTest.java b/container-di/src/test/java/demo/DeconstructTest.java deleted file mode 100644 index e3dc5e22416..00000000000 --- a/container-di/src/test/java/demo/DeconstructTest.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package demo; - -import com.yahoo.container.di.ContainerTest; -import com.yahoo.container.di.ContainerTestBase; -import org.junit.Test; - -import static org.junit.Assert.assertTrue; - -/** - * @author Tony Vaagenes - * @author gjoranv - */ -public class DeconstructTest extends ContainerTestBase { - public static class DeconstructableComponent extends ContainerTest.DestructableComponent { - private boolean isDeconstructed = false; - - @Override - public void deconstruct() { - isDeconstructed = true; - } - } - - @Test - public void require_that_unused_components_are_deconstructed() { - writeBootstrapConfigs("d1", DeconstructableComponent.class); - complete(); - - DeconstructableComponent d1 = getInstance(DeconstructableComponent.class); - - writeBootstrapConfigs("d2", DeconstructableComponent.class); - complete(); - - assertTrue(d1.isDeconstructed); - } -} -- cgit v1.2.3 From 1d92ada09722c4277ac51f35c1a1d5e40c2e6734 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 8 Jul 2020 23:53:47 +0200 Subject: Move test for Guice injector out of the 'demo' package. --- .../core/FallbackToGuiceInjectorTest.java | 151 +++++++++++++++++++++ container-di/src/test/java/demo/Base.java | 59 -------- .../java/demo/FallbackToGuiceInjectorTest.java | 104 -------------- 3 files changed, 151 insertions(+), 163 deletions(-) create mode 100644 container-di/src/test/java/com/yahoo/container/di/componentgraph/core/FallbackToGuiceInjectorTest.java delete mode 100644 container-di/src/test/java/demo/Base.java delete mode 100644 container-di/src/test/java/demo/FallbackToGuiceInjectorTest.java diff --git a/container-di/src/test/java/com/yahoo/container/di/componentgraph/core/FallbackToGuiceInjectorTest.java b/container-di/src/test/java/com/yahoo/container/di/componentgraph/core/FallbackToGuiceInjectorTest.java new file mode 100644 index 00000000000..7c517d67960 --- /dev/null +++ b/container-di/src/test/java/com/yahoo/container/di/componentgraph/core/FallbackToGuiceInjectorTest.java @@ -0,0 +1,151 @@ +// 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.componentgraph.core; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Inject; +import com.google.inject.Injector; +import com.google.inject.name.Named; +import com.google.inject.name.Names; +import com.yahoo.component.AbstractComponent; +import com.yahoo.component.ComponentId; +import com.yahoo.config.ConfigInstance; +import com.yahoo.container.di.componentgraph.core.ComponentGraph; +import com.yahoo.container.di.componentgraph.core.ComponentNode; +import com.yahoo.container.di.componentgraph.core.Node; +import com.yahoo.vespa.config.ConfigKey; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertNotNull; + +/** + * @author Tony Vaagenes + * @author gjoranv + */ +@SuppressWarnings("unused") +public class FallbackToGuiceInjectorTest { + + private ComponentGraph componentGraph; + private Injector injector; + private Map, ConfigInstance> configs = + new HashMap<>(); + + @Rule + public final ExpectedException exception = ExpectedException.none(); + + @Before + public void createGraph() { + injector = Guice.createInjector(); + componentGraph = new ComponentGraph(0); + } + + public static class MyComponent extends AbstractComponent { + private final String url; + private final Executor executor; + + @Inject + public MyComponent(@Named("url") String url, Executor executor) { + this.url = url; + this.executor = executor; + } + + public MyComponent() { + throw new RuntimeException("Constructor annotated with @Inject is preferred."); + } + } + + public static class ComponentTakingDefaultString{ + private final String injectedString; + + public ComponentTakingDefaultString(String empty_string_created_by_guice) { + this.injectedString = empty_string_created_by_guice; + } + } + + public static class ComponentThatCannotBeConstructed { + public ComponentThatCannotBeConstructed(Integer cannot_be_injected_because_Integer_has_no_default_ctor) { } + } + + @Test + public void guice_injector_is_used_when_no_global_component_exists() { + setInjector( + Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + bind(Executor.class).toInstance(Executors.newSingleThreadExecutor()); + bind(String.class).annotatedWith(Names.named("url")).toInstance("http://yahoo.com"); + } + })); + + register(MyComponent.class); + complete(); + + MyComponent component = getInstance(MyComponent.class); + assertThat(component.url, is("http://yahoo.com")); + assertNotNull(component.executor); + } + + @Test + public void guice_injector_creates_a_new_instance_with_default_ctor_when_no_explicit_binding_exists() { + setInjector(emptyGuiceInjector()); + register(ComponentTakingDefaultString.class); + complete(); + + ComponentTakingDefaultString component = getInstance(ComponentTakingDefaultString.class); + assertThat(component.injectedString, is("")); + } + + @Test + public void guice_injector_fails_when_no_explicit_binding_exists_and_class_has_no_default_ctor() { + setInjector(emptyGuiceInjector()); + register(ComponentThatCannotBeConstructed.class); + + exception.expect(RuntimeException.class); + exception.expectMessage("When resolving dependencies of 'com.yahoo.container.di.componentgraph.core.FallbackToGuiceInjectorTest$ComponentThatCannotBeConstructed'"); + complete(); + } + + public void register(Class componentClass) { + componentGraph.add(mockComponentNode(componentClass)); + } + + public ComponentId toId(Class componentClass) { + return ComponentId.fromString(componentClass.getName()); + } + + @SuppressWarnings("unchecked") + private Node mockComponentNode(Class componentClass) { + return new ComponentNode(toId(componentClass), toId(componentClass).toString(), (Class)componentClass, null); + } + + public T getInstance(Class componentClass) { + return componentGraph.getInstance(componentClass); + } + + public void complete() { + componentGraph.complete(injector); + componentGraph.setAvailableConfigs(configs); + } + + public void setInjector(Injector injector) { + this.injector = injector; + } + + private Injector emptyGuiceInjector() { + return Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + } + }); + } +} diff --git a/container-di/src/test/java/demo/Base.java b/container-di/src/test/java/demo/Base.java deleted file mode 100644 index e98d85a9193..00000000000 --- a/container-di/src/test/java/demo/Base.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package demo; - -import com.google.inject.Guice; -import com.google.inject.Injector; -import com.yahoo.component.ComponentId; -import com.yahoo.config.ConfigInstance; -import com.yahoo.container.di.componentgraph.core.ComponentGraph; -import com.yahoo.container.di.componentgraph.core.ComponentNode; -import com.yahoo.container.di.componentgraph.core.Node; -import com.yahoo.vespa.config.ConfigKey; -import org.junit.Before; - -import java.util.HashMap; -import java.util.Map; - -/** - * @author Tony Vaagenes - * @author gjoranv - */ -public class Base { - private ComponentGraph componentGraph; - private Injector injector; - private Map, ConfigInstance> configs = - new HashMap<>(); - - @Before - public void createGraph() { - injector = Guice.createInjector(); - componentGraph = new ComponentGraph(0); - } - - public void register(Class componentClass) { - componentGraph.add(mockComponentNode(componentClass)); - } - - public ComponentId toId(Class componentClass) { - return ComponentId.fromString(componentClass.getName()); - } - - @SuppressWarnings("unchecked") - private Node mockComponentNode(Class componentClass) { - return new ComponentNode(toId(componentClass), toId(componentClass).toString(), (Class)componentClass, null); - } - - public T getInstance(Class componentClass) { - return componentGraph.getInstance(componentClass); - } - - public void complete() { - componentGraph.complete(injector); - componentGraph.setAvailableConfigs(configs); - } - - public void setInjector(Injector injector) { - this.injector = injector; - } - -} diff --git a/container-di/src/test/java/demo/FallbackToGuiceInjectorTest.java b/container-di/src/test/java/demo/FallbackToGuiceInjectorTest.java deleted file mode 100644 index 4b7d9d54725..00000000000 --- a/container-di/src/test/java/demo/FallbackToGuiceInjectorTest.java +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package demo; - -import com.google.inject.AbstractModule; -import com.google.inject.Guice; -import com.google.inject.Inject; -import com.google.inject.Injector; -import com.google.inject.name.Named; -import com.google.inject.name.Names; -import com.yahoo.component.AbstractComponent; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; - -import java.util.concurrent.Executor; -import java.util.concurrent.Executors; - -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertNotNull; - -/** - * @author Tony Vaagenes - * @author gjoranv - */ -@SuppressWarnings("unused") -public class FallbackToGuiceInjectorTest extends Base { - public static class MyComponent extends AbstractComponent { - private final String url; - private final Executor executor; - - @Inject - public MyComponent(@Named("url") String url, Executor executor) { - this.url = url; - this.executor = executor; - } - - public MyComponent() { - throw new RuntimeException("Constructor annotated with @Inject is preferred."); - } - } - - public static class ComponentTakingDefaultString{ - private final String injectedString; - - public ComponentTakingDefaultString(String empty_string_created_by_guice) { - this.injectedString = empty_string_created_by_guice; - } - } - - public static class ComponentThatCannotBeConstructed { - public ComponentThatCannotBeConstructed(Integer cannot_be_injected_because_Integer_has_no_default_ctor) { } - } - - @Rule - public final ExpectedException exception = ExpectedException.none(); - - @Test - public void guice_injector_is_used_when_no_global_component_exists() { - setInjector( - Guice.createInjector(new AbstractModule() { - @Override - protected void configure() { - bind(Executor.class).toInstance(Executors.newSingleThreadExecutor()); - bind(String.class).annotatedWith(Names.named("url")).toInstance("http://yahoo.com"); - } - })); - - register(MyComponent.class); - complete(); - - MyComponent component = getInstance(MyComponent.class); - assertThat(component.url, is("http://yahoo.com")); - assertNotNull(component.executor); - } - - @Test - public void guice_injector_creates_a_new_instance_with_default_ctor_when_no_explicit_binding_exists() { - setInjector(emptyGuiceInjector()); - register(ComponentTakingDefaultString.class); - complete(); - - ComponentTakingDefaultString component = getInstance(ComponentTakingDefaultString.class); - assertThat(component.injectedString, is("")); - } - - @Test - public void guice_injector_fails_when_no_explicit_binding_exists_and_class_has_no_default_ctor() { - setInjector(emptyGuiceInjector()); - register(ComponentThatCannotBeConstructed.class); - - exception.expect(RuntimeException.class); - exception.expectMessage("When resolving dependencies of 'demo.FallbackToGuiceInjectorTest$ComponentThatCannotBeConstructed'"); - complete(); - } - - private Injector emptyGuiceInjector() { - return Guice.createInjector(new AbstractModule() { - @Override - protected void configure() { - } - }); - } -} -- cgit v1.2.3 From 554497f70c3f39b5e40adcf3dd4c305b18a2a826 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 10 Jul 2020 00:25:17 +0200 Subject: Let ContainerAndDiOsgi create its own BundleManager --- .../java/com/yahoo/container/core/config/HandlersConfigurerDi.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 270decfeaeb..f6dc696d945 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 @@ -81,7 +81,7 @@ public class HandlersConfigurerDi { OsgiFramework osgiFramework) { this(subscriberFactory, vespaContainer, configId, deconstructor, discInjector, - new ContainerAndDiOsgi(osgiFramework, new BundleManager(new OsgiImpl(osgiFramework)))); + new ContainerAndDiOsgi(osgiFramework)); } // Only public for testing @@ -103,10 +103,10 @@ public class HandlersConfigurerDi { private final OsgiFramework osgiFramework; private final BundleManager bundleLoader; - public ContainerAndDiOsgi(OsgiFramework osgiFramework, BundleManager bundleLoader) { + public ContainerAndDiOsgi(OsgiFramework osgiFramework) { super(osgiFramework); this.osgiFramework = osgiFramework; - this.bundleLoader = bundleLoader; + bundleLoader = new BundleManager(new OsgiImpl(osgiFramework)); } -- cgit v1.2.3 From 5ec850de6be84cf92bdd58dae770df94c7a5dc9c Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 10 Jul 2020 00:25:44 +0200 Subject: Rename field bundleLoader -> bundleManager --- .../java/com/yahoo/container/core/config/HandlersConfigurerDi.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 f6dc696d945..b983cbcfe37 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 @@ -101,12 +101,12 @@ public class HandlersConfigurerDi { private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper { private final OsgiFramework osgiFramework; - private final BundleManager bundleLoader; + private final BundleManager bundleManager; public ContainerAndDiOsgi(OsgiFramework osgiFramework) { super(osgiFramework); this.osgiFramework = osgiFramework; - bundleLoader = new BundleManager(new OsgiImpl(osgiFramework)); + bundleManager = new BundleManager(new OsgiImpl(osgiFramework)); } @@ -133,7 +133,7 @@ public class HandlersConfigurerDi { @Override public Set useBundles(Collection bundles) { log.info("Installing bundles from the latest application"); - return bundleLoader.use(new ArrayList<>(bundles)); + return bundleManager.use(new ArrayList<>(bundles)); } } -- cgit v1.2.3