Change export transactionTime to end-time. (#6842)

* Change export transactionTime to end-time.

Author: michaelabuckley

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6841-fix-export-transactionTime.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6841
+title: "The bulk export status reported by $poll-export-status now correctly shows the start-time, not the end-time.
+   This avoids missing changes or updates in subsequent $export calls which use this as the `_since` parameter."

hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java

@@ -46,6 +46,7 @@
 import ca.uhn.fhir.util.JsonUtil;
 import ca.uhn.fhir.util.OperationOutcomeUtil;
 import com.google.common.annotations.VisibleForTesting;
+import jakarta.annotation.Nonnull;
 import jakarta.servlet.http.HttpServletResponse;
 import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
 import org.hl7.fhir.instance.model.api.IIdType;
@@ -398,41 +399,13 @@ public void exportPollStatus(
 					response.setStatus(Constants.STATUS_HTTP_200_OK);
 					response.setContentType(Constants.CT_JSON);
 
-					// Create a JSON response
-					BulkExportResponseJson bulkResponseDocument = new BulkExportResponseJson();
-					bulkResponseDocument.setTransactionTime(info.getEndTime()); // completed
-
-					bulkResponseDocument.setRequiresAccessToken(true);
-
-					String report = info.getReport();
-					if (isEmpty(report)) {
+					if (isEmpty(info.getReport())) {
 						// this should never happen, but just in case...
 						ourLog.error("No report for completed bulk export job.");
 						response.getWriter().close();
 					} else {
-						BulkExportJobResults results = JsonUtil.deserialize(report, BulkExportJobResults.class);
-						bulkResponseDocument.setMsg(results.getReportMsg());
-						bulkResponseDocument.setRequest(results.getOriginalRequestUrl());
-
 						String serverBase = BulkDataExportUtil.getServerBase(theRequestDetails);
-
-						// an output is required, even if empty, according to HL7 FHIR IG
-						bulkResponseDocument.getOutput();
-
-						for (Map.Entry<String, List<String>> entrySet :
-								results.getResourceTypeToBinaryIds().entrySet()) {
-							String resourceType = entrySet.getKey();
-							List<String> binaryIds = entrySet.getValue();
-							for (String binaryId : binaryIds) {
-								IIdType iId = new IdType(binaryId);
-								String nextUrl = serverBase + "/"
-										+ iId.toUnqualifiedVersionless().getValue();
-								bulkResponseDocument
-										.addOutput()
-										.setType(resourceType)
-										.setUrl(nextUrl);
-							}
-						}
+						BulkExportResponseJson bulkResponseDocument = buildCompleteResponseDocument(serverBase, info);
 						JsonUtil.serialize(bulkResponseDocument, response.getWriter());
 						response.getWriter().close();
 					}
@@ -449,6 +422,7 @@ public void exportPollStatus(
 				myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToWriter(oo, response.getWriter());
 				response.getWriter().close();
 				break;
+				//noinspection DefaultNotLastCaseInSwitch
 			default:
 				// Deliberate fall through
 				ourLog.warn(
@@ -459,6 +433,7 @@ public void exportPollStatus(
 			case QUEUED:
 			case IN_PROGRESS:
 			case CANCELLED:
+				//noinspection deprecation - we need to support old jobs after upgrade.
 			case ERRORED:
 				if (theRequestDetails.getRequestType() == RequestTypeEnum.DELETE) {
 					handleDeleteRequest(theJobId, response, info.getStatus());
@@ -474,6 +449,34 @@ public void exportPollStatus(
 		}
 	}
 
+	@Nonnull
+	static BulkExportResponseJson buildCompleteResponseDocument(String theServerBase, JobInstance theJob) {
+		// Create a JSON response
+		BulkExportResponseJson response = new BulkExportResponseJson();
+		response.setTransactionTime(theJob.getStartTime());
+		response.setRequiresAccessToken(true);
+
+		BulkExportJobResults results = JsonUtil.deserialize(theJob.getReport(), BulkExportJobResults.class);
+		response.setMsg(results.getReportMsg());
+		response.setRequest(results.getOriginalRequestUrl());
+
+		// an output is required, even if empty, according to HL7 FHIR IG
+		response.getOutput();
+
+		for (Map.Entry<String, List<String>> entrySet :
+				results.getResourceTypeToBinaryIds().entrySet()) {
+			String resourceType = entrySet.getKey();
+			List<String> binaryIds = entrySet.getValue();
+			for (String binaryId : binaryIds) {
+				IIdType iId = new IdType(binaryId);
+				String nextUrl =
+						theServerBase + "/" + iId.toUnqualifiedVersionless().getValue();
+				response.addOutput().setType(resourceType).setUrl(nextUrl);
+			}
+		}
+		return response;
+	}
+
 	private void handleDeleteRequest(
 			IPrimitiveType<String> theJobId, HttpServletResponse response, StatusEnum theOrigStatus)
 			throws IOException {

hapi-fhir-storage-batch2-jobs/src/test/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProviderTest.java

@@ -1,22 +1,29 @@
 package ca.uhn.fhir.batch2.jobs.export;
 
+import ca.uhn.fhir.batch2.model.JobInstance;
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
+import ca.uhn.fhir.jpa.api.model.BulkExportJobResults;
+import ca.uhn.fhir.jpa.bulk.export.model.BulkExportResponseJson;
+import ca.uhn.fhir.util.JsonUtil;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Stream;
 
 import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 @ExtendWith(MockitoExtension.class)
 class BulkDataExportProviderTest {
-	private static final Set<FhirVersionEnum> PATIENT_COMPARTMENT_FHIR_VERSIONS_SUPPORT_DEVICE = Set.of(FhirVersionEnum.DSTU2, FhirVersionEnum.DSTU2_1, FhirVersionEnum.DSTU2_HL7ORG, FhirVersionEnum.DSTU3, FhirVersionEnum.R4, FhirVersionEnum.R4B);
-
 	private static Stream<Arguments> fhirContexts() {
 			return Stream.of(
 				Arguments.arguments(FhirContext.forDstu2()),
@@ -44,4 +51,38 @@ void checkDeviceIsSupportedInPatientCompartment(FhirContext theFhirContext) {
 			assertThat(resourceNames).doesNotContain("Device");
 		}
 	}
+
+	@Test
+	void testCompleteStatusDocument() {
+	    // given
+		JobInstance job = new JobInstance();
+
+		Date now = new Date();
+		Date start = new Date(now.getTime() - 10000);
+		Date end = new Date(now.getTime() - 2000);
+		job.setStartTime(start);
+		job.setEndTime(end);
+
+		String reportMessage = "Report Message";
+		BulkExportJobResults jobResults = new BulkExportJobResults();
+		jobResults.setReportMsg(reportMessage);
+		jobResults.setOriginalRequestUrl("http://example.com/fhir-endpoint/Group/123/$export");
+		jobResults.setResourceTypeToBinaryIds(Map.of("Patient", List.of("Binary/1123", "Binary/1124")));
+
+		job.setReport(JsonUtil.serialize(jobResults));
+
+	    // when
+		BulkExportResponseJson response = BulkDataExportProvider.buildCompleteResponseDocument("http://example.com/fhir-endpoint", job);
+
+	    // then
+		// see https://hl7.org/fhir/uv/bulkdata/export.html#response---complete-status
+	    assertEquals(start, response.getTransactionTime(), "transactionTime should be useful as the _since param in the next $export request");
+	    assertEquals(reportMessage, response.getMsg());
+		assertEquals(true, response.getRequiresAccessToken());
+		assertEquals(jobResults.getOriginalRequestUrl(), response.getRequest());
+		assertEquals(2, response.getOutput().size());
+		assertEquals(new BulkExportResponseJson.Output().setType("Patient").setUrl("http://example.com/fhir-endpoint/Binary/1123"), response.getOutput().get(0));
+
+	}
+
 }

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/model/BulkExportResponseJson.java

@@ -27,6 +27,8 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 
 import java.util.ArrayList;
 import java.util.Date;
@@ -52,15 +54,16 @@ public class BulkExportResponseJson {
 	@JsonProperty("requiresAccessToken")
 	private Boolean myRequiresAccessToken;
 
+	@JsonInclude
 	@JsonProperty("output")
-	private List<Output> myOutput;
+	private final List<Output> myOutput = new ArrayList<>();
 
 	/*
 	 * Note that we override the include here as ONC regulations require that we actually serialize the empty error array.
 	 */
 	@JsonInclude
 	@JsonProperty("error")
-	private List<Output> myError = new ArrayList<>();
+	private final List<Output> myError = new ArrayList<>();
 
 	@JsonProperty("message")
 	private String myMsg;
@@ -93,16 +96,10 @@ public BulkExportResponseJson setRequiresAccessToken(Boolean theRequiresAccessTo
 	}
 
 	public List<Output> getOutput() {
-		if (myOutput == null) {
-			myOutput = new ArrayList<>();
-		}
 		return myOutput;
 	}
 
 	public List<Output> getError() {
-		if (myError == null) {
-			myError = new ArrayList<>();
-		}
 		return myError;
 	}
 
@@ -145,5 +142,24 @@ public Output setUrl(String theUrl) {
 			myUrl = theUrl;
 			return this;
 		}
+
+		@Override
+		public boolean equals(Object theO) {
+			if (this == theO) return true;
+
+			if (!(theO instanceof Output)) return false;
+
+			Output output = (Output) theO;
+
+			return new EqualsBuilder()
+					.append(myType, output.myType)
+					.append(myUrl, output.myUrl)
+					.isEquals();
+		}
+
+		@Override
+		public int hashCode() {
+			return new HashCodeBuilder(17, 37).append(myType).append(myUrl).toHashCode();
+		}
 	}
 }