EVA-3664 Reserve new block - create new transaction for every retry (#90

) * reserve new block - create new transaction for every retry
EBIvariation · Oct 9, 2024 · ac3d8bb · ac3d8bb
1 parent 09dcf00
commit ac3d8bb
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 14 deletions.
diff --git a/.../ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java b/.../ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java
@@ -18,6 +18,7 @@
 package uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.service;
 
 import org.springframework.transaction.annotation.Isolation;
+import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
 import uk.ac.ebi.ampt2d.commons.accession.block.initialization.BlockParameters;
 import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.entities.ContiguousIdBlock;
@@ -91,7 +92,7 @@ public void save(ContiguousIdBlock block) {
         entityManager.flush();
     }
 
-    @Transactional(isolation = Isolation.SERIALIZABLE)
+    @Transactional(isolation = Isolation.SERIALIZABLE, propagation = Propagation.REQUIRES_NEW)
     public ContiguousIdBlock reserveNewBlock(String categoryId, String instanceId) {
         ContiguousIdBlock lastBlock = repository.findFirstByCategoryIdOrderByLastValueDesc(categoryId);
         BlockParameters blockParameters = getBlockParameters(categoryId);

diff --git a/.../ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java b/.../ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java
@@ -21,6 +21,7 @@
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringRunner;
 import uk.ac.ebi.ampt2d.commons.accession.block.initialization.BlockInitializationException;
@@ -53,6 +54,7 @@
 @RunWith(SpringRunner.class)
 @DataJpaTest
 @ContextConfiguration(classes = {TestMonotonicDatabaseServiceTestConfiguration.class})
+@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
 public class BasicMonotonicAccessioningWithAlternateRangesTest {
 
     private static final String INSTANCE_ID = "test-instance";

diff --git a/...ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java b/...ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java
@@ -24,6 +24,7 @@
 import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringRunner;
 import uk.ac.ebi.ampt2d.commons.accession.core.exceptions.AccessionCouldNotBeGeneratedException;
@@ -59,6 +60,7 @@
 @RunWith(SpringRunner.class)
 @DataJpaTest
 @ContextConfiguration(classes = {MonotonicAccessionGeneratorTestConfiguration.class, TestMonotonicDatabaseServiceTestConfiguration.class})
+@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
 public class MonotonicAccessionGeneratorTest {
 
     private static final int BLOCK_SIZE = 1000;

diff --git a/...t2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockServiceTest.java b/...t2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockServiceTest.java
@@ -22,14 +22,14 @@
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager;
+import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringRunner;
 import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.entities.ContiguousIdBlock;
 import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.repositories.ContiguousIdBlockRepository;
 import uk.ac.ebi.ampt2d.test.configuration.MonotonicAccessionGeneratorTestConfiguration;
 
-import javax.persistence.EntityManager;
-import javax.persistence.PersistenceContext;
 import javax.persistence.PersistenceException;
 import java.time.LocalDateTime;
 import java.util.Arrays;
@@ -46,6 +46,7 @@
 @RunWith(SpringRunner.class)
 @DataJpaTest
 @ContextConfiguration(classes = {MonotonicAccessionGeneratorTestConfiguration.class})
+@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
 public class ContiguousIdBlockServiceTest {
 
     private static final String CATEGORY_ID = "cat-test";
@@ -59,8 +60,8 @@ public class ContiguousIdBlockServiceTest {
     @Autowired
     private ContiguousIdBlockService service;
 
-    @PersistenceContext
-    EntityManager entityManager;
+    @Autowired
+    private TestEntityManager testEntityManager;
 
     @Test
     public void testReserveNewBlocks() {
@@ -78,6 +79,9 @@ public void testReserveNewBlocks() {
     public void testReserveWithExistingData() {
         //Save a block
         service.save(Arrays.asList(new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID, 0, 5)));
+        testEntityManager.flush();
+        testEntityManager.getEntityManager().getTransaction().commit();
+
         ContiguousIdBlock block = service.reserveNewBlock(CATEGORY_ID, INSTANCE_ID);
         assertEquals(5, block.getFirstValue());
         assertEquals(1004, block.getLastValue());
@@ -166,10 +170,14 @@ public void testBlockSizeAndIntervalWithMultipleInstances() {
         ContiguousIdBlock block2 = service.reserveNewBlock(CATEGORY_ID_2, INSTANCE_ID_2);
         assertEquals(2000, block2.getFirstValue());
         assertEquals(2999, block2.getLastValue());
-        assertEquals(block2, repository.findFirstByCategoryIdOrderByLastValueDesc(CATEGORY_ID_2));
+        ContiguousIdBlock block2InDB = repository.findFirstByCategoryIdOrderByLastValueDesc(CATEGORY_ID_2);
+        assertEquals(2000, block2InDB.getFirstValue());
+        assertEquals(2999, block2InDB.getLastValue());
 
         //Manually save a block of size 500, so for the current range only a block size of 500 reserved
         repository.save(new ContiguousIdBlock(CATEGORY_ID_2, INSTANCE_ID, 4000, 500));
+        testEntityManager.flush();
+        testEntityManager.getEntityManager().getTransaction().commit();
         //Reserve a new block with size 1000
         ContiguousIdBlock block3 = service.reserveNewBlock(CATEGORY_ID_2, INSTANCE_ID_2);
 
@@ -260,12 +268,12 @@ public void testNextBlockWithStartingPointOtherThanZero() {
     public void testBlocksWithDuplicateCategoryAndFirstValue() {
         ContiguousIdBlock block1 = new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID, 100, 1000);
         repository.save(block1);
-        entityManager.flush();
+        testEntityManager.flush();
 
         ContiguousIdBlock block2 = new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID_2, 100, 2000);
         repository.save(block2);
 
-        Throwable exception = assertThrows(PersistenceException.class, () -> entityManager.flush());
+        Throwable exception = assertThrows(PersistenceException.class, () -> testEntityManager.flush());
         assertTrue(exception.getCause() instanceof ConstraintViolationException);
     }
 
@@ -274,7 +282,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
         // block saved with initial value
         ContiguousIdBlock block = new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID, 100, 1000);
         repository.save(block);
-        entityManager.flush();
+        testEntityManager.flush();
 
         // assert block values
         List<ContiguousIdBlock> blockInDBList = StreamSupport.stream(repository.findAll().spliterator(), false)
@@ -292,7 +300,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
         // block updated with last committed 100
         block.setLastCommitted(100);
         repository.save(block);
-        entityManager.flush();
+        testEntityManager.flush();
         blockInDB = repository.findAll().iterator().next();
         assertEquals(100, blockInDB.getLastCommitted());
 
@@ -301,7 +309,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
         // block updated with new instance id
         block.setApplicationInstanceId(INSTANCE_ID_2);
         repository.save(block);
-        entityManager.flush();
+        testEntityManager.flush();
         blockInDB = repository.findAll().iterator().next();
         assertEquals(INSTANCE_ID_2, blockInDB.getApplicationInstanceId());
 
@@ -310,7 +318,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
         // block updated - release reserved
         block.releaseReserved();
         repository.save(block);
-        entityManager.flush();
+        testEntityManager.flush();
         blockInDB = repository.findAll().iterator().next();
         assertTrue(blockInDB.isNotReserved());
 
@@ -319,7 +327,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
         // block update - mark as reserved
         block.markAsReserved();
         repository.save(block);
-        entityManager.flush();
+        testEntityManager.flush();
         blockInDB = repository.findAll().iterator().next();
         assertTrue(blockInDB.isReserved());
 
@@ -349,7 +357,7 @@ public void testGetBlocksWithLastUpdatedTimeStampLessThan() throws InterruptedEx
         repository.save(block3);
         repository.save(block4);
         repository.save(block5);
-        entityManager.flush();
+        testEntityManager.flush();
 
         LocalDateTime cutOffTimestamp = block4.getLastUpdatedTimestamp();
         List<ContiguousIdBlock> blocksList = service.allBlocksForCategoryIdReservedBeforeTheGivenTimeFrame(CATEGORY_ID, cutOffTimestamp);