New Guard actor request structure is incorrect
Missing various fields within the request structure (e.g.,
ONAPName). Fixed.
In the process, also modified makeGuardPayload() so that it
only constructs the inner "guard" JSON object, making it easier
for invoking code to modify it.
Issue-ID: POLICY-2434
Signed-off-by: Jim Hahn <jrh3@att.com>
Change-Id: I7d34a279845bb98425caf66565eab7513d310815
diff --git a/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardConfig.java b/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardConfig.java
index 0e711d1..7dca3bf 100644
--- a/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardConfig.java
+++ b/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardConfig.java
@@ -20,19 +20,18 @@
package org.onap.policy.controlloop.actor.guard;
-import java.util.LinkedHashMap;
-import java.util.Map;
import java.util.concurrent.Executor;
import lombok.Getter;
import org.onap.policy.common.endpoints.http.client.HttpClient;
import org.onap.policy.common.endpoints.http.client.HttpClientFactory;
import org.onap.policy.controlloop.actorserviceprovider.parameters.HttpConfig;
+import org.onap.policy.models.decisions.concepts.DecisionRequest;
/**
* Configuration for Guard Operators.
*/
public class GuardConfig extends HttpConfig {
- private final Map<String, Object> defaultRequest = new LinkedHashMap<>();
+ private final DecisionRequest defaultRequest = new DecisionRequest();
/**
* {@code True} if the associated guard operation is disabled.
@@ -50,32 +49,20 @@
public GuardConfig(Executor blockingExecutor, GuardParams params, HttpClientFactory clientFactory) {
super(blockingExecutor, params, clientFactory);
- addProperty("ONAPComponent", params.getOnapComponent());
- addProperty("ONAPInstance", params.getOnapInstance());
- addProperty("ONAPName", params.getOnapName());
- addProperty("action", params.getAction());
+ defaultRequest.setOnapComponent(params.getOnapComponent());
+ defaultRequest.setOnapInstance(params.getOnapInstance());
+ defaultRequest.setOnapName(params.getOnapName());
+ defaultRequest.setAction(params.getAction());
this.disabled = params.isDisabled();
}
/**
- * Adds a property to the default request, if the value is not {@code null}.
- *
- * @param key property key
- * @param value property value, or {@code null}
- */
- private void addProperty(String key, String value) {
- if (value != null) {
- defaultRequest.put(key, value);
- }
- }
-
- /**
* Creates a new request, with the default values.
*
- * @return a new request map
+ * @return a new request
*/
- public Map<String, Object> makeRequest() {
- return new LinkedHashMap<>(defaultRequest);
+ public DecisionRequest makeRequest() {
+ return new DecisionRequest(defaultRequest);
}
}
diff --git a/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardOperation.java b/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardOperation.java
index b9dfb9e..edd2e23 100644
--- a/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardOperation.java
+++ b/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardOperation.java
@@ -31,7 +31,6 @@
import org.onap.policy.common.endpoints.utils.NetLoggerUtil.EventType;
import org.onap.policy.controlloop.actorserviceprovider.CallbackManager;
import org.onap.policy.controlloop.actorserviceprovider.OperationOutcome;
-import org.onap.policy.controlloop.actorserviceprovider.Util;
import org.onap.policy.controlloop.actorserviceprovider.impl.HttpOperation;
import org.onap.policy.controlloop.actorserviceprovider.impl.OperationPartial;
import org.onap.policy.controlloop.actorserviceprovider.parameters.ControlLoopOperationParams;
@@ -109,8 +108,7 @@
protected CompletableFuture<OperationOutcome> startOperationAsync(int attempt, OperationOutcome outcome) {
outcome.setSubRequestId(String.valueOf(attempt));
- DecisionRequest request = Util.translate(getName(), makeRequest(), DecisionRequest.class);
-
+ DecisionRequest request = makeRequest();
Entity<DecisionRequest> entity = Entity.entity(request, MediaType.APPLICATION_JSON);
Map<String, Object> headers = makeHeaders();
@@ -129,16 +127,16 @@
/**
* Makes a request from the payload.
*
- * @return a new request map
+ * @return a new request
*/
- protected Map<String, Object> makeRequest() {
+ protected DecisionRequest makeRequest() {
if (params.getPayload() == null) {
throw new IllegalArgumentException("missing payload");
}
- Map<String, Object> req = config.makeRequest();
- req.putAll(params.getPayload());
- req.computeIfAbsent("requestId", key -> UUID.randomUUID().toString());
+ DecisionRequest req = config.makeRequest();
+ req.setRequestId(UUID.randomUUID().toString());
+ req.setResource(Map.of("guard", params.getPayload()));
return req;
}
diff --git a/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardParams.java b/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardParams.java
index 5f83603..959b4d3 100644
--- a/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardParams.java
+++ b/models-interactions/model-actors/actor.guard/src/main/java/org/onap/policy/controlloop/actor/guard/GuardParams.java
@@ -23,20 +23,19 @@
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.experimental.SuperBuilder;
+import org.onap.policy.common.parameters.annotations.NotBlank;
+import org.onap.policy.common.parameters.annotations.NotNull;
import org.onap.policy.controlloop.actorserviceprovider.parameters.HttpParams;
/**
- * Default values to be included if not specified in the payload.
+ * Guard parameters.
*/
+@NotBlank
+@NotNull
@Data
@EqualsAndHashCode(callSuper = true)
@SuperBuilder(toBuilder = true)
public class GuardParams extends HttpParams {
-
- /*
- * Optional, default values that are used if missing from the payload.
- */
-
private String onapName;
private String onapComponent;
private String onapInstance;
diff --git a/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardConfigTest.java b/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardConfigTest.java
index 49c1c91..d30c711 100644
--- a/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardConfigTest.java
+++ b/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardConfigTest.java
@@ -33,7 +33,6 @@
import org.mockito.MockitoAnnotations;
import org.onap.policy.common.endpoints.http.client.HttpClient;
import org.onap.policy.common.endpoints.http.client.HttpClientFactory;
-import org.onap.policy.controlloop.actorserviceprovider.Util;
import org.onap.policy.models.decisions.concepts.DecisionRequest;
public class GuardConfigTest {
@@ -77,7 +76,7 @@
expected.setOnapName(ONAP_NAME);
expected.setAction(MY_ACTION);
- DecisionRequest actual = Util.translate("", config.makeRequest(), DecisionRequest.class);
+ DecisionRequest actual = config.makeRequest();
assertEquals(expected, actual);
// check value from superclass
@@ -89,7 +88,7 @@
config = new GuardConfig(executor, params, factory);
assertFalse(config.isDisabled());
- actual = Util.translate("", config.makeRequest(), DecisionRequest.class);
+ actual = config.makeRequest();
assertEquals(new DecisionRequest(), actual);
// try with disabled=true
diff --git a/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardOperationTest.java b/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardOperationTest.java
index be40ef3..13a2411 100644
--- a/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardOperationTest.java
+++ b/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardOperationTest.java
@@ -23,6 +23,7 @@
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.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
@@ -63,7 +64,14 @@
super.setUpBasic();
guardConfig = mock(GuardConfig.class);
- when(guardConfig.makeRequest()).thenAnswer(args -> new TreeMap<>(Map.of("action", "guard")));
+ when(guardConfig.makeRequest()).thenAnswer(args -> {
+ DecisionRequest req = new DecisionRequest();
+ req.setAction("guard");
+ req.setOnapComponent("my-onap-component");
+ req.setOnapInstance("my-onap-instance");
+ req.setOnapName("my-onap-name");
+ return req;
+ });
config = guardConfig;
initConfig();
@@ -128,19 +136,6 @@
verifyPayload("makeReqStd.json", makePayload());
verifyPayload("makeReqDefault.json", new TreeMap<>());
- Map<String, Object> payload = new TreeMap<>();
- payload.put("action", "some action");
- payload.put("hello", "world");
- payload.put("r u there?", "yes");
- payload.put("requestId", "some request id");
-
- Map<String, Object> resource = new TreeMap<>();
- payload.put("resource", resource);
- resource.put("abc", "def");
- resource.put("ghi", "jkl");
-
- verifyPayload("makeReq.json", payload);
-
// null payload - start with fresh parameters and operation
params = params.toBuilder().payload(null).build();
oper = new GuardOperation(params, config);
@@ -151,9 +146,16 @@
params.getPayload().clear();
params.getPayload().putAll(payload);
- Map<String, Object> requestMap = oper.makeRequest();
+ DecisionRequest request = oper.makeRequest();
- verifyRequest(expectedJsonFile, requestMap, "requestId");
+ assertEquals("guard", request.getAction());
+ assertEquals("my-onap-component", request.getOnapComponent());
+ assertEquals("my-onap-instance", request.getOnapInstance());
+ assertEquals("my-onap-name", request.getOnapName());
+ assertNotNull(request.getRequestId());
+ assertEquals(Map.of("guard", payload), request.getResource());
+
+ verifyRequest(expectedJsonFile, request, "requestId");
}
@Test
@@ -189,22 +191,6 @@
@Override
protected Map<String, Object> makePayload() {
- DecisionRequest req = new DecisionRequest();
- req.setAction("my-action");
- req.setOnapComponent("my-onap-component");
- req.setOnapInstance("my-onap-instance");
- req.setOnapName("my-onap-name");
- req.setRequestId("my-request-id");
-
- // add resources
- Map<String, Object> resource = new TreeMap<>();
- req.setResource(resource);
- resource.put("actor", "resource-actor");
- resource.put("operation", "resource-operation");
-
- @SuppressWarnings("unchecked")
- Map<String, Object> map = Util.translate("", req, TreeMap.class);
-
- return map;
+ return new TreeMap<>(Map.of("hello", "world", "abc", "123"));
}
}
diff --git a/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardParamsTest.java b/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardParamsTest.java
index 1723683..102dcbb 100644
--- a/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardParamsTest.java
+++ b/models-interactions/model-actors/actor.guard/src/test/java/org/onap/policy/controlloop/actor/guard/GuardParamsTest.java
@@ -60,12 +60,13 @@
public void testValidate() {
assertTrue(params.validate(CONTAINER).isValid());
+ testValidateField("onapName", "null", bldr -> bldr.onapName(null));
+ testValidateField("onapComponent", "null", bldr -> bldr.onapComponent(null));
+ testValidateField("onapInstance", "null", bldr -> bldr.onapInstance(null));
+ testValidateField("action", "null", bldr -> bldr.action(null));
+
// validate one of the superclass fields
testValidateField("clientName", "null", bldr -> bldr.clientName(null));
-
- // validate with mostly empty params
- params = GuardParams.builder().clientName(CLIENT).path(PATH).timeoutSec(TIMEOUT).build();
- assertTrue(params.validate(CONTAINER).isValid());
}
@Test
diff --git a/models-interactions/model-actors/actor.guard/src/test/resources/makeReq.json b/models-interactions/model-actors/actor.guard/src/test/resources/makeReq.json
deleted file mode 100644
index 2716d03..0000000
--- a/models-interactions/model-actors/actor.guard/src/test/resources/makeReq.json
+++ /dev/null
@@ -1,10 +0,0 @@
-{
- "action": "some action",
- "hello": "world",
- "r u there?": "yes",
- "requestId": "abcdefghi",
- "resource": {
- "abc": "def",
- "ghi": "jkl"
- }
-}
diff --git a/models-interactions/model-actors/actor.guard/src/test/resources/makeReqDefault.json b/models-interactions/model-actors/actor.guard/src/test/resources/makeReqDefault.json
index d757eb9..a08f13f 100644
--- a/models-interactions/model-actors/actor.guard/src/test/resources/makeReqDefault.json
+++ b/models-interactions/model-actors/actor.guard/src/test/resources/makeReqDefault.json
@@ -1,4 +1,10 @@
{
+ "ONAPName": "my-onap-name",
+ "ONAPComponent": "my-onap-component",
+ "ONAPInstance": "my-onap-instance",
+ "requestId": "abcdefghi",
"action": "guard",
- "requestId": "abcdefghi"
+ "resource": {
+ "guard": {}
+ }
}
diff --git a/models-interactions/model-actors/actor.guard/src/test/resources/makeReqStd.json b/models-interactions/model-actors/actor.guard/src/test/resources/makeReqStd.json
index 6ae8867..fd1f4c7 100644
--- a/models-interactions/model-actors/actor.guard/src/test/resources/makeReqStd.json
+++ b/models-interactions/model-actors/actor.guard/src/test/resources/makeReqStd.json
@@ -1,11 +1,13 @@
{
+ "ONAPName": "my-onap-name",
"ONAPComponent": "my-onap-component",
"ONAPInstance": "my-onap-instance",
- "ONAPName": "my-onap-name",
- "action": "my-action",
"requestId": "abcdefghi",
+ "action": "guard",
"resource": {
- "actor": "resource-actor",
- "operation": "resource-operation"
+ "guard": {
+ "abc": "123",
+ "hello": "world"
+ }
}
-}
\ No newline at end of file
+}
diff --git a/models-interactions/model-actors/actor.guard/src/test/resources/service.yaml b/models-interactions/model-actors/actor.guard/src/test/resources/service.yaml
index 131cf78..998928d 100644
--- a/models-interactions/model-actors/actor.guard/src/test/resources/service.yaml
+++ b/models-interactions/model-actors/actor.guard/src/test/resources/service.yaml
@@ -26,6 +26,10 @@
actors:
GUARD:
clientName: my-client
+ onapName: my-onap-name
+ onapComponent: my-onap-component
+ onapInstance: my-onap-instance
+ action: guard
operations:
Decision:
path: decide
\ No newline at end of file
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 8c084b8..1d5d44c 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
@@ -132,15 +132,7 @@
Map<String, Object> payload = captor.getValue().getPayload();
assertNotNull(payload);
- @SuppressWarnings("unchecked")
- Map<String, Object> resource = (Map<String, Object>) payload.get("resource");
- assertNotNull(resource);
-
- @SuppressWarnings("unchecked")
- Map<String, Object> guard = (Map<String, Object>) resource.get("guard");
- assertNotNull(guard);
-
- Integer newCount = (Integer) guard.get(VfModuleCreate.PAYLOAD_KEY_VF_COUNT);
+ Integer newCount = (Integer) payload.get(VfModuleCreate.PAYLOAD_KEY_VF_COUNT);
assertNotNull(newCount);
assertEquals(origCount + 1, newCount.intValue());
}
diff --git a/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleDeleteTest.java b/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleDeleteTest.java
index c08fa37..cb2bcef 100644
--- a/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleDeleteTest.java
+++ b/models-interactions/model-actors/actor.so/src/test/java/org/onap/policy/controlloop/actor/so/VfModuleDeleteTest.java
@@ -166,15 +166,7 @@
Map<String, Object> payload = captor.getValue().getPayload();
assertNotNull(payload);
- @SuppressWarnings("unchecked")
- Map<String, Object> resource = (Map<String, Object>) payload.get("resource");
- assertNotNull(resource);
-
- @SuppressWarnings("unchecked")
- Map<String, Object> guard = (Map<String, Object>) resource.get("guard");
- assertNotNull(guard);
-
- Integer newCount = (Integer) guard.get(VfModuleDelete.PAYLOAD_KEY_VF_COUNT);
+ Integer newCount = (Integer) payload.get(VfModuleDelete.PAYLOAD_KEY_VF_COUNT);
assertNotNull(newCount);
assertEquals(origCount - 1, newCount.intValue());
}
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 cdbdc2a..a9d7f4e 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
@@ -230,14 +230,7 @@
*/
protected CompletableFuture<OperationOutcome> startGuardAsync() {
// get the guard payload
- Map<String, Object> guardPayload = makeGuardPayload();
-
- // wrap it in a "resource"
- Map<String, Object> resource = new LinkedHashMap<>();
- resource.put("guard", guardPayload);
-
- Map<String, Object> payload = new LinkedHashMap<>();
- payload.put("resource", resource);
+ Map<String, Object> payload = makeGuardPayload();
/*
* Note: can't use constants from actor.guard, because that would create a
@@ -255,7 +248,7 @@
protected Map<String, Object> makeGuardPayload() {
Map<String, Object> guard = new LinkedHashMap<>();
guard.put("actor", params.getActor());
- guard.put("recipe", params.getOperation());
+ guard.put("operation", params.getOperation());
guard.put("target", params.getTargetEntity());
guard.put("requestId", params.getRequestId());
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 b73a3a0..7b8eed5 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
@@ -324,13 +324,7 @@
Map<String, Object> payload = params.getPayload();
assertNotNull(payload);
- @SuppressWarnings("unchecked")
- Map<String, Object> resource = (Map<String, Object>) payload.get("resource");
- assertNotNull(resource);
-
- @SuppressWarnings("unchecked")
- Map<String, Object> guard = (Map<String, Object>) resource.get("guard");
- assertEquals(oper.makeGuardPayload(), guard);
+ assertEquals(oper.makeGuardPayload(), payload);
}
@Test
@@ -341,13 +335,13 @@
// request id changes, so remove it
payload.remove("requestId");
- assertEquals("{actor=my-actor, recipe=my-operation, target=my-entity}", payload.toString());
+ assertEquals("{actor=my-actor, operation=my-operation, target=my-entity}", payload.toString());
// repeat, but with closed loop name
event.setClosedLoopControlName("my-loop");
payload = oper.makeGuardPayload();
payload.remove("requestId");
- assertEquals("{actor=my-actor, recipe=my-operation, target=my-entity, clname=my-loop}", payload.toString());
+ assertEquals("{actor=my-actor, operation=my-operation, target=my-entity, clname=my-loop}", payload.toString());
}
@Test