From c484c6966480680bdf0038fe3a22d86a1cf88baa Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 3 Dec 2018 11:36:09 -0500 Subject: [PATCH] Better error message for unqualified search parameter types --- .../RuntimeChildResourceDefinition.java | 3 +- .../rest/gclient/ReferenceClientParam.java | 52 +++++++-- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 17 ++- .../dstu3/ResourceProviderDstu3Test.java | 101 +++++++++++++----- src/changes/changes.xml | 10 ++ 5 files changed, 144 insertions(+), 39 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildResourceDefinition.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildResourceDefinition.java index b25dcbb8cdb..7db7c93400c 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildResourceDefinition.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildResourceDefinition.java @@ -49,9 +49,8 @@ public class RuntimeChildResourceDefinition extends BaseRuntimeDeclaredChildDefi myResourceTypes = theResourceTypes; if (theResourceTypes == null || theResourceTypes.isEmpty()) { - myResourceTypes = new ArrayList>(); + myResourceTypes = new ArrayList<>(); myResourceTypes.add(IBaseResource.class); -// throw new ConfigurationException("Field '" + theField.getName() + "' on type '" + theField.getDeclaringClass().getCanonicalName() + "' has no resource types noted"); } } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java index 405261690b6..0b59ed2fc72 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java @@ -1,9 +1,11 @@ package ca.uhn.fhir.rest.gclient; +import ca.uhn.fhir.context.FhirContext; +import org.hl7.fhir.instance.model.api.IIdType; + import java.util.Collection; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.model.primitive.IdDt; +import static org.apache.commons.lang3.StringUtils.isNotBlank; /* * #%L @@ -38,17 +40,43 @@ public class ReferenceClientParam extends BaseClientParam implements IParam { public String getParamName() { return myName; } - + + /** + * Include a chained search. For example: + * 
+	 * Bundle resp = ourClient
+	 *   .search()
+	 *   .forResource(QuestionnaireResponse.class)
+	 *   .where(QuestionnaireResponse.SUBJECT.hasChainedProperty(Patient.FAMILY.matches().value("SMITH")))
+	 *   .returnBundle(Bundle.class)
+	 *   .execute();
+	 *
+ */ public ICriterion hasChainedProperty(ICriterion theCriterion) { return new ReferenceChainCriterion(getParamName(), theCriterion); } + /** + * Include a chained search with a resource type. For example: + * 
+	 * Bundle resp = ourClient
+	 *   .search()
+	 *   .forResource(QuestionnaireResponse.class)
+	 *   .where(QuestionnaireResponse.SUBJECT.hasChainedProperty("Patient", Patient.FAMILY.matches().value("SMITH")))
+	 *   .returnBundle(Bundle.class)
+	 *   .execute();
+	 *
+ */ + public ICriterion hasChainedProperty(String theResourceType, ICriterion theCriterion) { + return new ReferenceChainCriterion(getParamName(), theResourceType, theCriterion); + } + /** * Match the referenced resource if the resource has the given ID (this can be * the logical ID or the absolute URL of the resource) */ - public ICriterion hasId(IdDt theId) { - return new StringCriterion(getParamName(), theId.getValue()); + public ICriterion hasId(IIdType theId) { + return new StringCriterion<>(getParamName(), theId.getValue()); } /** @@ -56,7 +84,7 @@ public class ReferenceClientParam extends BaseClientParam implements IParam { * the logical ID or the absolute URL of the resource) */ public ICriterion hasId(String theId) { - return new StringCriterion(getParamName(), theId); + return new StringCriterion<>(getParamName(), theId); } /** @@ -67,22 +95,28 @@ public class ReferenceClientParam extends BaseClientParam implements IParam { * with the same parameter. */ public ICriterion hasAnyOfIds(Collection theIds) { - return new StringCriterion(getParamName(), theIds); + return new StringCriterion<>(getParamName(), theIds); } private static class ReferenceChainCriterion implements ICriterion, ICriterionInternal { + private final String myResourceTypeQualifier; private String myParamName; private ICriterionInternal myWrappedCriterion; - public ReferenceChainCriterion(String theParamName, ICriterion theWrappedCriterion) { + ReferenceChainCriterion(String theParamName, ICriterion theWrappedCriterion) { + this(theParamName, null, theWrappedCriterion); + } + + ReferenceChainCriterion(String theParamName, String theResourceType, ICriterion theWrappedCriterion) { myParamName = theParamName; + myResourceTypeQualifier = isNotBlank(theResourceType) ? ":" + theResourceType : ""; myWrappedCriterion = (ICriterionInternal) theWrappedCriterion; } @Override public String getParameterName() { - return myParamName + "." + myWrappedCriterion.getParameterName(); + return myParamName + myResourceTypeQualifier + "." + myWrappedCriterion.getParameterName(); } @Override diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index d88dfeafa8e..0aab8ba3a54 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -462,6 +462,11 @@ public class SearchBuilder implements ISearchBuilder { } else if (def instanceof RuntimeChildResourceDefinition) { RuntimeChildResourceDefinition resDef = (RuntimeChildResourceDefinition) def; resourceTypes.addAll(resDef.getResourceTypes()); + if (resourceTypes.size() == 1) { + if (resourceTypes.get(0).isInterface()) { + throw new InvalidRequestException("Unable to perform search for unqualified chain '" + theParamName + "' as this SearchParameter does not declare any target types. Add a qualifier of the form '" + theParamName + ":[ResourceType]' to perform this search."); + } + } } else { throw new ConfigurationException("Property " + paramPath + " of type " + myResourceName + " is not a resource: " + def.getClass()); } @@ -479,10 +484,14 @@ public class SearchBuilder implements ISearchBuilder { resourceId = ref.getValue(); } else { - RuntimeResourceDefinition resDef = myContext.getResourceDefinition(ref.getResourceType()); - resourceTypes = new ArrayList<>(1); - resourceTypes.add(resDef.getImplementingClass()); - resourceId = ref.getIdPart(); + try { + RuntimeResourceDefinition resDef = myContext.getResourceDefinition(ref.getResourceType()); + resourceTypes = new ArrayList<>(1); + resourceTypes.add(resDef.getImplementingClass()); + resourceId = ref.getIdPart(); + } catch (DataFormatException e) { + throw new InvalidRequestException("Invalid resource type: " + ref.getResourceType()); + } } boolean foundChainMatch = false; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 142a4963378..bf963b4e79e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -75,11 +75,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderDstu3Test.class); private SearchCoordinatorSvcImpl mySearchCoordinatorSvcRaw; - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } - @Override @After public void after() throws Exception { @@ -240,6 +235,59 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } } + @Test + public void testSearchChainedReference() { + + Patient p = new Patient(); + p.addName().setFamily("SMITH"); + IIdType pid = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + QuestionnaireResponse qr = new QuestionnaireResponse(); + qr.getSubject().setReference(pid.getValue()); + ourClient.create().resource(qr).execute(); + + Subscription subs = new Subscription(); + subs.setStatus(SubscriptionStatus.ACTIVE); + subs.getChannel().setType(SubscriptionChannelType.WEBSOCKET); + subs.setCriteria("Observation?"); + IIdType id = ourClient.create().resource(subs).execute().getId().toUnqualifiedVersionless(); + + // Unqualified (doesn't work because QuestionnaireRespone.subject is a Refercence(Any)) + try { + ourClient + .search() + .forResource(QuestionnaireResponse.class) + .where(QuestionnaireResponse.SUBJECT.hasChainedProperty(Patient.FAMILY.matches().value("SMITH"))) + .returnBundle(Bundle.class) + .execute(); + fail(); + } catch (InvalidRequestException e) { + assertEquals("HTTP 400 Bad Request: Unable to perform search for unqualified chain 'subject' as this SearchParameter does not declare any target types. Add a qualifier of the form 'subject:[ResourceType]' to perform this search.", e.getMessage()); + } + + // Qualified + Bundle resp = ourClient + .search() + .forResource(QuestionnaireResponse.class) + .where(QuestionnaireResponse.SUBJECT.hasChainedProperty("Patient", Patient.FAMILY.matches().value("SMITH"))) + .returnBundle(Bundle.class) + .execute(); + assertEquals(1, resp.getEntry().size()); + + // Qualified With an invalid name + try { + ourClient + .search() + .forResource(QuestionnaireResponse.class) + .where(QuestionnaireResponse.SUBJECT.hasChainedProperty("FOO", Patient.FAMILY.matches().value("SMITH"))) + .returnBundle(Bundle.class) + .execute(); + } catch (InvalidRequestException e) { + assertEquals("HTTP 400 Bad Request: Invalid resource type: FOO", e.getMessage()); + } + + } + @Test public void testCodeSearch() { Subscription subs = new Subscription(); @@ -1588,6 +1636,25 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { assertEquals(77, ids.size()); } + @Test + public void testEverythingWithOnlyPatient() { + Patient p = new Patient(); + p.setActive(true); + IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + myFhirCtx.getRestfulClientFactory().setSocketTimeout(300 * 1000); + + Bundle response = ourClient + .operation() + .onInstance(id) + .named("everything") + .withNoParameters(Parameters.class) + .returnResourceType(Bundle.class) + .execute(); + + assertEquals(1, response.getEntry().size()); + } + // private void delete(String theResourceType, String theParamName, String theParamValue) { // Bundle resources; // do { @@ -1613,25 +1680,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { // } // } - @Test - public void testEverythingWithOnlyPatient() { - Patient p = new Patient(); - p.setActive(true); - IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); - - myFhirCtx.getRestfulClientFactory().setSocketTimeout(300 * 1000); - - Bundle response = ourClient - .operation() - .onInstance(id) - .named("everything") - .withNoParameters(Parameters.class) - .returnResourceType(Bundle.class) - .execute(); - - assertEquals(1, response.getEntry().size()); - } - @SuppressWarnings("unused") @Test public void testFullTextSearch() throws Exception { @@ -4293,4 +4341,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { return new InstantDt(theDate).getValueAsString(); } + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a9df3c26181..c481462712b 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -102,6 +102,16 @@ Oracle or SQL Server. In addition, when using the "Dry Run" option, all generated SQL statements will be logged at the end of the run. + + In the JPA server, when performing a chained reference search on a search parameter with + a target type of + Reference(Any)]]>, the search failed with an incomprehensible + error. This has been corrected to return an error message indicating that the chain + must be qualified with a resource type for such a field. For example, + QuestionnaireResponse?subject:Patient.name=smith]]> + instead of + QuestionnaireResponse?subject.name=smith]]>. +