Add "finalOutcome" flag to OperationOutcome

The Actor clients receive notifications of operation failures, but
no indication if it's the final failure, thus they do not know if
they should progress on to the "failure" policies.

Issue-ID: POLICY-2385
Signed-off-by: Jim Hahn <jrh3@att.com>
Change-Id: If053ed459e1d790e58eca950a8feeabaf4d67a41

3 files changed, 76 insertions, 29 deletions

diff --git a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationOutcome.java b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationOutcome.java
index 6b0924807..d8db70619 100644
--- a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationOutcome.java
+++ b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationOutcome.java
@@ -41,6 +41,7 @@ public class OperationOutcome {
     private String subRequestId;
     private PolicyResult result = PolicyResult.SUCCESS;
     private String message;
+    private boolean finalOutcome;
 
     /**
      * Copy constructor.
@@ -56,6 +57,7 @@ public class OperationOutcome {
         this.subRequestId = source.subRequestId;
         this.result = source.result;
         this.message = source.message;
+        this.finalOutcome = source.finalOutcome;
     }
 
     /**
diff --git a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartial.java b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartial.java
index c998209bc..be357b8f3 100644
--- a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartial.java
+++ b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartial.java
@@ -175,6 +175,7 @@ public abstract class OperationPartial implements Operation {
 
             // TODO need a FAILURE_MISSING_DATA (e.g., A&AI)
 
+            outcome2.setFinalOutcome(true);
             outcome2.setResult(PolicyResult.FAILURE_GUARD);
             outcome2.setMessage(outcome != null ? outcome.getMessage() : null);
 
@@ -220,13 +221,13 @@ public abstract class OperationPartial implements Operation {
      */
     protected CompletableFuture<OperationOutcome> startGuardAsync() {
         // get the guard payload
-        Map<String,Object> guardPayload = makeGuardPayload();
+        Map<String, Object> guardPayload = makeGuardPayload();
 
         // wrap it in a "resource"
-        Map<String,Object> resource = new LinkedHashMap<>();
+        Map<String, Object> resource = new LinkedHashMap<>();
         resource.put("guard", guardPayload);
 
-        Map<String,Object> payload = new LinkedHashMap<>();
+        Map<String, Object> payload = new LinkedHashMap<>();
         payload.put("resource", resource);
 
         /*
@@ -411,35 +412,47 @@ public abstract class OperationPartial implements Operation {
      */
     private Function<OperationOutcome, OperationOutcome> setRetryFlag(int attempt) {
 
-        return operation -> {
-            if (operation != null && !isActorFailed(operation)) {
-                /*
-                 * wrong type or wrong operation - just leave it as is. No need to log
-                 * anything here, as retryOnFailure() will log a message
-                 */
-                return operation;
+        return origOutcome -> {
+            // ensure we have a non-null outcome
+            OperationOutcome outcome;
+            if (origOutcome != null) {
+                outcome = origOutcome;
+            } else {
+                logger.warn("{}: null outcome; treating as a failure for {}", getFullName(), params.getRequestId());
+                outcome = this.setOutcome(params.makeOutcome(), PolicyResult.FAILURE);
             }
 
-            // get a non-null operation
-            OperationOutcome oper2;
-            if (operation != null) {
-                oper2 = operation;
-            } else {
-                oper2 = params.makeOutcome();
-                oper2.setResult(PolicyResult.FAILURE);
+            // ensure correct actor/operation
+            outcome.setActor(getActorName());
+            outcome.setOperation(getName());
+
+            // determine if we should retry, based on the result
+            if (outcome.getResult() != PolicyResult.FAILURE) {
+                // do not retry success or other failure types (e.g., exception)
+                outcome.setFinalOutcome(true);
+                return outcome;
             }
 
             int retry = getRetry(params.getRetry());
-            if (retry > 0 && attempt > retry) {
+            if (retry <= 0) {
+                // no retries were specified
+                outcome.setFinalOutcome(true);
+
+            } else if (attempt <= retry) {
+                // have more retries - not the final outcome
+                outcome.setFinalOutcome(false);
+
+            } else {
                 /*
                  * retries were specified and we've already tried them all - change to
                  * FAILURE_RETRIES
                  */
                 logger.info("operation {} retries exhausted for {}", getFullName(), params.getRequestId());
-                oper2.setResult(PolicyResult.FAILURE_RETRIES);
+                outcome.setResult(PolicyResult.FAILURE_RETRIES);
+                outcome.setFinalOutcome(true);
             }
 
-            return oper2;
+            return outcome;
         };
     }
 
@@ -870,10 +883,13 @@ public abstract class OperationPartial implements Operation {
         return (outcome, thrown) -> {
 
             if (callbacks.canStart()) {
-                // haven't invoked "start" callback yet
                 outcome.setStart(callbacks.getStartTime());
                 outcome.setEnd(null);
-                params.callbackStarted(outcome);
+
+                // pass a copy to the callback
+                OperationOutcome outcome2 = new OperationOutcome(outcome);
+                outcome2.setFinalOutcome(false);
+                params.callbackStarted(outcome2);
             }
         };
     }
@@ -894,11 +910,12 @@ public abstract class OperationPartial implements Operation {
     private BiConsumer<OperationOutcome, Throwable> callbackCompleted(CallbackManager callbacks) {
 
         return (outcome, thrown) -> {
-
             if (callbacks.canEnd()) {
                 outcome.setStart(callbacks.getStartTime());
                 outcome.setEnd(callbacks.getEndTime());
-                params.callbackCompleted(outcome);
+
+                // pass a copy to the callback
+                params.callbackCompleted(new OperationOutcome(outcome));
             }
         };
     }
diff --git a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartialTest.java b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartialTest.java
index 2893cb627..229e5a32a 100644
--- a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartialTest.java
+++ b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/OperationPartialTest.java
@@ -34,7 +34,9 @@ import static org.mockito.Mockito.when;
 
 import ch.qos.logback.classic.Logger;
 import java.time.Instant;
+import java.util.ArrayDeque;
 import java.util.Arrays;
+import java.util.Deque;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -128,6 +130,9 @@ public class OperationPartialTest {
     private OperationOutcome opstart;
     private OperationOutcome opend;
 
+    private Deque<OperationOutcome> starts;
+    private Deque<OperationOutcome> ends;
+
     private OperatorConfig config;
 
     /**
@@ -182,6 +187,9 @@ public class OperationPartialTest {
 
         opstart = null;
         opend = null;
+
+        starts = new ArrayDeque<>(10);
+        ends = new ArrayDeque<>(10);
     }
 
     @Test
@@ -519,10 +527,10 @@ public class OperationPartialTest {
         // arrange to return null from doOperation()
         oper = new MyOper() {
             @Override
-            protected OperationOutcome doOperation(int attempt, OperationOutcome operation) {
+            protected OperationOutcome doOperation(int attempt, OperationOutcome outcome) {
 
                 // update counters
-                super.doOperation(attempt, operation);
+                super.doOperation(attempt, outcome);
                 return null;
             }
         };
@@ -587,7 +595,7 @@ public class OperationPartialTest {
      * Tests handleFailure() when the outcome is a success.
      */
     @Test
-    public void testHandlePreprocessorFailureTrue() {
+    public void testHandlePreprocessorFailureSuccess() {
         oper.setPreProc(CompletableFuture.completedFuture(makeSuccess()));
         verifyRun("testHandlePreprocessorFailureTrue", 1, 1, PolicyResult.SUCCESS);
     }
@@ -596,7 +604,7 @@ public class OperationPartialTest {
      * Tests handleFailure() when the outcome is <i>not</i> a success.
      */
     @Test
-    public void testHandlePreprocessorFailureFalse() throws Exception {
+    public void testHandlePreprocessorFailureFailed() throws Exception {
         oper.setPreProc(CompletableFuture.completedFuture(makeFailure()));
         verifyRun("testHandlePreprocessorFailureFalse", 1, 0, PolicyResult.FAILURE_GUARD);
     }
@@ -1130,11 +1138,13 @@ public class OperationPartialTest {
         ++numStart;
         tstart = oper.getStart();
         opstart = oper;
+        starts.add(oper);
     }
 
     private void completer(OperationOutcome oper) {
         ++numEnd;
         opend = oper;
+        ends.add(oper);
     }
 
     /**
@@ -1194,6 +1204,12 @@ public class OperationPartialTest {
     private void verifyRun(String testName, int expectedCallbacks, int expectedOperations, PolicyResult expectedResult,
                     String expectedSubRequestId, Consumer<CompletableFuture<OperationOutcome>> manipulator) {
 
+        tstart = null;
+        opstart = null;
+        opend = null;
+        starts.clear();
+        ends.clear();
+
         CompletableFuture<OperationOutcome> future = oper.start();
 
         manipulator.accept(future);
@@ -1213,7 +1229,19 @@ public class OperationPartialTest {
 
             try {
                 assertTrue(future.isDone());
-                assertSame(testName, opend, future.get());
+                assertEquals(testName, opend, future.get());
+
+                // "start" is never final
+                for (OperationOutcome outcome : starts) {
+                    assertFalse(testName, outcome.isFinalOutcome());
+                }
+
+                // only the last "complete" is final
+                assertTrue(testName, ends.removeLast().isFinalOutcome());
+
+                for (OperationOutcome outcome : ends) {
+                    assertFalse(outcome.isFinalOutcome());
+                }
 
             } catch (InterruptedException | ExecutionException e) {
                 throw new IllegalStateException(e);