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>

2 files changed, 69 insertions, 41 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();
+        }
+    }
 }