From 7268ef679c7bb299c8fe7d8f824a39fcf45b3d9d Mon Sep 17 00:00:00 2001 From: jochen-vermeulen <35036658+jochen-vermeulen@users.noreply.github.com> Date: Thu, 5 Sep 2024 12:29:45 +0200 Subject: [PATCH] Feat/met 5712 add status for problem pattern analysis (#162) * MET-5712: Return status for problem pattern analysis. * MET-5712: Improve unit tests. Return 500 in case of error. --- .../controller/PatternAnalysisController.java | 57 +++++++++---- .../PatternAnalysisControllerTest.java | 83 ++++++++++++++----- 2 files changed, 104 insertions(+), 36 deletions(-) diff --git a/src/main/java/eu/europeana/metis/sandbox/controller/PatternAnalysisController.java b/src/main/java/eu/europeana/metis/sandbox/controller/PatternAnalysisController.java index e374c2bb..572c5456 100644 --- a/src/main/java/eu/europeana/metis/sandbox/controller/PatternAnalysisController.java +++ b/src/main/java/eu/europeana/metis/sandbox/controller/PatternAnalysisController.java @@ -99,33 +99,53 @@ public PatternAnalysisController(PatternAnalysisService pa @GetMapping(value = "{id}/get-dataset-pattern-analysis", produces = APPLICATION_JSON_VALUE) public ResponseEntity> getDatasetPatternAnalysis( @Parameter(description = "id of the dataset", required = true) @PathVariable("id") String datasetId) { - Optional datasetExecutionPointOptional = executionPointService.getExecutionPoint(datasetId, - Step.VALIDATE_INTERNAL.toString()); - return datasetExecutionPointOptional.flatMap(executionPoint -> { - finalizeDatasetPatternAnalysis(datasetId, executionPoint); - return patternAnalysisService.getDatasetPatternAnalysis( - datasetId, Step.VALIDATE_INTERNAL, datasetExecutionPointOptional.get().getExecutionTimestamp()); - }).map(analysis -> new ResponseEntity<>(new DatasetProblemPatternAnalysisView<>(analysis), HttpStatus.OK)) - .orElseGet(() -> new ResponseEntity<>( - DatasetProblemPatternAnalysisView.getEmptyDatasetProblemPatternAnalysisView(), - HttpStatus.NOT_FOUND)); + // Get the execution point. If it does not exist, we are done. + final ExecutionPoint executionPoint = executionPointService + .getExecutionPoint(datasetId, Step.VALIDATE_INTERNAL.toString()).orElse(null); + if (executionPoint == null) { + return new ResponseEntity<>(DatasetProblemPatternAnalysisView.getEmptyAnalysis(datasetId, + ProblemPatternAnalysisStatus.PENDING), HttpStatus.NOT_FOUND); + } + + // Finalize the problem pattern analysis if we can (i.e. if the dataset finished processing). + final ProblemPatternAnalysisStatus status = finalizeDatasetPatternAnalysis(datasetId, executionPoint); + + // Now get the pattern analysis. + final DatasetProblemPatternAnalysisView analysisView = patternAnalysisService.getDatasetPatternAnalysis( + datasetId, Step.VALIDATE_INTERNAL, executionPoint.getExecutionTimestamp()) + .map(analysis -> new DatasetProblemPatternAnalysisView<>(analysis, status)) + .orElseGet(() -> { + LOGGER.error("Result not expected to be empty when there is a non-null execution point."); + return DatasetProblemPatternAnalysisView.getEmptyAnalysis(datasetId, + ProblemPatternAnalysisStatus.ERROR); + }); + + // Wrap and return. + final HttpStatus httpStatus = analysisView.analysisStatus == ProblemPatternAnalysisStatus.ERROR + ? HttpStatus.INTERNAL_SERVER_ERROR : HttpStatus.OK; + return new ResponseEntity<>(analysisView, httpStatus); } - private void finalizeDatasetPatternAnalysis(String datasetId, ExecutionPoint datasetExecutionPoint) { + private ProblemPatternAnalysisStatus finalizeDatasetPatternAnalysis(String datasetId, + ExecutionPoint datasetExecutionPoint) { if (datasetReportService.getReport(datasetId).getStatus() == Status.COMPLETED) { final Lock lock = datasetIdLocksMap.computeIfAbsent(datasetId, s -> lockRegistry.obtain("finalizePatternAnalysis_" + datasetId)); try { lock.lock(); LOGGER.debug("Finalize analysis: {} lock, Locked", datasetId); patternAnalysisService.finalizeDatasetPatternAnalysis(datasetExecutionPoint); + return ProblemPatternAnalysisStatus.FINALIZED; } catch (PatternAnalysisException e) { LOGGER.error("Something went wrong during finalizing pattern analysis", e); + return ProblemPatternAnalysisStatus.ERROR; } finally { lock.unlock(); LOGGER.debug("Finalize analysis: {} lock, Unlocked", datasetId); } } + // This method is only executed if we have an execution point, i.e. if processing is underway. + return ProblemPatternAnalysisStatus.IN_PROGRESS; } /** @@ -191,6 +211,8 @@ private void cleanCache() { } } + enum ProblemPatternAnalysisStatus {PENDING, IN_PROGRESS, FINALIZED, ERROR} + private static final class DatasetProblemPatternAnalysisView { @JsonProperty @@ -201,18 +223,23 @@ private static final class DatasetProblemPatternAnalysisView { private final String executionTimestamp; @JsonProperty private final List problemPatternList; + @JsonProperty + private final ProblemPatternAnalysisStatus analysisStatus; - private DatasetProblemPatternAnalysisView(DatasetProblemPatternAnalysis datasetProblemPatternAnalysis) { + private DatasetProblemPatternAnalysisView(DatasetProblemPatternAnalysis datasetProblemPatternAnalysis, + ProblemPatternAnalysisStatus status) { this.datasetId = datasetProblemPatternAnalysis.getDatasetId(); this.executionStep = datasetProblemPatternAnalysis.getExecutionStep(); this.executionTimestamp = datasetProblemPatternAnalysis.getExecutionTimestamp() == null ? null : datasetProblemPatternAnalysis.getExecutionTimestamp().toString(); this.problemPatternList = getSortedProblemPatternList(datasetProblemPatternAnalysis); + this.analysisStatus = status; } - private static DatasetProblemPatternAnalysisView getEmptyDatasetProblemPatternAnalysisView() { - return new DatasetProblemPatternAnalysisView<>(new DatasetProblemPatternAnalysis<>("0", null, null, - new ArrayList<>())); + private static DatasetProblemPatternAnalysisView getEmptyAnalysis(String datasetId, + ProblemPatternAnalysisStatus status) { + return new DatasetProblemPatternAnalysisView<>(new DatasetProblemPatternAnalysis<>( + datasetId, null, null, new ArrayList<>()), status); } @NotNull diff --git a/src/test/java/eu/europeana/metis/sandbox/controller/PatternAnalysisControllerTest.java b/src/test/java/eu/europeana/metis/sandbox/controller/PatternAnalysisControllerTest.java index f47c8d5e..548e41f1 100644 --- a/src/test/java/eu/europeana/metis/sandbox/controller/PatternAnalysisControllerTest.java +++ b/src/test/java/eu/europeana/metis/sandbox/controller/PatternAnalysisControllerTest.java @@ -2,19 +2,26 @@ import static java.util.Collections.emptyList; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import eu.europeana.metis.sandbox.common.Step; +import eu.europeana.metis.sandbox.controller.PatternAnalysisController.ProblemPatternAnalysisStatus; import eu.europeana.metis.sandbox.controller.ratelimit.RateLimitInterceptor; import eu.europeana.metis.sandbox.dto.report.ProgressInfoDto; +import eu.europeana.metis.sandbox.dto.report.ProgressInfoDto.Status; import eu.europeana.metis.sandbox.entity.RecordLogEntity; import eu.europeana.metis.sandbox.entity.problempatterns.ExecutionPoint; import eu.europeana.metis.sandbox.service.dataset.DatasetReportService; @@ -39,6 +46,7 @@ import java.util.Optional; import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.io.IOUtils; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; @@ -75,8 +83,16 @@ class PatternAnalysisControllerTest { private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSSSS"); + @BeforeEach + void resetMocks() { + reset(mockPatternAnalysisService, mockExecutionPointService, mockRecordLogService, + mockDatasetReportService, lockRegistry); + } + @Test void getDatasetAnalysis_expectSuccess() throws Exception { + + // Set up the tests LocalDateTime executionTimestamp = LocalDateTime.now(); ExecutionPoint executionPoint = new ExecutionPoint(); executionPoint.setExecutionTimestamp(executionTimestamp); @@ -91,16 +107,35 @@ void getDatasetAnalysis_expectSuccess() throws Exception { .thenReturn(Optional.of(datasetProblemPatternAnalysis)); when(lockRegistry.obtain(anyString())).thenReturn(new ReentrantLock()); - when(mockDatasetReportService.getReport("datasetId")).thenReturn( - new ProgressInfoDto("", 1L, 1L, Collections.emptyList(), false, "", emptyList(), null)); - doNothing().when(mockPatternAnalysisService).finalizeDatasetPatternAnalysis(executionPoint); + // First check with dataset that is still processing + final ProgressInfoDto inProgressInfo = new ProgressInfoDto("", 1L, 0L, + Collections.emptyList(), false, "", emptyList(), null); + when(mockDatasetReportService.getReport("datasetId")).thenReturn(inProgressInfo); + assertNotEquals(Status.COMPLETED, inProgressInfo.getStatus()); mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.datasetId", is("datasetId"))) - .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) - .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) - .andExpect(jsonPath("$.problemPatternList", is(Collections.EMPTY_LIST))); + .andExpect(status().isOk()) + .andExpect(jsonPath("$.datasetId", is("datasetId"))) + .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) + .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) + .andExpect(jsonPath("$.problemPatternList", is(Collections.EMPTY_LIST))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.IN_PROGRESS.name()))); + verify(mockPatternAnalysisService, never()).finalizeDatasetPatternAnalysis(any()); + + // Now check with finalized dataset. + final ProgressInfoDto completedInfo = new ProgressInfoDto("", 1L, 1L, + Collections.emptyList(), false, "", emptyList(), null); + when(mockDatasetReportService.getReport("datasetId")).thenReturn(completedInfo); + assertEquals(Status.COMPLETED, completedInfo.getStatus()); + + mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.datasetId", is("datasetId"))) + .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) + .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) + .andExpect(jsonPath("$.problemPatternList", is(Collections.EMPTY_LIST))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.FINALIZED.name()))); + verify(mockPatternAnalysisService, times(1)).finalizeDatasetPatternAnalysis(executionPoint); } @Test @@ -130,13 +165,13 @@ void getDatasetAnalysis_withProblemPatternList_checkSorting_expectSuccess() thro when(mockDatasetReportService.getReport("datasetId")).thenReturn( new ProgressInfoDto("", 1L, 1L, Collections.emptyList(), false, "", emptyList(), null)); - doNothing().when(mockPatternAnalysisService).finalizeDatasetPatternAnalysis(executionPoint); mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) .andExpect(status().isOk()) .andExpect(jsonPath("$.datasetId", is("datasetId"))) .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.FINALIZED.name()))) .andExpect(jsonPath("$.problemPatternList[0].problemPatternDescription.problemPatternId", is(ProblemPatternDescription.ProblemPatternId.P2.toString()))) .andExpect(jsonPath("$.problemPatternList[0].recordAnalysisList[0].recordId", is("recordId1"))) @@ -146,6 +181,7 @@ void getDatasetAnalysis_withProblemPatternList_checkSorting_expectSuccess() thro .andExpect(jsonPath("$.problemPatternList[1].recordAnalysisList[0].recordId", is("recordId1"))) .andExpect(jsonPath("$.problemPatternList[1].recordAnalysisList[1].recordId", is("recordId2"))) .andExpect(jsonPath("$.problemPatternList[1].recordAnalysisList[2].recordId", is("recordId3"))); + verify(mockPatternAnalysisService, times(1)).finalizeDatasetPatternAnalysis(executionPoint); } @Test @@ -169,11 +205,13 @@ void getDatasetAnalysis_finalize_fail_expectSuccess() throws Exception { doThrow(PatternAnalysisException.class).when(mockPatternAnalysisService).finalizeDatasetPatternAnalysis(executionPoint); mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.datasetId", is("datasetId"))) - .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) - .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) - .andExpect(jsonPath("$.problemPatternList", is(Collections.EMPTY_LIST))); + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.datasetId", is("datasetId"))) + .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) + .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) + .andExpect(jsonPath("$.problemPatternList", is(Collections.EMPTY_LIST))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.ERROR.name()))); + verify(mockPatternAnalysisService, times(1)).finalizeDatasetPatternAnalysis(executionPoint); } @Test @@ -185,8 +223,10 @@ void getDatasetAnalysis_getEmptyResult_expectSuccess() throws Exception { .thenReturn(Optional.empty()); mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) - .andExpect(status().isNotFound()) - .andExpect(jsonPath("$.datasetId", is("0"))); + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.datasetId", is("datasetId"))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.PENDING.name()))); + verify(mockPatternAnalysisService, never()).finalizeDatasetPatternAnalysis(any()); } @Test @@ -204,11 +244,11 @@ void getDatasetAnalysis_executionPoint_getEmptyResult_expectSuccess() throws Exc when(mockDatasetReportService.getReport("datasetId")).thenReturn( new ProgressInfoDto("", 1L, 1L, Collections.emptyList(), false, "", emptyList(), null)); - doNothing().when(mockPatternAnalysisService).finalizeDatasetPatternAnalysis(executionPoint); mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) - .andExpect(status().isNotFound()) - .andExpect(jsonPath("$.datasetId", is("0"))); + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.datasetId", is("datasetId"))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.ERROR.name()))); } @Test @@ -284,13 +324,13 @@ void getDatasetAnalysis_withCleanMessageReportP7AndSorted_expectSuccess() throws when(mockDatasetReportService.getReport("datasetId")).thenReturn( new ProgressInfoDto("", 1L, 1L, Collections.emptyList(), false, "", emptyList(), null)); - doNothing().when(mockPatternAnalysisService).finalizeDatasetPatternAnalysis(executionPoint); mvc.perform(get("/pattern-analysis/{id}/get-dataset-pattern-analysis", "datasetId")) .andExpect(status().isOk()) .andExpect(jsonPath("$.datasetId", is("datasetId"))) .andExpect(jsonPath("$.executionStep", is(Step.VALIDATE_INTERNAL.name()))) .andExpect(jsonPath("$.executionTimestamp", is(executionTimestamp.toString()))) + .andExpect(jsonPath("$.analysisStatus", is(ProblemPatternAnalysisStatus.FINALIZED.name()))) .andExpect(jsonPath("$.problemPatternList[0].problemPatternDescription.problemPatternId", is(ProblemPatternDescription.ProblemPatternId.P7.toString()))) .andExpect(jsonPath("$.problemPatternList[0].recordAnalysisList[0].recordId", is("recordId1"))) @@ -299,6 +339,7 @@ void getDatasetAnalysis_withCleanMessageReportP7AndSorted_expectSuccess() throws .andExpect(jsonPath("$.problemPatternList[0].recordAnalysisList[1].problemOccurrenceList[0].messageReport", is(""))) .andExpect(jsonPath("$.problemPatternList[0].recordAnalysisList[2].recordId", is("recordId3"))) .andExpect(jsonPath("$.problemPatternList[0].recordAnalysisList[2].problemOccurrenceList[0].messageReport", is(""))); + verify(mockPatternAnalysisService, times(1)).finalizeDatasetPatternAnalysis(executionPoint); } @Test