Skip to content

Commit

Permalink
#3061 - Fix NPE when Measure refers to non-existent library
Browse files Browse the repository at this point in the history
I applied a fix before the holiday that corrected a category of NPE
errors related to execution against a measure or library resources that
don't exist. It turns out that wasn't the core of the issue 3061. This
goes back and addresses when a valid measure reference is used, but that
Measure refers to a Library resource that does not exist. I also added
some handling that allows resolution of Library resources by reference
in addition to by canonical URL in the registry and also updated the
error handling around retrieving the primary library reference from the
Measure resource.

Signed-off-by: Corey Sanders <corey.thecolonel@gmail.com>
  • Loading branch information
csandersdev committed Jan 12, 2022
1 parent 6915377 commit 0fde33d
Show file tree
Hide file tree
Showing 15 changed files with 461 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ protected static List<Library> loadLibraries(Library fhirLibrary, Set<String> vi
String canonicalURL = canonical.getValue();
if (!visited.contains(canonicalURL)) {
if (isLibraryReference(canonicalURL)) {
// This won't resolve canonical URLs that happen to be ResourceType/ID references as
// we've done for some of the values passed into the CQL-related operations. R4B
// updates the examples to all contain valid canonical URLs and the spec
// is somewhat ambiguous on what you would do if they aren't found in the registry, so
// we are accepting this as ok.
Library dependency = FHIRRegistry.getInstance().getResource(canonicalURL, Library.class);
if (dependency != null) {
if (isLogicLibrary(dependency)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* (C) Copyright IBM Corp. 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
package com.ibm.fhir.ecqm.r4;

import com.ibm.fhir.exception.FHIROperationException;
import com.ibm.fhir.model.resource.Measure;

/**
* Utility methods for working with Measure resources
*/
public class MeasureHelper {
public static String getPrimaryLibraryId(Measure measure) throws FHIROperationException {
String primaryLibraryId = null;
if( measure.getLibrary() != null && measure.getLibrary().size() == 1 ) {
primaryLibraryId = measure.getLibrary().get(0).getValue();
} else {
// See https://hl7.org/fhir/us/cqfmeasures/2021May/StructureDefinition-computable-measure-cqfm.html
throw new FHIROperationException("Measures utilizing CQL SHALL reference one and only one CQL library (and that referenced library MUST be the primary library for the measure)");
}

return primaryLibraryId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import java.time.temporal.TemporalAccessor;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

import org.opencds.cqf.cql.engine.data.DataProvider;
import org.opencds.cqf.cql.engine.execution.Context;
Expand All @@ -33,10 +31,8 @@
import com.ibm.fhir.cql.helpers.LibraryHelper;
import com.ibm.fhir.cql.helpers.ModelHelper;
import com.ibm.fhir.cql.helpers.ParameterMap;
import com.ibm.fhir.cql.translator.CqlTranslationProvider;
import com.ibm.fhir.cql.translator.FHIRLibraryLibrarySourceProvider;
import com.ibm.fhir.cql.translator.impl.InJVMCqlTranslationProvider;
import com.ibm.fhir.ecqm.common.MeasureReportType;
import com.ibm.fhir.ecqm.r4.MeasureHelper;
import com.ibm.fhir.ecqm.r4.R4MeasureEvaluation;
import com.ibm.fhir.exception.FHIROperationException;
import com.ibm.fhir.model.resource.Library;
Expand All @@ -45,9 +41,6 @@
import com.ibm.fhir.model.resource.Parameters.Parameter;
import com.ibm.fhir.model.resource.Patient;
import com.ibm.fhir.model.type.Date;
import com.ibm.fhir.model.type.code.ResourceType;
import com.ibm.fhir.persistence.SingleResourceResult;
import com.ibm.fhir.registry.FHIRRegistry;
import com.ibm.fhir.server.spi.operation.AbstractOperation;
import com.ibm.fhir.server.spi.operation.FHIRResourceHelpers;

Expand Down Expand Up @@ -86,19 +79,16 @@ public RetrieveProvider getRetrieveProvider(FHIRResourceHelpers resourceHelper,
return retrieveProvider;
}

public MeasureReport.Builder doMeasureEvaluation(Measure measure, ZoneOffset zoneOffset, Interval measurementPeriod, String subjectOrPractitionerId,
public MeasureReport.Builder doMeasureEvaluation(FHIRResourceHelpers resourceHelpers, Measure measure, ZoneOffset zoneOffset, Interval measurementPeriod, String subjectOrPractitionerId,
MeasureReportType reportType,
TerminologyProvider termProvider, Map<String, DataProvider> dataProviders) {
TerminologyProvider termProvider, Map<String, DataProvider> dataProviders) throws FHIROperationException {

String primaryLibraryId = MeasureHelper.getPrimaryLibraryId(measure);

int numLibraries = (measure.getLibrary() != null) ? measure.getLibrary().size() : 0;
if (numLibraries != 1) {
throw new IllegalArgumentException(String.format("Unexpected number of libraries '%d' referenced by measure '%s'", numLibraries, measure.getId()));
}

Library primaryLibrary = FHIRRegistry.getInstance().getResource(measure.getLibrary().get(0).getValue(), Library.class);
Library primaryLibrary = OperationHelper.loadLibraryByReference(resourceHelpers, primaryLibraryId);
List<Library> fhirLibraries = LibraryHelper.loadLibraries(primaryLibrary);

List<org.cqframework.cql.elm.execution.Library> cqlLibraries = loadCqlLibraries(fhirLibraries);
List<org.cqframework.cql.elm.execution.Library> cqlLibraries = OperationHelper.loadCqlLibraries(fhirLibraries);
LibraryLoader ll = new InMemoryLibraryLoader(cqlLibraries);

ZonedDateTime zdt = ZonedDateTime.now(zoneOffset);
Expand Down Expand Up @@ -182,62 +172,4 @@ public Interval getMeasurementPeriod(ParameterMap paramMap, ZoneOffset zoneOffse

return new Interval(dtStart, true, dtEnd, true);
}

/**
* Create a library loader that will server up the CQL library content of the
* provided list of FHIR Library resources.
*
* @param libraries
* FHIR library resources
* @return LibraryLoader that will serve the CQL Libraries for the provided FHIR resources
*/
protected LibraryLoader createLibraryLoader(List<Library> libraries) {
List<org.cqframework.cql.elm.execution.Library> result = loadCqlLibraries(libraries);
return new InMemoryLibraryLoader(result);
}

/**
* Load the CQL Library content for each of the provided FHIR Library resources with
* translation as needed for Libraries with CQL attachments and no corresponding
* ELM attachment.
*
* @param libraries
* FHIR Libraries
* @return CQL Libraries
*/
protected List<org.cqframework.cql.elm.execution.Library> loadCqlLibraries(List<Library> libraries) {
FHIRLibraryLibrarySourceProvider sourceProvider = new FHIRLibraryLibrarySourceProvider(libraries);
CqlTranslationProvider translator = new InJVMCqlTranslationProvider(sourceProvider);

List<org.cqframework.cql.elm.execution.Library> result =
libraries.stream().flatMap(fl -> LibraryHelper.loadLibrary(translator, fl).stream()).filter(Objects::nonNull).collect(Collectors.toList());
return result;
}

protected Measure loadMeasureByReference(FHIRResourceHelpers resourceHelper, String reference) throws FHIROperationException {
Measure measure;
if( reference.startsWith("Measure/") ) {
String resourceId = reference.substring("Measure/".length());
measure = loadMeasureById(resourceHelper, resourceId);
} else {
measure = FHIRRegistry.getInstance().getResource(reference, Measure.class);
if( measure == null ) {
// At this point an error will be thrown if not found
measure = loadMeasureById(resourceHelper, reference);
}
}

return measure;
}

protected Measure loadMeasureById(FHIRResourceHelpers resourceHelper, String reference) throws FHIROperationException {
Measure measure;
try {
SingleResourceResult<?> readResult = resourceHelper.doRead(ResourceType.MEASURE.getValue(), reference, true, false, null);
measure = (Measure) readResult.getResource();
} catch (Exception ex) {
throw new FHIROperationException("Failed to resolve resource " + reference, ex);
}
return measure;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,16 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
}
}

protected Bundle processAllMeasures(FHIRBundleCursor cursor, String subject, ZoneOffset zoneOffset, Interval measurementPeriod, FHIRResourceHelpers resourceHelper, TerminologyProvider termProvider, Map<String,DataProvider> dataProviders) {
protected Bundle processAllMeasures(FHIRBundleCursor cursor, String subject, ZoneOffset zoneOffset, Interval measurementPeriod, FHIRResourceHelpers resourceHelper, TerminologyProvider termProvider, Map<String,DataProvider> dataProviders) throws FHIROperationException {
Bundle.Builder reports = Bundle.builder().type(BundleType.COLLECTION);

AtomicInteger count = new AtomicInteger(0);
cursor.forEach(resource -> {
for(Object resource : cursor) {
Measure measure = (Measure) resource;
MeasureReport report = doMeasureEvaluation(measure, zoneOffset, measurementPeriod, subject, MeasureReportType.INDIVIDUAL, termProvider, dataProviders).build();
MeasureReport report = doMeasureEvaluation(resourceHelper, measure, zoneOffset, measurementPeriod, subject, MeasureReportType.INDIVIDUAL, termProvider, dataProviders).build();
reports.entry( Bundle.Entry.builder().resource(report).build() );
count.incrementAndGet();
});
}

reports.total(UnsignedInt.of(count.get()));
return reports.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext

Measure measure = null;
if (operationContext.getType().equals(FHIROperationContext.Type.INSTANCE)) {
measure = loadMeasureById(resourceHelper, logicalId);
measure = OperationHelper.loadMeasureById(resourceHelper, logicalId);
} else if (operationContext.getType().equals(FHIROperationContext.Type.RESOURCE_TYPE)) {
Parameter param = paramMap.getSingletonParameter(PARAM_IN_MEASURE);
String reference = ((com.ibm.fhir.model.type.String) param.getValue()).getValue();
measure = loadMeasureByReference(resourceHelper, reference);
measure = OperationHelper.loadMeasureByReference(resourceHelper, reference);
} else {
assert false;
}
Expand All @@ -81,7 +81,7 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext

Map<String, DataProvider> dataProviders = DataProviderFactory.createDataProviders(retrieveProvider);

MeasureReport.Builder report = doMeasureEvaluation(measure, zoneOffset, measurementPeriod, subjectOrPractitionerId, reportType, termProvider, dataProviders);
MeasureReport.Builder report = doMeasureEvaluation(resourceHelper, measure, zoneOffset, measurementPeriod, subjectOrPractitionerId, reportType, termProvider, dataProviders);

return FHIROperationUtil.getOutputParameters(PARAM_OUT_RETURN, report.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.List;

import com.ibm.fhir.cql.helpers.LibraryHelper;
import com.ibm.fhir.ecqm.r4.MeasureHelper;
import com.ibm.fhir.exception.FHIROperationException;
import com.ibm.fhir.model.resource.Library;
import com.ibm.fhir.model.resource.Measure;
Expand Down Expand Up @@ -47,7 +48,8 @@ public Parameters doInvoke(FHIROperationContext operationContext, Class<? extend
throw new IllegalArgumentException(String.format("Unexpected number of libraries '%d' referenced by measure '%s'", numLibraries, measure.getId()));
}

Library primaryLibrary = FHIRRegistry.getInstance().getResource(measure.getLibrary().get(0).getValue(), Library.class);
String primaryLibraryId = MeasureHelper.getPrimaryLibraryId(measure);
Library primaryLibrary = OperationHelper.loadLibraryByReference(resourceHelper, primaryLibraryId);
List<Library> fhirLibraries = LibraryHelper.loadLibraries(primaryLibrary);

RelatedArtifact related = RelatedArtifact.builder().type(RelatedArtifactType.DEPENDS_ON).resource(measure.getLibrary().get(0)).build();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* (C) Copyright IBM Corp. 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
package com.ibm.fhir.operation.cqf;

import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

import org.opencds.cqf.cql.engine.execution.InMemoryLibraryLoader;
import org.opencds.cqf.cql.engine.execution.LibraryLoader;

import com.ibm.fhir.cql.helpers.LibraryHelper;
import com.ibm.fhir.cql.translator.CqlTranslationProvider;
import com.ibm.fhir.cql.translator.FHIRLibraryLibrarySourceProvider;
import com.ibm.fhir.cql.translator.impl.InJVMCqlTranslationProvider;
import com.ibm.fhir.exception.FHIROperationException;
import com.ibm.fhir.model.resource.Library;
import com.ibm.fhir.model.resource.Measure;
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.model.type.code.ResourceType;
import com.ibm.fhir.persistence.SingleResourceResult;
import com.ibm.fhir.registry.FHIRRegistry;
import com.ibm.fhir.server.spi.operation.FHIRResourceHelpers;

public class OperationHelper {
/**
* Create a library loader that will server up the CQL library content of the
* provided list of FHIR Library resources.
*
* @param libraries
* FHIR library resources
* @return LibraryLoader that will serve the CQL Libraries for the provided FHIR resources
*/
public static LibraryLoader createLibraryLoader(List<Library> libraries) {
List<org.cqframework.cql.elm.execution.Library> result = loadCqlLibraries(libraries);
return new InMemoryLibraryLoader(result);
}

/**
* Load the CQL Library content for each of the provided FHIR Library resources with
* translation as needed for Libraries with CQL attachments and no corresponding
* ELM attachment.
*
* @param libraries
* FHIR Libraries
* @return CQL Libraries
*/
public static List<org.cqframework.cql.elm.execution.Library> loadCqlLibraries(List<Library> libraries) {
FHIRLibraryLibrarySourceProvider sourceProvider = new FHIRLibraryLibrarySourceProvider(libraries);
CqlTranslationProvider translator = new InJVMCqlTranslationProvider(sourceProvider);

List<org.cqframework.cql.elm.execution.Library> result =
libraries.stream().flatMap(fl -> LibraryHelper.loadLibrary(translator, fl).stream()).filter(Objects::nonNull).collect(Collectors.toList());
return result;
}

public static Measure loadMeasureByReference(FHIRResourceHelpers resourceHelper, String reference) throws FHIROperationException {
return loadResourceByReference(resourceHelper, ResourceType.MEASURE, Measure.class, reference);
}


public static Measure loadMeasureById(FHIRResourceHelpers resourceHelper, String reference) throws FHIROperationException {
return loadResourceById(resourceHelper, ResourceType.MEASURE, reference);
}

public static Library loadLibraryByReference(FHIRResourceHelpers resourceHelper, String reference) throws FHIROperationException {
return loadResourceByReference(resourceHelper, ResourceType.LIBRARY, Library.class, reference);
}


public static Library loadLibraryById(FHIRResourceHelpers resourceHelper, String reference) throws FHIROperationException {
return loadResourceById(resourceHelper, ResourceType.LIBRARY, reference);
}

@SuppressWarnings("unchecked")
public static <T extends Resource> T loadResourceByReference(FHIRResourceHelpers resourceHelper, ResourceType resourceType, Class<T> resourceClass, String reference) throws FHIROperationException {
T resource;
int pos = reference.indexOf('/');
if( pos == -1 || reference.startsWith(resourceType.getValue() + "/") ) {
String resourceId = reference;
if( pos > -1 ) {
resourceId = reference.substring(pos + 1);
}
resource = (T) loadResourceById(resourceHelper, resourceType, resourceId);
} else {
resource = FHIRRegistry.getInstance().getResource(reference, resourceClass);
if( resource == null ) {
throw new FHIROperationException(String.format("Failed to resolve %s resource \"%s\"", resourceType.getValue(), reference));
}
}

return resource;
}

@SuppressWarnings("unchecked")
public static <T extends Resource> T loadResourceById(FHIRResourceHelpers resourceHelper, ResourceType resourceType, String reference) throws FHIROperationException {
T resource;
try {
SingleResourceResult<?> readResult = resourceHelper.doRead(resourceType.getValue(), reference, true, false, null);
resource = (T) readResult.getResource();
} catch (Exception ex) {
throw new FHIROperationException(String.format("Failed to resolve %s resource \"%s\"", resourceType.getValue(), reference), ex);
}
return resource;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ public Parameters runTest(FHIROperationContext context, Class<? extends Resource
return outParameters;
}
}

protected Library initializeLibraries(FHIRRegistry mockRegistry, FHIRResourceHelpers resourceHelper) throws Exception {
return initializeLibraries(mockRegistry, resourceHelper, true);
}

protected Library initializeLibraries(FHIRRegistry mockRegistry, FHIRResourceHelpers resourceHelper, boolean exists) throws Exception {
String cql = CqlBuilder.builder()
.library("Test", "1.0.0")
.using("FHIR", Constants.FHIR_VERSION)
Expand All @@ -96,9 +100,11 @@ protected Library initializeLibraries(FHIRRegistry mockRegistry, FHIRResourceHel
Library primaryLibrary = getPrimaryLibrary(cql, modelInfo, fhirHelpers, sde);
List<Library> fhirLibraries = Arrays.asList(primaryLibrary, getSupplementalDataElementsLibrary(), getFHIRHelpers(), getFHIRModelInfo());

when(resourceHelper.doRead(eq("Library"), eq(primaryLibrary.getId()), anyBoolean(), anyBoolean(), any())).thenAnswer(x -> TestHelper.asResult(primaryLibrary));

fhirLibraries.stream().forEach( l -> when(mockRegistry.getResource( canonical(l.getUrl(), l.getVersion()).getValue(), Library.class )).thenReturn(l) );
if( exists ) {
when(resourceHelper.doRead(eq("Library"), eq(primaryLibrary.getId()), anyBoolean(), anyBoolean(), any())).thenAnswer(x -> TestHelper.asResult(primaryLibrary));

fhirLibraries.stream().forEach( l -> when(mockRegistry.getResource( canonical(l.getUrl(), l.getVersion()).getValue(), Library.class )).thenReturn(l) );
}
return primaryLibrary;
}

Expand Down
Loading

0 comments on commit 0fde33d

Please sign in to comment.