Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure most cases of IllegalArgumentException for measures and caregaps are converted to FHIR InvalidRequestException #599

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.opencds.cqf.fhir.cr.measure.common;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import java.util.List;
import java.util.Objects;
import org.hl7.elm.r1.VersionedIdentifier;
Expand Down Expand Up @@ -99,8 +100,10 @@ protected MeasureReportType evalTypeToReportType(MeasureEvalType measureEvalType
case POPULATION:
return MeasureReportType.SUMMARY;
default:
throw new IllegalArgumentException(
String.format("Unsupported MeasureEvalType: %s", measureEvalType.toCode()));
throw new InvalidRequestException(String.format(
"Unsupported MeasureEvalType: %s for Measure: %s", measureEvalType.toCode(), getMeasureUrl()));
}
}

protected abstract String getMeasureUrl();
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static org.opencds.cqf.fhir.cr.measure.common.MeasurePopulationType.TOTALDENOMINATOR;
import static org.opencds.cqf.fhir.cr.measure.common.MeasurePopulationType.TOTALNUMERATOR;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
Expand Down Expand Up @@ -89,7 +90,7 @@ public MeasureDef evaluate(
Objects.requireNonNull(subjectIds, "subjectIds is a required argument");

// measurementPeriod is not required, because it's often defaulted in CQL
this.setMeasurementPeriod(measurementPeriod);
setMeasurementPeriod(measureDef, measurementPeriod);

final ZonedDateTime zonedDateTime = getZonedTimeZoneForEval(measurementPeriod);

Expand All @@ -110,8 +111,9 @@ public MeasureDef evaluate(
measureDef, MeasureReportType.SUMMARY, subjectIds, versionedIdentifier, zonedDateTime);
default:
// never hit because this value is set upstream
throw new IllegalArgumentException(
String.format("Unsupported Measure Evaluation type: %s", measureEvalType.getDisplay()));
throw new InvalidRequestException(String.format(
"Unsupported Measure Evaluation type: %s for MeasureDef: %s",
measureEvalType.getDisplay(), measureDef.url()));
}
}

Expand Down Expand Up @@ -145,7 +147,7 @@ protected ParameterDef getMeasurementPeriodParameterDef() {
return null;
}

protected void setMeasurementPeriod(Interval measurementPeriod) {
protected void setMeasurementPeriod(MeasureDef measureDef, Interval measurementPeriod) {
ParameterDef pd = this.getMeasurementPeriodParameterDef();
if (pd == null) {
logger.warn(
Expand Down Expand Up @@ -183,7 +185,7 @@ protected void setMeasurementPeriod(Interval measurementPeriod) {

NamedTypeSpecifier pointType = (NamedTypeSpecifier) intervalTypeSpecifier.getPointType();
String targetType = pointType.getName().getLocalPart();
Interval convertedPeriod = convertInterval(measurementPeriod, targetType);
Interval convertedPeriod = convertInterval(measureDef, measurementPeriod, targetType);

this.context.getState().setParameter(null, this.measurementPeriodParameterName, convertedPeriod);
}
Expand Down Expand Up @@ -225,7 +227,7 @@ private static DateTime cloneDateTimeWithUtc(DateTime dateTime) {
return newDateTime;
}

protected Interval convertInterval(Interval interval, String targetType) {
protected Interval convertInterval(MeasureDef measureDef, Interval interval, String targetType) {
String sourceTypeQualified = interval.getPointType().getTypeName();
String sourceType =
sourceTypeQualified.substring(sourceTypeQualified.lastIndexOf(".") + 1, sourceTypeQualified.length());
Expand All @@ -243,9 +245,9 @@ protected Interval convertInterval(Interval interval, String targetType) {
interval.getHighClosed());
}

throw new IllegalArgumentException(String.format(
"The interval type of %s did not match the expected type of %s and no conversion was possible.",
sourceType, targetType));
throw new InvalidRequestException(String.format(
"The interval type of %s did not match the expected type of %s and no conversion was possible for MeasureDef: %s.",
sourceType, targetType, measureDef.url()));
}

protected Date truncateDateTime(DateTime dateTime) {
Expand All @@ -258,7 +260,7 @@ protected Pair<String, String> getSubjectTypeAndId(String subjectId) {
String[] subjectIdParts = subjectId.split("/");
return Pair.of(subjectIdParts[0], subjectIdParts[1]);
} else {
throw new IllegalArgumentException(String.format(
throw new InvalidRequestException(String.format(
"Unable to determine Subject type for id: %s. SubjectIds must be in the format {subjectType}/{subjectId} (e.g. Patient/123)",
subjectId));
}
Expand Down Expand Up @@ -361,7 +363,7 @@ protected Object evaluateObservationCriteria(
criteriaExpression, this.context.getState().getCurrentLibrary());

if (!(ed instanceof FunctionDef)) {
throw new IllegalArgumentException(String.format(
throw new InvalidRequestException(String.format(
"Measure observation %s does not reference a function definition", criteriaExpression));
}

Expand Down Expand Up @@ -622,7 +624,8 @@ protected void evaluateStratifiers(
}

if (resultIter.hasNext()) {
throw new IllegalArgumentException("stratifiers may not return multiple values");
throw new InvalidRequestException(
"stratifiers may not return multiple values for subjectId: " + subjectId);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package org.opencds.cqf.fhir.cr.measure.common;

public interface MeasureReportScorer<MeasureReportT> {
public void score(MeasureDef measureDef, MeasureReportT measureReport);
void score(String measureUrl, MeasureDef measureDef, MeasureReportT measureReport);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static org.opencds.cqf.fhir.cr.measure.constant.MeasureReportConstants.IMPROVEMENT_NOTATION_SYSTEM_DECREASE;
import static org.opencds.cqf.fhir.cr.measure.constant.MeasureReportConstants.IMPROVEMENT_NOTATION_SYSTEM_INCREASE;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -62,7 +63,8 @@ public MeasureDef build(Measure measure) {
// This might not be the best behavior, but we want to ensure that the behavior is the same
// between versions
if (measureScoring == null && measure.hasGroup()) {
throw new IllegalArgumentException("MeasureScoring must be specified on Measure");
throw new InvalidRequestException(
String.format("MeasureScoring must be specified on Measure: %s", measure.getUrl()));
}
List<GroupDef> groups = new ArrayList<>();
for (MeasureGroupComponent group : measure.getGroup()) {
Expand Down Expand Up @@ -160,14 +162,15 @@ private void checkId(Resource r) {
}
}

private void validateImprovementNotationCode(CodeDef improvementNotation) {
private void validateImprovementNotationCode(Measure measure, CodeDef improvementNotation) {
if (improvementNotation != null) {
var code = improvementNotation.code();
boolean hasValidCode = IMPROVEMENT_NOTATION_SYSTEM_INCREASE.equals(code)
|| IMPROVEMENT_NOTATION_SYSTEM_DECREASE.equals(code);
if (!hasValidCode) {
throw new IllegalArgumentException(String.format(
"ImprovementNotation Coding has invalid code: %s, combination for Measure.", code));
"ImprovementNotation Coding has invalid code: %s, combination for Measure: %s",
code, measure.getUrl()));
}
}
}
Expand All @@ -186,7 +189,7 @@ public CodeDef getMeasureBasis(Measure measure) {
public CodeDef getMeasureImprovementNotation(Measure measure) {
if (measure.hasImprovementNotation()) {
var impNot = new CodeDef(null, measure.getImprovementNotation());
validateImprovementNotationCode(impNot);
validateImprovementNotationCode(measure, impNot);
// Dstu3 only has a string defined for improvementNotation
return impNot;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,9 @@ public Dstu3MeasureEvaluation(
libraryEngine,
versionIdentifier);
}

@Override
protected String getMeasureUrl() {
return measure.getUrl();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.opencds.cqf.fhir.cr.measure.dstu3;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -73,7 +74,7 @@ protected MeasureReport evaluateMeasure(
Parameters parameters) {

if (!measure.hasLibrary()) {
throw new IllegalArgumentException(
throw new InvalidRequestException(
String.format("Measure %s does not have a primary library specified", measure.getUrl()));
}

Expand Down Expand Up @@ -148,7 +149,7 @@ protected MeasureReportType evalTypeToReportType(MeasureEvalType measureEvalType
case POPULATION:
return MeasureReportType.SUMMARY;
default:
throw new IllegalArgumentException(
throw new InvalidRequestException(
String.format("Unsupported MeasureEvalType: %s", measureEvalType.toCode()));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.opencds.cqf.fhir.cr.measure.dstu3;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import com.google.common.collect.Lists;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -94,7 +95,7 @@ public MeasureReport build(
buildGroups(measure, measureDef);
processSdes(measure, measureDef, subjectIds);

this.measureReportScorer.score(measureDef, this.report);
this.measureReportScorer.score(measure.getUrl(), measureDef, this.report);

// Only add evaluated resources to individual reports
if (measureReportType == MeasureReportType.INDIVIDUAL) {
Expand All @@ -110,6 +111,7 @@ public MeasureReport build(

protected void buildGroups(Measure measure, MeasureDef measureDef) {
if (measure.getGroup().size() != measureDef.groups().size()) {
// This is not a user error:
throw new IllegalArgumentException(
"The Measure has a different number of groups defined than the MeasureDef");
}
Expand Down Expand Up @@ -138,6 +140,7 @@ protected void buildGroup(
// TOTAL-DENOMINATOR), and will not be added to group populations.
// Subtracting '2' from groupDef to balance with Measure defined Groups
if (measureGroup.getPopulation().size() != (groupDef.populations().size() - 2)) {
// This is not a user error:
throw new IllegalArgumentException(
"The MeasureGroup has a different number of populations defined than the GroupDef");
}
Expand Down Expand Up @@ -485,7 +488,8 @@ protected Period getPeriod(Interval measurementPeriod) {
Date dEnd = (Date) measurementPeriod.getEnd();
return new Period().setStart(dStart.toJavaDate()).setEnd(dEnd.toJavaDate());
} else {
throw new IllegalArgumentException("Measurement period should be an interval of CQL DateTime or Date");
throw new InvalidRequestException(
"Measurement period should be an interval of CQL DateTime or Date: " + measurementPeriod);
}
}

Expand Down Expand Up @@ -687,7 +691,7 @@ public String getKey() {
}

if (key == null) {
throw new IllegalArgumentException(String.format("found a null key for the wrapped value: %s", value));
throw new InvalidRequestException(String.format("found a null key for the wrapped value: %s", value));
}

return key;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.opencds.cqf.fhir.cr.measure.dstu3;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import java.util.Optional;
import org.hl7.fhir.dstu3.model.MeasureReport;
import org.hl7.fhir.dstu3.model.MeasureReport.MeasureReportGroupComponent;
Expand All @@ -15,10 +16,12 @@
public class Dstu3MeasureReportScorer extends BaseMeasureReportScorer<MeasureReport> {

@Override
public void score(MeasureDef measureDef, MeasureReport measureReport) {
public void score(String measureUrl, MeasureDef measureDef, MeasureReport measureReport) {
// Measure Def Check
if (measureDef == null) {
throw new IllegalArgumentException("MeasureDef is required in order to score a Measure.");
// This isn't due to user error
throw new IllegalArgumentException(
"MeasureDef is required in order to score a Measure for Measure: " + measureUrl);
}
// No groups, nothing to score
if (measureReport.getGroup().isEmpty()) {
Expand All @@ -29,8 +32,9 @@ public void score(MeasureDef measureDef, MeasureReport measureReport) {

// validate scoring
if (measureScoring == null) {
throw new IllegalArgumentException(
"Measure does not have a scoring methodology defined. Add a \"scoring\" property to the measure definition or the group definition.");
throw new InvalidRequestException(String.format(
"Measure: %s does not have a scoring methodology defined. Add a \"scoring\" property to the measure definition or the group definition.",
measureUrl));
}

for (MeasureReportGroupComponent mrgc : measureReport.getGroup()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@
import static org.opencds.cqf.fhir.cr.measure.constant.MeasureReportConstants.RESOURCE_TYPE_ORGANIZATION;
import static org.opencds.cqf.fhir.utility.Resources.newResource;

import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import jakarta.annotation.Nullable;
import java.time.ZonedDateTime;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.hl7.fhir.r4.model.CanonicalType;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Measure;
import org.hl7.fhir.r4.model.Measure.MeasureGroupComponent;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Parameters;
import org.hl7.fhir.r4.model.PrimitiveType;
import org.hl7.fhir.r4.model.Resource;
import org.opencds.cqf.fhir.api.Repository;
import org.opencds.cqf.fhir.cr.measure.CareGapsProperties;
Expand Down Expand Up @@ -80,7 +83,7 @@ public Parameters getCareGapsReport(
checkConfigurationReferences();

// validate required parameter values
checkValidStatusCode(r4CareGapsParams.getStatus());
checkValidStatusCode(measure, r4CareGapsParams.getStatus());
List<Measure> measures = resolveMeasure(r4CareGapsParams.getMeasure());
measureCompatibilityCheck(measures);

Expand Down Expand Up @@ -165,16 +168,17 @@ protected Parameters initializeResult() {
return newResource(Parameters.class, "care-gaps-report-" + UUID.randomUUID());
}

protected void checkValidStatusCode(List<String> statuses) {
protected void checkValidStatusCode(List<Either3<IdType, String, CanonicalType>> measure, List<String> statuses) {
r4MeasureServiceUtils.listThrowIllegalArgumentIfEmpty(statuses, "status");

for (String status : statuses) {
if (!CareGapsStatusCode.CLOSED_GAP.toString().equals(status)
&& !CareGapsStatusCode.OPEN_GAP.toString().equals(status)
&& !CareGapsStatusCode.NOT_APPLICABLE.toString().equals(status)
&& !CareGapsStatusCode.PROSPECTIVE_GAP.toString().equals(status)) {
throw new IllegalArgumentException(
String.format("CareGap status parameter: %s, is not an accepted value", status));
throw new InvalidRequestException(String.format(
"CareGap status parameter: %s, is not an accepted value for Measure: %s",
status, printEithers(measure)));
}
}
}
Expand All @@ -195,7 +199,7 @@ protected void checkMeasureBasis(Measure measure) {

for (GroupDef groupDef : measureDef.groups()) {
if (!groupDef.isBooleanBasis()) {
throw new IllegalArgumentException(msg);
throw new InvalidRequestException(msg);
}
}
}
Expand All @@ -211,8 +215,9 @@ protected void checkMeasureGroupComponents(Measure measure) {
for (MeasureGroupComponent group : measure.getGroup()) {
if (measure.getGroup().size() > 1
&& (group.getId() == null || group.getId().isEmpty())) {
throw new IllegalArgumentException(
"Multi-rate Measure resources require unique 'id' for GroupComponents to be populated.");
throw new InvalidRequestException(
"Multi-rate Measure resources require unique 'id' for GroupComponents to be populated for Measure: "
+ measure.getUrl());
}
}
}
Expand All @@ -223,9 +228,9 @@ protected void checkMeasureScoringType(Measure measure) {
for (MeasureScoring measureScoringType : scoringTypes) {
if (!MeasureScoring.PROPORTION.equals(measureScoringType)
&& !MeasureScoring.RATIO.equals(measureScoringType)) {
throw new IllegalArgumentException(String.format(
"MeasureScoring type: %s, is not an accepted Type for care-gaps service",
measureScoringType.getDisplay()));
throw new InvalidRequestException(String.format(
"MeasureScoring type: %s, is not an accepted Type for care-gaps service for Measure: %s",
measureScoringType.getDisplay(), measure.getUrl()));
}
}
}
Expand All @@ -238,4 +243,10 @@ protected void checkConfigurationReferences() {
careGapsProperties.getCareGapsCompositionSectionAuthor(),
CareGapsConstants.CARE_GAPS_SECTION_AUTHOR_KEY);
}

private static List<String> printEithers(List<Either3<IdType, String, CanonicalType>> either) {
return either.stream()
.map(x -> x.fold(IdType::getIdPart, Function.identity(), PrimitiveType::asStringValue))
.collect(Collectors.toList());
}
}
Loading