diff --git a/pom.xml b/pom.xml index bc06050..d8425b3 100644 --- a/pom.xml +++ b/pom.xml @@ -79,6 +79,10 @@ hypersistence-utils-hibernate-55 3.7.5 + + org.hibernate + hibernate-envers + diff --git a/src/main/java/uk/ac/ebi/eva/submission/controller/admin/AdminController.java b/src/main/java/uk/ac/ebi/eva/submission/controller/admin/AdminController.java index e91a464..0b69e9a 100644 --- a/src/main/java/uk/ac/ebi/eva/submission/controller/admin/AdminController.java +++ b/src/main/java/uk/ac/ebi/eva/submission/controller/admin/AdminController.java @@ -48,8 +48,12 @@ public AdminController(SubmissionService submissionService, WebinTokenService we @PutMapping("submission/{submissionId}/status/{status}") public ResponseEntity markSubmissionStatus(@PathVariable("submissionId") String submissionId, @PathVariable("status") SubmissionStatus status) { - Submission submission = this.submissionService.markSubmissionStatus(submissionId, status); - return new ResponseEntity<>(stripUserDetails(submission), HttpStatus.OK); + try { + Submission submission = this.submissionService.markSubmissionStatus(submissionId, status); + return new ResponseEntity<>(stripUserDetails(submission), HttpStatus.OK); + } catch (SubmissionDoesNotExistException ex) { + return new ResponseEntity<>(ex.getMessage(), HttpStatus.NOT_FOUND); + } } @Operation(summary = "This endpoint retrieves all the submissions of a specific status present in the database") @@ -78,8 +82,12 @@ public ResponseEntity getSubmissionsbyStatus(@PathVariable("status") Submissi public ResponseEntity markSubmissionProcessStepAndStatus(@PathVariable("submissionId") String submissionId, @PathVariable("step") SubmissionProcessingStep step, @PathVariable("status") SubmissionProcessingStatus status) { - SubmissionProcessing submissionProc = this.submissionService.markSubmissionProcessStepAndStatus(submissionId, step, status); - return new ResponseEntity<>(submissionProc, HttpStatus.OK); + try { + SubmissionProcessing submissionProc = this.submissionService.markSubmissionProcessStepAndStatus(submissionId, step, status); + return new ResponseEntity<>(submissionProc, HttpStatus.OK); + } catch (SubmissionDoesNotExistException ex) { + return new ResponseEntity<>(ex.getMessage(), HttpStatus.NOT_FOUND); + } } @Operation(summary = "This endpoint retrieves all the submissions from the database with given step and status") diff --git a/src/main/java/uk/ac/ebi/eva/submission/entity/SubmissionProcessing.java b/src/main/java/uk/ac/ebi/eva/submission/entity/SubmissionProcessing.java index 4aa6378..52db166 100644 --- a/src/main/java/uk/ac/ebi/eva/submission/entity/SubmissionProcessing.java +++ b/src/main/java/uk/ac/ebi/eva/submission/entity/SubmissionProcessing.java @@ -1,6 +1,7 @@ package uk.ac.ebi.eva.submission.entity; import org.hibernate.annotations.UpdateTimestamp; +import org.hibernate.envers.Audited; import org.springframework.lang.NonNull; import javax.persistence.Column; @@ -12,7 +13,10 @@ import java.time.LocalDateTime; import java.util.Objects; +import static org.hibernate.envers.RelationTargetAuditMode.NOT_AUDITED; + @Entity +@Audited(targetAuditMode = NOT_AUDITED) @Table(schema = "eva_submissions", name = "submission_processing_status") public class SubmissionProcessing { diff --git a/src/main/java/uk/ac/ebi/eva/submission/exception/SubmissionDoesNotExistException.java b/src/main/java/uk/ac/ebi/eva/submission/exception/SubmissionDoesNotExistException.java index 9f2f6fc..52dfad8 100644 --- a/src/main/java/uk/ac/ebi/eva/submission/exception/SubmissionDoesNotExistException.java +++ b/src/main/java/uk/ac/ebi/eva/submission/exception/SubmissionDoesNotExistException.java @@ -1,10 +1,8 @@ package uk.ac.ebi.eva.submission.exception; -public class SubmissionDoesNotExistException extends RuntimeException{ - public SubmissionDoesNotExistException(){ - super("Submission does not exist"); - } - public SubmissionDoesNotExistException(String msg){ - super(msg); +public class SubmissionDoesNotExistException extends RuntimeException { + + public SubmissionDoesNotExistException(String submissionId) { + super("Submission with id " + submissionId + " does not exist"); } } diff --git a/src/main/java/uk/ac/ebi/eva/submission/service/SubmissionService.java b/src/main/java/uk/ac/ebi/eva/submission/service/SubmissionService.java index b87427e..cdc0152 100644 --- a/src/main/java/uk/ac/ebi/eva/submission/service/SubmissionService.java +++ b/src/main/java/uk/ac/ebi/eva/submission/service/SubmissionService.java @@ -109,7 +109,7 @@ public Submission uploadMetadataJsonAndMarkUploaded(String submissionId, JsonNod public String getSubmissionStatus(String submissionId) { Submission submission = submissionRepository.findBySubmissionId(submissionId); if (submission == null) { - throw new SubmissionDoesNotExistException("Submission with Id " + submissionId + " does not exist"); + throw new SubmissionDoesNotExistException(submissionId); } return submission.getStatus(); @@ -117,6 +117,9 @@ public String getSubmissionStatus(String submissionId) { public Submission markSubmissionStatus(String submissionId, SubmissionStatus status) { Submission submission = submissionRepository.findBySubmissionId(submissionId); + if (submission == null) { + throw new SubmissionDoesNotExistException(submissionId); + } submission.setStatus(status.toString()); if (status == SubmissionStatus.COMPLETED) { submission.setCompletionTime(LocalDateTime.now()); @@ -131,7 +134,7 @@ public boolean checkUserHasAccessToSubmission(SubmissionAccount account, String SubmissionAccount submissionAccount = optSubmission.get().getSubmissionAccount(); return submissionAccount.getId().equals(account.getId()); } else { - throw new SubmissionDoesNotExistException("Given submission with id " + submissionId + " does not exist"); + throw new SubmissionDoesNotExistException(submissionId); } } @@ -156,7 +159,16 @@ public List getSubmissionsByProcessingStepAndStatus(Submis public SubmissionProcessing markSubmissionProcessStepAndStatus(String submissionId, SubmissionProcessingStep step, SubmissionProcessingStatus status) { + Optional submission = submissionRepository.findById(submissionId); + if (!submission.isPresent()) { + throw new SubmissionDoesNotExistException(submissionId); + } + SubmissionProcessing submissionProc = submissionProcessingRepository.findBySubmissionId(submissionId); + if (submissionProc == null) { + submissionProc = new SubmissionProcessing(submissionId); + } + submissionProc.setStep(step.toString()); submissionProc.setStatus(status.toString()); return submissionProcessingRepository.save(submissionProc); diff --git a/src/test/java/uk/ac/ebi/eva/submission/integration/SubmissionWSIntegrationTest.java b/src/test/java/uk/ac/ebi/eva/submission/integration/SubmissionWSIntegrationTest.java index e3f2154..7aed856 100644 --- a/src/test/java/uk/ac/ebi/eva/submission/integration/SubmissionWSIntegrationTest.java +++ b/src/test/java/uk/ac/ebi/eva/submission/integration/SubmissionWSIntegrationTest.java @@ -1,7 +1,9 @@ package uk.ac.ebi.eva.submission.integration; import com.fasterxml.jackson.databind.ObjectMapper; -import org.jetbrains.annotations.NotNull; +import org.hibernate.envers.AuditReader; +import org.hibernate.envers.AuditReaderFactory; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -35,6 +37,7 @@ import uk.ac.ebi.eva.submission.service.WebinTokenService; import uk.ac.ebi.eva.submission.util.MailSender; +import javax.persistence.EntityManager; import java.time.LocalDateTime; import java.util.ArrayList; import java.util.HashSet; @@ -51,7 +54,9 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @SpringBootTest @AutoConfigureMockMvc @@ -64,6 +69,9 @@ public class SubmissionWSIntegrationTest { @Value("${controller.auth.admin.password}") private String TEST_ADMIN_PASSWORD; + @Autowired + private EntityManager entityManager; + @Autowired private MockMvc mvc; @@ -253,7 +261,7 @@ public void testSubmissionGetStatusSubmissionDoesNotExist() throws Exception { mvc.perform(get("/v1/submission/" + submissionId + "/status") .headers(httpHeaders) .contentType(MediaType.APPLICATION_JSON)) - .andExpectAll(status().isNotFound(), content().string("Submission with Id test123 does not exist")); + .andExpectAll(status().isNotFound(), content().string("Submission with id " + submissionId + " does not exist")); } @Test @@ -335,7 +343,7 @@ public void testSubmissionDoesNotExistException() throws Exception { .content(metadataJson) .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isNotFound()) - .andExpect(content().string("Given submission with id " + submissionId + " does not exist")); + .andExpect(content().string("Submission with id " + submissionId + " does not exist")); } @Test @@ -377,6 +385,23 @@ public void testMarkSubmissionStatusWrong() throws Exception { .andExpect(status().isBadRequest()); } + @Test + @Transactional + public void testMarkSubmissionStatusSubmissionDoesNotExist() throws Exception { + SubmissionAccount submissionAccount = getWebinUserAccount(); + when(webinTokenService.getWebinUserAccountFromToken(anyString())).thenReturn(submissionAccount); + + String submissionId = "test-wrong-submission-id"; + + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.setBasicAuth(TEST_ADMIN_USERNAME, TEST_ADMIN_PASSWORD); + mvc.perform(put("/v1/admin/submission/" + submissionId + "/status/COMPLETED") + .headers(httpHeaders) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isNotFound()) + .andExpect(content().string("Submission with id " + submissionId + " does not exist")); + } + @Test @Transactional public void testGetSubmissionByStatus() throws Exception { @@ -449,6 +474,98 @@ public void testMarkSubmissionProcessStepAndStatus() throws Exception { assertThat(submissionProc.getLastUpdateTime()).isNotNull(); } + @Test + @Transactional + public void testMarkSubmissionProcessStepAndStatusCreateFirstEntry() throws Exception { + SubmissionAccount submissionAccount = getWebinUserAccount(); + when(webinTokenService.getWebinUserAccountFromToken(anyString())).thenReturn(submissionAccount); + + String submissionId = createNewSubmissionEntry(submissionAccount, SubmissionStatus.PROCESSING); + + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.setBasicAuth(TEST_ADMIN_USERNAME, TEST_ADMIN_PASSWORD); + mvc.perform(put("/v1/admin/submission-process/" + submissionId + "/" + SubmissionProcessingStep.VALIDATION + "/" + SubmissionProcessingStatus.SUCCESS) + .headers(httpHeaders) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + + SubmissionProcessing submissionProc = submissionProcessingRepository.findBySubmissionId(submissionId); + assertThat(submissionProc).isNotNull(); + assertThat(submissionProc.getSubmissionId()).isEqualTo(submissionId); + assertThat(submissionProc.getStep()).isEqualTo(SubmissionProcessingStep.VALIDATION.toString()); + assertThat(submissionProc.getStatus()).isEqualTo(SubmissionProcessingStatus.SUCCESS.toString()); + assertThat(submissionProc.getLastUpdateTime()).isNotNull(); + } + + @Test + @Transactional + public void testMarkSubmissionProcessStepAndStatusSubmissionDoesNotExist() throws Exception { + SubmissionAccount submissionAccount = getWebinUserAccount(); + when(webinTokenService.getWebinUserAccountFromToken(anyString())).thenReturn(submissionAccount); + + String submissionId = "test-wrong-submission-id"; + + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.setBasicAuth(TEST_ADMIN_USERNAME, TEST_ADMIN_PASSWORD); + mvc.perform(put("/v1/admin/submission-process/" + submissionId + "/" + SubmissionProcessingStep.VALIDATION + "/" + SubmissionProcessingStatus.SUCCESS) + .headers(httpHeaders) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isNotFound()) + .andExpect(content().string("Submission with id " + submissionId + " does not exist")); + } + + + @Disabled + @Test + @Transactional + public void testSubmissionProcessingHistory() throws Exception { + SubmissionAccount submissionAccount = getWebinUserAccount(); + when(webinTokenService.getWebinUserAccountFromToken(anyString())).thenReturn(submissionAccount); + + String submissionId = createNewSubmissionEntry(submissionAccount, SubmissionStatus.PROCESSING); + + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.setBasicAuth(TEST_ADMIN_USERNAME, TEST_ADMIN_PASSWORD); + mvc.perform(put("/v1/admin/submission-process/" + submissionId + "/" + SubmissionProcessingStep.VALIDATION + "/" + SubmissionProcessingStatus.READY_FOR_PROCESSING) + .headers(httpHeaders) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + + SubmissionProcessing submissionProc = submissionProcessingRepository.findBySubmissionId(submissionId); + assertThat(submissionProc.getSubmissionId()).isEqualTo(submissionId); + assertThat(submissionProc.getStep()).isEqualTo(SubmissionProcessingStep.VALIDATION.toString()); + assertThat(submissionProc.getStatus()).isEqualTo(SubmissionProcessingStatus.READY_FOR_PROCESSING.toString()); + + mvc.perform(put("/v1/admin/submission-process/" + submissionId + "/" + SubmissionProcessingStep.VALIDATION + "/" + SubmissionProcessingStatus.FAILURE) + .headers(httpHeaders) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + + submissionProc = submissionProcessingRepository.findBySubmissionId(submissionId); + assertThat(submissionProc.getSubmissionId()).isEqualTo(submissionId); + assertThat(submissionProc.getStep()).isEqualTo(SubmissionProcessingStep.VALIDATION.toString()); + assertThat(submissionProc.getStatus()).isEqualTo(SubmissionProcessingStatus.FAILURE.toString()); + + // Assert Submission Processing History Entry inserted successfully + AuditReader auditReader = AuditReaderFactory.get(entityManager); + List revisions = auditReader.getRevisions(SubmissionProcessing.class, submissionId); + assertThat(revisions).isNotEmpty(); + + Number firstRevision = revisions.get(0); + SubmissionProcessing auditEntryFirstRev = auditReader.find(SubmissionProcessing.class, submissionId, firstRevision); + assertThat(auditEntryFirstRev).isNotNull(); + assertThat(auditEntryFirstRev.getSubmissionId()).isEqualTo(submissionId); + assertThat(auditEntryFirstRev.getStep()).isEqualTo(SubmissionProcessingStep.VALIDATION.toString()); + assertThat(auditEntryFirstRev.getStatus()).isEqualTo(SubmissionProcessingStatus.READY_FOR_PROCESSING.toString()); + + Number secondRevision = revisions.get(1); + SubmissionProcessing auditEntrySecondRev = auditReader.find(SubmissionProcessing.class, submissionId, secondRevision); + assertThat(auditEntrySecondRev).isNotNull(); + assertThat(auditEntrySecondRev.getSubmissionId()).isEqualTo(submissionId); + assertThat(auditEntrySecondRev.getStep()).isEqualTo(SubmissionProcessingStep.VALIDATION.toString()); + assertThat(auditEntrySecondRev.getStatus()).isEqualTo(SubmissionProcessingStatus.FAILURE.toString()); + } + private SubmissionAccount getWebinUserAccount() { String accountId = "webinAccountId";