fix concepts with no display element for $apply-codesystem-delta-add and $apply-codesystem-delta-remove (#6164)

This commit is contained in:
Emre Dincturk 2024-07-26 14:17:22 -04:00 committed by GitHub
parent 10eaad3cbb
commit e5a8fc2f1b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 191 additions and 45 deletions

View File

@ -0,0 +1,8 @@
---
type: fix
issue: 6159
jira: SMILE-8604
title: "Previously, `$apply-codesystem-delta-add` and `$apply-codesystem-delta-remove` operations were failing
with a 500 Server Error when invoked with a CodeSystem Resource payload that had a concept without a
`display` element. This has now been fixed so that concepts without display field is accepted, as `display`
element is not required."

View File

@ -475,6 +475,9 @@ public class TerminologyUploaderProvider extends BaseJpaProvider {
} }
private static String csvEscape(String theValue) { private static String csvEscape(String theValue) {
if (theValue == null) {
return "";
}
return '"' + theValue.replace("\"", "\"\"").replace("\n", "\\n").replace("\r", "") + '"'; return '"' + theValue.replace("\"", "\"\"").replace("\n", "\\n").replace("\r", "") + '"';
} }
} }

View File

@ -347,7 +347,7 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
"\"reference\": \"CodeSystem/" "\"reference\": \"CodeSystem/"
); );
assertHierarchyContains( assertHierarchyContainsExactly(
"CHEM seq=0", "CHEM seq=0",
" HB seq=0", " HB seq=0",
" NEUT seq=1", " NEUT seq=1",
@ -387,7 +387,7 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
"\"reference\": \"CodeSystem/" "\"reference\": \"CodeSystem/"
); );
assertHierarchyContains( assertHierarchyContainsExactly(
"CHEM seq=0", "CHEM seq=0",
" HB seq=0", " HB seq=0",
" NEUT seq=1", " NEUT seq=1",
@ -457,7 +457,7 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
"\"reference\": \"CodeSystem/" "\"reference\": \"CodeSystem/"
); );
assertHierarchyContains( assertHierarchyContainsExactly(
"1111222233 seq=0", "1111222233 seq=0",
" 1111222234 seq=0" " 1111222234 seq=0"
); );
@ -527,13 +527,45 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
"\"reference\": \"CodeSystem/" "\"reference\": \"CodeSystem/"
); );
assertHierarchyContains( assertHierarchyContainsExactly(
"CHEM seq=0", "CHEM seq=0",
" HB seq=0", " HB seq=0",
" HBA seq=0" " HBA seq=0"
); );
} }
@Test
public void testApplyDeltaAdd_UsingCodeSystem_NoDisplaySetOnConcepts() throws IOException {
CodeSystem codeSystem = new CodeSystem();
codeSystem.setUrl("http://foo/cs");
// setting codes are enough, no need to call setDisplay etc
codeSystem.addConcept().setCode("Code1");
codeSystem.addConcept().setCode("Code2");
LoggingInterceptor interceptor = new LoggingInterceptor(true);
myClient.registerInterceptor(interceptor);
Parameters outcome = myClient
.operation()
.onType(CodeSystem.class)
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_ADD)
.withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs"))
.andParameter(TerminologyUploaderProvider.PARAM_CODESYSTEM, codeSystem)
.prettyPrint()
.execute();
myClient.unregisterInterceptor(interceptor);
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
ourLog.info(encoded);
assertThat(encoded).contains("\"valueInteger\": 2");
// assert other codes remain, and HB and NEUT is removed
assertHierarchyContainsExactly(
"Code1 seq=0",
"Code2 seq=0"
);
}
@Test @Test
public void testApplyDeltaAdd_MissingSystem() throws IOException { public void testApplyDeltaAdd_MissingSystem() throws IOException {
String conceptsCsv = loadResource("/custom_term/concepts.csv"); String conceptsCsv = loadResource("/custom_term/concepts.csv");
@ -582,30 +614,12 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
} }
@Test @Test
public void testApplyDeltaRemove() throws IOException { public void testApplyDeltaRemove_UsingCsvFiles_RemoveAllCodes() throws IOException {
String conceptsCsv = loadResource("/custom_term/concepts.csv");
Attachment conceptsAttachment = new Attachment()
.setData(conceptsCsv.getBytes(Charsets.UTF_8))
.setContentType("text/csv")
.setUrl("file:/foo/concepts.csv");
String hierarchyCsv = loadResource("/custom_term/hierarchy.csv");
Attachment hierarchyAttachment = new Attachment()
.setData(hierarchyCsv.getBytes(Charsets.UTF_8))
.setContentType("text/csv")
.setUrl("file:/foo/hierarchy.csv");
// Add the codes // Add the codes
myClient applyDeltaAddCustomTermCodes();
.operation()
.onType(CodeSystem.class)
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_ADD)
.withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs"))
.andParameter(TerminologyUploaderProvider.PARAM_FILE, conceptsAttachment)
.andParameter(TerminologyUploaderProvider.PARAM_FILE, hierarchyAttachment)
.prettyPrint()
.execute();
// And remove them // And remove all of them using the same set of csv files
LoggingInterceptor interceptor = new LoggingInterceptor(true); LoggingInterceptor interceptor = new LoggingInterceptor(true);
myClient.registerInterceptor(interceptor); myClient.registerInterceptor(interceptor);
Parameters outcome = myClient Parameters outcome = myClient
@ -613,8 +627,8 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
.onType(CodeSystem.class) .onType(CodeSystem.class)
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_REMOVE) .named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_REMOVE)
.withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs")) .withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs"))
.andParameter(TerminologyUploaderProvider.PARAM_FILE, conceptsAttachment) .andParameter(TerminologyUploaderProvider.PARAM_FILE, getCustomTermConceptsAttachment())
.andParameter(TerminologyUploaderProvider.PARAM_FILE, hierarchyAttachment) .andParameter(TerminologyUploaderProvider.PARAM_FILE, getCustomTermHierarchyAttachment())
.prettyPrint() .prettyPrint()
.execute(); .execute();
myClient.unregisterInterceptor(interceptor); myClient.unregisterInterceptor(interceptor);
@ -622,8 +636,129 @@ public class TerminologyUploaderProviderR4Test extends BaseResourceProviderR4Tes
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome); String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
ourLog.info(encoded); ourLog.info(encoded);
assertThat(encoded).contains("\"valueInteger\": 5"); assertThat(encoded).contains("\"valueInteger\": 5");
// providing no arguments, since there should be no code left
assertHierarchyContainsExactly();
} }
@Test
public void testApplyDeltaRemove_UsingConceptsCsvFileOnly() throws IOException {
//add some concepts
applyDeltaAddCustomTermCodes();
// And remove 2 of them, providing values for DISPLAY is not necessary
String conceptsToRemoveCsvData = """
CODE,DISPLAY
HB,
NEUT,
""";
Attachment conceptsAttachment = createCsvAttachment(conceptsToRemoveCsvData, "file:/concepts.csv");
LoggingInterceptor interceptor = new LoggingInterceptor(true);
myClient.registerInterceptor(interceptor);
Parameters outcome = myClient
.operation()
.onType(CodeSystem.class)
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_REMOVE)
.withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs"))
// submitting concepts is enough (no need to submit hierarchy)
.andParameter(TerminologyUploaderProvider.PARAM_FILE, conceptsAttachment)
.prettyPrint()
.execute();
myClient.unregisterInterceptor(interceptor);
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
ourLog.info(encoded);
assertThat(encoded).contains("\"valueInteger\": 2");
// assert other codes remain, and HB and NEUT is removed
assertHierarchyContainsExactly(
"CHEM seq=0",
"MICRO seq=0",
" C&S seq=0"
);
}
@Test
public void testApplyDeltaRemove_UsingCodeSystemPayload() throws IOException {
// add some custom codes
applyDeltaAddCustomTermCodes();
// remove 2 of them using CodeSystemPayload
CodeSystem codeSystem = new CodeSystem();
codeSystem.setUrl("http://foo/cs");
// setting codes are enough for remove, no need to call setDisplay etc
codeSystem.addConcept().setCode("HB");
codeSystem.addConcept().setCode("NEUT");
LoggingInterceptor interceptor = new LoggingInterceptor(true);
myClient.registerInterceptor(interceptor);
Parameters outcome = myClient
.operation()
.onType(CodeSystem.class)
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_REMOVE)
.withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs"))
.andParameter(TerminologyUploaderProvider.PARAM_CODESYSTEM, codeSystem)
.prettyPrint()
.execute();
myClient.unregisterInterceptor(interceptor);
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
ourLog.info(encoded);
assertThat(encoded).contains("\"valueInteger\": 2");
// assert other codes remain, and HB and NEUT is removed
assertHierarchyContainsExactly(
"CHEM seq=0",
"MICRO seq=0",
" C&S seq=0"
);
}
private Attachment createCsvAttachment(String theData, String theUrl) {
return new Attachment()
.setData(theData.getBytes(Charsets.UTF_8))
.setContentType("text/csv")
.setUrl(theUrl);
}
private Attachment getCustomTermConceptsAttachment() throws IOException {
String conceptsCsv = loadResource("/custom_term/concepts.csv");
return createCsvAttachment(conceptsCsv, "file:/foo/concepts.csv");
}
private Attachment getCustomTermHierarchyAttachment() throws IOException {
String hierarchyCsv = loadResource("/custom_term/hierarchy.csv");
return createCsvAttachment(hierarchyCsv, "file:/foo/hierarchy.csv");
}
private void applyDeltaAddCustomTermCodes() throws IOException {
myClient
.operation()
.onType(CodeSystem.class)
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_ADD)
.withParameter(Parameters.class, TerminologyUploaderProvider.PARAM_SYSTEM, new UriType("http://foo/cs"))
.andParameter(TerminologyUploaderProvider.PARAM_FILE, getCustomTermConceptsAttachment())
.andParameter(TerminologyUploaderProvider.PARAM_FILE, getCustomTermHierarchyAttachment())
.prettyPrint()
.execute();
assertHierarchyContainsExactly(
"CHEM seq=0",
" HB seq=0",
" NEUT seq=1",
"MICRO seq=0",
" C&S seq=0"
);
}
private static void addFile(ZipOutputStream theZos, String theFileName) throws IOException { private static void addFile(ZipOutputStream theZos, String theFileName) throws IOException {
theZos.putNextEntry(new ZipEntry(theFileName)); theZos.putNextEntry(new ZipEntry(theFileName));

View File

@ -61,7 +61,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
delta.addRootConcept("RootA", "Root A"); delta.addRootConcept("RootA", "Root A");
delta.addRootConcept("RootB", "Root B"); delta.addRootConcept("RootB", "Root B");
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
"RootB seq=0" "RootB seq=0"
); );
@ -70,7 +70,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
delta.addRootConcept("RootC", "Root C"); delta.addRootConcept("RootC", "Root C");
delta.addRootConcept("RootD", "Root D"); delta.addRootConcept("RootD", "Root D");
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
"RootB seq=0", "RootB seq=0",
"RootC seq=0", "RootC seq=0",
@ -104,7 +104,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
ourLog.info("Starting testAddHierarchyConcepts"); ourLog.info("Starting testAddHierarchyConcepts");
createNotPresentCodeSystem(); createNotPresentCodeSystem();
assertHierarchyContains(); assertHierarchyContainsExactly();
ourLog.info("Have created code system"); ourLog.info("Have created code system");
runInTransaction(() -> { runInTransaction(() -> {
@ -117,7 +117,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
delta.addRootConcept("RootA", "Root A"); delta.addRootConcept("RootA", "Root A");
delta.addRootConcept("RootB", "Root B"); delta.addRootConcept("RootB", "Root B");
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
"RootB seq=0" "RootB seq=0"
); );
@ -139,7 +139,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
myCaptureQueriesListener.logAllQueriesForCurrentThread(); myCaptureQueriesListener.logAllQueriesForCurrentThread();
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
" ChildAA seq=0", " ChildAA seq=0",
" ChildAB seq=1", " ChildAB seq=1",
@ -151,7 +151,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
@Test @Test
public void testAddMoveConceptFromOneParentToAnother() { public void testAddMoveConceptFromOneParentToAnother() {
createNotPresentCodeSystem(); createNotPresentCodeSystem();
assertHierarchyContains(); assertHierarchyContainsExactly();
UploadStatistics outcome; UploadStatistics outcome;
CustomTerminologySet delta; CustomTerminologySet delta;
@ -162,7 +162,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAA").setDisplay("Child AAA"); .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAA").setDisplay("Child AAA");
delta.addRootConcept("RootB", "Root B"); delta.addRootConcept("RootB", "Root B");
outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
" ChildAA seq=0", " ChildAA seq=0",
" ChildAAA seq=0", " ChildAAA seq=0",
@ -174,7 +174,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
delta.addRootConcept("RootB", "Root B") delta.addRootConcept("RootB", "Root B")
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA"); .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA");
outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); outcome = myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
" ChildAA seq=0", " ChildAA seq=0",
" ChildAAA seq=0", " ChildAAA seq=0",
@ -195,7 +195,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
@Test @Test
public void testReAddingConceptsDoesntRecreateExistingLinks() { public void testReAddingConceptsDoesntRecreateExistingLinks() {
createNotPresentCodeSystem(); createNotPresentCodeSystem();
assertHierarchyContains(); assertHierarchyContainsExactly();
UploadStatistics outcome; UploadStatistics outcome;
CustomTerminologySet delta; CustomTerminologySet delta;
@ -206,7 +206,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
delta.addRootConcept("RootA", "Root A") delta.addRootConcept("RootA", "Root A")
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA"); .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA");
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
" ChildAA seq=0" " ChildAA seq=0"
); );
@ -223,7 +223,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA") .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAA").setDisplay("Child AA")
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAA").setDisplay("Child AAA"); .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAA").setDisplay("Child AAA");
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
" ChildAA seq=0", " ChildAA seq=0",
" ChildAAA seq=0" " ChildAAA seq=0"
@ -242,7 +242,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAA").setDisplay("Child AAA") .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAA").setDisplay("Child AAA")
.addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAAA").setDisplay("Child AAAA"); .addChild(TermConceptParentChildLink.RelationshipTypeEnum.ISA).setCode("ChildAAAA").setDisplay("Child AAAA");
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo/cs", delta);
assertHierarchyContains( assertHierarchyContainsExactly(
"RootA seq=0", "RootA seq=0",
" ChildAA seq=0", " ChildAA seq=0",
" ChildAAA seq=0", " ChildAAA seq=0",
@ -293,7 +293,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set);
// Check so far // Check so far
assertHierarchyContains( assertHierarchyContainsExactly(
"ParentA seq=0", "ParentA seq=0",
" ChildA seq=0" " ChildA seq=0"
); );
@ -306,7 +306,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set);
// Check so far // Check so far
assertHierarchyContains( assertHierarchyContainsExactly(
"ParentA seq=0", "ParentA seq=0",
" ChildA seq=0", " ChildA seq=0",
" ChildAA seq=0" " ChildAA seq=0"
@ -331,7 +331,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set);
// Check so far // Check so far
assertHierarchyContains( assertHierarchyContainsExactly(
"ParentA seq=0", "ParentA seq=0",
" ChildA seq=0" " ChildA seq=0"
); );
@ -344,7 +344,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set); myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd("http://foo", set);
// Check so far // Check so far
assertHierarchyContains( assertHierarchyContainsExactly(
"ParentA seq=0", "ParentA seq=0",
" ChildA seq=0", " ChildA seq=0",
" ChildAA seq=0" " ChildAA seq=0"
@ -416,7 +416,7 @@ public class TerminologySvcDeltaR4Test extends BaseJpaR4Test {
expectedHierarchy.add(expected); expectedHierarchy.add(expected);
} }
assertHierarchyContains(expectedHierarchy.toArray(new String[0])); assertHierarchyContainsExactly(expectedHierarchy.toArray(new String[0]));
} }

View File

@ -740,7 +740,7 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil
dao.update(resourceParsed); dao.update(resourceParsed);
} }
protected void assertHierarchyContains(String... theStrings) { protected void assertHierarchyContainsExactly(String... theStrings) {
List<String> hierarchy = runInTransaction(() -> { List<String> hierarchy = runInTransaction(() -> {
List<String> hierarchyHolder = new ArrayList<>(); List<String> hierarchyHolder = new ArrayList<>();
TermCodeSystem codeSystem = myTermCodeSystemDao.findAll().iterator().next(); TermCodeSystem codeSystem = myTermCodeSystemDao.findAll().iterator().next();