Split Engine start method into "start" and "open"

"open" meaning to open external configuration interfaces
to external provisioning systems.

Issue-ID: POLICY-2055
Signed-off-by: jhh <jorge.hernandez-herrero@att.com>
Change-Id: Ic984f0ebccd088503b6b13620c3b80ed8e640899
Signed-off-by: jhh <jorge.hernandez-herrero@att.com>

5 files changed, 174 insertions, 61 deletions

diff --git a/policy-management/src/main/java/org/onap/policy/drools/features/PolicyEngineFeatureApi.java b/policy-management/src/main/java/org/onap/policy/drools/features/PolicyEngineFeatureApi.java
index b6d827a4..fe31eb50 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/features/PolicyEngineFeatureApi.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/features/PolicyEngineFeatureApi.java
@@ -1,6 +1,6 @@
 /*
  * ============LICENSE_START=======================================================
- * policy-engine
+ * ONAP
  * ================================================================================
  * Copyright (C) 2017-2019 AT&T Intellectual Property. All rights reserved.
  * ================================================================================
@@ -33,200 +33,200 @@ import org.onap.policy.drools.system.PolicyEngine;
 public interface PolicyEngineFeatureApi extends OrderedService {
 
     /**
-     * intercept before the Policy Engine is commanded to boot.
+     * Intercept before the Policy Engine is commanded to boot.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeBoot(PolicyEngine engine, String[] cliArgs) {
+    default boolean beforeBoot(PolicyEngine engine, String[] cliArgs) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine is booted.
+     * Intercept after the Policy Engine is booted.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterBoot(PolicyEngine engine) {
+    default boolean afterBoot(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine is configured.
+     * Intercept before the Policy Engine is configured.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeConfigure(PolicyEngine engine, Properties properties) {
+    default boolean beforeConfigure(PolicyEngine engine, Properties properties) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine is configured.
+     * Intercept after the Policy Engine is configured.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterConfigure(PolicyEngine engine) {
+    default boolean afterConfigure(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine goes active.
+     * Intercept before the Policy Engine goes active.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeActivate(PolicyEngine engine) {
+    default boolean beforeActivate(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine goes active.
+     * Intercept after the Policy Engine goes active.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterActivate(PolicyEngine engine) {
+    default boolean afterActivate(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine goes standby.
+     * Intercept before the Policy Engine goes standby.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeDeactivate(PolicyEngine engine) {
+    default boolean beforeDeactivate(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine goes standby.
+     * Intercept after the Policy Engine goes standby.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterDeactivate(PolicyEngine engine) {
+    default boolean afterDeactivate(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine is started.
+     * Intercept before the Policy Engine is started.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeStart(PolicyEngine engine) {
+    default boolean beforeStart(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine is started.
+     * Intercept after the Policy Engine is started.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterStart(PolicyEngine engine) {
+    default boolean afterStart(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine is stopped.
+     * Intercept before the Policy Engine is stopped.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise..
      */
-    public default boolean beforeStop(PolicyEngine engine) {
+    default boolean beforeStop(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine is stopped.
+     * Intercept after the Policy Engine is stopped.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.d.
      */
-    public default boolean afterStop(PolicyEngine engine) {
+    default boolean afterStop(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine is locked.
+     * Intercept before the Policy Engine is locked.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeLock(PolicyEngine engine) {
+    default boolean beforeLock(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine is locked.
+     * Intercept after the Policy Engine is locked.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise..
      */
-    public default boolean afterLock(PolicyEngine engine) {
+    default boolean afterLock(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept before the Policy Engine is locked.
+     * Intercept before the Policy Engine is locked.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean beforeUnlock(PolicyEngine engine) {
+    default boolean beforeUnlock(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept after the Policy Engine is locked.
+     * Intercept after the Policy Engine is locked.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterUnlock(PolicyEngine engine) {
+    default boolean afterUnlock(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * intercept the Policy Engine is shut down.
+     * Intercept the Policy Engine is shut down.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise..
      */
-    public default boolean beforeShutdown(PolicyEngine engine) {
+    default boolean beforeShutdown(PolicyEngine engine) {
         return false;
     }
 
     /**
-     * called after the Policy Engine is shut down.
+     * Called after the Policy Engine is shut down.
      *
      * @return true if this feature intercepts and takes ownership
      *     of the operation preventing the invocation of
      *     lower priority features.   False, otherwise.
      */
-    public default boolean afterShutdown(PolicyEngine engine) {
+    default boolean afterShutdown(PolicyEngine engine) {
         return false;
     }
 
@@ -236,7 +236,7 @@ public interface PolicyEngineFeatureApi extends OrderedService {
      * @return True if this feature intercepts and takes ownership of the operation
      *         preventing the invocation of lower priority features. False, otherwise.
      */
-    public default boolean beforeOnTopicEvent(PolicyEngine engine, CommInfrastructure commType, String topic,
+    default boolean beforeOnTopicEvent(PolicyEngine engine, CommInfrastructure commType, String topic,
                     String event) {
         return false;
     }
@@ -247,8 +247,28 @@ public interface PolicyEngineFeatureApi extends OrderedService {
      * @return True if this feature intercepts and takes ownership of the operation
      *         preventing the invocation of lower priority features. False, otherwise
      */
-    public default boolean afterOnTopicEvent(PolicyEngine engine, PdpdConfiguration configuration,
+    default boolean afterOnTopicEvent(PolicyEngine engine, PdpdConfiguration configuration,
                     CommInfrastructure commType, String topic, String event) {
         return false;
     }
+
+    /**
+     * Called before the PolicyEngine opens its external configuration interfaces.
+     *
+     * @return True if this feature intercepts and takes ownership of the operation
+     *         preventing the invocation of lower priority features. False, otherwise
+     */
+    default boolean beforeOpen(PolicyEngine engine) {
+        return false;
+    }
+
+    /**
+     * Called after the PolicyEngine opens its external configuration interfaces.
+     *
+     * @return True if this feature intercepts and takes ownership of the operation
+     *         preventing the invocation of lower priority features. False, otherwise
+     */
+    default boolean afterOpen(PolicyEngine engine) {
+        return false;
+    }
 }
diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/Main.java b/policy-management/src/main/java/org/onap/policy/drools/system/Main.java
index 1ef578c1..0e7b44f2 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/system/Main.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/Main.java
@@ -175,5 +175,7 @@ public class Main {
                         e);
             }
         }
+
+        PolicyEngineConstants.getManager().open();
     }
 }
diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngine.java b/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngine.java
index 506474cb..653ff72e 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngine.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngine.java
@@ -73,6 +73,14 @@ public interface PolicyEngine extends Startable, Lockable, TopicListener {
     boolean configure(PdpdConfiguration configuration);
 
     /**
+     * open the Policy Engine to external configuration systems.
+     *
+     * @return success or failure
+     */
+    boolean open();
+
+
+    /**
      * configure the engine's environment. General lab installation configuration is made available
      * to the Engine. Typically, custom lab installation that may be needed by arbitrary drools
      * applications are made available, for example network component and database host addresses.
diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngineManager.java b/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngineManager.java
index 32e3f674..766848c6 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngineManager.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/system/PolicyEngineManager.java
@@ -506,20 +506,6 @@ class PolicyEngineManager implements PolicyEngine {
             (item, ex) -> logger.error("{}: cannot start http-server {} because of {}",
                             this, item, ex.getMessage(), ex));
 
-        /* Start Policy Engine exclusively-owned (unmanaged) sources */
-
-        attempt(success, this.sources,
-            TopicSource::start,
-            (item, ex) -> logger.error("{}: cannot start topic-source {} because of {}",
-                            this, item, ex.getMessage(), ex));
-
-        /* Start Policy Engine owned (unmanaged) sinks */
-
-        attempt(success, this.sinks,
-            TopicSink::start,
-            (item, ex) -> logger.error("{}: cannot start topic-sink {} because of {}",
-                            this, item, ex.getMessage(), ex));
-
         /* Start Policy Controllers */
 
         attempt(success, getControllerFactory().inventory(),
@@ -554,6 +540,48 @@ class PolicyEngineManager implements PolicyEngine {
         return success.get();
     }
 
+    @Override
+    public synchronized boolean open() {
+
+        /* pre-open hook */
+        if (FeatureApiUtils.apply(getEngineProviders(),
+            feature -> feature.beforeOpen(this),
+            (feature, ex) -> logger.error("{}: feature {} before-open failure because of {}", this,
+                feature.getClass().getName(), ex.getMessage(), ex))) {
+            return true;
+        }
+
+        if (this.locked) {
+            throw new IllegalStateException(ENGINE_LOCKED_MSG);
+        }
+
+        if (!this.alive) {
+            throw new IllegalStateException(ENGINE_STOPPED_MSG);
+        }
+
+        AtomicReference<Boolean> success = new AtomicReference<>(true);
+
+        /* Open the unmanaged topics to external components for configuration purposes */
+
+        attempt(success, this.sources,
+            TopicSource::start,
+            (item, ex) -> logger.error("{}: cannot start topic-source {} because of {}",
+                this, item, ex.getMessage(), ex));
+
+        attempt(success, this.sinks,
+            TopicSink::start,
+            (item, ex) -> logger.error("{}: cannot start topic-sink {} because of {}",
+                this, item, ex.getMessage(), ex));
+
+        /* post-open hook */
+        FeatureApiUtils.apply(getEngineProviders(),
+            feature -> feature.afterOpen(this),
+            (feature, ex) -> logger.error("{}: feature {} after-open failure because of {}", this,
+                feature.getClass().getName(), ex.getMessage(), ex));
+
+        return success.get();
+    }
+
     @FunctionalInterface
     private static interface PredicateWithEx<T> {
         public boolean test(T value) throws InterruptedException;
diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java
index e9f0b48a..5e0ead9d 100644
--- a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java
+++ b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java
@@ -670,11 +670,11 @@ public class PolicyEngineManagerTest {
         // servlet wait fails - still does everything
         testStart(false, () -> when(server1.waitedStart(anyLong())).thenReturn(false));
 
-        // topic source fails to start - still does everything
-        testStart(false, () -> when(source1.start()).thenReturn(false));
+        // topic source is not started with start
+        testStart(true, () -> when(source1.start()).thenReturn(false));
 
-        // topic sink fails to start - still does everything
-        testStart(false, () -> when(sink1.start()).thenReturn(false));
+        // topic sink is not started with start
+        testStart(true, () -> when(sink1.start()).thenReturn(false));
 
         // controller fails to start - still does everything
         testStart(false, () -> when(controller.start()).thenReturn(false));
@@ -735,11 +735,11 @@ public class PolicyEngineManagerTest {
         verify(server1).waitedStart(anyLong());
         verify(server2).waitedStart(anyLong());
 
-        verify(source1).start();
-        verify(source2).start();
+        verify(source1, never()).start();
+        verify(source2, never()).start();
 
-        verify(sink1).start();
-        verify(sink2).start();
+        verify(sink1, never()).start();
+        verify(sink2, never()).start();
 
         verify(controller).start();
         verify(controller2).start();
@@ -1484,6 +1484,61 @@ public class PolicyEngineManagerTest {
     }
 
     @Test
+    public void testOpen() throws Throwable {
+        when(prov1.beforeOpen(mgr)).thenThrow(new RuntimeException(EXPECTED));
+        when(prov1.afterOpen(mgr)).thenThrow(new RuntimeException(EXPECTED));
+
+        assertTrue(mgr.lock());
+        assertThatIllegalStateException().isThrownBy(() -> mgr.open());
+        unsuccessfulOpen();
+
+        assertTrue(mgr.unlock());
+        unsuccessfulOpen();
+
+        setUp();
+        mgr.configure(properties);
+        assertTrue(mgr.start());
+
+        verify(source1, never()).start();
+        verify(source2, never()).start();
+
+        assertTrue(mgr.open());
+
+        verify(prov1).beforeOpen(mgr);
+        verify(prov2).beforeOpen(mgr);
+
+        verify(source1).start();
+        verify(source2).start();
+
+        verify(prov1).afterOpen(mgr);
+        verify(prov2).afterOpen(mgr);
+
+        when(source1.start()).thenReturn(false);
+        assertFalse(mgr.open());
+        when(source1.start()).thenReturn(true);
+
+        when(sink1.start()).thenReturn(false);
+        assertFalse(mgr.open());
+        when(sink1.start()).thenReturn(true);
+
+        assertTrue(mgr.open());
+    }
+
+    private void unsuccessfulOpen() {
+        verify(prov1).beforeOpen(mgr);
+        verify(prov2).beforeOpen(mgr);
+
+        verify(prov1, never()).afterOpen(mgr);
+        verify(prov2, never()).afterOpen(mgr);
+
+        verify(source1, never()).start();
+        verify(source2, never()).start();
+
+        verify(sink1, never()).start();
+        verify(sink2, never()).start();
+    }
+
+    @Test
     public void testControllerConfig() throws Exception {
         mgr.configure(properties);
         assertTrue(mgr.configure(pdpConfig));