Add coverage for policy-management

Added coverage for PolicyControllerFactory and AggregatedPolicyController.
Fixed some typos in comments.
Reformatted some code.

Change-Id: I33aea8e1e7dde29bd51218d0ecad7b34047b33e5
Issue-ID: POLICY-1148
Signed-off-by: Jim Hahn <jrh3@att.com>

5 files changed, 1439 insertions, 42 deletions

diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/PolicyControllerFactory.java b/policy-management/src/main/java/org/onap/policy/drools/system/PolicyControllerFactory.java
index 1241acad..c0749790 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/system/PolicyControllerFactory.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/PolicyControllerFactory.java
@@ -218,8 +218,7 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
 
         /* A PolicyController does not exist */
 
-        PolicyController controller =
-                new AggregatedPolicyController(name, properties);
+        PolicyController controller = newPolicyController(name, properties);
 
         String coordinates = toKey(controller.getDrools().getGroupId(),
                                    controller.getDrools().getArtifactId());
@@ -254,9 +253,7 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
 
         this.patch(controller, droolsConfig);
 
-        if (logger.isInfoEnabled()) {
-            logger.info("UPDATED drools configuration: {} on {}", droolsConfig, this);
-        }
+        logger.info("UPDATED drools configuration: {} on {}", droolsConfig, this);
 
         return controller;
     }
@@ -281,9 +278,7 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
             throw new IllegalArgumentException("Cannot update drools configuration Drools Configuration");
         }
 
-        if (logger.isInfoEnabled()) {
-            logger.info("UPDATED drools configuration: {} on {}", droolsConfig, this);
-        }
+        logger.info("UPDATED drools configuration: {} on {}", droolsConfig, this);
 
         String coordinates = toKey(controller.getDrools().getGroupId(),
                                    controller.getDrools().getArtifactId());
@@ -484,7 +479,7 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
     @Override
     public List<String> getFeatures() {
         List<String> features = new ArrayList<>();
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : getProviders()) {
             features.add(feature.getName());
         }
         return features;
@@ -496,7 +491,7 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
     @JsonIgnore
     @Override
     public List<PolicyControllerFeatureAPI> getFeatureProviders() {
-        return PolicyControllerFeatureAPI.providers.getList();
+        return getProviders();
     }
 
     /**
@@ -508,7 +503,7 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
             throw new IllegalArgumentException("A feature name must be provided");
         }
         
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : getProviders()) {
             if (feature.getName().equals(featureName)) {
                 return feature;
             }
@@ -520,4 +515,14 @@ class IndexedPolicyControllerFactory implements PolicyControllerFactory {
     private IllegalArgumentException makeArgEx(String argName) {
         return new IllegalArgumentException("Invalid " + argName);
     }
+    
+    // these methods can be overridden by junit tests
+        
+    protected PolicyController newPolicyController(String name, Properties properties) {
+        return new AggregatedPolicyController(name, properties);
+    }
+
+    protected List<PolicyControllerFeatureAPI> getProviders() {
+        return PolicyControllerFeatureAPI.providers.getList();
+    }
 }
diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/AggregatedPolicyController.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/AggregatedPolicyController.java
index 8d290674..581184ee 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/system/internal/AggregatedPolicyController.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/AggregatedPolicyController.java
@@ -32,6 +32,7 @@ import org.onap.policy.common.endpoints.event.comm.TopicListener;
 import org.onap.policy.common.endpoints.event.comm.TopicSink;
 import org.onap.policy.common.endpoints.event.comm.TopicSource;
 import org.onap.policy.drools.controller.DroolsController;
+import org.onap.policy.drools.controller.DroolsControllerFactory;
 import org.onap.policy.drools.features.PolicyControllerFeatureAPI;
 import org.onap.policy.drools.persistence.SystemPersistence;
 import org.onap.policy.drools.properties.DroolsProperties;
@@ -52,6 +53,11 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     private static final Logger logger = LoggerFactory.getLogger(AggregatedPolicyController.class);
 
     /**
+     * Used to access various objects.  Can be overridden by junit tests.
+     */
+    private static Factory factory = new Factory();
+
+    /**
      * identifier for this policy controller.
      */
     private final String name;
@@ -115,14 +121,14 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         // Create/Reuse Readers/Writers for all event sources endpoints
 
-        this.sources = TopicEndpoint.manager.addTopicSources(properties);
-        this.sinks = TopicEndpoint.manager.addTopicSinks(properties);
+        this.sources = factory.getEndpointManager().addTopicSources(properties);
+        this.sinks = factory.getEndpointManager().addTopicSinks(properties);
 
         initDrools(properties);
         initSinks();
 
         /* persist new properties */
-        SystemPersistence.manager.storeController(name, properties);
+        factory.getPersistenceManager().storeController(name, properties);
         this.properties = properties;
     }
 
@@ -134,7 +140,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     private void initDrools(Properties properties) {
         try {
             // Register with drools infrastructure
-            this.droolsController = DroolsController.factory.build(properties, sources, sinks);
+            this.droolsController = factory.getDroolsFactory().build(properties, sources, sinks);
         } catch (Exception | LinkageError e) {
             logger.error("{}: cannot init-drools because of {}", this, e.getMessage(), e);
             throw new IllegalArgumentException(e);
@@ -177,7 +183,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
             this.properties.setProperty(DroolsProperties.RULES_ARTIFACTID, newDroolsConfiguration.getArtifactId());
             this.properties.setProperty(DroolsProperties.RULES_VERSION, newDroolsConfiguration.getVersion());
 
-            SystemPersistence.manager.storeController(name, this.properties);
+            factory.getPersistenceManager().storeController(name, this.properties);
 
             this.initDrools(this.properties);
 
@@ -220,7 +226,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     public boolean start() {
         logger.info("{}: start", this);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeStart(this)) {
                     return true;
@@ -259,7 +265,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
             }
         }
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterStart(this)) {
                     return true;
@@ -280,7 +286,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     public boolean stop() {
         logger.info("{}: stop", this);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeStop(this)) {
                     return true;
@@ -309,7 +315,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         boolean success = this.droolsController.stop();
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterStop(this)) {
                     return true;
@@ -330,7 +336,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     public void shutdown() {
         logger.info("{}: shutdown", this);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeShutdown(this)) {
                     return;
@@ -343,9 +349,9 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         this.stop();
 
-        DroolsController.factory.shutdown(this.droolsController);
+        factory.getDroolsFactory().shutdown(this.droolsController);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterShutdown(this)) {
                     return;
@@ -364,7 +370,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     public void halt() {
         logger.info("{}: halt", this);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeHalt(this)) {
                     return;
@@ -376,10 +382,10 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
         }
 
         this.stop();
-        DroolsController.factory.destroy(this.droolsController);
-        SystemPersistence.manager.deleteController(this.name);
+        factory.getDroolsFactory().destroy(this.droolsController);
+        factory.getPersistenceManager().deleteController(this.name);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterHalt(this)) {
                     return;
@@ -397,11 +403,9 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     @Override
     public void onTopicEvent(Topic.CommInfrastructure commType, String topic, String event) {
 
-        if (logger.isDebugEnabled()) {
-            logger.debug("{}: event offered from {}:{}: {}", this, commType, topic, event);
-        }
+        logger.debug("{}: event offered from {}:{}: {}", this, commType, topic, event);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeOffer(this, commType, topic, event)) {
                     return;
@@ -422,7 +426,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         boolean success = this.droolsController.offer(topic, event);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterOffer(this, commType, topic, event, success)) {
                     return;
@@ -440,11 +444,9 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     @Override
     public boolean deliver(Topic.CommInfrastructure commType, String topic, Object event) {
 
-        if (logger.isDebugEnabled()) {
-            logger.debug("{}: deliver event to {}:{}: {}", this, commType, topic, event);
-        }
+        logger.debug("{}: deliver event to {}:{}: {}", this, commType, topic, event);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeDeliver(this, commType, topic, event)) {
                     return true;
@@ -479,7 +481,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         boolean success = this.droolsController.deliver(this.topic2Sinks.get(topic), event);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterDeliver(this, commType, topic, event, success)) {
                     return success;
@@ -508,7 +510,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
     public boolean lock() {
         logger.info("{}: lock", this);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeLock(this)) {
                     return true;
@@ -532,7 +534,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         boolean success = this.droolsController.lock();
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterLock(this)) {
                     return true;
@@ -554,7 +556,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         logger.info("{}: unlock", this);
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.beforeUnlock(this)) {
                     return true;
@@ -575,7 +577,7 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
 
         boolean success = this.droolsController.unlock();
 
-        for (PolicyControllerFeatureAPI feature : PolicyControllerFeatureAPI.providers.getList()) {
+        for (PolicyControllerFeatureAPI feature : factory.getFeatureProviders()) {
             try {
                 if (feature.afterUnlock(this)) {
                     return true;
@@ -637,5 +639,26 @@ public class AggregatedPolicyController implements PolicyController, TopicListen
                 + ", locked=" + locked + ", droolsController=" + droolsController + "]";
     }
 
+    /**
+     * Factory to access various objects.  Can be overridden by junit tests.
+     */
+    public static class Factory {
+        
+        public SystemPersistence getPersistenceManager() {
+            return SystemPersistence.manager;
+        }
+
+        public TopicEndpoint getEndpointManager() {
+            return TopicEndpoint.manager;
+        }
+
+        public DroolsControllerFactory getDroolsFactory() {
+            return DroolsController.factory;
+        }
+
+        public List<PolicyControllerFeatureAPI> getFeatureProviders() {
+            return PolicyControllerFeatureAPI.providers.getList();
+        }
+    }
 }
 
diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyControllerFactoryTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyControllerFactoryTest.java
new file mode 100644
index 00000000..f5c71064
--- /dev/null
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyControllerFactoryTest.java
@@ -0,0 +1,421 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.drools.system;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.onap.policy.common.utils.test.PolicyAssert.assertThrows;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Properties;
+import org.junit.Before;
+import org.junit.Test;
+import org.onap.policy.drools.controller.DroolsController;
+import org.onap.policy.drools.features.PolicyControllerFeatureAPI;
+import org.onap.policy.drools.protocol.configuration.DroolsConfiguration;
+
+public class PolicyControllerFactoryTest {
+
+    private static final String MY_NAME = "my-name-a";
+    private static final String MY_NAME2 = "my-name-b";
+
+    private static final String ARTIFACT1 = "artifact-a";
+    private static final String GROUP1 = "group-a";
+    private static final String VERSION1 = "version-a";
+
+    private static final String ARTIFACT2 = "artifact-b";
+    private static final String GROUP2 = "group-b";
+    private static final String VERSION2 = "version-b";
+
+    private static final String FEATURE1 = "feature-a";
+    private static final String FEATURE2 = "feature-b";
+
+    private PolicyController controller;
+    private PolicyController controller2;
+    private Properties properties;
+    private DroolsController drools;
+    private DroolsController drools2;
+    private DroolsConfiguration config;
+    private PolicyControllerFeatureAPI feature1;
+    private PolicyControllerFeatureAPI feature2;
+    private List<PolicyControllerFeatureAPI> providers;
+    private IndexedPolicyControllerFactory ipc;
+
+    /**
+     * Initializes the object to be tested.
+     */
+    @Before
+    public void setUp() {
+        controller = mock(PolicyController.class);
+        controller2 = mock(PolicyController.class);
+        properties = new Properties();
+        drools = mock(DroolsController.class);
+        drools2 = mock(DroolsController.class);
+        config = mock(DroolsConfiguration.class);
+        feature1 = mock(PolicyControllerFeatureAPI.class);
+        feature2 = mock(PolicyControllerFeatureAPI.class);
+        providers = Arrays.asList(feature1, feature2);
+
+        when(feature1.getName()).thenReturn(FEATURE1);
+        when(feature2.getName()).thenReturn(FEATURE2);
+
+        when(drools.getArtifactId()).thenReturn(ARTIFACT1);
+        when(drools.getGroupId()).thenReturn(GROUP1);
+        when(drools.getVersion()).thenReturn(VERSION1);
+
+        when(drools2.getArtifactId()).thenReturn(ARTIFACT2);
+        when(drools2.getGroupId()).thenReturn(GROUP2);
+        when(drools2.getVersion()).thenReturn(VERSION2);
+
+        when(controller.getName()).thenReturn(MY_NAME);
+        when(controller.getDrools()).thenReturn(drools);
+        when(controller.updateDrools(any())).thenReturn(true);
+
+        when(controller2.getName()).thenReturn(MY_NAME2);
+        when(controller2.getDrools()).thenReturn(drools2);
+        when(controller2.updateDrools(any())).thenReturn(true);
+
+        ipc = new IndexedPolicyControllerFactoryImpl();
+    }
+
+    @Test
+    public void testFactory() {
+        // use a REAL object instead of an Impl
+        ipc = new IndexedPolicyControllerFactory();
+        assertNotNull(ipc.getProviders());
+    }
+
+    @Test
+    public void testBuild() {
+        assertEquals(controller, ipc.build(MY_NAME, properties));
+
+        // re-build - should not create another one
+        assertEquals(controller, ipc.build(MY_NAME, properties));
+
+        // brained
+        setUp();
+        when(drools.isBrained()).thenReturn(true);
+        ipc.build(MY_NAME, properties);
+    }
+
+    @Test
+    public void testPatchStringDroolsConfiguration() {
+        // unknown controller
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch(MY_NAME, config));
+
+        /*
+         * Build controller to be used by remaining tests.
+         */
+        ipc.build(MY_NAME, properties);
+
+        // null name
+        String nullName = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch(nullName, config));
+
+        // empty name
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch("", config));
+
+        // success
+        ipc.patch(MY_NAME, config);
+        verify(controller).updateDrools(config);
+
+        // create a factory whose get() method returns null
+        ipc = new IndexedPolicyControllerFactory() {
+            @Override
+            public PolicyController get(String name) {
+                return null;
+            }
+        };
+        ipc.build(MY_NAME, properties);
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch(MY_NAME, config));
+    }
+
+    @Test
+    public void testPatchPolicyControllerDroolsConfiguration() {
+        ipc.patch(controller, config);
+        verify(controller).updateDrools(config);
+
+        // null controller
+        PolicyController nullCtlr = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch(nullCtlr, config));
+
+        // null config
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch(controller, null));
+
+        // brained
+        when(drools.isBrained()).thenReturn(true);
+        ipc.patch(controller, config);
+
+        // update failed
+        when(controller.updateDrools(config)).thenReturn(false);
+        assertThrows(IllegalArgumentException.class, () -> ipc.patch(controller, config));
+    }
+
+    @Test
+    public void testShutdownString() {
+        // null name
+        String nullName = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.shutdown(nullName));
+
+        // empty name
+        assertThrows(IllegalArgumentException.class, () -> ipc.shutdown(""));
+
+        // unknown controller
+        ipc.shutdown(MY_NAME);
+        verify(controller, never()).shutdown();
+
+        // valid controller
+        ipc.build(MY_NAME, properties);
+        ipc.shutdown(MY_NAME);
+        verify(controller).shutdown();
+    }
+
+    @Test
+    public void testShutdownPolicyController() {
+        ipc.build(MY_NAME, properties);
+
+        ipc.shutdown(controller);
+
+        verify(controller).shutdown();
+
+        // should no longer be managed
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME));
+    }
+
+    @Test
+    public void testShutdown() {
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        ipc.shutdown();
+
+        verify(controller).shutdown();
+        verify(controller2).shutdown();
+
+        // should no longer be managed
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME));
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME2));
+    }
+
+    @Test
+    public void testUnmanage() {
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        ipc.shutdown(MY_NAME);
+
+        verify(controller).shutdown();
+        verify(controller2, never()).shutdown();
+
+        // should no longer be managed
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME));
+
+        // should still be managed
+        assertEquals(controller2, ipc.get(MY_NAME2));
+
+        // null controller
+        PolicyController nullCtlr = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.shutdown(nullCtlr));
+
+        // unknown controller
+        ipc.shutdown(controller);
+        verify(controller, times(2)).shutdown();
+    }
+
+    @Test
+    public void testDestroyString() {
+        // null name
+        String nullName = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.destroy(nullName));
+
+        // empty name
+        assertThrows(IllegalArgumentException.class, () -> ipc.destroy(""));
+
+        // unknown controller
+        ipc.destroy(MY_NAME);
+        verify(controller, never()).halt();
+
+        // valid controller
+        ipc.build(MY_NAME, properties);
+        ipc.destroy(MY_NAME);
+        verify(controller).halt();
+    }
+
+    @Test
+    public void testDestroyPolicyController() {
+        ipc.build(MY_NAME, properties);
+
+        ipc.destroy(controller);
+
+        verify(controller).halt();
+
+        // should no longer be managed
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME));
+    }
+
+    @Test
+    public void testDestroy() {
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        ipc.destroy();
+
+        verify(controller).halt();
+        verify(controller2).halt();
+
+        // should no longer be managed
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME));
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME2));
+    }
+
+    @Test
+    public void testGetString() {
+        // unknown name
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(MY_NAME));
+
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        assertEquals(controller, ipc.get(MY_NAME));
+        assertEquals(controller2, ipc.get(MY_NAME2));
+
+        // null name
+        String nullName = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(nullName));
+
+        // empty name
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(""));
+    }
+
+    @Test
+    public void testGetStringString_testToKey() {
+        // unknown controller
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(GROUP1, ARTIFACT1));
+
+        when(drools.isBrained()).thenReturn(true);
+        when(drools2.isBrained()).thenReturn(true);
+
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        assertEquals(controller, ipc.get(GROUP1, ARTIFACT1));
+        assertEquals(controller2, ipc.get(GROUP2, ARTIFACT2));
+
+        // null group
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(null, ARTIFACT1));
+
+        // empty group
+        assertThrows(IllegalArgumentException.class, () -> ipc.get("", ARTIFACT1));
+
+        // null artifact
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(GROUP1, null));
+
+        // empty artifact
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(GROUP1, ""));
+    }
+
+    @Test
+    public void testGetDroolsController() {
+        // unknown controller
+        assertThrows(IllegalStateException.class, () -> ipc.get(drools));
+
+        when(drools.isBrained()).thenReturn(true);
+        when(drools2.isBrained()).thenReturn(true);
+
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        assertEquals(controller, ipc.get(drools));
+        assertEquals(controller2, ipc.get(drools2));
+
+        // null controller
+        DroolsController nullDrools = null;
+        assertThrows(IllegalArgumentException.class, () -> ipc.get(nullDrools));
+    }
+
+    @Test
+    public void testInventory() {
+        ipc.build(MY_NAME, properties);
+        ipc.build(MY_NAME2, properties);
+
+        List<PolicyController> lst = ipc.inventory();
+        Collections.sort(lst, (left, right) -> left.getName().compareTo(right.getName()));
+        assertEquals(Arrays.asList(controller, controller2), lst);
+    }
+
+    @Test
+    public void testGetFeatures() {
+        assertEquals(Arrays.asList(FEATURE1, FEATURE2), ipc.getFeatures());
+    }
+
+    @Test
+    public void testGetFeatureProviders() {
+        assertEquals(providers, ipc.getFeatureProviders());
+    }
+
+    @Test
+    public void testGetFeatureProvider() {
+        // null name
+        assertThrows(IllegalArgumentException.class, () -> ipc.getFeatureProvider(null));
+
+        // empty name
+        assertThrows(IllegalArgumentException.class, () -> ipc.getFeatureProvider(""));
+
+        // unknown name
+        assertThrows(IllegalArgumentException.class, () -> ipc.getFeatureProvider("unknown-feature"));
+
+        assertEquals(feature1, ipc.getFeatureProvider(FEATURE1));
+        assertEquals(feature2, ipc.getFeatureProvider(FEATURE2));
+    }
+
+    /**
+     * Factory with overrides.
+     */
+    private class IndexedPolicyControllerFactoryImpl extends IndexedPolicyControllerFactory {
+
+        @Override
+        protected PolicyController newPolicyController(String name, Properties properties) {
+            if (MY_NAME.equals(name)) {
+                return controller;
+
+            } else if (MY_NAME2.equals(name)) {
+                return controller2;
+
+            } else {
+                throw new IllegalArgumentException("unknown controller name: " + name);
+
+            }
+        }
+
+        @Override
+        protected List<PolicyControllerFeatureAPI> getProviders() {
+            return providers;
+        }
+    }
+}
diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/test/PolicyEngineTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineTest.java
index 7b02d755..cd93d94c 100644
--- a/policy-management/src/test/java/org/onap/policy/drools/system/test/PolicyEngineTest.java
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineTest.java
@@ -18,7 +18,7 @@
  * ============LICENSE_END=========================================================
  */
 
-package org.onap.policy.drools.system.test;
+package org.onap.policy.drools.system;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java
new file mode 100644
index 00000000..4f26419f
--- /dev/null
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java
@@ -0,0 +1,948 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.drools.system.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.onap.policy.common.utils.test.PolicyAssert.assertThrows;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.onap.policy.common.endpoints.event.comm.Topic.CommInfrastructure;
+import org.onap.policy.common.endpoints.event.comm.TopicEndpoint;
+import org.onap.policy.common.endpoints.event.comm.TopicSink;
+import org.onap.policy.common.endpoints.event.comm.TopicSource;
+import org.onap.policy.drools.controller.DroolsController;
+import org.onap.policy.drools.controller.DroolsControllerFactory;
+import org.onap.policy.drools.features.PolicyControllerFeatureAPI;
+import org.onap.policy.drools.persistence.SystemPersistence;
+import org.onap.policy.drools.protocol.configuration.DroolsConfiguration;
+import org.onap.policy.drools.system.internal.AggregatedPolicyController.Factory;
+import org.powermock.reflect.Whitebox;
+
+public class AggregatedPolicyControllerTest {
+
+    /**
+     * Name of the "factory" field within the {@link AggregatedPolicyController} class.
+     */
+    private static final String FACTORY_FIELD = "factory";
+
+    private static final String AGG_NAME = "agg-name";
+    private static final String SINK_TOPIC1 = "sink-a";
+    private static final String SINK_TOPIC2 = "sink-b";
+    private static final String SOURCE_TOPIC1 = "source-a";
+    private static final String SOURCE_TOPIC2 = "source-b";
+
+    private static final String EXPECTED = "expected exception";
+
+    private static final String MY_EVENT = "my-event";
+
+    private static final String ARTIFACT1 = "artifact-a";
+    private static final String GROUP1 = "group-a";
+    private static final String VERSION1 = "version-a";
+
+    private static final String ARTIFACT2 = "artifact-b";
+    private static final String GROUP2 = "group-b";
+    private static final String VERSION2 = "version-b";
+
+    private static Factory savedFactory;
+
+    private Properties properties;
+    private Factory factory;
+    private TopicEndpoint endpointMgr;
+    private List<TopicSource> sources;
+    private TopicSource source1;
+    private TopicSource source2;
+    private List<TopicSink> sinks;
+    private TopicSink sink1;
+    private TopicSink sink2;
+    private SystemPersistence persist;
+    private DroolsControllerFactory droolsFactory;
+    private DroolsController drools;
+    private DroolsConfiguration config;
+    private List<PolicyControllerFeatureAPI> providers;
+    private PolicyControllerFeatureAPI prov1;
+    private PolicyControllerFeatureAPI prov2;
+    private AggregatedPolicyController apc;
+
+    @BeforeClass
+    public static void setUpBeforeClass() {
+        savedFactory = Whitebox.getInternalState(AggregatedPolicyController.class, FACTORY_FIELD);
+    }
+
+    @AfterClass
+    public static void tearDownAfterClass() {
+        Whitebox.setInternalState(AggregatedPolicyController.class, FACTORY_FIELD, savedFactory);
+    }
+
+    /**
+     * Initializes the object to be tested.
+     */
+    @Before
+    public void setUp() {
+        properties = new Properties();
+
+        source1 = mock(TopicSource.class);
+        source2 = mock(TopicSource.class);
+        when(source1.getTopic()).thenReturn(SOURCE_TOPIC1);
+        when(source2.getTopic()).thenReturn(SOURCE_TOPIC2);
+
+        sink1 = mock(TopicSink.class);
+        sink2 = mock(TopicSink.class);
+        when(sink1.getTopic()).thenReturn(SINK_TOPIC1);
+        when(sink2.getTopic()).thenReturn(SINK_TOPIC2);
+
+        sources = Arrays.asList(source1, source2);
+        sinks = Arrays.asList(sink1, sink2);
+
+        endpointMgr = mock(TopicEndpoint.class);
+        when(endpointMgr.addTopicSources(any())).thenReturn(sources);
+        when(endpointMgr.addTopicSinks(any())).thenReturn(sinks);
+
+        persist = mock(SystemPersistence.class);
+
+        drools = mock(DroolsController.class);
+        when(drools.start()).thenReturn(true);
+        when(drools.stop()).thenReturn(true);
+        when(drools.offer(any(), any())).thenReturn(true);
+        when(drools.deliver(any(), any())).thenReturn(true);
+        when(drools.lock()).thenReturn(true);
+        when(drools.unlock()).thenReturn(true);
+        when(drools.getArtifactId()).thenReturn(ARTIFACT1);
+        when(drools.getGroupId()).thenReturn(GROUP1);
+        when(drools.getVersion()).thenReturn(VERSION1);
+
+        config = mock(DroolsConfiguration.class);
+        when(config.getArtifactId()).thenReturn(ARTIFACT2);
+        when(config.getGroupId()).thenReturn(GROUP2);
+        when(config.getVersion()).thenReturn(VERSION2);
+
+        droolsFactory = mock(DroolsControllerFactory.class);
+        when(droolsFactory.build(any(), any(), any())).thenReturn(drools);
+
+        prov1 = mock(PolicyControllerFeatureAPI.class);
+        prov2 = mock(PolicyControllerFeatureAPI.class);
+
+        providers = Arrays.asList(prov1, prov2);
+
+        factory = mock(Factory.class);
+        Whitebox.setInternalState(AggregatedPolicyController.class, FACTORY_FIELD, factory);
+
+        when(factory.getEndpointManager()).thenReturn(endpointMgr);
+        when(factory.getPersistenceManager()).thenReturn(persist);
+        when(factory.getDroolsFactory()).thenReturn(droolsFactory);
+        when(factory.getFeatureProviders()).thenReturn(providers);
+
+        apc = new AggregatedPolicyController(AGG_NAME, properties);
+    }
+
+    @Test
+    public void testFactory() {
+        assertNotNull(savedFactory);
+
+        Factory factory = new Factory();
+        assertNotNull(factory.getDroolsFactory());
+        assertNotNull(factory.getEndpointManager());
+        assertNotNull(factory.getFeatureProviders());
+        assertNotNull(factory.getPersistenceManager());
+    }
+
+    @Test
+    public void testAggregatedPolicyController_() {
+        verify(persist).storeController(AGG_NAME, properties);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInitDrools_Ex() {
+        when(factory.getDroolsFactory()).thenThrow(new RuntimeException(EXPECTED));
+        new AggregatedPolicyController(AGG_NAME, properties);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInitDrools_Error() {
+        when(factory.getDroolsFactory()).thenThrow(new LinkageError(EXPECTED));
+        new AggregatedPolicyController(AGG_NAME, properties);
+    }
+
+    @Test
+    public void testUpdateDrools_ConfigVariations() {
+
+        // config should return same values as current controller
+        when(config.getArtifactId()).thenReturn(ARTIFACT1.toUpperCase());
+        when(config.getGroupId()).thenReturn(GROUP1.toUpperCase());
+        when(config.getVersion()).thenReturn(VERSION1.toUpperCase());
+
+        assertTrue(apc.updateDrools(config));
+
+        // number of times store should have been called
+        int count = 0;
+
+        // invoked once during construction, but shouldn't be invoked during update
+        verify(persist, times(++count)).storeController(any(), any());
+
+
+        // different artifact
+        when(config.getArtifactId()).thenReturn(ARTIFACT2);
+
+        assertTrue(apc.updateDrools(config));
+
+        // should be invoked during update
+        verify(persist, times(++count)).storeController(any(), any());
+
+
+        // different group
+        when(config.getArtifactId()).thenReturn(ARTIFACT1);
+        when(config.getGroupId()).thenReturn(GROUP2);
+
+        assertTrue(apc.updateDrools(config));
+
+        // should be invoked during update
+        verify(persist, times(++count)).storeController(any(), any());
+
+
+        // different version
+        when(config.getGroupId()).thenReturn(GROUP1);
+        when(config.getVersion()).thenReturn(VERSION2);
+
+        assertTrue(apc.updateDrools(config));
+
+        // should be invoked during update
+        verify(persist, times(++count)).storeController(any(), any());
+
+
+        /*
+         * Exception case.
+         */
+        when(drools.lock()).thenThrow(new IllegalArgumentException(EXPECTED));
+        when(drools.unlock()).thenThrow(new IllegalArgumentException(EXPECTED));
+
+        assertFalse(apc.updateDrools(config));
+    }
+
+    @Test
+    public void testUpdateDrools_LockVariations() {
+        // not locked
+        apc.updateDrools(config);
+        verify(drools, never()).lock();
+        verify(drools).unlock();
+
+        // locked
+        setUp();
+        apc.lock();
+        apc.updateDrools(config);
+        verify(drools, times(2)).lock();
+        verify(drools, never()).unlock();
+    }
+
+    @Test
+    public void testUpdateDrools_AliveVariations() {
+        // not started
+        apc.updateDrools(config);
+        verify(drools, never()).start();
+        verify(drools).stop();
+
+        // started
+        setUp();
+        apc.start();
+        apc.updateDrools(config);
+        verify(drools, times(2)).start();
+        verify(drools, never()).stop();
+    }
+
+    @Test
+    public void testGetName() {
+        assertEquals(AGG_NAME, apc.getName());
+    }
+
+    @Test
+    public void testStart() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeStart(any())).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterStart(any())).thenThrow(new RuntimeException(EXPECTED));
+
+        // arrange for first sink to throw exception
+        when(sink1.start()).thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        assertTrue(apc.start());
+
+        assertTrue(apc.isAlive());
+
+        verify(prov1).beforeStart(apc);
+        verify(prov2).beforeStart(apc);
+
+        verify(source1).register(apc);
+        verify(source2).register(apc);
+
+        verify(sink1).start();
+        verify(sink2).start();
+
+        verify(prov1).afterStart(apc);
+        verify(prov2).afterStart(apc);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeStart(apc)).thenReturn(flag),
+            (prov, flag) -> when(prov.afterStart(apc)).thenReturn(flag),
+            () -> apc.start(),
+            prov -> verify(prov).beforeStart(apc),
+            () -> verify(source1).register(apc),
+            prov -> verify(prov).afterStart(apc));
+    }
+
+    @Test
+    public void testStart_AlreadyStarted() {
+        apc.start();
+
+        // re-start it
+        assertTrue(apc.start());
+
+        assertTrue(apc.isAlive());
+
+        // these should now have been called twice
+        verify(prov1, times(2)).beforeStart(apc);
+        verify(prov2, times(2)).beforeStart(apc);
+
+        // these should still only have been called once
+        verify(source1).register(apc);
+        verify(sink1).start();
+        verify(prov1).afterStart(apc);
+    }
+
+    @Test
+    public void testStart_Locked() {
+        apc.lock();
+
+        // start it
+        assertThrows(IllegalStateException.class, () -> apc.start());
+
+        assertFalse(apc.isAlive());
+
+        // should call beforeStart(), but stop after that
+        verify(prov1).beforeStart(apc);
+        verify(prov2).beforeStart(apc);
+
+        verify(source1, never()).register(apc);
+        verify(sink1, never()).start();
+        verify(prov1, never()).afterStart(apc);
+    }
+
+    @Test
+    public void testStop() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeStop(any())).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterStop(any())).thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+
+        // now stop it
+        assertTrue(apc.stop());
+
+        assertFalse(apc.isAlive());
+
+        verify(prov1).beforeStop(apc);
+        verify(prov2).beforeStop(apc);
+
+        verify(source1).unregister(apc);
+        verify(source2).unregister(apc);
+
+        verify(prov1).afterStop(apc);
+        verify(prov2).afterStop(apc);
+
+        // ensure no shutdown operations were called
+        verify(prov1, never()).beforeShutdown(apc);
+        verify(droolsFactory, never()).shutdown(drools);
+        verify(prov2, never()).afterShutdown(apc);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeStop(apc)).thenReturn(flag),
+            (prov, flag) -> when(prov.afterStop(apc)).thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.stop();
+            },
+            prov -> verify(prov).beforeStop(apc),
+            () -> verify(source1).unregister(apc),
+            prov -> verify(prov).afterStop(apc));
+    }
+
+    @Test
+    public void testStop_AlreadyStopped() {
+        apc.start();
+        apc.stop();
+
+        // now re-stop it
+        assertTrue(apc.stop());
+
+        // called again
+        verify(prov1, times(2)).beforeStop(apc);
+        verify(prov2, times(2)).beforeStop(apc);
+
+        // should NOT be called again
+        verify(source1).unregister(apc);
+        verify(prov1).afterStop(apc);
+    }
+
+    @Test
+    public void testShutdown() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeShutdown(any())).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterShutdown(any())).thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+
+        // now shut it down
+        apc.shutdown();
+
+        verify(prov1).beforeShutdown(apc);
+        verify(prov2).beforeShutdown(apc);
+
+        assertFalse(apc.isAlive());
+
+        verify(prov1).afterStop(apc);
+        verify(prov2).afterStop(apc);
+
+        verify(droolsFactory).shutdown(drools);
+
+        verify(prov1).afterShutdown(apc);
+        verify(prov2).afterShutdown(apc);
+
+        // ensure no halt operation was called
+        verify(prov1, never()).beforeHalt(apc);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeShutdown(apc)).thenReturn(flag),
+            (prov, flag) -> when(prov.afterShutdown(apc)).thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.shutdown();
+            },
+            prov -> verify(prov).beforeShutdown(apc),
+            () -> verify(source1).unregister(apc),
+            prov -> verify(prov).afterShutdown(apc));
+    }
+
+    @Test
+    public void testHalt() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeHalt(any())).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterHalt(any())).thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+
+        // now halt it
+        apc.halt();
+
+        verify(prov1).beforeHalt(apc);
+        verify(prov2).beforeHalt(apc);
+
+        assertFalse(apc.isAlive());
+
+        verify(prov1).beforeStop(apc);
+        verify(prov2).beforeStop(apc);
+
+        verify(droolsFactory).destroy(drools);
+        verify(persist).deleteController(AGG_NAME);
+
+        verify(prov1).afterHalt(apc);
+        verify(prov2).afterHalt(apc);
+
+        // ensure no shutdown operation was called
+        verify(prov1, never()).beforeShutdown(apc);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeHalt(apc)).thenReturn(flag),
+            (prov, flag) -> when(prov.afterHalt(apc)).thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.halt();
+            },
+            prov -> verify(prov).beforeHalt(apc),
+            () -> verify(source1).unregister(apc),
+            prov -> verify(prov).afterHalt(apc));
+    }
+
+    @Test
+    public void testOnTopicEvent() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT))
+                        .thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true))
+                        .thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+
+        // now offer it
+        apc.onTopicEvent(CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+
+        verify(prov1).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+        verify(prov2).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+
+        verify(drools).offer(SOURCE_TOPIC1, MY_EVENT);
+
+        verify(prov1).afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true);
+        verify(prov2).afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT))
+                            .thenReturn(flag),
+            (prov, flag) -> when(
+                            prov.afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true))
+                                            .thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.onTopicEvent(CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+            },
+            prov -> verify(prov).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT),
+            () -> verify(drools).offer(SOURCE_TOPIC1, MY_EVENT),
+            prov -> verify(prov).afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true));
+    }
+
+    @Test
+    public void testOnTopicEvent_Locked() {
+        // start it
+        apc.start();
+
+        apc.lock();
+
+        // now offer it
+        apc.onTopicEvent(CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+
+        verify(prov1).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+        verify(prov2).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+
+        // never gets this far
+        verify(drools, never()).offer(SOURCE_TOPIC1, MY_EVENT);
+        verify(prov1, never()).afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true);
+    }
+
+    @Test
+    public void testOnTopicEvent_NotStarted() {
+
+        // offer it
+        apc.onTopicEvent(CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+
+        verify(prov1).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+        verify(prov2).beforeOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT);
+
+        // never gets this far
+        verify(drools, never()).offer(SOURCE_TOPIC1, MY_EVENT);
+        verify(prov1, never()).afterOffer(apc, CommInfrastructure.NOOP, SOURCE_TOPIC1, MY_EVENT, true);
+    }
+
+    @Test
+    public void testDeliver_testInitSinks() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT))
+                        .thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT, true))
+                        .thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+
+        // now offer it
+        assertTrue(apc.deliver(CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT));
+
+        verify(prov1).beforeDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT);
+        verify(prov2).beforeDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT);
+
+        verify(drools).deliver(sink1, MY_EVENT);
+        verify(drools, never()).deliver(sink2, MY_EVENT);
+
+        verify(prov1).afterDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT, true);
+        verify(prov2).afterDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT, true);
+
+        // offer to the other topic
+        assertTrue(apc.deliver(CommInfrastructure.NOOP, SINK_TOPIC2, MY_EVENT));
+
+        // now both topics should show one message delivered
+        verify(drools).deliver(sink1, MY_EVENT);
+        verify(drools).deliver(sink2, MY_EVENT);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT))
+                            .thenReturn(flag),
+            (prov, flag) -> when(
+                            prov.afterDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT, true))
+                                            .thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.deliver(CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT);
+            },
+            prov -> verify(prov).beforeDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT),
+            () -> verify(drools).deliver(sink1, MY_EVENT),
+            prov -> verify(prov).afterDeliver(apc, CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT, true));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testDeliver_NullTopic() {
+        apc.start();
+        apc.deliver(CommInfrastructure.NOOP, null, MY_EVENT);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testDeliver_EmptyTopic() {
+        apc.start();
+        apc.deliver(CommInfrastructure.NOOP, "", MY_EVENT);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testDeliver_NullEvent() {
+        apc.start();
+        apc.deliver(CommInfrastructure.NOOP, SINK_TOPIC1, null);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testDeliver_NotStarted() {
+        // do NOT start
+        apc.deliver(CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testDeliver_Locked() {
+        apc.start();
+        apc.lock();
+        apc.deliver(CommInfrastructure.NOOP, SINK_TOPIC1, MY_EVENT);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testDeliver_UnknownTopic() {
+        apc.start();
+        apc.deliver(CommInfrastructure.NOOP, "unknown-topic", MY_EVENT);
+    }
+
+    @Test
+    public void testIsAlive() {
+        assertFalse(apc.isAlive());
+
+        apc.start();
+        assertTrue(apc.isAlive());
+
+        apc.stop();
+        assertFalse(apc.isAlive());
+    }
+
+    @Test
+    public void testLock() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeLock(any())).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterLock(any())).thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+
+        // now lock it
+        assertTrue(apc.lock());
+
+        verify(prov1).beforeLock(apc);
+        verify(prov2).beforeLock(apc);
+
+        assertTrue(apc.isLocked());
+
+        verify(drools).lock();
+
+        verify(prov1).afterLock(apc);
+        verify(prov2).afterLock(apc);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeLock(apc)).thenReturn(flag),
+            (prov, flag) -> when(prov.afterLock(apc)).thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.lock();
+            },
+            prov -> verify(prov).beforeLock(apc),
+            () -> verify(drools).lock(),
+            prov -> verify(prov).afterLock(apc));
+    }
+
+    @Test
+    public void testLock_AlreadyLocked() {
+        apc.start();
+        apc.lock();
+
+        // now re-lock it
+        assertTrue(apc.lock());
+
+        // these should be invoked a second time
+        verify(prov1, times(2)).beforeLock(apc);
+        verify(prov2, times(2)).beforeLock(apc);
+
+        assertTrue(apc.isLocked());
+
+        // these shouldn't be invoked a second time
+        verify(drools).lock();
+        verify(prov1).afterLock(apc);
+    }
+
+    @Test
+    public void testUnlock() {
+        // arrange for first provider to throw exceptions
+        when(prov1.beforeUnlock(any())).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterUnlock(any())).thenThrow(new RuntimeException(EXPECTED));
+
+        // start it
+        apc.start();
+        apc.lock();
+
+        // now unlock it
+        assertTrue(apc.unlock());
+
+        verify(prov1).beforeUnlock(apc);
+        verify(prov2).beforeUnlock(apc);
+
+        assertFalse(apc.isLocked());
+
+        verify(drools).unlock();
+
+        verify(prov1).afterUnlock(apc);
+        verify(prov2).afterUnlock(apc);
+
+        checkBeforeAfter(
+            (prov, flag) -> when(prov.beforeUnlock(apc)).thenReturn(flag),
+            (prov, flag) -> when(prov.afterUnlock(apc)).thenReturn(flag),
+            () -> {
+                apc.start();
+                apc.lock();
+                apc.unlock();
+            },
+            prov -> verify(prov).beforeUnlock(apc),
+            () -> verify(drools).unlock(),
+            prov -> verify(prov).afterUnlock(apc));
+    }
+
+    @Test
+    public void testUnlock_NotLocked() {
+        apc.start();
+
+        // now unlock it
+        assertTrue(apc.unlock());
+
+        verify(prov1).beforeUnlock(apc);
+        verify(prov2).beforeUnlock(apc);
+
+        assertFalse(apc.isLocked());
+
+        // these shouldn't be invoked
+        verify(drools, never()).unlock();
+        verify(prov1, never()).afterLock(apc);
+    }
+
+    @Test
+    public void testIsLocked() {
+        assertFalse(apc.isLocked());
+
+        apc.lock();
+        assertTrue(apc.isLocked());
+
+        apc.unlock();
+        assertFalse(apc.isLocked());
+    }
+
+    @Test
+    public void testGetTopicSources() {
+        assertEquals(sources, apc.getTopicSources());
+    }
+
+    @Test
+    public void testGetTopicSinks() {
+        assertEquals(sinks, apc.getTopicSinks());
+    }
+
+    @Test
+    public void testGetDrools() {
+        assertEquals(drools, apc.getDrools());
+    }
+
+    @Test
+    public void testGetProperties() {
+        assertEquals(properties, apc.getProperties());
+    }
+
+    @Test
+    public void testToString() {
+        assertTrue(apc.toString().startsWith("AggregatedPolicyController ["));
+    }
+
+    /**
+     * Performs an operation that has a beforeXxx method and an afterXxx method. Tries
+     * combinations where beforeXxx and afterXxx return {@code true} and {@code false}.
+     *
+     * @param setBefore function to set the return value of a provider's beforeXxx method
+     * @param setAfter function to set the return value of a provider's afterXxx method
+     * @param action invokes the operation
+     * @param verifyBefore verifies that a provider's beforeXxx method was invoked
+     * @param verifyMiddle verifies that the action occurring between the beforeXxx loop
+     *        and the afterXxx loop was invoked
+     * @param verifyAfter verifies that a provider's afterXxx method was invoked
+     */
+    private void checkBeforeAfter(BiConsumer<PolicyControllerFeatureAPI, Boolean> setBefore,
+                    BiConsumer<PolicyControllerFeatureAPI, Boolean> setAfter, Runnable action,
+                    Consumer<PolicyControllerFeatureAPI> verifyBefore, Runnable verifyMiddle,
+                    Consumer<PolicyControllerFeatureAPI> verifyAfter) {
+
+        checkBeforeAfter_FalseFalse(setBefore, setAfter, action, verifyBefore, verifyMiddle, verifyAfter);
+        checkBeforeAfter_FalseTrue(setBefore, setAfter, action, verifyBefore, verifyMiddle, verifyAfter);
+        checkBeforeAfter_TrueFalse(setBefore, setAfter, action, verifyBefore, verifyMiddle, verifyAfter);
+
+        // don't need to test true-true, as it's behavior is a subset of true-false
+    }
+
+    /**
+     * Performs an operation that has a beforeXxx method and an afterXxx method. Tries the
+     * case where both the beforeXxx and afterXxx methods return {@code false}.
+     *
+     * @param setBefore function to set the return value of a provider's beforeXxx method
+     * @param setAfter function to set the return value of a provider's afterXxx method
+     * @param action invokes the operation
+     * @param verifyBefore verifies that a provider's beforeXxx method was invoked
+     * @param verifyMiddle verifies that the action occurring between the beforeXxx loop
+     *        and the afterXxx loop was invoked
+     * @param verifyAfter verifies that a provider's afterXxx method was invoked
+     */
+    private void checkBeforeAfter_FalseFalse(BiConsumer<PolicyControllerFeatureAPI, Boolean> setBefore,
+                    BiConsumer<PolicyControllerFeatureAPI, Boolean> setAfter, Runnable action,
+                    Consumer<PolicyControllerFeatureAPI> verifyBefore, Runnable verifyMiddle,
+                    Consumer<PolicyControllerFeatureAPI> verifyAfter) {
+
+        setUp();
+
+        // configure for the test
+        setBefore.accept(prov1, false);
+        setBefore.accept(prov2, false);
+
+        setAfter.accept(prov1, false);
+        setAfter.accept(prov2, false);
+
+        // run the action
+        action.run();
+
+        // verify that various methods were invoked
+        verifyBefore.accept(prov1);
+        verifyBefore.accept(prov2);
+
+        verifyMiddle.run();
+
+        verifyAfter.accept(prov1);
+        verifyAfter.accept(prov2);
+    }
+
+    /**
+     * Performs an operation that has a beforeXxx method and an afterXxx method. Tries the
+     * case where the first provider's afterXxx returns {@code true}, while the others
+     * return {@code false}.
+     *
+     * @param setBefore function to set the return value of a provider's beforeXxx method
+     * @param setAfter function to set the return value of a provider's afterXxx method
+     * @param action invokes the operation
+     * @param verifyBefore verifies that a provider's beforeXxx method was invoked
+     * @param verifyMiddle verifies that the action occurring between the beforeXxx loop
+     *        and the afterXxx loop was invoked
+     * @param verifyAfter verifies that a provider's afterXxx method was invoked
+     */
+    private void checkBeforeAfter_FalseTrue(BiConsumer<PolicyControllerFeatureAPI, Boolean> setBefore,
+                    BiConsumer<PolicyControllerFeatureAPI, Boolean> setAfter, Runnable action,
+                    Consumer<PolicyControllerFeatureAPI> verifyBefore, Runnable verifyMiddle,
+                    Consumer<PolicyControllerFeatureAPI> verifyAfter) {
+
+        setUp();
+
+        // configure for the test
+        setBefore.accept(prov1, false);
+        setBefore.accept(prov2, false);
+
+        setAfter.accept(prov1, true);
+        setAfter.accept(prov2, false);
+
+        // run the action
+        action.run();
+
+        // verify that various methods were invoked
+        verifyBefore.accept(prov1);
+        verifyBefore.accept(prov2);
+
+        verifyMiddle.run();
+
+        verifyAfter.accept(prov1);
+        assertThrows(AssertionError.class, () -> verifyAfter.accept(prov2));
+    }
+
+    /**
+     * Performs an operation that has a beforeXxx method and an afterXxx method. Tries the
+     * case where the first provider's beforeXxx returns {@code true}, while the others
+     * return {@code false}.
+     *
+     * @param setBefore function to set the return value of a provider's beforeXxx method
+     * @param setAfter function to set the return value of a provider's afterXxx method
+     * @param action invokes the operation
+     * @param verifyBefore verifies that a provider's beforeXxx method was invoked
+     * @param verifyMiddle verifies that the action occurring between the beforeXxx loop
+     *        and the afterXxx loop was invoked
+     * @param verifyAfter verifies that a provider's afterXxx method was invoked
+     */
+    private void checkBeforeAfter_TrueFalse(BiConsumer<PolicyControllerFeatureAPI, Boolean> setBefore,
+                    BiConsumer<PolicyControllerFeatureAPI, Boolean> setAfter, Runnable action,
+                    Consumer<PolicyControllerFeatureAPI> verifyBefore, Runnable verifyMiddle,
+                    Consumer<PolicyControllerFeatureAPI> verifyAfter) {
+
+        setUp();
+
+        // configure for the test
+        setBefore.accept(prov1, true);
+        setBefore.accept(prov2, false);
+
+        setAfter.accept(prov1, false);
+        setAfter.accept(prov2, false);
+
+        // run the action
+        action.run();
+
+        // verify that various methods were invoked
+        verifyBefore.accept(prov1);
+
+        // remaining methods should not have been invoked
+        assertThrows(AssertionError.class, () -> verifyBefore.accept(prov2));
+
+        assertThrows(AssertionError.class, () -> verifyMiddle.run());
+
+        assertThrows(AssertionError.class, () -> verifyAfter.accept(prov1));
+        assertThrows(AssertionError.class, () -> verifyAfter.accept(prov2));
+    }
+}