Skip to content

Commit 65a7165

Browse files
Lucas0TLucas0T
committed
Move Validation before the Pipeline
1 parent bb63cbe commit 65a7165

File tree

8 files changed

+207
-66
lines changed

8 files changed

+207
-66
lines changed

‎src/main/java/de/medizininformatikinitiative/torch/management/OperationOutcomeCreator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public static OperationOutcome createOperationOutcome(String jobId, Throwable th
1111
OperationOutcome.OperationOutcomeIssueComponent issueComponent = new OperationOutcome.OperationOutcomeIssueComponent();
1212
issueComponent.setSeverity(OperationOutcome.IssueSeverity.FATAL);
1313
issueComponent.setCode(createIssueType(throwable));
14-
issueComponent.setDiagnostics(throwable.getClass().getSimpleName() + ": " + throwable.getMessage());
14+
issueComponent.setDiagnostics(throwable.getMessage());
1515
operationOutcome.addIssue(issueComponent);
1616
return operationOutcome;
1717
}

‎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/main/java/de/medizininformatikinitiative/torch/util/ResultFileManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public Map<String, HttpStatus> getJobStatusMap() {
7777
return jobStatusMap;
7878
}
7979

80-
public void loadExistingResults() {
80+
private void loadExistingResults() {
8181
try (Stream<Path> jobDirs = Files.list(resultsDirPath)) {
8282
jobDirs.filter(Files::isDirectory)
8383
.forEach(jobDir -> {

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,7 @@
1717
import de.numcodex.sq2cql.Translator;
1818
import de.numcodex.sq2cql.model.structured_query.StructuredQuery;
1919
import org.hl7.fhir.r4.model.OperationOutcome;
20-
import org.junit.jupiter.api.Assertions;
21-
import org.junit.jupiter.api.BeforeAll;
22-
import org.junit.jupiter.api.Nested;
23-
import org.junit.jupiter.api.Test;
24-
import org.junit.jupiter.api.TestInstance;
20+
import org.junit.jupiter.api.*;
2521
import org.junit.jupiter.params.ParameterizedTest;
2622
import org.junit.jupiter.params.provider.ValueSource;
2723
import org.slf4j.Logger;
@@ -31,11 +27,7 @@
3127
import org.springframework.beans.factory.annotation.Value;
3228
import org.springframework.boot.test.context.SpringBootTest;
3329
import org.springframework.boot.test.web.client.TestRestTemplate;
34-
import org.springframework.http.HttpEntity;
35-
import org.springframework.http.HttpHeaders;
36-
import org.springframework.http.HttpMethod;
37-
import org.springframework.http.MediaType;
38-
import org.springframework.http.ResponseEntity;
30+
import org.springframework.http.*;
3931
import org.springframework.test.annotation.DirtiesContext;
4032
import org.springframework.test.context.ActiveProfiles;
4133
import org.springframework.web.client.HttpStatusCodeException;
@@ -181,7 +173,7 @@ void testCoreMustHave() throws IOException, ValidationException {
181173
fis.close();
182174
}
183175

184-
void testExecutor(String filePath, String url, HttpHeaders headers, int expectedFinalCode) {
176+
void testExecutor(String filePath, String url, HttpHeaders headers) {
185177
TestRestTemplate restTemplate = new TestRestTemplate();
186178
try {
187179
String fileContent = Files.readString(Paths.get(filePath), StandardCharsets.UTF_8);
@@ -194,7 +186,7 @@ void testExecutor(String filePath, String url, HttpHeaders headers, int expected
194186
assertThat(durationSecondsSince(start)).isLessThan(1);
195187
List<String> locations = response.getHeaders().get("Content-Location");
196188
assertThat(locations).hasSize(1);
197-
pollStatusEndpoint(restTemplate, headers, locations.getFirst(), expectedFinalCode);
189+
pollStatusEndpoint(restTemplate, headers, locations.getFirst(), 200);
198190
clearDirectory(locations.getFirst().substring(locations.getFirst().lastIndexOf('/')));
199191
} catch (HttpStatusCodeException e) {
200192
logger.error("HTTP Status code error: {}", e.getStatusCode(), e);
@@ -282,7 +274,7 @@ class Endpoint {
282274
void validObservation(String parametersFile) {
283275
HttpHeaders headers = new HttpHeaders();
284276
headers.add("content-type", "application/fhir+json");
285-
testExecutor(parametersFile, "http://localhost:" + port + "/fhir/$extract-data", headers, 200);
277+
testExecutor(parametersFile, "http://localhost:" + port + "/fhir/$extract-data", headers);
286278
}
287279

288280
@Test
@@ -299,12 +291,18 @@ void emptyRequestBodyReturnsBadRequest() {
299291
assertThat(response.getBody()).contains("Empty request body");
300292
}
301293

302-
@ParameterizedTest
303-
@ValueSource(strings = {"src/test/resources/CRTDL_Parameters/Parameters_invalid_CRTDL.json"})
304-
void invalidCRTDLReturnsValidationException(String parametersFile) {
294+
@Test
295+
void invalidCRTDLReturnsBadRequest() throws IOException {
296+
TestRestTemplate restTemplate = new TestRestTemplate();
305297
HttpHeaders headers = new HttpHeaders();
306298
headers.add("content-type", "application/fhir+json");
307-
testExecutor(parametersFile, "http://localhost:" + port + "/fhir/$extract-data", headers, 400);
299+
String fileContent = Files.readString(Paths.get("src/test/resources/CRTDL_Parameters/Parameters_invalid_CRTDL.json"), StandardCharsets.UTF_8);
300+
HttpEntity<String> entity = new HttpEntity<>(fileContent, headers);
301+
long start = System.nanoTime();
302+
ResponseEntity<String> response = restTemplate.exchange("http://localhost:" + port + "/fhir/$extract-data", HttpMethod.POST, entity, String.class);
303+
assertThat(response.getStatusCode().value()).isEqualTo(400);
304+
assertThat(durationSecondsSince(start)).isLessThan(1);
308305
}
306+
309307
}
310308
}

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

Lines changed: 25 additions & 5 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.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;
@@ -24,15 +26,13 @@
2426
import java.util.Map;
2527

2628
import static org.assertj.core.api.Assertions.assertThat;
27-
import static org.mockito.ArgumentMatchers.any;
28-
import static org.mockito.ArgumentMatchers.anyString;
29-
import static org.mockito.ArgumentMatchers.eq;
29+
import static org.mockito.ArgumentMatchers.*;
3030
import static org.mockito.Mockito.when;
3131

3232
@ExtendWith(MockitoExtension.class)
3333
class FhirControllerTest {
3434

35-
private final static String BASE_URL = "http://base-url";
35+
private static final String BASE_URL = "http://base-url";
3636

3737
@Mock
3838
ResultFileManager resultFileManager;
@@ -43,12 +43,15 @@ class FhirControllerTest {
4343
@Mock
4444
ExtractDataService extractDataService;
4545

46+
@Mock
47+
CrtdlValidatorService validator;
48+
4649
WebTestClient client;
4750

4851
@BeforeEach
4952
void setup() {
5053
FhirContext fhirContext = FhirContext.forR4();
51-
FhirController fhirController = new FhirController(fhirContext, resultFileManager, extractDataParametersParser, extractDataService, BASE_URL);
54+
FhirController fhirController = new FhirController(fhirContext, resultFileManager, extractDataParametersParser, extractDataService, BASE_URL, validator);
5255
client = WebTestClient.bindToRouterFunction(fhirController.queryRouter()).build();
5356
}
5457

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

‎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)