From 78a323f1052dc6f212854704c99965be5ae8e145 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 3 Nov 2019 21:20:22 -0500 Subject: [PATCH 1/2] Test fixes --- .../ResourceProviderDstu3BundleTest.java | 21 ------ .../r4/ResourceProviderR4BundleTest.java | 11 +-- .../extractor/BaseSearchParamExtractor.java | 7 ++ .../extractor/ResourceLinkExtractor.java | 72 +++++++++++-------- 4 files changed, 56 insertions(+), 55 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3BundleTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3BundleTest.java index 290c1cf7fa4..d77bba5e503 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3BundleTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3BundleTest.java @@ -18,27 +18,6 @@ public class ResourceProviderDstu3BundleTest extends BaseResourceProviderDstu3Te private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderDstu3BundleTest.class); - /** - * See #401 - */ - @Test - public void testBundlePreservesFullUrl() { - - Bundle bundle = new Bundle(); - bundle.setType(BundleType.DOCUMENT); - - Composition composition = new Composition(); - composition.setTitle("Visit Summary"); - bundle.addEntry().setFullUrl("http://foo").setResource(composition); - - IIdType id = ourClient.create().resource(bundle).execute().getId(); - - Bundle retBundle = ourClient.read().resource(Bundle.class).withId(id).execute(); - ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(retBundle)); - - assertEquals("http://foo", bundle.getEntry().get(0).getFullUrl()); - } - @Test public void testProcessMessage() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4BundleTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4BundleTest.java index 218e52dfa35..2bbbba76c04 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4BundleTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4BundleTest.java @@ -8,6 +8,7 @@ import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Bundle.BundleType; import org.hl7.fhir.r4.model.Composition; import org.hl7.fhir.r4.model.Parameters; +import org.hl7.fhir.r4.model.Patient; import org.junit.AfterClass; import org.junit.Test; @@ -25,11 +26,11 @@ public class ResourceProviderR4BundleTest extends BaseResourceProviderR4Test { public void testBundlePreservesFullUrl() { Bundle bundle = new Bundle(); - bundle.setType(BundleType.DOCUMENT); + bundle.setType(BundleType.COLLECTION); - Composition composition = new Composition(); - composition.setTitle("Visit Summary"); - bundle.addEntry().setFullUrl("http://foo").setResource(composition); + Patient composition = new Patient(); + composition.setActive(true); + bundle.addEntry().setFullUrl("http://foo/").setResource(composition); IIdType id = ourClient.create().resource(bundle).execute().getId(); @@ -37,7 +38,7 @@ public class ResourceProviderR4BundleTest extends BaseResourceProviderR4Test { ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(retBundle)); - assertEquals("http://foo", bundle.getEntry().get(0).getFullUrl()); + assertEquals("http://foo/", bundle.getEntry().get(0).getFullUrl()); } @Test diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index 370b4ba849c..8180d77e867 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -400,6 +400,13 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor for (String nextPath : nextPathsSplit) { List allValues; + // This path is hard to parse and isn't likely to produce anything useful anyway + if (myContext.getVersion().getVersion().equals(FhirVersionEnum.DSTU2)) { + if (nextPath.equals("Bundle.entry.resource(0)")) { + continue; + } + } + nextPath = trim(nextPath); IValueExtractor allValuesFunc = getPathValueExtractor(theResource, nextPath); try { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java index 2bbccc2228b..0fa013eb64a 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java @@ -39,9 +39,12 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import org.apache.commons.lang3.StringUtils; -import org.hl7.fhir.instance.model.api.*; +import org.hl7.fhir.instance.model.api.IBase; +import org.hl7.fhir.instance.model.api.IBaseExtension; +import org.hl7.fhir.instance.model.api.IBaseReference; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.CanonicalType; -import org.hl7.fhir.r4.model.Reference; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -100,8 +103,8 @@ public class ResourceLinkExtractor { } } - private void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, String theResourceType, RuntimeSearchParam nextSpDef, String theNextPathsUnsplit, boolean theMultiType, PathAndRef nextPathAndRef, boolean theFailOnInvalidReference, RequestDetails theRequest) { - Object nextObject = nextPathAndRef.getRef(); + private void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, String theResourceType, RuntimeSearchParam theRuntimeSearchParam, String thePath, boolean theMultiType, PathAndRef thePathAndRef, boolean theFailOnInvalidReference, RequestDetails theRequest) { + Object nextObject = thePathAndRef.getRef(); /* * A search parameter on an extension field that contains @@ -142,35 +145,40 @@ public class ResourceLinkExtractor { nextId = nextValue.getResource().getIdElement(); } - if (nextId.isEmpty() || nextId.getValue().startsWith("#")) { - // This is a blank or contained resource reference - return; - } } else if (nextObject instanceof IBaseResource) { - nextId = ((IBaseResource) nextObject).getIdElement(); - if (nextId == null || nextId.hasIdPart() == false) { - return; - } - } else if (myContext.getElementDefinition((Class) nextObject.getClass()).getName().equals("uri")) { - return; - } else if (theResourceType.equals("Consent") && nextPathAndRef.getPath().equals("Consent.source")) { - // Consent#source-identifier has a path that isn't typed - This is a one-off to deal with that - return; + nextId = ((IBaseResource) nextObject).getIdElement().toUnqualified(); } else { - if (!theMultiType) { - if (nextSpDef.getName().equals("sourceuri")) { + @SuppressWarnings("unchecked") + Class clazz = (Class) nextObject.getClass(); + if (myContext.getElementDefinition(clazz).getName().equals("uri")) { + return; + } else if (theResourceType.equals("Consent") && thePathAndRef.getPath().equals("Consent.source")) { + // Consent#source-identifier has a path that isn't typed - This is a one-off to deal with that + return; + } else { + if (!theMultiType) { + if (theRuntimeSearchParam.getName().equals("sourceuri")) { + return; + } + throw new ConfigurationException("Search param " + theRuntimeSearchParam.getName() + " is of unexpected datatype: " + nextObject.getClass()); + } else { return; } - throw new ConfigurationException("Search param " + nextSpDef.getName() + " is of unexpected datatype: " + nextObject.getClass()); - } else { - return; } } - theParams.myPopulatedResourceLinkParameters.add(nextSpDef.getName()); + if (nextId == null || + nextId.isEmpty() || + nextId.hasIdPart() == false || + nextId.getValue().startsWith("#") || + nextId.getValue().startsWith("urn:")) { + return; + } + + theParams.myPopulatedResourceLinkParameters.add(theRuntimeSearchParam.getName()); if (LogicalReferenceHelper.isLogicalReference(myModelConfig, nextId)) { - ResourceLink resourceLink = new ResourceLink(nextPathAndRef.getPath(), theEntity, nextId, theUpdateTime); + ResourceLink resourceLink = new ResourceLink(thePathAndRef.getPath(), theEntity, nextId, theUpdateTime); if (theParams.myLinks.add(resourceLink)) { ourLog.debug("Indexing remote resource reference URL: {}", nextId); } @@ -180,7 +188,7 @@ public class ResourceLinkExtractor { String baseUrl = nextId.getBaseUrl(); String typeString = nextId.getResourceType(); if (isBlank(typeString)) { - String msg = "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Does not contain resource type - " + nextId.getValue(); + String msg = "Invalid resource reference found at path[" + thePath + "] - Does not contain resource type - " + nextId.getValue(); if (theFailOnInvalidReference) { throw new InvalidRequestException(msg); } else { @@ -192,7 +200,7 @@ public class ResourceLinkExtractor { try { resourceDefinition = myContext.getResourceDefinition(typeString); } catch (DataFormatException e) { - String msg = "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Resource type is unknown or not supported on this server - " + nextId.getValue(); + String msg = "Invalid resource reference found at path[" + thePath + "] - Resource type is unknown or not supported on this server - " + nextId.getValue(); if (theFailOnInvalidReference) { throw new InvalidRequestException(msg); } else { @@ -201,12 +209,18 @@ public class ResourceLinkExtractor { } } + if (!theRuntimeSearchParam.getTargets().isEmpty()) { + if (!theRuntimeSearchParam.getTargets().contains(typeString)) { + return; + } + } + if (isNotBlank(baseUrl)) { if (!myModelConfig.getTreatBaseUrlsAsLocal().contains(baseUrl) && !myModelConfig.isAllowExternalReferences()) { String msg = myContext.getLocalizer().getMessage(BaseSearchParamExtractor.class, "externalReferenceNotAllowed", nextId.getValue()); throw new InvalidRequestException(msg); } else { - ResourceLink resourceLink = new ResourceLink(nextPathAndRef.getPath(), theEntity, nextId, theUpdateTime); + ResourceLink resourceLink = new ResourceLink(thePathAndRef.getPath(), theEntity, nextId, theUpdateTime); if (theParams.myLinks.add(resourceLink)) { ourLog.debug("Indexing remote resource reference URL: {}", nextId); } @@ -217,7 +231,7 @@ public class ResourceLinkExtractor { Class type = resourceDefinition.getImplementingClass(); String id = nextId.getIdPart(); if (StringUtils.isBlank(id)) { - String msg = "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Does not contain resource ID - " + nextId.getValue(); + String msg = "Invalid resource reference found at path[" + thePath + "] - Does not contain resource ID - " + nextId.getValue(); if (theFailOnInvalidReference) { throw new InvalidRequestException(msg); } else { @@ -227,7 +241,7 @@ public class ResourceLinkExtractor { } theResourceLinkResolver.validateTypeOrThrowException(type); - ResourceLink resourceLink = createResourceLink(theEntity, theUpdateTime, theResourceLinkResolver, nextSpDef, theNextPathsUnsplit, nextPathAndRef, nextId, typeString, type, id, theRequest); + ResourceLink resourceLink = createResourceLink(theEntity, theUpdateTime, theResourceLinkResolver, theRuntimeSearchParam, thePath, thePathAndRef, nextId, typeString, type, id, theRequest); if (resourceLink == null) return; theParams.myLinks.add(resourceLink); } From 9c93d5d7f7e2c8d3b4a9177a12765733b1823a7f Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Mon, 4 Nov 2019 07:04:26 -0500 Subject: [PATCH 2/2] Test fixes --- .../ca/uhn/fhir/context/RuntimeSearchParam.java | 12 ++++++++++-- .../java/ca/uhn/fhir/jpa/dao/SearchBuilder.java | 7 +++---- .../jpa/dao/index/DaoResourceLinkResolver.java | 2 +- .../jpa/dao/dstu2/FhirResourceDaoDstu2Test.java | 14 -------------- .../jpa/dao/dstu3/FhirResourceDaoDstu3Test.java | 13 ------------- .../extractor/ResourceLinkExtractor.java | 15 ++++++++++----- 6 files changed, 24 insertions(+), 39 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeSearchParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeSearchParam.java index 0a87ed758ee..9ce0051fbff 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeSearchParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeSearchParam.java @@ -13,6 +13,8 @@ import org.hl7.fhir.instance.model.api.IIdType; import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; +import javax.annotation.Nonnull; + /* * #%L * HAPI FHIR - Core Library @@ -117,7 +119,7 @@ public class RuntimeSearchParam { if (theTargets != null && theTargets.isEmpty() == false) { myTargets = Collections.unmodifiableSet(theTargets); } else { - myTargets = null; + myTargets = Collections.emptySet(); } if (theBase == null || theBase.isEmpty()) { @@ -138,10 +140,15 @@ public class RuntimeSearchParam { return myBase; } + @Nonnull public Set getTargets() { return myTargets; } + public boolean hasTargets() { + return !myTargets.isEmpty(); + } + public RuntimeSearchParamStatusEnum getStatus() { return myStatus; } @@ -176,7 +183,7 @@ public class RuntimeSearchParam { return Collections.singletonList(path); } - List retVal = new ArrayList(); + List retVal = new ArrayList<>(); StringTokenizer tok = new StringTokenizer(path, "|"); while (tok.hasMoreElements()) { String nextPath = tok.nextToken().trim(); @@ -192,6 +199,7 @@ public class RuntimeSearchParam { return myProvidesMembershipInCompartments; } + public enum RuntimeSearchParamStatusEnum { ACTIVE, DRAFT, 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 1e3f16fcfd6..916e855c181 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 @@ -579,9 +579,8 @@ public class SearchBuilder implements ISearchBuilder { RuntimeSearchParam param = mySearchParamRegistry.getActiveSearchParam(theResourceName, theParamName); resourceTypes = new ArrayList<>(); - Set targetTypes = param.getTargets(); - - if (targetTypes != null && !targetTypes.isEmpty()) { + if (param.hasTargets()) { + Set targetTypes = param.getTargets(); for (String next : targetTypes) { resourceTypes.add(myContext.getResourceDefinition(next).getImplementingClass()); } @@ -2494,7 +2493,7 @@ public class SearchBuilder implements ISearchBuilder { for (String nextPath : paths) { String sql; - boolean haveTargetTypesDefinedByParam = param.getTargets() != null && param.getTargets().isEmpty() == false; + boolean haveTargetTypesDefinedByParam = param.hasTargets(); if (targetResourceType != null) { sql = "SELECT r FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchFieldName + " IN (:target_pids) AND r.myTargetResourceType = :target_resource_type"; } else if (haveTargetTypesDefinedByParam) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java index 6fd2cd320ad..bd440e4a18e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java @@ -100,7 +100,7 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { throw new InvalidRequestException("Resource " + resName + "/" + theId + " is deleted, specified in path: " + theNextPathsUnsplit); } - if (theNextSpDef.getTargets() != null && !theNextSpDef.getTargets().contains(theTypeString)) { + if (!theNextSpDef.hasTargets() && theNextSpDef.getTargets().contains(theTypeString)) { return null; } return target; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java index 315fddc14ac..e529b3bebd3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirResourceDaoDstu2Test.java @@ -484,20 +484,6 @@ public class FhirResourceDaoDstu2Test extends BaseJpaDstu2Test { } - @Test - public void testCreateWithInvalidReferenceNoId() { - Patient p = new Patient(); - p.addName().addFamily("Hello"); - p.getManagingOrganization().setReference("Organization/"); - - try { - myPatientDao.create(p, mySrd); - fail(); - } catch (InvalidRequestException e) { - assertThat(e.getMessage(), containsString("Does not contain resource ID")); - } - } - @Test public void testCreateWithReferenceBadType() { Patient p = new Patient(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java index 7e7bbf8de12..a4e00e8c9c2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java @@ -756,19 +756,6 @@ public class FhirResourceDaoDstu3Test extends BaseJpaDstu3Test { } - @Test - public void testCreateWithInvalidReferenceNoId() { - Patient p = new Patient(); - p.addName().setFamily("Hello"); - p.getManagingOrganization().setReference("Organization/"); - - try { - myPatientDao.create(p, mySrd); - fail(); - } catch (InvalidRequestException e) { - assertThat(e.getMessage(), containsString("Does not contain resource ID")); - } - } @Test public void testCreateWithReferenceBadType() { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java index 0fa013eb64a..cd3600da0f3 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java @@ -146,7 +146,8 @@ public class ResourceLinkExtractor { } } else if (nextObject instanceof IBaseResource) { - nextId = ((IBaseResource) nextObject).getIdElement().toUnqualified(); +// nextId = ((IBaseResource) nextObject).getIdElement().toUnqualified(); + return; } else { @SuppressWarnings("unchecked") Class clazz = (Class) nextObject.getClass(); @@ -169,7 +170,7 @@ public class ResourceLinkExtractor { if (nextId == null || nextId.isEmpty() || - nextId.hasIdPart() == false || +// nextId.hasIdPart() == false || nextId.getValue().startsWith("#") || nextId.getValue().startsWith("urn:")) { return; @@ -209,7 +210,7 @@ public class ResourceLinkExtractor { } } - if (!theRuntimeSearchParam.getTargets().isEmpty()) { + if (theRuntimeSearchParam.hasTargets()) { if (!theRuntimeSearchParam.getTargets().contains(typeString)) { return; } @@ -242,14 +243,18 @@ public class ResourceLinkExtractor { theResourceLinkResolver.validateTypeOrThrowException(type); ResourceLink resourceLink = createResourceLink(theEntity, theUpdateTime, theResourceLinkResolver, theRuntimeSearchParam, thePath, thePathAndRef, nextId, typeString, type, id, theRequest); - if (resourceLink == null) return; + if (resourceLink == null) { + return; + } theParams.myLinks.add(resourceLink); } private ResourceLink createResourceLink(ResourceTable theEntity, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, RuntimeSearchParam nextSpDef, String theNextPathsUnsplit, PathAndRef nextPathAndRef, IIdType theNextId, String theTypeString, Class theType, String theId, RequestDetails theRequest) { ResourceTable targetResource = theResourceLinkResolver.findTargetResource(nextSpDef, theNextPathsUnsplit, theNextId, theTypeString, theType, theId, theRequest); - if (targetResource == null) return null; + if (targetResource == null) { + return null; + } ResourceLink resourceLink = new ResourceLink(nextPathAndRef.getPath(), theEntity, targetResource, theUpdateTime); return resourceLink; }