From b965347b8b104c2ddbaba4506c8bcbf39edfce13 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 4 Nov 2022 07:21:06 -0400 Subject: [PATCH] Allow Batch2 transition from ERRORED to COMPLETE (#4242) * Allow Batch2 transition from ERRORED to COMPLETE * Add changelog * Test fix Co-authored-by: James Agnew --- ...ition-from-errored-to-complete-batch2.yaml | 4 ++ .../ca/uhn/fhir/batch2/model/StatusEnum.java | 2 +- .../progress/JobInstanceStatusUpdater.java | 2 +- .../uhn/fhir/batch2/model/StatusEnumTest.java | 4 +- .../JobInstanceStatusUpdaterTest.java | 50 ++++++++++++++----- 5 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4242-allow-transition-from-errored-to-complete-batch2.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4242-allow-transition-from-errored-to-complete-batch2.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4242-allow-transition-from-errored-to-complete-batch2.yaml new file mode 100644 index 00000000000..e34df4e634d --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4242-allow-transition-from-errored-to-complete-batch2.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 4242 +title: "Batch2 jobs were incorrectly prevented from transitioning from ERRORED to COMPLETE status." diff --git a/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/model/StatusEnum.java b/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/model/StatusEnum.java index 9628d28dee6..40be01e160a 100644 --- a/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/model/StatusEnum.java +++ b/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/model/StatusEnum.java @@ -146,7 +146,7 @@ public enum StatusEnum { case IN_PROGRESS: return theNewStatus != QUEUED; case ERRORED: - return theNewStatus == FAILED; + return theNewStatus == FAILED || theNewStatus == COMPLETED || theNewStatus == CANCELLED; case COMPLETED: case CANCELLED: case FAILED: diff --git a/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdater.java b/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdater.java index 062afa53e5b..400587c7dcf 100644 --- a/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdater.java +++ b/hapi-fhir-storage-batch2/src/main/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdater.java @@ -87,10 +87,10 @@ public class JobInstanceStatusUpdater { invokeCompletionHandler(theJobInstance, definition, definition.getCompletionHandler()); break; case FAILED: - case ERRORED: case CANCELLED: invokeCompletionHandler(theJobInstance, definition, definition.getErrorHandler()); break; + case ERRORED: case QUEUED: case IN_PROGRESS: default: diff --git a/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/model/StatusEnumTest.java b/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/model/StatusEnumTest.java index 8b07421d5ea..2faabc05431 100644 --- a/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/model/StatusEnumTest.java +++ b/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/model/StatusEnumTest.java @@ -50,8 +50,8 @@ class StatusEnumTest { "ERRORED, QUEUED, false", "ERRORED, IN_PROGRESS, false", - "ERRORED, COMPLETED, false", - "ERRORED, CANCELLED, false", + "ERRORED, COMPLETED, true", + "ERRORED, CANCELLED, true", "ERRORED, ERRORED, true", "ERRORED, FAILED, true", diff --git a/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdaterTest.java b/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdaterTest.java index a3035db7b48..a371783dbc5 100644 --- a/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdaterTest.java +++ b/hapi-fhir-storage-batch2/src/test/java/ca/uhn/fhir/batch2/progress/JobInstanceStatusUpdaterTest.java @@ -20,6 +20,7 @@ import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -28,7 +29,7 @@ class JobInstanceStatusUpdaterTest { private static final String TEST_NAME = "test name"; private static final String TEST_ERROR_MESSAGE = "test error message"; private static final int TEST_ERROR_COUNT = 729; - private final JobInstance ourQueuedInstance = new JobInstance().setStatus(StatusEnum.QUEUED); + private final JobInstance myQueuedInstance = new JobInstance().setStatus(StatusEnum.QUEUED); @Mock IJobPersistence myJobPersistence; @@ -51,36 +52,58 @@ class JobInstanceStatusUpdaterTest { myInstance.setParameters(myTestParameters); myInstance.setErrorMessage(TEST_ERROR_MESSAGE); myInstance.setErrorCount(TEST_ERROR_COUNT); - - when(myJobDefinition.getParametersType()).thenReturn(TestParameters.class); } @Test public void testCompletionHandler() { - AtomicReference calledDetails = new AtomicReference<>(); - // setup - when(myJobPersistence.fetchInstance(TEST_INSTANCE_ID)).thenReturn(Optional.of(ourQueuedInstance)); - when(myJobPersistence.updateInstance(myInstance)).thenReturn(true); - IJobCompletionHandler completionHandler = details -> calledDetails.set(details); - when(myJobDefinition.getCompletionHandler()).thenReturn(completionHandler); + setupCompleteCallback(); // execute mySvc.updateInstanceStatus(myInstance, StatusEnum.COMPLETED); - JobCompletionDetails receivedDetails = calledDetails.get(); + assertCompleteCallbackCalled(); + } + + @Test + public void testCompletionHandler_ERROR_to_COMPLETED() { + setupCompleteCallback(); + myInstance.setStatus(StatusEnum.ERRORED); + myQueuedInstance.setStatus(StatusEnum.ERRORED); + when(myJobDefinition.getParametersType()).thenReturn(TestParameters.class); + + // execute + mySvc.updateInstanceStatus(myInstance, StatusEnum.COMPLETED); + + assertCompleteCallbackCalled(); + } + + private void assertCompleteCallbackCalled() { + JobCompletionDetails receivedDetails = myDetails.get(); assertEquals(TEST_INSTANCE_ID, receivedDetails.getInstance().getInstanceId()); assertEquals(TEST_NAME, receivedDetails.getParameters().name); } + private void setupCompleteCallback() { + myDetails = new AtomicReference<>(); + when(myJobPersistence.fetchInstance(TEST_INSTANCE_ID)).thenReturn(Optional.of(myQueuedInstance)); + when(myJobPersistence.updateInstance(myInstance)).thenReturn(true); + IJobCompletionHandler completionHandler = details -> myDetails.set(details); + when(myJobDefinition.getCompletionHandler()).thenReturn(completionHandler); + when(myJobDefinition.getParametersType()).thenReturn(TestParameters.class); + } + @Test public void testErrorHandler_ERROR() { - setupErrorCallback(); + // setup + myDetails = new AtomicReference<>(); + when(myJobPersistence.fetchInstance(TEST_INSTANCE_ID)).thenReturn(Optional.of(myQueuedInstance)); + when(myJobPersistence.updateInstance(myInstance)).thenReturn(true); // execute mySvc.updateInstanceStatus(myInstance, StatusEnum.ERRORED); - assertErrorCallbackCalled(StatusEnum.ERRORED); + assertNull(myDetails.get()); } @Test @@ -117,10 +140,11 @@ class JobInstanceStatusUpdaterTest { myDetails = new AtomicReference<>(); // setup - when(myJobPersistence.fetchInstance(TEST_INSTANCE_ID)).thenReturn(Optional.of(ourQueuedInstance)); + when(myJobPersistence.fetchInstance(TEST_INSTANCE_ID)).thenReturn(Optional.of(myQueuedInstance)); when(myJobPersistence.updateInstance(myInstance)).thenReturn(true); IJobCompletionHandler errorHandler = details -> myDetails.set(details); when(myJobDefinition.getErrorHandler()).thenReturn(errorHandler); + when(myJobDefinition.getParametersType()).thenReturn(TestParameters.class); }