Unit test for Apex engine

Coverage for unit test of engie state and state machine handling.
Added coverage of the facade and executor context classes
Added coverage for executors
Added coverage for the remainder of the core engine

Issue-ID: POLICY-1034
Change-Id: I85c66005dfdffdf2b4ee5672473a3ae4823d0d9c
Signed-off-by: liamfallon <liam.fallon@ericsson.com>

9 files changed, 98 insertions, 62 deletions

diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/context/ApexInternalContext.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/context/ApexInternalContext.java
index f73281ada..8eb92a0f8 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/context/ApexInternalContext.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/context/ApexInternalContext.java
@@ -51,7 +51,7 @@ import org.onap.policy.apex.model.utilities.comparison.KeyedMapDifference;
  *
  * @author Liam Fallon
  */
-public final class ApexInternalContext implements AxConceptGetter<ContextAlbum> {
+public class ApexInternalContext implements AxConceptGetter<ContextAlbum> {
     // The key of the currently running Apex model
     private AxArtifactKey key;
 
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/engine/impl/ApexEngineImpl.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/engine/impl/ApexEngineImpl.java
index efd6aec5e..b703506ea 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/engine/impl/ApexEngineImpl.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/engine/impl/ApexEngineImpl.java
@@ -33,7 +33,9 @@ import org.onap.policy.apex.core.engine.engine.ApexEngine;
 import org.onap.policy.apex.core.engine.engine.EnEventListener;
 import org.onap.policy.apex.core.engine.event.EnEvent;
 import org.onap.policy.apex.core.engine.executor.exception.StateMachineException;
+import org.onap.policy.apex.core.infrastructure.threading.ThreadUtilities;
 import org.onap.policy.apex.model.basicmodel.concepts.ApexException;
+import org.onap.policy.apex.model.basicmodel.concepts.ApexRuntimeException;
 import org.onap.policy.apex.model.basicmodel.concepts.AxArtifactKey;
 import org.onap.policy.apex.model.basicmodel.concepts.AxReferenceKey;
 import org.onap.policy.apex.model.enginemodel.concepts.AxEngineModel;
@@ -201,30 +203,37 @@ public class ApexEngineImpl implements ApexEngine {
     public void stop() throws ApexException {
         LOGGER.entry("stop()->" + key);
 
+        // Check if the engine is already stopped
+        if (state == AxEngineState.STOPPED) {
+            throw new ApexException(
+                            STOP + key.getId() + "," + state + ", cannot stop engine, engine is already stopped");
+        }
+
         // Stop the engine if it is in state READY, if it is in state EXECUTING, wait for execution to finish
         for (int increment = ApexEngineConstants.STOP_EXECUTION_WAIT_TIMEOUT;
-                        increment > 0;
-                        increment = ApexEngineConstants.STOP_EXECUTION_WAIT_TIMEOUT) {
+                        increment > 0; increment -= ApexEngineConstants.APEX_ENGINE_STOP_EXECUTION_WAIT_INCREMENT) {
+            ThreadUtilities.sleep(ApexEngineConstants.APEX_ENGINE_STOP_EXECUTION_WAIT_INCREMENT);
+            
             synchronized (state) {
                 switch (state) {
-                    // Already stopped
-                    case STOPPED:
-
-                        throw new ApexException(STOP + key.getId() + "," + state
-                                        + ", cannot stop engine, engine is already stopped");
-                        // The normal case, the engine wasn't doing anything or it was executing
+                    // Engine is OK to stop or has been stopped on return of an event
                     case READY:
-                    case STOPPING:
-
+                    case STOPPED:
                         state = AxEngineState.STOPPED;
                         stateMachineHandler.stop();
                         engineStats.engineStop();
                         LOGGER.exit("stop()" + key);
                         return;
+
                     // Engine is executing a policy, wait for it to stop
                     case EXECUTING:
                         state = AxEngineState.STOPPING;
                         break;
+
+                    // Wait for the engine to stop
+                    case STOPPING:
+                        break;
+
                     default:
                         throw new ApexException(STOP + key.getId() + "," + state
                                         + ", cannot stop engine, engine is in an undefined state");
@@ -232,7 +241,12 @@ public class ApexEngineImpl implements ApexEngine {
             }
         }
 
-        throw new ApexException(STOP + key.getId() + "," + state + ", cannot stop engine, engine stop timed out");
+        // Force the engine to STOPPED state
+        synchronized (state) {
+            state = AxEngineState.STOPPED;
+        }
+        
+        throw new ApexException(STOP + key.getId() + "," + state + ", error stopping engine, engine stop timed out");
     }
 
     /*
@@ -336,9 +350,11 @@ public class ApexEngineImpl implements ApexEngine {
             ret = false;
         }
         synchronized (state) {
-            // Only go to READY if we are still in state EXECUTING, we could be in state STOPPING
+            // Only go to READY if we are still in state EXECUTING, we go to state STOPPED if we were STOPPING
             if (state == AxEngineState.EXECUTING) {
                 state = AxEngineState.READY;
+            } else if (state == AxEngineState.STOPPING) {
+                state = AxEngineState.STOPPED;
             }
         }
         return ret;
@@ -352,6 +368,18 @@ public class ApexEngineImpl implements ApexEngine {
      */
     @Override
     public void addEventListener(final String listenerName, final EnEventListener listener) {
+        if (listenerName == null) {
+            String message = "addEventListener()<-" + key.getId() + "," + state + ", listenerName is null";
+            LOGGER.warn(message);
+            throw new ApexRuntimeException(message);
+        }
+
+        if (listener == null) {
+            String message = "addEventListener()<-" + key.getId() + "," + state + ", listener is null";
+            LOGGER.warn(message);
+            throw new ApexRuntimeException(message);
+        }
+
         eventListeners.put(listenerName, listener);
     }
 
@@ -362,6 +390,12 @@ public class ApexEngineImpl implements ApexEngine {
      */
     @Override
     public void removeEventListener(final String listenerName) {
+        if (listenerName == null) {
+            String message = "removeEventListener()<-" + key.getId() + "," + state + ", listenerName is null";
+            LOGGER.warn(message);
+            throw new ApexRuntimeException(message);
+        }
+
         eventListeners.remove(listenerName);
     }
 
@@ -411,7 +445,7 @@ public class ApexEngineImpl implements ApexEngine {
         if (internalContext == null) {
             return currentContext;
         }
-        
+
         for (final Entry<AxArtifactKey, ContextAlbum> contextAlbumEntry : internalContext.getContextAlbums()
                         .entrySet()) {
             currentContext.put(contextAlbumEntry.getKey(), contextAlbumEntry.getValue());
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/ExecutorFactory.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/ExecutorFactory.java
index f092ce716..fb6c7b45e 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/ExecutorFactory.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/ExecutorFactory.java
@@ -32,7 +32,7 @@ import org.onap.policy.apex.model.policymodel.concepts.AxTask;
  * @author Liam Fallon
  */
 
-public abstract class ExecutorFactory {
+public interface ExecutorFactory {
     /**
      * Get an executor for task selection logic.
      *
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutor.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutor.java
index 1c225f7b8..c30bda1b9 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutor.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutor.java
@@ -101,7 +101,8 @@ public abstract class StateFinalizerExecutor
     public void prepare() throws StateMachineException {
         LOGGER.debug("prepare:" + finalizerLogic.getId() + "," + finalizerLogic.getLogicFlavour() + ","
                         + finalizerLogic.getLogic());
-        argumentOfClassNotNull(finalizerLogic.getLogic(), StateMachineException.class, "task logic cannot be null.");
+        argumentOfClassNotNull(finalizerLogic.getLogic(), StateMachineException.class,
+                        "state finalizer logic cannot be null.");
     }
 
     /*
@@ -237,7 +238,11 @@ public abstract class StateFinalizerExecutor
      */
     @Override
     public String getOutgoing() {
-        return executionContext.getSelectedStateOutputName();
+        if (executionContext != null) {
+            return executionContext.getSelectedStateOutputName();
+        } else {
+            return null;
+        }
     }
 
     /*
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutor.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutor.java
index 97d51bf78..34ba3942c 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutor.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutor.java
@@ -146,11 +146,8 @@ public class StateMachineExecutor implements Executor<EnEvent, EnEvent, AxPolicy
         StateOutput stateOutput = new StateOutput(new AxStateOutput(firstExecutor.getSubject().getKey(),
                 incomingEvent.getKey(), firstExecutor.getSubject().getKey()), incomingEvent);
         while (true) {
-            // Execute the state
+            // Execute the state, it returns an output or throws an exception
             stateOutput = stateExecutor.execute(executionId, stateOutput.getOutputEvent());
-            if (stateOutput == null) {
-                throw new StateMachineException("state execution failed, invalid state output returned");
-            }
 
             // Use the next state of the state output to find if all the states have executed
             if (stateOutput.getNextState().equals(AxReferenceKey.getNullKey())) {
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/TaskExecutor.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/TaskExecutor.java
index 2a62f3ae2..c55d4924d 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/TaskExecutor.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/TaskExecutor.java
@@ -41,14 +41,14 @@ import org.slf4j.ext.XLogger;
 import org.slf4j.ext.XLoggerFactory;
 
 /**
- * This abstract class executes a task in a state of an Apex policy and is specialized by classes
- * that implement execution of task logic.
+ * This abstract class executes a task in a state of an Apex policy and is specialized by classes that implement
+ * execution of task logic.
  *
  * @author Sven van der Meer (sven.van.der.meer@ericsson.com)
  * @author Liam Fallon (liam.fallon@ericsson.com)
  */
 public abstract class TaskExecutor
-        implements Executor<Map<String, Object>, Map<String, Object>, AxTask, ApexInternalContext> {
+                implements Executor<Map<String, Object>, Map<String, Object>, AxTask, ApexInternalContext> {
     // Logger for this class
     private static final XLogger LOGGER = XLoggerFactory.getXLogger(TaskExecutor.class);
 
@@ -86,7 +86,7 @@ public abstract class TaskExecutor
      */
     @Override
     public void setContext(final Executor<?, ?, ?, ?> newParent, final AxTask newAxTask,
-            final ApexInternalContext newInternalContext) {
+                    final ApexInternalContext newInternalContext) {
         this.parent = newParent;
         this.axTask = newAxTask;
         this.internalContext = newInternalContext;
@@ -100,7 +100,7 @@ public abstract class TaskExecutor
     @Override
     public void prepare() throws StateMachineException {
         LOGGER.debug("prepare:" + axTask.getKey().getId() + "," + axTask.getTaskLogic().getLogicFlavour() + ","
-                + axTask.getTaskLogic().getLogic());
+                        + axTask.getTaskLogic().getLogic());
         argumentOfClassNotNull(axTask.getTaskLogic().getLogic(), StateMachineException.class,
                         "task logic cannot be null.");
     }
@@ -108,43 +108,43 @@ public abstract class TaskExecutor
     /*
      * (non-Javadoc)
      *
-     * @see org.onap.policy.apex.core.engine.executor.Executor#execute(java.lang.long,
-     * java.lang.Object)
+     * @see org.onap.policy.apex.core.engine.executor.Executor#execute(java.lang.long, java.lang.Object)
      */
     @Override
     public Map<String, Object> execute(final long executionId, final Map<String, Object> newIncomingFields)
-            throws StateMachineException, ContextException {
+                    throws StateMachineException, ContextException {
         throw new StateMachineException(
-                "execute() not implemented on abstract TaskExecutor class, only on its subclasses");
+                        "execute() not implemented on abstract TaskExecutor class, only on its subclasses");
     }
 
     /*
      * (non-Javadoc)
      *
-     * @see org.onap.policy.apex.core.engine.executor.Executor#executePre(java.lang.long,
-     * java.lang.Object)
+     * @see org.onap.policy.apex.core.engine.executor.Executor#executePre(java.lang.long, java.lang.Object)
      */
     @Override
     public final void executePre(final long executionId, final Map<String, Object> newIncomingFields)
-            throws StateMachineException, ContextException {
+                    throws StateMachineException, ContextException {
         LOGGER.debug("execute-pre:" + getSubject().getTaskLogic().getLogicFlavour() + ","
-                + getSubject().getKey().getId() + "," + getSubject().getTaskLogic().getLogic());
+                        + getSubject().getKey().getId() + "," + getSubject().getTaskLogic().getLogic());
 
         // Check that the incoming event has all the input fields for this state
         final Set<String> missingTaskInputFields = new TreeSet<>(axTask.getInputFields().keySet());
         missingTaskInputFields.removeAll(newIncomingFields.keySet());
 
         // Remove fields from the set that are optional
+        final Set<String> optionalFields = new TreeSet<>();
         for (final Iterator<String> missingFieldIterator = missingTaskInputFields.iterator(); missingFieldIterator
-                .hasNext();) {
+                        .hasNext();) {
             final String missingField = missingFieldIterator.next();
             if (axTask.getInputFields().get(missingField).getOptional()) {
-                missingTaskInputFields.remove(missingField);
+                optionalFields.add(missingField);
             }
         }
+        missingTaskInputFields.removeAll(optionalFields);
         if (!missingTaskInputFields.isEmpty()) {
             throw new StateMachineException("task input fields \"" + missingTaskInputFields
-                    + "\" are missing for task \"" + axTask.getKey().getId() + "\"");
+                            + "\" are missing for task \"" + axTask.getKey().getId() + "\"");
         }
 
         // Record the incoming fields
@@ -157,8 +157,8 @@ public abstract class TaskExecutor
         }
 
         // Get task context object
-        executionContext =
-                new TaskExecutionContext(this, executionId, getSubject(), getIncoming(), getOutgoing(), getContext());
+        executionContext = new TaskExecutionContext(this, executionId, getSubject(), getIncoming(), getOutgoing(),
+                        getContext());
     }
 
     /*
@@ -170,7 +170,7 @@ public abstract class TaskExecutor
     public final void executePost(final boolean returnValue) throws StateMachineException, ContextException {
         if (!returnValue) {
             String errorMessage = "execute-post: task logic execution failure on task \"" + axTask.getKey().getName()
-                    + "\" in model " + internalContext.getKey().getId();
+                            + "\" in model " + internalContext.getKey().getId();
             if (executionContext.getMessage() != null) {
                 errorMessage += ", user message: " + executionContext.getMessage();
             }
@@ -191,16 +191,19 @@ public abstract class TaskExecutor
         missingTaskOutputFields.removeAll(outgoingFields.keySet());
 
         // Remove fields from the set that are optional
+        final Set<String> optionalOrCopiedFields = new TreeSet<>();
         for (final Iterator<String> missingFieldIterator = missingTaskOutputFields.iterator(); missingFieldIterator
-                .hasNext();) {
+                        .hasNext();) {
             final String missingField = missingFieldIterator.next();
-            if (axTask.getInputFields().get(missingField).getOptional()) {
-                missingTaskOutputFields.remove(missingField);
+            if (axTask.getInputFields().containsKey(missingField)
+                            || axTask.getOutputFields().get(missingField).getOptional()) {
+                optionalOrCopiedFields.add(missingField);
             }
         }
+        missingTaskOutputFields.removeAll(optionalOrCopiedFields);
         if (!missingTaskOutputFields.isEmpty()) {
             throw new StateMachineException("task output fields \"" + missingTaskOutputFields
-                    + "\" are missing for task \"" + axTask.getKey().getId() + "\"");
+                            + "\" are missing for task \"" + axTask.getKey().getId() + "\"");
         }
 
         // Finally, check that the outgoing field map don't have any extra fields, if present, raise
@@ -210,7 +213,7 @@ public abstract class TaskExecutor
         extraTaskOutputFields.removeAll(axTask.getOutputFields().keySet());
         if (!extraTaskOutputFields.isEmpty()) {
             throw new StateMachineException("task output fields \"" + extraTaskOutputFields
-                    + "\" are unwanted for task \"" + axTask.getKey().getId() + "\"");
+                            + "\" are unwanted for task \"" + axTask.getKey().getId() + "\"");
         }
 
         String message = "execute-post:" + axTask.getKey().getId() + ", returning fields " + outgoingFields.toString();
@@ -218,14 +221,13 @@ public abstract class TaskExecutor
     }
 
     /**
-     * If the input field exists on the output and it is not set in the task, then it should
-     * be copied to the output.
+     * If the input field exists on the output and it is not set in the task, then it should be copied to the output.
      * 
-     * @param field the input field 
+     * @param field the input field
      */
     private void copyInputField2Output(String field) {
         // Check if the field exists and is not set on the output
-        if (!getOutgoing().containsKey(field) || getOutgoing().get(field) != null) {
+        if (getOutgoing().containsKey(field) && getOutgoing().get(field) != null) {
             return;
         }
 
@@ -316,14 +318,13 @@ public abstract class TaskExecutor
     /*
      * (non-Javadoc)
      *
-     * @see
-     * org.onap.policy.apex.core.engine.executor.Executor#setNext(org.onap.policy.apex.core.engine.
+     * @see org.onap.policy.apex.core.engine.executor.Executor#setNext(org.onap.policy.apex.core.engine.
      * executor.Executor)
      */
     @Override
     public void setNext(
-            final Executor<Map<String, Object>, Map<String, Object>, AxTask, ApexInternalContext> newNextExecutor) {
-        this.nextExecutor = newNextExecutor;
+                    final Executor<Map<String, Object>, Map<String, Object>, AxTask, ApexInternalContext> nextEx) {
+        this.nextExecutor = nextEx;
     }
 
     /*
@@ -339,10 +340,10 @@ public abstract class TaskExecutor
     /*
      * (non-Javadoc)
      *
-     * @see
-     * org.onap.policy.apex.core.engine.executor.Executor#setParameters(org.onap.policy.apex.core.
-     * engine. ExecutorParameters)
+     * @see org.onap.policy.apex.core.engine.executor.Executor#setParameters(org.onap.policy.apex.core. engine.
+     * ExecutorParameters)
      */
     @Override
-    public void setParameters(final ExecutorParameters parameters) {}
+    public void setParameters(final ExecutorParameters parameters) {
+    }
 }
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/AxTaskFacade.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/AxTaskFacade.java
index f7b53554d..f3906213f 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/AxTaskFacade.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/AxTaskFacade.java
@@ -20,7 +20,6 @@
 
 package org.onap.policy.apex.core.engine.executor.context;
 
-import org.onap.policy.apex.context.ContextRuntimeException;
 import org.onap.policy.apex.context.SchemaHelper;
 import org.onap.policy.apex.context.impl.schema.SchemaHelperFactory;
 import org.onap.policy.apex.core.engine.event.EnException;
@@ -120,7 +119,7 @@ public class AxTaskFacade {
         // Get a schema helper to handle translations of fields to and from the schema
         try {
             return new SchemaHelperFactory().createSchemaHelper(field.getKey(), field.getSchema());
-        } catch (final ContextRuntimeException e) {
+        } catch (final Exception e) {
             final String message = "schema helper cannot be created for task field \"" + fieldName + "\" with key \""
                     + field.getId() + "\" with schema \"" + field.getSchema() + "\"";
             LOGGER.warn(message, e);
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/TaskSelectionExecutionContext.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/TaskSelectionExecutionContext.java
index 63052348a..312c2a058 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/TaskSelectionExecutionContext.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/context/TaskSelectionExecutionContext.java
@@ -76,7 +76,7 @@ public class TaskSelectionExecutionContext {
      * sets this field in its logic prior to executing and the Apex engine executes this task as the
      * task for this state.
      */
-    public final AxArtifactKey selectedTask;
+    public AxArtifactKey selectedTask;
 
     /**
      * Logger for task selection execution, task selection logic can use this field to access and
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/impl/ExecutorFactoryImpl.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/impl/ExecutorFactoryImpl.java
index d1f07e19a..c0e24dd5a 100644
--- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/impl/ExecutorFactoryImpl.java
+++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/impl/ExecutorFactoryImpl.java
@@ -49,7 +49,7 @@ import org.slf4j.ext.XLoggerFactory;
  *
  * @author Liam Fallon (liam.fallon@ericsson.com)
  */
-public class ExecutorFactoryImpl extends ExecutorFactory {
+public class ExecutorFactoryImpl implements ExecutorFactory {
     // Get a reference to the logger
     private static final XLogger LOGGER = XLoggerFactory.getXLogger(ExecutorFactoryImpl.class);
 
@@ -220,7 +220,7 @@ public class ExecutorFactoryImpl extends ExecutorFactory {
             throw new StateMachineRuntimeException(errorMessage, e);
         }
 
-        // Check the class is a Task Selection Executor
+        // Check the class is the correct type of executor
         if (!(executorSuperClass.isAssignableFrom(executorObject.getClass()))) {
             final String errorMessage = "Executor on \"" + logicFlavour + "\" of type \"" + executorClass
                             + "\" is not an instance of \"" + executorSuperClass.getCanonicalName() + "\"";