Skip to content

Commit

Permalink
Jackson Serialization Issue with Library Refresh Operation (#527)
Browse files Browse the repository at this point in the history
Removed jackson elm serialization deps

- Replaced Jackson ELM serialization with JAXB
- Added shouldApplySoftwareSystemStamp to Library refresh parameters
- Added tests
- Removed tooling-cli directory from tooling module (seemed like that was committed by mistake(?))
  • Loading branch information
c-schuler authored May 1, 2024
1 parent f322c86 commit 1f3a5c8
Show file tree
Hide file tree
Showing 31 changed files with 57 additions and 3,678 deletions.
14 changes: 1 addition & 13 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<maven.compiler.release>11</maven.compiler.release>
<cql.version>3.9.0</cql.version>
<cql.version>3.10.0-SNAPSHOT</cql.version>
<hapi.version>7.0.2</hapi.version>
<spring-boot.version>2.1.5.RELEASE</spring-boot.version>
<slf4j.version>2.0.5</slf4j.version>
Expand Down Expand Up @@ -170,18 +170,6 @@
<version>${cql.version}</version>
</dependency>

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>elm-jackson</artifactId>
<version>${cql.version}</version>
</dependency>

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>model-jackson</artifactId>
<version>${cql.version}</version>
</dependency>

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>elm-jaxb</artifactId>
Expand Down
4 changes: 2 additions & 2 deletions tooling-cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>elm-jackson</artifactId>
<artifactId>elm-jaxb</artifactId>
</dependency>

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>model-jackson</artifactId>
<artifactId>model-jaxb</artifactId>
</dependency>

<dependency>
Expand Down
4 changes: 2 additions & 2 deletions tooling/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,13 @@

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>elm-jackson</artifactId>
<artifactId>elm-jaxb</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>info.cqframework</groupId>
<artifactId>model-jackson</artifactId>
<artifactId>model-jaxb</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class RefreshLibraryArgumentProcessor {
public static final String[] FHIR_VERSION_OPTIONS = {"fv", "fhir-version"};
public static final String[] OUTPUT_ENCODING = {"e", "encoding"};
public static final String[] VERSIONED_OPTIONS = {"v", "versioned"};
public static final String[] SOFTWARE_STAMP = { "ss", "stamp" };

@SuppressWarnings("unused")
public OptionParser build() {
Expand All @@ -36,6 +37,7 @@ public OptionParser build() {
OptionSpecBuilder libraryOutputDirectoryBuilder = parser.acceptsAll(asList(Library_Output_DIRECTORY_OPTIONS),"If omitted, the libraries will overwrite any existing libraries");
OptionSpecBuilder fhirVersionBuilder = parser.acceptsAll(asList(FHIR_VERSION_OPTIONS),"Limited to a single version of FHIR.");
OptionSpecBuilder outputEncodingBuilder = parser.acceptsAll(asList(OUTPUT_ENCODING), "If omitted, output will be generated using JSON encoding.");
OptionSpecBuilder shouldApplySoftwareSystemStampBuilder = parser.acceptsAll(asList(SOFTWARE_STAMP),"Indicates whether refreshed Library resources should be stamped with the 'cqf-tooling' stamp via the cqfm-softwaresystem Extension.");

OptionSpec<String> ini = iniBuilder.withOptionalArg().describedAs("IG ini file");
OptionSpec<String> igCanonicalBasePath = igCanonicalBaseBuilder.withOptionalArg().describedAs("resource canonical base");
Expand All @@ -44,6 +46,7 @@ public OptionParser build() {
OptionSpec<String> libraryOutputDirectoryPath = libraryOutputDirectoryBuilder.withOptionalArg().describedAs("path to the output directory for updated libraries");
OptionSpec<String> fhirVersion = fhirVersionBuilder.withOptionalArg().describedAs("fhir version");
OptionSpec<String> outputEncoding = outputEncodingBuilder.withOptionalArg().describedAs("desired output encoding for resources");
OptionSpec<String> shouldApplySoftwareSystemStamp = shouldApplySoftwareSystemStampBuilder.withOptionalArg().describedAs("Indicates whether refreshed Library resources should be stamped with the 'cqf-tooling' stamp via the cqfm-softwaresystem Extension");

parser.acceptsAll(asList(OPERATION_OPTIONS),"The operation to run.");
parser.acceptsAll(asList(VERSIONED_OPTIONS),"If omitted resources must be uniquely named.");
Expand All @@ -64,6 +67,7 @@ public RefreshLibraryParameters parseAndConvert(String[] args) {
String cqlPath = (String)options.valueOf(CQL_PATH_OPTIONS[0]);
String fhirVersion = (String)options.valueOf(FHIR_VERSION_OPTIONS[0]);
String encoding = (String)options.valueOf(OUTPUT_ENCODING[0]);
String softwareStamp = (String)options.valueOf(SOFTWARE_STAMP[0]);
String libraryPath = (String)options.valueOf(Library_PATH_OPTIONS[0]);
if (libraryPath == null) {
libraryPath = "";
Expand All @@ -77,6 +81,10 @@ public RefreshLibraryParameters parseAndConvert(String[] args) {
outputEncodingEnum = Encoding.parse(encoding.toLowerCase());
}
Boolean versioned = options.has(VERSIONED_OPTIONS[0]);
Boolean shouldApplySoftwareSystemStamp = true;
if ((softwareStamp != null) && softwareStamp.equalsIgnoreCase("false")) {
shouldApplySoftwareSystemStamp = false;
}

RefreshLibraryParameters lp = new RefreshLibraryParameters();
lp.ini = ini;
Expand All @@ -87,6 +95,7 @@ public RefreshLibraryParameters parseAndConvert(String[] args) {
lp.versioned = versioned;
lp.libraryPath = libraryPath;
lp.libraryOutputDirectory = libraryOutputDirectory;
lp.shouldApplySoftwareSystemStamp = shouldApplySoftwareSystemStamp;

return lp;
}
Expand Down
12 changes: 12 additions & 0 deletions tooling/src/test/java/org/opencds/cqf/tooling/RefreshTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opencds.cqf.tooling;

import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -62,6 +63,17 @@ protected void validateCqfmSoftwareSystemExtension(String domainResourcePath) {
}
}

protected void validateNoCqfmSoftwareSystemExtension(String domainResourcePath) {
IBaseResource resource = IOUtils.readResource(domainResourcePath, getFhirContext());
if (resource == null || !(resource instanceof Library)) {
// log error
} else {
DomainResource domainResource = (DomainResource)resource;
Extension softwareSystemExtension = domainResource.getExtensionByUrl(cqfmSoftwareSystemExtensionUrl);
assertNull(softwareSystemExtension);
}
}

protected void copyResourcesToTargetDir(String targetDirectory, String resourceDirectoryPath) throws IOException {
File outputDirectory = new File(targetDirectory);
outputDirectory.mkdirs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,18 @@ protected void runRefresh(String targetDirectory, String libraryResourcePath, St
params.shouldApplySoftwareSystemStamp = true;
getLibraryProcessor().refreshLibraryContent(params);
}

protected void runRefresh(String targetDirectory, String libraryResourcePath, String libraryOutputDirectoryPath,
String cqlResourcePath, boolean versioned, boolean shouldApplySoftwareSystemStamp) {
RefreshLibraryParameters params = new RefreshLibraryParameters();
params.encoding = Encoding.JSON;
params.fhirContext = getFhirContext();
params.libraryPath = libraryResourcePath;
params.libraryOutputDirectory = libraryOutputDirectoryPath;
params.cqlContentPath = cqlResourcePath;
params.ini = targetDirectory + separator + "ig.ini";
params.versioned = versioned;
params.shouldApplySoftwareSystemStamp = shouldApplySoftwareSystemStamp;
getLibraryProcessor().refreshLibraryContent(params);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void setUp() throws Exception {
}

@Test
void testRefreshOverwriteLibraries() throws Exception {
void testRefreshOverwriteLibrariesWithCqfmSoftwareSystemExtension() throws Exception {
String targetDirectory = "target" + separator + "refreshLibraries" + separator + this.resourceDirectory;
copyResourcesToTargetDir(targetDirectory, this.resourceDirectory);

Expand All @@ -48,6 +48,22 @@ void testRefreshOverwriteLibraries() throws Exception {
validateCqfmSoftwareSystemExtension(targetDirectory + libraryPath);
}

@Test
void testRefreshOverwriteLibrariesWithoutCqfmSoftwareSystemExtension() throws Exception {
String targetDirectory = "target" + separator + "refreshLibraries" + separator + this.resourceDirectory;
copyResourcesToTargetDir(targetDirectory, this.resourceDirectory);

String libraryPath = separator + "input" + separator + "resources" + separator + "library" + separator + "library-EXM124_FHIR4-8.2.000.json";
runRefresh(
targetDirectory,
targetDirectory + libraryPath, null,
targetDirectory + separator + "input" + separator + "pagecontent" + separator + "cql" + separator + "EXM124_FHIR4-8.2.000.cql",
false, false
);

validateNoCqfmSoftwareSystemExtension(targetDirectory + libraryPath);
}

@Test
void testRefreshOutputDirectory() throws Exception {
// create a output directory under target directory
Expand Down
Loading

0 comments on commit 1f3a5c8

Please sign in to comment.