From 7a5e85e17d58e4d75812a3015adc7cbc06e4e49c Mon Sep 17 00:00:00 2001 From: liamfallon Date: Sat, 6 Oct 2018 13:33:26 +0100 Subject: 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 --- .../core/engine/context/ApexInternalContext.java | 2 +- .../core/engine/engine/impl/ApexEngineImpl.java | 60 ++++++++++++++---- .../apex/core/engine/executor/ExecutorFactory.java | 2 +- .../engine/executor/StateFinalizerExecutor.java | 9 ++- .../core/engine/executor/StateMachineExecutor.java | 5 +- .../apex/core/engine/executor/TaskExecutor.java | 73 +++++++++++----------- .../core/engine/executor/context/AxTaskFacade.java | 3 +- .../context/TaskSelectionExecutionContext.java | 2 +- .../engine/executor/impl/ExecutorFactoryImpl.java | 4 +- 9 files changed, 98 insertions(+), 62 deletions(-) (limited to 'core/core-engine/src/main/java/org/onap') 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 { +public class ApexInternalContext implements AxConceptGetter { // 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 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, Map, AxTask, ApexInternalContext> { + implements Executor, Map, 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 execute(final long executionId, final Map 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 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 missingTaskInputFields = new TreeSet<>(axTask.getInputFields().keySet()); missingTaskInputFields.removeAll(newIncomingFields.keySet()); // Remove fields from the set that are optional + final Set optionalFields = new TreeSet<>(); for (final Iterator 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 optionalOrCopiedFields = new TreeSet<>(); for (final Iterator 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, AxTask, ApexInternalContext> newNextExecutor) { - this.nextExecutor = newNextExecutor; + final Executor, Map, 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() + "\""; -- cgit 1.2.3-korg