From 467247c7970f9ae83464d78929ed970bbf03c593 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Thu, 20 Feb 2020 09:40:14 -0500 Subject: More actor clean-up Currently, Operator classes refer to Operation classes, and vice versa, creating a dependency cycle. In addition, there is a slight problem in that if an operator is reconfigured, any running operation may get inconsistent configuration data. Modified the code to create Config objects that are passed to the operation, instead of passing the Operator to the operations. This solved both issues. Replaceed makeOperator() with constructors. Added parameter type to HttpActors. Modified guard to get "ONAP" properties from its configuration, as a default. Changed setUp() to setUpBasic(), so "throws Exception" could be removed, thus resolving a sonar issue. Issue-ID: POLICY-1625 Signed-off-by: Jim Hahn Change-Id: I21eb8798acfbc636ff1bd8741b21c7278365b6e4 --- .../test/BasicBidirectionalTopicOperation.java | 42 +++++++--------------- .../controlloop/actor/test/BasicHttpOperation.java | 30 ++++++---------- .../controlloop/actor/test/BasicOperation.java | 2 +- .../test/BasicBidirectionalTopicOperationTest.java | 24 +++++-------- .../actor/test/BasicHttpOperationTest.java | 20 ++++------- .../controlloop/actor/test/BasicOperationTest.java | 2 +- 6 files changed, 42 insertions(+), 78 deletions(-) (limited to 'models-interactions/model-actors/actor.test/src') diff --git a/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperation.java b/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperation.java index e10e94142..bba2bc13a 100644 --- a/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperation.java +++ b/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperation.java @@ -26,12 +26,9 @@ import java.util.function.BiConsumer; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; import org.onap.policy.common.utils.coder.CoderException; import org.onap.policy.common.utils.coder.StandardCoderObject; -import org.onap.policy.common.utils.time.PseudoExecutor; -import org.onap.policy.controlloop.actorserviceprovider.impl.BidirectionalTopicOperator; -import org.onap.policy.controlloop.actorserviceprovider.parameters.BidirectionalTopicParams; +import org.onap.policy.controlloop.actorserviceprovider.parameters.BidirectionalTopicConfig; import org.onap.policy.controlloop.actorserviceprovider.topic.BidirectionalTopicHandler; import org.onap.policy.controlloop.actorserviceprovider.topic.Forwarder; @@ -41,7 +38,8 @@ import org.onap.policy.controlloop.actorserviceprovider.topic.Forwarder; public class BasicBidirectionalTopicOperation extends BasicOperation { protected static final String MY_SINK = "my-sink"; protected static final String MY_SOURCE = "my-source"; - protected static final int TIMEOUT = 10; + protected static final int TIMEOUT_SEC = 10; + protected static final long TIMEOUT_MS = 1000L * TIMEOUT_SEC; @Captor protected ArgumentCaptor> listenerCaptor; @@ -51,9 +49,7 @@ public class BasicBidirectionalTopicOperation extends BasicOperation { @Mock protected Forwarder forwarder; @Mock - protected BidirectionalTopicOperator operator; - - protected BidirectionalTopicParams topicParams; + protected BidirectionalTopicConfig config; /** * Constructs the object using a default actor and operation name. @@ -75,31 +71,19 @@ public class BasicBidirectionalTopicOperation extends BasicOperation { /** * Initializes mocks and sets up. */ - public void setUp() throws Exception { - MockitoAnnotations.initMocks(this); - - executor = new PseudoExecutor(); - - makeContext(); - - outcome = params.makeOutcome(); - topicParams = BidirectionalTopicParams.builder().sinkTopic(MY_SINK).sourceTopic(MY_SOURCE).timeoutSec(TIMEOUT) - .build(); - - initOperator(); + @Override + public void setUpBasic() { + super.setUpBasic(); + initConfig(); } /** - * Initializes an operator so that it is "alive" and has the given names. + * Initializes a configuration. */ - protected void initOperator() { - when(operator.isAlive()).thenReturn(true); - when(operator.getFullName()).thenReturn(actorName + "." + operationName); - when(operator.getActorName()).thenReturn(actorName); - when(operator.getName()).thenReturn(operationName); - when(operator.getTopicHandler()).thenReturn(topicHandler); - when(operator.getForwarder()).thenReturn(forwarder); - when(operator.getParams()).thenReturn(topicParams); + protected void initConfig() { + when(config.getTopicHandler()).thenReturn(topicHandler); + when(config.getForwarder()).thenReturn(forwarder); + when(config.getTimeoutMs()).thenReturn(TIMEOUT_MS); } /** diff --git a/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicHttpOperation.java b/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicHttpOperation.java index 05fa2260b..e803df8c9 100644 --- a/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicHttpOperation.java +++ b/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicHttpOperation.java @@ -33,7 +33,7 @@ import org.mockito.Mock; import org.mockito.stubbing.Answer; import org.onap.policy.common.endpoints.http.client.HttpClient; import org.onap.policy.common.endpoints.http.client.HttpClientFactory; -import org.onap.policy.controlloop.actorserviceprovider.impl.HttpOperator; +import org.onap.policy.controlloop.actorserviceprovider.parameters.HttpConfig; /** * Superclass for various HttpOperation tests. @@ -47,25 +47,20 @@ public class BasicHttpOperation extends BasicOperation { @Captor protected ArgumentCaptor> callbackCaptor; - @Captor protected ArgumentCaptor> requestCaptor; - @Captor protected ArgumentCaptor> headerCaptor; + @Mock + protected HttpConfig config; @Mock protected HttpClient client; - @Mock protected HttpClientFactory factory; - @Mock protected Response rawResponse; - @Mock - protected HttpOperator operator; - /** * Constructs the object using a default actor and operation name. @@ -87,8 +82,9 @@ public class BasicHttpOperation extends BasicOperation { /** * Initializes mocks and sets up. */ - public void setUp() throws Exception { - super.setUp(); + @Override + public void setUpBasic() { + super.setUpBasic(); when(factory.get(MY_CLIENT)).thenReturn(client); @@ -96,19 +92,15 @@ public class BasicHttpOperation extends BasicOperation { when(client.getBaseUrl()).thenReturn(BASE_URI); - initOperator(); + initConfig(); } /** - * Initializes an operator so that it is "alive" and has the given names. + * Initializes a configuration. */ - protected void initOperator() { - when(operator.isAlive()).thenReturn(true); - when(operator.getFullName()).thenReturn(actorName + "." + operationName); - when(operator.getActorName()).thenReturn(actorName); - when(operator.getName()).thenReturn(operationName); - when(operator.getClient()).thenReturn(client); - when(operator.getPath()).thenReturn(PATH); + protected void initConfig() { + when(config.getClient()).thenReturn(client); + when(config.getPath()).thenReturn(PATH); } /** diff --git a/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicOperation.java b/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicOperation.java index dbdd553a9..f027c1c18 100644 --- a/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicOperation.java +++ b/models-interactions/model-actors/actor.test/src/main/java/org/onap/policy/controlloop/actor/test/BasicOperation.java @@ -86,7 +86,7 @@ public class BasicOperation { /** * Initializes mocks and sets up. */ - public void setUp() throws Exception { + public void setUpBasic() { MockitoAnnotations.initMocks(this); future = new CompletableFuture<>(); diff --git a/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperationTest.java b/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperationTest.java index 9ec118e69..101f130a3 100644 --- a/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperationTest.java +++ b/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicBidirectionalTopicOperationTest.java @@ -24,7 +24,6 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; @@ -54,14 +53,14 @@ public class BasicBidirectionalTopicOperationTest { MockitoAnnotations.initMocks(this); oper = new BasicBidirectionalTopicOperation(ACTOR, OPERATION); - oper.setUp(); + oper.setUpBasic(); } @Test public void testBasicBidirectionalTopicOperation() { oper = new BasicBidirectionalTopicOperation(); - assertEquals(BasicHttpOperation.DEFAULT_ACTOR, oper.actorName); - assertEquals(BasicHttpOperation.DEFAULT_OPERATION, oper.operationName); + assertEquals(BasicOperation.DEFAULT_ACTOR, oper.actorName); + assertEquals(BasicOperation.DEFAULT_OPERATION, oper.operationName); } @Test @@ -72,24 +71,19 @@ public class BasicBidirectionalTopicOperationTest { @Test public void testSetUp() { - assertNotNull(oper.topicParams); + assertNotNull(oper.config); assertNotNull(oper.context); assertNotNull(oper.outcome); assertNotNull(oper.executor); - assertTrue(oper.operator.isAlive()); } @Test public void testInitOperator() { - oper.initOperator(); - - assertTrue(oper.operator.isAlive()); - assertEquals(ACTOR + "." + OPERATION, oper.operator.getFullName()); - assertEquals(ACTOR, oper.operator.getActorName()); - assertEquals(OPERATION, oper.operator.getName()); - assertSame(oper.topicHandler, oper.operator.getTopicHandler()); - assertSame(oper.forwarder, oper.operator.getForwarder()); - assertSame(oper.topicParams, oper.operator.getParams()); + oper.initConfig(); + + assertSame(oper.topicHandler, oper.config.getTopicHandler()); + assertSame(oper.forwarder, oper.config.getForwarder()); + assertEquals(BasicBidirectionalTopicOperation.TIMEOUT_MS, oper.config.getTimeoutMs()); } @Test diff --git a/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicHttpOperationTest.java b/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicHttpOperationTest.java index c463d6d49..c19aa487c 100644 --- a/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicHttpOperationTest.java +++ b/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicHttpOperationTest.java @@ -23,7 +23,6 @@ package org.onap.policy.controlloop.actor.test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -42,14 +41,14 @@ public class BasicHttpOperationTest { @Before public void setUp() throws Exception { oper = new BasicHttpOperation<>(ACTOR, OPERATION); - oper.setUp(); + oper.setUpBasic(); } @Test public void testBasicHttpOperation() { oper = new BasicHttpOperation<>(); - assertEquals(BasicHttpOperation.DEFAULT_ACTOR, oper.actorName); - assertEquals(BasicHttpOperation.DEFAULT_OPERATION, oper.operationName); + assertEquals(BasicOperation.DEFAULT_ACTOR, oper.actorName); + assertEquals(BasicOperation.DEFAULT_OPERATION, oper.operationName); } @Test @@ -68,19 +67,14 @@ public class BasicHttpOperationTest { assertNotNull(oper.context); assertNotNull(oper.outcome); assertNotNull(oper.executor); - assertTrue(oper.operator.isAlive()); } @Test public void testInitOperator() throws Exception { - oper.initOperator(); - - assertTrue(oper.operator.isAlive()); - assertEquals(ACTOR + "." + OPERATION, oper.operator.getFullName()); - assertEquals(ACTOR, oper.operator.getActorName()); - assertEquals(OPERATION, oper.operator.getName()); - assertSame(oper.client, oper.operator.getClient()); - assertEquals(BasicHttpOperation.PATH, oper.operator.getPath()); + oper.initConfig(); + + assertSame(oper.client, oper.config.getClient()); + assertEquals(BasicHttpOperation.PATH, oper.config.getPath()); } @Test diff --git a/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicOperationTest.java b/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicOperationTest.java index ee0e5cee4..60bb00892 100644 --- a/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicOperationTest.java +++ b/models-interactions/model-actors/actor.test/src/test/java/org/onap/policy/controlloop/actor/test/BasicOperationTest.java @@ -43,7 +43,7 @@ public class BasicOperationTest { @Before public void setUp() throws Exception { oper = new BasicHttpOperation<>(ACTOR, OPERATION); - oper.setUp(); + oper.setUpBasic(); } @Test -- cgit 1.2.3-korg