diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4675-fhir-search-with-non-existant-searchparameters.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4675-fhir-search-with-non-existant-searchparameters.yaml new file mode 100644 index 00000000000..82a62c1f85f --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4675-fhir-search-with-non-existant-searchparameters.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 4675 +title: "Previously, when a `_has` query was performed with a non-existent reference field, the query succeed with +empty results in H2 and failed with a 500 Server Error in Postgres. Now all database vendors will throw an +`InvalidRequestException` if a non-existent reference field is provided for a `_has` query." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java index 85fc03e37c7..443ac58db47 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java @@ -105,6 +105,7 @@ import org.apache.commons.lang3.tuple.Triple; import org.hl7.fhir.instance.model.api.IAnyResource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.util.CollectionUtils; import javax.annotation.Nullable; import java.math.BigDecimal; @@ -877,6 +878,9 @@ public class QueryStack { Condition partitionPredicate = join.createPartitionIdPredicate(theRequestPartitionId); List paths = join.createResourceLinkPaths(targetResourceType, paramReference, new ArrayList<>()); + if (CollectionUtils.isEmpty(paths)) { + throw new InvalidRequestException(Msg.code(2305) + "Reference field does not exist: " + paramReference); + } Condition typePredicate = BinaryCondition.equalTo(join.getColumnTargetResourceType(), mySqlBuilder.generatePlaceholder(theResourceType)); Condition pathPredicate = toEqualToOrInPredicate(join.getColumnSourcePath(), mySqlBuilder.generatePlaceholders(paths)); Condition linkedPredicate = searchForIdsWithAndOr(join.getColumnSrcResourceId(), targetResourceType, parameterName, Collections.singletonList(orValues), theRequest, theRequestPartitionId, SearchContainedModeEnum.FALSE); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/QueryParameterUtilsTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/QueryParameterUtilsTest.java new file mode 100644 index 00000000000..e69de29bb2d diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5SearchNoFtTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5SearchNoFtTest.java index 7f26b55b81f..6869faf12f5 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5SearchNoFtTest.java @@ -4,13 +4,16 @@ import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.test.config.TestHSearchAddInConfig; +import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.param.HasAndListParam; import ca.uhn.fhir.rest.param.HasOrListParam; import ca.uhn.fhir.rest.param.HasParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import org.hamcrest.Matchers; import org.hl7.fhir.r5.model.Bundle; import org.hl7.fhir.r5.model.ClinicalUseDefinition; @@ -39,6 +42,7 @@ import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; @ContextConfiguration(classes = TestHSearchAddInConfig.NoFT.class) @SuppressWarnings({"Duplicates"}) @@ -310,5 +314,25 @@ public class FhirResourceDaoR5SearchNoFtTest extends BaseJpaR5Test { assertEquals(1, outcome.size()); } + @Test + public void testHasWithNonExistentReferenceField() { + String targetResource = "Encounter"; + String referenceFieldName = "non_existent_reference"; + String parameterValue = "123"; + HasParam hasParam = new HasParam(targetResource, referenceFieldName, Constants.PARAM_ID, parameterValue); + + HasAndListParam hasAnd = new HasAndListParam(); + hasAnd.addValue(new HasOrListParam().add(hasParam)); + SearchParameterMap params = SearchParameterMap.newSynchronous(); + params.add(Constants.PARAM_HAS, hasAnd); + + try { + myObservationDao.search(params, new SystemRequestDetails()); + fail(); + } catch (InvalidRequestException e) { + assertEquals("HAPI-2305: Reference field does not exist: " + referenceFieldName, e.getMessage()); + } + } + } diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java index f654cf76c66..36766739cab 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java @@ -7,54 +7,56 @@ import com.healthmarketscience.sqlbuilder.InCondition; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSchema; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSpec; import com.healthmarketscience.sqlbuilder.dbspec.basic.DbTable; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.anyCollection; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) public class ResourceLinkPredicateBuilderTest { + private static final String PLACEHOLDER_BASE = UUID.randomUUID().toString(); private ResourceLinkPredicateBuilder myResourceLinkPredicateBuilder; + @Mock + private SearchQueryBuilder mySearchQueryBuilder; + @BeforeEach public void init() { DbSpec spec = new DbSpec(); DbSchema schema = new DbSchema(spec, "schema"); DbTable table = new DbTable(schema, "table"); - - SearchQueryBuilder sb = Mockito.mock(SearchQueryBuilder.class); - Mockito.when(sb.addTable(Mockito.anyString())) - .thenReturn(table); - myResourceLinkPredicateBuilder = new ResourceLinkPredicateBuilder( - null, - sb, - false - ); + when(mySearchQueryBuilder.addTable(Mockito.anyString())).thenReturn(table); + myResourceLinkPredicateBuilder = new ResourceLinkPredicateBuilder(null, mySearchQueryBuilder, false); } @Test public void createEverythingPredicate_withListOfPids_returnsInPredicate() { - Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), - 1l, 2l); - - Assertions.assertTrue(condition instanceof InCondition); + when(myResourceLinkPredicateBuilder.generatePlaceholders(anyCollection())).thenReturn(List.of(PLACEHOLDER_BASE+"1", PLACEHOLDER_BASE+"2")); + Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), 1l, 2l); + assertEquals(InCondition.class, condition.getClass()); } @Test public void createEverythingPredicate_withSinglePid_returnsInCondition() { - Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), - 1l); - - Assertions.assertTrue(condition instanceof InCondition); + when(myResourceLinkPredicateBuilder.generatePlaceholders(anyCollection())).thenReturn(List.of(PLACEHOLDER_BASE+"1")); + Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), 1l); + assertEquals(BinaryCondition.class, condition.getClass()); } @Test public void createEverythingPredicate_withNoPids_returnsBinaryCondition() { - Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), - new Long[0]); - - Assertions.assertTrue(condition instanceof BinaryCondition); + Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), new Long[0]); + assertEquals(BinaryCondition.class, condition.getClass()); } }