From 433f782f16f43a8431d8d91c2a055fd2a0603f42 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 25 Feb 2020 12:29:08 -0500 Subject: [PATCH] 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 Change-Id: Ia23eabea0edf6857372e269a2db1a21e741824c6 --- .../policy/controlloop/actor/so/SoConstants.java | 2 +- .../policy/controlloop/actor/so/SoOperation.java | 87 ++++++++++++++++------ .../controlloop/actor/so/VfModuleCreate.java | 19 ++--- .../controlloop/actor/so/SoOperationTest.java | 80 +++++++++++++------- .../controlloop/actor/so/VfModuleCreateTest.java | 11 ++- 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 { 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 { 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 { * 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 { } /** - * 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 storeVfCountRunGuard() { - String custId = params.getTarget().getModelCustomizationId(); - String invId = params.getTarget().getModelInvariantId(); - String verId = params.getTarget().getModelVersionId(); + @SuppressWarnings("unchecked") + protected CompletableFuture 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 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 { /** * Builds the request parameters from the policy payload. */ - protected SoRequestParameters buildRequestParameters() { + protected Optional 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 { /** * Builds the configuration parameters from the policy payload. */ - protected List> buildConfigurationParameters() { + protected Optional>> 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> 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 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 makeGuardPayload() { Map 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 getter, Consumer 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 future2 = oper.obtainVfCount(); + assertNotNull(future2); + assertTrue(executor.runAll(100)); + + // not done yet + assertFalse(future2.isDone()); + + provideCqResponse(makeCqResponse()); - CompletableFuture 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 getter, Consumer 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> 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 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()); } /** -- 2.16.6