From 40319683571427a6b9637c457fe38cebd48fe820 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 5 May 2021 16:57:12 -0400 Subject: [PATCH 1/3] Calls with zero request details and non-partitionable resource automatically go to default now. This updates the test to pass in servletrequest details, simulating a user request (#2636) --- .../jpa/interceptor/PartitioningInterceptorR4Test.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java index b2fe10e4e25..764a622c0db 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/PartitioningInterceptorR4Test.java @@ -12,6 +12,7 @@ import ca.uhn.fhir.jpa.interceptor.ex.PartitionInterceptorReadPartitionsBasedOnS import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.partition.IPartitionLookupSvc; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; @@ -92,7 +93,7 @@ public class PartitioningInterceptorR4Test extends BaseJpaR4SystemTest { StructureDefinition sd = new StructureDefinition(); sd.setUrl("http://foo"); - myStructureDefinitionDao.create(sd); + myStructureDefinitionDao.create(sd, new ServletRequestDetails()); runInTransaction(()->{ List resources = myResourceTableDao.findAll(); @@ -108,7 +109,7 @@ public class PartitioningInterceptorR4Test extends BaseJpaR4SystemTest { StructureDefinition sd = new StructureDefinition(); sd.setUrl("http://foo"); - myStructureDefinitionDao.create(sd); + myStructureDefinitionDao.create(sd, new ServletRequestDetails()); runInTransaction(()->{ List resources = myResourceTableDao.findAll(); @@ -124,7 +125,7 @@ public class PartitioningInterceptorR4Test extends BaseJpaR4SystemTest { StructureDefinition sd = new StructureDefinition(); sd.setUrl("http://foo"); try { - myStructureDefinitionDao.create(sd); + myStructureDefinitionDao.create(sd, new ServletRequestDetails()); fail(); } catch (UnprocessableEntityException e) { assertEquals("Resource type StructureDefinition can not be partitioned", e.getMessage()); From a4856bba1799e0bce496b5f0cf6aa9c32b1706ee Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 5 May 2021 21:50:51 -0400 Subject: [PATCH 2/3] fix iBundleProvider.getResources(from,to) throwing a NPE (#2632) * fix iBundleProvider.getResources(from,to) throwing a NPE * change log * review feedback * rename test to IT that has concurrency failures --- .../5_4_0/2632-measure-evaluation-npe.yaml | 4 + ...seJpaResourceProviderCompositionDstu3.java | 2 +- .../BaseJpaResourceProviderCompositionR4.java | 2 +- .../BaseJpaResourceProviderCompositionR5.java | 4 +- .../retrieve/JpaFhirRetrieveProvider.java | 5 +- .../uhn/fhir/cql/config/CqlDstu3Config.java | 6 +- .../ca/uhn/fhir/cql/config/CqlR4Config.java | 6 +- .../dstu3/evaluation/MeasureEvaluation.java | 73 +++++++++---------- .../provider/JpaTerminologyProvider.java | 4 +- .../LibraryResolutionProviderImpl.java | 12 +-- .../cql/r4/evaluation/MeasureEvaluation.java | 25 +++---- .../r4/provider/JpaTerminologyProvider.java | 4 +- .../LibraryResolutionProviderImpl.java | 12 +-- .../svc/candidate/MdmCandidateSearchSvc.java | 10 +-- ...vcImplTest.java => MdmBatchSvcImplIT.java} | 4 +- ...scriptionDeliveringRestHookSubscriber.java | 4 +- .../fhir/rest/api/server/IBundleProvider.java | 22 ++++++ .../method/TransactionMethodBinding.java | 17 +++-- .../rest/api/server/IBundleProviderTest.java | 18 +++++ 19 files changed, 133 insertions(+), 101 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2632-measure-evaluation-npe.yaml rename hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/{MdmBatchSvcImplTest.java => MdmBatchSvcImplIT.java} (98%) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2632-measure-evaluation-npe.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2632-measure-evaluation-npe.yaml new file mode 100644 index 00000000000..6b1428a5ad2 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2632-measure-evaluation-npe.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 2632 +title: "Certain calls to the $evaluate-measure operation could result in a NullPointerException. This is now corrected." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/BaseJpaResourceProviderCompositionDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/BaseJpaResourceProviderCompositionDstu3.java index 61ee419974f..bc7c0924dcb 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/BaseJpaResourceProviderCompositionDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/BaseJpaResourceProviderCompositionDstu3.java @@ -81,7 +81,7 @@ public class BaseJpaResourceProviderCompositionDstu3 extends JpaResourceProvider startRequest(theServletRequest); try { IBundleProvider bundleProvider = ((IFhirResourceDaoComposition) getDao()).getDocumentForComposition(theServletRequest, theId, theCount, theOffset, theLastUpdated, theSortSpec, theRequestDetails); - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); boolean foundCompositionResource = false; Bundle bundle = new Bundle().setType(Bundle.BundleType.DOCUMENT); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseJpaResourceProviderCompositionR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseJpaResourceProviderCompositionR4.java index 96d1aea0200..6d5d805c908 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseJpaResourceProviderCompositionR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseJpaResourceProviderCompositionR4.java @@ -78,7 +78,7 @@ public class BaseJpaResourceProviderCompositionR4 extends JpaResourceProviderR4< startRequest(theServletRequest); try { IBundleProvider bundleProvider = ((IFhirResourceDaoComposition) getDao()).getDocumentForComposition(theServletRequest, theId, theCount, theOffset, theLastUpdated, theSortSpec, theRequestDetails); - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); boolean foundCompositionResource = false; Bundle bundle = new Bundle().setType(Bundle.BundleType.DOCUMENT); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r5/BaseJpaResourceProviderCompositionR5.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r5/BaseJpaResourceProviderCompositionR5.java index 69cb61b51b4..5106c844d0b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r5/BaseJpaResourceProviderCompositionR5.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r5/BaseJpaResourceProviderCompositionR5.java @@ -77,8 +77,8 @@ public class BaseJpaResourceProviderCompositionR5 extends JpaResourceProviderR5< startRequest(theServletRequest); try { - IBundleProvider bundleProvider = ((IFhirResourceDaoComposition) getDao()).getDocumentForComposition(theServletRequest, theId, theCount, theOffset,theLastUpdated, theSortSpec, theRequestDetails); - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + IBundleProvider bundleProvider = ((IFhirResourceDaoComposition) getDao()).getDocumentForComposition(theServletRequest, theId, theCount, theOffset, theLastUpdated, theSortSpec, theRequestDetails); + List resourceList = bundleProvider.getAllResources(); boolean foundCompositionResource = false; Bundle bundle = new Bundle().setType(Bundle.BundleType.DOCUMENT); diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/common/retrieve/JpaFhirRetrieveProvider.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/common/retrieve/JpaFhirRetrieveProvider.java index 6c3f02c5a80..0cd17acdaa7 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/common/retrieve/JpaFhirRetrieveProvider.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/common/retrieve/JpaFhirRetrieveProvider.java @@ -95,10 +95,7 @@ public class JpaFhirRetrieveProvider extends SearchParamFhirRetrieveProvider { if (bundleProvider.size() == null) { return resolveResourceList(bundleProvider.getResources(0, 10000)); } - if (bundleProvider.size() == 0) { - return new ArrayList<>(); - } - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); return resolveResourceList(resourceList); } diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlDstu3Config.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlDstu3Config.java index 0b054d8c985..90ef7663471 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlDstu3Config.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlDstu3Config.java @@ -33,10 +33,8 @@ import ca.uhn.fhir.cql.dstu3.provider.MeasureOperationsProvider; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.cache.IResourceChangeListenerRegistry; -import ca.uhn.fhir.jpa.term.api.ITermReadSvcDstu3; - import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; - +import ca.uhn.fhir.jpa.term.api.ITermReadSvcDstu3; import org.cqframework.cql.cql2elm.CqlTranslatorOptions; import org.cqframework.cql.cql2elm.model.Model; import org.cqframework.cql.elm.execution.Library; @@ -99,7 +97,7 @@ public class CqlDstu3Config extends BaseCqlConfig { @Bean public ElmCacheResourceChangeListener elmCacheResourceChangeListener(IResourceChangeListenerRegistry resourceChangeListenerRegistry, IFhirResourceDao libraryDao, Map globalLibraryCache) { ElmCacheResourceChangeListener listener = new ElmCacheResourceChangeListener(libraryDao, globalLibraryCache); - resourceChangeListenerRegistry.registerResourceResourceChangeListener("Library", new SearchParameterMap(), listener, 1000); + resourceChangeListenerRegistry.registerResourceResourceChangeListener("Library", SearchParameterMap.newSynchronous(), listener, 1000); return listener; } } diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlR4Config.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlR4Config.java index add489a137d..a1e7f1eaf4d 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlR4Config.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/config/CqlR4Config.java @@ -34,10 +34,8 @@ import ca.uhn.fhir.cql.r4.provider.MeasureOperationsProvider; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.cache.IResourceChangeListenerRegistry; -import ca.uhn.fhir.jpa.term.api.ITermReadSvcR4; - import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; - +import ca.uhn.fhir.jpa.term.api.ITermReadSvcR4; import org.cqframework.cql.cql2elm.CqlTranslatorOptions; import org.cqframework.cql.cql2elm.model.Model; import org.cqframework.cql.elm.execution.Library; @@ -110,7 +108,7 @@ public class CqlR4Config extends BaseCqlConfig { @Bean public ElmCacheResourceChangeListener elmCacheResourceChangeListener(IResourceChangeListenerRegistry resourceChangeListenerRegistry, IFhirResourceDao libraryDao, Map globalLibraryCache) { ElmCacheResourceChangeListener listener = new ElmCacheResourceChangeListener(libraryDao, globalLibraryCache); - resourceChangeListenerRegistry.registerResourceResourceChangeListener("Library", new SearchParameterMap(), listener, 1000); + resourceChangeListenerRegistry.registerResourceResourceChangeListener("Library", SearchParameterMap.newSynchronous(), listener, 1000); return listener; } } diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/evaluation/MeasureEvaluation.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/evaluation/MeasureEvaluation.java index 5a408e73887..0c191590377 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/evaluation/MeasureEvaluation.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/evaluation/MeasureEvaluation.java @@ -33,7 +33,6 @@ import org.hl7.fhir.dstu3.model.CodeableConcept; import org.hl7.fhir.dstu3.model.Coding; import org.hl7.fhir.dstu3.model.Extension; import org.hl7.fhir.dstu3.model.IdType; -import org.hl7.fhir.dstu3.model.IntegerType; import org.hl7.fhir.dstu3.model.ListResource; import org.hl7.fhir.dstu3.model.Measure; import org.hl7.fhir.dstu3.model.MeasureReport; @@ -65,20 +64,20 @@ import java.util.stream.Collectors; public class MeasureEvaluation { - private static final Logger logger = LoggerFactory.getLogger(MeasureEvaluation.class); + private static final Logger logger = LoggerFactory.getLogger(MeasureEvaluation.class); - private Interval measurementPeriod; - private DaoRegistry registry; + private final Interval measurementPeriod; + private final DaoRegistry registry; - public MeasureEvaluation(DaoRegistry registry, Interval measurementPeriod) { - this.registry = registry; - this.measurementPeriod = measurementPeriod; - } + public MeasureEvaluation(DaoRegistry registry, Interval measurementPeriod) { + this.registry = registry; + this.measurementPeriod = measurementPeriod; + } - public MeasureReport evaluatePatientMeasure(Measure measure, Context context, String patientId) { - logger.info("Generating individual report"); + public MeasureReport evaluatePatientMeasure(Measure measure, Context context, String patientId) { + logger.info("Generating individual report"); - if (patientId == null) { + if (patientId == null) { return evaluatePopulationMeasure(measure, context); } @@ -107,24 +106,24 @@ public class MeasureEvaluation { } private List getPractitionerPatients(String practitionerRef) { - SearchParameterMap map = new SearchParameterMap(); - map.add("general-practitioner", new ReferenceParam( - practitionerRef.startsWith("Practitioner/") ? practitionerRef : "Practitioner/" + practitionerRef)); + SearchParameterMap map = SearchParameterMap.newSynchronous(); + map.add("general-practitioner", new ReferenceParam( + practitionerRef.startsWith("Practitioner/") ? practitionerRef : "Practitioner/" + practitionerRef)); - List patients = new ArrayList<>(); - IBundleProvider patientProvider = registry.getResourceDao("Patient").search(map); - List patientList = patientProvider.getResources(0, patientProvider.size()); - patientList.forEach(x -> patients.add((Patient) x)); - return patients; - } + List patients = new ArrayList<>(); + IBundleProvider patientProvider = registry.getResourceDao("Patient").search(map); + List patientList = patientProvider.getAllResources(); + patientList.forEach(x -> patients.add((Patient) x)); + return patients; + } private List getAllPatients() { - List patients = new ArrayList<>(); - IBundleProvider patientProvider = registry.getResourceDao("Patient").search(new SearchParameterMap()); - List patientList = patientProvider.getResources(0, patientProvider.size()); - patientList.forEach(x -> patients.add((Patient) x)); - return patients; - } + List patients = new ArrayList<>(); + IBundleProvider patientProvider = registry.getResourceDao("Patient").search(SearchParameterMap.newSynchronous()); + List patientList = patientProvider.getAllResources(); + patientList.forEach(x -> patients.add((Patient) x)); + return patients; + } public MeasureReport evaluatePopulationMeasure(Measure measure, Context context) { logger.info("Generating summary report"); @@ -567,8 +566,8 @@ public class MeasureEvaluation { } if (!list.isEmpty()) { - list.setId("List/" + UUID.randomUUID().toString()); - list.setTitle(key); + list.setId("List/" + UUID.randomUUID()); + list.setTitle(key); resources.put(list.getId(), list); list.getEntry().forEach(listResource -> evaluatedResourcesList.add(listResource.getItem().getReference())); } @@ -713,17 +712,15 @@ public class MeasureEvaluation { for (Object o : context.getEvaluatedResources()) { if (o instanceof Resource) { - Resource r = (Resource) o; - String id = (r.getIdElement().getResourceType() != null ? (r.getIdElement().getResourceType() + "/") - : "") + r.getIdElement().getIdPart(); - if (!codeHashSet.contains(id)) { - codeHashSet.add(id); - } + Resource r = (Resource) o; + String id = (r.getIdElement().getResourceType() != null ? (r.getIdElement().getResourceType() + "/") + : "") + r.getIdElement().getIdPart(); + codeHashSet.add(id); - if (!resources.containsKey(id)) { - resources.put(id, r); - } - } + if (!resources.containsKey(id)) { + resources.put(id, r); + } + } } context.clearEvaluatedResources(); diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/JpaTerminologyProvider.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/JpaTerminologyProvider.java index 9808b664271..ea200b61547 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/JpaTerminologyProvider.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/JpaTerminologyProvider.java @@ -96,8 +96,8 @@ public class JpaTerminologyProvider implements TerminologyProvider { } IBundleProvider bundleProvider = myValueSetDao - .search(new SearchParameterMap().add(ValueSet.SP_URL, new UriParam(valueSet.getId()))); - List valueSets = bundleProvider.getResources(0, bundleProvider.size()); + .search(SearchParameterMap.newSynchronous().add(ValueSet.SP_URL, new UriParam(valueSet.getId()))); + List valueSets = bundleProvider.getAllResources(); if (valueSets.isEmpty()) { throw new IllegalArgumentException(String.format("Could not resolve value set %s.", valueSet.getId())); } else if (valueSets.size() == 1) { diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/LibraryResolutionProviderImpl.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/LibraryResolutionProviderImpl.java index de88337c0de..af8076d68b7 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/LibraryResolutionProviderImpl.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/dstu3/provider/LibraryResolutionProviderImpl.java @@ -67,7 +67,7 @@ public class LibraryResolutionProviderImpl implements LibraryResolutionProvider< version = parts[1]; } - SearchParameterMap map = new SearchParameterMap(); + SearchParameterMap map = SearchParameterMap.newSynchronous(); map.add("url", new UriParam(resourceUrl)); if (version != null) { map.add("version", new TokenParam(version)); @@ -75,10 +75,10 @@ public class LibraryResolutionProviderImpl implements LibraryResolutionProvider< ca.uhn.fhir.rest.api.server.IBundleProvider bundleProvider = myLibraryDao.search(map); - if (bundleProvider.size() == 0) { + if (bundleProvider.size() == null || bundleProvider.size() == 0) { return null; } - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); return LibraryResolutionProvider.selectFromList(resolveLibraries(resourceList), version, x -> x.getVersion()); } @@ -97,14 +97,14 @@ public class LibraryResolutionProviderImpl implements LibraryResolutionProvider< private Iterable getLibrariesByName(String name) { // Search for libraries by name - SearchParameterMap map = new SearchParameterMap(); + SearchParameterMap map = SearchParameterMap.newSynchronous(); map.add("name", new StringParam(name, true)); ca.uhn.fhir.rest.api.server.IBundleProvider bundleProvider = myLibraryDao.search(map); - if (bundleProvider.size() == 0) { + if (bundleProvider.size() == null || bundleProvider.size() == 0) { return new ArrayList<>(); } - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); return resolveLibraries(resourceList); } diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/evaluation/MeasureEvaluation.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/evaluation/MeasureEvaluation.java index 3fb85e7e248..01a215cf52b 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/evaluation/MeasureEvaluation.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/evaluation/MeasureEvaluation.java @@ -35,7 +35,6 @@ import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.Extension; import org.hl7.fhir.r4.model.IdType; -import org.hl7.fhir.r4.model.IntegerType; import org.hl7.fhir.r4.model.ListResource; import org.hl7.fhir.r4.model.Measure; import org.hl7.fhir.r4.model.MeasureReport; @@ -68,9 +67,9 @@ public class MeasureEvaluation { private static final Logger logger = LoggerFactory.getLogger(MeasureEvaluation.class); - private DataProvider provider; - private Interval measurementPeriod; - private DaoRegistry registry; + private final DataProvider provider; + private final Interval measurementPeriod; + private final DaoRegistry registry; public MeasureEvaluation(DataProvider provider, DaoRegistry registry, Interval measurementPeriod) { this.provider = provider; @@ -106,21 +105,21 @@ public class MeasureEvaluation { } private List getPractitionerPatients(String practitionerRef) { - SearchParameterMap map = new SearchParameterMap(); + SearchParameterMap map = SearchParameterMap.newSynchronous(); map.add("general-practitioner", new ReferenceParam( - practitionerRef.startsWith("Practitioner/") ? practitionerRef : "Practitioner/" + practitionerRef)); + practitionerRef.startsWith("Practitioner/") ? practitionerRef : "Practitioner/" + practitionerRef)); List patients = new ArrayList<>(); IBundleProvider patientProvider = registry.getResourceDao("Patient").search(map); - List patientList = patientProvider.getResources(0, patientProvider.size()); + List patientList = patientProvider.getAllResources(); patientList.forEach(x -> patients.add((Patient) x)); return patients; } private List getAllPatients() { List patients = new ArrayList<>(); - IBundleProvider patientProvider = registry.getResourceDao("Patient").search(new SearchParameterMap()); - List patientList = patientProvider.getResources(0, patientProvider.size()); + IBundleProvider patientProvider = registry.getResourceDao("Patient").search(SearchParameterMap.newSynchronous()); + List patientList = patientProvider.getAllResources(); patientList.forEach(x -> patients.add((Patient) x)); return patients; } @@ -552,7 +551,7 @@ public class MeasureEvaluation { } if (!list.isEmpty()) { - list.setId("List/" + UUID.randomUUID().toString()); + list.setId("List/" + UUID.randomUUID()); list.setTitle(key); resources.put(list.getId(), list); list.getEntry().forEach(listResource -> evaluatedResourcesList.add(listResource.getItem().getReference())); @@ -700,10 +699,8 @@ public class MeasureEvaluation { if (o instanceof Resource) { Resource r = (Resource) o; String id = (r.getIdElement().getResourceType() != null ? (r.getIdElement().getResourceType() + "/") : "") - + r.getIdElement().getIdPart(); - if (!codeHashSet.contains(id)) { - codeHashSet.add(id); - } + + r.getIdElement().getIdPart(); + codeHashSet.add(id); if (!resources.containsKey(id)) { resources.put(id, r); diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/JpaTerminologyProvider.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/JpaTerminologyProvider.java index 8aae7654ddf..83336966834 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/JpaTerminologyProvider.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/JpaTerminologyProvider.java @@ -94,8 +94,8 @@ public class JpaTerminologyProvider implements TerminologyProvider { } IBundleProvider bundleProvider = myValueSetDao - .search(new SearchParameterMap().add(ValueSet.SP_URL, new UriParam(valueSet.getId()))); - List valueSets = bundleProvider.getResources(0, bundleProvider.size()); + .search(SearchParameterMap.newSynchronous().add(ValueSet.SP_URL, new UriParam(valueSet.getId()))); + List valueSets = bundleProvider.getAllResources(); if (valueSets.isEmpty()) { throw new IllegalArgumentException(String.format("Could not resolve value set %s.", valueSet.getId())); } else if (valueSets.size() == 1) { diff --git a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/LibraryResolutionProviderImpl.java b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/LibraryResolutionProviderImpl.java index c9daf29443c..70274de7dc3 100644 --- a/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/LibraryResolutionProviderImpl.java +++ b/hapi-fhir-jpaserver-cql/src/main/java/ca/uhn/fhir/cql/r4/provider/LibraryResolutionProviderImpl.java @@ -86,7 +86,7 @@ public class LibraryResolutionProviderImpl implements LibraryResolutionProvider< version = parts[1]; } - SearchParameterMap map = new SearchParameterMap(); + SearchParameterMap map = SearchParameterMap.newSynchronous(); map.add("url", new UriParam(resourceUrl)); if (version != null) { map.add("version", new TokenParam(version)); @@ -94,23 +94,23 @@ public class LibraryResolutionProviderImpl implements LibraryResolutionProvider< ca.uhn.fhir.rest.api.server.IBundleProvider bundleProvider = myLibraryDao.search(map); - if (bundleProvider.size() == 0) { + if (bundleProvider.size() == null || bundleProvider.size() == 0) { return null; } - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); return LibraryResolutionProvider.selectFromList(resolveLibraries(resourceList), version, x -> x.getVersion()); } private Iterable getLibrariesByName(String name) { // Search for libraries by name - SearchParameterMap map = new SearchParameterMap(); + SearchParameterMap map = SearchParameterMap.newSynchronous(); map.add("name", new StringParam(name, true)); ca.uhn.fhir.rest.api.server.IBundleProvider bundleProvider = myLibraryDao.search(map); - if (bundleProvider.size() == 0) { + if (bundleProvider.size() == null || bundleProvider.size() == 0) { return new ArrayList<>(); } - List resourceList = bundleProvider.getResources(0, bundleProvider.size()); + List resourceList = bundleProvider.getAllResources(); return resolveLibraries(resourceList); } diff --git a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/candidate/MdmCandidateSearchSvc.java b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/candidate/MdmCandidateSearchSvc.java index 47ce0b9ca0e..4b3a0e17d4e 100644 --- a/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/candidate/MdmCandidateSearchSvc.java +++ b/hapi-fhir-jpaserver-mdm/src/main/java/ca/uhn/fhir/jpa/mdm/svc/candidate/MdmCandidateSearchSvc.java @@ -20,15 +20,15 @@ package ca.uhn.fhir.jpa.mdm.svc.candidate; * #L% */ -import ca.uhn.fhir.mdm.api.IMdmSettings; -import ca.uhn.fhir.mdm.log.Logs; -import ca.uhn.fhir.mdm.rules.json.MdmFilterSearchParamJson; -import ca.uhn.fhir.mdm.rules.json.MdmResourceSearchParamJson; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.mdm.svc.MdmSearchParamSvc; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.mdm.api.IMdmSettings; +import ca.uhn.fhir.mdm.log.Logs; +import ca.uhn.fhir.mdm.rules.json.MdmFilterSearchParamJson; +import ca.uhn.fhir.mdm.rules.json.MdmResourceSearchParamJson; import ca.uhn.fhir.rest.api.server.IBundleProvider; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -136,7 +136,7 @@ public class MdmCandidateSearchSvc { //3. IFhirResourceDao resourceDao = myDaoRegistry.getResourceDao(theResourceType); IBundleProvider search = resourceDao.search(searchParameterMap); - List resources = search.getResources(0, search.size()); + List resources = search.getAllResources(); int initialSize = theMatchedPidsToResources.size(); diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmBatchSvcImplTest.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmBatchSvcImplIT.java similarity index 98% rename from hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmBatchSvcImplTest.java rename to hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmBatchSvcImplIT.java index b972988e423..3365a204ba8 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmBatchSvcImplTest.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/svc/MdmBatchSvcImplIT.java @@ -1,9 +1,9 @@ package ca.uhn.fhir.jpa.mdm.svc; -import ca.uhn.fhir.mdm.api.IMdmSubmitSvc; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.mdm.BaseMdmR4Test; +import ca.uhn.fhir.mdm.api.IMdmSubmitSvc; import ca.uhn.test.concurrency.PointcutLatch; import org.apache.commons.lang3.time.DateUtils; import org.junit.jupiter.api.AfterEach; @@ -14,7 +14,7 @@ import org.springframework.beans.factory.annotation.Autowired; import java.io.IOException; import java.util.Date; -class MdmBatchSvcImplTest extends BaseMdmR4Test { +class MdmBatchSvcImplIT extends BaseMdmR4Test { @Autowired IMdmSubmitSvc myMdmSubmitSvc; diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/deliver/resthook/SubscriptionDeliveringRestHookSubscriber.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/deliver/resthook/SubscriptionDeliveringRestHookSubscriber.java index 35b7972f575..db761b8670f 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/deliver/resthook/SubscriptionDeliveringRestHookSubscriber.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/match/deliver/resthook/SubscriptionDeliveringRestHookSubscriber.java @@ -66,7 +66,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; @Scope("prototype") public class SubscriptionDeliveringRestHookSubscriber extends BaseSubscriptionDeliverySubscriber { - private static Logger ourLog = LoggerFactory.getLogger(SubscriptionDeliveringRestHookSubscriber.class); + private static final Logger ourLog = LoggerFactory.getLogger(SubscriptionDeliveringRestHookSubscriber.class); @Autowired private DaoRegistry myDaoRegistry; @@ -155,7 +155,7 @@ public class SubscriptionDeliveringRestHookSubscriber extends BaseSubscriptionDe IBundleProvider searchResults = dao.search(payloadSearchMap); BundleBuilder builder = new BundleBuilder(myFhirContext); - for (IBaseResource next : searchResults.getResources(0, searchResults.size())) { + for (IBaseResource next : searchResults.getAllResources()) { builder.addTransactionUpdateEntry(next); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java index ec0c7753ca4..a276d031165 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java @@ -1,11 +1,13 @@ package ca.uhn.fhir.rest.api.server; +import ca.uhn.fhir.context.ConfigurationException; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -113,6 +115,26 @@ public interface IBundleProvider { @Nonnull List getResources(int theFromIndex, int theToIndex); + /** + * Get all resources + * + * @return getResources(0, this.size ()). Return an empty list if size() is zero. + * @throws ConfigurationException if size() is null + */ + @Nonnull + default List getAllResources() { + List retval = new ArrayList<>(); + + Integer size = size(); + if (size == null) { + throw new ConfigurationException("Attempt to request all resources from an asynchronous search result. The SearchParameterMap for this search probably should have been synchronous."); + } + if (size > 0) { + retval.addAll(getResources(0, size)); + } + return retval; + } + /** * Returns the UUID associated with this search. Note that this * does not need to return a non-null value unless it a diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java index 5796d1cc3e0..e2a4b6fd29e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java @@ -19,12 +19,6 @@ package ca.uhn.fhir.rest.server.method; * limitations under the License. * #L% */ -import static org.apache.commons.lang3.StringUtils.isNotBlank; - -import java.lang.reflect.Method; -import java.util.List; - -import org.hl7.fhir.instance.model.api.IBaseResource; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; @@ -34,13 +28,20 @@ import ca.uhn.fhir.rest.annotation.Transaction; import ca.uhn.fhir.rest.annotation.TransactionParam; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; -import ca.uhn.fhir.rest.api.server.*; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.IRestfulServer; +import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.rest.server.method.TransactionParameter.ParamStyle; +import org.hl7.fhir.instance.model.api.IBaseResource; import javax.annotation.Nonnull; +import java.lang.reflect.Method; +import java.util.List; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; public class TransactionMethodBinding extends BaseResourceReturningMethodBinding { @@ -127,7 +128,7 @@ public class TransactionMethodBinding extends BaseResourceReturningMethodBinding * " entries, but server method response contained " + retVal.size() + " entries (must be the same)"); } } */ - List retResources = retVal.getResources(0, retVal.size()); + List retResources = retVal.getAllResources(); for (int i = 0; i < retResources.size(); i++) { IBaseResource newRes = retResources.get(i); if (newRes.getIdElement() == null || newRes.getIdElement().isEmpty()) { diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/api/server/IBundleProviderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/api/server/IBundleProviderTest.java index 66df32cd6c7..82b5e7c09ef 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/api/server/IBundleProviderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/api/server/IBundleProviderTest.java @@ -1,12 +1,15 @@ package ca.uhn.fhir.rest.api.server; +import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.rest.server.SimpleBundleProvider; import com.google.common.collect.Lists; import org.hl7.fhir.instance.model.api.IBaseResource; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.mock; public class IBundleProviderTest { @@ -39,4 +42,19 @@ public class IBundleProviderTest { assertFalse(provider.isEmpty()); } + @Test + void getResources() { + SimpleBundleProvider provider = new SimpleBundleProvider() { + @Override + public Integer size() { + return null; + } + }; + try { + provider.getAllResources(); + fail(); + } catch (ConfigurationException e) { + assertEquals("Attempt to request all resources from an asynchronous search result. The SearchParameterMap for this search probably should have been synchronous.", e.getMessage()); + } + } } From 1424d4891236d4f67f6447339409e9fb432e9383 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 6 May 2021 05:25:38 -0400 Subject: [PATCH 3/3] Avoid NPE in concept mapping (#2637) --- .../jpa/term/TermConceptMappingSvcImpl.java | 107 ++++++++-------- .../r4/FhirResourceDaoR4ConceptMapTest.java | 118 +++++++++++++++++- 2 files changed, 170 insertions(+), 55 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermConceptMappingSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermConceptMappingSvcImpl.java index 5817e396960..8a717afd929 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermConceptMappingSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermConceptMappingSvcImpl.java @@ -202,69 +202,68 @@ public class TermConceptMappingSvcImpl implements ITermConceptMappingSvc { termConceptMap = myConceptMapDao.save(termConceptMap); int codesSaved = 0; - if (theConceptMap.hasGroup()) { - TermConceptMapGroup termConceptMapGroup; - for (ConceptMap.ConceptMapGroupComponent group : theConceptMap.getGroup()) { + TermConceptMapGroup termConceptMapGroup; + for (ConceptMap.ConceptMapGroupComponent group : theConceptMap.getGroup()) { - String groupSource = group.getSource(); - if (isBlank(groupSource)) { - groupSource = source; - } - if (isBlank(groupSource)) { - throw new UnprocessableEntityException("ConceptMap[url='" + theConceptMap.getUrl() + "'] contains at least one group without a value in ConceptMap.group.source"); - } + String groupSource = group.getSource(); + if (isBlank(groupSource)) { + groupSource = source; + } + if (isBlank(groupSource)) { + throw new UnprocessableEntityException("ConceptMap[url='" + theConceptMap.getUrl() + "'] contains at least one group without a value in ConceptMap.group.source"); + } - String groupTarget = group.getTarget(); - if (isBlank(groupTarget)) { - groupTarget = target; - } - if (isBlank(groupTarget)) { - throw new UnprocessableEntityException("ConceptMap[url='" + theConceptMap.getUrl() + "'] contains at least one group without a value in ConceptMap.group.target"); - } + String groupTarget = group.getTarget(); + if (isBlank(groupTarget)) { + groupTarget = target; + } + if (isBlank(groupTarget)) { + throw new UnprocessableEntityException("ConceptMap[url='" + theConceptMap.getUrl() + "'] contains at least one group without a value in ConceptMap.group.target"); + } - termConceptMapGroup = new TermConceptMapGroup(); - termConceptMapGroup.setConceptMap(termConceptMap); - termConceptMapGroup.setSource(groupSource); - termConceptMapGroup.setSourceVersion(group.getSourceVersion()); - termConceptMapGroup.setTarget(groupTarget); - termConceptMapGroup.setTargetVersion(group.getTargetVersion()); - myConceptMapGroupDao.save(termConceptMapGroup); + termConceptMapGroup = new TermConceptMapGroup(); + termConceptMapGroup.setConceptMap(termConceptMap); + termConceptMapGroup.setSource(groupSource); + termConceptMapGroup.setSourceVersion(group.getSourceVersion()); + termConceptMapGroup.setTarget(groupTarget); + termConceptMapGroup.setTargetVersion(group.getTargetVersion()); + termConceptMapGroup = myConceptMapGroupDao.save(termConceptMapGroup); - if (group.hasElement()) { - TermConceptMapGroupElement termConceptMapGroupElement; - for (ConceptMap.SourceElementComponent element : group.getElement()) { - if (isBlank(element.getCode())) { - continue; - } - termConceptMapGroupElement = new TermConceptMapGroupElement(); - termConceptMapGroupElement.setConceptMapGroup(termConceptMapGroup); - termConceptMapGroupElement.setCode(element.getCode()); - termConceptMapGroupElement.setDisplay(element.getDisplay()); - myConceptMapGroupElementDao.save(termConceptMapGroupElement); + if (group.hasElement()) { + TermConceptMapGroupElement termConceptMapGroupElement; + for (ConceptMap.SourceElementComponent element : group.getElement()) { + if (isBlank(element.getCode())) { + continue; + } + termConceptMapGroupElement = new TermConceptMapGroupElement(); + termConceptMapGroupElement.setConceptMapGroup(termConceptMapGroup); + termConceptMapGroupElement.setCode(element.getCode()); + termConceptMapGroupElement.setDisplay(element.getDisplay()); + termConceptMapGroupElement = myConceptMapGroupElementDao.save(termConceptMapGroupElement); - if (element.hasTarget()) { - TermConceptMapGroupElementTarget termConceptMapGroupElementTarget; - for (ConceptMap.TargetElementComponent elementTarget : element.getTarget()) { - if (isBlank(elementTarget.getCode())) { - continue; - } - termConceptMapGroupElementTarget = new TermConceptMapGroupElementTarget(); - termConceptMapGroupElementTarget.setConceptMapGroupElement(termConceptMapGroupElement); - termConceptMapGroupElementTarget.setCode(elementTarget.getCode()); - termConceptMapGroupElementTarget.setDisplay(elementTarget.getDisplay()); - termConceptMapGroupElementTarget.setEquivalence(elementTarget.getEquivalence()); - myConceptMapGroupElementTargetDao.save(termConceptMapGroupElementTarget); + if (element.hasTarget()) { + TermConceptMapGroupElementTarget termConceptMapGroupElementTarget; + for (ConceptMap.TargetElementComponent elementTarget : element.getTarget()) { + if (isBlank(elementTarget.getCode())) { + continue; + } + termConceptMapGroupElementTarget = new TermConceptMapGroupElementTarget(); + termConceptMapGroupElementTarget.setConceptMapGroupElement(termConceptMapGroupElement); + termConceptMapGroupElementTarget.setCode(elementTarget.getCode()); + termConceptMapGroupElementTarget.setDisplay(elementTarget.getDisplay()); + termConceptMapGroupElementTarget.setEquivalence(elementTarget.getEquivalence()); + myConceptMapGroupElementTargetDao.save(termConceptMapGroupElementTarget); - if (++codesSaved % 250 == 0) { - ourLog.info("Have saved {} codes in ConceptMap", codesSaved); - myConceptMapGroupElementTargetDao.flush(); - } + if (++codesSaved % 250 == 0) { + ourLog.info("Have saved {} codes in ConceptMap", codesSaved); + myConceptMapGroupElementTargetDao.flush(); } } } } } } + } else { TermConceptMap existingTermConceptMap = optionalExistingTermConceptMapByUrl.get(); @@ -558,9 +557,9 @@ public class TermConceptMappingSvcImpl implements ITermConceptMappingSvc { private boolean alreadyContainsMapping(List elements, TranslateConceptResult translationMatch) { for (TranslateConceptResult nextExistingElement : elements) { - if (nextExistingElement.getSystem().equals(translationMatch.getSystem())) { - if (nextExistingElement.getSystemVersion().equals(translationMatch.getSystemVersion())) { - if (nextExistingElement.getCode().equals(translationMatch.getCode())) { + if (StringUtils.equals(nextExistingElement.getSystem(), translationMatch.getSystem())) { + if (StringUtils.equals(nextExistingElement.getSystemVersion(), translationMatch.getSystemVersion())) { + if (StringUtils.equals(nextExistingElement.getCode(), translationMatch.getCode())) { return true; } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java index 9c2a5d6e8ee..59df75a50d2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConceptMapTest.java @@ -4,6 +4,9 @@ import ca.uhn.fhir.context.support.IValidationSupport; import ca.uhn.fhir.context.support.TranslateConceptResult; import ca.uhn.fhir.context.support.TranslateConceptResults; import ca.uhn.fhir.jpa.api.model.TranslationRequest; +import ca.uhn.fhir.jpa.dao.data.ITermConceptMapGroupDao; +import ca.uhn.fhir.jpa.dao.data.ITermConceptMapGroupElementDao; +import ca.uhn.fhir.jpa.dao.data.ITermConceptMapGroupElementTargetDao; import ca.uhn.fhir.jpa.entity.TermConceptMap; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.CanonicalType; @@ -16,6 +19,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.transaction.TransactionStatus; @@ -38,7 +42,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4ConceptMapTest.class); - + @Autowired + protected ITermConceptMapGroupDao myConceptMapGroupDao; + @Autowired + protected ITermConceptMapGroupElementDao myConceptMapGroupElementDao; + @Autowired + protected ITermConceptMapGroupElementTargetDao myConceptMapGroupElementTargetDao; private IIdType myConceptMapId; @BeforeEach @@ -597,6 +606,113 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test { }); } + /** + * Make sure we can handle mapping where the system has no explicit version + * specified in the ConceptMap + */ + @Test + public void testTranslateWithReverse_NonVersionedSystem() { + + ConceptMap conceptMap1 = new ConceptMap(); + conceptMap1.setUrl(CM_URL); + + conceptMap1.setSource(new UriType(VS_URL)); + conceptMap1.setTarget(new UriType(VS_URL_2)); + + ConceptMap.ConceptMapGroupComponent group = conceptMap1.addGroup(); + group.setSource(CS_URL); + group.setTarget(CS_URL_2); + + ConceptMap.SourceElementComponent element = group.addElement(); + element.setCode("12345"); + element.setDisplay("Source Code 12345"); + + ConceptMap.TargetElementComponent target = element.addTarget(); + target.setCode("34567"); + target.setDisplay("Target Code 34567"); + target.setEquivalence(ConceptMapEquivalence.EQUAL); + + element = group.addElement(); + element.setCode("23456"); + element.setDisplay("Source Code 23456"); + + target = element.addTarget(); + target.setCode("45678"); + target.setDisplay("Target Code 45678"); + target.setEquivalence(ConceptMapEquivalence.WIDER); + + group = conceptMap1.addGroup(); + group.setSource(CS_URL); + group.setTarget(CS_URL_3); + + element = group.addElement(); + element.setCode("12345"); + element.setDisplay("Source Code 12345"); + + target = element.addTarget(); + target.setCode("56789"); + target.setDisplay("Target Code 56789"); + target.setEquivalence(ConceptMapEquivalence.EQUAL); + + target = element.addTarget(); + target.setCode("67890"); + target.setDisplay("Target Code 67890"); + target.setEquivalence(ConceptMapEquivalence.WIDER); + + group = conceptMap1.addGroup(); + group.setSource(CS_URL_4); + group.setTarget(CS_URL_2); + + element = group.addElement(); + element.setCode("12345"); + element.setDisplay("Source Code 12345"); + + target = element.addTarget(); + target.setCode("34567"); + target.setDisplay("Target Code 34567"); + target.setEquivalence(ConceptMapEquivalence.NARROWER); + + conceptMap1.setId(myConceptMapId); + myConceptMapDao.update(conceptMap1, mySrd).getId().toUnqualifiedVersionless(); + + + runInTransaction(() -> { + /* + * Provided: + * source code + * reverse = true + */ + TranslationRequest translationRequest = new TranslationRequest(); + translationRequest.getCodeableConcept().addCoding() + .setCode("34567"); + translationRequest.setReverse(true); + + TranslateConceptResults translationResult = myConceptMapDao.translate(translationRequest, null); + + assertTrue(translationResult.getResult()); + assertEquals("Matches found", translationResult.getMessage()); + + assertEquals(2, translationResult.getResults().size()); + + TranslateConceptResult translationMatch = translationResult.getResults().get(0); + assertEquals(ConceptMapEquivalence.EQUAL.toCode(), translationMatch.getEquivalence()); + assertEquals("12345", translationMatch.getCode()); + assertEquals("Source Code 12345", translationMatch.getDisplay()); + assertEquals(CS_URL, translationMatch.getSystem()); + assertEquals(null, translationMatch.getSystemVersion()); + assertEquals(CM_URL, translationMatch.getConceptMapUrl()); + + translationMatch = translationResult.getResults().get(1); + assertEquals(ConceptMapEquivalence.NARROWER.toCode(), translationMatch.getEquivalence()); + assertEquals("12345", translationMatch.getCode()); + assertEquals("Source Code 12345", translationMatch.getDisplay()); + assertEquals(CS_URL_4, translationMatch.getSystem()); + assertEquals(null, translationMatch.getSystemVersion()); + assertEquals(CM_URL, translationMatch.getConceptMapUrl()); + }); + + } + @Test public void testTranslateWithReverseHavingEquivalence() { ConceptMap conceptMap = myConceptMapDao.read(myConceptMapId);