From 8722cb8a654ddcdcb472546a58809e041fe84eba Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 18 Aug 2020 09:39:19 -0400 Subject: Make targetEntity a property The target entity is not always known when an Operation is first constructed, thus it should be treated as an Operation property instead of being included within the ControlLoopParams. Started the process of moving it from the Params to the properties. Also fixed a bug in custom query - it was setting the outcome response to the String response instead of setting it to the AaiCqResponse object. Also added logging when an Operation's properties are set. Issue-ID: POLICY-2746 Change-Id: I56c0cd90985f6140363548b0b8d031471b586e88 Signed-off-by: Jim Hahn --- .../actorserviceprovider/Operation.java | 17 +++++++++ .../actorserviceprovider/OperationProperties.java | 44 +++++++++++++--------- .../impl/OperationPartial.java | 44 ++++++++++------------ .../parameters/ControlLoopOperationParams.java | 19 ++++++++-- .../impl/BidirectionalTopicOperationTest.java | 2 +- .../impl/HttpOperationTest.java | 2 +- .../impl/HttpPollingOperationTest.java | 2 +- .../impl/OperationPartialTest.java | 43 +++++++++++++-------- .../parameters/ControlLoopOperationParamsTest.java | 9 +++-- 9 files changed, 116 insertions(+), 66 deletions(-) (limited to 'models-interactions/model-actors/actorServiceProvider/src') diff --git a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/Operation.java b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/Operation.java index dfa086595..2c63e98ee 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/Operation.java +++ b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/Operation.java @@ -50,6 +50,15 @@ public interface Operation { */ List getPropertyNames(); + /** + * Determines if a property has been assigned for the operation. + * + * @param name property name + * @return {@code true} if the given property has been assigned for the operation, + * {@code false} otherwise + */ + public boolean containsProperty(String name); + /** * Sets a property. * @@ -58,6 +67,14 @@ public interface Operation { */ public void setProperty(String name, Object value); + /** + * Gets a property's value. + * + * @param name name of the property of interest + * @return the property's value, or {@code null} if it has no value + */ + public T getProperty(String name); + /** * Called by enforcement PDP engine to start the operation. As part of the operation, * it invokes the "start" and "complete" call-backs found within the parameters. diff --git a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationProperties.java b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationProperties.java index c36b61e8b..718daedb1 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationProperties.java +++ b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/OperationProperties.java @@ -23,6 +23,9 @@ package org.onap.policy.controlloop.actorserviceprovider; /** * Names of properties needed by the Actors defined within this repo. Note: this is not * exhaustive, as additional property names may be returned by company-defined Actors. + *

+ * Note: any time a property is added, applications using the actors must be updated to + * provide the property's value when requested. */ public class OperationProperties { @@ -42,6 +45,23 @@ public class OperationProperties { */ public static final String AAI_DEFAULT_TENANT = "AAI/defaultTenant"; + /** + * A&AI PNF. Obtained as follows: + *

    + *
  1. using the target entity, invoke AaiGetPnfOperation
    2. + *
+ */ + public static final String AAI_PNF = "AAI/pnf"; + + /** + * A&AI VNF id for the target resource ID. Obtained as follows: + *
    + *
  1. using the target resource ID, invoke the custom query + * getGenericVnfByModelInvariantId() method to get the generic VNF
    2. + *
+ */ + public static final String AAI_RESOURCE_VNF = "AAI/resourceId/vnf"; + /** * A&AI Service instance. Obtained as follows: *
    @@ -59,6 +79,13 @@ public class OperationProperties { */ public static final String AAI_SERVICE_MODEL = "AAI/service/model"; + /** + * A&AI Target Entity. This is a String that can typically be found in the enrichment + * data, depending on the Target type. Sometimes, however, it must be retrieved via an + * A&AI query. + */ + public static final String AAI_TARGET_ENTITY = "AAI/targetEntity"; + /** * A&AI VNF. Obtained as follows: *
      @@ -82,23 +109,6 @@ public class OperationProperties { */ public static final String AAI_VNF_MODEL = "AAI/vnf/model"; - /** - * A&AI VNF id for the target resource ID. Obtained as follows: - *
        - *
      1. using the target resource ID, invoke the custom query - * getGenericVnfByModelInvariantId() method to get the generic VNF
        2. - *
      - */ - public static final String AAI_RESOURCE_VNF = "AAI/resourceId/vnf"; - - /** - * A&AI PNF. Obtained as follows: - *
        - *
      1. using the target entity, invoke AaiGetPnfOperation
        2. - *
      - */ - public static final String AAI_PNF = "AAI/pnf"; - /** * A&AI link to the vserver. Obtained as follows: *
        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 0aa112234..b5cc15e19 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 @@ -53,6 +53,7 @@ import org.onap.policy.controlloop.ControlLoopOperation; import org.onap.policy.controlloop.actorserviceprovider.CallbackManager; import org.onap.policy.controlloop.actorserviceprovider.Operation; import org.onap.policy.controlloop.actorserviceprovider.OperationOutcome; +import org.onap.policy.controlloop.actorserviceprovider.OperationProperties; import org.onap.policy.controlloop.actorserviceprovider.parameters.ControlLoopOperationParams; import org.onap.policy.controlloop.actorserviceprovider.parameters.OperatorConfig; import org.onap.policy.controlloop.actorserviceprovider.pipeline.PipelineControllerFuture; @@ -139,34 +140,19 @@ public abstract class OperationPartial implements Operation { return params.getOperation(); } - /** - * Determines if a property has been assigned for the operation. - * - * @param name property name - * @return {@code true} if the given property has been assigned for the operation, - * {@code false} otherwise - */ + @Override public boolean containsProperty(String name) { return properties.containsKey(name); } - /** - * Sets a property. - * - * @param name property name - * @param value new value - */ + @Override public void setProperty(String name, Object value) { + logger.info("{}: set property {}={}", getFullName(), name, value); properties.put(name, value); } - /** - * Gets a property's value. - * - * @param name name of the property of interest - * @return the property's value, or {@code null} if it has no value - */ @SuppressWarnings("unchecked") + @Override public T getProperty(String name) { return (T) properties.get(name); } @@ -230,7 +216,7 @@ public abstract class OperationPartial implements Operation { // propagate "stop" to the callbacks controller.add(callbacks); - final OperationOutcome outcome2 = params.makeOutcome(); + final OperationOutcome outcome2 = params.makeOutcome(getTargetEntity()); // TODO need a FAILURE_MISSING_DATA (e.g., A&AI) @@ -304,7 +290,7 @@ public abstract class OperationPartial implements Operation { Map guard = new LinkedHashMap<>(); guard.put("actor", params.getActor()); guard.put("operation", params.getOperation()); - guard.put("target", params.getTargetEntity()); + guard.put("target", getTargetEntity()); guard.put("requestId", params.getRequestId()); String clname = params.getContext().getEvent().getClosedLoopControlName(); @@ -358,7 +344,7 @@ public abstract class OperationPartial implements Operation { logger.info("{}: start operation attempt {} for {}", getFullName(), attempt, params.getRequestId()); final Executor executor = params.getExecutor(); - final OperationOutcome outcome = params.makeOutcome(); + final OperationOutcome outcome = params.makeOutcome(getTargetEntity()); final CallbackManager callbacks = new CallbackManager(); // this operation attempt gets its own controller @@ -489,7 +475,7 @@ public abstract class OperationPartial implements Operation { outcome = origOutcome; } else { logger.warn("{}: null outcome; treating as a failure for {}", getFullName(), params.getRequestId()); - outcome = this.setOutcome(params.makeOutcome(), PolicyResult.FAILURE); + outcome = this.setOutcome(params.makeOutcome(getTargetEntity()), PolicyResult.FAILURE); } // ensure correct actor/operation @@ -588,7 +574,7 @@ public abstract class OperationPartial implements Operation { private Function fromException(String type) { return thrown -> { - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(getTargetEntity()); if (thrown instanceof CancellationException || thrown.getCause() instanceof CancellationException) { // do not include exception in the message, as it just clutters the log @@ -1106,6 +1092,16 @@ public abstract class OperationPartial implements Operation { return DEFAULT_RETRY_WAIT_MS; } + /** + * Gets the target entity, first trying the properties and then the parameters. + * + * @return the target entity + */ + protected String getTargetEntity() { + String targetEntity = getProperty(OperationProperties.AAI_TARGET_ENTITY); + return (targetEntity != null ? targetEntity : params.getTargetEntity()); + } + /** * Gets the operation timeout. * diff --git a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParams.java b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParams.java index d0b7c26a8..66573f38a 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParams.java +++ b/models-interactions/model-actors/actorServiceProvider/src/main/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParams.java @@ -69,6 +69,7 @@ public class ControlLoopOperationParams { /** * Event for which the operation applies. */ + // TODO to be removed private ControlLoopEventContext context; /** @@ -106,15 +107,15 @@ public class ControlLoopOperationParams { private Integer retry; /** - * The entity's target information. May be {@code null}, depending on the requirement - * of the operation to be invoked. + * The Target information, extracted from the Policy. May be {@code null}, depending + * on the requirement of the operation to be invoked. */ private Target target; /** * Target entity. */ - @NotNull + // TODO to be removed private String targetEntity; /** @@ -193,7 +194,19 @@ public class ControlLoopOperationParams { * * @return a new operation outcome */ + // TODO to be removed public OperationOutcome makeOutcome() { + return makeOutcome(getTargetEntity()); + } + + /** + * Makes an operation outcome, populating it from the parameters. + * + * @param targetEntity the target entity + * + * @return a new operation outcome + */ + public OperationOutcome makeOutcome(String targetEntity) { OperationOutcome outcome = new OperationOutcome(); outcome.setActor(getActor()); outcome.setOperation(getOperation()); diff --git a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/BidirectionalTopicOperationTest.java b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/BidirectionalTopicOperationTest.java index e28ddeb09..f63e07e57 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/BidirectionalTopicOperationTest.java +++ b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/BidirectionalTopicOperationTest.java @@ -110,7 +110,7 @@ public class BidirectionalTopicOperationTest { executor = new PseudoExecutor(); params = ControlLoopOperationParams.builder().actor(ACTOR).operation(OPERATION).executor(executor).build(); - outcome = params.makeOutcome(); + outcome = params.makeOutcome(null); response = new MyResponse(); response.setRequestId(REQ_ID); diff --git a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpOperationTest.java b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpOperationTest.java index 33e530334..daabaa237 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpOperationTest.java +++ b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpOperationTest.java @@ -190,7 +190,7 @@ public class HttpOperationTest { context = new ControlLoopEventContext(event); params = ControlLoopOperationParams.builder().actor(ACTOR).operation(OPERATION).context(context).build(); - outcome = params.makeOutcome(); + outcome = params.makeOutcome(null); callback = new AtomicReference<>(); future = new CompletableFuture<>(); diff --git a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpPollingOperationTest.java b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpPollingOperationTest.java index 780964629..ede2b7966 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpPollingOperationTest.java +++ b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/impl/HttpPollingOperationTest.java @@ -100,7 +100,7 @@ public class HttpPollingOperationTest { when(rawResponse.readEntity(String.class)).thenReturn(response); params = ControlLoopOperationParams.builder().actor(MY_ACTOR).operation(MY_OPERATION).build(); - outcome = params.makeOutcome(); + outcome = params.makeOutcome(null); oper = new MyOper(params, config); } 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 6d5435827..6db824f98 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 @@ -75,6 +75,7 @@ import org.onap.policy.controlloop.VirtualControlLoopEvent; import org.onap.policy.controlloop.actorserviceprovider.ActorService; import org.onap.policy.controlloop.actorserviceprovider.Operation; import org.onap.policy.controlloop.actorserviceprovider.OperationOutcome; +import org.onap.policy.controlloop.actorserviceprovider.OperationProperties; import org.onap.policy.controlloop.actorserviceprovider.Operator; import org.onap.policy.controlloop.actorserviceprovider.controlloop.ControlLoopEventContext; import org.onap.policy.controlloop.actorserviceprovider.parameters.ControlLoopOperationParams; @@ -399,7 +400,7 @@ public class OperationPartialTest { public void testIsActorFailed() { assertFalse(oper.isActorFailed(null)); - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(null); // incorrect outcome outcome.setResult(PolicyResult.SUCCESS); @@ -459,7 +460,7 @@ public class OperationPartialTest { @Override protected CompletableFuture startOperationAsync(int attempt, OperationOutcome outcome) { - OperationOutcome outcome2 = params.makeOutcome(); + OperationOutcome outcome2 = params.makeOutcome(null); outcome2.setResult(PolicyResult.SUCCESS); /* @@ -596,7 +597,7 @@ public class OperationPartialTest { public void testIsSameOperation() { assertFalse(oper.isSameOperation(null)); - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(null); // wrong actor - should be false outcome.setActor(null); @@ -667,7 +668,7 @@ public class OperationPartialTest { // first task completes, others do not List>> tasks = new LinkedList<>(); - final OperationOutcome outcome = params.makeOutcome(); + final OperationOutcome outcome = params.makeOutcome(null); tasks.add(() -> CompletableFuture.completedFuture(outcome)); tasks.add(() -> new CompletableFuture<>()); @@ -732,7 +733,7 @@ public class OperationPartialTest { @Test public void testAllOfArray() throws Exception { - final OperationOutcome outcome = params.makeOutcome(); + final OperationOutcome outcome = params.makeOutcome(null); CompletableFuture future1 = new CompletableFuture<>(); CompletableFuture future2 = new CompletableFuture<>(); @@ -763,7 +764,7 @@ public class OperationPartialTest { @Test public void testAllOfList() throws Exception { - final OperationOutcome outcome = params.makeOutcome(); + final OperationOutcome outcome = params.makeOutcome(null); CompletableFuture future1 = new CompletableFuture<>(); CompletableFuture future2 = new CompletableFuture<>(); @@ -852,9 +853,9 @@ public class OperationPartialTest { // null outcome - takes precedence over a success List>> tasks = new LinkedList<>(); - tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome())); + tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome(null))); tasks.add(() -> CompletableFuture.completedFuture(null)); - tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome())); + tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome(null))); CompletableFuture result = oper.allOf(tasks); assertTrue(executor.runAll(MAX_REQUESTS)); @@ -865,9 +866,9 @@ public class OperationPartialTest { IllegalStateException except = new IllegalStateException(EXPECTED_EXCEPTION); tasks.clear(); - tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome())); + tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome(null))); tasks.add(() -> CompletableFuture.failedFuture(except)); - tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome())); + tasks.add(() -> CompletableFuture.completedFuture(params.makeOutcome(null))); result = oper.allOf(tasks); assertTrue(executor.runAll(MAX_REQUESTS)); @@ -880,7 +881,7 @@ public class OperationPartialTest { */ @Test public void testSequence() throws Exception { - final OperationOutcome outcome = params.makeOutcome(); + final OperationOutcome outcome = params.makeOutcome(null); List>> tasks = new LinkedList<>(); tasks.add(() -> CompletableFuture.completedFuture(outcome)); @@ -902,7 +903,7 @@ public class OperationPartialTest { assertSame(outcome, result.get()); // second task fails, third should not run - OperationOutcome failure = params.makeOutcome(); + OperationOutcome failure = params.makeOutcome(null); failure.setResult(PolicyResult.FAILURE); tasks.clear(); tasks.add(() -> CompletableFuture.completedFuture(outcome)); @@ -941,7 +942,7 @@ public class OperationPartialTest { OperationOutcome expectedOutcome = null; for (int count = 0; count < results.length; ++count) { - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(null); outcome.setResult(results[count]); tasks.add(() -> CompletableFuture.completedFuture(outcome)); @@ -961,7 +962,7 @@ public class OperationPartialTest { public void testDetmPriority() throws CoderException { assertEquals(1, oper.detmPriority(null)); - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(null); Map map = Map.of(PolicyResult.SUCCESS, 0, PolicyResult.FAILURE_GUARD, 2, PolicyResult.FAILURE_RETRIES, 3, PolicyResult.FAILURE, 4, PolicyResult.FAILURE_TIMEOUT, 5, @@ -1150,6 +1151,16 @@ public class OperationPartialTest { assertEquals(OperationPartial.DEFAULT_RETRY_WAIT_MS, oper2.getRetryWaitMs()); } + @Test + public void testGetTargetEntity() { + // get it from the params + assertEquals(MY_TARGET_ENTITY, oper.getTargetEntity()); + + // now get it from the properties + oper.setProperty(OperationProperties.AAI_TARGET_ENTITY, "entityX"); + assertEquals("entityX", oper.getTargetEntity()); + } + @Test public void testGetTimeOutMs() { assertEquals(TIMEOUT * 1000, oper.getTimeoutMs(params.getTimeoutSec())); @@ -1187,14 +1198,14 @@ public class OperationPartialTest { } private OperationOutcome makeSuccess() { - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(null); outcome.setResult(PolicyResult.SUCCESS); return outcome; } private OperationOutcome makeFailure() { - OperationOutcome outcome = params.makeOutcome(); + OperationOutcome outcome = params.makeOutcome(null); outcome.setResult(PolicyResult.FAILURE); return outcome; diff --git a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParamsTest.java b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParamsTest.java index 634d7b132..5e79247ed 100644 --- a/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParamsTest.java +++ b/models-interactions/model-actors/actorServiceProvider/src/test/java/org/onap/policy/controlloop/actorserviceprovider/parameters/ControlLoopOperationParamsTest.java @@ -131,7 +131,7 @@ public class ControlLoopOperationParamsTest { .retry(RETRY).target(TARGET).targetEntity(TARGET_ENTITY).timeoutSec(TIMEOUT) .startCallback(starter).preprocessed(true).build(); - outcome = params.makeOutcome(); + outcome = params.makeOutcome(TARGET_ENTITY); } @Test @@ -245,7 +245,10 @@ public class ControlLoopOperationParamsTest { testValidate("actorService", NULL_MSG, bldr -> bldr.actorService(null)); testValidate("executor", NULL_MSG, bldr -> bldr.executor(null)); testValidate("operation", NULL_MSG, bldr -> bldr.operation(null)); - testValidate("target", NULL_MSG, bldr -> bldr.targetEntity(null)); + + // has no target entity + BeanValidationResult result = params.toBuilder().targetEntity(null).build().validate(); + assertTrue(result.isValid()); // note: if context is null, then it will ACTUALLY complain about the request ID testValidate(REQUEST_ID_NAME, NULL_MSG, bldr -> bldr.context(null)); @@ -263,7 +266,7 @@ public class ControlLoopOperationParamsTest { // test when event has no request ID when(event.getRequestId()).thenReturn(null); - BeanValidationResult result = params.validate(); + result = params.validate(); assertFalse(result.isValid()); assertThat(result.getResult()).contains("event").contains(REQUEST_ID_NAME).contains(NULL_MSG); -- cgit 1.2.3-korg