Prevent Compiling Empty List of Attribute Group Filters

bastianschaffer · alexanderkiel · commit 1a7791b52aa6 · 2025-06-11T14:09:33.000Z
Fixes: #299
diff --git a/src/main/java/de/medizininformatikinitiative/torch/service/CrtdlValidatorService.java b/src/main/java/de/medizininformatikinitiative/torch/service/CrtdlValidatorService.java
@@ -114,16 +114,20 @@ private AnnotatedAttributeGroup annotateGroup(AttributeGroup attributeGroup, Str
             annotatedAttributes.add(new AnnotatedAttribute(attribute.attributeRef(), fhirTerser[0], fhirTerser[1], attribute.mustHave(), attribute.linkedGroups()));
         }
 
-        AnnotatedAttributeGroup standardGroup = attributeGenerator.generate(attributeGroup, patientGroupId);
-        try {
-            Predicate<Resource> filter = filterService.compileFilter(attributeGroup.filter(), definition.getType());
-            return standardGroup
-                    .addAttributes(annotatedAttributes)
-                    .setCompiledFilter(filter);
-        } catch (Exception e) {
-            throw new RuntimeException(e);
+        AnnotatedAttributeGroup group = attributeGenerator
+                .generate(attributeGroup, patientGroupId)
+                .addAttributes(annotatedAttributes);
+
+        if (!attributeGroup.filter().isEmpty()) {
+            try {
+                Predicate<Resource> filter = filterService.compileFilter(attributeGroup.filter(), definition.getType());
+                return group.setCompiledFilter(filter);
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
         }
 
+        return group;
     }
 
 
diff --git a/src/main/java/de/medizininformatikinitiative/torch/service/FilterService.java b/src/main/java/de/medizininformatikinitiative/torch/service/FilterService.java
@@ -23,6 +23,8 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
+import static java.util.Objects.requireNonNull;
+
 /**
  * A Service that handles filter preprocessing.
  */
@@ -66,27 +68,31 @@ public void init() {
      * Preprocesses multiple filters to be ready for evaluation on a specific resource type.
      * The predicate can be evaluated on as many resources as needed.
      *
-     * @param attributeGroupFilters the filters to be preprocessed
-     * @param resourceType          the resource type of the resource that the filters should later be applied on
-     * @return the compiled filters containing parsed FHIRPath expressions that are ready to be evaluated
+     * @param attributeGroupFilters     the filters to be preprocessed
+     * @param resourceType              the resource type of the resource that the filters should later be applied on
+     * @return                          the compiled filters containing parsed FHIRPath expressions that are ready to be evaluated
+     * @throws IllegalArgumentException if attributeGroupFilters is null or empty
      */
     public Predicate<Resource> compileFilter(List<Filter> attributeGroupFilters, String resourceType) {
+        if (attributeGroupFilters == null || attributeGroupFilters.isEmpty()) {
+            throw new IllegalArgumentException("An empty list of filters can't be compiled.");
+        }
         var parsedFilters = attributeGroupFilters.stream()
                 .map(filter ->
                         new ParsedFilter(
                                 filter,
                                 parseFhirPath(searchParameters.get(List.of(filter.name(), filter.type(), resourceType)))))
                 .toList();
 
-        if (parsedFilters.isEmpty()) {
-            logger.warn("Could not find any matching search parameter for filter. This can later result in unexpected false" +
-                    " negative results of the 'test' method of 'CompiledFilter'.");
-        }
-
         return new CompiledFilter(parsedFilters, fhirPathEngine);
     }
 
     private IFhirPath.IParsedExpression parseFhirPath(String path) {
+        if (path == null || path.isEmpty()) {
+            throw new IllegalArgumentException("Could not find any matching search parameter for filter. This can later " +
+                    "result in unexpected false negative results of the 'test' method of 'CompiledFilter'.");
+        }
+
         try {
             return fhirPathEngine.parse(path);
         } catch (Exception e) {
@@ -122,6 +128,13 @@ private static boolean isValidSearchParam(SearchParameter searchParam) {
     private record CompiledFilter(List<ParsedFilter> filters,
                                   IFhirPath fhirPathEngine) implements Predicate<Resource> {
 
+        public CompiledFilter {
+            if (filters.isEmpty()) {
+                throw new IllegalArgumentException("CompiledFilter must have at least one parsed filter, but got none.");
+            }
+            requireNonNull(fhirPathEngine);
+        }
+
         @Override
         public boolean test(Resource resource) {
 
@@ -213,5 +226,9 @@ private boolean evaluateDate(Base found, Filter filter) {
     }
 
     private record ParsedFilter(Filter filter, IFhirPath.IParsedExpression parsedExpression) {
+        public ParsedFilter {
+            requireNonNull(filter);
+            requireNonNull(parsedExpression);
+        }
     }
 }
diff --git a/src/test/java/de/medizininformatikinitiative/torch/service/CrtdlValidatorServiceTest.java b/src/test/java/de/medizininformatikinitiative/torch/service/CrtdlValidatorServiceTest.java
@@ -12,6 +12,7 @@
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
+import java.time.LocalDate;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -70,23 +71,38 @@ void referenceWithoutLinkedGroups() throws ValidationException {
     }
 
     @Test
-    void validInput() throws ValidationException {
-        Crtdl crtdl = new Crtdl(node, new DataExtraction(List.of(patientGroup, new AttributeGroup("test", "https://www.medizininformatik-initiative.de/fhir/core/modul-labor/StructureDefinition/ObservationLab", List.of(), List.of(new Filter("token", "code", List.of(new Code("some-system", "some-code"))))))));
+    void validInput_withoutFilter() throws ValidationException {
+        Crtdl crtdl = new Crtdl(node, new DataExtraction(List.of(patientGroup)));
 
-        assertThat(validatorService.validate(crtdl).dataExtraction().attributeGroups().get(0).attributes()).isEqualTo(
+        var validatedCrtdl = validatorService.validate(crtdl);
+
+        assertThat(validatedCrtdl).isNotNull();
+        assertThat(validatedCrtdl.dataExtraction().attributeGroups().getFirst().compiledFilter()).isNull();
+        assertThat(validatedCrtdl.dataExtraction().attributeGroups().getFirst().attributes()).isEqualTo(
                 List.of(
                         new AnnotatedAttribute("Patient.id", "Patient.id", "Patient.id", false),
                         new AnnotatedAttribute("Patient.meta.profile", "Patient.meta.profile", "Patient.meta.profile", false)
                 ));
+    }
+
+    @Test
+    void validInput_withFilter() throws ValidationException {
+        Crtdl crtdl = new Crtdl(node, new DataExtraction(List.of(patientGroup,
+                new AttributeGroup("test", "https://www.medizininformatik-initiative.de/fhir/core/modul-labor/StructureDefinition/ObservationLab",
+                        List.of(), List.of(new Filter("token", "code", List.of(new Code("some-system", "some-code"))))))));
 
+        var validatedCrtdl = validatorService.validate(crtdl);
+
+        assertThat(validatedCrtdl).isNotNull();
+        assertThat(validatedCrtdl.dataExtraction().attributeGroups().get(1).compiledFilter()).isNotNull();
         assertThat(validatorService.validate(crtdl).dataExtraction().attributeGroups().get(1).attributes()).isEqualTo(
                 List.of(
                         new AnnotatedAttribute("Observation.id", "Observation.id", "Observation.id", false),
                         new AnnotatedAttribute("Observation.meta.profile", "Observation.meta.profile", "Observation.meta.profile", false),
                         new AnnotatedAttribute("Observation.subject", "Observation.subject", "Observation.subject", false, List.of("patientGroupId"))
                 ));
-
     }
 
 
+
 }
diff --git a/src/test/java/de/medizininformatikinitiative/torch/service/FilterServiceTest.java b/src/test/java/de/medizininformatikinitiative/torch/service/FilterServiceTest.java
@@ -1,6 +1,9 @@
 package de.medizininformatikinitiative.torch.service;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.fhirpath.IFhirPath;
+import de.medizininformatikinitiative.torch.exceptions.ReferenceToPatientException;
+import de.medizininformatikinitiative.torch.model.crtdl.AttributeGroup;
 import de.medizininformatikinitiative.torch.model.crtdl.Code;
 import de.medizininformatikinitiative.torch.model.crtdl.Filter;
 import org.hl7.fhir.r4.model.AllergyIntolerance;
@@ -20,6 +23,8 @@
 import java.time.format.DateTimeFormatter;
 import java.util.List;
 
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -66,6 +71,21 @@ public void test_compile_MultipleResources() {
         assertFalse(result_3);
     }
 
+    @Test
+    public void testEmptyFilter() {
+        assertThatThrownBy(() -> filterService.compileFilter(List.of(), OBSERVATION))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("An empty list of filters can't be compiled.");
+    }
+
+    @Test
+    public void test_nonExistingSearchParam() {
+        assertThatThrownBy(() -> filterService.compileFilter(List.of(new Filter("foo", "bar", List.of())), OBSERVATION))
+                .isInstanceOf(RuntimeException.class)
+                .hasMessageContaining("Could not find any matching search parameter for filter. This can later " +
+                        "result in unexpected false negative results of the 'test' method of 'CompiledFilter'.");
+    }
+
     @Nested
     class TestCodeableConcept {
 
