Support separate VF Counts for different Targets

Changed the key by which the VF Count is stored within the context
so that each Target can have its own VF Count.  Also moved VF Count
code from VfModuleCreate to SoOperation, to hide the determination
of the VF Count "key".
Fixed sonar issue about return "null" instead of returning an empty
list - modified the code to use Optional.

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

5 files changed, 132 insertions, 67 deletions

diff --git a/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoConstants.java b/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoConstants.java
index faafb43a6..b238671b4 100644
--- a/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoConstants.java
+++ b/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoConstants.java
@@ -21,7 +21,7 @@
 package org.onap.policy.controlloop.actor.so;
 
 public class SoConstants {
-    public static final String CONTEXT_KEY_VF_COUNT = "SO.VFCount";
+    public static final String VF_COUNT_PREFIX = "SO.VFCount";
 
     private SoConstants() {
         // do nothing
diff --git a/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoOperation.java b/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoOperation.java
index 53fb973a5..41ecd07a0 100644
--- a/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoOperation.java
+++ b/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/SoOperation.java
@@ -23,6 +23,7 @@ package org.onap.policy.controlloop.actor.so;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
@@ -32,6 +33,7 @@ import org.onap.aai.domain.yang.CloudRegion;
 import org.onap.aai.domain.yang.GenericVnf;
 import org.onap.aai.domain.yang.ServiceInstance;
 import org.onap.aai.domain.yang.Tenant;
+import org.onap.policy.aai.AaiConstants;
 import org.onap.policy.aai.AaiCqResponse;
 import org.onap.policy.common.endpoints.event.comm.Topic.CommInfrastructure;
 import org.onap.policy.common.endpoints.utils.NetLoggerUtil.EventType;
@@ -71,6 +73,13 @@ public abstract class SoOperation extends HttpOperation<SoResponse> {
 
     private final SoConfig config;
 
+    // values extracted from the parameter Target
+    private final String modelCustomizationId;
+    private final String modelInvariantId;
+    private final String modelVersionId;
+
+    private final String vfCountKey;
+
     /**
      * Number of "get" requests issued so far, on the current operation attempt.
      */
@@ -87,6 +96,15 @@ public abstract class SoOperation extends HttpOperation<SoResponse> {
     public SoOperation(ControlLoopOperationParams params, HttpConfig config) {
         super(params, config, SoResponse.class);
         this.config = (SoConfig) config;
+
+        verifyNotNull("Target information", params.getTarget());
+
+        this.modelCustomizationId = params.getTarget().getModelCustomizationId();
+        this.modelInvariantId = params.getTarget().getModelInvariantId();
+        this.modelVersionId = params.getTarget().getModelVersionId();
+
+        vfCountKey = SoConstants.VF_COUNT_PREFIX + "[" + modelCustomizationId + "][" + modelInvariantId + "]["
+                        + modelVersionId + "]";
     }
 
     /**
@@ -101,10 +119,9 @@ public abstract class SoOperation extends HttpOperation<SoResponse> {
      * the VF count from the custom query.
      */
     protected void validateTarget() {
-        verifyNotNull("Target information", params.getTarget());
-        verifyNotNull("model-customization-id", params.getTarget().getModelCustomizationId());
-        verifyNotNull("model-invariant-id", params.getTarget().getModelInvariantId());
-        verifyNotNull("model-version-id", params.getTarget().getModelVersionId());
+        verifyNotNull("model-customization-id", modelCustomizationId);
+        verifyNotNull("model-invariant-id", modelInvariantId);
+        verifyNotNull("model-version-id", modelVersionId);
     }
 
     private void verifyNotNull(String type, Object value) {
@@ -122,21 +139,47 @@ public abstract class SoOperation extends HttpOperation<SoResponse> {
     }
 
     /**
-     * Stores the VF count and then runs the guard.
+     * Gets the VF Count.
      *
-     * @return a future to cancel or await the guard response
+     * @return a future to cancel or await the VF Count
      */
-    protected CompletableFuture<OperationOutcome> storeVfCountRunGuard() {
-        String custId = params.getTarget().getModelCustomizationId();
-        String invId = params.getTarget().getModelInvariantId();
-        String verId = params.getTarget().getModelVersionId();
+    @SuppressWarnings("unchecked")
+    protected CompletableFuture<OperationOutcome> obtainVfCount() {
+        if (params.getContext().contains(vfCountKey)) {
+            // already have the VF count
+            return null;
+        }
 
-        AaiCqResponse cq = params.getContext().getProperty(AaiCqResponse.CONTEXT_KEY);
-        int vfcount = cq.getVfModuleCount(custId, invId, verId);
+        // need custom query from which to extract the VF count
+        ControlLoopOperationParams cqParams = params.toBuilder().actor(AaiConstants.ACTOR_NAME)
+                        .operation(AaiCqResponse.OPERATION).payload(null).retry(null).timeoutSec(null).build();
 
-        params.getContext().setProperty(SoConstants.CONTEXT_KEY_VF_COUNT, vfcount);
+        // run Custom Query and then extract the VF count
+        return sequence(() -> params.getContext().obtain(AaiCqResponse.CONTEXT_KEY, cqParams), this::storeVfCount);
+    }
 
-        return startGuardAsync();
+    /**
+     * Stores the VF count.
+     *
+     * @return {@code null}
+     */
+    private CompletableFuture<OperationOutcome> storeVfCount() {
+        if (!params.getContext().contains(vfCountKey)) {
+            AaiCqResponse cq = params.getContext().getProperty(AaiCqResponse.CONTEXT_KEY);
+            int vfcount = cq.getVfModuleCount(modelCustomizationId, modelInvariantId, modelVersionId);
+
+            params.getContext().setProperty(vfCountKey, vfcount);
+        }
+
+        return null;
+    }
+
+    protected int getVfCount() {
+        return params.getContext().getProperty(vfCountKey);
+    }
+
+    protected void setVfCount(int vfCount) {
+        params.getContext().setProperty(vfCountKey, vfCount);
     }
 
     /**
@@ -289,18 +332,18 @@ public abstract class SoOperation extends HttpOperation<SoResponse> {
     /**
      * Builds the request parameters from the policy payload.
      */
-    protected SoRequestParameters buildRequestParameters() {
+    protected Optional<SoRequestParameters> buildRequestParameters() {
         if (params.getPayload() == null) {
-            return null;
+            return Optional.empty();
         }
 
         Object data = params.getPayload().get(REQ_PARAM_NM);
         if (data == null) {
-            return null;
+            return Optional.empty();
         }
 
         try {
-            return coder.decode(data.toString(), SoRequestParameters.class);
+            return Optional.of(coder.decode(data.toString(), SoRequestParameters.class));
         } catch (CoderException e) {
             throw new IllegalArgumentException("invalid payload value: " + REQ_PARAM_NM);
         }
@@ -309,20 +352,20 @@ public abstract class SoOperation extends HttpOperation<SoResponse> {
     /**
      * Builds the configuration parameters from the policy payload.
      */
-    protected List<Map<String, String>> buildConfigurationParameters() {
+    protected Optional<List<Map<String, String>>> buildConfigurationParameters() {
         if (params.getPayload() == null) {
-            return null;
+            return Optional.empty();
         }
 
         Object data = params.getPayload().get(CONFIG_PARAM_NM);
         if (data == null) {
-            return null;
+            return Optional.empty();
         }
 
         try {
             @SuppressWarnings("unchecked")
             List<Map<String, String>> result = coder.decode(data.toString(), ArrayList.class);
-            return result;
+            return Optional.of(result);
         } catch (CoderException | RuntimeException e) {
             throw new IllegalArgumentException("invalid payload value: " + CONFIG_PARAM_NM);
         }
diff --git a/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/VfModuleCreate.java b/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/VfModuleCreate.java
index a0a1ec031..fb8fe6072 100644
--- a/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/VfModuleCreate.java
+++ b/models-interactions/model-actors/actor.so/src/main/java/org/onap/policy/controlloop/actor/so/VfModuleCreate.java
@@ -77,27 +77,25 @@ public class VfModuleCreate extends SoOperation {
     @Override
     @SuppressWarnings("unchecked")
     protected CompletableFuture<OperationOutcome> startPreprocessorAsync() {
-        if (params.getContext().contains(SoConstants.CONTEXT_KEY_VF_COUNT)) {
-            return startGuardAsync();
-        }
 
         // need the VF count
         ControlLoopOperationParams cqParams = params.toBuilder().actor(AaiConstants.ACTOR_NAME)
                         .operation(AaiCqResponse.OPERATION).payload(null).retry(null).timeoutSec(null).build();
 
         // run Custom Query, extract the VF count, and then run the Guard
+
+        // @formatter:off
         return sequence(() -> params.getContext().obtain(AaiCqResponse.CONTEXT_KEY, cqParams),
-                        this::storeVfCountRunGuard);
+                        this::obtainVfCount, this::startGuardAsync);
+        // @formatter:on
     }
 
     @Override
     protected Map<String, Object> makeGuardPayload() {
         Map<String, Object> payload = super.makeGuardPayload();
 
-        int vfcount = params.getContext().getProperty(SoConstants.CONTEXT_KEY_VF_COUNT);
-
         // run guard with the proposed vf count
-        payload.put(PAYLOAD_KEY_VF_COUNT, vfcount + 1);
+        payload.put(PAYLOAD_KEY_VF_COUNT, getVfCount() + 1);
 
         return payload;
     }
@@ -127,8 +125,7 @@ public class VfModuleCreate extends SoOperation {
      */
     @Override
     protected void successfulCompletion() {
-        int vfcount = params.getContext().getProperty(SoConstants.CONTEXT_KEY_VF_COUNT);
-        params.getContext().setProperty(SoConstants.CONTEXT_KEY_VF_COUNT, vfcount + 1);
+        setVfCount(getVfCount() + 1);
     }
 
     /**
@@ -205,10 +202,10 @@ public class VfModuleCreate extends SoOperation {
         request.getRequestDetails().getRelatedInstanceList().add(relatedInstanceListElement2);
 
         // Request Parameters
-        request.getRequestDetails().setRequestParameters(buildRequestParameters());
+        buildRequestParameters().ifPresent(request.getRequestDetails()::setRequestParameters);
 
         // Configuration Parameters
-        request.getRequestDetails().setConfigurationParameters(buildConfigurationParameters());
+        buildConfigurationParameters().ifPresent(request.getRequestDetails()::setConfigurationParameters);
 
         // compute the path
         String path = "/serviceInstantiation/v7/serviceInstances/" + vnfServiceItem.getServiceInstanceId() + "/vnfs/"
diff --git a/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/SoOperationTest.java b/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/SoOperationTest.java
index b2ae5727b..129c19381 100644
--- a/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/SoOperationTest.java
+++ b/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/SoOperationTest.java
@@ -23,6 +23,7 @@ package org.onap.policy.controlloop.actor.so;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
@@ -33,7 +34,6 @@ import static org.mockito.Mockito.when;
 
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.TimeUnit;
@@ -53,13 +53,14 @@ import org.onap.policy.controlloop.policy.PolicyResult;
 import org.onap.policy.so.SoModelInfo;
 import org.onap.policy.so.SoRequest;
 import org.onap.policy.so.SoRequestInfo;
-import org.onap.policy.so.SoRequestParameters;
 import org.onap.policy.so.SoRequestReferences;
 import org.onap.policy.so.SoRequestStatus;
 import org.onap.policy.so.SoResponse;
 
 public class SoOperationTest extends BasicSoOperation {
 
+    private static final String VF_COUNT_KEY = SoConstants.VF_COUNT_PREFIX
+                    + "[my-model-customization-id][my-model-invariant-id][my-model-version-id]";
     private SoOperation oper;
 
     /**
@@ -80,6 +81,11 @@ public class SoOperationTest extends BasicSoOperation {
         assertEquals(DEFAULT_OPERATION, oper.getName());
         assertSame(config, oper.getConfig());
         assertEquals(1000 * WAIT_SEC_GETS, oper.getWaitMsGet());
+
+        // check when Target is null
+        params = params.toBuilder().target(null).build();
+        assertThatIllegalArgumentException().isThrownBy(() -> new SoOperation(params, config) {})
+                        .withMessageContaining("Target information");
     }
 
     @Test
@@ -91,11 +97,6 @@ public class SoOperationTest extends BasicSoOperation {
 
         // verify it's still valid
         assertThatCode(() -> new VfModuleCreate(params, config)).doesNotThrowAnyException();
-
-        // check when Target, itself, is null
-        params = params.toBuilder().target(null).build();
-        assertThatIllegalArgumentException().isThrownBy(() -> new VfModuleCreate(params, config))
-                        .withMessageContaining("Target information");
     }
 
     private void verifyNotNull(String expectedText, Supplier<String> getter, Consumer<String> setter) {
@@ -115,23 +116,51 @@ public class SoOperationTest extends BasicSoOperation {
     }
 
     @Test
-    public void testStoreVfCountRunGuard() throws Exception {
-        // insert CQ data so it's there for the guard
+    public void testObtainVfCount_testGetVfCount_testSetVfCount() throws Exception {
+        // insert CQ data so it's there for the check
         context.setProperty(AaiCqResponse.CONTEXT_KEY, makeCqResponse());
 
-        // cause guard to fail
-        OperationOutcome outcome2 = params.makeOutcome();
-        outcome2.setResult(PolicyResult.FAILURE);
-        when(guardOperation.start()).thenReturn(CompletableFuture.completedFuture(outcome2));
+        // shouldn't actually need to do anything
+        assertNull(oper.obtainVfCount());
+
+        // verify that the count was stored
+        Integer vfcount = context.getProperty(VF_COUNT_KEY);
+        assertEquals(VF_COUNT, vfcount);
+        assertEquals(VF_COUNT.intValue(), oper.getVfCount());
+
+        // change the count and then verify that it isn't overwritten by another call
+        oper.setVfCount(VF_COUNT + 1);
+
+        assertNull(oper.obtainVfCount());
+        vfcount = context.getProperty(VF_COUNT_KEY);
+        assertEquals(VF_COUNT + 1, vfcount.intValue());
+        assertEquals(VF_COUNT + 1, oper.getVfCount());
+    }
+
+    /**
+     * Tests obtainVfCount() when it actually has to query.
+     */
+    @Test
+    public void testObtainVfCountQuery() throws Exception {
+        CompletableFuture<OperationOutcome> future2 = oper.obtainVfCount();
+        assertNotNull(future2);
+        assertTrue(executor.runAll(100));
+
+        // not done yet
+        assertFalse(future2.isDone());
+
+        provideCqResponse(makeCqResponse());
 
-        CompletableFuture<OperationOutcome> future2 = oper.storeVfCountRunGuard();
         assertTrue(executor.runAll(100));
         assertTrue(future2.isDone());
-        assertEquals(PolicyResult.FAILURE, future2.get().getResult());
+        assertEquals(PolicyResult.SUCCESS, future2.get().getResult());
 
         // verify that the count was stored
-        Integer vfcount = context.getProperty(SoConstants.CONTEXT_KEY_VF_COUNT);
+        Integer vfcount = context.getProperty(VF_COUNT_KEY);
         assertEquals(VF_COUNT, vfcount);
+
+        // repeat - shouldn't need to do anything now
+        assertNull(oper.obtainVfCount());
     }
 
     @Test
@@ -273,9 +302,8 @@ public class SoOperationTest extends BasicSoOperation {
 
         // try with null target
         params = params.toBuilder().target(null).build();
-        oper = new SoOperation(params, config) {};
-
-        assertThatIllegalArgumentException().isThrownBy(() -> oper.prepareSoModelInfo()).withMessage("missing Target");
+        assertThatIllegalArgumentException().isThrownBy(() -> new SoOperation(params, config) {})
+                        .withMessageContaining("missing Target");
     }
 
     private void verifyMissingModelInfo(Supplier<String> getter, Consumer<String> setter) {
@@ -297,8 +325,7 @@ public class SoOperationTest extends BasicSoOperation {
     @Test
     public void testBuildRequestParameters() throws CoderException {
         // valid data
-        SoRequestParameters reqParams = oper.buildRequestParameters();
-        verifyRequest("reqparams.json", reqParams);
+        verifyRequest("reqparams.json", oper.buildRequestParameters().get());
 
         // invalid json
         params.getPayload().put(SoOperation.REQ_PARAM_NM, "{invalid json");
@@ -307,19 +334,18 @@ public class SoOperationTest extends BasicSoOperation {
 
         // missing data
         params.getPayload().remove(SoOperation.REQ_PARAM_NM);
-        assertNull(oper.buildRequestParameters());
+        assertTrue(oper.buildRequestParameters().isEmpty());
 
         // null payload
         params = params.toBuilder().payload(null).build();
         oper = new SoOperation(params, config) {};
-        assertNull(oper.buildRequestParameters());
+        assertTrue(oper.buildRequestParameters().isEmpty());
     }
 
     @Test
     public void testBuildConfigurationParameters() {
         // valid data
-        List<Map<String, String>> result = oper.buildConfigurationParameters();
-        assertEquals(List.of(Collections.emptyMap()), result);
+        assertEquals(List.of(Collections.emptyMap()), oper.buildConfigurationParameters().get());
 
         // invalid json
         params.getPayload().put(SoOperation.CONFIG_PARAM_NM, "{invalid json");
@@ -328,12 +354,12 @@ public class SoOperationTest extends BasicSoOperation {
 
         // missing data
         params.getPayload().remove(SoOperation.CONFIG_PARAM_NM);
-        assertNull(oper.buildConfigurationParameters());
+        assertTrue(oper.buildConfigurationParameters().isEmpty());
 
         // null payload
         params = params.toBuilder().payload(null).build();
         oper = new SoOperation(params, config) {};
-        assertNull(oper.buildConfigurationParameters());
+        assertTrue(oper.buildConfigurationParameters().isEmpty());
     }
 
     @Test
diff --git a/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleCreateTest.java b/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleCreateTest.java
index 63cf744c1..98c645b16 100644
--- a/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleCreateTest.java
+++ b/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleCreateTest.java
@@ -83,8 +83,8 @@ public class VfModuleCreateTest extends BasicSoOperation {
 
     @Test
     public void testStartPreprocessorAsync() throws Exception {
-        // put the count in the context so that it will skip the custom query
-        params.getContext().setProperty(SoConstants.CONTEXT_KEY_VF_COUNT, 20);
+        // insert CQ data so it's there for the check
+        context.setProperty(AaiCqResponse.CONTEXT_KEY, makeCqResponse());
 
         AtomicBoolean guardStarted = new AtomicBoolean();
 
@@ -119,7 +119,7 @@ public class VfModuleCreateTest extends BasicSoOperation {
     @Test
     public void testMakeGuardPayload() {
         final int origCount = 30;
-        params.getContext().setProperty(SoConstants.CONTEXT_KEY_VF_COUNT, origCount);
+        oper.setVfCount(origCount);
 
         CompletableFuture<OperationOutcome> future2 = oper.startPreprocessorAsync();
         assertTrue(executor.runAll(100));
@@ -148,7 +148,7 @@ public class VfModuleCreateTest extends BasicSoOperation {
     @Test
     public void testStartOperationAsync_testSuccessfulCompletion() throws Exception {
         final int origCount = 30;
-        params.getContext().setProperty(SoConstants.CONTEXT_KEY_VF_COUNT, origCount);
+        oper.setVfCount(origCount);
 
         when(client.post(any(), any(), any(), any())).thenAnswer(provideResponse(rawResponse));
 
@@ -167,8 +167,7 @@ public class VfModuleCreateTest extends BasicSoOperation {
         outcome = future2.get(500, TimeUnit.SECONDS);
         assertEquals(PolicyResult.SUCCESS, outcome.getResult());
 
-        Integer newCount = (Integer) params.getContext().getProperty(SoConstants.CONTEXT_KEY_VF_COUNT);
-        assertEquals(origCount + 1, newCount.intValue());
+        assertEquals(origCount + 1, oper.getVfCount());
     }
 
     /**