diff options
Diffstat (limited to 'models-interactions')
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); |