From a474e88ed4e6f40aa51dd25de3a555d128514a9c Mon Sep 17 00:00:00 2001 From: TipzCM Date: Thu, 7 Oct 2021 10:46:23 -0400 Subject: [PATCH] issue-2397 implementing the mdm expansion on everything and searches (#3044) * issue-2397 implementing the mdm expansion on everything and searches * issue-2397 code cleanup and removal of test code * issue-2397 some tests * issue-2397 changelog added * issue-2397 review fixes * issue-2397 updated change log with example and fixed failing tests * issue-2397 test fixing * issue-2397 fixing test * issue-2397 review fix * issue 2397 fixing changelog file Co-authored-by: leif stawnyczy Co-authored-by: Tadgh --- .../ca/uhn/fhir/rest/param/StringParam.java | 11 ++ .../ca/uhn/fhir/rest/param/TokenParam.java | 11 ++ .../5_6_0/2397-mdm-support-on-everything.yaml | 5 + .../export/svc/BulkDataExportSvcImpl.java | 4 - .../fhir/jpa/dao/index/IdHelperService.java | 120 +++++++++----- .../fhir/jpa/dao/mdm/MdmLinkExpandSvc.java | 1 + .../jpa/dao/r4/FhirResourceDaoPatientR4.java | 27 +++- .../MdmSearchExpandingInterceptor.java | 91 ++++++++++- .../jpa/search/SearchCoordinatorSvcImpl.java | 1 - .../fhir/jpa/search/builder/QueryStack.java | 5 +- .../jpa/search/builder/SearchBuilder.java | 56 +++++-- .../ResourceLinkPredicateBuilder.java | 11 +- .../ca/uhn/fhir/jpa/config/TestJPAConfig.java | 5 +- .../jpa/dao/index/IdHelperServiceTest.java | 136 ++++++++++++++++ .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 3 - .../ResourceLinkPredicateBuilderTest.java | 58 +++++++ .../MdmSearchExpandingInterceptorIT.java | 148 +++++++++++++++--- .../ResourceIndexedSearchParamsTest.java | 1 - 18 files changed, 601 insertions(+), 93 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2397-mdm-support-on-everything.yaml create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java index 0d583b4e3d8..1653d10a21e 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/StringParam.java @@ -38,6 +38,8 @@ public class StringParam extends BaseParam implements IQueryParameterType { private boolean myExact; private String myValue; + private Boolean myMdmExpand; + /** * Constructor */ @@ -75,6 +77,15 @@ public class StringParam extends BaseParam implements IQueryParameterType { return ParameterUtil.escape(myValue); } + public boolean isMdmExpand() { + return myMdmExpand != null && myMdmExpand; + } + + public StringParam setMdmExpand(boolean theMdmExpand) { + myMdmExpand = theMdmExpand; + return this; + } + @Override public int hashCode() { return new HashCodeBuilder(17, 37) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParam.java index 9549b678e48..55b60364d13 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/TokenParam.java @@ -40,6 +40,8 @@ public class TokenParam extends BaseParam /*implements IQueryParameterType*/ { private String mySystem; private String myValue; + private Boolean myMdmExpand; + /** * Constructor */ @@ -99,6 +101,15 @@ public class TokenParam extends BaseParam /*implements IQueryParameterType*/ { this(null, theCode); } + public boolean isMdmExpand() { + return myMdmExpand != null && myMdmExpand; + } + + public TokenParam setMdmExpand(boolean theMdmExpand) { + myMdmExpand = theMdmExpand; + return this; + } + @Override String doGetQueryParameterQualifier() { if (getModifier() != null) { diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2397-mdm-support-on-everything.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2397-mdm-support-on-everything.yaml new file mode 100644 index 00000000000..2564a16f179 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2397-mdm-support-on-everything.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 2397 +title: "Added mdm support on $everything operation for patients by adding _mdm=true query parameter. + Eg: /Patient/123/$everything?_mdm=true." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/export/svc/BulkDataExportSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/export/svc/BulkDataExportSvcImpl.java index 2caa72135bc..16e176c1457 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/export/svc/BulkDataExportSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/export/svc/BulkDataExportSvcImpl.java @@ -311,7 +311,6 @@ public class BulkDataExportSvcImpl implements IBulkDataExportSvc { @Transactional @Override public JobInfo submitJob(BulkDataExportOptions theBulkDataExportOptions, Boolean useCache, RequestDetails theRequestDetails) { - String outputFormat = Constants.CT_FHIR_NDJSON; if (isNotBlank(theBulkDataExportOptions.getOutputFormat())) { outputFormat = theBulkDataExportOptions.getOutputFormat(); @@ -359,11 +358,8 @@ public class BulkDataExportSvcImpl implements IBulkDataExportSvc { requestBuilder.append("&").append(JpaConstants.PARAM_EXPORT_MDM).append("=").append(theBulkDataExportOptions.isExpandMdm()); } - - String request = requestBuilder.toString(); - //If we are using the cache, then attempt to retrieve a matching job based on the Request String, otherwise just make a new one. if (useCache) { Date cutoff = DateUtils.addMilliseconds(new Date(), -myReuseBulkExportForMillis); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java index 1f9908c458c..9020827b3fe 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java @@ -122,15 +122,16 @@ public class IdHelperService { */ @Nonnull public IResourceLookup resolveResourceIdentity(@Nonnull RequestPartitionId theRequestPartitionId, String theResourceType, String theResourceId) throws ResourceNotFoundException { - // We only pass 1 input in so only 0..1 will come back IdDt id = new IdDt(theResourceType, theResourceId); - Collection matches = translateForcedIdToPids(theRequestPartitionId, Collections.singletonList(id)); + Map> matches = translateForcedIdToPids(theRequestPartitionId, + Collections.singletonList(id)); - if (matches.isEmpty()) { + // We only pass 1 input in so only 0..1 will come back + if (matches.isEmpty() || !matches.containsKey(theResourceId)) { throw new ResourceNotFoundException(id); } - if (matches.size() > 1) { + if (matches.size() > 1 || matches.get(theResourceId).size() > 1) { /* * This means that: * 1. There are two resources with the exact same resource type and forced id @@ -140,7 +141,60 @@ public class IdHelperService { throw new PreconditionFailedException(msg); } - return matches.iterator().next(); + return matches.get(theResourceId).get(0); + } + + /** + * Returns a mapping of Id -> ResourcePersistentId. + * If any resource is not found, it will throw ResourceNotFound exception + * (and no map will be returned) + * + * @param theRequestPartitionId + * @param theResourceType + * @param theIds + * @return + */ + @Nonnull + public Map resolveResourcePersistentIds(@Nonnull RequestPartitionId theRequestPartitionId, + String theResourceType, + List theIds) { + Validate.notNull(theIds, "theIds cannot be null"); + Validate.isTrue(!theIds.isEmpty(), "theIds must not be empty"); + + Map retVals = new HashMap<>(); + + for (String id : theIds) { + ResourcePersistentId retVal; + if (!idRequiresForcedId(id)) { + // is already a PID + retVal = new ResourcePersistentId(Long.parseLong(id)); + retVals.put(id, retVal); + } + else { + // is a forced id + // we must resolve! + if (myDaoConfig.isDeleteEnabled()) { + retVal = new ResourcePersistentId(resolveResourceIdentity(theRequestPartitionId, theResourceType, id).getResourceId()); + retVals.put(id, retVal); + } + else { + // fetch from cache... adding to cache if not available + String key = toForcedIdToPidKey(theRequestPartitionId, theResourceType, id); + retVal = myMemoryCacheService.getThenPutAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, t -> { + List ids = Collections.singletonList(new IdType(theResourceType, id)); + // fetches from cache using a function that checks cache first... + List resolvedIds = resolveResourcePersistentIdsWithCache(theRequestPartitionId, ids); + if (resolvedIds.isEmpty()) { + throw new ResourceNotFoundException(ids.get(0)); + } + return resolvedIds.get(0); + }); + retVals.put(id, retVal); + } + } + } + + return retVals; } /** @@ -152,27 +206,10 @@ public class IdHelperService { public ResourcePersistentId resolveResourcePersistentIds(@Nonnull RequestPartitionId theRequestPartitionId, String theResourceType, String theId) { Validate.notNull(theId, "theId must not be null"); - ResourcePersistentId retVal; - if (idRequiresForcedId (theId)) { - if (myDaoConfig.isDeleteEnabled()) { - retVal = new ResourcePersistentId(resolveResourceIdentity(theRequestPartitionId, theResourceType, theId).getResourceId()); - } else { - String key = toForcedIdToPidKey(theRequestPartitionId, theResourceType, theId); - retVal = myMemoryCacheService.getThenPutAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, t -> { - List ids = Collections.singletonList(new IdType(theResourceType, theId)); - List resolvedIds = resolveResourcePersistentIdsWithCache(theRequestPartitionId, ids); - if (resolvedIds.isEmpty()) { - throw new ResourceNotFoundException(ids.get(0)); - } - return resolvedIds.get(0); - }); - } - - } else { - retVal = new ResourcePersistentId(Long.parseLong(theId)); - } - - return retVal; + Map retVal = resolveResourcePersistentIds(theRequestPartitionId, + theResourceType, + Collections.singletonList(theId)); + return retVal.get(theId); // should be only one } /** @@ -332,14 +369,14 @@ public class IdHelperService { return typeToIds; } - private Collection translateForcedIdToPids(@Nonnull RequestPartitionId theRequestPartitionId, Collection theId) { + private Map> translateForcedIdToPids(@Nonnull RequestPartitionId theRequestPartitionId, Collection theId) { theId.forEach(id -> Validate.isTrue(id.hasIdPart())); if (theId.isEmpty()) { - return Collections.emptyList(); + return new HashMap<>(); } - List retVal = new ArrayList<>(); + Map> retVal = new HashMap<>(); RequestPartitionId requestPartitionId = replaceDefault(theRequestPartitionId); if (myDaoConfig.getResourceClientIdStrategy() != DaoConfig.ClientIdStrategyEnum.ANY) { @@ -353,6 +390,7 @@ public class IdHelperService { } } + // returns a map of resourcetype->id ListMultimap typeToIds = organizeIdsByResourceType(theId); for (Map.Entry> nextEntry : typeToIds.asMap().entrySet()) { String nextResourceType = nextEntry.getKey(); @@ -365,7 +403,10 @@ public class IdHelperService { IResourceLookup cachedLookup = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.RESOURCE_LOOKUP, nextKey); if (cachedLookup != null) { forcedIdIterator.remove(); - retVal.add(cachedLookup); + if (!retVal.containsKey(nextForcedId)) { + retVal.put(nextForcedId, new ArrayList<>()); + } + retVal.get(nextForcedId).add(cachedLookup); } } } @@ -392,7 +433,10 @@ public class IdHelperService { String forcedId = (String) next[2]; Date deletedAt = (Date) next[3]; ResourceLookup lookup = new ResourceLookup(resourceType, resourcePid, deletedAt); - retVal.add(lookup); + if (!retVal.containsKey(forcedId)) { + retVal.put(forcedId, new ArrayList<>()); + } + retVal.get(forcedId).add(lookup); if (!myDaoConfig.isDeleteEnabled()) { String key = resourceType + "/" + forcedId; @@ -420,8 +464,7 @@ public class IdHelperService { return theRequestPartitionId; } - private void resolvePids(@Nonnull RequestPartitionId theRequestPartitionId, List thePidsToResolve, List theTarget) { - + private void resolvePids(@Nonnull RequestPartitionId theRequestPartitionId, List thePidsToResolve, Map> theTargets) { if (!myDaoConfig.isDeleteEnabled()) { for (Iterator forcedIdIterator = thePidsToResolve.iterator(); forcedIdIterator.hasNext(); ) { Long nextPid = forcedIdIterator.next(); @@ -429,7 +472,10 @@ public class IdHelperService { IResourceLookup cachedLookup = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.RESOURCE_LOOKUP, nextKey); if (cachedLookup != null) { forcedIdIterator.remove(); - theTarget.add(cachedLookup); + if (!theTargets.containsKey(nextKey)) { + theTargets.put(nextKey, new ArrayList<>()); + } + theTargets.get(nextKey).add(cachedLookup); } } } @@ -451,7 +497,11 @@ public class IdHelperService { .stream() .map(t -> new ResourceLookup((String) t[0], (Long) t[1], (Date) t[2])) .forEach(t -> { - theTarget.add(t); + String id = t.getResourceId().toString(); + if (!theTargets.containsKey(id)) { + theTargets.put(id, new ArrayList<>()); + } + theTargets.get(id).add(t); if (!myDaoConfig.isDeleteEnabled()) { String nextKey = Long.toString(t.getResourceId()); myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.RESOURCE_LOOKUP, nextKey, t); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkExpandSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkExpandSvc.java index 145b6be175f..c3febeb6d1a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkExpandSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkExpandSvc.java @@ -112,6 +112,7 @@ public class MdmLinkExpandSvc { List goldenPidSourcePidTuples = myMdmLinkDao.expandPidsByGoldenResourcePidAndMatchResult(theGoldenResourcePid, MdmMatchResultEnum.MATCH); return flattenPidTuplesToSet(theGoldenResourcePid, goldenPidSourcePidTuples); } + public Set expandMdmByGoldenResourceId(IdDt theId) { ourLog.debug("About to expand golden resource with golden resource id {}", theId); Long pidOrThrowException = myIdHelperService.getPidOrThrowException(theId); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoPatientR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoPatientR4.java index ad860b3de0c..a69da66af9f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoPatientR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoPatientR4.java @@ -41,6 +41,7 @@ import org.hl7.fhir.r4.model.Patient; import org.springframework.beans.factory.annotation.Autowired; import javax.servlet.http.HttpServletRequest; +import java.util.Arrays; import java.util.Collections; public class FhirResourceDaoPatientR4 extends BaseHapiFhirResourceDaoimplements IFhirResourceDaoPatient { @@ -48,7 +49,15 @@ public class FhirResourceDaoPatientR4 extends BaseHapiFhirResourceDaoim @Autowired private IRequestPartitionHelperSvc myPartitionHelperSvc; - private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType theCount, IPrimitiveType theOffset, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative, StringAndListParam theFilter, RequestDetails theRequest) { + private IBundleProvider doEverythingOperation(IIdType theId, + IPrimitiveType theCount, + IPrimitiveType theOffset, + DateRangeParam theLastUpdated, + SortSpec theSort, + StringAndListParam theContent, + StringAndListParam theNarrative, + StringAndListParam theFilter, + RequestDetails theRequest) { SearchParameterMap paramMap = new SearchParameterMap(); if (theCount != null) { paramMap.setCount(theCount.getValue()); @@ -67,7 +76,14 @@ public class FhirResourceDaoPatientR4 extends BaseHapiFhirResourceDaoim paramMap.setSort(theSort); paramMap.setLastUpdated(theLastUpdated); if (theId != null) { - paramMap.add("_id", new StringParam(theId.getIdPart())); + StringParam idParam = new StringParam(theId.getIdPart()); + if (theRequest.getParameters().containsKey("_mdm")) { + String[] paramVal = theRequest.getParameters().get("_mdm"); + if (Arrays.asList(paramVal).contains("true")) { + idParam.setMdmExpand(true); + } + } + paramMap.add("_id", idParam); } if (!isPagingProviderDatabaseBacked(theRequest)) { @@ -75,7 +91,12 @@ public class FhirResourceDaoPatientR4 extends BaseHapiFhirResourceDaoim } RequestPartitionId requestPartitionId = myPartitionHelperSvc.determineReadPartitionForRequestForSearchType(theRequest, getResourceName(), paramMap, null); - return mySearchCoordinatorSvc.registerSearch(this, paramMap, getResourceName(), new CacheControlDirective().parse(theRequest.getHeaders(Constants.HEADER_CACHE_CONTROL)), theRequest, requestPartitionId); + return mySearchCoordinatorSvc.registerSearch(this, + paramMap, + getResourceName(), + new CacheControlDirective().parse(theRequest.getHeaders(Constants.HEADER_CACHE_CONTROL)), + theRequest, + requestPartitionId); } @Override diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/MdmSearchExpandingInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/MdmSearchExpandingInterceptor.java index 5eb66b5aab2..caca45b167c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/MdmSearchExpandingInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/MdmSearchExpandingInterceptor.java @@ -30,11 +30,16 @@ import ca.uhn.fhir.mdm.log.Logs; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.param.ReferenceParam; +import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.param.TokenParam; +import org.hl7.fhir.instance.model.api.IIdType; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; +import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -43,6 +48,11 @@ import java.util.Set; */ @Interceptor public class MdmSearchExpandingInterceptor { + // A simple interface to turn ids into some form of IQueryParameterTypes + private interface Creator { + T create(String id); + } + private static final Logger ourLog = Logs.getMdmTroubleshootingLog(); @Autowired @@ -54,9 +64,13 @@ public class MdmSearchExpandingInterceptor { @Hook(Pointcut.STORAGE_PRESEARCH_REGISTERED) public void hook(SearchParameterMap theSearchParameterMap) { if (myDaoConfig.isAllowMdmExpansion()) { - for (List> andList : theSearchParameterMap.values()) { + for (Map.Entry>> set : theSearchParameterMap.entrySet()) { + String paramName = set.getKey(); + List> andList = set.getValue(); for (List orList : andList) { - expandAnyReferenceParameters(orList); + // here we will know if it's an _id param or not + // from theSearchParameterMap.keySet() + expandAnyReferenceParameters(paramName, orList); } } } @@ -65,7 +79,7 @@ public class MdmSearchExpandingInterceptor { /** * If a Parameter is a reference parameter, and it has been set to expand MDM, perform the expansion. */ - private void expandAnyReferenceParameters(List orList) { + private void expandAnyReferenceParameters(String theParamName, List orList) { List toRemove = new ArrayList<>(); List toAdd = new ArrayList<>(); for (IQueryParameterType iQueryParameterType : orList) { @@ -85,12 +99,81 @@ public class MdmSearchExpandingInterceptor { if (!expandedResourceIds.isEmpty()) { ourLog.debug("Parameter has been expanded to: {}", String.join(", ", expandedResourceIds)); toRemove.add(refParam); - expandedResourceIds.stream().map(resourceId -> new ReferenceParam(refParam.getResourceType() + "/" + resourceId)).forEach(toAdd::add); + expandedResourceIds.stream() + .map(resourceId -> new ReferenceParam(refParam.getResourceType() + "/" + resourceId)) + .forEach(toAdd::add); } } } + else if (theParamName.equalsIgnoreCase("_id")) { + expandIdParameter(iQueryParameterType, toAdd, toRemove); + } } + orList.removeAll(toRemove); orList.addAll(toAdd); } + + /** + * Expands out the provided _id parameter into all the various + * ids of linked resources. + * @param theIdParameter + * @param theAddList + * @param theRemoveList + */ + private void expandIdParameter(IQueryParameterType theIdParameter, + List theAddList, + List theRemoveList) { + // id parameters can either be StringParam (for $everything operation) + // or TokenParam (for searches) + // either case, we want to expand it out and grab all related resources + IIdType id; + Creator creator; + boolean mdmExpand = false; + if (theIdParameter instanceof StringParam) { + StringParam param = (StringParam) theIdParameter; + mdmExpand = param.isMdmExpand(); + id = new IdDt(param.getValue()); + creator = StringParam::new; + } + else if (theIdParameter instanceof TokenParam) { + TokenParam param = (TokenParam) theIdParameter; + mdmExpand = param.isMdmExpand(); + id = new IdDt(param.getValue()); + creator = TokenParam::new; + } + else { + creator = null; + id = null; + } + + if (id == null || creator == null) { + // in case the _id paramter type is different from the above + ourLog.warn("_id parameter of incorrect type. Expected StringParam or TokenParam, but got {}. No expansion will be done!", + theIdParameter.getClass().getSimpleName()); + } + else if (mdmExpand) { + ourLog.debug("_id parameter must be expanded out from: {}", id.getValue()); + + Set expandedResourceIds = myMdmLinkExpandSvc.expandMdmBySourceResourceId(id); + + if (expandedResourceIds.isEmpty()) { + expandedResourceIds = myMdmLinkExpandSvc.expandMdmByGoldenResourceId(id.getIdPartAsLong()); + } + + //Rebuild + if (!expandedResourceIds.isEmpty()) { + ourLog.debug("_id parameter has been expanded to: {}", String.join(", ", expandedResourceIds)); + + // remove the original + theRemoveList.add(theIdParameter); + + // add in all the linked values + expandedResourceIds.stream() + .map(creator::create) + .forEach(theAddList::add); + } + } + // else - no expansion required + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index a64ba464246..47f3490806f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -356,7 +356,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { PersistedJpaSearchFirstPageBundleProvider retVal = submitSearch(theCallingDao, theParams, theResourceType, theRequestDetails, searchUuid, sb, queryString, theRequestPartitionId, search); retVal.setCacheStatus(cacheStatus); return retVal; - } @Override 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 475cba4758b..5b73e4a3072 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 @@ -1232,9 +1232,10 @@ public class QueryStack { } - public void addPredicateEverythingOperation(String theResourceName, Long theTargetPid) { + // expand out the pids + public void addPredicateEverythingOperation(String theResourceName, Long... theTargetPids) { ResourceLinkPredicateBuilder table = mySqlBuilder.addReferencePredicateBuilder(this, null); - Condition predicate = table.createEverythingPredicate(theResourceName, theTargetPid); + Condition predicate = table.createEverythingPredicate(theResourceName, theTargetPids); mySqlBuilder.addPredicate(predicate); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index f48bc3b3a3f..7ddaf011b17 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -375,6 +375,44 @@ public class SearchBuilder implements ISearchBuilder { query.ifPresent(t -> theQueries.add(t)); } + /** + * Combs through the params for any _id parameters and extracts the PIDs for them + * @param theTargetPids + */ + private void extractTargetPidsFromIdParams(HashSet theTargetPids) { + // get all the IQueryParameterType objects + // for _id -> these should all be StringParam values + HashSet ids = new HashSet<>(); + List> params = myParams.get(IAnyResource.SP_RES_ID); + for (List paramList : params) { + for (IQueryParameterType param : paramList) { + if (param instanceof StringParam) { + // we expect all _id values to be StringParams + ids.add(((StringParam) param).getValue()); + } + else { + // we do not expect the _id parameter to be a non-string value + throw new IllegalArgumentException("_id parameter must be a StringParam"); + } + } + } + + // fetch our target Pids + // this will throw if an id is not found + Map idToPid = myIdHelperService.resolveResourcePersistentIds(myRequestPartitionId, + myResourceName, + new ArrayList<>(ids)); + if (myAlsoIncludePids == null) { + myAlsoIncludePids = new ArrayList<>(); + } + + // add the pids to targetPids + for (ResourcePersistentId pid : idToPid.values()) { + myAlsoIncludePids.add(pid); + theTargetPids.add(pid.getIdAsLong()); + } + } + private Optional createChunkedQuery(SearchParameterMap theParams, SortSpec sort, Integer theOffset, Integer theMaximumResults, boolean theCount, RequestDetails theRequest, List thePidList) { String sqlBuilderResourceName = myParams.getEverythingMode() == null ? myResourceName : null; SearchQueryBuilder sqlBuilder = new SearchQueryBuilder(myContext, myDaoConfig.getModelConfig(), myPartitionSettings, myRequestPartitionId, sqlBuilderResourceName, mySqlBuilderFactory, myDialectProvider, theCount); @@ -394,17 +432,10 @@ public class SearchBuilder implements ISearchBuilder { } if (myParams.getEverythingMode() != null) { - Long targetPid = null; + HashSet targetPids = new HashSet<>(); if (myParams.get(IAnyResource.SP_RES_ID) != null) { - StringParam idParam = (StringParam) myParams.get(IAnyResource.SP_RES_ID).get(0).get(0); - ResourcePersistentId pid = myIdHelperService.resolveResourcePersistentIds(myRequestPartitionId, myResourceName, idParam.getValue()); - if (myAlsoIncludePids == null) { - myAlsoIncludePids = new ArrayList<>(1); - } - myAlsoIncludePids.add(pid); - targetPid = pid.getIdAsLong(); + extractTargetPidsFromIdParams(targetPids); } else { - // For Everything queries, we make the query root by the ResourceLink table, since this query // is basically a reverse-include search. For type/Everything (as opposed to instance/Everything) // the one problem with this approach is that it doesn't catch Patients that have absolutely @@ -420,10 +451,9 @@ public class SearchBuilder implements ISearchBuilder { myAlsoIncludePids.addAll(ResourcePersistentId.fromLongList(output)); } - queryStack3.addPredicateEverythingOperation(myResourceName, targetPid); + queryStack3.addPredicateEverythingOperation(myResourceName, targetPids.toArray(new Long[0])); } else { - /* * If we're doing a filter, always use the resource table as the root - This avoids the possibility of * specific filters with ORs as their root from working around the natural resource type / deletion @@ -438,7 +468,6 @@ public class SearchBuilder implements ISearchBuilder { // Normal search searchForIdsWithAndOr(sqlBuilder, queryStack3, myParams, theRequest); - } // If we haven't added any predicates yet, we're doing a search for all resources. Make sure we add the @@ -463,8 +492,9 @@ public class SearchBuilder implements ISearchBuilder { } //-- exclude the pids already in the previous iterator - if (hasNextIteratorQuery) + if (hasNextIteratorQuery) { sqlBuilder.excludeResourceIdsPredicate(myPidSet); + } /* * Sort diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java index 50e6de2a610..f2ba50a66fc 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java @@ -80,6 +80,7 @@ import org.springframework.beans.factory.annotation.Autowired; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.ListIterator; @@ -622,10 +623,14 @@ public class ResourceLinkPredicateBuilder extends BaseJoiningPredicateBuilder { } @Nonnull - public Condition createEverythingPredicate(String theResourceName, Long theTargetPid) { - if (theTargetPid != null) { - return BinaryCondition.equalTo(myColumnTargetResourceId, generatePlaceholder(theTargetPid)); + public Condition createEverythingPredicate(String theResourceName, Long... theTargetPids) { + if (theTargetPids != null && theTargetPids.length >= 1) { + // if resource ids are provided, we'll create the predicate + // with ids in or equal to this value + return toEqualToOrInPredicate(myColumnTargetResourceId, + generatePlaceholders(Arrays.stream(theTargetPids).map(Object::toString).collect(Collectors.toList()))); } else { + // ... otherwise we look for resource types return BinaryCondition.equalTo(myColumnTargetResourceType, generatePlaceholder(theResourceName)); } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestJPAConfig.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestJPAConfig.java index 660c76e7556..4f98f112a5c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestJPAConfig.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestJPAConfig.java @@ -3,14 +3,12 @@ package ca.uhn.fhir.jpa.config; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.entity.ModelConfig; -import ca.uhn.fhir.jpa.search.elastic.IndexNamePrefixLayoutStrategy; import ca.uhn.fhir.jpa.subscription.SubscriptionTestUtil; import ca.uhn.fhir.jpa.subscription.channel.config.SubscriptionChannelConfig; import ca.uhn.fhir.jpa.subscription.match.config.SubscriptionProcessorConfig; import ca.uhn.fhir.jpa.subscription.match.deliver.resthook.SubscriptionDeliveringRestHookSubscriber; import ca.uhn.fhir.jpa.subscription.submit.config.SubscriptionSubmitterConfig; import ca.uhn.fhir.test.utilities.BatchJobHelper; -import org.hibernate.search.backend.elasticsearch.index.layout.IndexLayoutStrategy; import org.springframework.batch.core.explore.JobExplorer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -40,7 +38,8 @@ public class TestJPAConfig { @Bean public ModelConfig modelConfig() { - return daoConfig().getModelConfig(); + ModelConfig config = daoConfig().getModelConfig(); + return config; } /* diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java new file mode 100644 index 00000000000..8b19feec931 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java @@ -0,0 +1,136 @@ +package ca.uhn.fhir.jpa.dao.index; + +import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.util.MemoryCacheService; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.function.Function; + +@ExtendWith(MockitoExtension.class) +public class IdHelperServiceTest { + + @Mock + private DaoConfig myDaoConfig; + + @Mock + private IForcedIdDao myForcedIdDao; + + @Mock + private MemoryCacheService myMemoryCacheService; + + @Mock + private PartitionSettings myPartitionSettings; + + @InjectMocks + private IdHelperService myHelperService; + + @Test + public void resolveResourcePersistentIds_withValidPids_returnsMap() { + RequestPartitionId partitionId = RequestPartitionId.allPartitions(); + String resourceType = Patient.class.getSimpleName(); + List patientIdsToResolve = new ArrayList<>(); + patientIdsToResolve.add("123"); + patientIdsToResolve.add("456"); + + // test + Map idToPid = myHelperService.resolveResourcePersistentIds(partitionId, + resourceType, + patientIdsToResolve); + + Assertions.assertFalse(idToPid.isEmpty()); + for (String pid : patientIdsToResolve) { + Assertions.assertTrue(idToPid.containsKey(pid)); + } + } + + @Test + public void resolveResourcePersistentIds_withForcedIdsAndDeleteEnabled_returnsMap() { + RequestPartitionId partitionId = RequestPartitionId.allPartitions(); + String resourceType = Patient.class.getSimpleName(); + List patientIdsToResolve = new ArrayList<>(); + patientIdsToResolve.add("RED"); + patientIdsToResolve.add("BLUE"); + + Object[] redView = new Object[] { + "Patient", + new Long(123l), + "RED", + new Date() + }; + Object[] blueView = new Object[] { + "Patient", + new Long(456l), + "BLUE", + new Date() + }; + + // when + Mockito.when(myDaoConfig.isDeleteEnabled()) + .thenReturn(true); + Mockito.when(myForcedIdDao.findAndResolveByForcedIdWithNoType(Mockito.anyString(), + Mockito.anyList())) + .thenReturn(Collections.singletonList(redView)) + .thenReturn(Collections.singletonList(blueView)); + + // test + Map map = myHelperService.resolveResourcePersistentIds( + partitionId, + resourceType, + patientIdsToResolve + ); + + Assertions.assertFalse(map.isEmpty()); + for (String id : patientIdsToResolve) { + Assertions.assertTrue(map.containsKey(id)); + } + } + + @Test + public void resolveResourcePersistenIds_withForcedIdAndDeleteDisabled_returnsMap() { + RequestPartitionId partitionId = RequestPartitionId.allPartitions(); + String resourceType = Patient.class.getSimpleName(); + List patientIdsToResolve = new ArrayList<>(); + patientIdsToResolve.add("RED"); + patientIdsToResolve.add("BLUE"); + + ResourcePersistentId red = new ResourcePersistentId("Patient", new Long(123l)); + ResourcePersistentId blue = new ResourcePersistentId("Patient", new Long(456l)); + + // we will pretend the lookup value is in the cache + Mockito.when(myMemoryCacheService.getThenPutAfterCommit(Mockito.any(MemoryCacheService.CacheEnum.class), + Mockito.anyString(), + Mockito.any(Function.class))) + .thenReturn(red) + .thenReturn(blue); + + // test + Map map = myHelperService.resolveResourcePersistentIds( + partitionId, + resourceType, + patientIdsToResolve + ); + + Assertions.assertFalse(map.isEmpty()); + for (String id : patientIdsToResolve) { + Assertions.assertTrue(map.containsKey(id)); + } + Assertions.assertEquals(red, map.get("RED")); + Assertions.assertEquals(blue, map.get("BLUE")); + } +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index fd513807f31..8b9882bc98a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -10,7 +10,6 @@ import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.entity.ResourceTag; import ca.uhn.fhir.jpa.model.entity.TagTypeEnum; -import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; @@ -4361,8 +4360,6 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { id = myAllergyIntoleranceDao.read(ai.getIdElement().toUnqualifiedVersionless()).getIdElement(); assertEquals("3", id.getVersionIdPart()); - } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java new file mode 100644 index 00000000000..ea3dd225f77 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilderTest.java @@ -0,0 +1,58 @@ +package ca.uhn.fhir.jpa.search.builder.predicate; + +import ca.uhn.fhir.jpa.search.builder.sql.SearchQueryBuilder; +import com.healthmarketscience.sqlbuilder.BinaryCondition; +import com.healthmarketscience.sqlbuilder.Condition; +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.mockito.Mockito; + +public class ResourceLinkPredicateBuilderTest { + + private ResourceLinkPredicateBuilder myResourceLinkPredicateBuilder; + + @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 + ); + } + + @Test + public void createEverythingPredicate_withListOfPids_returnsInPredicate() { + Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", + 1l, 2l); + + Assertions.assertTrue(condition instanceof InCondition); + } + + @Test + public void createEverythingPredicate_withSinglePid_returnsInCondition() { + Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", + 1l); + + Assertions.assertTrue(condition instanceof InCondition); + } + + @Test + public void createEverythingPredicate_withNoPids_returnsBinaryCondition() { + Condition condition = myResourceLinkPredicateBuilder.createEverythingPredicate("Patient", + new Long[0]); + + Assertions.assertTrue(condition instanceof BinaryCondition); + } +} diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/interceptor/MdmSearchExpandingInterceptorIT.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/interceptor/MdmSearchExpandingInterceptorIT.java index 2c1d2a40bad..8a87c2aa1e6 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/interceptor/MdmSearchExpandingInterceptorIT.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/interceptor/MdmSearchExpandingInterceptorIT.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.mdm.interceptor; import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoPatient; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.entity.MdmLink; import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test; @@ -8,23 +9,31 @@ import ca.uhn.fhir.jpa.mdm.helper.MdmHelperConfig; import ca.uhn.fhir.jpa.mdm.helper.MdmHelperR4; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.mdm.api.MdmConstants; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.param.ReferenceOrListParam; import ca.uhn.fhir.rest.param.ReferenceParam; -import org.elasticsearch.common.collect.Set; +import ca.uhn.fhir.rest.param.TokenOrListParam; +import ca.uhn.fhir.rest.param.TokenParam; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.Mockito; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; +import javax.servlet.http.HttpServletRequest; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import static org.hamcrest.MatcherAssert.assertThat; @@ -44,26 +53,52 @@ public class MdmSearchExpandingInterceptorIT extends BaseMdmR4Test { @Autowired private DaoConfig myDaoConfig; + /** + * creates a GoldenPatient + * a number of patients, + * and Observations for each created patient. + * + * Returns a list of stringified ids for the various resources. + * + * Currently, order of returned resources is patientids first, + * observation ids next. But this can be refined as needed. + * + * @param theResourceCount - number of patients to create + * @return + */ + private List createAndLinkNewResources(int theResourceCount) throws InterruptedException { + boolean expansion = myDaoConfig.isAllowMdmExpansion(); + myDaoConfig.setAllowMdmExpansion(false); + List createdResourceIds = new ArrayList<>(); + + List observationIds = new ArrayList<>(); + for (int i = 0; i < theResourceCount; i++) { + // create patient + MdmHelperR4.OutcomeAndLogMessageWrapper withLatch = myMdmHelper.createWithLatch(addExternalEID(buildJanePatient(), "123")); + createdResourceIds.add(withLatch.getDaoMethodOutcome().getId().getIdPart()); + + // create observation with patient + Observation observation = createObservationWithSubject(createdResourceIds.get(i)); + // we put the observation ids in a separate list so we can + // ensure our returned list is + // patient ids, followed by observation ids + observationIds.add(observation.getIdElement().getIdPart()); + } + + assertLinkCount(theResourceCount); + + // add in our observationIds + createdResourceIds.addAll(observationIds); + + myDaoConfig.setAllowMdmExpansion(expansion); + return createdResourceIds; + } + @Test public void testReferenceExpansionWorks() throws InterruptedException { - myDaoConfig.setAllowMdmExpansion(false); - MdmHelperR4.OutcomeAndLogMessageWrapper withLatch = myMdmHelper.createWithLatch(addExternalEID(buildJanePatient(), "123")); - MdmHelperR4.OutcomeAndLogMessageWrapper withLatch1 = myMdmHelper.createWithLatch(addExternalEID(buildJanePatient(), "123")); - MdmHelperR4.OutcomeAndLogMessageWrapper withLatch2 = myMdmHelper.createWithLatch(addExternalEID(buildJanePatient(), "123")); - MdmHelperR4.OutcomeAndLogMessageWrapper withLatch3 = myMdmHelper.createWithLatch(addExternalEID(buildJanePatient(), "123")); - - assertLinkCount(4); - - String id = withLatch.getDaoMethodOutcome().getId().getIdPart(); - String id1 = withLatch1.getDaoMethodOutcome().getId().getIdPart(); - String id2 = withLatch2.getDaoMethodOutcome().getId().getIdPart(); - String id3 = withLatch3.getDaoMethodOutcome().getId().getIdPart(); - - //Create an Observation for each Patient - createObservationWithSubject(id); - createObservationWithSubject(id1); - createObservationWithSubject(id2); - createObservationWithSubject(id3); + int resourceCount = 4; + List ids = createAndLinkNewResources(resourceCount); + String id = ids.get(0); SearchParameterMap searchParameterMap = new SearchParameterMap(); searchParameterMap.setLoadSynchronous(true); @@ -72,6 +107,7 @@ public class MdmSearchExpandingInterceptorIT extends BaseMdmR4Test { searchParameterMap.add(Observation.SP_SUBJECT, referenceOrListParam); //With MDM Expansion disabled, this should return 1 result. + myDaoConfig.setAllowMdmExpansion(false); IBundleProvider search = myObservationDao.search(searchParameterMap); assertThat(search.size(), is(equalTo(1))); @@ -91,7 +127,78 @@ public class MdmSearchExpandingInterceptorIT extends BaseMdmR4Test { goldenSpMap.add(Observation.SP_SUBJECT, goldenReferenceOrListParam); search = myObservationDao.search(goldenSpMap); - assertThat(search.size(), is(equalTo(4))); + assertThat(search.size(), is(equalTo(resourceCount))); + } + + @Test + public void testMdmExpansionExpands_idParameter() throws InterruptedException { + int resourceCount = 4; + List expectedIds = createAndLinkNewResources(resourceCount); + String patientId = expectedIds.get(0); + + myDaoConfig.setAllowMdmExpansion(true); + + SearchParameterMap map = new SearchParameterMap(); + TokenOrListParam orListParam = new TokenOrListParam(); + TokenParam patientIdParam = new TokenParam(); + patientIdParam.setValue(patientId); + patientIdParam.setMdmExpand(true); + orListParam.add(patientIdParam); + map.add("_id", orListParam); + + IBundleProvider outcome = myPatientDao.search(map); + + Assertions.assertNotNull(outcome); + // we know 4 cause that's how many patients are created + // plus one golden resource + Assertions.assertEquals(resourceCount + 1, outcome.size()); + List resourceIds = outcome.getAllResourceIds(); + // check the patients - first 4 ids + for (int i = 0; i < resourceIds.size() - 1; i++) { + Assertions.assertTrue(resourceIds.contains(expectedIds.get(i))); + } + } + + @Test + public void testMdmExpansionIsSupportedOnEverythingOperation() throws InterruptedException { + int resourceCount = 4; + List expectedIds = createAndLinkNewResources(resourceCount); + String id = expectedIds.get(0); + + HashMap queryParameters = new HashMap<>(); + queryParameters.put("_mdm", new String[] { "true" }); + + HttpServletRequest req = Mockito.mock(HttpServletRequest.class); + RequestDetails theDetails = Mockito.mock(RequestDetails.class); + + Mockito.when(theDetails.getParameters()) + .thenReturn(queryParameters); + + // test + myDaoConfig.setAllowMdmExpansion(true); + IFhirResourceDaoPatient dao = (IFhirResourceDaoPatient) myPatientDao; + IBundleProvider outcome = dao.patientInstanceEverything( + req, + new IdDt(id), + null, + null, + null, + null, + null, + null, + null, + theDetails + ); + + // verify return results + // we expect all the linked ids to be returned too + Assertions.assertNotNull(outcome); + // plus 1 for the golden resource + Assertions.assertEquals(expectedIds.size() + 1, outcome.size()); + List returnedIds = outcome.getAllResourceIds(); + for (String expected : expectedIds) { + Assertions.assertTrue(returnedIds.contains(expected)); + } } @Test @@ -116,6 +223,5 @@ public class MdmSearchExpandingInterceptorIT extends BaseMdmR4Test { observation.setCode(new CodeableConcept().setText("Made for Patient/" + thePatientId)); DaoMethodOutcome daoMethodOutcome = myObservationDao.create(observation); return (Observation) daoMethodOutcome.getResource(); - } } diff --git a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java index b7b1ab7c08f..0c5323ada8f 100644 --- a/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java +++ b/hapi-fhir-jpaserver-searchparam/src/test/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceIndexedSearchParamsTest.java @@ -80,7 +80,6 @@ public class ResourceIndexedSearchParamsTest { return retVal; } - @Test public void testExtractCompositeStringUniquesValueChains() { List> partsChoices;