diff options
author | liamfallon <liam.fallon@est.tech> | 2020-03-31 11:29:20 +0100 |
---|---|---|
committer | liamfallon <liam.fallon@est.tech> | 2020-03-31 12:27:57 +0100 |
commit | 5bcf94e7e286ea9d84ebcdef65f8318f4a72111a (patch) | |
tree | f29cd65b1b5d24065f71cae6723c82d431247a59 /core/core-engine | |
parent | b1b9ef7f981d44a8bfbe4b4ee469de0a01708acd (diff) |
Fix State Finalizer Prepare and Cleanup
The prepare() and cleanup() method were not called on state finalizers
for states. This review adds calls for them.
Issue-ID: POLICY-2450
Change-Id: I27aec4dea51f3e22b5922c04c7b7b974fca24292
Signed-off-by: liamfallon <liam.fallon@est.tech>
Diffstat (limited to 'core/core-engine')
5 files changed, 75 insertions, 65 deletions
diff --git a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateExecutor.java b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateExecutor.java index caaa1842b..192653930 100644 --- a/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateExecutor.java +++ b/core/core-engine/src/main/java/org/onap/policy/apex/core/engine/executor/StateExecutor.java @@ -1,7 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2016-2018 Ericsson. All rights reserved. - * Modifications Copyright (C) 2019 Nordix Foundation. + * Modifications Copyright (C) 2019-2020 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -97,7 +97,7 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap */ @Override public void setContext(final Executor<?, ?, ?, ?> incomingParent, final AxState incomingAxState, - final ApexInternalContext incomingContext) { + final ApexInternalContext incomingContext) { // Save the state and context definition this.parent = incomingParent; this.axState = incomingAxState; @@ -108,7 +108,7 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap // Set a task executor for each task for (final Entry<AxArtifactKey, AxStateTaskReference> stateTaskReferenceEntry : axState.getTaskReferences() - .entrySet()) { + .entrySet()) { final AxArtifactKey taskKey = stateTaskReferenceEntry.getKey(); final AxStateTaskReference taskReference = stateTaskReferenceEntry.getValue(); @@ -125,20 +125,20 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap } else if (taskReference.getStateTaskOutputType().equals(AxStateTaskOutputType.LOGIC)) { // Get the state finalizer logic for this task final AxStateFinalizerLogic finalizerLogic = - axState.getStateFinalizerLogicMap().get(taskReference.getOutput().getLocalName()); + axState.getStateFinalizerLogicMap().get(taskReference.getOutput().getLocalName()); if (finalizerLogic == null) { // Finalizer logic for the task does not exist throw new StateMachineRuntimeException("state finalizer logic on task reference \"" + taskReference - + "\" on state \"" + axState.getId() + "\" does not exist"); + + "\" on state \"" + axState.getId() + "\" does not exist"); } // Create a state finalizer executor for the task task2StateFinalizerMap.put(taskKey, - executorFactory.getStateFinalizerExecutor(this, finalizerLogic, context)); + executorFactory.getStateFinalizerExecutor(this, finalizerLogic, context)); } else { // This should never happen but..... throw new StateMachineRuntimeException("invalid state output type on task reference \"" + taskReference - + "\" on state \"" + axState.getId() + "\""); + + "\" on state \"" + axState.getId() + "\""); } } } @@ -158,6 +158,10 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap for (final TaskExecutor taskExecutor : taskExecutorMap.values()) { taskExecutor.prepare(); } + + for (final StateFinalizerExecutor stateFinalizer : task2StateFinalizerMap.values()) { + stateFinalizer.prepare(); + } } /** @@ -165,13 +169,13 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap */ @Override public StateOutput execute(final long executionId, final Properties executionProperties, - final EnEvent incomingEvent) throws StateMachineException, ContextException { + final EnEvent incomingEvent) throws StateMachineException, ContextException { this.lastIncomingEvent = incomingEvent; // Check that the incoming event matches the trigger for this state if (!incomingEvent.getAxEvent().getKey().equals(axState.getTrigger())) { throw new StateMachineException("incoming event \"" + incomingEvent.getId() + "\" does not match trigger \"" - + axState.getTrigger().getId() + "\" of state \"" + axState.getId() + "\""); + + axState.getTrigger().getId() + "\" of state \"" + axState.getId() + "\""); } // The key of the task to execute @@ -194,7 +198,7 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap final TreeMap<String, Object> incomingValues = new TreeMap<>(); incomingValues.putAll(incomingEvent); final Map<String, Object> taskExecutionResultMap = - taskExecutorMap.get(taskKey).execute(executionId, executionProperties, incomingValues); + taskExecutorMap.get(taskKey).execute(executionId, executionProperties, incomingValues); final AxTask task = taskExecutorMap.get(taskKey).getSubject(); // Check if this task has direct output @@ -207,20 +211,20 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap final StateFinalizerExecutor finalizerLogicExecutor = task2StateFinalizerMap.get(taskKey); if (finalizerLogicExecutor == null) { throw new StateMachineException("state finalizer logic for task \"" + taskKey.getId() - + "\" not found for state \"" + axState.getId() + "\""); + + "\" not found for state \"" + axState.getId() + "\""); } // Execute the state finalizer logic to select a state output and to adjust the // taskExecutionResultMap stateOutputName = - finalizerLogicExecutor.execute(executionId, executionProperties, taskExecutionResultMap); + finalizerLogicExecutor.execute(executionId, executionProperties, taskExecutionResultMap); } // Now look up the the actual state output final AxStateOutput stateOutputDefinition = axState.getStateOutputs().get(stateOutputName); if (stateOutputDefinition == null) { throw new StateMachineException("state output definition for state output \"" + stateOutputName - + "\" not found for state \"" + axState.getId() + "\""); + + "\" not found for state \"" + axState.getId() + "\""); } // Create the state output and transfer all the fields across to its event @@ -242,7 +246,7 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap return stateOutput; } catch (final Exception e) { final String errorMessage = "State execution of state \"" + axState.getId() + "\" on task \"" - + (taskKey != null ? taskKey.getId() : "null") + "\" failed: " + e.getMessage(); + + (taskKey != null ? taskKey.getId() : "null") + "\" failed: " + e.getMessage(); LOGGER.warn(errorMessage); throw new StateMachineException(errorMessage, e); @@ -254,7 +258,7 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap */ @Override public final void executePre(final long executionId, final Properties executionProperties, - final EnEvent incomingEntity) throws StateMachineException { + final EnEvent incomingEntity) throws StateMachineException { throw new StateMachineException("execution pre work not implemented on class"); } @@ -277,6 +281,10 @@ public class StateExecutor implements Executor<EnEvent, StateOutput, AxState, Ap // Clean the task selector taskSelectExecutor.cleanUp(); } + + for (final StateFinalizerExecutor stateFinalizer : task2StateFinalizerMap.values()) { + stateFinalizer.cleanUp(); + } } /** diff --git a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutorTest.java b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutorTest.java index 6fb28bca8..a46ddfb9e 100644 --- a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutorTest.java +++ b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateFinalizerExecutorTest.java @@ -1,6 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2018 Ericsson. All rights reserved. + * Modifications Copyright (C) 2020 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +21,7 @@ package org.onap.policy.apex.core.engine.executor; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -123,11 +125,8 @@ public class StateFinalizerExecutorTest { assertEquals("task input fields \"[InField0]\" are missing for task \"Task0:0.0.1\"", ex.getMessage()); } - try { - executor.executePre(0, null, incomingEvent); - } catch (Exception ex) { - assertEquals("executionProperties is marked @NonNull but is null", ex.getMessage()); - } + assertThatThrownBy(() -> executor.executePre(0, null, incomingEvent)) + .hasMessageMatching("^executionProperties is marked .*on.*ull but is null$"); try { executor.executePre(0, new Properties(), incomingEvent); @@ -140,7 +139,7 @@ public class StateFinalizerExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("execute() not implemented on abstract StateFinalizerExecutionContext class, " - + "only on its subclasses", ex.getMessage()); + + "only on its subclasses", ex.getMessage()); } try { @@ -148,7 +147,7 @@ public class StateFinalizerExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("execute-post: state finalizer logic execution failure on state \"NULL:0.0.0:NULL:NULL\" " - + "on finalizer logic null", ex.getMessage()); + + "on finalizer logic null", ex.getMessage()); } executor.getExecutionContext().setMessage("Execution message"); @@ -157,7 +156,7 @@ public class StateFinalizerExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("execute-post: state finalizer logic execution failure on state \"NULL:0.0.0:NULL:NULL\" " - + "on finalizer logic null, user message: Execution message", ex.getMessage()); + + "on finalizer logic null, user message: Execution message", ex.getMessage()); } try { @@ -171,7 +170,7 @@ public class StateFinalizerExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("execute-post: state finalizer logic \"null\" did not select an output state", - ex.getMessage()); + ex.getMessage()); } try { @@ -185,9 +184,10 @@ public class StateFinalizerExecutorTest { executor.executePost(true); fail("test should throw an exception"); } catch (Exception ex) { - assertEquals("execute-post: state finalizer logic \"null\" selected output state " - + "\"ThisOutputDoesNotExist\" that does not exsist on state \"NULL:0.0.0:NULL:NULL\"", - ex.getMessage()); + assertEquals( + "execute-post: state finalizer logic \"null\" selected output state " + + "\"ThisOutputDoesNotExist\" that does not exsist on state \"NULL:0.0.0:NULL:NULL\"", + ex.getMessage()); } try { diff --git a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutorTest.java b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutorTest.java index c2abd1e96..7256b3f31 100644 --- a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutorTest.java +++ b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/StateMachineExecutorTest.java @@ -1,6 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2018 Ericsson. All rights reserved. + * Modifications Copyright (C) 2020 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +23,7 @@ package org.onap.policy.apex.core.engine.executor; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.util.LinkedHashMap; @@ -37,7 +39,6 @@ import org.onap.policy.apex.context.parameters.SchemaParameters; import org.onap.policy.apex.core.engine.ExecutorParameters; import org.onap.policy.apex.core.engine.context.ApexInternalContext; import org.onap.policy.apex.core.engine.event.EnEvent; -import org.onap.policy.apex.core.engine.executor.exception.StateMachineException; import org.onap.policy.apex.model.basicmodel.concepts.AxArtifactKey; import org.onap.policy.apex.model.basicmodel.concepts.AxReferenceKey; import org.onap.policy.apex.model.basicmodel.service.ModelService; @@ -164,15 +165,15 @@ public class StateMachineExecutorTest { state1.getTaskReferences().put(task1Key, str1); Mockito.doReturn(new DummyTaskExecutor(true)).when(executorFactoryMock).getTaskExecutor(Mockito.anyObject(), - Mockito.anyObject(), Mockito.anyObject()); + Mockito.anyObject(), Mockito.anyObject()); dummyTsle = new DummyTaskSelectExecutor(true); Mockito.doReturn(dummyTsle).when(executorFactoryMock).getTaskSelectionExecutor(Mockito.anyObject(), - Mockito.anyObject(), Mockito.anyObject()); + Mockito.anyObject(), Mockito.anyObject()); dummySfle = new DummyStateFinalizerExecutor(true); Mockito.doReturn(dummySfle).when(executorFactoryMock).getStateFinalizerExecutor(Mockito.anyObject(), - Mockito.anyObject(), Mockito.anyObject()); + Mockito.anyObject(), Mockito.anyObject()); } @After @@ -183,8 +184,8 @@ public class StateMachineExecutorTest { @Test public void testStateMachineExecutor() { - StateMachineExecutor executor = new StateMachineExecutor(executorFactoryMock, - new AxArtifactKey("OwnerKey:0.0.1")); + StateMachineExecutor executor = + new StateMachineExecutor(executorFactoryMock, new AxArtifactKey("OwnerKey:0.0.1")); try { executor.execute(0, null, incomingEventMock); @@ -224,8 +225,9 @@ public class StateMachineExecutorTest { try { executor.prepare(); - } catch (StateMachineException e) { - fail("test should not throw an exception"); + fail("test should throw an exception"); + } catch (Exception e) { + assertTrue(e instanceof NullPointerException); } axPolicy.setFirstState("BadState"); @@ -260,11 +262,11 @@ public class StateMachineExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("state execution failed, next state \"Policy:0.0.1:PName:BadState\" not found", - ex.getMessage()); + ex.getMessage()); } axPolicy.getStateMap().get("State1").getStateOutputs().get("stateOutput1") - .setNextState(AxReferenceKey.getNullKey()); + .setNextState(AxReferenceKey.getNullKey()); dummyTsle.setTaskNo(0); try { executor.execute(0, null, incomingEventMock); @@ -279,7 +281,7 @@ public class StateMachineExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("incoming event \"Event1:0.0.1\" does not match trigger \"BadTrigger:0.0.1\" " - + "of state \"Policy:0.0.1:NULL:state1\"", ex.getMessage()); + + "of state \"Policy:0.0.1:NULL:state1\"", ex.getMessage()); } axPolicy.getStateMap().get("State1").setTrigger(new AxArtifactKey("Event1:0.0.1")); @@ -297,11 +299,11 @@ public class StateMachineExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("state finalizer logic on task reference " - + "\"AxStateTaskReference:(stateKey=AxReferenceKey:(parentKeyName=Policy," - + "parentKeyVersion=0.0.1,parentLocalName=state1,localName=str1)," - + "outputType=LOGIC,output=AxReferenceKey:(parentKeyName=Policy,parentKeyVersion=0.0.1," - + "parentLocalName=state1,localName=sfl))\" on state \"Policy:0.0.1:NULL:state1\" " - + "does not exist", ex.getMessage()); + + "\"AxStateTaskReference:(stateKey=AxReferenceKey:(parentKeyName=Policy," + + "parentKeyVersion=0.0.1,parentLocalName=state1,localName=str1)," + + "outputType=LOGIC,output=AxReferenceKey:(parentKeyName=Policy,parentKeyVersion=0.0.1," + + "parentLocalName=state1,localName=sfl))\" on state \"Policy:0.0.1:NULL:state1\" " + "does not exist", + ex.getMessage()); } axPolicy.getStateMap().get("State1").getStateFinalizerLogicMap().put("sfl", savedSfl); @@ -317,19 +319,19 @@ public class StateMachineExecutorTest { AxArtifactKey task1Key = new AxArtifactKey("task1:0.0.1"); try { axPolicy.getStateMap().get("State1").getTaskReferences().get(task1Key) - .setStateTaskOutputType(AxStateTaskOutputType.UNDEFINED); + .setStateTaskOutputType(AxStateTaskOutputType.UNDEFINED); executor.setContext(null, axPolicy, internalContextMock); fail("test should throw an exception"); } catch (Exception ex) { assertEquals("invalid state output type on task reference \"AxStateTaskReference:(stateKey=AxReferenceKey:" - + "(parentKeyName=Policy,parentKeyVersion=0.0.1,parentLocalName=state1,localName=str1)," - + "outputType=UNDEFINED,output=AxReferenceKey:(parentKeyName=Policy," - + "parentKeyVersion=0.0.1,parentLocalName=state1,localName=sfl))\" " - + "on state \"Policy:0.0.1:NULL:state1\"", ex.getMessage()); + + "(parentKeyName=Policy,parentKeyVersion=0.0.1,parentLocalName=state1,localName=str1)," + + "outputType=UNDEFINED,output=AxReferenceKey:(parentKeyName=Policy," + + "parentKeyVersion=0.0.1,parentLocalName=state1,localName=sfl))\" " + + "on state \"Policy:0.0.1:NULL:state1\"", ex.getMessage()); } axPolicy.getStateMap().get("State1").getTaskReferences().get(task1Key) - .setStateTaskOutputType(AxStateTaskOutputType.LOGIC); + .setStateTaskOutputType(AxStateTaskOutputType.LOGIC); executor.setContext(null, axPolicy, internalContextMock); dummyTsle.setTaskNo(0); @@ -346,8 +348,8 @@ public class StateMachineExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("State execution of state \"Policy:0.0.1:NULL:state1\" on task \"task1:0.0.1\" failed: " - + "state output definition for state output \"stateOutputBad\" not found for " - + "state \"Policy:0.0.1:NULL:state1\"", ex.getMessage()); + + "state output definition for state output \"stateOutputBad\" not found for " + + "state \"Policy:0.0.1:NULL:state1\"", ex.getMessage()); } dummyTsle.setTaskNo(0); @@ -360,15 +362,16 @@ public class StateMachineExecutorTest { try { executor.cleanUp(); + fail("test should throw an exception"); } catch (Exception ex) { - fail("test should not throw an exception"); + assertEquals("cleanUp() not implemented on class", ex.getMessage()); } } @Test public void testStateOutput() { - StateOutput output = new StateOutput( - axPolicy.getStateMap().get("State0").getStateOutputs().get("stateOutput0")); + StateOutput output = + new StateOutput(axPolicy.getStateMap().get("State0").getStateOutputs().get("stateOutput0")); assertNotNull(output); assertEquals("stateOutput0", output.getStateOutputDefinition().getKey().getLocalName()); @@ -401,7 +404,7 @@ public class StateMachineExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("field definitions and values do not match for event Event1:0.0.1\n[]\n[key]", - ex.getMessage()); + ex.getMessage()); } AxField axBadFieldDefinition = new AxField(); diff --git a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskExecutorTest.java b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskExecutorTest.java index 81b7d9424..a9218ea98 100644 --- a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskExecutorTest.java +++ b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskExecutorTest.java @@ -32,6 +32,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Properties; + import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -208,7 +209,7 @@ public class TaskExecutorTest { executor.executePost(true); assertThatThrownBy(() -> executor.executePre(0, null, incomingFields)) - .hasMessageContaining("executionProperties is marked @NonNull but is null"); + .hasMessageMatching("^executionProperties is marked .*on.*ull but is null$"); } @Test diff --git a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskSelectExecutorTest.java b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskSelectExecutorTest.java index 8e907e12a..817b1615f 100644 --- a/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskSelectExecutorTest.java +++ b/core/core-engine/src/test/java/org/onap/policy/apex/core/engine/executor/TaskSelectExecutorTest.java @@ -1,6 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2018 Ericsson. All rights reserved. + * Modifications Copyright (C) 2020 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +21,7 @@ package org.onap.policy.apex.core.engine.executor; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -151,7 +153,7 @@ public class TaskSelectExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("execute-post: task selection logic failed on state \"State0Parent:0.0.1:Parent:State0\"", - ex.getMessage()); + ex.getMessage()); } executor.getExecutionContext().setMessage("Execution message"); @@ -160,7 +162,7 @@ public class TaskSelectExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("execute-post: task selection logic failed on state \"State0Parent:0.0.1:Parent:State0\", " - + "user message: Execution message", ex.getMessage()); + + "user message: Execution message", ex.getMessage()); } try { @@ -188,7 +190,7 @@ public class TaskSelectExecutorTest { fail("test should throw an exception"); } catch (Exception ex) { assertEquals("task \"IDontExist:0.0.0\" returned by task selection logic not defined " - + "on state \"State0Parent:0.0.1:Parent:State0\"", ex.getMessage()); + + "on state \"State0Parent:0.0.1:Parent:State0\"", ex.getMessage()); } try { @@ -206,11 +208,7 @@ public class TaskSelectExecutorTest { fail("test should not throw an exception"); } - try { - executor.executePre(0, null, incomingEvent); - fail("test should throw an exception"); - } catch (Exception ex) { - assertEquals("executionProperties is marked @NonNull but is null", ex.getMessage()); - } + assertThatThrownBy(() -> executor.executePre(0, null, incomingEvent)) + .hasMessageMatching("^executionProperties is marked .*on.*ull but is null$"); } } |