Skip to content

Commit 3035d44

Browse files
committed
feat: Default bulk export manifest to plain JSON format
Build native JSON first in ExportResponse, avoiding lossy Parameters-to-JSON conversion. The manifest now always includes output and error as arrays, with native JSON attached via userData for direct extraction by the interceptor. Invert default in ParametersToJsonInterceptor to return plain JSON unless client explicitly requests application/fhir+json.
1 parent 8ff1b38 commit 3035d44

File tree

4 files changed

+217
-27
lines changed

4 files changed

+217
-27
lines changed

server/src/main/java/au/csiro/pathling/interceptors/ParametersToJsonInterceptor.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public class ParametersToJsonInterceptor {
6161

6262
private static final String APPLICATION_JSON = "application/json";
6363
private static final String APPLICATION_FHIR_JSON = "application/fhir+json";
64-
private static final String WILDCARD = "*/*";
64+
private static final String NATIVE_JSON_KEY = "nativeJson";
6565

6666
private final ObjectMapper mapper = new ObjectMapper();
6767

@@ -100,8 +100,10 @@ public boolean transformResponse(
100100
return true;
101101
}
102102

103-
// Transform Parameters to plain JSON and write directly to the response.
104-
final String json = parametersToJson(parameters);
103+
// Check for native JSON first (used by ExportResponse for lossless conversion).
104+
final Object nativeJson = parameters.getUserData(NATIVE_JSON_KEY);
105+
final String json = (nativeJson != null) ? (String) nativeJson : parametersToJson(parameters);
106+
105107
response.setContentType(APPLICATION_JSON);
106108
response.setCharacterEncoding("UTF-8");
107109
response.getWriter().write(json);
@@ -111,27 +113,22 @@ public boolean transformResponse(
111113
}
112114

113115
/**
114-
* Determines whether the client prefers plain JSON based on the Accept header. Returns true only
115-
* if the Accept header explicitly requests application/json.
116+
* Determines whether the client prefers plain JSON based on the Accept header. Returns true by
117+
* default, only returning false if the client explicitly requests FHIR JSON.
116118
*
117119
* @param request the HTTP servlet request
118120
* @return true if the client prefers plain JSON
119121
*/
120122
private boolean prefersPlainJson(@Nonnull final HttpServletRequest request) {
121123
final String acceptHeader = request.getHeader("Accept");
122124

123-
// No Accept header or wildcard means continue with normal FHIR processing.
124-
if (acceptHeader == null || acceptHeader.isBlank() || acceptHeader.contains(WILDCARD)) {
125-
return false;
126-
}
127-
128-
// If explicitly requesting FHIR JSON, continue with normal processing.
129-
if (acceptHeader.contains(APPLICATION_FHIR_JSON)) {
125+
// Only use FHIR format if explicitly requested.
126+
if (acceptHeader != null && acceptHeader.contains(APPLICATION_FHIR_JSON)) {
130127
return false;
131128
}
132129

133-
// Only transform if explicitly requesting plain JSON.
134-
return acceptHeader.contains(APPLICATION_JSON);
130+
// Default to plain JSON for all other cases.
131+
return true;
135132
}
136133

137134
/**

server/src/main/java/au/csiro/pathling/operations/bulkexport/ExportResponse.java

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
import au.csiro.pathling.OperationResponse;
2121
import au.csiro.pathling.library.io.sink.FileInformation;
2222
import au.csiro.pathling.library.io.sink.WriteDetails;
23+
import au.csiro.pathling.shaded.com.fasterxml.jackson.databind.ObjectMapper;
24+
import au.csiro.pathling.shaded.com.fasterxml.jackson.databind.node.ArrayNode;
25+
import au.csiro.pathling.shaded.com.fasterxml.jackson.databind.node.ObjectNode;
2326
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
2427
import jakarta.annotation.Nonnull;
2528
import java.net.URISyntaxException;
@@ -41,6 +44,9 @@
4144
*/
4245
public class ExportResponse implements OperationResponse<Parameters> {
4346

47+
private static final String NATIVE_JSON_KEY = "nativeJson";
48+
private static final ObjectMapper MAPPER = new ObjectMapper();
49+
4450
@Nonnull private final String kickOffRequestUrl;
4551

4652
@Nonnull private final String serverBaseUrl;
@@ -72,14 +78,66 @@ public ExportResponse(
7278
@Nonnull
7379
@Override
7480
public Parameters toOutput() {
75-
final Parameters parameters = new Parameters();
76-
7781
// Ensure the base URL ends with a slash for proper URL construction.
7882
final String normalizedBaseUrl =
7983
serverBaseUrl.endsWith("/") ? serverBaseUrl : serverBaseUrl + "/";
8084

85+
// Build native JSON first.
86+
final ObjectNode json = buildJson(normalizedBaseUrl);
87+
88+
// Convert to Parameters for FHIR compatibility.
89+
final Parameters parameters = jsonToParameters(json, normalizedBaseUrl);
90+
91+
// Attach native JSON for direct extraction by the interceptor.
92+
parameters.setUserData(NATIVE_JSON_KEY, json.toString());
93+
94+
return parameters;
95+
}
96+
97+
/**
98+
* Builds the native JSON representation of the export manifest.
99+
*
100+
* @param normalizedBaseUrl the normalised base server URL
101+
* @return the JSON object
102+
*/
103+
@Nonnull
104+
private ObjectNode buildJson(@Nonnull final String normalizedBaseUrl) {
105+
final ObjectNode json = MAPPER.createObjectNode();
106+
json.put("transactionTime", InstantType.now().getValueAsString());
107+
json.put("request", kickOffRequestUrl);
108+
json.put("requiresAccessToken", requiresAccessToken);
109+
110+
// Output is always an array.
111+
final ArrayNode outputArray = json.putArray("output");
112+
for (final FileInformation fileInfo : writeDetails.fileInfos()) {
113+
final ObjectNode entry = outputArray.addObject();
114+
entry.put("type", fileInfo.fhirResourceType());
115+
entry.put("url", buildResultUrl(normalizedBaseUrl, fileInfo.absoluteUrl()));
116+
}
117+
118+
// Error is always an array (even when empty).
119+
json.putArray("error");
120+
121+
return json;
122+
}
123+
124+
/**
125+
* Converts the JSON manifest to FHIR Parameters for compatibility.
126+
*
127+
* @param json the JSON object
128+
* @param normalizedBaseUrl the normalised base server URL
129+
* @return the Parameters resource
130+
*/
131+
@Nonnull
132+
private Parameters jsonToParameters(
133+
@Nonnull final ObjectNode json, @Nonnull final String normalizedBaseUrl) {
134+
final Parameters parameters = new Parameters();
135+
81136
// Add transactionTime parameter.
82-
parameters.addParameter().setName("transactionTime").setValue(InstantType.now());
137+
parameters
138+
.addParameter()
139+
.setName("transactionTime")
140+
.setValue(new InstantType(json.get("transactionTime").asText()));
83141

84142
// Add request parameter.
85143
parameters.addParameter().setName("request").setValue(new UriType(kickOffRequestUrl));
@@ -100,6 +158,9 @@ public Parameters toOutput() {
100158
.setValue(new UriType(buildResultUrl(normalizedBaseUrl, fileInfo.absoluteUrl())));
101159
}
102160

161+
// Add empty error parameter to match JSON structure.
162+
parameters.addParameter().setName("error");
163+
103164
return parameters;
104165
}
105166

server/src/test/java/au/csiro/pathling/interceptors/ParametersToJsonInterceptorTest.java

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,20 +62,22 @@ void setUp() {
6262
// -------------------------------------------------------------------------
6363

6464
@Test
65-
void continuesNormallyWhenNoAcceptHeader() throws Exception {
66-
// When no Accept header is present, continue with normal FHIR processing.
65+
void transformsToJsonWhenNoAcceptHeader() throws Exception {
66+
// When no Accept header is present, default to plain JSON.
6767
final Parameters parameters = new Parameters();
6868
parameters.addParameter().setName("test").setValue(new StringType("value"));
6969

70+
final StringWriter writer = new StringWriter();
7071
final HttpServletRequest request = mockRequest(null);
71-
final HttpServletResponse response = mockResponse(new StringWriter());
72+
final HttpServletResponse response = mockResponse(writer);
7273
final RequestDetails requestDetails = mock(RequestDetails.class);
7374

7475
final boolean result =
7576
interceptor.transformResponse(requestDetails, parameters, request, response);
7677

77-
assertThat(result).isTrue();
78-
verify(response, never()).setContentType("application/json");
78+
assertThat(result).isFalse();
79+
verify(response).setContentType("application/json");
80+
verify(response).setCharacterEncoding("UTF-8");
7981
}
8082

8183
@Test
@@ -96,20 +98,22 @@ void continuesNormallyWhenAcceptIsFhirJson() throws Exception {
9698
}
9799

98100
@Test
99-
void continuesNormallyWhenAcceptIsWildcard() throws Exception {
100-
// When Accept is */*, continue with normal FHIR processing.
101+
void transformsToJsonWhenAcceptIsWildcard() throws Exception {
102+
// When Accept is */*, default to plain JSON.
101103
final Parameters parameters = new Parameters();
102104
parameters.addParameter().setName("test").setValue(new StringType("value"));
103105

106+
final StringWriter writer = new StringWriter();
104107
final HttpServletRequest request = mockRequest("*/*");
105-
final HttpServletResponse response = mockResponse(new StringWriter());
108+
final HttpServletResponse response = mockResponse(writer);
106109
final RequestDetails requestDetails = mock(RequestDetails.class);
107110

108111
final boolean result =
109112
interceptor.transformResponse(requestDetails, parameters, request, response);
110113

111-
assertThat(result).isTrue();
112-
verify(response, never()).setContentType("application/json");
114+
assertThat(result).isFalse();
115+
verify(response).setContentType("application/json");
116+
verify(response).setCharacterEncoding("UTF-8");
113117
}
114118

115119
@Test
@@ -301,6 +305,40 @@ void handlesEmptyParameters() throws Exception {
301305
assertThat(node.isEmpty()).isTrue();
302306
}
303307

308+
// -------------------------------------------------------------------------
309+
// Native JSON tests
310+
// -------------------------------------------------------------------------
311+
312+
@Test
313+
void usesNativeJsonWhenAvailable() throws Exception {
314+
// When Parameters has native JSON attached, it should be used directly.
315+
final Parameters parameters = new Parameters();
316+
parameters.addParameter().setName("test").setValue(new StringType("converted-value"));
317+
318+
// Attach native JSON with a different value to prove it's being used.
319+
final String nativeJson = "{\"test\":\"native-value\",\"output\":[],\"error\":[]}";
320+
parameters.setUserData("nativeJson", nativeJson);
321+
322+
final String json = transformAndCapture(parameters);
323+
final JsonNode node = mapper.readTree(json);
324+
325+
// Should use the native JSON value, not the converted one.
326+
assertThat(node.get("test").asText()).isEqualTo("native-value");
327+
}
328+
329+
@Test
330+
void fallsBackToConversionWhenNoNativeJson() throws Exception {
331+
// When Parameters does not have native JSON, conversion should be used.
332+
final Parameters parameters = new Parameters();
333+
parameters.addParameter().setName("test").setValue(new StringType("converted-value"));
334+
335+
final String json = transformAndCapture(parameters);
336+
final JsonNode node = mapper.readTree(json);
337+
338+
// Should use the converted value.
339+
assertThat(node.get("test").asText()).isEqualTo("converted-value");
340+
}
341+
304342
// -------------------------------------------------------------------------
305343
// Helper methods
306344
// -------------------------------------------------------------------------

server/src/test/java/au/csiro/pathling/operations/bulkexport/ExportResponseTest.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
import au.csiro.pathling.library.io.sink.FileInformation;
2323
import au.csiro.pathling.library.io.sink.WriteDetails;
24+
import au.csiro.pathling.shaded.com.fasterxml.jackson.databind.JsonNode;
25+
import au.csiro.pathling.shaded.com.fasterxml.jackson.databind.ObjectMapper;
2426
import jakarta.annotation.Nullable;
2527
import java.util.List;
2628
import org.hl7.fhir.r4.model.BooleanType;
@@ -228,6 +230,98 @@ void serverBaseUrlWithTrailingSlashHandledCorrectly() {
228230
assertThat(url).startsWith("http://example.org/fhir/$result").doesNotContain("fhir//$result");
229231
}
230232

233+
// -------------------------------------------------------------------------
234+
// Native JSON tests
235+
// -------------------------------------------------------------------------
236+
237+
@Test
238+
void manifestIncludesEmptyErrorArray() {
239+
// The manifest should always include an error parameter, even when there are no errors.
240+
final WriteDetails writeDetails = new WriteDetails(List.of());
241+
242+
final ExportResponse response =
243+
new ExportResponse(
244+
"http://example.org/fhir/$export", "http://example.org/fhir", writeDetails, false);
245+
246+
final Parameters parameters = response.toOutput();
247+
248+
assertThat(hasParameter(parameters, "error")).isTrue();
249+
}
250+
251+
@Test
252+
void manifestAttachesNativeJson() {
253+
// The Parameters should have native JSON attached via userData.
254+
final WriteDetails writeDetails = new WriteDetails(List.of());
255+
256+
final ExportResponse response =
257+
new ExportResponse(
258+
"http://example.org/fhir/$export", "http://example.org/fhir", writeDetails, false);
259+
260+
final Parameters parameters = response.toOutput();
261+
final Object nativeJson = parameters.getUserData("nativeJson");
262+
263+
assertThat(nativeJson).isNotNull();
264+
assertThat(nativeJson).isInstanceOf(String.class);
265+
}
266+
267+
@Test
268+
void nativeJsonHasCorrectStructure() throws Exception {
269+
// The native JSON should have output and error as arrays, with all expected fields.
270+
final FileInformation patientFile =
271+
new FileInformation("Patient", "file:///tmp/jobs/job-id/Patient.ndjson");
272+
final FileInformation observationFile =
273+
new FileInformation("Observation", "file:///tmp/jobs/job-id/Observation.ndjson");
274+
final WriteDetails writeDetails = new WriteDetails(List.of(patientFile, observationFile));
275+
276+
final ExportResponse response =
277+
new ExportResponse(
278+
"http://example.org/fhir/$export", "http://example.org/fhir", writeDetails, false);
279+
280+
final Parameters parameters = response.toOutput();
281+
final String nativeJson = (String) parameters.getUserData("nativeJson");
282+
final ObjectMapper mapper = new ObjectMapper();
283+
final JsonNode json = mapper.readTree(nativeJson);
284+
285+
// Verify all expected fields are present.
286+
assertThat(json.has("transactionTime")).isTrue();
287+
assertThat(json.has("request")).isTrue();
288+
assertThat(json.has("requiresAccessToken")).isTrue();
289+
assertThat(json.has("output")).isTrue();
290+
assertThat(json.has("error")).isTrue();
291+
292+
// Verify output is always an array.
293+
assertThat(json.get("output").isArray()).isTrue();
294+
assertThat(json.get("output").size()).isEqualTo(2);
295+
assertThat(json.get("output").get(0).get("type").asText()).isEqualTo("Patient");
296+
assertThat(json.get("output").get(1).get("type").asText()).isEqualTo("Observation");
297+
298+
// Verify error is always an array (empty when no errors).
299+
assertThat(json.get("error").isArray()).isTrue();
300+
assertThat(json.get("error").isEmpty()).isTrue();
301+
302+
// Verify field values.
303+
assertThat(json.get("request").asText()).isEqualTo("http://example.org/fhir/$export");
304+
assertThat(json.get("requiresAccessToken").asBoolean()).isFalse();
305+
}
306+
307+
@Test
308+
void nativeJsonOutputIsEmptyArrayWhenNoFiles() throws Exception {
309+
// When there are no files, output should be an empty array, not missing.
310+
final WriteDetails writeDetails = new WriteDetails(List.of());
311+
312+
final ExportResponse response =
313+
new ExportResponse(
314+
"http://example.org/fhir/$export", "http://example.org/fhir", writeDetails, false);
315+
316+
final Parameters parameters = response.toOutput();
317+
final String nativeJson = (String) parameters.getUserData("nativeJson");
318+
final ObjectMapper mapper = new ObjectMapper();
319+
final JsonNode json = mapper.readTree(nativeJson);
320+
321+
assertThat(json.get("output").isArray()).isTrue();
322+
assertThat(json.get("output").isEmpty()).isTrue();
323+
}
324+
231325
// -------------------------------------------------------------------------
232326
// Helper methods
233327
// -------------------------------------------------------------------------

0 commit comments

Comments
 (0)