From 6b5917d8695361ee039fb6622b1bb7dc6d24f58f Mon Sep 17 00:00:00 2001 From: waterflow80 Date: Sat, 13 Jan 2024 15:49:49 +0100 Subject: [PATCH] removed unnecessary json response for some ingestion cases and set the right response status codes --- .../controller/admin/AdminController.java | 7 ++- .../AssemblyAlreadyIngestedException.java | 7 +++ .../eva/evaseqcol/service/SeqColService.java | 37 ++++++---------- .../AdminControllerIntegrationTest.java | 44 +++++++++++++++++++ 4 files changed, 70 insertions(+), 25 deletions(-) create mode 100644 src/main/java/uk/ac/ebi/eva/evaseqcol/exception/AssemblyAlreadyIngestedException.java diff --git a/src/main/java/uk/ac/ebi/eva/evaseqcol/controller/admin/AdminController.java b/src/main/java/uk/ac/ebi/eva/evaseqcol/controller/admin/AdminController.java index 3c3b74b..adef458 100644 --- a/src/main/java/uk/ac/ebi/eva/evaseqcol/controller/admin/AdminController.java +++ b/src/main/java/uk/ac/ebi/eva/evaseqcol/controller/admin/AdminController.java @@ -12,6 +12,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; +import uk.ac.ebi.eva.evaseqcol.exception.AssemblyAlreadyIngestedException; import uk.ac.ebi.eva.evaseqcol.exception.AssemblyNotFoundException; import uk.ac.ebi.eva.evaseqcol.exception.DuplicateSeqColException; import uk.ac.ebi.eva.evaseqcol.exception.IncorrectAccessionException; @@ -42,7 +43,7 @@ public AdminController(SeqColService seqColService) { "contained in the assembly report) and eventually save these seqCol objects into the database. " + "This is an authenticated endpoint, so it requires admin privileges to run it.") @ApiResponses(value = { - @ApiResponse(responseCode = "200", description = "seqCol object(s) successfully inserted"), + @ApiResponse(responseCode = "201", description = "seqCol object(s) successfully inserted"), @ApiResponse(responseCode = "409", description = "seqCol object(s) already exist(s)"), @ApiResponse(responseCode = "404", description = "Assembly not found"), @ApiResponse(responseCode = "400", description = "Bad request. (It can be a bad accession value)"), @@ -56,7 +57,7 @@ public ResponseEntity fetchAndInsertSeqColByAssemblyAccession( required = true) @PathVariable String asmAccession) { try { IngestionResultEntity ingestionResult = seqColService.fetchAndInsertAllSeqColByAssemblyAccession(asmAccession); - return ResponseEntity.ok(ingestionResult); + return new ResponseEntity<>(ingestionResult, HttpStatus.CREATED); } catch (IncorrectAccessionException e) { return new ResponseEntity<>(e.getMessage(), HttpStatus.BAD_REQUEST); } catch (IllegalArgumentException e) { @@ -69,6 +70,8 @@ public ResponseEntity fetchAndInsertSeqColByAssemblyAccession( return new ResponseEntity<>(e.getMessage(), HttpStatus.CONFLICT); } catch (AssemblyNotFoundException e) { return new ResponseEntity<>(e.getMessage(), HttpStatus.NOT_FOUND); + } catch (AssemblyAlreadyIngestedException e) { + return new ResponseEntity<>(e.getMessage(), HttpStatus.CONFLICT); } } } diff --git a/src/main/java/uk/ac/ebi/eva/evaseqcol/exception/AssemblyAlreadyIngestedException.java b/src/main/java/uk/ac/ebi/eva/evaseqcol/exception/AssemblyAlreadyIngestedException.java new file mode 100644 index 0000000..74b7360 --- /dev/null +++ b/src/main/java/uk/ac/ebi/eva/evaseqcol/exception/AssemblyAlreadyIngestedException.java @@ -0,0 +1,7 @@ +package uk.ac.ebi.eva.evaseqcol.exception; + +public class AssemblyAlreadyIngestedException extends RuntimeException{ + public AssemblyAlreadyIngestedException(String assemblyAccession) { + super("Seqcol objects for assembly " + assemblyAccession + " has been already ingested"); + } +} diff --git a/src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColService.java b/src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColService.java index 3343069..a733350 100644 --- a/src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColService.java +++ b/src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColService.java @@ -14,6 +14,8 @@ import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelOneEntity; import uk.ac.ebi.eva.evaseqcol.entities.SeqColExtendedDataEntity; import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelTwoEntity; +import uk.ac.ebi.eva.evaseqcol.exception.AssemblyAlreadyIngestedException; +import uk.ac.ebi.eva.evaseqcol.exception.AssemblyNotFoundException; import uk.ac.ebi.eva.evaseqcol.exception.AttributeNotDefinedException; import uk.ac.ebi.eva.evaseqcol.exception.DuplicateSeqColException; import uk.ac.ebi.eva.evaseqcol.exception.SeqColNotFoundException; @@ -158,17 +160,17 @@ public void removeAllSeqCol() { * assembly report. * Return the list of level 0 digests of the inserted seqcol objects*/ public IngestionResultEntity fetchAndInsertAllSeqColByAssemblyAccession( - String assemblyAccession) throws IOException, DuplicateSeqColException { - IngestionResultEntity ingestionResultEntity = new IngestionResultEntity(); - ingestionResultEntity.setAssemblyAccession(assemblyAccession); + String assemblyAccession) throws IOException, DuplicateSeqColException, AssemblyNotFoundException, + AssemblyAlreadyIngestedException{ Optional> seqColDataMap = ncbiSeqColDataSource .getAllPossibleSeqColExtendedData(assemblyAccession); if (!seqColDataMap.isPresent()) { logger.warn("No seqCol data corresponding to assemblyAccession " + assemblyAccession + " could be found on NCBI datasource"); - ingestionResultEntity.setErrorMessage("No seqCol data corresponding to assemblyAccession " + assemblyAccession + " could be found on NCBI datasource"); - return ingestionResultEntity; + throw new AssemblyNotFoundException(assemblyAccession); } + IngestionResultEntity ingestionResultEntity = new IngestionResultEntity(); + ingestionResultEntity.setAssemblyAccession(assemblyAccession); // Retrieving the Map's data List>> possibleSequencesNamesList = (List>>) seqColDataMap.get().get("namesAttributes"); @@ -185,10 +187,6 @@ public IngestionResultEntity fetchAndInsertAllSeqColByAssemblyAccession( SeqColExtendedDataEntity> extendedSortedNameLengthPair = SeqColExtendedDataEntity. constructSeqColSortedNameLengthPairs(extendedNamesEntity, extendedLengthsEntity); - -// seqColExtendedDataEntities.add(extendedNamesEntity); -// seqColExtendedDataEntities.add(seqColSortedNameLengthPairEntity); - // Constructing a list of seqColExtData that has the type List List>> seqColStringListExtDataEntities = levelOneService.constructStringListExtDataEntities(sameValueAttributesMap, extendedNamesEntity, @@ -218,23 +216,16 @@ public IngestionResultEntity fetchAndInsertAllSeqColByAssemblyAccession( } catch (DuplicateSeqColException e) { logger.info("Seqcol for " + assemblyAccession + " and naming convention " + extendedNamesEntity.getNamingConvention() + " already exists. Skipping."); - continue; } } - return ingestionResultEntity; - } - - /** - * Return the extended data entity that corresponds to the seqCol lengths attribute*/ - // TODO: REFACTOR - /*public SeqColExtendedDataEntity retrieveExtendedLengthEntity(List extendedDataEntities) { - for (SeqColExtendedDataEntity entity: extendedDataEntities) { - if (entity.getAttributeType() == SeqColExtendedDataEntity.AttributeType.lengths) { - return entity; - } + if (ingestionResultEntity.getNumberOfInsertedSeqcols() == 0) { + logger.warn("Seqcol objects for assembly " + assemblyAccession + " has been already ingested"); + throw new AssemblyAlreadyIngestedException(assemblyAccession); + } else { + return ingestionResultEntity; } - return null; - }*/ + + } @Transactional /** diff --git a/src/test/java/uk/ac/ebi/eva/evaseqcol/controller/AdminControllerIntegrationTest.java b/src/test/java/uk/ac/ebi/eva/evaseqcol/controller/AdminControllerIntegrationTest.java index ecbef5b..0dbd088 100644 --- a/src/test/java/uk/ac/ebi/eva/evaseqcol/controller/AdminControllerIntegrationTest.java +++ b/src/test/java/uk/ac/ebi/eva/evaseqcol/controller/AdminControllerIntegrationTest.java @@ -1,15 +1,22 @@ package uk.ac.ebi.eva.evaseqcol.controller; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.http.client.support.BasicAuthenticationInterceptor; import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; import org.testcontainers.containers.PostgreSQLContainer; import org.testcontainers.junit.jupiter.Container; @@ -17,6 +24,7 @@ import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelOneEntity; import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelTwoEntity; +import uk.ac.ebi.eva.evaseqcol.exception.AssemblyAlreadyIngestedException; import uk.ac.ebi.eva.evaseqcol.service.SeqColService; import java.util.Optional; @@ -72,6 +80,11 @@ void setUp() { baseUrl = baseUrl + ":" + port + contextPath + ADMIN_PATH; } + @AfterEach + void tearDown() { + seqColService.removeAllSeqCol(); + } + @Test @Transactional /** @@ -89,4 +102,35 @@ void ingestSeqColsTest() { assertNotNull(levelTwoEntity.get().getLengths()); } + @Test + /** + * Testing the response for the ingestion endpoint for + * different kind of ingestion cases: + * 1. Not existed + * 2. Already existed + * 3. Invalid assembly accession + * 4. Not found assembly accession + * Note: the order of execution is important */ + void ingestionResponseTest() { + // 1. Testing Valid Non-Existing Accession + String finalRequest1 = baseUrl + "/" + ASM_ACCESSION; + ResponseEntity putResponse1 = restTemplate.exchange(finalRequest1, HttpMethod.PUT, null, String.class); + assertEquals(HttpStatus.CREATED, putResponse1.getStatusCode()); + + // 2. Testing Valid Existing Accession + final String finalRequest2 = baseUrl + "/" + ASM_ACCESSION; // Same as above + assertThrows(HttpClientErrorException.Conflict.class, + () -> restTemplate.exchange(finalRequest2, HttpMethod.PUT, null, String.class)); + + // 3. Testing Invalid Assembly Accession + final String finalRequest3 = baseUrl + "/" + ASM_ACCESSION.substring(0, ASM_ACCESSION.length()-4); // Less than 15 characters + assertThrows(HttpClientErrorException.BadRequest.class, + () -> restTemplate.exchange(finalRequest3, HttpMethod.PUT, null, String.class)); + + // 4. Testing Assembly Not Found + final String finalRequest4 = baseUrl + "/" + ASM_ACCESSION + "55"; // Accession doesn't correspond to any assembly + assertThrows(HttpClientErrorException.NotFound.class, + () -> restTemplate.exchange(finalRequest4, HttpMethod.PUT, null, String.class)); + } + }