diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/common/api/Criterion.java b/src/main/java/de/numcodex/feasibility_gui_backend/common/api/Criterion.java index 3c2c1054..18619e71 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/common/api/Criterion.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/common/api/Criterion.java @@ -6,6 +6,7 @@ import de.numcodex.feasibility_gui_backend.query.api.AttributeFilter; import de.numcodex.feasibility_gui_backend.query.api.TimeRestriction; import de.numcodex.feasibility_gui_backend.query.api.ValueFilter; +import de.numcodex.feasibility_gui_backend.query.api.status.ValidationIssue; import lombok.Builder; import java.util.List; @@ -17,7 +18,17 @@ public record Criterion ( @JsonProperty("termCodes") List termCodes, @JsonProperty("attributeFilters") List attributeFilters, @JsonProperty("valueFilter") ValueFilter valueFilter, - @JsonProperty("timeRestriction") TimeRestriction timeRestriction + @JsonProperty("timeRestriction") TimeRestriction timeRestriction, + @JsonProperty("issues") List validationIssues ) { - + public static Criterion createImmutableCriterion(MutableCriterion mutableCriterion) { + return Criterion.builder() + .termCodes(mutableCriterion.termCodes) + .context(mutableCriterion.context) + .attributeFilters(mutableCriterion.attributeFilters) + .valueFilter(mutableCriterion.valueFilter) + .timeRestriction(mutableCriterion.timeRestriction) + .validationIssues(mutableCriterion.validationIssues) + .build(); + } } diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/common/api/MutableCriterion.java b/src/main/java/de/numcodex/feasibility_gui_backend/common/api/MutableCriterion.java new file mode 100644 index 00000000..9d30e4d0 --- /dev/null +++ b/src/main/java/de/numcodex/feasibility_gui_backend/common/api/MutableCriterion.java @@ -0,0 +1,36 @@ +package de.numcodex.feasibility_gui_backend.common.api; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; +import com.fasterxml.jackson.annotation.JsonProperty; +import de.numcodex.feasibility_gui_backend.query.api.AttributeFilter; +import de.numcodex.feasibility_gui_backend.query.api.TimeRestriction; +import de.numcodex.feasibility_gui_backend.query.api.ValueFilter; +import de.numcodex.feasibility_gui_backend.query.api.status.ValidationIssue; +import lombok.Builder; +import lombok.Data; + +import java.util.List; + +@JsonInclude(Include.NON_NULL) +@Builder +@Data +public class MutableCriterion { + @JsonProperty("context") TermCode context; + @JsonProperty("termCodes") List termCodes; + @JsonProperty("attributeFilters") List attributeFilters; + @JsonProperty("valueFilter") ValueFilter valueFilter; + @JsonProperty("timeRestriction") TimeRestriction timeRestriction; + @JsonProperty("issues") List validationIssues; + + public static MutableCriterion createMutableCriterion (Criterion criterion) { + return MutableCriterion.builder() + .termCodes(criterion.termCodes()) + .context(criterion.context()) + .attributeFilters(criterion.attributeFilters()) + .valueFilter(criterion.valueFilter()) + .timeRestriction(criterion.timeRestriction()) + .validationIssues(criterion.validationIssues()) + .build(); + } +} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java index 77af30d1..802573ac 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerService.java @@ -2,7 +2,6 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; import de.numcodex.feasibility_gui_backend.query.api.Query; import de.numcodex.feasibility_gui_backend.query.api.QueryTemplate; import de.numcodex.feasibility_gui_backend.query.api.SavedQuery; @@ -15,7 +14,7 @@ import de.numcodex.feasibility_gui_backend.query.result.ResultService; import de.numcodex.feasibility_gui_backend.query.templates.QueryTemplateException; import de.numcodex.feasibility_gui_backend.query.templates.QueryTemplateHandler; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; import lombok.NonNull; import lombok.RequiredArgsConstructor; import org.springframework.dao.DataIntegrityViolationException; @@ -61,7 +60,7 @@ public enum ResultDetail { private final SavedQueryRepository savedQueryRepository; @NonNull - private final TermCodeValidation termCodeValidation; + private final StructuredQueryValidation structuredQueryValidation; @NonNull private ObjectMapper jsonUtil; @@ -242,35 +241,52 @@ public String getAuthorId(Long queryId) throws QueryNotFoundException { public QueryListEntry convertQueryToQueryListEntry(de.numcodex.feasibility_gui_backend.query.persistence.Query query, boolean skipValidation) { - List invalidCriteria; - if (skipValidation) { - invalidCriteria = List.of(); - } else { - try { - var sq = jsonUtil.readValue(query.getQueryContent().getQueryContent(), StructuredQuery.class); - invalidCriteria = termCodeValidation.getInvalidCriteria(sq); - } catch (JsonProcessingException e) { - invalidCriteria = List.of(); - } + boolean isValid = true; + if (!skipValidation) { + try { + var sq = jsonUtil.readValue(query.getQueryContent().getQueryContent(), StructuredQuery.class); + isValid = structuredQueryValidation.isValid(sq); + } catch (JsonProcessingException e) { + isValid = false; + } } if (query.getSavedQuery() != null) { - return - QueryListEntry.builder() - .id(query.getId()) - .label(query.getSavedQuery().getLabel()) - .comment(query.getSavedQuery().getComment()) - .totalNumberOfPatients(query.getSavedQuery().getResultSize()) - .createdAt(query.getCreatedAt()) - .invalidCriteria(invalidCriteria) - .build(); + if (skipValidation) { + return + QueryListEntry.builder() + .id(query.getId()) + .label(query.getSavedQuery().getLabel()) + .comment(query.getSavedQuery().getComment()) + .totalNumberOfPatients(query.getSavedQuery().getResultSize()) + .createdAt(query.getCreatedAt()) + .build(); + } else { + return + QueryListEntry.builder() + .id(query.getId()) + .label(query.getSavedQuery().getLabel()) + .comment(query.getSavedQuery().getComment()) + .totalNumberOfPatients(query.getSavedQuery().getResultSize()) + .createdAt(query.getCreatedAt()) + .isValid(isValid) + .build(); + } } else { - return - QueryListEntry.builder() - .id(query.getId()) - .createdAt(query.getCreatedAt()) - .invalidCriteria(invalidCriteria) - .build(); + if (skipValidation) { + return + QueryListEntry.builder() + .id(query.getId()) + .createdAt(query.getCreatedAt()) + .build(); + } else { + return + QueryListEntry.builder() + .id(query.getId()) + .createdAt(query.getCreatedAt()) + .isValid(isValid) + .build(); + } } } diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/MutableStructuredQuery.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/MutableStructuredQuery.java new file mode 100644 index 00000000..2017db6d --- /dev/null +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/MutableStructuredQuery.java @@ -0,0 +1,54 @@ +package de.numcodex.feasibility_gui_backend.query.api; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; +import com.fasterxml.jackson.annotation.JsonProperty; +import de.numcodex.feasibility_gui_backend.common.api.Criterion; +import de.numcodex.feasibility_gui_backend.common.api.MutableCriterion; +import de.numcodex.feasibility_gui_backend.query.api.validation.StructuredQueryValidation; +import lombok.Builder; +import lombok.Data; + +import java.net.URI; +import java.util.ArrayList; +import java.util.List; + +@JsonInclude(Include.NON_EMPTY) +@Builder +@Data +public class MutableStructuredQuery { + @JsonProperty URI version; + @JsonProperty("inclusionCriteria") List> inclusionCriteria; + @JsonProperty("exclusionCriteria") List> exclusionCriteria; + @JsonProperty("display") String display; + + public static MutableStructuredQuery createMutableStructuredQuery(StructuredQuery structuredQuery) { + List> mutableInclusionCriteria = new ArrayList<>(); + if (structuredQuery.inclusionCriteria() != null) { + for (List outerList : structuredQuery.inclusionCriteria()) { + List innerList = new ArrayList<>(); + for (Criterion criterion : outerList) { + innerList.add(MutableCriterion.createMutableCriterion(criterion)); + } + mutableInclusionCriteria.add(innerList); + } + } + + List> mutableExclusionCriteria = new ArrayList<>(); + if (structuredQuery.exclusionCriteria() != null) { + for (List outerList : structuredQuery.exclusionCriteria()) { + List innerList = new ArrayList<>(); + for (Criterion criterion : outerList) { + innerList.add(MutableCriterion.createMutableCriterion(criterion)); + } + mutableExclusionCriteria.add(innerList); + } + } + return MutableStructuredQuery.builder() + .version(structuredQuery.version()) + .inclusionCriteria(mutableInclusionCriteria) + .exclusionCriteria(mutableExclusionCriteria) + .display(structuredQuery.display()) + .build(); + } +} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/Query.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/Query.java index 04e59d4e..afd5e2a2 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/Query.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/Query.java @@ -3,19 +3,15 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonProperty; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; import lombok.Builder; -import java.util.List; - @JsonInclude(Include.NON_NULL) @Builder public record Query( @JsonProperty long id, @JsonProperty StructuredQuery content, @JsonProperty String label, - @JsonProperty String comment, - @JsonProperty List invalidCriteria + @JsonProperty String comment ) { } diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryListEntry.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryListEntry.java index 34a2c548..c69de384 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryListEntry.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryListEntry.java @@ -3,11 +3,9 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonProperty; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; import lombok.Builder; import java.sql.Timestamp; -import java.util.List; @JsonInclude(Include.NON_NULL) @Builder @@ -17,7 +15,7 @@ public record QueryListEntry( @JsonProperty String comment, @JsonProperty Timestamp createdAt, @JsonProperty Long totalNumberOfPatients, - @JsonProperty List invalidCriteria + @JsonProperty Boolean isValid ) { } diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryTemplate.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryTemplate.java index e1f45cd4..d3877d7d 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryTemplate.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/QueryTemplate.java @@ -3,12 +3,9 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonProperty; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; import de.numcodex.feasibility_gui_backend.query.api.validation.QueryTemplateValidation; import lombok.Builder; -import java.util.List; - @JsonInclude(Include.NON_NULL) @QueryTemplateValidation @Builder @@ -19,7 +16,6 @@ public record QueryTemplate( @JsonProperty String comment, @JsonProperty String lastModified, @JsonProperty String createdBy, - @JsonProperty List invalidCriteria, @JsonProperty Boolean isValid ) { diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/StructuredQuery.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/StructuredQuery.java index bb540911..2595c402 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/StructuredQuery.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/StructuredQuery.java @@ -4,11 +4,12 @@ import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonProperty; import de.numcodex.feasibility_gui_backend.common.api.Criterion; -import de.numcodex.feasibility_gui_backend.common.api.TermCode; +import de.numcodex.feasibility_gui_backend.common.api.MutableCriterion; import de.numcodex.feasibility_gui_backend.query.api.validation.StructuredQueryValidation; import lombok.Builder; import java.net.URI; +import java.util.ArrayList; import java.util.List; @JsonInclude(Include.NON_EMPTY) @@ -20,5 +21,30 @@ public record StructuredQuery( @JsonProperty("exclusionCriteria") List> exclusionCriteria, @JsonProperty("display") String display ) { + public static StructuredQuery createImmutableStructuredQuery(MutableStructuredQuery mutableStructuredQuery) { + List> inclusionCriteria = new ArrayList<>(); + for (List outerList : mutableStructuredQuery.getInclusionCriteria()) { + List innerList = new ArrayList<>(); + for (MutableCriterion criterion : outerList) { + innerList.add(Criterion.createImmutableCriterion(criterion)); + } + inclusionCriteria.add(innerList); + } + List> exclusionCriteria = new ArrayList<>(); + for (List outerList : mutableStructuredQuery.getExclusionCriteria()) { + List innerList = new ArrayList<>(); + for (MutableCriterion criterion : outerList) { + innerList.add(Criterion.createImmutableCriterion(criterion)); + } + exclusionCriteria.add(innerList); + } + + return StructuredQuery.builder() + .version(mutableStructuredQuery.getVersion()) + .display(mutableStructuredQuery.getDisplay()) + .inclusionCriteria(inclusionCriteria) + .exclusionCriteria(exclusionCriteria) + .build(); + } } diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/ValidatedStructuredQuery.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/ValidatedStructuredQuery.java deleted file mode 100644 index 799185e7..00000000 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/ValidatedStructuredQuery.java +++ /dev/null @@ -1,15 +0,0 @@ -package de.numcodex.feasibility_gui_backend.query.api; - -import com.fasterxml.jackson.annotation.JsonProperty; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; -import lombok.Builder; - -import java.util.List; - -@Builder -public record ValidatedStructuredQuery( - @JsonProperty StructuredQuery query, - @JsonProperty List invalidCriteria -) { - -} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssue.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssue.java new file mode 100644 index 00000000..f42212a1 --- /dev/null +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssue.java @@ -0,0 +1,51 @@ +package de.numcodex.feasibility_gui_backend.query.api.status; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import jakarta.annotation.Nullable; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@JsonSerialize(using = ValidationIssueSerializer.class) +public enum ValidationIssue { + TERMCODE_CONTEXT_COMBINATION_INVALID(20001, "The combination of context and termcode(s) is not found."); + + private static final ValidationIssue[] VALUES; + + static { + VALUES = values(); + } + + private final int code; + private final String detail; + + ValidationIssue(int code, String detail) { + this.code = code; + this.detail = detail; + } + + public int code() { + return this.code; + } + + public String detail() { + return this.detail; + } + + public static ValidationIssue valueOf(int validationIssueCode) { + ValidationIssue validationIssue = resolve(validationIssueCode); + if (validationIssue == null) { + throw new IllegalArgumentException("No matching Validation issue for code " + validationIssueCode); + } + return validationIssue; + } + + @Nullable + public static ValidationIssue resolve(int validationIssueCode) { + for (ValidationIssue validationIssue : VALUES) { + if (validationIssue.code == validationIssueCode) { + return validationIssue; + } + } + return null; + } +} \ No newline at end of file diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssueSerializer.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssueSerializer.java new file mode 100644 index 00000000..9ce9319a --- /dev/null +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssueSerializer.java @@ -0,0 +1,29 @@ +package de.numcodex.feasibility_gui_backend.query.api.status; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; + +import java.io.IOException; + +/** + * Custom Serializer for {@link ValidationIssue} to add prefix to code and replace boolean with yes/no. + */ +public class ValidationIssueSerializer extends StdSerializer { + + public ValidationIssueSerializer() { + this(null); + } + + public ValidationIssueSerializer(Class t) { + super(t); + } + + @Override + public void serialize(ValidationIssue validationIssue, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException { + jsonGenerator.writeStartObject(); + jsonGenerator.writeStringField("code", "VAL-" + validationIssue.code()); + jsonGenerator.writeStringField("detail", validationIssue.detail()); + jsonGenerator.writeEndObject(); + } +} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestController.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestController.java index ef1c75e9..3b54b39c 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestController.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestController.java @@ -1,8 +1,6 @@ package de.numcodex.feasibility_gui_backend.query.v3; import com.fasterxml.jackson.core.JsonProcessingException; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; -import de.numcodex.feasibility_gui_backend.common.api.TermCode; import de.numcodex.feasibility_gui_backend.config.WebSecurityConfig; import de.numcodex.feasibility_gui_backend.query.QueryHandlerService; import de.numcodex.feasibility_gui_backend.query.QueryHandlerService.ResultDetail; @@ -16,7 +14,7 @@ import de.numcodex.feasibility_gui_backend.query.ratelimiting.AuthenticationHelper; import de.numcodex.feasibility_gui_backend.query.ratelimiting.InvalidAuthenticationException; import de.numcodex.feasibility_gui_backend.query.ratelimiting.RateLimitingService; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import jakarta.ws.rs.core.Context; @@ -50,7 +48,7 @@ public class QueryHandlerRestController { public static final String HEADER_X_DETAILED_OBFUSCATED_RESULT_WAS_EMPTY = "X-Detailed-Obfuscated-Result-Was-Empty"; private final QueryHandlerService queryHandlerService; - private final TermCodeValidation termCodeValidation; + private final StructuredQueryValidation structuredQueryValidation; private final RateLimitingService rateLimitingService; private final UserBlacklistRepository userBlacklistRepository; private final AuthenticationHelper authenticationHelper; @@ -87,13 +85,13 @@ public class QueryHandlerRestController { public QueryHandlerRestController(QueryHandlerService queryHandlerService, RateLimitingService rateLimitingService, - TermCodeValidation termCodeValidation, + StructuredQueryValidation structuredQueryValidation, UserBlacklistRepository userBlacklistRepository, AuthenticationHelper authenticationHelper, @Value("${app.apiBaseUrl}") String apiBaseUrl) { this.queryHandlerService = queryHandlerService; this.rateLimitingService = rateLimitingService; - this.termCodeValidation = termCodeValidation; + this.structuredQueryValidation = structuredQueryValidation; this.userBlacklistRepository = userBlacklistRepository; this.authenticationHelper = authenticationHelper; this.apiBaseUrl = apiBaseUrl; @@ -298,20 +296,19 @@ public List getQueryListForUser( @GetMapping("/{id}") public ResponseEntity getQuery(@PathVariable("id") Long queryId, + @RequestParam(value = "skipValidation", required = false, defaultValue = "false") boolean skipValidation, Authentication authentication) throws JsonProcessingException { if (!hasAccess(queryId, authentication)) { return new ResponseEntity<>(HttpStatus.FORBIDDEN); } var query = queryHandlerService.getQuery(queryId); - List invalidCriteria = termCodeValidation.getInvalidCriteria(query.content()); - var queryWithInvalidCriteria = Query.builder() + var annotatedQuery = Query.builder() .id(query.id()) - .content(query.content()) + .content(structuredQueryValidation.annotateStructuredQuery(query.content(), skipValidation)) .label(query.label()) .comment(query.comment()) - .invalidCriteria(invalidCriteria) .build(); - return new ResponseEntity<>(queryWithInvalidCriteria, HttpStatus.OK); + return new ResponseEntity<>(annotatedQuery, HttpStatus.OK); } @GetMapping("/{id}" + WebSecurityConfig.PATH_DETAILED_RESULT) @@ -400,14 +397,9 @@ public ResponseEntity getQueryContent( } @PostMapping("/validate") - public ResponseEntity validateStructuredQuery( + public ResponseEntity validateStructuredQuery( @Valid @RequestBody StructuredQuery query) { - var invalidCriteria = termCodeValidation.getInvalidCriteria(query); - var validatedQuery = ValidatedStructuredQuery.builder() - .query(query) - .invalidCriteria(invalidCriteria) - .build(); - return new ResponseEntity<>(validatedQuery, HttpStatus.OK); + return new ResponseEntity<>(structuredQueryValidation.annotateStructuredQuery(query, false), HttpStatus.OK); } private boolean hasAccess(Long queryId, Authentication authentication) { diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestController.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestController.java index 567a3d22..e42b9cce 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestController.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestController.java @@ -1,15 +1,14 @@ package de.numcodex.feasibility_gui_backend.query.v3; import com.fasterxml.jackson.core.JsonProcessingException; -import de.numcodex.feasibility_gui_backend.common.api.Criterion; -import de.numcodex.feasibility_gui_backend.common.api.TermCode; import de.numcodex.feasibility_gui_backend.query.QueryHandlerService; import de.numcodex.feasibility_gui_backend.query.api.QueryTemplate; import de.numcodex.feasibility_gui_backend.query.templates.QueryTemplateException; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; + import java.security.Principal; import java.util.ArrayList; -import java.util.List; + import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import jakarta.ws.rs.core.Context; @@ -33,13 +32,13 @@ public class QueryTemplateHandlerRestController { private final QueryHandlerService queryHandlerService; - private final TermCodeValidation termCodeValidation; + private final StructuredQueryValidation structuredQueryValidation; private final String apiBaseUrl; public QueryTemplateHandlerRestController(QueryHandlerService queryHandlerService, - TermCodeValidation termCodeValidation, @Value("${app.apiBaseUrl}") String apiBaseUrl) { + StructuredQueryValidation structuredQueryValidation, @Value("${app.apiBaseUrl}") String apiBaseUrl) { this.queryHandlerService = queryHandlerService; - this.termCodeValidation = termCodeValidation; + this.structuredQueryValidation = structuredQueryValidation; this.apiBaseUrl = apiBaseUrl; } @@ -72,21 +71,19 @@ public ResponseEntity storeQueryTemplate(@Valid @RequestBody QueryTempla @GetMapping(path = "/{queryId}") public ResponseEntity getQueryTemplate(@PathVariable(value = "queryId") Long queryId, + @RequestParam(value = "skipValidation", required = false, defaultValue = "false") boolean skipValidation, Principal principal) { try { var query = queryHandlerService.getQueryTemplate(queryId, principal.getName()); var queryTemplate = queryHandlerService.convertTemplatePersistenceToApi(query); - List invalidCriteria = termCodeValidation.getInvalidCriteria( - queryTemplate.content()); var queryTemplateWithInvalidCritiera = QueryTemplate.builder() .id(queryTemplate.id()) - .content(queryTemplate.content()) + .content(structuredQueryValidation.annotateStructuredQuery(queryTemplate.content(), skipValidation)) .label(queryTemplate.label()) .comment(queryTemplate.comment()) .lastModified(queryTemplate.lastModified()) .createdBy(queryTemplate.createdBy()) - .invalidCriteria(invalidCriteria) .isValid(queryTemplate.isValid()) .build(); return new ResponseEntity<>(queryTemplateWithInvalidCritiera, HttpStatus.OK); @@ -107,23 +104,28 @@ public ResponseEntity getQueryTemplates( queries.forEach(q -> { try { QueryTemplate convertedQuery = queryHandlerService.convertTemplatePersistenceToApi(q); - List invalidCriteria; if (skipValidation) { - invalidCriteria = List.of(); + ret.add( + QueryTemplate.builder() + .id(convertedQuery.id()) + .label(convertedQuery.label()) + .comment(convertedQuery.comment()) + .lastModified(convertedQuery.lastModified()) + .createdBy(convertedQuery.createdBy()) + .build() + ); } else { - invalidCriteria = termCodeValidation.getInvalidCriteria( - convertedQuery.content()); + ret.add( + QueryTemplate.builder() + .id(convertedQuery.id()) + .label(convertedQuery.label()) + .comment(convertedQuery.comment()) + .lastModified(convertedQuery.lastModified()) + .createdBy(convertedQuery.createdBy()) + .isValid(structuredQueryValidation.isValid(convertedQuery.content())) + .build() + ); } - var convertedQueryWithoutContent = QueryTemplate.builder() - .id(convertedQuery.id()) - .label(convertedQuery.label()) - .comment(convertedQuery.comment()) - .lastModified(convertedQuery.lastModified()) - .createdBy(convertedQuery.createdBy()) - .invalidCriteria(invalidCriteria) - .isValid(convertedQuery.isValid()) - .build(); - ret.add(convertedQueryWithoutContent); } catch (JsonProcessingException e) { log.error("Error converting query"); } diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/terminology/validation/StructuredQueryValidation.java b/src/main/java/de/numcodex/feasibility_gui_backend/terminology/validation/StructuredQueryValidation.java new file mode 100644 index 00000000..214f41c1 --- /dev/null +++ b/src/main/java/de/numcodex/feasibility_gui_backend/terminology/validation/StructuredQueryValidation.java @@ -0,0 +1,119 @@ +package de.numcodex.feasibility_gui_backend.terminology.validation; + +import de.numcodex.feasibility_gui_backend.common.api.Criterion; +import de.numcodex.feasibility_gui_backend.common.api.MutableCriterion; +import de.numcodex.feasibility_gui_backend.common.api.TermCode; +import de.numcodex.feasibility_gui_backend.query.api.MutableStructuredQuery; +import de.numcodex.feasibility_gui_backend.query.api.StructuredQuery; +import de.numcodex.feasibility_gui_backend.query.api.status.ValidationIssue; +import de.numcodex.feasibility_gui_backend.terminology.TerminologyService; +import java.util.List; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +@Service +@Slf4j +public class StructuredQueryValidation { + + private final TerminologyService terminologyService; + + @Autowired + public StructuredQueryValidation(TerminologyService terminologyService) { + this.terminologyService = terminologyService; + } + + /** + * Check a structured query for invalid/outdated termcodes in criteria and annotate it with the issues. + * + * For now, just check if the term codes still exist in the current ui profiles. Further + * iterations may contain checking for availability of values and units of the term codes as well. + * + * @param structuredQuery the structured query to check + * @param skipValidation if set to true, the issues list will always be empty + * @return the structuredQuery with issue annotation + */ + public StructuredQuery annotateStructuredQuery(StructuredQuery structuredQuery, boolean skipValidation) { + var mutableStructuredQuery = MutableStructuredQuery.createMutableStructuredQuery(structuredQuery); + + for (List inclusionCriteria : mutableStructuredQuery.getInclusionCriteria()) { + annotateCriteria(inclusionCriteria, skipValidation); + } + + for (List exclusionCriteria : mutableStructuredQuery.getExclusionCriteria()) { + annotateCriteria(exclusionCriteria, skipValidation); + } + + return StructuredQuery.createImmutableStructuredQuery(mutableStructuredQuery); + } + + /** + * Check a structured query for invalid/outdated termcodes in criteria and annotate it with the issues. + * + * For now, just check if the term codes still exist in the current ui profiles. Further + * iterations may contain checking for availability of values and units of the term codes as well. + * + * @param structuredQuery the structured query to check + * @return the structuredQuery with issue annotation + */ + public boolean isValid(StructuredQuery structuredQuery) { + if (structuredQuery.inclusionCriteria() != null) { + for (List inclusionCriteria : structuredQuery.inclusionCriteria()) { + if (containsInvalidCriteria(inclusionCriteria)) { + return false; + } + } + } + + if (structuredQuery.exclusionCriteria() != null) { + for (List exclusionCriteria : structuredQuery.exclusionCriteria()) { + if (containsInvalidCriteria(exclusionCriteria)) { + return false; + } + } + } + + return true; + } + + private void annotateCriteria(List criteria, boolean skipValidation) { + for (MutableCriterion criterion : criteria) { + if (skipValidation) { + criterion.setValidationIssues(List.of()); + continue; + } + if (criterion.getContext() == null) { + criterion.setValidationIssues(List.of(ValidationIssue.TERMCODE_CONTEXT_COMBINATION_INVALID)); + continue; + } + for (TermCode termCode : criterion.getTermCodes()) { + if (terminologyService.isExistingTermCode(termCode.system(), termCode.code(), termCode.version())) { + log.trace("termcode ok: {} - {} - {}", termCode.system(), termCode.code(), termCode.version()); + criterion.setValidationIssues(List.of()); // empty list is expected + } else { + log.debug("termcode invalid: {} - {} - {}", termCode.system(), termCode.code(), + termCode.version()); + criterion.setValidationIssues(List.of(ValidationIssue.TERMCODE_CONTEXT_COMBINATION_INVALID)); + } + } + } + } + + private boolean containsInvalidCriteria(List inclusionCriteria) { + for (Criterion criterion : inclusionCriteria) { + if (criterion.context() == null) { + return true; + } + for (TermCode termCode : criterion.termCodes()) { + if (terminologyService.isExistingTermCode(termCode.system(), termCode.code(), termCode.version())) { + log.trace("termcode ok: {} - {} - {}", termCode.system(), termCode.code(), termCode.version()); + } else { + log.debug("termcode invalid: {} - {} - {}", termCode.system(), termCode.code(), + termCode.version()); + return true; + } + } + } + return false; + } +} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/terminology/validation/TermCodeValidation.java b/src/main/java/de/numcodex/feasibility_gui_backend/terminology/validation/TermCodeValidation.java deleted file mode 100644 index 2ee9f709..00000000 --- a/src/main/java/de/numcodex/feasibility_gui_backend/terminology/validation/TermCodeValidation.java +++ /dev/null @@ -1,76 +0,0 @@ -package de.numcodex.feasibility_gui_backend.terminology.validation; - -import de.numcodex.feasibility_gui_backend.common.api.Criterion; -import de.numcodex.feasibility_gui_backend.common.api.TermCode; -import de.numcodex.feasibility_gui_backend.query.api.StructuredQuery; -import de.numcodex.feasibility_gui_backend.terminology.TerminologyService; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.stream.Stream; -import lombok.extern.slf4j.Slf4j; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Service; - -@Service -@Slf4j -public class TermCodeValidation { - - private final TerminologyService terminologyService; - - @Autowired - public TermCodeValidation(TerminologyService terminologyService) { - this.terminologyService = terminologyService; - } - - private Criterion removeFilters(Criterion in) { - return Criterion.builder() - .termCodes(in.termCodes()) - .context(in.context()) - .build(); - } - - /** - * Check a structured query for invalid/outdated termcodes in criteria. - * - * For now, just check if the term codes still exist in the current ui profiles. Further - * iterations may contain checking for availability of values and units of the term codes as well. - * - * @param structuredQuery the structured query to check - * @return a list of criteria that are no longer valid (or have never been) - */ - public List getInvalidCriteria(StructuredQuery structuredQuery) { - var invalidCriteria = new ArrayList(); - - List> combinedCriteria; - - if (structuredQuery.exclusionCriteria() != null && !structuredQuery.exclusionCriteria().isEmpty()) { - combinedCriteria = Stream.of( - structuredQuery.inclusionCriteria(), - structuredQuery.exclusionCriteria()).flatMap( - Collection::stream).toList(); - } else { - combinedCriteria = structuredQuery.inclusionCriteria(); - } - - for (List criterionList : combinedCriteria) { - for (Criterion criterion : criterionList) { - if (criterion.context() == null) { - log.debug("Missing context"); - invalidCriteria.add(removeFilters(criterion)); - } - for (TermCode termCode : criterion.termCodes()) { - if (terminologyService.isExistingTermCode(termCode.system(), termCode.code(), termCode.version())) { - log.trace("termcode ok: {} - {} - {}", termCode.system(), termCode.code(), termCode.version()); - } else { - log.debug("termcode invalid: {} - {} - {}", termCode.system(), termCode.code(), - termCode.version()); - invalidCriteria.add(removeFilters(criterion)); - } - } - } - } - - return invalidCriteria; - } -} diff --git a/src/main/resources/static/v3/api-docs/swagger.yaml b/src/main/resources/static/v3/api-docs/swagger.yaml index 224b1cd9..da4a48c2 100644 --- a/src/main/resources/static/v3/api-docs/swagger.yaml +++ b/src/main/resources/static/v3/api-docs/swagger.yaml @@ -131,7 +131,7 @@ paths: schema: type: array items: - $ref: '#/components/schemas/ValidatedStructuredQuery' + $ref: '#/components/schemas/StructuredQuery' 400: description: Query does not adhere to json schema content: { } @@ -159,6 +159,13 @@ paths: type: string enum: - saved + - name: skipValidation + in: query + description: If true, do not validate the query and do not include a list of invalid terms + required: false + schema: + type: boolean + default: false responses: 200: description: successful operation @@ -195,6 +202,13 @@ paths: schema: type: integer format: int64 + - name: skipValidation + in: query + description: If true, do not validate the query and do not include a list of invalid terms + required: false + schema: + type: boolean + default: false responses: 200: description: OK @@ -840,7 +854,6 @@ components: required: - id - label - - invalidCriteria properties: id: type: integer @@ -854,10 +867,8 @@ components: format: 'date-time' totalNumberOfPatients: type: integer - invalidCriteria: - type: array - items: - $ref: "#/components/schemas/Criterion" + isValid: + type: boolean Query: type: object required: @@ -873,10 +884,6 @@ components: type: string results: $ref: "#/components/schemas/QueryResult" - invalidCriteria: - type: array - items: - $ref: "#/components/schemas/Criterion" QueryResultSummary: type: object properties: @@ -940,7 +947,6 @@ components: type: object required: - label - - invalidCriteria properties: id: type: integer @@ -959,10 +965,6 @@ components: createdBy: type: string description: Keycloak id of the user who created the query - invalidCriteria: - type: array - items: - $ref: "#/components/schemas/Criterion" isValid: type: boolean description: is the query valid? @@ -990,10 +992,6 @@ components: createdBy: type: string description: Keycloak id of the user who created the query - invalidCriteria: - type: array - items: - $ref: "#/components/schemas/Criterion" StructuredQuery: type: object required: @@ -1016,18 +1014,6 @@ components: type: array items: $ref: "#/components/schemas/CriterionList" - ValidatedStructuredQuery: - type: object - required: - - query - - invalidCriteria - properties: - query: - $ref: "#/components/schemas/StructuredQuery" - invalidCriteria: - type: array - items: - $ref: "#/components/schemas/Criterion" SavedQuery: type: object required: @@ -1161,6 +1147,10 @@ components: $ref: "#/components/schemas/ValueFilter" timeRestriction: $ref: "#/components/schemas/TimeRestriction" + issues: + type: array + items: + $ref: "#/components/schemas/ValidationIssue" CriterionList: type: array items: @@ -1217,7 +1207,18 @@ components: $ref: "#/components/schemas/TermCode" context: $ref: "#/components/schemas/TermCode" - + ValidationIssue: + type: object + required: + - code + - detail + properties: + code: + type: string + example: VAL-20001 + detail: + type: string + example: The combination of context and termcode(s) is not found. securitySchemes: feasibility_auth: type: oauth2 diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/common/api/MutableCriterionTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/common/api/MutableCriterionTest.java new file mode 100644 index 00000000..3e1453a2 --- /dev/null +++ b/src/test/java/de/numcodex/feasibility_gui_backend/common/api/MutableCriterionTest.java @@ -0,0 +1,74 @@ +package de.numcodex.feasibility_gui_backend.common.api; + +import de.numcodex.feasibility_gui_backend.query.api.TimeRestriction; +import de.numcodex.feasibility_gui_backend.query.api.ValueFilter; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static de.numcodex.feasibility_gui_backend.common.api.Comparator.GREATER_EQUAL; +import static de.numcodex.feasibility_gui_backend.query.api.ValueFilterType.QUANTITY_COMPARATOR; +import static org.junit.jupiter.api.Assertions.*; + +class MutableCriterionTest { + + @Test + void createMutableCriterion() { + var immutableCriterion = Criterion.builder() + .attributeFilters(List.of()) + .context(createTermCode()) + .validationIssues(List.of()) + .termCodes(List.of(createTermCode())) + .timeRestriction(createTimeRestriction()) + .valueFilter(createValueFilter()) + .build(); + var mutableCriterion = MutableCriterion.builder() + .attributeFilters(List.of()) + .context(createTermCode()) + .validationIssues(List.of()) + .termCodes(List.of(createTermCode())) + .timeRestriction(createTimeRestriction()) + .valueFilter(createValueFilter()) + .build(); + + var createdMutableCriterion = MutableCriterion.createMutableCriterion(immutableCriterion); + + assertEquals(mutableCriterion, createdMutableCriterion); + } + + @NotNull + private static TermCode createTermCode() { + return TermCode.builder() + .code("LL2191-6") + .system("http://loinc.org") + .display("Geschlecht") + .build(); + } + + @NotNull + private static TimeRestriction createTimeRestriction() { + return TimeRestriction.builder() + .afterDate("2021-01-01") + .beforeDate("2021-12-31") + .build(); + } + + @NotNull + private static ValueFilter createValueFilter() { + return ValueFilter.builder() + .type(QUANTITY_COMPARATOR) + .comparator(GREATER_EQUAL) + .quantityUnit(createUnit()) + .value(50.0) + .build(); + } + + @NotNull + private static Unit createUnit() { + return Unit.builder() + .code("kg") + .display("kilogram") + .build(); + } +} \ No newline at end of file diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java index 0ea53a1c..a7e3cec1 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceIT.java @@ -24,7 +24,7 @@ import java.net.URI; import java.util.List; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -102,7 +102,7 @@ public class QueryHandlerServiceIT { private QueryHashCalculator queryHashCalculator; @MockBean - private TermCodeValidation termCodeValidation; + private StructuredQueryValidation structuredQueryValidation; @Autowired @Qualifier("translation") diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceTest.java index ffe5c994..1a0a0c2d 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceTest.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceTest.java @@ -11,7 +11,7 @@ import de.numcodex.feasibility_gui_backend.query.persistence.*; import de.numcodex.feasibility_gui_backend.query.result.ResultService; import de.numcodex.feasibility_gui_backend.query.templates.QueryTemplateHandler; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -30,6 +30,7 @@ import java.net.URI; import java.sql.Timestamp; import java.util.List; +import java.util.Random; import static org.assertj.core.api.Assertions.assertThat; @@ -70,13 +71,13 @@ class QueryHandlerServiceTest { private SavedQueryRepository savedQueryRepository; @Mock - private TermCodeValidation termCodeValidation; + private StructuredQueryValidation structuredQueryValidation; private QueryHandlerService queryHandlerService; private QueryHandlerService createQueryHandlerService() { return new QueryHandlerService(queryDispatcher, queryTemplateHandler, queryRepository, queryContentRepository, - resultService, queryTemplateRepository, savedQueryRepository, termCodeValidation, jsonUtil); + resultService, queryTemplateRepository, savedQueryRepository, structuredQueryValidation, jsonUtil); } @BeforeEach @@ -113,8 +114,8 @@ void convertQueriesToQueryListEntries(String withSavedQuery, String skipValidati var queryList = List.of(createQuery(Boolean.parseBoolean(withSavedQuery))); if (!Boolean.parseBoolean(skipValidation)) { doReturn( - List.of(createInvalidCriterion()) - ).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + new Random().nextBoolean() + ).when(structuredQueryValidation).isValid(any(StructuredQuery.class)); } List queryListEntries = queryHandlerService.convertQueriesToQueryListEntries(queryList, Boolean.parseBoolean(skipValidation)); @@ -127,14 +128,14 @@ void convertQueriesToQueryListEntries(String withSavedQuery, String skipValidati assertThat(queryListEntries.get(0).totalNumberOfPatients()).isEqualTo(RESULT_SIZE); } if (Boolean.parseBoolean(skipValidation)) { - assertThat(queryListEntries.get(0).invalidCriteria().size()).isEqualTo(0); + assertThat(queryListEntries.get(0).isValid()).isNull(); } else { - assertThat(queryListEntries.get(0).invalidCriteria().size()).isEqualTo(1); + assertThat(queryListEntries.get(0).isValid()).isNotNull(); } } @Test - void convertQueriesToQueryListEntries_JsonProcessingExceptionCausesEmptyList() throws JsonProcessingException { + void convertQueriesToQueryListEntries_JsonProcessingExceptionCausesInvalidQuery() throws JsonProcessingException { var queryList = List.of(createQuery(false)); doThrow(JsonProcessingException.class).when(jsonUtil).readValue(any(String.class), any(Class.class)); @@ -143,7 +144,7 @@ void convertQueriesToQueryListEntries_JsonProcessingExceptionCausesEmptyList() t assertThat(queryListEntries.size()).isEqualTo(1); assertThat(queryListEntries.get(0).id()).isEqualTo(QUERY_ID); assertThat(queryListEntries.get(0).createdAt()).isEqualTo(LAST_MODIFIED); - assertThat(queryListEntries.get(0).invalidCriteria().size()).isEqualTo(0); + assertThat(queryListEntries.get(0).isValid()).isFalse(); } private Query createQuery(boolean withSavedQuery) throws JsonProcessingException { diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/api/status/FeasibilityIssueTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/api/status/FeasibilityIssueTest.java new file mode 100644 index 00000000..b3c8bf66 --- /dev/null +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/api/status/FeasibilityIssueTest.java @@ -0,0 +1,47 @@ +package de.numcodex.feasibility_gui_backend.query.api.status; + + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import static org.junit.jupiter.api.Assertions.*; + + +@Tag("query") +@Tag("validation") +class FeasibilityIssueTest { + + @ParameterizedTest + @EnumSource(FeasibilityIssue.class) + void testValueOf_succeeds(FeasibilityIssue feasibilityIssue) { + var issueCode = feasibilityIssue.code(); + + var issue = FeasibilityIssue.valueOf(issueCode); + + assertEquals(issue, feasibilityIssue); + } + + @Test + void testValueOf_throwsOnUnknown() { + assertThrows(IllegalArgumentException.class, () -> FeasibilityIssue.valueOf(-1)); + } + + @ParameterizedTest + @EnumSource(FeasibilityIssue.class) + void testResolve_succeeds(FeasibilityIssue feasibilityIssue) { + var issueCode = feasibilityIssue.code(); + + var issue = FeasibilityIssue.resolve(issueCode); + + assertEquals(issue, feasibilityIssue); + } + + @Test + void testResolve_nullOnUnknown() { + var issue = FeasibilityIssue.resolve(-1); + + assertNull(issue); + } +} \ No newline at end of file diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssueTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssueTest.java new file mode 100644 index 00000000..d896907c --- /dev/null +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/api/status/ValidationIssueTest.java @@ -0,0 +1,45 @@ +package de.numcodex.feasibility_gui_backend.query.api.status; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import static org.junit.jupiter.api.Assertions.*; + +@Tag("query") +@Tag("validation") +class ValidationIssueTest { + + @ParameterizedTest + @EnumSource(ValidationIssue.class) + void testValueOf_succeeds(ValidationIssue validationIssue) { + var issueCode = validationIssue.code(); + + var issue = ValidationIssue.valueOf(issueCode); + + assertEquals(issue, validationIssue); + } + + @Test + void testValueOf_throwsOnUnknown() { + assertThrows(IllegalArgumentException.class, () -> ValidationIssue.valueOf(-1)); + } + + @ParameterizedTest + @EnumSource(ValidationIssue.class) + void testResolve_succeeds(ValidationIssue validationIssue) { + var issueCode = validationIssue.code(); + + var issue = ValidationIssue.resolve(issueCode); + + assertEquals(issue, validationIssue); + } + + @Test + void testResolve_nullOnUnknown() { + var issue = ValidationIssue.resolve(-1); + + assertNull(issue); + } +} \ No newline at end of file diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/api/validation/QueryTemplateValidatorTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/api/validation/QueryTemplateValidatorTest.java index 08dc35a1..5ec9e1b3 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/api/validation/QueryTemplateValidatorTest.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/api/validation/QueryTemplateValidatorTest.java @@ -123,7 +123,6 @@ private QueryTemplate buildInvalidValidQueryWithoutLabel() { .comment(validQuery.comment()) .lastModified(validQuery.lastModified()) .createdBy(validQuery.createdBy()) - .invalidCriteria(validQuery.invalidCriteria()) .isValid(validQuery.isValid()) .build(); } @@ -136,7 +135,6 @@ private QueryTemplate buildInvalidValidQueryWithoutContent() { .comment(validQuery.comment()) .lastModified(validQuery.lastModified()) .createdBy(validQuery.createdBy()) - .invalidCriteria(validQuery.invalidCriteria()) .isValid(validQuery.isValid()) .build(); } @@ -155,7 +153,6 @@ private QueryTemplate buildInvalidQueryWithMalformedStructuredQuery() { .comment(validQuery.comment()) .lastModified(validQuery.lastModified()) .createdBy(validQuery.createdBy()) - .invalidCriteria(validQuery.invalidCriteria()) .isValid(validQuery.isValid()) .build(); } diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/ratelimiting/RateLimitingInterceptorIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/ratelimiting/RateLimitingInterceptorIT.java index f0936446..3c05b5d5 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/ratelimiting/RateLimitingInterceptorIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/ratelimiting/RateLimitingInterceptorIT.java @@ -19,7 +19,8 @@ import de.numcodex.feasibility_gui_backend.query.persistence.UserBlacklistRepository; import de.numcodex.feasibility_gui_backend.query.result.ResultLine; import de.numcodex.feasibility_gui_backend.query.v3.QueryHandlerRestController; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; + import java.net.URI; import java.util.List; import java.util.UUID; @@ -64,7 +65,7 @@ public class RateLimitingInterceptorIT { private QueryHandlerService queryHandlerService; @MockBean - private TermCodeValidation termCodeValidation; + private StructuredQueryValidation structuredQueryValidation; @MockBean AuthenticationHelper authenticationHelper; diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestControllerIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestControllerIT.java index 625b9d5e..3e9dcc6a 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestControllerIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryHandlerRestControllerIT.java @@ -8,6 +8,7 @@ import de.numcodex.feasibility_gui_backend.common.api.TermCode; import de.numcodex.feasibility_gui_backend.query.QueryHandlerService; import de.numcodex.feasibility_gui_backend.query.api.status.FeasibilityIssue; +import de.numcodex.feasibility_gui_backend.query.api.status.ValidationIssue; import de.numcodex.feasibility_gui_backend.query.api.validation.StructuredQueryValidatorSpringConfig; import de.numcodex.feasibility_gui_backend.query.dispatch.QueryDispatchException; import de.numcodex.feasibility_gui_backend.query.persistence.UserBlacklist; @@ -16,7 +17,8 @@ import de.numcodex.feasibility_gui_backend.query.ratelimiting.RateLimitingInterceptor; import de.numcodex.feasibility_gui_backend.query.ratelimiting.RateLimitingServiceSpringConfig; import de.numcodex.feasibility_gui_backend.query.result.ResultLine; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; + import java.sql.Timestamp; import java.util.Optional; @@ -79,7 +81,7 @@ public class QueryHandlerRestControllerIT { private QueryHandlerService queryHandlerService; @MockBean - private TermCodeValidation termCodeValidation; + private StructuredQueryValidation structuredQueryValidation; @MockBean private RateLimitingInterceptor rateLimitingInterceptor; @@ -128,9 +130,10 @@ public void testRunQueryEndpoint_FailsOnInvalidStructuredQueryWith400() throws E @WithMockUser(roles = "FEASIBILITY_TEST_USER", username = "test") public void testRunQueryEndpoint_SucceedsOnValidStructuredQueryWith201() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn(Mono.just(1L)).when(queryHandlerService).runQuery(any(StructuredQuery.class), eq("test")); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); var mvcResult = mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY)).with(csrf()) .contentType(APPLICATION_JSON) @@ -148,11 +151,12 @@ public void testRunQueryEndpoint_SucceedsOnValidStructuredQueryWith201() throws @WithMockUser(roles = "FEASIBILITY_TEST_USER", username = "test") public void testRunQueryEndpoint_FailsOnDownstreamServiceError() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); var dispatchError = new QueryDispatchException("something went wrong"); doReturn(Mono.error(dispatchError)).when(queryHandlerService).runQuery(any(StructuredQuery.class), eq("test")); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); var mvcResult = mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY)).with(csrf()) .contentType(APPLICATION_JSON) @@ -168,9 +172,10 @@ public void testRunQueryEndpoint_FailsOnDownstreamServiceError() throws Exceptio @WithMockUser(roles = "FEASIBILITY_TEST_USER", username = "test") public void testRunQueryEndpoint_FailsOnSoftQuotaExceeded() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn((long)quotaSoftCreateAmount + 1).when(queryHandlerService).getAmountOfQueriesByUserAndInterval(any(String.class), any(Integer.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); var mvcResult = mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY)).with(csrf()) .contentType(APPLICATION_JSON) @@ -186,36 +191,39 @@ public void testRunQueryEndpoint_FailsOnSoftQuotaExceeded() throws Exception { @WithMockUser(roles = "FEASIBILITY_TEST_USER", username = "test") public void testValidateQueryEndpoint_SucceedsOnValidQuery() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY + "/validate")).with(csrf()) .contentType(APPLICATION_JSON) .content(jsonUtil.writeValueAsString(testQuery))) .andExpect(status().isOk()) - .andExpect(jsonPath("$.invalidCriteria").isEmpty()); + .andExpect(jsonPath("$.inclusionCriteria[0].[0].issues").isArray()) + .andExpect(jsonPath("$.inclusionCriteria[0].[0].issues").isEmpty()); } @Test @WithMockUser(roles = "FEASIBILITY_TEST_USER") public void testValidateQueryEndpoint_SucceedsDespiteInvalidCriteriaWith200() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); - var invalidCriterion = createInvalidCriterion(); + var annotatedQuery = createValidAnnotatedStructuredQuery(true); - doReturn(List.of(invalidCriterion)).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY + "/validate")).with(csrf()) .contentType(APPLICATION_JSON) .content(jsonUtil.writeValueAsString(testQuery))) .andExpect(status().isOk()) - .andExpect(jsonPath("$.invalidCriteria").exists()) - .andExpect(jsonPath("$.invalidCriteria").isNotEmpty()); + .andExpect(jsonPath("$.inclusionCriteria[0].[0].issues").isArray()) + .andExpect(jsonPath("$.inclusionCriteria[0].[0].issues").isNotEmpty()); } @Test @WithMockUser(roles = "FEASIBILITY_TEST_USER", username = "test") public void testRunQueryEndpoint_FailsOnBeingBlacklistedWith403() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); UserBlacklist userBlacklistEntry = new UserBlacklist(); userBlacklistEntry.setId(1L); userBlacklistEntry.setUserId("test"); @@ -223,7 +231,7 @@ public void testRunQueryEndpoint_FailsOnBeingBlacklistedWith403() throws Excepti Optional userBlacklistOptional = Optional.of(userBlacklistEntry); doReturn(userBlacklistOptional).when(userBlacklistRepository).findByUserId(any(String.class)); doReturn(Mono.just(1L)).when(queryHandlerService).runQuery(any(StructuredQuery.class), eq("test")); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); var mvcResult = mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY)).with(csrf()) .contentType(APPLICATION_JSON) @@ -239,10 +247,11 @@ public void testRunQueryEndpoint_FailsOnBeingBlacklistedWith403() throws Excepti @WithMockUser(roles = "FEASIBILITY_TEST_USER", username = "test") public void testRunQueryEndpoint_FailsOnExceedingHardLimitWith403() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn((long)quotaHardCreateAmount).when(queryHandlerService).getAmountOfQueriesByUserAndInterval(any(String.class), eq(quotaHardCreateIntervalMinutes)); doReturn(Mono.just(1L)).when(queryHandlerService).runQuery(any(StructuredQuery.class), eq("test")); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); var mvcResult = mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY)).with(csrf()) .contentType(APPLICATION_JSON) @@ -258,12 +267,13 @@ public void testRunQueryEndpoint_FailsOnExceedingHardLimitWith403() throws Excep @WithMockUser(roles = {"FEASIBILITY_TEST_USER", "FEASIBILITY_TEST_POWER"}, username = "test") public void testRunQueryEndpoint_SucceedsOnExceedingHardlimitAsPowerUserWith201() throws Exception { StructuredQuery testQuery = createValidStructuredQuery(); + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn(true).when(authenticationHelper).hasAuthority(any(Authentication.class), eq("FEASIBILITY_TEST_POWER")); doReturn((long)quotaHardCreateAmount).when(queryHandlerService).getAmountOfQueriesByUserAndInterval(any(String.class), eq(quotaHardCreateIntervalMinutes)); doReturn((long)(quotaSoftCreateAmount - 1)).when(queryHandlerService).getAmountOfQueriesByUserAndInterval(any(String.class), eq(quotaSoftCreateIntervalMinutes)); doReturn(Mono.just(1L)).when(queryHandlerService).runQuery(any(StructuredQuery.class), eq("test")); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); var mvcResult = mockMvc.perform(post(URI.create(PATH_API + PATH_QUERY)).with(csrf()) .contentType(APPLICATION_JSON) @@ -287,7 +297,8 @@ public void testGetQueryList_SucceedsWithValidation() throws Exception { mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY)).with(csrf()).param("skipValidation", "false")) .andExpect(status().isOk()) .andExpect(jsonPath("$[0].id").value(queryId)) - .andExpect(jsonPath("$[0].invalidCriteria").isNotEmpty()); + .andExpect(jsonPath("$[0].isValid").exists()) + .andExpect(jsonPath("$[0].isValid").value(true)); } @Test @@ -300,8 +311,7 @@ public void testGetQueryList_SucceedsWithoutValidation() throws Exception { mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY)).with(csrf()).param("skipValidation", "true")) .andExpect(status().isOk()) .andExpect(jsonPath("$[0].id").value(queryId)) - .andExpect(jsonPath("$[0].invalidCriteria").isArray()) - .andExpect(jsonPath("$[0].invalidCriteria").isEmpty()); + .andExpect(jsonPath("$[0].isValid").doesNotExist()); } @Test @@ -314,7 +324,8 @@ public void testGetQueryList_SucceedsWithoutDefiningSkipValidation() throws Exce mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY)).with(csrf())) .andExpect(status().isOk()) .andExpect(jsonPath("$[0].id").value(queryId)) - .andExpect(jsonPath("$[0].invalidCriteria").isNotEmpty()); + .andExpect(jsonPath("$[0].isValid").exists()) + .andExpect(jsonPath("$[0].isValid").value(true)); } @Test @@ -346,9 +357,10 @@ public void testGetQueryListForUser_ReturnsEmptyOnUnknownUser() throws Exception @WithMockUser(roles = {"FEASIBILITY_TEST_USER"}, username = "test") public void testGetQuery_succeeds() throws Exception { long queryId = 1; + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn("test").when(queryHandlerService).getAuthorId(any(Long.class)); doReturn(createValidApiQuery(queryId)).when(queryHandlerService).getQuery(any(Long.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + "/" + queryId)).with(csrf())) .andExpect(status().isOk()) @@ -359,9 +371,10 @@ public void testGetQuery_succeeds() throws Exception { @WithMockUser(roles = {"FEASIBILITY_TEST_USER"}, username = "test") public void testGetQuery_failsOnWrongAuthorWith403() throws Exception { long queryId = 1; + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn("some-other-user").when(queryHandlerService).getAuthorId(any(Long.class)); doReturn(createValidApiQuery(queryId)).when(queryHandlerService).getQuery(any(Long.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + "/" + queryId)).with(csrf())) .andExpect(status().isForbidden()); @@ -660,6 +673,7 @@ private static StructuredQuery createValidStructuredQuery() { var inclusionCriterion = Criterion.builder() .termCodes(List.of(termCode)) .attributeFilters(List.of()) + .context(termCode) .build(); return StructuredQuery.builder() .version(URI.create("http://to_be_decided.com/draft-2/schema#")) @@ -669,6 +683,27 @@ private static StructuredQuery createValidStructuredQuery() { .build(); } + @NotNull + private static StructuredQuery createValidAnnotatedStructuredQuery(boolean withIssues) { + var termCode = TermCode.builder() + .code("LL2191-6") + .system("http://loinc.org") + .display("Geschlecht") + .build(); + var inclusionCriterion = Criterion.builder() + .termCodes(List.of(termCode)) + .attributeFilters(List.of()) + .context(termCode) + .validationIssues(withIssues ? List.of(ValidationIssue.TERMCODE_CONTEXT_COMBINATION_INVALID) : List.of()) + .build(); + return StructuredQuery.builder() + .version(URI.create("http://to_be_decided.com/draft-2/schema#")) + .inclusionCriteria(List.of(List.of(inclusionCriterion))) + .exclusionCriteria(List.of()) + .display("foo") + .build(); + } + @NotNull private static de.numcodex.feasibility_gui_backend.query.persistence.Query createValidQuery(long id) { var query = new de.numcodex.feasibility_gui_backend.query.persistence.Query(); @@ -690,12 +725,20 @@ private static de.numcodex.feasibility_gui_backend.query.persistence.QueryConten @NotNull private static QueryListEntry createValidQueryListEntry(long id, boolean skipValidation) { - return QueryListEntry.builder() + if (skipValidation) { + return QueryListEntry.builder() .id(id) .label("abc") .createdAt(new Timestamp(new java.util.Date().getTime())) - .invalidCriteria(skipValidation ? List.of() : List.of(createInvalidCriterion())) .build(); + } else { + return QueryListEntry.builder() + .id(id) + .label("abc") + .createdAt(new Timestamp(new java.util.Date().getTime())) + .isValid(true) + .build(); + } } @NotNull @@ -722,7 +765,6 @@ private static Query createValidApiQuery(long id) { .content(createValidStructuredQuery()) .label("test") .comment("test") - .invalidCriteria(List.of()) .build(); } diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestControllerIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestControllerIT.java index 4dc454b9..54d52bb8 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestControllerIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/v3/QueryTemplateHandlerRestControllerIT.java @@ -7,17 +7,19 @@ import de.numcodex.feasibility_gui_backend.query.QueryHandlerService; import de.numcodex.feasibility_gui_backend.query.api.QueryTemplate; import de.numcodex.feasibility_gui_backend.query.api.StructuredQuery; +import de.numcodex.feasibility_gui_backend.query.api.status.ValidationIssue; import de.numcodex.feasibility_gui_backend.query.api.validation.QueryTemplateValidatorSpringConfig; import de.numcodex.feasibility_gui_backend.query.persistence.Query; import de.numcodex.feasibility_gui_backend.query.ratelimiting.AuthenticationHelper; import de.numcodex.feasibility_gui_backend.query.ratelimiting.RateLimitingServiceSpringConfig; import de.numcodex.feasibility_gui_backend.query.templates.QueryTemplateException; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; -import org.hamcrest.Matchers; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Tag; 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.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; @@ -34,6 +36,8 @@ import java.util.concurrent.ThreadLocalRandom; import static de.numcodex.feasibility_gui_backend.config.WebSecurityConfig.*; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; import static org.springframework.http.MediaType.APPLICATION_JSON; @@ -64,7 +68,7 @@ public class QueryTemplateHandlerRestControllerIT { private QueryHandlerService queryHandlerService; @MockBean - private TermCodeValidation termCodeValidation; + private StructuredQueryValidation structuredQueryValidation; @MockBean private AuthenticationHelper authenticationHelper; @@ -118,10 +122,11 @@ public void testStoreQueryTemplate_failsOnDuplicateWith409() throws Exception { @WithMockUser(roles = "FEASIBILITY_TEST_USER") public void testGetQueryTemplate_succeeds() throws Exception { long queryTemplateId = 1; + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn(createValidPersistenceQueryTemplateToGet(queryTemplateId)).when(queryHandlerService).getQueryTemplate(any(Long.class), any(String.class)); doReturn(createValidApiQueryTemplateToGet(queryTemplateId)).when(queryHandlerService).convertTemplatePersistenceToApi(any(de.numcodex.feasibility_gui_backend.query.persistence.QueryTemplate.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + PATH_TEMPLATE + "/" + queryTemplateId)).with(csrf())) .andExpect(status().isOk()) @@ -132,10 +137,11 @@ public void testGetQueryTemplate_succeeds() throws Exception { @WithMockUser(roles = "FEASIBILITY_TEST_USER") public void testGetQueryTemplate_failsOnNotFound() throws Exception { long queryTemplateId = 1; + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doThrow(QueryTemplateException.class).when(queryHandlerService).getQueryTemplate(any(Long.class), any(String.class)); doReturn(createValidApiQueryTemplateToGet(queryTemplateId)).when(queryHandlerService).convertTemplatePersistenceToApi(any(de.numcodex.feasibility_gui_backend.query.persistence.QueryTemplate.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + PATH_TEMPLATE + "/" + queryTemplateId)).with(csrf())) .andExpect(status().isNotFound()); @@ -145,10 +151,11 @@ public void testGetQueryTemplate_failsOnNotFound() throws Exception { @WithMockUser(roles = "FEASIBILITY_TEST_USER") public void testGetQueryTemplate_failsOnJsonError() throws Exception { long queryTemplateId = 1; + var annotatedQuery = createValidAnnotatedStructuredQuery(false); doReturn(createValidPersistenceQueryTemplateToGet(queryTemplateId)).when(queryHandlerService).getQueryTemplate(any(Long.class), any(String.class)); doThrow(JsonProcessingException.class).when(queryHandlerService).convertTemplatePersistenceToApi(any(de.numcodex.feasibility_gui_backend.query.persistence.QueryTemplate.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(annotatedQuery).when(structuredQueryValidation).annotateStructuredQuery(any(StructuredQuery.class), any(Boolean.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + PATH_TEMPLATE + "/" + queryTemplateId)).with(csrf())) .andExpect(status().isInternalServerError()); @@ -178,32 +185,21 @@ public void testGetQueryTemplateList_emptyListOnJsonErrors() throws Exception { .andExpect(jsonPath("$.length()").value(0)); } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) @WithMockUser(roles = "FEASIBILITY_TEST_USER") - public void testGetQueryTemplateListWithValidation_succeedsWithoutValidationErrors() throws Exception { + public void testGetQueryTemplateListWithValidation_succeeds(boolean isValid) throws Exception { int listSize = 5; doReturn(createValidPersistenceQueryTemplateListToGet(listSize)).when(queryHandlerService).getQueryTemplatesForAuthor(any(String.class)); doReturn(createValidApiQueryTemplateToGet(ThreadLocalRandom.current().nextInt())).when(queryHandlerService).convertTemplatePersistenceToApi(any(de.numcodex.feasibility_gui_backend.query.persistence.QueryTemplate.class)); - doReturn(List.of()).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(isValid).when(structuredQueryValidation).isValid(any(StructuredQuery.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + PATH_TEMPLATE)).with(csrf())) .andExpect(status().isOk()) .andExpect(jsonPath("$.length()").value(listSize)) .andExpect(jsonPath("$.[*].id").exists()) - .andExpect(jsonPath("$.[*].isValid", Matchers.not(Matchers.contains(false)))); - } - - @Test - @WithMockUser(roles = "FEASIBILITY_TEST_USER") - public void testGetQueryTemplateListWithValidation_succeedsWithValidationErrors() throws Exception { - int listSize = 5; - doReturn(createValidPersistenceQueryTemplateListToGet(listSize)).when(queryHandlerService).getQueryTemplatesForAuthor(any(String.class)); - doReturn(createValidApiQueryTemplateToGet(ThreadLocalRandom.current().nextInt())).when(queryHandlerService).convertTemplatePersistenceToApi(any(de.numcodex.feasibility_gui_backend.query.persistence.QueryTemplate.class)); - doReturn(List.of(createInvalidCriterion())).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); - - mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + PATH_TEMPLATE)).with(csrf())) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.length()").value(listSize)); + .andExpect(jsonPath("$.[*].isValid").exists()) + .andExpect(jsonPath("$.[*].isValid", everyItem(equalTo(isValid)))); } @Test @@ -212,7 +208,7 @@ public void testGetQueryTemplateListWithValidation_emptyListOnJsonErrors() throw int listSize = 5; doReturn(createValidPersistenceQueryTemplateListToGet(listSize)).when(queryHandlerService).getQueryTemplatesForAuthor(any(String.class)); doThrow(JsonProcessingException.class).when(queryHandlerService).convertTemplatePersistenceToApi(any(de.numcodex.feasibility_gui_backend.query.persistence.QueryTemplate.class)); - doReturn(List.of(createInvalidCriterion())).when(termCodeValidation).getInvalidCriteria(any(StructuredQuery.class)); + doReturn(false).when(structuredQueryValidation).isValid(any(StructuredQuery.class)); mockMvc.perform(get(URI.create(PATH_API + PATH_QUERY + PATH_TEMPLATE)).with(csrf())) .andExpect(status().isOk()) @@ -275,7 +271,6 @@ private static QueryTemplate createValidQueryTemplateToStore(long id) { .content(createValidStructuredQuery()) .label("TestLabel") .comment("TestComment") - .invalidCriteria(List.of()) .isValid(true) .build(); } @@ -309,7 +304,6 @@ private static QueryTemplate createValidApiQueryTemplateToGet(long id) { .comment("TestComment") .lastModified(new Timestamp(new java.util.Date().getTime()).toString()) .createdBy("someone") - .invalidCriteria(List.of()) .isValid(true) .build(); } @@ -319,6 +313,7 @@ private static StructuredQuery createValidStructuredQuery() { var inclusionCriterion = Criterion.builder() .termCodes(List.of(createTermCode())) .attributeFilters(List.of()) + .context(createTermCode()) .build(); return StructuredQuery.builder() .version(URI.create("http://to_be_decided.com/draft-2/schema#")) @@ -328,6 +323,27 @@ private static StructuredQuery createValidStructuredQuery() { .build(); } + @NotNull + private static StructuredQuery createValidAnnotatedStructuredQuery(boolean withIssues) { + var termCode = TermCode.builder() + .code("LL2191-6") + .system("http://loinc.org") + .display("Geschlecht") + .build(); + var inclusionCriterion = Criterion.builder() + .termCodes(List.of(termCode)) + .attributeFilters(List.of()) + .context(termCode) + .validationIssues(withIssues ? List.of(ValidationIssue.TERMCODE_CONTEXT_COMBINATION_INVALID) : List.of()) + .build(); + return StructuredQuery.builder() + .version(URI.create("http://to_be_decided.com/draft-2/schema#")) + .inclusionCriteria(List.of(List.of(inclusionCriterion))) + .exclusionCriteria(List.of()) + .display("foo") + .build(); + } + @NotNull private static TermCode createTermCode() { return TermCode.builder() diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/terminology/v3/TerminologyRestControllerIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/terminology/v3/TerminologyRestControllerIT.java index 519a0894..7cb43e33 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/terminology/v3/TerminologyRestControllerIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/terminology/v3/TerminologyRestControllerIT.java @@ -10,7 +10,7 @@ import de.numcodex.feasibility_gui_backend.terminology.UiProfileNotFoundException; import de.numcodex.feasibility_gui_backend.terminology.api.CategoryEntry; import de.numcodex.feasibility_gui_backend.terminology.api.TerminologyEntry; -import de.numcodex.feasibility_gui_backend.terminology.validation.TermCodeValidation; +import de.numcodex.feasibility_gui_backend.terminology.validation.StructuredQueryValidation; import org.hamcrest.Matchers; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -59,7 +59,7 @@ public class TerminologyRestControllerIT { private ObjectMapper jsonUtil; @MockBean - private TermCodeValidation termCodeValidation; + private StructuredQueryValidation structuredQueryValidation; @MockBean private TerminologyService terminologyService; diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/terminology/validation/StructuredQueryValidationTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/terminology/validation/StructuredQueryValidationTest.java new file mode 100644 index 00000000..b0c1239f --- /dev/null +++ b/src/test/java/de/numcodex/feasibility_gui_backend/terminology/validation/StructuredQueryValidationTest.java @@ -0,0 +1,154 @@ +package de.numcodex.feasibility_gui_backend.terminology.validation; + +import de.numcodex.feasibility_gui_backend.common.api.Criterion; +import de.numcodex.feasibility_gui_backend.common.api.TermCode; +import de.numcodex.feasibility_gui_backend.query.api.StructuredQuery; +import de.numcodex.feasibility_gui_backend.terminology.TerminologyService; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +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.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.net.URI; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +@Tag("terminology") +@ExtendWith(MockitoExtension.class) +class StructuredQueryValidationTest { + + @Mock + private TerminologyService terminologyService; + + private StructuredQueryValidation structuredQueryValidation; + + @BeforeEach + void setUp() { + structuredQueryValidation = new StructuredQueryValidation(terminologyService); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testIsValid_trueOnValidCriteria(boolean withExclusionCriteria) { + doReturn(true).when(terminologyService).isExistingTermCode(any(String.class), any(String.class), isNull()); + + var isValid = structuredQueryValidation.isValid(createValidStructuredQuery(withExclusionCriteria)); + + assertTrue(isValid); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testIsValid_falseOnInvalidCriteria(boolean withExclusionCriteria) { + doReturn(false).when(terminologyService).isExistingTermCode(any(String.class), any(String.class), isNull()); + + var isValid = structuredQueryValidation.isValid(createValidStructuredQuery(withExclusionCriteria)); + + assertFalse(isValid); + } + + @Test + void testIsValid_falseOnMissingContext() { + var isValid = structuredQueryValidation.isValid(createStructuredQueryWithoutContext()); + + assertFalse(isValid); + } + + @ParameterizedTest + @CsvSource({"true,true", "true,false", "false,true", "false,false"}) + void testAnnotateStructuredQuery_emptyIssuesOnValidCriteriaOrSkippedValidation(String withExclusionCriteriaString, String skipValidationString) { + boolean withExclusionCriteria = Boolean.parseBoolean(withExclusionCriteriaString); + boolean skipValidation = Boolean.parseBoolean(skipValidationString); + if (!skipValidation) { + doReturn(true).when(terminologyService).isExistingTermCode(any(String.class), any(String.class), isNull()); + } + + var annotatedStructuredQuery = structuredQueryValidation.annotateStructuredQuery(createValidStructuredQuery(withExclusionCriteria), skipValidation); + + assertTrue(annotatedStructuredQuery.inclusionCriteria().get(0).get(0).validationIssues().isEmpty()); + } + + @ParameterizedTest + @CsvSource({"true,true", "true,false", "false,true", "false,false"}) + void testAnnotateStructuredQuery_nonEmptyIssuesOnInvalidCriteria(String withExclusionCriteriaString, String skipValidationString) { + boolean withExclusionCriteria = Boolean.parseBoolean(withExclusionCriteriaString); + boolean skipValidation = Boolean.parseBoolean(skipValidationString); + if (!skipValidation) { + doReturn(false).when(terminologyService).isExistingTermCode(any(String.class), any(String.class), isNull()); + } + + var annotatedStructuredQuery = structuredQueryValidation.annotateStructuredQuery(createValidStructuredQuery(withExclusionCriteria), skipValidation); + + if (skipValidation) { + assertTrue(annotatedStructuredQuery.inclusionCriteria().get(0).get(0).validationIssues().isEmpty()); + } else { + assertFalse(annotatedStructuredQuery.inclusionCriteria().get(0).get(0).validationIssues().isEmpty()); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testAnnotateStructuredQuery_nonEmptyIssuesOnMissingContext(boolean skipValidation) { + var annotatedStructuredQuery = structuredQueryValidation.annotateStructuredQuery(createStructuredQueryWithoutContext(), skipValidation); + + if (skipValidation) { + assertTrue(annotatedStructuredQuery.inclusionCriteria().get(0).get(0).validationIssues().isEmpty()); + } else { + assertFalse(annotatedStructuredQuery.inclusionCriteria().get(0).get(0).validationIssues().isEmpty()); + } + } + + @NotNull + private static StructuredQuery createValidStructuredQuery(boolean withExclusionCriteria) { + var context = TermCode.builder() + .code("Laboruntersuchung") + .system("fdpg.mii.cds") + .display("Laboruntersuchung") + .version("1.0.0") + .build(); + var termCode = TermCode.builder() + .code("19113-0") + .system("http://loinc.org") + .display("IgE") + .build(); + var criterion = Criterion.builder() + .termCodes(List.of(termCode)) + .context(context) + .attributeFilters(List.of()) + .build(); + return StructuredQuery.builder() + .version(URI.create("http://to_be_decided.com/draft-2/schema#")) + .inclusionCriteria(List.of(List.of(criterion))) + .exclusionCriteria(withExclusionCriteria ? List.of(List.of(criterion)) : List.of()) + .display("foo") + .build(); + } + + @NotNull + private static StructuredQuery createStructuredQueryWithoutContext() { + var termCode = TermCode.builder() + .code("19113-0") + .system("http://loinc.org") + .display("IgE") + .build(); + var criterion = Criterion.builder() + .termCodes(List.of(termCode)) + .attributeFilters(List.of()) + .build(); + return StructuredQuery.builder() + .version(URI.create("http://to_be_decided.com/draft-2/schema#")) + .inclusionCriteria(List.of(List.of(criterion))) + .exclusionCriteria(List.of()) + .display("foo") + .build(); + } +} diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/terminology/validation/TermCodeValidationTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/terminology/validation/TermCodeValidationTest.java deleted file mode 100644 index 129309e2..00000000 --- a/src/test/java/de/numcodex/feasibility_gui_backend/terminology/validation/TermCodeValidationTest.java +++ /dev/null @@ -1,83 +0,0 @@ -package de.numcodex.feasibility_gui_backend.terminology.validation; - -import de.numcodex.feasibility_gui_backend.common.api.Criterion; -import de.numcodex.feasibility_gui_backend.common.api.TermCode; -import de.numcodex.feasibility_gui_backend.query.api.StructuredQuery; -import de.numcodex.feasibility_gui_backend.terminology.TerminologyService; -import org.jetbrains.annotations.NotNull; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Tag; -import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.net.URI; -import java.util.List; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; - -@Tag("terminology") -@ExtendWith(MockitoExtension.class) -class TermCodeValidationTest { - - @Mock - private TerminologyService terminologyService; - - private TermCodeValidation termCodeValidation; - - @BeforeEach - void setUp() { - termCodeValidation = new TermCodeValidation(terminologyService); - } - - @ParameterizedTest - @ValueSource(strings = {"true", "false"}) - void getInvalidCriteria_emptyOnValidCriteria(boolean withExclusionCriteria) { - doReturn(true).when(terminologyService).isExistingTermCode(any(String.class), any(String.class), isNull()); - - var invalidCriteria = termCodeValidation.getInvalidCriteria(createValidStructuredQuery(withExclusionCriteria)); - - assertTrue(invalidCriteria.isEmpty()); - } - - @ParameterizedTest - @ValueSource(strings = {"true", "false"}) - void getInvalidCriteria_notEmptyOnInvalidTermcode(boolean withExclusionCriteria) { - doReturn(false).when(terminologyService).isExistingTermCode(any(String.class), any(String.class), isNull()); - - var invalidCriteria = termCodeValidation.getInvalidCriteria(createValidStructuredQuery(withExclusionCriteria)); - - assertFalse(invalidCriteria.isEmpty()); - assertEquals(withExclusionCriteria ? 2 : 1, invalidCriteria.size()); - } - - @NotNull - private static StructuredQuery createValidStructuredQuery(boolean withExclusionCriteria) { - var context = TermCode.builder() - .code("Laboruntersuchung") - .system("fdpg.mii.cds") - .display("Laboruntersuchung") - .version("1.0.0") - .build(); - var termCode = TermCode.builder() - .code("19113-0") - .system("http://loinc.org") - .display("IgE") - .build(); - var criterion = Criterion.builder() - .termCodes(List.of(termCode)) - .context(context) - .attributeFilters(List.of()) - .build(); - return StructuredQuery.builder() - .version(URI.create("http://to_be_decided.com/draft-2/schema#")) - .inclusionCriteria(List.of(List.of(criterion))) - .exclusionCriteria(withExclusionCriteria ? List.of(List.of(criterion)) : List.of()) - .display("foo") - .build(); - } -}