Enable or disable support for JSON fhir_comments depending on the release. Move HapiFhirStorageResponseCode.json to the module in which it's used (#4374)

* First draft of disabling JsonParser "fhir_comment" code.  Unit test assertion that JSON is not generated with "fhir_comment" in the resulting resource String.

* Surround fhir_comments logic with conditional logic.  Set this to false while awaiting final word from stakeholders.  Add unit tests for STU3 and STU2 proving that fhir_comments do not get generated.

* Add conditional logic for JsonParser and fhir_comments and adjust unit tests accordingly.  Also, move the HapiFhirStorageResponseCode.json file to the same module as DefaultProfileValidationSupport, namely, the code that actually reads it.

* Add changelog.

* Add missing change to JsonParser that I thought I had pushed.   Fix changelog.  Fix unit tests assertions to be clearer.

* JsonParser:  Cache value of isSupportsFhirComment.

* Support version DSTU2_1 as well and add a new unit test.

* Fix pipeline unit test failures.
This commit is contained in:
Luke deGruchy 2022-12-19 12:31:21 -05:00 committed by GitHub
parent 81e313b02e
commit fd94e16ee0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 173 additions and 92 deletions

View File

@ -329,7 +329,7 @@ public class DefaultProfileValidationSupport implements IValidationSupport {
// Load built-in system
if (myCtx.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.DSTU3)) {
String storageCodeEnum = ClasspathUtil.loadResource("org/hl7/fhir/common/hapi/validation/support/HapiFhirStorageResponseCode.json");
String storageCodeEnum = ClasspathUtil.loadResource("ca/uhn/fhir/context/support/HapiFhirStorageResponseCode.json");
IBaseResource storageCodeCodeSystem = myCtx.newJsonParser().setParserErrorHandler(new LenientErrorHandler()).parseResource(storageCodeEnum);
String url = myCtx.newTerser().getSinglePrimitiveValueOrNull(storageCodeCodeSystem, "url");
theCodeSystems.put(url, storageCodeCodeSystem);

View File

@ -913,6 +913,11 @@ public abstract class BaseParser implements IParser {
return true;
}
protected boolean isFhirVersionLessThanOrEqualTo(FhirVersionEnum theFhirVersionEnum) {
final FhirVersionEnum apiFhirVersion = myContext.getVersion().getVersion();
return theFhirVersionEnum == apiFhirVersion || apiFhirVersion.isOlderThan(theFhirVersionEnum);
}
class ChildNameAndDef {
private final BaseRuntimeElementDefinition<?> myChildDef;

View File

@ -26,6 +26,7 @@ import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
import ca.uhn.fhir.context.BaseRuntimeElementDefinition.ChildTypeEnum;
import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.FhirVersionEnum;
import ca.uhn.fhir.context.RuntimeChildContainedResources;
import ca.uhn.fhir.context.RuntimeChildDeclaredExtensionDefinition;
import ca.uhn.fhir.context.RuntimeChildNarrativeDefinition;
@ -97,6 +98,8 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
private boolean myPrettyPrint;
private Boolean myIsSupportsFhirComment;
/**
* Do not use this constructor, the recommended way to obtain a new instance of the JSON parser is to invoke
* {@link FhirContext#newJsonParser()}.
@ -600,11 +603,13 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
write(theEventWriter, "id", elementId);
}
if (nextComments != null && !nextComments.isEmpty()) {
beginArray(theEventWriter, "fhir_comments");
for (String next : nextComments) {
theEventWriter.write(next);
if (isSupportsFhirComment()) {
beginArray(theEventWriter, "fhir_comments");
for (String next : nextComments) {
theEventWriter.write(next);
}
theEventWriter.endArray();
}
theEventWriter.endArray();
}
writeExtensionsAsDirectChild(theResource, theEventWriter, theResDef, heldExts, heldModExts, theEncodeContext, theContainedResource);
if (inArray) {
@ -622,6 +627,13 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
}
}
private boolean isSupportsFhirComment() {
if (myIsSupportsFhirComment == null) {
myIsSupportsFhirComment = isFhirVersionLessThanOrEqualTo(FhirVersionEnum.DSTU2_1);
}
return myIsSupportsFhirComment;
}
private boolean isMultipleCardinality(int maxCardinality) {
return maxCardinality > 1 || maxCardinality == Child.MAX_UNLIMITED;
}
@ -1190,14 +1202,16 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
}
private void parseFhirComments(BaseJsonLikeValue theObject, ParserState<?> theState) {
if (theObject.isArray()) {
BaseJsonLikeArray comments = theObject.getAsArray();
for (int i = 0; i < comments.size(); i++) {
BaseJsonLikeValue nextComment = comments.get(i);
if (nextComment.isString()) {
String commentText = nextComment.getAsString();
if (commentText != null) {
theState.commentPre(commentText);
if (isSupportsFhirComment()) {
if (theObject.isArray()) {
BaseJsonLikeArray comments = theObject.getAsArray();
for (int i = 0; i < comments.size(); i++) {
BaseJsonLikeValue nextComment = comments.get(i);
if (nextComment.isString()) {
String commentText = nextComment.getAsString();
if (commentText != null) {
theState.commentPre(commentText);
}
}
}
}
@ -1323,20 +1337,22 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
private void writeCommentsPreAndPost(IBase theNextValue, BaseJsonLikeWriter theEventWriter) throws IOException {
if (theNextValue.hasFormatComment()) {
beginArray(theEventWriter, "fhir_comments");
List<String> pre = theNextValue.getFormatCommentsPre();
if (pre.isEmpty() == false) {
for (String next : pre) {
theEventWriter.write(next);
if (isSupportsFhirComment()) {
beginArray(theEventWriter, "fhir_comments");
List<String> pre = theNextValue.getFormatCommentsPre();
if (pre.isEmpty() == false) {
for (String next : pre) {
theEventWriter.write(next);
}
}
}
List<String> post = theNextValue.getFormatCommentsPost();
if (post.isEmpty() == false) {
for (String next : post) {
theEventWriter.write(next);
List<String> post = theNextValue.getFormatCommentsPost();
if (post.isEmpty() == false) {
for (String next : post) {
theEventWriter.write(next);
}
}
theEventWriter.endArray();
}
theEventWriter.endArray();
}
}

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 4375
jira: SMILE-4554
title: "Depending on configuration, validation will reject resource XML that contains comments, specifically, from the JSON that's generated from the resource object. Also, for this same configuration validation will fail with an error that it can't locate resource HapiFhirStorageResponseCode.json. Both errors are now fixed."

View File

@ -36,6 +36,7 @@ import org.hl7.fhir.dstu2016may.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.dstu2016may.model.Extension;
import org.hl7.fhir.dstu2016may.model.HumanName;
import org.hl7.fhir.dstu2016may.model.IdType;
import org.hl7.fhir.dstu2016may.model.Identifier;
import org.hl7.fhir.dstu2016may.model.Identifier.IdentifierUse;
import org.hl7.fhir.dstu2016may.model.Linkage;
import org.hl7.fhir.dstu2016may.model.Medication;
@ -73,6 +74,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.stringContainsInOrder;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -1872,6 +1874,31 @@ public class JsonParserDstu2_1Test {
assertEquals(refVal, ((Reference) extlst.get(0).getValue()).getReference());
}
@Test
public void testPreCommentsToFhirComments() {
final Patient patient = new Patient();
final Identifier identifier = new Identifier();
identifier.setValue("myId");
identifier.getFormatCommentsPre().add("This is a comment");
patient.getIdentifier().add(identifier);
final HumanName humanName1 = new HumanName();
humanName1.addGiven("given1");
humanName1.getFormatCommentsPre().add("This is another comment");
patient.getName().add(humanName1);
final HumanName humanName2 = new HumanName();
humanName2.addGiven("given1");
humanName2.getFormatCommentsPre().add("This is yet another comment");
patient.getName().add(humanName2);
final String patientString = ourCtx.newJsonParser().encodeResourceToString(patient);
assertThat(patientString, is(containsString("fhir_comment")));
final String expectedJson = "{\"resourceType\":\"Patient\",\"identifier\":[{\"fhir_comments\":[\"This is a comment\"],\"value\":\"myId\"}],\"name\":[{\"fhir_comments\":[\"This is another comment\"],\"given\":[\"given1\"]},{\"fhir_comments\":[\"This is yet another comment\"],\"given\":[\"given1\"]}]}";
assertEquals(expectedJson, patientString);
}
@AfterAll
public static void afterClassClearContext() {

View File

@ -82,6 +82,7 @@ import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.stringContainsInOrder;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.hamcrest.core.IsNot.not;
@ -2213,6 +2214,31 @@ public class JsonParserDstu2Test {
assertEquals("Observation/testid", ((ResourceReferenceDt)answer.getValue()).getReference().getValue());
}
@Test
public void testPreCommentsToFhirComments() {
final Patient patient = new Patient();
final IdentifierDt identifier = new IdentifierDt();
identifier.setValue("myId");
identifier.getFormatCommentsPre().add("This is a comment");
patient.getIdentifier().add(identifier);
final HumanNameDt humanName1 = new HumanNameDt();
humanName1.addGiven("given1");
humanName1.getFormatCommentsPre().add("This is another comment");
patient.getName().add(humanName1);
final HumanNameDt humanName2 = new HumanNameDt();
humanName2.addGiven("given1");
humanName2.getFormatCommentsPre().add("This is yet another comment");
patient.getName().add(humanName2);
final String patientString = ourCtx.newJsonParser().encodeResourceToString(patient);
assertThat(patientString, is(containsString("fhir_comment")));
final String expectedJson = "{\"resourceType\":\"Patient\",\"identifier\":[{\"fhir_comments\":[\"This is a comment\"],\"value\":\"myId\"}],\"name\":[{\"fhir_comments\":[\"This is another comment\"],\"given\":[\"given1\"]},{\"fhir_comments\":[\"This is yet another comment\"],\"given\":[\"given1\"]}]}";
assertEquals(expectedJson, patientString);
}
@AfterAll
public static void afterClassClearContext() {

View File

@ -51,6 +51,7 @@ import org.hl7.fhir.dstu3.model.ExplanationOfBenefit;
import org.hl7.fhir.dstu3.model.Extension;
import org.hl7.fhir.dstu3.model.HumanName;
import org.hl7.fhir.dstu3.model.IdType;
import org.hl7.fhir.dstu3.model.Identifier;
import org.hl7.fhir.dstu3.model.Identifier.IdentifierUse;
import org.hl7.fhir.dstu3.model.Linkage;
import org.hl7.fhir.dstu3.model.Medication;
@ -95,6 +96,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.stringContainsInOrder;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -1920,34 +1922,16 @@ public class JsonParserDstu3Test {
assertEquals("654321", res.getIdentifier().get(0).getValue());
assertEquals(true, res.getActive());
assertThat(res.getIdentifier().get(0).getFormatCommentsPre(), contains("identifier comment 1", "identifier comment 2"));
assertThat(res.getIdentifier().get(0).getUseElement().getFormatCommentsPre(), contains("use comment 1", "use comment 2"));
assertThat(res.getIdentifier().get(0).getFormatCommentsPre(), not(contains("identifier comment 1", "identifier comment 2")));
assertThat(res.getIdentifier().get(0).getUseElement().getFormatCommentsPre(), not(contains("use comment 1", "use comment 2")));
String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(res);
ourLog.info(encoded);
//@formatter:off
assertThat(encoded, stringContainsInOrder(
"\"identifier\": [",
"{",
"\"fhir_comments\":",
"[",
"\"identifier comment 1\"",
",",
"\"identifier comment 2\"",
"]",
"\"use\": \"usual\",",
"\"_use\": {",
"\"fhir_comments\":",
"[",
"\"use comment 1\"",
",",
"\"use comment 2\"",
"]",
"},",
"\"type\""
));
//@formatter:off
assertThat(encoded, not(containsString("use comment 1")));
assertThat(encoded, not(containsString("use comment 2")));
assertThat(encoded, not(containsString("identifier comment 1")));
assertThat(encoded, not(containsString("identifier comment 2")));
}
@Test
@ -2537,6 +2521,29 @@ public class JsonParserDstu3Test {
assertEquals(1, containedResource.getMeta().getTag().size());
}
@Test
public void testPreCommentsToFhirComments() {
final Patient patient = new Patient();
final Identifier identifier = new Identifier();
identifier.setValue("myId");
identifier.getFormatCommentsPre().add("This is a comment");
patient.getIdentifier().add(identifier);
final HumanName humanName1 = new HumanName();
humanName1.addGiven("given1");
humanName1.getFormatCommentsPre().add("This is another comment");
patient.getName().add(humanName1);
final HumanName humanName2 = new HumanName();
humanName2.addGiven("given1");
humanName2.getFormatCommentsPre().add("This is yet another comment");
patient.getName().add(humanName2);
final String patientString = ourCtx.newJsonParser().encodeResourceToString(patient);
assertThat(patientString, is(not(containsString("fhir_comment"))));
}
@AfterAll
public static void afterClassClearContext() {
TestUtil.randomizeLocaleAndTimezone();

View File

@ -2502,25 +2502,10 @@ public class XmlParserDstu3Test {
String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(res);
ourLog.info(encoded);
assertThat(encoded, stringContainsInOrder(
"\"identifier\": [",
"{",
"\"fhir_comments\":",
"[",
"\"identifier comment 1\"",
",",
"\"identifier comment 2\"",
"]",
"\"use\": \"usual\",",
"\"_use\": {",
"\"fhir_comments\":",
"[",
"\"use comment 1\"",
",",
"\"use comment 2\"",
"]",
"},",
"\"type\""));
assertThat(encoded, not(containsString("use comment 1")));
assertThat(encoded, not(containsString("use comment 2")));
assertThat(encoded, not(containsString("identifier comment 1")));
assertThat(encoded, not(containsString("identifier comment 2")));
encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(res);
ourLog.info(encoded);
@ -2599,29 +2584,13 @@ public class XmlParserDstu3Test {
output = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(pat);
ourLog.info(output);
assertThat(output, stringContainsInOrder("{",
" \"resourceType\": \"Patient\",",
" \"id\": \"someid\",",
" \"_id\": {",
" \"fhir_comments\": [ \" comment 1 \" ]",
" },",
" \"extension\": [ {",
" \"fhir_comments\": [ \" comment 2 \", \" comment 7 \" ],",
" \"url\": \"urn:patientext:att\",",
" \"valueAttachment\": {",
" \"fhir_comments\": [ \" comment 3 \", \" comment 6 \" ],",
" \"contentType\": \"aaaa\",",
" \"_contentType\": {",
" \"fhir_comments\": [ \" comment 4 \" ]",
" },",
" \"data\": \"AAAA\",",
" \"_data\": {",
" \"fhir_comments\": [ \" comment 5 \" ]",
" }",
" }",
" } ]",
"}"));
assertThat(output, not(containsString("comment 1")));
assertThat(output, not(containsString("comment 2")));
assertThat(output, not(containsString("comment 3")));
assertThat(output, not(containsString("comment 4")));
assertThat(output, not(containsString("comment 5")));
assertThat(output, not(containsString("comment 6")));
assertThat(output, not(containsString("comment 6")));
}
@Test

View File

@ -25,6 +25,9 @@ import org.hl7.fhir.r4.model.Device;
import org.hl7.fhir.r4.model.DocumentReference;
import org.hl7.fhir.r4.model.Encounter;
import org.hl7.fhir.r4.model.Extension;
import org.hl7.fhir.r4.model.HumanName;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Identifier;
import org.hl7.fhir.r4.model.Medication;
import org.hl7.fhir.r4.model.MedicationDispense;
import org.hl7.fhir.r4.model.MedicationRequest;
@ -69,6 +72,7 @@ import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.stringContainsInOrder;
import static org.hamcrest.core.IsNot.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -1099,6 +1103,28 @@ public class JsonParserR4Test extends BaseTest {
}
@Test
public void testPreCommentsToFhirComments() {
final Patient patient = new Patient();
final Identifier identifier = new Identifier();
identifier.setValue("myId");
identifier.getFormatCommentsPre().add("This is a comment");
patient.getIdentifier().add(identifier);
final HumanName humanName1 = new HumanName();
humanName1.addGiven("given1");
humanName1.getFormatCommentsPre().add("This is another comment");
patient.getName().add(humanName1);
final HumanName humanName2 = new HumanName();
humanName2.addGiven("given1");
humanName2.getFormatCommentsPre().add("This is yet another comment");
patient.getName().add(humanName2);
final String patientString = ourCtx.newJsonParser().encodeResourceToString(patient);
assertThat(patientString, is(not(containsString("fhir_comment"))));
}
@DatatypeDef(
name = "UnknownPrimitiveType"

View File

@ -28,12 +28,12 @@ public class HapiFhirCodeSystemGeneratorTest {
@BeforeEach
public void findDirectory() {
String path = "hapi-fhir-validation/src/main/resources/org/hl7/fhir/common/hapi/validation/support";
String path = "hapi-fhir-base/src/main/resources/ca/uhn/fhir/context/support";
if (new File(path + "/" + HAPI_FHIR_STORAGE_RESPONSE_CODE_JSON).exists()) {
myPath = path;
return;
} else {
path = "./src/main/resources/org/hl7/fhir/common/hapi/validation/support";
path = "../hapi-fhir-base/src/main/resources/ca/uhn/fhir/context/support";
if (new File(path + "/" + HAPI_FHIR_STORAGE_RESPONSE_CODE_JSON).exists()) {
myPath = path;
return;