Skip to content

Commit 0d3e05e

Browse files
Lucas0TLucas0T
committed
Fix Torch does not provide Base URL in Response
1 parent 9aff9ef commit 0d3e05e

File tree

10 files changed

+91
-80
lines changed

10 files changed

+91
-80
lines changed

‎src/main/java/de/medizininformatikinitiative/torch/rest/FhirController.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.slf4j.Logger;
88
import org.slf4j.LoggerFactory;
99
import org.springframework.beans.factory.annotation.Autowired;
10+
import org.springframework.beans.factory.annotation.Value;
1011
import org.springframework.context.annotation.Bean;
1112
import org.springframework.http.HttpStatus;
1213
import org.springframework.http.MediaType;
@@ -43,14 +44,16 @@ public class FhirController {
4344
private final ResultFileManager resultFileManager;
4445
private final ExtractDataParametersParser extractDataParametersParser;
4546
private final ExtractDataService extractDataService;
47+
private final String baseUrl;
4648

4749
@Autowired
4850
public FhirController(FhirContext fhirContext, ResultFileManager resultFileManager,
49-
ExtractDataParametersParser parser, ExtractDataService extractDataService) {
51+
ExtractDataParametersParser parser, ExtractDataService extractDataService, @Value("${torch.base.url}") String baseUrl) {
5052
this.fhirContext = requireNonNull(fhirContext);
5153
this.resultFileManager = requireNonNull(resultFileManager);
5254
this.extractDataParametersParser = requireNonNull(parser);
5355
this.extractDataService = requireNonNull(extractDataService);
56+
this.baseUrl = baseUrl;
5457
}
5558

5659
private Mono<ServerResponse> getGlobalStatus(ServerRequest serverRequest) {
@@ -94,9 +97,7 @@ public Mono<ServerResponse> handleExtractData(ServerRequest request) {
9497
jobMono.subscribe();
9598
return ServerResponse.accepted()
9699
.header("Content-Location",
97-
request.uriBuilder()
98-
.replacePath("/fhir/__status/" + jobId)
99-
.build().toString())
100+
baseUrl + "/fhir/__status/" + jobId)
100101
.build();
101102
}).onErrorResume(Exception.class, e -> {
102103
HttpStatus status = (e instanceof IllegalArgumentException)

‎src/main/java/de/medizininformatikinitiative/torch/util/CrtdlFactory.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package de.medizininformatikinitiative.torch.util;
2+
3+
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import de.medizininformatikinitiative.torch.model.crtdl.Crtdl;
7+
import de.medizininformatikinitiative.torch.model.crtdl.DataExtraction;
8+
9+
import java.util.List;
10+
11+
public class CrtdlFactory {
12+
13+
private static final ObjectMapper objectMapper = new ObjectMapper();
14+
15+
/**
16+
* Returns a Crtdl instance with empty/default fields.
17+
*/
18+
public static Crtdl empty() {
19+
JsonNode emptyCohort = objectMapper.createObjectNode(); // empty JSON
20+
DataExtraction emptyExtraction = new DataExtraction(List.of()); // minimal instance
21+
return new Crtdl(emptyCohort, emptyExtraction);
22+
}
23+
}

‎src/test/java/de/medizininformatikinitiative/torch/BlackBoxIntegrationTestEnv.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,4 @@ public FileServerClient fileServerClient() {
8282
.baseUrl("http://%s:%s".formatted(host, port))
8383
.build());
8484
}
85-
86-
public int getTorchPort() {
87-
return environment.getServicePort("torch", 8080);
88-
}
8985
}

‎src/test/java/de/medizininformatikinitiative/torch/CdsBlackBoxIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
* => mvn clean package -DskipTests && docker build -t torch:latest .
2525
*/
2626
@Testcontainers
27-
public class CdsBlackBoxIT {
27+
class CdsBlackBoxIT {
2828

2929
private static final Logger logger = LoggerFactory.getLogger(CdsBlackBoxIT.class);
3030

@@ -52,7 +52,7 @@ static void tearDown() {
5252
}
5353

5454
@Test
55-
public void testExamples() throws IOException {
55+
void testExamples() throws IOException {
5656
var statusUrl = torchClient.executeExtractData(TestUtils.loadCrtdl("CRTDL_test_it-kds-crtdl.json")).block();
5757
assertThat(statusUrl).isNotNull();
5858
var statusResponse = torchClient.pollStatus(statusUrl).block();

‎src/test/java/de/medizininformatikinitiative/torch/CdsPerformanceBlackBoxIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* => mvn clean package -DskipTests && docker build -t torch:latest . && mvn -P blackbox-integration-tests -B verify
1818
*/
1919
@Testcontainers
20-
public class CdsPerformanceBlackBoxIT {
20+
class CdsPerformanceBlackBoxIT {
2121

2222
private static final Logger logger = LoggerFactory.getLogger(CdsPerformanceBlackBoxIT.class);
2323

‎src/test/java/de/medizininformatikinitiative/torch/FhirControllerIT.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.springframework.beans.factory.annotation.Value;
3232
import org.springframework.boot.test.context.SpringBootTest;
3333
import org.springframework.boot.test.web.client.TestRestTemplate;
34-
import org.springframework.boot.test.web.server.LocalServerPort;
3534
import org.springframework.http.HttpEntity;
3635
import org.springframework.http.HttpHeaders;
3736
import org.springframework.http.HttpMethod;
@@ -60,7 +59,7 @@
6059
import static org.assertj.core.api.Assertions.assertThat;
6160

6261
@ActiveProfiles("test")
63-
@SpringBootTest(properties = {"spring.main.allow-bean-definition-overriding=true"}, classes = Torch.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
62+
@SpringBootTest(properties = {"spring.main.allow-bean-definition-overriding=true"}, classes = Torch.class, webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
6463
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
6564
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
6665
class FhirControllerIT {
@@ -90,7 +89,8 @@ class FhirControllerIT {
9089
ResultFileManager resultFileManager;
9190
@Value("${torch.fhir.testPopulation.path}")
9291
private String testPopulationPath;
93-
@LocalServerPort
92+
93+
@Value("${server.port}")
9494
private int port;
9595

9696
@BeforeAll

‎src/test/java/de/medizininformatikinitiative/torch/rest/FhirControllerTest.java‎

Lines changed: 54 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package de.medizininformatikinitiative.torch.rest;
22

33
import ca.uhn.fhir.context.FhirContext;
4+
import de.medizininformatikinitiative.torch.model.crtdl.ExtractDataParameters;
45
import de.medizininformatikinitiative.torch.service.ExtractDataService;
6+
import de.medizininformatikinitiative.torch.util.CrtdlFactory;
57
import de.medizininformatikinitiative.torch.util.ResultFileManager;
68
import org.junit.jupiter.api.BeforeEach;
79
import org.junit.jupiter.api.Nested;
@@ -14,20 +16,24 @@
1416
import org.springframework.http.HttpStatus;
1517
import org.springframework.http.MediaType;
1618
import org.springframework.test.web.reactive.server.WebTestClient;
19+
import reactor.core.publisher.Mono;
1720

1821
import java.io.IOException;
1922
import java.util.Collections;
2023
import java.util.HashMap;
2124
import java.util.Map;
2225

2326
import static org.assertj.core.api.Assertions.assertThat;
27+
import static org.mockito.ArgumentMatchers.any;
2428
import static org.mockito.ArgumentMatchers.anyString;
2529
import static org.mockito.ArgumentMatchers.eq;
2630
import static org.mockito.Mockito.when;
2731

2832
@ExtendWith(MockitoExtension.class)
2933
class FhirControllerTest {
3034

35+
private final static String BASE_URL = "http://base-url";
36+
3137
@Mock
3238
ResultFileManager resultFileManager;
3339

@@ -42,25 +48,32 @@ class FhirControllerTest {
4248
@BeforeEach
4349
void setup() {
4450
FhirContext fhirContext = FhirContext.forR4();
45-
FhirController fhirController = new FhirController(
46-
fhirContext,
47-
resultFileManager,
48-
extractDataParametersParser,
49-
extractDataService
50-
);
51+
FhirController fhirController = new FhirController(fhirContext, resultFileManager, extractDataParametersParser, extractDataService, BASE_URL);
5152
client = WebTestClient.bindToRouterFunction(fhirController.queryRouter()).build();
5253
}
5354

5455
@Test
5556
void checkAcceptedStatus() {
5657
when(resultFileManager.getStatus("accepted-job")).thenReturn(HttpStatus.ACCEPTED);
5758

58-
var response = client.get()
59-
.uri("/fhir/__status/{jobId}", "accepted-job")
60-
.exchange();
59+
var response = client.get().uri("/fhir/__status/{jobId}", "accepted-job").exchange();
60+
61+
response.expectStatus().isEqualTo(HttpStatus.ACCEPTED).expectBody().isEmpty();
62+
}
63+
64+
@Test
65+
void extractDataSuccess() {
66+
67+
ExtractDataParameters params = new ExtractDataParameters(CrtdlFactory.empty(), Collections.emptyList());
68+
when(extractDataParametersParser.parseParameters(any())).thenReturn(params);
69+
when(extractDataService.startJob(any(), any(), any())).thenReturn(Mono.empty());
70+
71+
WebTestClient.ResponseSpec response = client.post().uri("/fhir/$extract-data").contentType(MediaType.APPLICATION_JSON).bodyValue("{}").exchange();
72+
73+
response.expectStatus().isAccepted().expectHeader()
74+
.value("Content-Location",
75+
location -> assertThat(location).startsWith(BASE_URL + "/fhir/__status/"));
6176

62-
response.expectStatus().isEqualTo(HttpStatus.ACCEPTED)
63-
.expectBody().isEmpty();
6477
}
6578

6679
@ParameterizedTest
@@ -77,52 +90,42 @@ void loadStatusBranches(String scenario, String expectedContentType, int expecte
7790

7891
case "ok" -> {
7992
when(resultFileManager.getStatus(jobId)).thenReturn(HttpStatus.OK);
80-
when(resultFileManager.loadBundleFromFileSystem(eq(jobId), anyString()))
81-
.thenReturn(Map.of("message", "bundle content"));
93+
when(resultFileManager.loadBundleFromFileSystem(eq(jobId), anyString())).thenReturn(Map.of("message", "bundle content"));
8294
}
8395
case "ok-null" -> {
8496
when(resultFileManager.getStatus(jobId)).thenReturn(HttpStatus.OK);
85-
when(resultFileManager.loadBundleFromFileSystem(eq(jobId), anyString()))
86-
.thenReturn(Map.of());
97+
when(resultFileManager.loadBundleFromFileSystem(eq(jobId), anyString())).thenReturn(Map.of());
8798
}
8899
case "missing" -> {
89100
when(resultFileManager.getStatus(jobId)).thenReturn(HttpStatus.NOT_FOUND);
90-
when(resultFileManager.loadErrorFromFileSystem(jobId))
91-
.thenReturn("Error file not found for job: " + jobId);
101+
when(resultFileManager.loadErrorFromFileSystem(jobId)).thenReturn("Error file not found for job: " + jobId);
92102
}
93103
case "error" -> {
94104
when(resultFileManager.getStatus(jobId)).thenReturn(HttpStatus.INTERNAL_SERVER_ERROR);
95-
when(resultFileManager.loadErrorFromFileSystem(jobId))
96-
.thenReturn("{\"resourceType\":\"OperationOutcome\"}");
105+
when(resultFileManager.loadErrorFromFileSystem(jobId)).thenReturn("{\"resourceType\":\"OperationOutcome\"}");
97106
}
98107
case "read-fail" -> {
99108
when(resultFileManager.getStatus(jobId)).thenReturn(HttpStatus.INTERNAL_SERVER_ERROR);
100109
// Throw the IOException when loadErrorFromFileSystem is called
101-
when(resultFileManager.loadErrorFromFileSystem(jobId))
102-
.thenAnswer(invocation -> {
103-
throw new IOException("disk failure");
104-
});
110+
when(resultFileManager.loadErrorFromFileSystem(jobId)).thenAnswer(invocation -> {
111+
throw new IOException("disk failure");
112+
});
105113
}
106114
default -> throw new IllegalArgumentException("Unexpected scenario: " + scenario);
107115
}
108116

109-
var response = client.get()
110-
.uri("/fhir/__status/{jobId}", jobId)
111-
.exchange();
112-
113-
response.expectStatus().isEqualTo(expectedStatus)
114-
.expectHeader().contentType(expectedContentType)
115-
.expectBody(String.class)
116-
.value(body -> {
117-
switch (scenario) {
118-
case "ok" -> assertThat(body).contains("bundle content");
119-
case "ok-null" -> assertThat(body).contains("Results could not be loaded for job: test-job");
120-
case "error" -> assertThat(body).contains("\"resourceType\":\"OperationOutcome\"");
121-
case "missing" -> assertThat(body).contains("Error file not found for job");
122-
case "read-fail" -> assertThat(body).contains("Error file could not be read: disk failure");
123-
default -> throw new IllegalArgumentException("Unexpected scenario: " + scenario);
124-
}
125-
});
117+
var response = client.get().uri("/fhir/__status/{jobId}", jobId).exchange();
118+
119+
response.expectStatus().isEqualTo(expectedStatus).expectHeader().contentType(expectedContentType).expectBody(String.class).value(body -> {
120+
switch (scenario) {
121+
case "ok" -> assertThat(body).contains("bundle content");
122+
case "ok-null" -> assertThat(body).contains("Results could not be loaded for job: test-job");
123+
case "error" -> assertThat(body).contains("\"resourceType\":\"OperationOutcome\"");
124+
case "missing" -> assertThat(body).contains("Error file not found for job");
125+
case "read-fail" -> assertThat(body).contains("Error file could not be read: disk failure");
126+
default -> throw new IllegalArgumentException("Unexpected scenario: " + scenario);
127+
}
128+
});
126129
}
127130

128131

@@ -133,14 +136,10 @@ class StatusEndpointTests {
133136
void emptyStatusMapReturnsEmptyJson() {
134137
when(resultFileManager.getJobStatusMap()).thenReturn(Collections.emptyMap());
135138

136-
var response = client.get()
137-
.uri("/fhir/__status/");
139+
var response = client.get().uri("/fhir/__status/").exchange();
138140

139-
response.exchange()
140-
.expectStatus().isOk()
141-
.expectHeader().contentType(MediaType.APPLICATION_JSON)
142-
.expectBody()
143-
.json("{}");
141+
response.expectStatus().isOk().expectHeader().contentType(MediaType.APPLICATION_JSON)
142+
.expectBody().json("{}");
144143
}
145144

146145
@Test
@@ -150,11 +149,9 @@ void statusMapReturnsStatusText() {
150149
map.put("job2", HttpStatus.OK);
151150
when(resultFileManager.getJobStatusMap()).thenReturn(map);
152151

153-
var response = client.get()
154-
.uri("/fhir/__status/");
152+
var response = client.get().uri("/fhir/__status/").exchange();
155153

156-
response.exchange().expectStatus().isOk()
157-
.expectHeader().contentType(MediaType.APPLICATION_JSON)
154+
response.expectStatus().isOk().expectHeader().contentType(MediaType.APPLICATION_JSON)
158155
.expectBody()
159156
.jsonPath("$.job1").isEqualTo("202 Accepted")
160157
.jsonPath("$.job2").isEqualTo("200 OK");
@@ -166,27 +163,21 @@ class ExtractDataErrorBranchTests {
166163

167164
@Test
168165
void emptyRequestBodyTriggersBadRequest() {
169-
client.post()
170-
.uri("/fhir/$extract-data")
171-
.exchange()
172-
.expectStatus().isBadRequest()
166+
var response = client.post().uri("/fhir/$extract-data").exchange();
167+
168+
response.expectStatus().isBadRequest()
173169
.expectHeader().contentType("application/fhir+json")
174170
.expectBody()
175171
.jsonPath("$.resourceType").isEqualTo("OperationOutcome");
176172
}
177173

178174
@Test
179175
void blankRequestBodyTriggersBadRequest() {
180-
client.post()
181-
.uri("/fhir/$extract-data")
182-
.contentType(MediaType.APPLICATION_JSON)
183-
.bodyValue(" ")
184-
.exchange()
185-
.expectStatus().isBadRequest()
186-
.expectHeader().contentType("application/fhir+json")
176+
var response = client.post().uri("/fhir/$extract-data").contentType(MediaType.APPLICATION_JSON).bodyValue(" ").exchange();
177+
178+
response.expectStatus().isBadRequest().expectHeader().contentType("application/fhir+json")
187179
.expectBody()
188180
.jsonPath("$.resourceType").isEqualTo("OperationOutcome");
189-
190181
}
191182
}
192183
}

‎src/test/resources/application-test.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ server:
44
port: 8086
55
torch:
66
base:
7-
url: dummy.torch.server
7+
url: http://localhost:8086
88
profile:
99
dir: structureDefinitions
1010
mapping:

‎src/test/resources/docker-compose-performance.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ services:
33
restart: unless-stopped
44
image: torch:latest
55
ports:
6-
- "8080"
6+
- "8080:8080"
77
environment:
88
TORCH_PROFILE_DIR: /app/structureDefinitions
99
TORCH_MAPPING_CONSENT: /app/mappings/consent-mappings_fhir.json

‎src/test/resources/docker-compose.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ services:
4646
restart: unless-stopped
4747
image: torch:latest
4848
ports:
49-
- "8080"
49+
- "8080:8080"
5050
environment:
5151
TORCH_PROFILE_DIR: /app/structureDefinitions
5252
TORCH_MAPPING_CONSENT: /app/mappings/consent-mappings_fhir.json

0 commit comments

Comments
 (0)