_has query with non-existent reference field (#4676)
* handle empty values for QueryParameterUtils.toEqualToOrInPredicate and QueryParameterUtils.toNotEqualToOrInPredicate
* fix tests that were improperly mocked
* Revert "handle empty values for QueryParameterUtils.toEqualToOrInPredicate and QueryParameterUtils.toNotEqualToOrInPredicate"
This reverts commit 4382cd31
* throw InvalidRequestException if non-existent reference field is provided for a _has query
This commit is contained in:
parent
47262f5bd5
commit
90da12fbdf
|
@ -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."
|
|
@ -105,6 +105,7 @@ import org.apache.commons.lang3.tuple.Triple;
|
||||||
import org.hl7.fhir.instance.model.api.IAnyResource;
|
import org.hl7.fhir.instance.model.api.IAnyResource;
|
||||||
import org.slf4j.Logger;
|
import org.slf4j.Logger;
|
||||||
import org.slf4j.LoggerFactory;
|
import org.slf4j.LoggerFactory;
|
||||||
|
import org.springframework.util.CollectionUtils;
|
||||||
|
|
||||||
import javax.annotation.Nullable;
|
import javax.annotation.Nullable;
|
||||||
import java.math.BigDecimal;
|
import java.math.BigDecimal;
|
||||||
|
@ -877,6 +878,9 @@ public class QueryStack {
|
||||||
Condition partitionPredicate = join.createPartitionIdPredicate(theRequestPartitionId);
|
Condition partitionPredicate = join.createPartitionIdPredicate(theRequestPartitionId);
|
||||||
|
|
||||||
List<String> paths = join.createResourceLinkPaths(targetResourceType, paramReference, new ArrayList<>());
|
List<String> 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 typePredicate = BinaryCondition.equalTo(join.getColumnTargetResourceType(), mySqlBuilder.generatePlaceholder(theResourceType));
|
||||||
Condition pathPredicate = toEqualToOrInPredicate(join.getColumnSourcePath(), mySqlBuilder.generatePlaceholders(paths));
|
Condition pathPredicate = toEqualToOrInPredicate(join.getColumnSourcePath(), mySqlBuilder.generatePlaceholders(paths));
|
||||||
Condition linkedPredicate = searchForIdsWithAndOr(join.getColumnSrcResourceId(), targetResourceType, parameterName, Collections.singletonList(orValues), theRequest, theRequestPartitionId, SearchContainedModeEnum.FALSE);
|
Condition linkedPredicate = searchForIdsWithAndOr(join.getColumnSrcResourceId(), targetResourceType, parameterName, Collections.singletonList(orValues), theRequest, theRequestPartitionId, SearchContainedModeEnum.FALSE);
|
||||||
|
|
|
@ -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.model.entity.ResourceTable;
|
||||||
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
||||||
import ca.uhn.fhir.jpa.test.config.TestHSearchAddInConfig;
|
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.IBundleProvider;
|
||||||
|
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
|
||||||
import ca.uhn.fhir.rest.param.HasAndListParam;
|
import ca.uhn.fhir.rest.param.HasAndListParam;
|
||||||
import ca.uhn.fhir.rest.param.HasOrListParam;
|
import ca.uhn.fhir.rest.param.HasOrListParam;
|
||||||
import ca.uhn.fhir.rest.param.HasParam;
|
import ca.uhn.fhir.rest.param.HasParam;
|
||||||
import ca.uhn.fhir.rest.param.ReferenceParam;
|
import ca.uhn.fhir.rest.param.ReferenceParam;
|
||||||
import ca.uhn.fhir.rest.param.StringParam;
|
import ca.uhn.fhir.rest.param.StringParam;
|
||||||
import ca.uhn.fhir.rest.param.TokenParam;
|
import ca.uhn.fhir.rest.param.TokenParam;
|
||||||
|
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||||
import org.hamcrest.Matchers;
|
import org.hamcrest.Matchers;
|
||||||
import org.hl7.fhir.r5.model.Bundle;
|
import org.hl7.fhir.r5.model.Bundle;
|
||||||
import org.hl7.fhir.r5.model.ClinicalUseDefinition;
|
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.MatcherAssert.assertThat;
|
||||||
import static org.hamcrest.Matchers.containsInAnyOrder;
|
import static org.hamcrest.Matchers.containsInAnyOrder;
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.fail;
|
||||||
|
|
||||||
@ContextConfiguration(classes = TestHSearchAddInConfig.NoFT.class)
|
@ContextConfiguration(classes = TestHSearchAddInConfig.NoFT.class)
|
||||||
@SuppressWarnings({"Duplicates"})
|
@SuppressWarnings({"Duplicates"})
|
||||||
|
@ -310,5 +314,25 @@ public class FhirResourceDaoR5SearchNoFtTest extends BaseJpaR5Test {
|
||||||
assertEquals(1, outcome.size());
|
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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -7,54 +7,56 @@ import com.healthmarketscience.sqlbuilder.InCondition;
|
||||||
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSchema;
|
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSchema;
|
||||||
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSpec;
|
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbSpec;
|
||||||
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbTable;
|
import com.healthmarketscience.sqlbuilder.dbspec.basic.DbTable;
|
||||||
import org.junit.jupiter.api.Assertions;
|
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
|
import org.mockito.Mock;
|
||||||
import org.mockito.Mockito;
|
import org.mockito.Mockito;
|
||||||
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
|
|
||||||
import java.util.ArrayList;
|
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 {
|
public class ResourceLinkPredicateBuilderTest {
|
||||||
|
|
||||||
|
private static final String PLACEHOLDER_BASE = UUID.randomUUID().toString();
|
||||||
private ResourceLinkPredicateBuilder myResourceLinkPredicateBuilder;
|
private ResourceLinkPredicateBuilder myResourceLinkPredicateBuilder;
|
||||||
|
|
||||||
|
@Mock
|
||||||
|
private SearchQueryBuilder mySearchQueryBuilder;
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
public void init() {
|
public void init() {
|
||||||
DbSpec spec = new DbSpec();
|
DbSpec spec = new DbSpec();
|
||||||
DbSchema schema = new DbSchema(spec, "schema");
|
DbSchema schema = new DbSchema(spec, "schema");
|
||||||
DbTable table = new DbTable(schema, "table");
|
DbTable table = new DbTable(schema, "table");
|
||||||
|
when(mySearchQueryBuilder.addTable(Mockito.anyString())).thenReturn(table);
|
||||||
SearchQueryBuilder sb = Mockito.mock(SearchQueryBuilder.class);
|
myResourceLinkPredicateBuilder = new ResourceLinkPredicateBuilder(null, mySearchQueryBuilder, false);
|
||||||
Mockito.when(sb.addTable(Mockito.anyString()))
|
|
||||||
.thenReturn(table);
|
|
||||||
myResourceLinkPredicateBuilder = new ResourceLinkPredicateBuilder(
|
|
||||||
null,
|
|
||||||
sb,
|
|
||||||
false
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void createEverythingPredicate_withListOfPids_returnsInPredicate() {
|
public void createEverythingPredicate_withListOfPids_returnsInPredicate() {
|
||||||
Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(),
|
when(myResourceLinkPredicateBuilder.generatePlaceholders(anyCollection())).thenReturn(List.of(PLACEHOLDER_BASE+"1", PLACEHOLDER_BASE+"2"));
|
||||||
1l, 2l);
|
Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), 1l, 2l);
|
||||||
|
assertEquals(InCondition.class, condition.getClass());
|
||||||
Assertions.assertTrue(condition instanceof InCondition);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void createEverythingPredicate_withSinglePid_returnsInCondition() {
|
public void createEverythingPredicate_withSinglePid_returnsInCondition() {
|
||||||
Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(),
|
when(myResourceLinkPredicateBuilder.generatePlaceholders(anyCollection())).thenReturn(List.of(PLACEHOLDER_BASE+"1"));
|
||||||
1l);
|
Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), 1l);
|
||||||
|
assertEquals(BinaryCondition.class, condition.getClass());
|
||||||
Assertions.assertTrue(condition instanceof InCondition);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void createEverythingPredicate_withNoPids_returnsBinaryCondition() {
|
public void createEverythingPredicate_withNoPids_returnsBinaryCondition() {
|
||||||
Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(),
|
Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", new ArrayList<>(), new Long[0]);
|
||||||
new Long[0]);
|
assertEquals(BinaryCondition.class, condition.getClass());
|
||||||
|
|
||||||
Assertions.assertTrue(condition instanceof BinaryCondition);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue