Skip to content

Commit 53a1339

Browse files
Lucas0TLucas0T
committed
Move Validation before the Pipeline
1 parent bb63cbe commit 53a1339

File tree

5 files changed

+71
-50
lines changed

5 files changed

+71
-50
lines changed

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package de.medizininformatikinitiative.torch.rest;
22

33
import ca.uhn.fhir.context.FhirContext;
4+
import de.medizininformatikinitiative.torch.exceptions.ValidationException;
5+
import de.medizininformatikinitiative.torch.service.CrtdlValidatorService;
46
import de.medizininformatikinitiative.torch.service.ExtractDataService;
57
import de.medizininformatikinitiative.torch.util.ResultFileManager;
68
import org.hl7.fhir.r4.model.OperationOutcome;
@@ -45,15 +47,17 @@ public class FhirController {
4547
private final ExtractDataParametersParser extractDataParametersParser;
4648
private final ExtractDataService extractDataService;
4749
private final String baseUrl;
50+
private final CrtdlValidatorService validator;
4851

4952
@Autowired
5053
public FhirController(FhirContext fhirContext, ResultFileManager resultFileManager,
51-
ExtractDataParametersParser parser, ExtractDataService extractDataService, @Value("${torch.base.url}") String baseUrl) {
54+
ExtractDataParametersParser parser, ExtractDataService extractDataService, @Value("${torch.base.url}") String baseUrl, CrtdlValidatorService validator) {
5255
this.fhirContext = requireNonNull(fhirContext);
5356
this.resultFileManager = requireNonNull(resultFileManager);
5457
this.extractDataParametersParser = requireNonNull(parser);
5558
this.extractDataService = requireNonNull(extractDataService);
5659
this.baseUrl = baseUrl;
60+
this.validator = requireNonNull(validator);
5761
}
5862

5963
private Mono<ServerResponse> getGlobalStatus(ServerRequest serverRequest) {
@@ -90,10 +94,13 @@ public Mono<ServerResponse> handleExtractData(ServerRequest request) {
9094
.switchIfEmpty(Mono.error(new IllegalArgumentException("Empty request body")))
9195
.map(extractDataParametersParser::parseParameters)
9296
.flatMap(parameters -> {
93-
Mono<Void> jobMono = extractDataService
94-
.startJob(parameters.crtdl(), parameters.patientIds(), jobId);
95-
96-
// Launch it asynchronously
97+
Mono<Void> jobMono;
98+
try {
99+
jobMono = extractDataService
100+
.startJob(validator.validate(parameters.crtdl()), parameters.patientIds(), jobId);
101+
} catch (ValidationException e) {
102+
return Mono.error(new IllegalArgumentException(e.getMessage()));
103+
}
97104
jobMono.subscribe();
98105
return ServerResponse.accepted()
99106
.header("Content-Location",

‎src/main/java/de/medizininformatikinitiative/torch/service/ExtractDataService.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package de.medizininformatikinitiative.torch.service;
22

33
import de.medizininformatikinitiative.torch.exceptions.ValidationException;
4-
import de.medizininformatikinitiative.torch.model.crtdl.Crtdl;
4+
import de.medizininformatikinitiative.torch.model.crtdl.annotated.AnnotatedCrtdl;
55
import de.medizininformatikinitiative.torch.util.ResultFileManager;
66
import org.hl7.fhir.r4.model.OperationOutcome;
77
import org.springframework.http.HttpStatus;
@@ -18,24 +18,19 @@
1818
public class ExtractDataService {
1919

2020
private final ResultFileManager resultFileManager;
21-
private final CrtdlValidatorService validatorService;
2221
private final CrtdlProcessingService processingService;
2322

2423
public ExtractDataService(ResultFileManager resultFileManager,
25-
CrtdlValidatorService validatorService,
2624
CrtdlProcessingService processingService) {
2725
this.resultFileManager = requireNonNull(resultFileManager);
28-
this.validatorService = requireNonNull(validatorService);
2926
this.processingService = requireNonNull(processingService);
3027
}
3128

32-
public Mono<Void> startJob(Crtdl crtdl, List<String> patientIds, String jobId) {
29+
public Mono<Void> startJob(AnnotatedCrtdl crtdl, List<String> patientIds, String jobId) {
3330
resultFileManager.setStatus(jobId, HttpStatus.ACCEPTED);
3431

3532
return resultFileManager.initJobDir(jobId)
36-
.then(Mono.fromCallable(() -> validatorService.validate(crtdl))
37-
.subscribeOn(Schedulers.boundedElastic()))
38-
.flatMap(validated -> processingService.process(validated, jobId, patientIds))
33+
.then(processingService.process(crtdl, jobId, patientIds))
3934
.doOnSuccess(v -> resultFileManager.setStatus(jobId, HttpStatus.OK))
4035
.doOnError(e -> handleJobError(jobId, e))
4136
.onErrorResume(e -> Mono.empty());

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ void testCoreMustHave() throws IOException, ValidationException {
181181
fis.close();
182182
}
183183

184-
void testExecutor(String filePath, String url, HttpHeaders headers, int expectedFinalCode) {
184+
void testExecutor(String filePath, String url, HttpHeaders headers) {
185185
TestRestTemplate restTemplate = new TestRestTemplate();
186186
try {
187187
String fileContent = Files.readString(Paths.get(filePath), StandardCharsets.UTF_8);
@@ -194,7 +194,7 @@ void testExecutor(String filePath, String url, HttpHeaders headers, int expected
194194
assertThat(durationSecondsSince(start)).isLessThan(1);
195195
List<String> locations = response.getHeaders().get("Content-Location");
196196
assertThat(locations).hasSize(1);
197-
pollStatusEndpoint(restTemplate, headers, locations.getFirst(), expectedFinalCode);
197+
pollStatusEndpoint(restTemplate, headers, locations.getFirst(), 200);
198198
clearDirectory(locations.getFirst().substring(locations.getFirst().lastIndexOf('/')));
199199
} catch (HttpStatusCodeException e) {
200200
logger.error("HTTP Status code error: {}", e.getStatusCode(), e);
@@ -282,7 +282,7 @@ class Endpoint {
282282
void validObservation(String parametersFile) {
283283
HttpHeaders headers = new HttpHeaders();
284284
headers.add("content-type", "application/fhir+json");
285-
testExecutor(parametersFile, "http://localhost:" + port + "/fhir/$extract-data", headers, 200);
285+
testExecutor(parametersFile, "http://localhost:" + port + "/fhir/$extract-data", headers);
286286
}
287287

288288
@Test
@@ -299,12 +299,29 @@ void emptyRequestBodyReturnsBadRequest() {
299299
assertThat(response.getBody()).contains("Empty request body");
300300
}
301301

302-
@ParameterizedTest
303-
@ValueSource(strings = {"src/test/resources/CRTDL_Parameters/Parameters_invalid_CRTDL.json"})
304-
void invalidCRTDLReturnsValidationException(String parametersFile) {
302+
@Test
303+
void invalidCRTDLReturnsBadRequest() {
304+
TestRestTemplate restTemplate = new TestRestTemplate();
305305
HttpHeaders headers = new HttpHeaders();
306306
headers.add("content-type", "application/fhir+json");
307-
testExecutor(parametersFile, "http://localhost:" + port + "/fhir/$extract-data", headers, 400);
307+
308+
try {
309+
String fileContent = Files.readString(Paths.get("src/test/resources/CRTDL_Parameters/Parameters_invalid_CRTDL.json"), StandardCharsets.UTF_8);
310+
HttpEntity<String> entity = new HttpEntity<>(fileContent, headers);
311+
try {
312+
long start = System.nanoTime();
313+
ResponseEntity<String> response = restTemplate.exchange("http://localhost:" + port + "/fhir/$extract-data", HttpMethod.POST, entity, String.class);
314+
assertThat(response.getStatusCode().value()).isEqualTo(400);
315+
assertThat(durationSecondsSince(start)).isLessThan(1);
316+
} catch (HttpStatusCodeException e) {
317+
logger.error("HTTP Status code error: {}", e.getStatusCode(), e);
318+
Assertions.fail("HTTP request failed with status code: " + e.getStatusCode());
319+
}
320+
321+
} catch (IOException e) {
322+
logger.error("CRTDL file not found", e);
323+
}
308324
}
325+
309326
}
310327
}

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

Lines changed: 23 additions & 1 deletion
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.exceptions.ValidationException;
45
import de.medizininformatikinitiative.torch.model.crtdl.ExtractDataParameters;
6+
import de.medizininformatikinitiative.torch.service.CrtdlValidatorService;
57
import de.medizininformatikinitiative.torch.service.ExtractDataService;
68
import de.medizininformatikinitiative.torch.util.CrtdlFactory;
79
import de.medizininformatikinitiative.torch.util.ResultFileManager;
@@ -43,12 +45,15 @@ class FhirControllerTest {
4345
@Mock
4446
ExtractDataService extractDataService;
4547

48+
@Mock
49+
CrtdlValidatorService validator;
50+
4651
WebTestClient client;
4752

4853
@BeforeEach
4954
void setup() {
5055
FhirContext fhirContext = FhirContext.forR4();
51-
FhirController fhirController = new FhirController(fhirContext, resultFileManager, extractDataParametersParser, extractDataService, BASE_URL);
56+
FhirController fhirController = new FhirController(fhirContext, resultFileManager, extractDataParametersParser, extractDataService, BASE_URL, validator);
5257
client = WebTestClient.bindToRouterFunction(fhirController.queryRouter()).build();
5358
}
5459

@@ -180,4 +185,21 @@ void blankRequestBodyTriggersBadRequest() {
180185
.jsonPath("$.resourceType").isEqualTo("OperationOutcome");
181186
}
182187
}
188+
189+
@Nested
190+
class Validator {
191+
@Test
192+
void invalidCrtdlTriggersBadRequest() throws ValidationException {
193+
ExtractDataParameters params = new ExtractDataParameters(CrtdlFactory.empty(), Collections.emptyList());
194+
when(extractDataParametersParser.parseParameters(any())).thenReturn(params);
195+
when(validator.validate(any())).thenThrow(new ValidationException("Invalid CRTDL"));
196+
197+
var response = client.post().uri("/fhir/$extract-data").contentType(MediaType.APPLICATION_JSON).bodyValue("{}").exchange();
198+
199+
response.expectStatus().isBadRequest().expectHeader().contentType("application/fhir+json")
200+
.expectBody()
201+
.jsonPath("$.resourceType").isEqualTo("OperationOutcome")
202+
.jsonPath("$.issue[0].diagnostics").isEqualTo("IllegalArgumentException: Invalid CRTDL");
203+
}
204+
}
183205
}

‎src/test/java/de/medizininformatikinitiative/torch/service/ExtractDataServiceTest.java‎

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

33
import de.medizininformatikinitiative.torch.exceptions.ValidationException;
4-
import de.medizininformatikinitiative.torch.model.crtdl.Crtdl;
54
import de.medizininformatikinitiative.torch.model.crtdl.annotated.AnnotatedCrtdl;
65
import de.medizininformatikinitiative.torch.util.ResultFileManager;
76
import org.hl7.fhir.r4.model.OperationOutcome;
@@ -24,84 +23,65 @@
2423
class ExtractDataServiceTest {
2524

2625
private ResultFileManager resultFileManager;
27-
private CrtdlValidatorService validatorService;
2826
private CrtdlProcessingService processingService;
2927
private ExtractDataService service;
30-
private Crtdl crtdl; // mocked
3128
private AnnotatedCrtdl annotatedCrtdl;
3229

3330
@BeforeEach
3431
void setUp() {
3532
resultFileManager = mock(ResultFileManager.class);
36-
validatorService = mock(CrtdlValidatorService.class);
3733
processingService = mock(CrtdlProcessingService.class);
38-
crtdl = mock(Crtdl.class);
3934
annotatedCrtdl = mock(AnnotatedCrtdl.class);
40-
service = new ExtractDataService(resultFileManager, validatorService, processingService);
35+
service = new ExtractDataService(resultFileManager, processingService);
4136

4237
when(resultFileManager.initJobDir(anyString())).thenReturn(Mono.empty());
4338
when(resultFileManager.saveErrorToJson(anyString(), any(OperationOutcome.class), any(HttpStatus.class)))
4439
.thenReturn(Mono.empty());
4540
}
4641

4742
@Test
48-
void startJob_success() throws ValidationException {
49-
when(validatorService.validate(crtdl)).thenReturn(annotatedCrtdl);
43+
void startJob_success() {
5044
when(processingService.process(any(), anyString(), anyList())).thenReturn(Mono.empty());
5145

52-
StepVerifier.create(service.startJob(crtdl, List.of("p1"), "job-ok"))
46+
StepVerifier.create(service.startJob(annotatedCrtdl, List.of("p1"), "job-ok"))
5347
.verifyComplete();
5448

5549
verify(resultFileManager).setStatus("job-ok", HttpStatus.ACCEPTED);
5650
verify(resultFileManager).initJobDir("job-ok");
57-
verify(validatorService).validate(crtdl);
5851
verify(processingService).process(eq(annotatedCrtdl), eq("job-ok"), eq(List.of("p1")));
5952
verify(resultFileManager).setStatus("job-ok", HttpStatus.OK);
6053
}
6154

6255
@Test
63-
void startJob_illegalArgumentError() throws ValidationException {
64-
when(validatorService.validate(crtdl)).thenReturn(annotatedCrtdl);
56+
void startJob_illegalArgumentError() {
6557
when(processingService.process(any(), anyString(), anyList()))
6658
.thenReturn(Mono.error(new IllegalArgumentException("bad arg")));
6759

68-
StepVerifier.create(service.startJob(crtdl, List.of("p1"), "job-bad-arg"))
60+
StepVerifier.create(service.startJob(annotatedCrtdl, List.of("p1"), "job-bad-arg"))
6961
.verifyComplete();
7062

7163
verify(resultFileManager).saveErrorToJson(eq("job-bad-arg"), any(OperationOutcome.class), eq(HttpStatus.BAD_REQUEST));
7264
}
7365

7466
@Test
75-
void startJob_validationExceptionError() throws ValidationException {
76-
when(validatorService.validate(crtdl)).thenReturn(annotatedCrtdl);
67+
void startJob_validationExceptionError() {
7768
when(processingService.process(any(), anyString(), anyList()))
7869
.thenReturn(Mono.error(new ValidationException("validation failed")));
7970

80-
StepVerifier.create(service.startJob(crtdl, List.of("p1"), "job-validation-fail"))
71+
StepVerifier.create(service.startJob(annotatedCrtdl, List.of("p1"), "job-validation-fail"))
8172
.verifyComplete();
8273

8374
verify(resultFileManager).saveErrorToJson(eq("job-validation-fail"), any(OperationOutcome.class), eq(HttpStatus.BAD_REQUEST));
8475
}
8576

8677
@Test
87-
void startJob_runtimeError() throws ValidationException {
88-
when(validatorService.validate(crtdl)).thenReturn(annotatedCrtdl);
78+
void startJob_runtimeError() {
8979
when(processingService.process(any(), anyString(), anyList()))
9080
.thenReturn(Mono.error(new RuntimeException("unexpected")));
9181

92-
StepVerifier.create(service.startJob(crtdl, List.of("p1"), "job-runtime-fail"))
82+
StepVerifier.create(service.startJob(annotatedCrtdl, List.of("p1"), "job-runtime-fail"))
9383
.verifyComplete();
9484

9585
verify(resultFileManager).saveErrorToJson(eq("job-runtime-fail"), any(OperationOutcome.class), eq(HttpStatus.INTERNAL_SERVER_ERROR));
9686
}
97-
98-
@Test
99-
void startJob_validatorThrows() throws ValidationException {
100-
when(validatorService.validate(crtdl)).thenThrow(new ValidationException("invalid"));
101-
102-
StepVerifier.create(service.startJob(crtdl, List.of("p1"), "job-validator-throw"))
103-
.verifyComplete();
104-
105-
verify(resultFileManager).saveErrorToJson(eq("job-validator-throw"), any(OperationOutcome.class), eq(HttpStatus.BAD_REQUEST));
106-
}
10787
}

0 commit comments

Comments
 (0)