diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/5996-do-not-throw-exceptions-if-encountering-unsupported-filters.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/5996-do-not-throw-exceptions-if-encountering-unsupported-filters.yaml new file mode 100644 index 000000000000..73b23b4b1eab --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/5996-do-not-throw-exceptions-if-encountering-unsupported-filters.yaml @@ -0,0 +1,8 @@ +--- +type: fix +issue: 5006 +title: "Expanding a ValueSet on a property using a filter operator that is + not supported (ISA, NOTISA, etc) would throw an exception and fail + expansion. + This has been fixed. +" diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java index 94c929910827..c1d0b605ceb3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java @@ -1452,7 +1452,6 @@ private void handleFilterPropertyDefault( } else { Term term = new Term(CONCEPT_PROPERTY_PREFIX_NAME + theFilter.getProperty(), value); switch (theFilter.getOp()) { - case ISA: case EQUAL: theB.must(theF.match().field(term.field()).matching(term.text())); break; @@ -1468,13 +1467,21 @@ private void handleFilterPropertyDefault( theB.filter(theF.terms().field(term.field()).matchingAny(valueSet)); } break; + case ISA: + case ISNOTA: + case DESCENDENTOF: + case GENERALIZES: default: /* * We do not need to handle REGEX, because that's handled in parent - * We also don't handle EXISTS because that's a separate area (with different term) + * We also don't handle EXISTS because that's a separate area (with different term). + * We add a match-none filter because otherwise it matches everything (not desired). */ - throw new InvalidRequestException(Msg.code(2526) + "Unsupported property filter " - + theFilter.getOp().getDisplay()); + ourLog.error( + "Unsupported property filter {}. This may affect expansion, but will not cause errors.", + theFilter.getOp().getDisplay()); + theB.must(theF.matchNone()); + break; } } } diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/term/IValueSetExpansionIT.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/term/IValueSetExpansionIT.java index 074b5eae0344..8c49da207f7b 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/term/IValueSetExpansionIT.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/term/IValueSetExpansionIT.java @@ -11,6 +11,10 @@ import ca.uhn.fhir.jpa.term.api.ITermReadSvc; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import org.hl7.fhir.r4.model.CodeSystem; import org.hl7.fhir.r4.model.DateTimeType; import org.hl7.fhir.r4.model.DecimalType; @@ -18,6 +22,8 @@ import org.hl7.fhir.r4.model.ValueSet; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import org.mockito.ArgumentCaptor; +import org.slf4j.LoggerFactory; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -28,6 +34,8 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public interface IValueSetExpansionIT { static final String CODE_SYSTEM_CODE = "PRODUCT-MULTI-SOURCE"; @@ -205,6 +213,66 @@ default void expandByIdentifier_withBooleanFilteredValuesInInclude_addsMatchingV } } + /** + * We exclude filters we do support (tested in this class), + * as well as NULL (which isn't a "real" filter), + * and REGEX (tested elsewhere). + */ + @ParameterizedTest + @EnumSource( + value = ValueSet.FilterOperator.class, + mode = EnumSource.Mode.EXCLUDE, + names = {"EQUAL", "EXISTS", "IN", "NOTIN", "REGEX", "NULL"}) + default void expandValueSet_withUnsupportedFilters_doesNotThrow(ValueSet.FilterOperator theOperator) { + // setup + IParser parser = getFhirContext().newJsonParser(); + Logger logger = (Logger) LoggerFactory.getLogger(TermReadSvcImpl.class); + ListAppender listAppender = mock(ListAppender.class); + + // setup CodeSystem + // one really isn't necessary for this test, but we'll include it for completeness + CodeSystem codeSystem = parser.parseResource(CodeSystem.class, CODE_SYSTEM_STR_BASE); + + // setup valueset + ValueSet valueSet = parser.parseResource(ValueSet.class, VALUE_SET_STR_BASE); + ValueSet.ConceptSetComponent conceptSetComponent = + valueSet.getCompose().getInclude().get(0); + ValueSet.ConceptSetFilterComponent filterComponent = new ValueSet.ConceptSetFilterComponent(); + filterComponent.setProperty(PROPERTY_NAME); + filterComponent.setOp(theOperator); + filterComponent.setValue("anything"); + conceptSetComponent.setFilter(List.of(filterComponent)); + + // test + boolean preExpand = getJpaStorageSettings().isPreExpandValueSets(); + Level logLevel = logger.getLevel(); + + getJpaStorageSettings().setPreExpandValueSets(true); + logger.setLevel(Level.ERROR); + logger.addAppender(listAppender); + try { + ValueSet expanded = createCodeSystemAndValueSetAndReturnExpandedValueSet(codeSystem, valueSet); + + assertNotNull(expanded); + assertNotNull(expanded.getExpansion()); + assertTrue(expanded.getExpansion().getContains().isEmpty()); + + ArgumentCaptor loggingEventArgumentCaptor = ArgumentCaptor.forClass(ILoggingEvent.class); + verify(listAppender).doAppend(loggingEventArgumentCaptor.capture()); + List errors = loggingEventArgumentCaptor.getAllValues().stream() + .filter(e -> e.getLevel() == Level.ERROR) + .toList(); + assertFalse(errors.isEmpty()); + ILoggingEvent first = errors.get(0); + assertTrue(first.getFormattedMessage().contains("Unsupported property filter")); + assertTrue(first.getFormattedMessage().contains(theOperator.getDisplay())); + } finally { + getJpaStorageSettings().setPreExpandValueSets(preExpand); + logger.setLevel(logLevel); + logger.detachAppender(listAppender); + } + } + @ParameterizedTest @EnumSource( value = ValueSet.FilterOperator.class,