Updated Lock to avoid starvation

The lock class is made more greedy so all locks are granted in the requested order.
Previously, an exclusive lock could be discriminated.

Issue-ID: CCSDK-3560
Signed-off-by: PatrikBuhr <patrik.buhr@est.tech>
Change-Id: If2dd171409c58eacbccce9569b2f6694e09992a2

3 files changed, 36 insertions, 24 deletions

diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/controllers/v2/ApplicationTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/controllers/v2/ApplicationTest.java
index c203d0fd..75215613 100644
--- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/controllers/v2/ApplicationTest.java
+++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/controllers/v2/ApplicationTest.java
@@ -57,6 +57,7 @@ import org.onap.ccsdk.oran.a1policymanagementservice.configuration.RicConfig;
 import org.onap.ccsdk.oran.a1policymanagementservice.configuration.WebClientConfig;
 import org.onap.ccsdk.oran.a1policymanagementservice.controllers.ServiceCallbackInfo;
 import org.onap.ccsdk.oran.a1policymanagementservice.exceptions.ServiceException;
+import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policies;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policy;
@@ -178,8 +179,8 @@ class ApplicationTest {
     @AfterEach
     void verifyNoRicLocks() {
         for (Ric ric : this.rics.getRics()) {
-            ric.getLock().lockBlocking(LockType.EXCLUSIVE);
-            ric.getLock().unlockBlocking();
+            Lock.Grant grant = ric.getLock().lockBlocking(LockType.EXCLUSIVE, "");
+            grant.unlockBlocking();
             assertThat(ric.getLock().getLockCounter()).isZero();
             assertThat(ric.getState()).isEqualTo(Ric.RicState.AVAILABLE);
         }
@@ -276,7 +277,7 @@ class ApplicationTest {
 
         // Check that a service callback for the AVAILABLE RIC is invoked
         final RappSimulatorController.TestResults receivedCallbacks = rAppSimulator.getTestResults();
-        await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo().size()).isEqualTo(1));
+        await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo()).hasSize(1));
         ServiceCallbackInfo callbackInfo = receivedCallbacks.getReceivedInfo().get(0);
         assertThat(callbackInfo.ricId).isEqualTo(RIC);
         assertThat(callbackInfo.eventType).isEqualTo(ServiceCallbackInfo.EventType.AVAILABLE);
@@ -305,7 +306,7 @@ class ApplicationTest {
         supervision.checkAllRics();
         waitForRicState(RIC, RicState.AVAILABLE);
 
-        await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo().size()).isEqualTo(1));
+        await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo()).hasSize(1));
     }
 
     @Test
@@ -842,7 +843,7 @@ class ApplicationTest {
         waitForRicState("ric1", RicState.AVAILABLE);
 
         final RappSimulatorController.TestResults receivedCallbacks = rAppSimulator.getTestResults();
-        await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo().size()).isEqualTo(1));
+        await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo()).hasSize(1));
         ServiceCallbackInfo callbackInfo = receivedCallbacks.getReceivedInfo().get(0);
         assertThat(callbackInfo.ricId).isEqualTo("ric1");
         assertThat(callbackInfo.eventType).isEqualTo(ServiceCallbackInfo.EventType.AVAILABLE);
diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/repository/LockTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/repository/LockTest.java
index a63f15f1..79879446 100644
--- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/repository/LockTest.java
+++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/repository/LockTest.java
@@ -21,21 +21,28 @@
 package org.onap.ccsdk.oran.a1policymanagementservice.repository;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
 
 import java.io.IOException;
+import java.lang.invoke.MethodHandles;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.onap.ccsdk.oran.a1policymanagementservice.exceptions.ServiceException;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
 @ExtendWith(MockitoExtension.class)
 class LockTest {
 
+    private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
     @SuppressWarnings("squid:S2925") // "Thread.sleep" should not be used in tests.
     private void sleep() {
         try {
@@ -45,10 +52,13 @@ class LockTest {
         }
     }
 
-    private void asynchUnlock(Lock lock) {
+    private void asynchUnlock(Lock.Grant grant, Lock lock) {
+        logger.info("Lock {} cnt: {}, exclusive: {}, queue: {}", grant.getLabel(), lock.getLockCounter(),
+                lock.isExclusive, lock.lockRequestQueue.size());
+
         Thread thread = new Thread(() -> {
             sleep();
-            lock.unlockBlocking();
+            grant.unlockBlocking();
         });
         thread.start();
     }
@@ -56,14 +66,15 @@ class LockTest {
     @Test
     void testLock() throws IOException, ServiceException {
         Lock lock = new Lock();
-        lock.lockBlocking(LockType.SHARED);
-        lock.unlockBlocking();
+        Lock.Grant grant = lock.lockBlocking(LockType.SHARED, "test");
+        grant.unlockBlocking();
+        assertThat(grant.getLabel()).isEqualTo("test");
 
-        lock.lockBlocking(LockType.EXCLUSIVE);
-        asynchUnlock(lock);
+        grant = lock.lockBlocking(LockType.EXCLUSIVE, "");
+        asynchUnlock(grant, lock);
 
-        lock.lockBlocking(LockType.SHARED);
-        lock.unlockBlocking();
+        grant = lock.lockBlocking(LockType.SHARED, "");
+        grant.unlockBlocking();
 
         assertThat(lock.getLockCounter()).isZero();
     }
@@ -72,18 +83,17 @@ class LockTest {
     void testReactiveLock() {
         Lock lock = new Lock();
 
-        Mono<Lock> seq = lock.lock(LockType.EXCLUSIVE) //
-                .flatMap(l -> lock.lock(LockType.EXCLUSIVE)) //
-                .flatMap(l -> lock.unlock());
+        Mono<?> l0 = lock.lock(LockType.EXCLUSIVE, "1").doOnNext(grant -> asynchUnlock(grant, lock));
+        Mono<?> l1 = lock.lock(LockType.SHARED, "2").doOnNext(grant -> asynchUnlock(grant, lock));
+        Mono<?> l2 = lock.lock(LockType.SHARED, "3").doOnNext(grant -> asynchUnlock(grant, lock));
+        Mono<?> l3 = lock.lock(LockType.EXCLUSIVE, "4").doOnNext(grant -> asynchUnlock(grant, lock));
+        Mono<?> l4 = lock.lock(LockType.SHARED, "5").doOnNext(grant -> asynchUnlock(grant, lock));
 
-        asynchUnlock(lock);
-        StepVerifier.create(seq) //
+        StepVerifier.create(Flux.zip(l0, l1, l2, l3, l4)) //
                 .expectSubscription() //
-                .expectNext(lock) //
+                .expectNextCount(1) //
                 .verifyComplete();
 
-        assertThat(lock.getLockCounter()).isZero();
-
+        await().untilAsserted(() -> assertThat(lock.getLockCounter()).isZero());
     }
-
 }
diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/tasks/RicSupervisionTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/tasks/RicSupervisionTest.java
index 313d5ddd..f6c93b95 100644
--- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/tasks/RicSupervisionTest.java
+++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/tasks/RicSupervisionTest.java
@@ -45,6 +45,7 @@ import org.onap.ccsdk.oran.a1policymanagementservice.clients.A1Client;
 import org.onap.ccsdk.oran.a1policymanagementservice.clients.A1ClientFactory;
 import org.onap.ccsdk.oran.a1policymanagementservice.configuration.ApplicationConfig;
 import org.onap.ccsdk.oran.a1policymanagementservice.configuration.ImmutableRicConfig;
+import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policies;
 import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policy;
@@ -121,8 +122,8 @@ class RicSupervisionTest {
     @AfterEach
     void verifyNoRicLocks() {
         for (Ric ric : this.rics.getRics()) {
-            ric.getLock().lockBlocking(LockType.EXCLUSIVE);
-            ric.getLock().unlockBlocking();
+            Lock.Grant grant = ric.getLock().lockBlocking(LockType.EXCLUSIVE, "");
+            grant.unlockBlocking();
             assertThat(ric.getLock().getLockCounter()).isZero();
         }
     }