Handle duplicate events in drools-apps

Modified the frankfurt rules to compare the event objects instead of
comparing the request ID when determining if an event is new.  The
event object's equals() method ignores the request ID when doing the
comparison, thus it will treat an event as a duplicate even if the
request ID is different, which is the behavior we want.

Also removed the @Ignore from the junit that tests for duplicate
events in the hope that this change will fix it.  If the docker build
still breaks, then @Ignore can be added back in.

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

5 files changed, 43 insertions, 16 deletions

diff --git a/controlloop/common/controller-frankfurt/src/main/resources/frankfurt.drl b/controlloop/common/controller-frankfurt/src/main/resources/frankfurt.drl
index f54786da9..c42131318 100644
--- a/controlloop/common/controller-frankfurt/src/main/resources/frankfurt.drl
+++ b/controlloop/common/controller-frankfurt/src/main/resources/frankfurt.drl
@@ -100,7 +100,7 @@ rule "EVENT"
         $params : ControlLoopParams( $clName : getClosedLoopControlName() )
         $event : CanonicalOnset( closedLoopControlName == $clName )
         not ( ControlLoopEventManager2( closedLoopControlName == $event.getClosedLoopControlName(),
-            requestId == $event.getRequestId() ) )
+            getContext().getEvent() == $event ) )
     then
 
     Logger logger = LoggerFactory.getLogger(drools.getRule().getPackage());
@@ -173,7 +173,7 @@ rule "EVENT.MANAGER.NEW.EVENT"
     when
         $event : VirtualControlLoopEvent( )
         $manager : ControlLoopEventManager2( closedLoopControlName == $event.getClosedLoopControlName(),
-            requestId == $event.getRequestId() )
+            getContext().getEvent() == $event )
     then
 
     Logger logger = LoggerFactory.getLogger(drools.getRule().getPackage());
@@ -202,6 +202,8 @@ rule "EVENT.MANAGER.NEW.EVENT"
             //
             // TODO: handle the abatement.  Currently, it's just discarded.
             //
+            logger.info("{}: {}.{}: abatement",
+                $manager.getClosedLoopControlName(), $manager.getPolicyName(), drools.getRule().getName());
             break;
 
         case FIRST_ONSET:
diff --git a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2.java b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2.java
index c79737ae6..5f611c098 100644
--- a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2.java
+++ b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2.java
@@ -36,6 +36,7 @@ import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
@@ -97,6 +98,12 @@ public class ControlLoopEventManager2 implements ManagerContext, Serializable {
 
     private static final Set<String> TRUE_VALUES = Set.of("true", "t", "yes", "y");
 
+    /**
+     * Counts the number of these objects that have been created.  This is used by junit
+     * tests.
+     */
+    private static final AtomicLong createCount = new AtomicLong(0);
+
     public enum NewEventStatus {
         FIRST_ONSET, SUBSEQUENT_ONSET, FIRST_ABATEMENT, SUBSEQUENT_ABATEMENT, SYNTAX_ERROR
     }
@@ -114,6 +121,7 @@ public class ControlLoopEventManager2 implements ManagerContext, Serializable {
     @Getter
     @ToString.Include
     private final UUID requestId;
+    @Getter
     private final ControlLoopEventContext context;
     @ToString.Include
     private int numOnsets = 1;
@@ -169,6 +177,8 @@ public class ControlLoopEventManager2 implements ManagerContext, Serializable {
     public ControlLoopEventManager2(ControlLoopParams params, VirtualControlLoopEvent event, WorkingMemory workMem)
                     throws ControlLoopException {
 
+        createCount.incrementAndGet();
+
         checkEventSyntax(event);
 
         if (isClosedLoopDisabled(event)) {
@@ -192,6 +202,14 @@ public class ControlLoopEventManager2 implements ManagerContext, Serializable {
     }
 
     /**
+     * Gets the number of managers objects that have been created.
+     * @return the number of managers objects that have been created
+     */
+    public static long getCreateCount() {
+        return createCount.get();
+    }
+
+    /**
      * Starts the manager.
      *
      * @throws ControlLoopException if the processor cannot get a policy
diff --git a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java
index 171a2acea..0baac9e8c 100644
--- a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java
+++ b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java
@@ -29,8 +29,6 @@ import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import lombok.AccessLevel;
 import lombok.Getter;
-
-import org.junit.Ignore;
 import org.junit.Test;
 import org.onap.policy.appc.Request;
 import org.onap.policy.appclcm.AppcLcmDmaapWrapper;
@@ -39,6 +37,7 @@ import org.onap.policy.common.utils.coder.StandardCoder;
 import org.onap.policy.common.utils.coder.StandardCoderInstantAsMillis;
 import org.onap.policy.controlloop.ControlLoopNotificationType;
 import org.onap.policy.controlloop.VirtualControlLoopNotification;
+import org.onap.policy.controlloop.eventmanager.ControlLoopEventManager2;
 import org.onap.policy.drools.system.PolicyController;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
 import org.onap.policy.sdnr.PciMessage;
@@ -245,9 +244,6 @@ public abstract class BaseRuleTest {
      * to obtain a lock since it is a different target. After processing of all events
      * there should only be the policy and params objects left in memory.
      */
-    // Ignoring test due to TimeoutException (for some reason this test fails only on docker build)
-    // https://jenkins.onap.org/job/policy-drools-applications-maven-docker-stage-master/294/
-    @Ignore
     @Test
     public void testDuplicatesEvents() {
         policyClMgt = topics.createListener(POLICY_CL_MGT_TOPIC, VirtualControlLoopNotification.class, controller);
@@ -257,6 +253,8 @@ public abstract class BaseRuleTest {
         policy = rules.setupPolicyFromFile(DUPLICATES_TOSCA_COMPLIANT_POLICY);
         assertEquals(2, controller.getDrools().factCount(rules.getControllerName()));
 
+        final long initCount = getCreateCount();
+
         /*
          * Inject ONSET events over the DCAE topic. First and last have the same target
          * entity, but different request IDs - only one should succeed. The middle one is
@@ -266,9 +264,6 @@ public abstract class BaseRuleTest {
         topics.inject(DCAE_TOPIC, DUPLICATES_ONSET_2);
         topics.inject(DCAE_TOPIC, DUPLICATES_ONSET_1, UUID.randomUUID().toString());
 
-        // one should immediately generate a FINAL failure
-        waitForFinal(policy, policyClMgt, ControlLoopNotificationType.FINAL_FAILURE);
-
         // should see two restarts
         for (int count = 0; count < 2; ++count) {
             AppcLcmDmaapWrapper appcreq = appcLcmRead.await(req -> APPC_RESTART_OP.equals(req.getRpcName()));
@@ -287,6 +282,9 @@ public abstract class BaseRuleTest {
                         .sorted().collect(Collectors.toList());
 
         assertEquals(List.of("duplicate-VNF", "vCPE_Infrastructure_vGMUX_demo_app").toString(), actual.toString());
+
+        long added = getCreateCount() - initCount;
+        assertEquals(2, added);
     }
 
     // VCPE
@@ -649,6 +647,10 @@ public abstract class BaseRuleTest {
         waitForFinalSuccess(policy, policyClMgt);
     }
 
+    protected long getCreateCount() {
+        return ControlLoopEventManager2.getCreateCount();
+    }
+
     /**
      * Waits for a OPERATION SUCCESS transaction notification.
      */
diff --git a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTestTest.java b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTestTest.java
index 0753aacd3..cfa9c49eb 100644
--- a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTestTest.java
+++ b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTestTest.java
@@ -25,6 +25,7 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -32,6 +33,7 @@ import static org.mockito.Mockito.when;
 import java.util.LinkedList;
 import java.util.Map;
 import java.util.Queue;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
@@ -262,18 +264,22 @@ public class BaseRuleTestTest {
 
     @Test
     public void testTestDuplicatesEvents() {
+        // the test expects the count to be incremented by 2 between calls
+        AtomicLong count = new AtomicLong(5);
+        base = spy(base);
+        when(base.getCreateCount()).thenAnswer(args -> count.getAndAdd(2));
+
         enqueueAppcLcm("restart", "restart");
-        enqueueClMgt(ControlLoopNotificationType.FINAL_FAILURE);
         enqueueClMgt(ControlLoopNotificationType.FINAL_SUCCESS);
         enqueueClMgt(ControlLoopNotificationType.FINAL_SUCCESS);
 
-        clMgtQueue.get(1).setAai(Map.of("generic-vnf.vnf-id", "duplicate-VNF"));
-        clMgtQueue.get(2).setAai(Map.of("generic-vnf.vnf-id", "vCPE_Infrastructure_vGMUX_demo_app"));
+        clMgtQueue.get(0).setAai(Map.of("generic-vnf.vnf-id", "duplicate-VNF"));
+        clMgtQueue.get(1).setAai(Map.of("generic-vnf.vnf-id", "vCPE_Infrastructure_vGMUX_demo_app"));
 
         base.testDuplicatesEvents();
 
         assertEquals(0, permitCount);
-        assertEquals(3, finalCount);
+        assertEquals(2, finalCount);
 
         assertTrue(appcLcmQueue.isEmpty());
         assertTrue(clMgtQueue.isEmpty());
diff --git a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest.java b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest.java
index 6ee526e4f..fea440541 100644
--- a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest.java
+++ b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest.java
@@ -65,8 +65,7 @@ public class NamedRunnerTest {
      */
     @Test
     @Ignore
-    @SuppressWarnings("java:S1607")
-    public void testIgnore() {
+    public void testIgnore() {      // NOSONAR
         fail("should not run");
     }