From a2c2752586d62493427bedac21d1b31c0f378a2c Mon Sep 17 00:00:00 2001 From: Greg Akins Date: Thu, 2 Jan 2025 12:48:37 -0500 Subject: [PATCH] MAT-7964: Fixing issue where ELMJson translator wasn't updated on Versioning --- .../madie/measure/services/BundleService.java | 36 +----- .../measure/services/ElmToJsonService.java | 44 ++++++++ .../services/QiCoreModelValidator.java | 21 ++-- .../measure/services/VersionService.java | 4 +- .../measure/services/BundleServiceTest.java | 54 +-------- .../services/ElmToJsonServiceTest.java | 105 ++++++++++++++++++ .../measure/services/ModelValidatorTest.java | 21 +++- .../measure/services/VersionServiceTest.java | 2 + 8 files changed, 190 insertions(+), 97 deletions(-) create mode 100644 src/main/java/cms/gov/madie/measure/services/ElmToJsonService.java create mode 100644 src/test/java/cms/gov/madie/measure/services/ElmToJsonServiceTest.java diff --git a/src/main/java/cms/gov/madie/measure/services/BundleService.java b/src/main/java/cms/gov/madie/measure/services/BundleService.java index 30c2c32ea..530bffb50 100644 --- a/src/main/java/cms/gov/madie/measure/services/BundleService.java +++ b/src/main/java/cms/gov/madie/measure/services/BundleService.java @@ -3,12 +3,7 @@ import java.lang.reflect.InvocationTargetException; import cms.gov.madie.measure.dto.PackageDto; -import cms.gov.madie.measure.exceptions.CqlElmTranslationErrorException; -import cms.gov.madie.measure.exceptions.InvalidResourceStateException; -import gov.cms.madie.models.measure.ElmJson; -import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Service; -import org.springframework.util.CollectionUtils; import org.springframework.web.client.RestClientException; import cms.gov.madie.measure.exceptions.BundleOperationException; @@ -27,8 +22,8 @@ public class BundleService { private final FhirServicesClient fhirServicesClient; - private final ElmTranslatorClient elmTranslatorClient; private final ExportRepository exportRepository; + private final ElmToJsonService elmToJsonService; /** * Get the bundle for measure. For draft measure- generate bundle because for draft measure, @@ -42,7 +37,7 @@ public String bundleMeasure(Measure measure, String accessToken, String bundleTy // for draft measures if (measure.getMeasureMetaData().isDraft()) { try { - retrieveElmJson(measure, accessToken); + elmToJsonService.retrieveElmJson(measure, accessToken); return fhirServicesClient.getMeasureBundle(measure, accessToken, bundleType); } catch (RestClientException | IllegalArgumentException ex) { log.error("An error occurred while bundling measure {}", measure.getId(), ex); @@ -66,7 +61,7 @@ public PackageDto getMeasureExport(Measure measure, String accessToken) { // for draft measures if (measure.getMeasureMetaData().isDraft()) { try { - retrieveElmJson(measure, accessToken); + elmToJsonService.retrieveElmJson(measure, accessToken); return PackageDto.builder() .fromStorage(false) .exportPackage(fhirServicesClient.getMeasureBundleExport(measure, accessToken)) @@ -105,29 +100,4 @@ public PackageDto getMeasureExport(Measure measure, String accessToken) { throw new BundleOperationException("Measure", measure.getId(), ex); } } - - protected void retrieveElmJson(Measure measure, String accessToken) { - if (StringUtils.isBlank(measure.getCql())) { - throw new InvalidResourceStateException( - "Measure", measure.getId(), "since there is no associated CQL."); - } - - if (measure.isCqlErrors()) { - throw new InvalidResourceStateException( - "Measure", measure.getId(), "since CQL errors exist."); - } - - if (CollectionUtils.isEmpty(measure.getGroups())) { - throw new InvalidResourceStateException( - "Measure", measure.getId(), "since there are no associated population criteria."); - } - - final ElmJson elmJson = - elmTranslatorClient.getElmJson(measure.getCql(), measure.getModel(), accessToken); - if (elmTranslatorClient.hasErrors(elmJson)) { - throw new CqlElmTranslationErrorException(measure.getMeasureName()); - } - measure.setElmJson(elmJson.getJson()); - measure.setElmXml(elmJson.getXml()); - } } diff --git a/src/main/java/cms/gov/madie/measure/services/ElmToJsonService.java b/src/main/java/cms/gov/madie/measure/services/ElmToJsonService.java new file mode 100644 index 000000000..d0198d75f --- /dev/null +++ b/src/main/java/cms/gov/madie/measure/services/ElmToJsonService.java @@ -0,0 +1,44 @@ +package cms.gov.madie.measure.services; + +import org.apache.commons.lang3.StringUtils; +import org.springframework.stereotype.Service; +import org.springframework.util.CollectionUtils; + +import cms.gov.madie.measure.exceptions.CqlElmTranslationErrorException; +import cms.gov.madie.measure.exceptions.InvalidResourceStateException; +import gov.cms.madie.models.measure.ElmJson; +import gov.cms.madie.models.measure.Measure; +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@AllArgsConstructor +@Service +public class ElmToJsonService { + private final ElmTranslatorClient elmTranslatorClient; + + protected void retrieveElmJson(Measure measure, String accessToken) { + if (StringUtils.isBlank(measure.getCql())) { + throw new InvalidResourceStateException( + "Measure", measure.getId(), "since there is no associated CQL."); + } + + if (measure.isCqlErrors()) { + throw new InvalidResourceStateException( + "Measure", measure.getId(), "since CQL errors exist."); + } + + if (CollectionUtils.isEmpty(measure.getGroups())) { + throw new InvalidResourceStateException( + "Measure", measure.getId(), "since there are no associated population criteria."); + } + + final ElmJson elmJson = + elmTranslatorClient.getElmJson(measure.getCql(), measure.getModel(), accessToken); + if (elmTranslatorClient.hasErrors(elmJson)) { + throw new CqlElmTranslationErrorException(measure.getMeasureName()); + } + measure.setElmJson(elmJson.getJson()); + measure.setElmXml(elmJson.getXml()); + } +} diff --git a/src/main/java/cms/gov/madie/measure/services/QiCoreModelValidator.java b/src/main/java/cms/gov/madie/measure/services/QiCoreModelValidator.java index 4ee4171b8..4b23d08ae 100644 --- a/src/main/java/cms/gov/madie/measure/services/QiCoreModelValidator.java +++ b/src/main/java/cms/gov/madie/measure/services/QiCoreModelValidator.java @@ -50,15 +50,18 @@ public void validateGroups(Measure measure) { } if (measure.getMeasureMetaData() != null && measure.getMeasureMetaData().isDraft()) { - measure.getGroups().forEach( - group -> { - if (StringUtils.isBlank(group.getImprovementNotation())) { - throw new InvalidResourceStateException( - "Measure", measure.getId(), "since there is at least one Population Criteria " + - "with no improvement notation."); - } - } - ); + measure + .getGroups() + .forEach( + group -> { + if (StringUtils.isBlank(group.getImprovementNotation())) { + throw new InvalidResourceStateException( + "Measure", + measure.getId(), + "since there is at least one Population Criteria " + + "with no improvement notation."); + } + }); } } } diff --git a/src/main/java/cms/gov/madie/measure/services/VersionService.java b/src/main/java/cms/gov/madie/measure/services/VersionService.java index 51f7b4b59..a7ea0539d 100644 --- a/src/main/java/cms/gov/madie/measure/services/VersionService.java +++ b/src/main/java/cms/gov/madie/measure/services/VersionService.java @@ -41,7 +41,7 @@ public class VersionService { private final QdmPackageService qdmPackageService; private final ExportService exportService; private final TestCaseSequenceService sequenceService; - private final AppConfigService appConfigService; + private final ElmToJsonService elmToJsonService; public enum VersionValidationResult { VALID, @@ -111,10 +111,12 @@ private Measure versionQdmMeasure( */ private Measure versionFhirMeasure( String versionType, String username, String accessToken, Measure measure) throws Exception { + elmToJsonService.retrieveElmJson(measure, accessToken); Measure upversionedMeasure = version(versionType, username, measure); var measureBundle = fhirServicesClient.getMeasureBundle(upversionedMeasure, accessToken, "export"); saveMeasureBundle(upversionedMeasure, measureBundle, username); + return applyMeasureVersion(versionType, username, upversionedMeasure); } diff --git a/src/test/java/cms/gov/madie/measure/services/BundleServiceTest.java b/src/test/java/cms/gov/madie/measure/services/BundleServiceTest.java index 9a95a8e0f..01f34c955 100644 --- a/src/test/java/cms/gov/madie/measure/services/BundleServiceTest.java +++ b/src/test/java/cms/gov/madie/measure/services/BundleServiceTest.java @@ -2,13 +2,9 @@ import cms.gov.madie.measure.dto.PackageDto; import cms.gov.madie.measure.exceptions.BundleOperationException; -import cms.gov.madie.measure.exceptions.CqlElmTranslationErrorException; -import cms.gov.madie.measure.exceptions.CqlElmTranslationServiceException; -import cms.gov.madie.measure.exceptions.InvalidResourceStateException; import cms.gov.madie.measure.repositories.ExportRepository; import cms.gov.madie.measure.utils.ResourceUtil; import gov.cms.madie.models.common.ModelType; -import gov.cms.madie.models.measure.ElmJson; import gov.cms.madie.models.measure.Export; import gov.cms.madie.models.measure.Group; import gov.cms.madie.models.measure.Measure; @@ -54,6 +50,7 @@ class BundleServiceTest implements ResourceUtil { @Mock private FhirServicesClient fhirServicesClient; @Mock private ElmTranslatorClient elmTranslatorClient; @Mock private ExportRepository exportRepository; + @Mock private ElmToJsonService elmToJsonService; @InjectMocks private BundleService bundleService; @@ -104,26 +101,9 @@ void testBundleMeasureReturnsNullForNullMeasure() { assertThat(output, is(nullValue())); } - @Test - void testBundleMeasureWhenThereIsNoCql() { - measure.setCql(null); - assertThrows( - InvalidResourceStateException.class, - () -> bundleService.bundleMeasure(measure, "Bearer TOKEN", "calculation")); - } - - @Test - void testBundleMeasureWhenThereAreCqlErrors() { - measure.setCqlErrors(true); - assertThrows( - InvalidResourceStateException.class, - () -> bundleService.bundleMeasure(measure, "Bearer TOKEN", "calculation")); - } - @Test void testBundleMeasureThrowsOperationException() { - when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) - .thenReturn(ElmJson.builder().json("{}").xml("<>").build()); + when(fhirServicesClient.getMeasureBundle(any(Measure.class), anyString(), anyString())) .thenThrow(new HttpClientErrorException(HttpStatus.FORBIDDEN)); assertThrows( @@ -131,34 +111,12 @@ void testBundleMeasureThrowsOperationException() { () -> bundleService.bundleMeasure(measure, "Bearer TOKEN", "calculation")); } - @Test - void testBundleMeasureThrowsCqlElmTranslationServiceException() { - when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) - .thenThrow( - new CqlElmTranslationServiceException( - "There was an error calling CQL-ELM translation service", new Exception())); - assertThrows( - CqlElmTranslationServiceException.class, - () -> bundleService.bundleMeasure(measure, "Bearer TOKEN", "calculation")); - } - - @Test - void testBundleMeasureThrowsCqlElmTranslatorExceptionWithErrors() { - when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) - .thenReturn(ElmJson.builder().json("{}").xml("<>").build()); - when(elmTranslatorClient.hasErrors(any(ElmJson.class))).thenReturn(true); - assertThrows( - CqlElmTranslationErrorException.class, - () -> bundleService.bundleMeasure(measure, "Bearer TOKEN", "calculation")); - } - @Test void testBundleMeasureReturnsBundleStringForDraftMeasure() { final String json = "{\"message\": \"GOOD JSON\"}"; when(fhirServicesClient.getMeasureBundle(any(Measure.class), anyString(), anyString())) .thenReturn(json); - when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) - .thenReturn(ElmJson.builder().json("{}").xml("<>").build()); + assertThat(measure.getMeasureMetaData().isDraft(), is(equalTo(true))); String output = bundleService.bundleMeasure(measure, "Bearer TOKEN", "calculation"); assertThat(output, is(equalTo(json))); @@ -256,8 +214,7 @@ void testExportBundleMeasureForDraftMeasure() throws IOException { .developers(List.of(Organization.builder().name("ICF").build())) .build()); measure.setModel("QI-Core v4.1.1"); - when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) - .thenReturn(ElmJson.builder().json("{}").xml("<>").build()); + // doThrow(new // HttpClientErrorException(HttpStatus.FORBIDDEN)).when(fhirServicesClient).getMeasureBundleExport(any(Measure.class), eq(""))) byte[] exportBytes = "TEST".getBytes(); @@ -283,8 +240,7 @@ void testExportBundleMeasureForDraftMeasureThrowsException() throws IOException .developers(List.of(Organization.builder().name("ICF").build())) .build()); measure.setModel("QI-Core v4.1.1"); - when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) - .thenReturn(ElmJson.builder().json("{}").xml("<>").build()); + doThrow(new HttpClientErrorException(HttpStatus.FORBIDDEN)) .when(fhirServicesClient) .getMeasureBundleExport(any(Measure.class), eq("Bearer TOKEN")); diff --git a/src/test/java/cms/gov/madie/measure/services/ElmToJsonServiceTest.java b/src/test/java/cms/gov/madie/measure/services/ElmToJsonServiceTest.java new file mode 100644 index 000000000..bf56a64c1 --- /dev/null +++ b/src/test/java/cms/gov/madie/measure/services/ElmToJsonServiceTest.java @@ -0,0 +1,105 @@ +package cms.gov.madie.measure.services; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import cms.gov.madie.measure.exceptions.CqlElmTranslationServiceException; +import cms.gov.madie.measure.exceptions.InvalidResourceStateException; +import cms.gov.madie.measure.utils.ResourceUtil; +import gov.cms.madie.models.common.ModelType; +import gov.cms.madie.models.common.Version; +import gov.cms.madie.models.measure.Group; +import gov.cms.madie.models.measure.Measure; +import gov.cms.madie.models.measure.MeasureGroupTypes; +import gov.cms.madie.models.measure.MeasureMetaData; +import gov.cms.madie.models.measure.Population; +import gov.cms.madie.models.measure.PopulationType; + +@ExtendWith(MockitoExtension.class) +class ElmToJsonServiceTest implements ResourceUtil { + + @Mock private ElmTranslatorClient elmTranslatorClient; + + @InjectMocks private ElmToJsonService elmToJsonService; + + private Measure measure; + + @BeforeEach + public void setUp() { + Group group = + Group.builder() + .id("xyz-p12r-12ert") + .populationBasis("Encounter") + .measureGroupTypes(List.of(MeasureGroupTypes.PROCESS)) + .populations( + List.of( + new Population( + "id-1", PopulationType.INITIAL_POPULATION, "FactorialOfFive", null, null))) + .groupDescription("Description") + .scoringUnit("test-scoring-unit") + .build(); + + List groups = new ArrayList<>(); + groups.add(group); + String elmJson = getData("/test_elm.json"); + MeasureMetaData metaData = MeasureMetaData.builder().draft(true).build(); + measure = + Measure.builder() + .active(true) + .id("xyz-p13r-13ert") + .cql("test cql") + .model(ModelType.QDM_5_6.getValue()) + .cqlErrors(false) + .elmJson(elmJson) + .measureSetId("IDIDID") + .measureName("MSR01") + .version(new Version(0, 0, 1)) + .groups(groups) + .measureMetaData(metaData) + .createdAt(Instant.now()) + .createdBy("test user") + .lastModifiedAt(Instant.now()) + .lastModifiedBy("test user") + .build(); + } + + @Test + void testBundleMeasureThrowsCqlElmTranslationServiceException() { + + when(elmTranslatorClient.getElmJson(anyString(), anyString(), anyString())) + .thenThrow( + new CqlElmTranslationServiceException( + "There was an error calling CQL-ELM translation service", new Exception())); + assertThrows( + CqlElmTranslationServiceException.class, + () -> elmToJsonService.retrieveElmJson(measure, "calculation")); + } + + @Test + void testBundleMeasureWhenThereAreCqlErrors() { + measure.setCqlErrors(true); + assertThrows( + InvalidResourceStateException.class, + () -> elmToJsonService.retrieveElmJson(measure, "calculation")); + } + + @Test + void testBundleMeasureWhenThereIsNoCql() { + measure.setCql(null); + assertThrows( + InvalidResourceStateException.class, + () -> elmToJsonService.retrieveElmJson(measure, "calculation")); + } +} diff --git a/src/test/java/cms/gov/madie/measure/services/ModelValidatorTest.java b/src/test/java/cms/gov/madie/measure/services/ModelValidatorTest.java index d2e3b1262..38cc83627 100644 --- a/src/test/java/cms/gov/madie/measure/services/ModelValidatorTest.java +++ b/src/test/java/cms/gov/madie/measure/services/ModelValidatorTest.java @@ -276,9 +276,14 @@ void testQdmModelValidatorTestMeasureDoesHaveMeasuretypes() { } @Test - void useQicoreModelValidatorTestDraftMeasureHasNoImprovementNotationInAtLeastOnePopulationCriteria() { + void + useQicoreModelValidatorTestDraftMeasureHasNoImprovementNotationInAtLeastOnePopulationCriteria() { assertNotNull(modelValidatorFactory); - Group group1 = Group.builder().measureGroupTypes(List.of(MeasureGroupTypes.OUTCOME)).improvementNotation("Decreased score indicates improvement").build(); + Group group1 = + Group.builder() + .measureGroupTypes(List.of(MeasureGroupTypes.OUTCOME)) + .improvementNotation("Decreased score indicates improvement") + .build(); Group group2 = Group.builder().measureGroupTypes(List.of(MeasureGroupTypes.OUTCOME)).build(); Measure measure = @@ -292,7 +297,8 @@ void useQicoreModelValidatorTestDraftMeasureHasNoImprovementNotationInAtLeastOne assertTrue(validator instanceof QiCoreModelValidator); try { validator.validateGroups(measure); - fail("Should fail because, since there is at least one Population Criteria with no improvement notation"); + fail( + "Should fail because, since there is at least one Population Criteria with no improvement notation"); } catch (InvalidResourceStateException e) { assertEquals( "Response could not be completed for Measure with ID 1, since there is at least one Population Criteria with no improvement notation.", @@ -301,9 +307,14 @@ void useQicoreModelValidatorTestDraftMeasureHasNoImprovementNotationInAtLeastOne } @Test - void useQicoreModelValidatorTestVersionedMeasureHasNoImprovementNotationInAtLeastOnePopulationCriteria() { + void + useQicoreModelValidatorTestVersionedMeasureHasNoImprovementNotationInAtLeastOnePopulationCriteria() { assertNotNull(modelValidatorFactory); - Group group1 = Group.builder().measureGroupTypes(List.of(MeasureGroupTypes.OUTCOME)).improvementNotation("Decreased score indicates improvement").build(); + Group group1 = + Group.builder() + .measureGroupTypes(List.of(MeasureGroupTypes.OUTCOME)) + .improvementNotation("Decreased score indicates improvement") + .build(); Group group2 = Group.builder().measureGroupTypes(List.of(MeasureGroupTypes.OUTCOME)).build(); Measure measure = diff --git a/src/test/java/cms/gov/madie/measure/services/VersionServiceTest.java b/src/test/java/cms/gov/madie/measure/services/VersionServiceTest.java index f8535c420..4323a65fc 100644 --- a/src/test/java/cms/gov/madie/measure/services/VersionServiceTest.java +++ b/src/test/java/cms/gov/madie/measure/services/VersionServiceTest.java @@ -68,6 +68,8 @@ public class VersionServiceTest { @Mock TestCaseSequenceService sequenceService; @Mock AppConfigService appConfigService; + @Mock ElmToJsonService elmToJsonService; + @InjectMocks VersionService versionService; @Captor private ArgumentCaptor measureCaptor;