From ac3d8bbd1c5040bd0e5255a8db46470882b4067e Mon Sep 17 00:00:00 2001 From: nitin-ebi <79518737+nitin-ebi@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:50:22 +0100 Subject: [PATCH] EVA-3664 Reserve new block - create new transaction for every retry (#90) * reserve new block - create new transaction for every retry --- .../service/ContiguousIdBlockService.java | 3 +- ...icAccessioningWithAlternateRangesTest.java | 2 ++ .../MonotonicAccessionGeneratorTest.java | 2 ++ .../service/ContiguousIdBlockServiceTest.java | 34 ++++++++++++------- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/accession-commons-monotonic-generator-jpa/src/main/java/uk/ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java b/accession-commons-monotonic-generator-jpa/src/main/java/uk/ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java index df355838..c0ca038f 100644 --- a/accession-commons-monotonic-generator-jpa/src/main/java/uk/ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java +++ b/accession-commons-monotonic-generator-jpa/src/main/java/uk/ac/ebi/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/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java b/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java index 73913e83..fbd9e35f 100644 --- a/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java +++ b/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/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/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java b/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java index fec40efc..eefffe47 100644 --- a/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java +++ b/accession-commons-monotonic-generator-jpa/src/test/java/uk/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/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockServiceTest.java b/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockServiceTest.java index af65c4a0..cba4c5ca 100644 --- a/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockServiceTest.java +++ b/accession-commons-monotonic-generator-jpa/src/test/java/uk/ac/ebi/ampt2d/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 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 blocksList = service.allBlocksForCategoryIdReservedBeforeTheGivenTimeFrame(CATEGORY_ID, cutOffTimestamp);