Skip to content

Commit

Permalink
review comments - test updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@nitin-ebi
nitin-ebi committed Mar 19, 2024
1 parent c1f5721 commit 6988429
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 75 deletions.
4 changes: 4 additions & 0 deletions ...ac/ebi/ampt2d/commons/accession/persistence/jpa/monotonic/entities/ContiguousIdBlock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ public long getId() {
return id;
}

public String getCategoryId() {
return categoryId;
}

public long getLastCommitted() {
return lastCommitted;
}
Expand Down
8 changes: 0 additions & 8 deletions ...commons/accession/persistence/jpa/monotonic/repositories/ContiguousIdBlockRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,9 @@
import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.entities.ContiguousIdBlock;

import java.util.List;
import java.util.stream.Stream;

@Repository
public interface ContiguousIdBlockRepository extends CrudRepository<ContiguousIdBlock, Long> {

ContiguousIdBlock findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(String categoryId,
String instanceId);

Stream<ContiguousIdBlock> findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(String categoryId,
String instanceId);

@Query("SELECT cib FROM ContiguousIdBlock cib WHERE cib.categoryId = :categoryId AND cib.lastCommitted != cib.lastValue AND (cib.reserved IS NULL OR cib.reserved IS FALSE) ORDER BY cib.lastValue asc")
List<ContiguousIdBlock> findUncompletedAndUnreservedBlocksOrderByLastValueAsc(@Param("categoryId") String categoryId);

Expand Down
3 changes: 0 additions & 3 deletions .../ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,4 @@ public List<ContiguousIdBlock> reserveUncompletedBlocksForCategoryIdAndApplicati
return blockList;
}

public ContiguousIdBlockRepository getRepository() {
return repository;
}
}
44 changes: 21 additions & 23 deletions .../ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static uk.ac.ebi.ampt2d.commons.accession.util.ContiguousIdBlockUtil.getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc;
import static uk.ac.ebi.ampt2d.commons.accession.util.ContiguousIdBlockUtil.getAllBlocksForCategoryId;
import static uk.ac.ebi.ampt2d.commons.accession.util.ContiguousIdBlockUtil.getAllUncompletedBlocksForCategoryId;
import static uk.ac.ebi.ampt2d.commons.accession.util.ContiguousIdBlockUtil.getUnreservedContiguousIdBlock;

@RunWith(SpringRunner.class)
Expand All @@ -62,6 +63,9 @@ public class BasicMonotonicAccessioningWithAlternateRangesTest {
@Autowired
private ContiguousIdBlockService contiguousIdBlockService;

@Autowired
private ContiguousIdBlockRepository contiguousIdBlockRepository;

@Test(expected = BlockInitializationException.class)
public void testUnknownCategory() throws AccessionCouldNotBeGeneratedException {
List<GetOrCreateAccessionWrapper<TestModel, String, Long>> evaAccessions =
Expand All @@ -82,7 +86,7 @@ public void testRecoverState() {
uncompletedBlocks.add(getUnreservedContiguousIdBlock(categoryId, instanceId2, 120, 10));
contiguousIdBlockService.save(uncompletedBlocks);

assertEquals(3, getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc(contiguousIdBlockService.getRepository(), categoryId, instanceId2).size());
assertEquals(3, getAllUncompletedBlocksForCategoryId(contiguousIdBlockRepository, categoryId).size());

// create and save accessions in db (100 to 124) - save 2 sets of same accessions with different hashes
List<AccessionWrapper<TestModel, String, Long>> accessionsSet1 = LongStream.range(100l, 125l)
Expand All @@ -103,8 +107,8 @@ public void testRecoverState() {
// block-1 (100 to 109) : fully complete
// block-2 (110 to 119) : fully complete
// block-3 (120 to 124) : partially complete
assertEquals(1, getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc(contiguousIdBlockService.getRepository(), categoryId, instanceId2).size());
ContiguousIdBlock uncompletedBlock = getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc(contiguousIdBlockService.getRepository(), categoryId, instanceId2).get(0);
assertEquals(1, getAllUncompletedBlocksForCategoryId(contiguousIdBlockRepository, categoryId).size());
ContiguousIdBlock uncompletedBlock = getAllUncompletedBlocksForCategoryId(contiguousIdBlockRepository, categoryId).get(0);
assertEquals(120l, uncompletedBlock.getFirstValue());
assertEquals(129l, uncompletedBlock.getLastValue());
assertEquals(124l, uncompletedBlock.getLastCommitted());
Expand All @@ -119,7 +123,7 @@ public void testRecoverState() {
public void testAlternateRangesWithDifferentGenerators() throws AccessionCouldNotBeGeneratedException {
/* blockStartValue= 0, blockSize= 10 , nextBlockInterval= 20
the new blocks are interleaved or jumped for each 20 items accessioned
so the accesions will be in the range of 0-19,40-59,80-99 */
so the accessions will be in the range of 0-19,40-59,80-99 */
String categoryId = "eva_2";
String instanceId2 = "test-instance_2";
BasicAccessioningService accService1 = getAccessioningService(categoryId, INSTANCE_ID);
Expand All @@ -128,8 +132,7 @@ public void testAlternateRangesWithDifferentGenerators() throws AccessionCouldNo
assertEquals(0, evaAccessions.get(0).getAccession().longValue());
assertEquals(8, evaAccessions.get(8).getAccession().longValue());
//BlockSize of 10 was reserved but only 9 elements have been accessioned
assertEquals(1, getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc(contiguousIdBlockService.getRepository(), categoryId, INSTANCE_ID)
.size());
assertEquals(1, getAllUncompletedBlocksForCategoryId(contiguousIdBlockRepository, categoryId).size());
accService1.shutDownAccessioning();

//Get another service for same category
Expand All @@ -147,7 +150,7 @@ public void testAlternateRangesWithDifferentGenerators() throws AccessionCouldNo
assertEquals(40, evaAccessions.get(11).getAccession().longValue());
assertEquals(48, evaAccessions.get(19).getAccession().longValue());
//BlockSize if 10 was reserved but only 9 elements have been accessioned
assertEquals(1, getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc(contiguousIdBlockService.getRepository(), categoryId, INSTANCE_ID).size());
assertEquals(1, getAllUncompletedBlocksForCategoryId(contiguousIdBlockRepository, categoryId).size());
accService2.shutDownAccessioning();

//Get another service for same category but different Instance
Expand All @@ -156,33 +159,30 @@ public void testAlternateRangesWithDifferentGenerators() throws AccessionCouldNo
assertEquals(9, evaAccessions.size());
//New Block from different instance have not jumped as still blocks are available before interleaving point
assertNotEquals(80, evaAccessions.get(0).getAccession().longValue());
assertEquals(50, evaAccessions.get(0).getAccession().longValue());
assertEquals(58, evaAccessions.get(8).getAccession().longValue());
assertEquals(1, getUncompletedBlocksByCategoryIdAndApplicationInstanceIdOrderByEndAsc(contiguousIdBlockService.getRepository(), categoryId, instanceId2).size());
assertEquals(49, evaAccessions.get(0).getAccession().longValue());
assertEquals(57, evaAccessions.get(8).getAccession().longValue());
assertEquals(1, getAllUncompletedBlocksForCategoryId(contiguousIdBlockRepository, categoryId).size());
accService3.shutDownAccessioning();

//Get previous uncompleted service from instance1 and create accessions
BasicAccessioningService accService4 = getAccessioningService(categoryId, INSTANCE_ID);
evaAccessions = accService4.getOrCreate(getObjectsForAccessionsInRange(40, 41));
assertEquals(2, evaAccessions.size());
assertEquals(49, evaAccessions.get(0).getAccession().longValue()); //Block ended here
evaAccessions = accService4.getOrCreate(getObjectsForAccessionsInRange(40, 42));
assertEquals(3, evaAccessions.size());
assertEquals(58, evaAccessions.get(0).getAccession().longValue()); //Block ended here
//New Block with 20 interval from last block made in instanceId2
assertEquals(80, evaAccessions.get(1).getAccession().longValue());
assertEquals(80, evaAccessions.get(2).getAccession().longValue());
}

@Test
public void testInitializeBlockManagerInMonotonicAccessionGenerator() {
String categoryId = "eva_2";
String instanceId2 = "test-instance_2";
ContiguousIdBlockRepository repository = contiguousIdBlockService.getRepository();

ContiguousIdBlock block = getUnreservedContiguousIdBlock(categoryId, instanceId2, 0, 10);
repository.save(block);
contiguousIdBlockRepository.save(block);

// assert block is not full and not reserved
List<ContiguousIdBlock> blockInDBList = repository
.findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(categoryId, instanceId2)
.collect(Collectors.toList());
List<ContiguousIdBlock> blockInDBList = getAllBlocksForCategoryId(contiguousIdBlockRepository, categoryId);
assertEquals(1, blockInDBList.size());
List<ContiguousIdBlock> unreservedAndNotFullBlocks = blockInDBList.stream()
.filter(b -> b.isNotFull() && b.isNotReserved())
Expand All @@ -196,9 +196,7 @@ public void testInitializeBlockManagerInMonotonicAccessionGenerator() {
BasicAccessioningService accService = getAccessioningService(categoryId, instanceId2);

// assert block gets reserved after recover state
blockInDBList = repository
.findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(categoryId, instanceId2)
.collect(Collectors.toList());
blockInDBList = getAllBlocksForCategoryId(contiguousIdBlockRepository, categoryId);
assertEquals(1, blockInDBList.size());
unreservedAndNotFullBlocks = blockInDBList.stream()
.filter(b -> b.isNotFull() && b.isNotReserved())
Expand Down
54 changes: 29 additions & 25 deletions ...ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import uk.ac.ebi.ampt2d.commons.accession.utils.exceptions.ExponentialBackOffMaxRetriesRuntimeException;
import uk.ac.ebi.ampt2d.test.configuration.MonotonicAccessionGeneratorTestConfiguration;

import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -51,6 +52,7 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.when;
import static uk.ac.ebi.ampt2d.commons.accession.util.ContiguousIdBlockUtil.getAllBlocksForCategoryId;
import static uk.ac.ebi.ampt2d.commons.accession.util.ContiguousIdBlockUtil.getUnreservedContiguousIdBlock;

@RunWith(SpringRunner.class)
Expand Down Expand Up @@ -112,14 +114,14 @@ public void assertNewBlockGeneratedInSecondInstance() throws Exception {

generator1.generateAccessions(TENTH_BLOCK_SIZE);
assertEquals(1, repository.count());
block = repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(0, block.getFirstValue());
assertEquals(BLOCK_SIZE - 1, block.getLastValue());
assertEquals(-1, block.getLastCommitted());

generator2.generateAccessions(TENTH_BLOCK_SIZE);
assertEquals(2, repository.count());
block = repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_2_ID);
block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(BLOCK_SIZE, block.getFirstValue());
assertEquals(2 * BLOCK_SIZE - 1, block.getLastValue());
assertEquals(BLOCK_SIZE - 1, block.getLastCommitted());
Expand Down Expand Up @@ -162,8 +164,7 @@ public void assertCommitModifiesLastCommitted() throws Exception {

generator.commit(accessions);

ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(TENTH_BLOCK_SIZE - 1, block.getLastCommitted());
}

Expand All @@ -172,8 +173,7 @@ public void assertNotCommittingDoesNotModifyLastCommitted() throws Exception {
MonotonicAccessionGenerator generator = getMonotonicAccessionGenerator();
long[] accessions = generator.generateAccessions(TENTH_BLOCK_SIZE);

ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(-1, block.getLastCommitted());
}

Expand All @@ -185,13 +185,12 @@ public void assertCommitOutOfOrderDoesNotModifyLastCommittedUntilTheSequenceIsCo

generator.commit(accessions2);

ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(-1, block.getLastCommitted());

generator.commit(accessions1);

block = repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(2 * TENTH_BLOCK_SIZE - 1, block.getLastCommitted());
}

Expand All @@ -204,14 +203,13 @@ public void assertCommitOutOfOrderDoesNotModifyLastCommittedUntilTheSequenceIsCo

generator.commit(accessions2);

ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(BLOCK_SIZE, block.getFirstValue());
assertEquals(BLOCK_SIZE - 1, block.getLastCommitted());

generator.commit(accessions1);

block = repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(BLOCK_SIZE, block.getFirstValue());
assertEquals(BLOCK_SIZE + 2 * TENTH_BLOCK_SIZE - 1, block.getLastCommitted());
}
Expand Down Expand Up @@ -274,8 +272,7 @@ public void assertMultipleReleaseAndCommitsWorks() throws Exception {
generator.release(8, 9, 10);
generator.commit(getLongArray(12, 998));
//999 is waiting somewhere taking a big nap and no elements have been confirmed due to element 0 being released
ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(-1, block.getLastCommitted());

long[] accessions2 = generator.generateAccessions(BLOCK_SIZE);
Expand All @@ -297,7 +294,7 @@ public void assertMultipleReleaseAndCommitsWorks() throws Exception {
assertEquals(998, blockResult.get().getLastCommitted());
// 999 is committed and then the remaining elements get confirmed
generator.commit(999);
block = repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(1991, block.getLastCommitted());
}

Expand All @@ -310,8 +307,7 @@ public void assertRecoverNoPendingCommit() throws Exception {

MonotonicAccessionGenerator generatorRecovering =
new MonotonicAccessionGenerator(CATEGORY_ID, INSTANCE_ID, service, new long[]{2, 3, 5});
ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(-1, block.getLastCommitted());
assertFalse(generatorRecovering.getAvailableRanges().isEmpty());
assertThat(generatorRecovering.getAvailableRanges(),
Expand All @@ -328,8 +324,7 @@ public void assertRecoverPendingCommit() throws Exception {

MonotonicAccessionGenerator generatorRecovering = new MonotonicAccessionGenerator(
CATEGORY_ID, INSTANCE_ID, service, new long[]{2, 3, 5});
ContiguousIdBlock block =
repository.findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID, INSTANCE_ID);
ContiguousIdBlock block = findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(CATEGORY_ID);
assertEquals(3, block.getLastCommitted());
assertThat(generatorRecovering.getAvailableRanges(),
contains(new MonotonicRange(4, 4), new MonotonicRange(6, BLOCK_SIZE - 1)));
Expand Down Expand Up @@ -464,9 +459,7 @@ public void testInitializeBlockManager() {
repository.save(block);

// To start with Block is UnCompleted and UnReserved
List<ContiguousIdBlock> blockInDBList = repository
.findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(CATEGORY_ID_2, INSTANCE_ID)
.collect(Collectors.toList());
List<ContiguousIdBlock> blockInDBList = findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(CATEGORY_ID_2);
assertEquals(1, blockInDBList.size());
List<ContiguousIdBlock> unreservedBlocks = blockInDBList.stream()
.filter(b -> b.isNotFull() && b.isNotReserved())
Expand All @@ -483,9 +476,7 @@ public void testInitializeBlockManager() {
assertEquals(new MonotonicRange(0, 9), generator1.getAvailableRanges().peek());

// Block is currently reserved by Generator-1
blockInDBList = repository
.findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(CATEGORY_ID_2, INSTANCE_ID)
.collect(Collectors.toList());
blockInDBList = findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(CATEGORY_ID_2);
assertEquals(1, blockInDBList.size());
List<ContiguousIdBlock> reservedBlocks = blockInDBList.stream()
.filter(b -> b.isNotFull() && b.isReserved())
Expand Down Expand Up @@ -519,4 +510,17 @@ public void testShutDownAccessionGenerator() {
assertThrows(AccessionGeneratorShutDownException.class, () -> generator.postSave(new SaveResponse<>()));
assertThrows(AccessionGeneratorShutDownException.class, () -> generator.getAvailableRanges());
}

private List<ContiguousIdBlock> findAllByCategoryIdAndApplicationInstanceIdOrderByLastValueAsc(String categoryId) {
return getAllBlocksForCategoryId(repository, categoryId).stream()
.sorted(Comparator.comparing(ContiguousIdBlock::getLastValue))
.collect(Collectors.toList());
}

private ContiguousIdBlock findFirstByCategoryIdAndApplicationInstanceIdOrderByLastValueDesc(String categoryId) {
return getAllBlocksForCategoryId(repository, categoryId).stream()
.sorted(Comparator.comparing(ContiguousIdBlock::getLastValue).reversed())
.findFirst().get();
}

}
Loading

0 comments on commit 6988429

Please sign in to comment.