[OpenbravoERP-commits] devel/pi: related with issue 39755: prevent time based thread sy...

[hg]

details:   https://code.openbravo.com/erp/devel/pi/rev/2df0dc2f8b08
changeset: 35235:2df0dc2f8b08
user:      Asier Lostalé <asier.lostale <at> openbravo.com>
date:      Thu Dec 13 12:07:51 2018 +0100
summary:   related with issue 39755: prevent time based thread syncrhonization 
in tests

  Time based synchronization is error-prone and can cause false positives.

  Synchronizing events now with CountDownLatch.

diffstat:

 src-test/src/org/openbravo/test/dal/DalLockingTest.java |  66 ++++++++++++----
 1 files changed, 48 insertions(+), 18 deletions(-)

diffs (139 lines):

diff -r 29bb5d02e0fe -r 2df0dc2f8b08 
src-test/src/org/openbravo/test/dal/DalLockingTest.java
--- a/src-test/src/org/openbravo/test/dal/DalLockingTest.java   Thu Dec 13 
00:01:44 2018 +0530
+++ b/src-test/src/org/openbravo/test/dal/DalLockingTest.java   Thu Dec 13 
12:07:51 2018 +0100
@@ -29,6 +29,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -67,7 +68,7 @@
 
   @BeforeClass
   public static void createTestEnvironment() {
-    OBContext.setAdminMode();
+    OBContext.setAdminMode(true);
     try {
       AlertRule newAlertRule = OBProvider.getInstance().get(AlertRule.class);
       newAlertRule.setName(DalLockingTest.class.getName() + " - Testing Alert 
Rule");
@@ -101,10 +102,10 @@
 
   @Test
   public void objectShouldBeLockedInDB() throws InterruptedException, 
ExecutionException {
+    CountDownLatch latch = new CountDownLatch(1);
     List<Callable<Void>> threads = Arrays.asList(
-        //
-        getDalCallable(this::acquireLock, "T1", 0, 200),
-        getDalCallable(this::acquireLock, "T2", 100, 0));
+        getDalCallable(() -> acquireLock(latch), "T1", 200), //
+        getDalCallable(this::acquireLock, "T2", latch));
 
     executeAndGetResults(threads);
     assertThat(
@@ -115,14 +116,15 @@
   @Test
   public void lockedObjectShouldAllowChildrenCreation() throws 
InterruptedException,
       ExecutionException {
+    CountDownLatch latch = new CountDownLatch(1);
     List<Callable<Void>> threads = Arrays.asList( //
-        getDalCallable(this::acquireLock, "T1", 0, 200), //
+        getDalCallable(() -> acquireLock(latch), "T1", 200), //
         getDalCallable(() -> {
           AlertRecipient recipient = 
OBProvider.getInstance().get(AlertRecipient.class);
           recipient.setRole(OBContext.getOBContext().getRole());
           recipient.setAlertRule(getTestingAlertRule());
           OBDal.getInstance().save(recipient);
-        }, "T2", 100, 0));
+        }, "T2", latch));
 
     executeAndGetResults(threads);
     assertThat(
@@ -134,21 +136,21 @@
   public void lockedInstanceGetsRefreshedFromDB() throws InterruptedException, 
ExecutionException {
     StringBuilder originalName = new StringBuilder();
     StringBuilder lockedName = new StringBuilder();
+    CountDownLatch gotRule = new CountDownLatch(1);
+    CountDownLatch ruleModified = new CountDownLatch(1);
     List<Callable<Void>> threads = Arrays.asList( //
         getDalCallable(() -> {
           AlertRule ar = getTestingAlertRule();
           originalName.append(ar.getName());
-
-          try {
-            TimeUnit.MILLISECONDS.sleep(200);
-          } catch (InterruptedException e) {
-          }
-
+          gotRule.countDown();
+          waitUnitl(ruleModified);
           
lockedName.append(OBDal.getInstance().getObjectLockForNoKeyUpdate(ar).getName());
-        }, "T1", 0, 0), //
+        }, "T1", 0), //
         getDalCallable(() -> {
           getTestingAlertRule().setName("Modified");
-        }, "T2", 100, 0));
+          OBDal.getInstance().commitAndClose();
+          ruleModified.countDown();
+        }, "T2", gotRule));
 
     executeAndGetResults(threads);
 
@@ -176,12 +178,21 @@
     }
   }
 
-  private Callable<Void> getDalCallable(Runnable r, String name, long 
waitBefore, long waitAfter) {
+  private Callable<Void> getDalCallable(Runnable r, String name, 
CountDownLatch waitFor) {
+    return getDalCallable(r, name, waitFor, 0);
+  }
+
+  private Callable<Void> getDalCallable(Runnable r, String name, long 
waitAfter) {
+    return getDalCallable(r, name, null, waitAfter);
+  }
+
+  private Callable<Void> getDalCallable(Runnable r, String name, 
CountDownLatch waitEvent,
+      long waitAfter) {
     return () -> {
       boolean errorOccurred = false;
       try {
-        OBContext.setAdminMode();
-        TimeUnit.MILLISECONDS.sleep(waitBefore);
+        OBContext.setAdminMode(true);
+        waitUnitl(waitEvent);
         r.run();
         TimeUnit.MILLISECONDS.sleep(waitAfter);
         return null;
@@ -213,11 +224,30 @@
   }
 
   private AlertRule acquireLock() {
-    return OBDal.getInstance().getObjectLockForNoKeyUpdate(
+    return acquireLock(null);
+  }
+
+  private AlertRule acquireLock(CountDownLatch latch) {
+    AlertRule lockedRule = OBDal.getInstance().getObjectLockForNoKeyUpdate(
         OBDal.getInstance().getProxy(AlertRule.class, testingRuleId));
+    if (latch != null) {
+      latch.countDown();
+    }
+    return lockedRule;
   }
 
   private static AlertRule getTestingAlertRule() {
     return OBDal.getInstance().get(AlertRule.class, testingRuleId);
   }
+
+  private void waitUnitl(CountDownLatch event) {
+    if (event == null) {
+      return;
+    }
+    try {
+      event.await(10L, TimeUnit.SECONDS);
+    } catch (InterruptedException e) {
+      throw new OBException(e);
+    }
+  }
 }


_______________________________________________
Openbravo-commits mailing list
Openbravo-commits@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbravo-commits