Skip to content

Commit 9a8dc0a

Browse files
YalingPeiSGary
YalingPeiS
authored and
Gary
committed
[7401] Smileutil upload-terminology command does not load all concept properties from CSV file (#7407)
* [7401] fix smileutil upload-terminology not loading all properties * [7401] add test * [7401] add duplicate concept property check
1 parent d629c3f commit 9a8dc0a

File tree

6 files changed

+202
-8
lines changed

6 files changed

+202
-8
lines changed

‎hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_8_0/7401-fix-smileutil-upload-terminology-not-load-all-concept-properties.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
type: fix
3+
issue: 7401
4+
jira: SMILE-11256
5+
title: "Previously, `smileutil upload-terminology` failed to load and save multiple concept properties that
6+
used the same property code for a single `CodeSystem.concept`. This is now fixed."
7+
8+

‎hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptPropertyTypeEnum.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
*/
2020
package ca.uhn.fhir.jpa.entity;
2121

22+
import static org.apache.commons.lang3.StringUtils.defaultString;
23+
2224
/**
2325
* @see TermConceptProperty#getType()
2426
*/
@@ -51,5 +53,15 @@ public enum TermConceptPropertyTypeEnum {
5153
/**
5254
* Date and time values.
5355
*/
54-
DATETIME
56+
DATETIME;
57+
58+
public static TermConceptPropertyTypeEnum fromString(String theString) {
59+
TermConceptPropertyTypeEnum retVal;
60+
try {
61+
retVal = TermConceptPropertyTypeEnum.valueOf(defaultString(theString));
62+
} catch (Exception e) {
63+
retVal = TermConceptPropertyTypeEnum.STRING;
64+
}
65+
return retVal;
66+
}
5567
}

‎hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/custom/PropertyHandler.java‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,23 @@
2222
import ca.uhn.fhir.jpa.entity.TermConceptProperty;
2323
import ca.uhn.fhir.jpa.entity.TermConceptPropertyTypeEnum;
2424
import ca.uhn.fhir.jpa.term.IZipContentsHandlerCsv;
25-
import ca.uhn.fhir.jpa.term.TermLoaderSvcImpl;
26-
import ca.uhn.fhir.util.ValidateUtil;
2725
import org.apache.commons.csv.CSVRecord;
26+
import org.slf4j.Logger;
27+
import org.slf4j.LoggerFactory;
28+
import org.springframework.util.CollectionUtils;
2829

2930
import java.util.ArrayList;
3031
import java.util.List;
3132
import java.util.Map;
33+
import java.util.Objects;
3234

3335
import static org.apache.commons.lang3.StringUtils.isNotBlank;
3436
import static org.apache.commons.lang3.StringUtils.trim;
3537

3638
public class PropertyHandler implements IZipContentsHandlerCsv {
3739

40+
private static final Logger ourLog = LoggerFactory.getLogger(PropertyHandler.class);
41+
3842
public static final String CODE = "CODE";
3943
public static final String KEY = "KEY";
4044
public static final String VALUE = "VALUE";
@@ -50,18 +54,24 @@ public void accept(CSVRecord theRecord) {
5054
String code = trim(theRecord.get(CODE));
5155
String key = trim(theRecord.get(KEY));
5256

53-
if (isNotBlank(code) && isNotBlank(KEY)) {
57+
if (isNotBlank(code) && isNotBlank(key)) {
5458
String value = trim(theRecord.get(VALUE));
5559
String type = trim(theRecord.get(TYPE));
5660

5761
List<TermConceptProperty> conceptProperties = myCode2Properties.get(code);
5862
if (conceptProperties == null) conceptProperties = new ArrayList<>();
5963

60-
TermConceptProperty conceptProperty =
61-
TermLoaderSvcImpl.getOrCreateConceptProperty(myCode2Properties, code, key);
62-
ValidateUtil.isNotNullOrThrowUnprocessableEntity(
63-
conceptProperty, "Concept property %s not found in file", conceptProperty);
64+
if (isDuplicateConceptProperty(conceptProperties, key, value, type)) {
65+
ourLog.info(
66+
"Duplicate concept property found for code {}, key {}, value {}, type {}. Skipping entry.",
67+
code,
68+
key,
69+
value,
70+
type);
71+
return;
72+
}
6473

74+
TermConceptProperty conceptProperty = new TermConceptProperty();
6575
conceptProperty.setKey(key);
6676
conceptProperty.setValue(value);
6777
// TODO: check this for different types, other types should be added once TermConceptPropertyTypeEnum
@@ -71,4 +81,15 @@ public void accept(CSVRecord theRecord) {
7181
myCode2Properties.put(code, conceptProperties);
7282
}
7383
}
84+
85+
public boolean isDuplicateConceptProperty(
86+
List<TermConceptProperty> conceptProperties, String key, String value, String typeString) {
87+
if (CollectionUtils.isEmpty(conceptProperties)) {
88+
return false;
89+
}
90+
return conceptProperties.stream()
91+
.anyMatch(p -> Objects.equals(p.getKey(), key)
92+
&& Objects.equals(p.getValue(), value)
93+
&& Objects.equals(p.getType(), TermConceptPropertyTypeEnum.fromString(typeString)));
94+
}
7495
}

‎hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/TerminologyUploaderProviderR4Test.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.io.FileOutputStream;
3131
import java.io.IOException;
3232
import java.util.Arrays;
33+
import java.util.Collection;
3334
import java.util.List;
3435
import java.util.Optional;
3536
import java.util.zip.ZipEntry;
@@ -318,6 +319,88 @@ public void testApplyDeltaAdd_UsingCsv_withPropertiesCsv() throws IOException {
318319
});
319320
}
320321

322+
@Test
323+
public void testApplyDeltaAdd_sameKeyWithDifferentValuePropertiesShouldBeSaved() throws IOException {
324+
String inputParamJson = loadResource("/custom_term/codeSystem-duplicatePropertyKeys.json");
325+
Parameters inputParameters = myFhirContext.newJsonParser().parseResource(Parameters.class, inputParamJson);
326+
327+
LoggingInterceptor interceptor = new LoggingInterceptor(true);
328+
myClient.registerInterceptor(interceptor);
329+
Parameters outcome = myClient
330+
.operation()
331+
.onType(CodeSystem.class)
332+
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_ADD)
333+
.withParameters(inputParameters)
334+
.prettyPrint()
335+
.execute();
336+
myClient.unregisterInterceptor(interceptor);
337+
338+
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
339+
ourLog.info(encoded);
340+
assertThat(encoded).containsSubsequence(
341+
"\"name\": \"conceptCount\"",
342+
"\"valueInteger\": 1",
343+
"\"name\": \"target\"",
344+
"\"reference\": \"CodeSystem/"
345+
);
346+
runInTransaction(() -> {
347+
TermCodeSystem cs = myTermCodeSystemDao.findByCodeSystemUri("http://www.nlm.nih.gov/research/umls/rxnorm_resource");
348+
TermCodeSystemVersion version = cs.getCurrentVersion();
349+
Optional<TermConcept> optional = myTermConceptDao.findByCodeSystemAndCode(version.getPid(), "748856");
350+
assertThat(optional).isPresent();
351+
352+
TermConcept concept = optional.get();
353+
Collection<TermConceptProperty> properties = concept.getProperties();
354+
355+
assertThat(properties).hasSize(3);
356+
357+
// Check that two properties were saved with key "STY" having different values
358+
List<String> valuesForSTY = properties.stream()
359+
.filter(p -> "STY".equalsIgnoreCase(p.getKey()))
360+
.map(TermConceptProperty::getValue)
361+
.toList();
362+
assertThat(valuesForSTY).hasSize(2);
363+
assertThat(valuesForSTY).containsExactlyInAnyOrder("STN:A1.3.1.1", "STY:Drug Delivery Device");
364+
});
365+
}
366+
367+
@Test
368+
public void testApplyDeltaAdd_noDuplicateConceptPropertySaved() throws IOException {
369+
String inputParamJson = loadResource("/custom_term/codeSystem-duplicateConceptProperties.json");
370+
Parameters inputParameters = myFhirContext.newJsonParser().parseResource(Parameters.class, inputParamJson);
371+
372+
LoggingInterceptor interceptor = new LoggingInterceptor(true);
373+
myClient.registerInterceptor(interceptor);
374+
myClient.operation()
375+
.onType(CodeSystem.class)
376+
.named(JpaConstants.OPERATION_APPLY_CODESYSTEM_DELTA_ADD)
377+
.withParameters(inputParameters)
378+
.prettyPrint()
379+
.execute();
380+
myClient.unregisterInterceptor(interceptor);
381+
382+
runInTransaction(() -> {
383+
TermCodeSystem cs = myTermCodeSystemDao.findByCodeSystemUri("http://www.nlm.nih.gov/research/umls/rxnorm_resource");
384+
TermCodeSystemVersion version = cs.getCurrentVersion();
385+
Optional<TermConcept> optional = myTermConceptDao.findByCodeSystemAndCode(version.getPid(), "748856");
386+
assertThat(optional).isPresent();
387+
388+
TermConcept concept = optional.get();
389+
Collection<TermConceptProperty> properties = concept.getProperties();
390+
391+
properties.forEach(prop -> ourLog.info("Property: {} = {}", prop.getKey(), prop.getValue()));
392+
assertThat(properties).hasSize(2);
393+
394+
// Check that a property with key "STY" and value "XYZ" was only saved once
395+
List<String> valuesForSTY = properties.stream()
396+
.filter(p -> "STY".equalsIgnoreCase(p.getKey()))
397+
.map(TermConceptProperty::getValue)
398+
.toList();
399+
assertThat(valuesForSTY).hasSize(1);
400+
assertThat(valuesForSTY).containsExactlyInAnyOrder("XYZ");
401+
});
402+
}
403+
321404
@Test
322405
public void testApplyDeltaAdd_UsingCodeSystem() {
323406
CodeSystem codeSystem = new CodeSystem();

‎hapi-fhir-jpaserver-test-utilities/src/main/resources/custom_term/codeSystem-duplicateConceptProperties.json‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"resourceType": "Parameters",
3+
"parameter": [
4+
{
5+
"name": "system",
6+
"valueUri": "http://www.nlm.nih.gov/research/umls/rxnorm_resource"
7+
},
8+
{
9+
"name": "codeSystem",
10+
"resource": {
11+
"resourceType": "CodeSystem",
12+
"concept": [
13+
{
14+
"code": "748856",
15+
"display": "{24 (drospirenone 3 MG / ethinyl estradiol 0.02 MG Oral Tablet) / 4 (inert ingredients 1 MG Oral Tablet) } Pack [Yaz 28 Day]",
16+
"property": [
17+
{
18+
"code": "STY",
19+
"valueString": "XYZ"
20+
},
21+
{
22+
"code": "STY",
23+
"valueString": "XYZ"
24+
},
25+
{
26+
"code": "TTY",
27+
"valueString": "BPCK"
28+
}
29+
]
30+
}
31+
]
32+
}
33+
}
34+
]
35+
}

‎hapi-fhir-jpaserver-test-utilities/src/main/resources/custom_term/codeSystem-duplicatePropertyKeys.json‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"resourceType": "Parameters",
3+
"parameter": [
4+
{
5+
"name": "system",
6+
"valueUri": "http://www.nlm.nih.gov/research/umls/rxnorm_resource"
7+
},
8+
{
9+
"name": "codeSystem",
10+
"resource": {
11+
"resourceType": "CodeSystem",
12+
"concept": [
13+
{
14+
"code": "748856",
15+
"display": "{24 (drospirenone 3 MG / ethinyl estradiol 0.02 MG Oral Tablet) / 4 (inert ingredients 1 MG Oral Tablet) } Pack [Yaz 28 Day]",
16+
"property": [
17+
{
18+
"code": "STY",
19+
"valueString": "STN:A1.3.1.1"
20+
},
21+
{
22+
"code": "STY",
23+
"valueString": "STY:Drug Delivery Device"
24+
},
25+
{
26+
"code": "TTY",
27+
"valueString": "BPCK"
28+
}
29+
]
30+
}
31+
]
32+
}
33+
}
34+
]
35+
}

0 commit comments

Comments
 (0)