From 5bcf94e7e286ea9d84ebcdef65f8318f4a72111a Mon Sep 17 00:00:00 2001 From: liamfallon Date: Tue, 31 Mar 2020 11:29:20 +0100 Subject: 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 --- .../apex/core/engine/executor/StateExecutor.java | 38 ++++++++------ .../executor/StateFinalizerExecutorTest.java | 24 ++++----- .../engine/executor/StateMachineExecutorTest.java | 59 ++++++++++++---------- .../core/engine/executor/TaskExecutorTest.java | 3 +- .../engine/executor/TaskSelectExecutorTest.java | 16 +++--- 5 files changed, 75 insertions(+), 65 deletions(-) (limited to 'core/core-engine') 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 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 stateTaskReferenceEntry : axState.getTaskReferences() - .entrySet()) { + .entrySet()) { final AxArtifactKey taskKey = stateTaskReferenceEntry.getKey(); final AxStateTaskReference taskReference = stateTaskReferenceEntry.getValue(); @@ -125,20 +125,20 @@ public class StateExecutor implements Executor incomingValues = new TreeMap<>(); incomingValues.putAll(incomingEvent); final Map 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 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$"); } } -- cgit 1.2.3-korg