You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2022/06/01 08:54:15 UTC

[isis] branch master updated: ISIS-3063: thread-safety f. reg. test jdo fixtures

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/master by this push:
     new f3c75970d3 ISIS-3063: thread-safety f. reg. test jdo fixtures
f3c75970d3 is described below

commit f3c75970d33e6ccc393003bf8e7bee1e4fa51b0d
Author: Andi Huber <ah...@apache.org>
AuthorDate: Wed Jun 1 10:54:08 2022 +0200

    ISIS-3063: thread-safety f. reg. test jdo fixtures
---
 .../jdo/JdoExceptionTranslationTest.java           |   2 -
 ...xceptionTranslationTest_usingTransactional.java |  14 +--
 .../testdomain/persistence/jdo/JdoJaxbTest.java    |   2 -
 .../testdomain/persistence/jdo/JdoQueryTest.java   |   3 -
 ...actionRollbackTest_usingTransactionService.java |  11 +-
 ...TransactionRollbackTest_usingTransactional.java |  12 ++-
 .../jdo/JdoTransactionScopeListenerTest.java       |  13 ++-
 .../isis/testdomain/jdo/JdoTestFixtures.java       | 113 ++++++++++++++-------
 .../publishing/PublishingTestFactoryJdo.java       |  13 ++-
 9 files changed, 111 insertions(+), 72 deletions(-)

diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest.java
index 6317beb795..9bac466e98 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest.java
@@ -65,8 +65,6 @@ class JdoExceptionTranslationTest extends RegressionTestAbstract {
         // when adding a book for which one with same ISBN already exists in the database,
         // we expect to see a Spring recognized DataAccessException been thrown
 
-        jdoTestFixtures.install();
-
         assertThrows(DataAccessException.class, ()->{
 
             run(()->{
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest_usingTransactional.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest_usingTransactional.java
index eb144f6b05..25a59b4ce7 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest_usingTransactional.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoExceptionTranslationTest_usingTransactional.java
@@ -73,12 +73,6 @@ extends RegressionTestAbstract {
     }
 
     @Test @Order(1)
-    @Transactional @Rollback(false)
-    void booksUniqueByIsbn_setupPhase() {
-        //testFixtures.install();
-    }
-
-    @Test @Order(2)
     void booksUniqueByIsbn_whenViolated_shouldThrowTranslatedException() {
 
         // when adding a book for which one with same ISBN already exists in the database,
@@ -103,7 +97,7 @@ extends RegressionTestAbstract {
 
     }
 
-    @Test @Order(3)
+    @Test @Order(2)
     @Transactional @Rollback(false)
     void booksUniqueByIsbn_verifyPhase() {
 
@@ -127,10 +121,4 @@ extends RegressionTestAbstract {
 
     }
 
-    @Test @Order(4)
-    @Transactional @Rollback(false)
-    void booksUniqueByIsbn_cleanupPhase() {
-        testFixtures.install(); // keep fixtures installed for other tests
-    }
-
 }
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoJaxbTest.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoJaxbTest.java
index 6def028ae5..cea1850da2 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoJaxbTest.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoJaxbTest.java
@@ -47,8 +47,6 @@ class JdoJaxbTest extends RegressionTestAbstract {
     @Test
     void inventoryJaxbVm_shouldRoundtripProperly() {
 
-        jdoTestFixtures.install();
-
         val xml = call(()->{
             val inventoryJaxbVm = jdoTestFixtures.setUpViewmodelWith3Books();
             // assert initial reference is populated as expected
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoQueryTest.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoQueryTest.java
index bc461e272d..beb1be1abd 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoQueryTest.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/persistence/jdo/JdoQueryTest.java
@@ -67,9 +67,6 @@ class JdoQueryTest extends IsisIntegrationTestAbstract {
     @Test @Order(1)
     void sampleInventory_shouldBeSetUpWith3Books() {
 
-        //testFixtures.setUp3Books();
-        jdoTestFixtures.install();
-
         // when
 
         val inventories = repositoryService.allInstances(JdoInventory.class);
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactionService.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactionService.java
index fc8c59b329..9862dac0ba 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactionService.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactionService.java
@@ -35,6 +35,7 @@ import org.apache.isis.commons.internal.exceptions._Exceptions;
 import org.apache.isis.core.config.presets.IsisPresets;
 import org.apache.isis.testdomain.conf.Configuration_usingJdo;
 import org.apache.isis.testdomain.jdo.JdoTestFixtures;
+import org.apache.isis.testdomain.jdo.JdoTestFixtures.Lock;
 import org.apache.isis.testdomain.jdo.entities.JdoBook;
 
 import lombok.val;
@@ -50,15 +51,17 @@ class JdoTransactionRollbackTest_usingTransactionService {
     @Inject private TransactionService transactionService;
     @Inject private RepositoryService repository;
 
+    private Lock lock;
+
     @BeforeEach
     void setUp() {
         // clear repository
-        jdoTestFixtures.clear();
+        lock = jdoTestFixtures.clearAndAquireLock();
     }
 
     @AfterEach
     void restore() {
-        jdoTestFixtures.reinstall(()->{});
+        lock.release();
     }
 
     @Test
@@ -68,7 +71,7 @@ class JdoTransactionRollbackTest_usingTransactionService {
             // expected pre condition
             assertEquals(0, repository.allInstances(JdoBook.class).size());
 
-            jdoTestFixtures.install();
+            jdoTestFixtures.install(lock);
 
             // expected post condition
             assertEquals(1, repository.allInstances(JdoBook.class).size());
@@ -86,7 +89,7 @@ class JdoTransactionRollbackTest_usingTransactionService {
 
         val result = transactionService.runWithinCurrentTransactionElseCreateNew(()->{
             //fixtureScripts.runPersona(JdoTestDomainPersona.InventoryWith1Book);
-            jdoTestFixtures.install();
+            jdoTestFixtures.install(lock);
             throw _Exceptions.unrecoverable("Test: force current tx to rollback");
         });
 
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactional.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactional.java
index 889ade1872..3b45109e6c 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactional.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionRollbackTest_usingTransactional.java
@@ -26,7 +26,6 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestMethodOrder;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.annotation.Commit;
-import org.springframework.test.annotation.Rollback;
 import org.springframework.transaction.annotation.Transactional;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -36,6 +35,7 @@ import org.apache.isis.applib.services.repository.RepositoryService;
 import org.apache.isis.commons.internal.debug._Probe;
 import org.apache.isis.testdomain.conf.Configuration_usingJdo;
 import org.apache.isis.testdomain.jdo.JdoTestFixtures;
+import org.apache.isis.testdomain.jdo.JdoTestFixtures.Lock;
 import org.apache.isis.testdomain.jdo.entities.JdoBook;
 
 /**
@@ -62,10 +62,12 @@ class JdoTransactionRollbackTest_usingTransactional {
     @Inject private JdoTestFixtures jdoTestFixtures;
     @Inject private RepositoryService repository;
     @Inject private InteractionService interactionService;
+    private static Lock lock;
 
     @Test @Order(1) @Commit
     void clearRepository() {
-        jdoTestFixtures.clear();
+        // clear repository
+        lock = jdoTestFixtures.clearAndAquireLock();
     }
 
     @Test @Order(2)
@@ -80,7 +82,7 @@ class JdoTransactionRollbackTest_usingTransactional {
 
             _Probe.errOut("before fixture");
 
-            jdoTestFixtures.install();
+            jdoTestFixtures.install(lock);
             //fixtureScripts.runPersona(JdoTestDomainPersona.InventoryWith1Book);
 
             _Probe.errOut("after fixture");
@@ -107,9 +109,9 @@ class JdoTransactionRollbackTest_usingTransactional {
 
     }
 
-    @Test @Order(4) @Rollback(false)
+    @Test @Order(4) @Commit
     void restoreDefaultCondition() {
-        jdoTestFixtures.reinstall(()->{});
+        lock.release();
     }
 
 }
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionScopeListenerTest.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionScopeListenerTest.java
index d541392b3c..883781e0b3 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionScopeListenerTest.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/transactions/jdo/JdoTransactionScopeListenerTest.java
@@ -20,6 +20,7 @@ package org.apache.isis.testdomain.transactions.jdo;
 
 import javax.inject.Inject;
 
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.context.SpringBootTest;
@@ -33,6 +34,7 @@ import org.apache.isis.applib.services.xactn.TransactionService;
 import org.apache.isis.core.config.presets.IsisPresets;
 import org.apache.isis.testdomain.conf.Configuration_usingJdo;
 import org.apache.isis.testdomain.jdo.JdoTestFixtures;
+import org.apache.isis.testdomain.jdo.JdoTestFixtures.Lock;
 import org.apache.isis.testdomain.jdo.entities.JdoBook;
 import org.apache.isis.testdomain.util.interaction.InteractionBoundaryProbe;
 import org.apache.isis.testdomain.util.kv.KVStoreForTesting;
@@ -54,6 +56,7 @@ class JdoTransactionScopeListenerTest {
     @Inject private RepositoryService repository;
     @Inject private InteractionService interactionService;
     @Inject private KVStoreForTesting kvStoreForTesting;
+    private Lock lock;
 
     /* Expectations:
      * 1. for each InteractionScope there should be a new InteractionBoundaryProbe instance
@@ -68,7 +71,13 @@ class JdoTransactionScopeListenerTest {
     void setUp() {
         // new InteractionScope with a new transaction (#1)
         // clear repository
-        jdoTestFixtures.clear();
+        lock = jdoTestFixtures.clearAndAquireLock();
+    }
+
+    @AfterEach
+    void restore() {
+        // restore repository
+        lock.release();
     }
 
     @Test
@@ -84,7 +93,7 @@ class JdoTransactionScopeListenerTest {
             // reuse transaction (#2)
             transactionService.runWithinCurrentTransactionElseCreateNew(()->{
                 // + 1 interaction + 1 transaction
-                jdoTestFixtures.install();
+                jdoTestFixtures.install(lock);
             });
 
             // expected post condition
diff --git a/regressiontests/stable/src/main/java/org/apache/isis/testdomain/jdo/JdoTestFixtures.java b/regressiontests/stable/src/main/java/org/apache/isis/testdomain/jdo/JdoTestFixtures.java
index fc54c7e1d6..0a77828b0f 100644
--- a/regressiontests/stable/src/main/java/org/apache/isis/testdomain/jdo/JdoTestFixtures.java
+++ b/regressiontests/stable/src/main/java/org/apache/isis/testdomain/jdo/JdoTestFixtures.java
@@ -22,6 +22,7 @@ import java.util.Collection;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
+import java.util.concurrent.LinkedBlockingQueue;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -39,7 +40,9 @@ import org.apache.isis.applib.services.factory.FactoryService;
 import org.apache.isis.applib.services.iactnlayer.InteractionService;
 import org.apache.isis.applib.services.repository.RepositoryService;
 import org.apache.isis.commons.collections.Can;
+import org.apache.isis.commons.internal.assertions._Assert;
 import org.apache.isis.commons.internal.base._NullSafe;
+import org.apache.isis.commons.internal.base._Oneshot;
 import org.apache.isis.commons.internal.base._Refs;
 import org.apache.isis.commons.internal.base._Refs.BooleanAtomicReference;
 import org.apache.isis.commons.internal.base._Strings;
@@ -48,6 +51,8 @@ import org.apache.isis.testdomain.jdo.entities.JdoInventory;
 import org.apache.isis.testdomain.jdo.entities.JdoProduct;
 import org.apache.isis.testdomain.util.dto.BookDto;
 
+import lombok.RequiredArgsConstructor;
+import lombok.SneakyThrows;
 import lombok.val;
 
 @Service
@@ -58,13 +63,30 @@ public class JdoTestFixtures implements MetamodelListener {
     @Inject private BookmarkService bookmarkService;
     @Inject private InteractionService interactionService;
 
+    @RequiredArgsConstructor
+    public static class Lock {
+        private final _Oneshot release = new _Oneshot();
+        private final JdoTestFixtures jdoTestFixtures;
+        public void release() {
+            release.trigger(()->jdoTestFixtures.release(this));
+        }
+    }
+
     private BooleanAtomicReference isInstalled = _Refs.booleanAtomicRef(false);
 
+    private LinkedBlockingQueue<Lock> lockQueue = new LinkedBlockingQueue<>(1);
+
+
     @Override
     public void onMetamodelLoaded() {
         install();
     }
 
+    public void install(final Lock lock) {
+        _Assert.assertEquals(lockQueue.peek(), lock);
+        install();
+    }
+
     public void reinstall(final Runnable onBeforeInstall) {
         isInstalled.compute(isInst->{
             interactionService.runAnonymous(()->{
@@ -76,45 +98,18 @@ public class JdoTestFixtures implements MetamodelListener {
         });
     }
 
-    public void install() {
-        isInstalled.computeIfFalse(()->{
-            interactionService.runAnonymous(()->{
-                cleanUpRepository();
-                setUp3Books();
-            });
-            return true;
-        });
-    }
-
-    public void clear() {
-        isInstalled.computeIfTrue(()->{
-            interactionService.runAnonymous(()->{
-                cleanUpRepository();
-            });
-            return false;
-        });
-    }
-
-    private void cleanUpRepository() {
-        repository.allInstances(JdoInventory.class).forEach(repository::remove);
-        repository.allInstances(JdoBook.class).forEach(repository::remove);
-        repository.allInstances(JdoProduct.class).forEach(repository::remove);
+    @SneakyThrows
+    public Lock clearAndAquireLock() {
+        Lock lock;
+        lockQueue.put(lock = new Lock(this)); // put next lock on the queue; blocks until space available
+        clear();
+        return lock;
     }
 
-    private void setUp3Books() {
-
-        // given - expected pre condition: no inventories
-        assertEquals(0, repository.allInstances(JdoInventory.class).size());
-
-        // setup sample Inventory with 3 Books
-        SortedSet<JdoProduct> products = new TreeSet<>();
-
-        BookDto.samples()
-        .map(JdoBook::fromDto)
-        .forEach(products::add);
-
-        val inventory = JdoInventory.of("Sample Inventory", products);
-        repository.persistAndFlush(inventory);
+    @SneakyThrows
+    void release(final Lock lock) {
+        reinstall(()->{});
+        lockQueue.take(); // remove lock from queue
     }
 
     public void addABookTo(final JdoInventory inventory) {
@@ -187,6 +182,50 @@ public class JdoTestFixtures implements MetamodelListener {
         return expectedTitles;
     }
 
+    // -- HELPER
+
+    private void clear() {
+
+
+
+        isInstalled.computeIfTrue(()->{
+            interactionService.runAnonymous(()->{
+                cleanUpRepository();
+            });
+            return false;
+        });
+    }
+
+    private void install() {
+        isInstalled.computeIfFalse(()->{
+            interactionService.runAnonymous(()->{
+                cleanUpRepository();
+                setUp3Books();
+            });
+            return true;
+        });
+    }
+
+    private void cleanUpRepository() {
+        repository.allInstances(JdoInventory.class).forEach(repository::remove);
+        repository.allInstances(JdoBook.class).forEach(repository::remove);
+        repository.allInstances(JdoProduct.class).forEach(repository::remove);
+    }
+
+    private void setUp3Books() {
+
+        // given - expected pre condition: no inventories
+        assertEquals(0, repository.allInstances(JdoInventory.class).size());
 
+        // setup sample Inventory with 3 Books
+        SortedSet<JdoProduct> products = new TreeSet<>();
+
+        BookDto.samples()
+        .map(JdoBook::fromDto)
+        .forEach(products::add);
+
+        val inventory = JdoInventory.of("Sample Inventory", products);
+        repository.persistAndFlush(inventory);
+    }
 
 }
diff --git a/regressiontests/stable/src/main/java/org/apache/isis/testdomain/publishing/PublishingTestFactoryJdo.java b/regressiontests/stable/src/main/java/org/apache/isis/testdomain/publishing/PublishingTestFactoryJdo.java
index db848807db..8c73bbbbcb 100644
--- a/regressiontests/stable/src/main/java/org/apache/isis/testdomain/publishing/PublishingTestFactoryJdo.java
+++ b/regressiontests/stable/src/main/java/org/apache/isis/testdomain/publishing/PublishingTestFactoryJdo.java
@@ -46,6 +46,7 @@ import org.apache.isis.core.metamodel.interactions.managed.PropertyInteraction;
 import org.apache.isis.core.metamodel.objectmanager.ObjectManager;
 import org.apache.isis.core.metamodel.spec.ManagedObject;
 import org.apache.isis.testdomain.jdo.JdoTestFixtures;
+import org.apache.isis.testdomain.jdo.JdoTestFixtures.Lock;
 import org.apache.isis.testdomain.jdo.entities.JdoBook;
 import org.apache.isis.testdomain.publishing.PublishingTestFactoryAbstract.CommitListener;
 import org.apache.isis.testdomain.util.dto.BookDto;
@@ -81,12 +82,16 @@ extends PublishingTestFactoryAbstract {
 
     @Named("transaction-aware-pmf-proxy")
     private final PersistenceManagerFactory pmf;
+    private Lock lock = null;
 
     // -- TEST SETUP
 
     @Override
     protected void releaseContext(final PublishingTestContext context) {
-
+        if(lock!=null) {
+            lock.release();
+            lock = null;
+        }
     }
 
     @Override
@@ -100,7 +105,7 @@ extends PublishingTestFactoryAbstract {
         case ENTITY_PERSISTING:
 
             // given
-            jdoTestFixtures.clear();
+            lock = jdoTestFixtures.clearAndAquireLock();
             break;
 
         case ENTITY_LOADING:
@@ -109,7 +114,7 @@ extends PublishingTestFactoryAbstract {
         case ENTITY_REMOVAL:
 
             // given
-            jdoTestFixtures.install();
+            //jdoTestFixtures.install(lock);
             break;
         default:
             throw _Exceptions.unmatchedCase(context.getScenario());
@@ -140,7 +145,7 @@ extends PublishingTestFactoryAbstract {
 
             context.runGiven();
             //when
-            jdoTestFixtures.install();
+            jdoTestFixtures.install(lock);
             break;
 
         case ENTITY_LOADING: