Skip to content

Commit 564ae46

Browse files
Martha MitranLuke deGruchy
Martha Mitran
and
Luke deGruchy
authored
Fixes for validation lookupCode method and improve remote terminology tests (#5535)
* Fixes for validation lookupCode method: (1) NullPointerException when processing designation in the remote terminology implementation (2) Inconsistency between input and output type vs. for property parameter across the board (3) Fix most of the warnings in the updated classes. (4) Code review changes * Fix unit test failure by reverting assertion to FOO_COLUMN. * Revert commit. * Update tests * Update tests - move utility class back * Update fix and tests * spotless fix * Missing branches for exception cases and add tests for the same * Spotless fix * fix tests * fix test * Address code review comments. Write the remote terminology tests in a way to make them more readable. * Revert moving utility test class * Small changes in tests * Small changes * fix checkstyle * Minor changes in exception handling and add a missing test * Address code review comments. Decouple other tests from lookupCode tests. * Update declaration of constants --------- Co-authored-by: Luke deGruchy <[email protected]>
1 parent b28fe3b commit 564ae46

File tree

14 files changed

+1329
-728
lines changed

14 files changed

+1329
-728
lines changed

‎hapi-fhir-base/src/main/java/ca/uhn/fhir/context/support/IValidationSupport.java

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,8 @@ default byte[] fetchBinary(String binaryKey) {
295295
*/
296296
@Nullable
297297
default CodeValidationResult validateCode(
298-
@Nonnull ValidationSupportContext theValidationSupportContext,
299-
@Nonnull ConceptValidationOptions theOptions,
298+
ValidationSupportContext theValidationSupportContext,
299+
ConceptValidationOptions theOptions,
300300
String theCodeSystem,
301301
String theCode,
302302
String theDisplay,
@@ -526,8 +526,13 @@ protected BaseConceptProperty(String thePropertyName) {
526526
public String getPropertyName() {
527527
return myPropertyName;
528528
}
529+
530+
public abstract String getType();
529531
}
530532

533+
String TYPE_STRING = "string";
534+
String TYPE_CODING = "Coding";
535+
531536
class StringConceptProperty extends BaseConceptProperty {
532537
private final String myValue;
533538

@@ -544,6 +549,10 @@ public StringConceptProperty(String theName, String theValue) {
544549
public String getValue() {
545550
return myValue;
546551
}
552+
553+
public String getType() {
554+
return TYPE_STRING;
555+
}
547556
}
548557

549558
class CodingConceptProperty extends BaseConceptProperty {
@@ -574,6 +583,10 @@ public String getCodeSystem() {
574583
public String getDisplay() {
575584
return myDisplay;
576585
}
586+
587+
public String getType() {
588+
return TYPE_CODING;
589+
}
577590
}
578591

579592
class CodeValidationResult {
@@ -881,25 +894,35 @@ public IBaseParameters toParameters(
881894
}
882895

883896
for (BaseConceptProperty next : myProperties) {
897+
String propertyName = next.getPropertyName();
884898

885-
if (!properties.isEmpty()) {
886-
if (!properties.contains(next.getPropertyName())) {
887-
continue;
888-
}
899+
if (!properties.isEmpty() && !properties.contains(propertyName)) {
900+
continue;
889901
}
890902

891903
IBase property = ParametersUtil.addParameterToParameters(theContext, retVal, "property");
892-
ParametersUtil.addPartCode(theContext, property, "code", next.getPropertyName());
893-
894-
if (next instanceof StringConceptProperty) {
895-
StringConceptProperty prop = (StringConceptProperty) next;
896-
ParametersUtil.addPartString(theContext, property, "value", prop.getValue());
897-
} else if (next instanceof CodingConceptProperty) {
898-
CodingConceptProperty prop = (CodingConceptProperty) next;
899-
ParametersUtil.addPartCoding(
900-
theContext, property, "value", prop.getCodeSystem(), prop.getCode(), prop.getDisplay());
901-
} else {
902-
throw new IllegalStateException(Msg.code(1739) + "Don't know how to handle " + next.getClass());
904+
ParametersUtil.addPartCode(theContext, property, "code", propertyName);
905+
906+
String propertyType = next.getType();
907+
switch (propertyType) {
908+
case TYPE_STRING:
909+
StringConceptProperty stringConceptProperty = (StringConceptProperty) next;
910+
ParametersUtil.addPartString(
911+
theContext, property, "value", stringConceptProperty.getValue());
912+
break;
913+
case TYPE_CODING:
914+
CodingConceptProperty codingConceptProperty = (CodingConceptProperty) next;
915+
ParametersUtil.addPartCoding(
916+
theContext,
917+
property,
918+
"value",
919+
codingConceptProperty.getCodeSystem(),
920+
codingConceptProperty.getCode(),
921+
codingConceptProperty.getDisplay());
922+
break;
923+
default:
924+
throw new IllegalStateException(
925+
Msg.code(1739) + "Don't know how to handle " + next.getClass());
903926
}
904927
}
905928
}

‎hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5512-codesystem-lookup-with-designation-no-language-nullpointerexception.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
type: fix
3+
issue: 5511
4+
title: 'Previously, CodeSystem `$lookup` with Remote Terminology Service enabled would throw NullPointerException
5+
when the CodeSystem included designations with no language value. Also, there was an inconsistency between
6+
input and output type `string` vs. `code` for property parameters. These issues have been fixed.'

‎hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3CodeSystemPropertiesTest.java

Lines changed: 74 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@
44
import ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil;
55
import ca.uhn.fhir.rest.gclient.IOperationUntypedWithInputAndPartialOutput;
66
import org.hl7.fhir.dstu3.model.CodeSystem;
7+
import org.hl7.fhir.dstu3.model.CodeSystem.ConceptDefinitionComponent;
8+
import org.hl7.fhir.dstu3.model.CodeSystem.ConceptPropertyComponent;
79
import org.hl7.fhir.dstu3.model.CodeType;
10+
import org.hl7.fhir.dstu3.model.Coding;
811
import org.hl7.fhir.dstu3.model.Parameters;
12+
import org.hl7.fhir.dstu3.model.Parameters.ParametersParameterComponent;
913
import org.hl7.fhir.dstu3.model.StringType;
1014
import org.hl7.fhir.dstu3.model.UriType;
1115
import org.junit.jupiter.params.ParameterizedTest;
@@ -21,81 +25,83 @@
2125
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.ourCodeSystemUrl;
2226
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.ourPropertyA;
2327
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.ourPropertyB;
28+
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.ourPropertyC;
2429
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.ourPropertyValueA;
2530
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.ourPropertyValueB;
31+
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.propertyCode;
32+
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.propertyCodeSystem;
33+
import static ca.uhn.fhir.jpa.provider.CodeSystemLookupWithPropertiesUtil.propertyDisplay;
2634
import static org.junit.jupiter.api.Assertions.assertEquals;
2735
import static org.junit.jupiter.api.Assertions.assertNotNull;
2836
import static org.junit.jupiter.api.Assertions.assertNull;
2937
import static org.junit.jupiter.api.Assertions.assertTrue;
3038

3139
public class ResourceProviderDstu3CodeSystemPropertiesTest extends BaseResourceProviderDstu3Test {
3240

33-
public static Stream<Arguments> parametersLookup() {
34-
return CodeSystemLookupWithPropertiesUtil.parametersLookupWithProperties();
35-
}
36-
37-
@ParameterizedTest
38-
@MethodSource(value = "parametersLookup")
39-
public void testLookup_withProperties_returnsCorrectParameters(List<String> theLookupProperties, List<String> theExpectedReturnedProperties) {
40-
// setup
41-
CodeSystem codeSystem = new CodeSystem();
42-
codeSystem.setId(ourCodeSystemId);
43-
codeSystem.setUrl(ourCodeSystemUrl);
44-
CodeSystem.ConceptDefinitionComponent concept = codeSystem.addConcept().setCode(ourCode);
45-
CodeSystem.ConceptPropertyComponent propertyComponent = new CodeSystem.ConceptPropertyComponent()
46-
.setCode(ourPropertyA).setValue(new StringType(ourPropertyValueA));
47-
concept.addProperty(propertyComponent);
48-
propertyComponent = new CodeSystem.ConceptPropertyComponent()
49-
.setCode(ourPropertyB).setValue(new StringType(ourPropertyValueB));
50-
concept.addProperty(propertyComponent);
51-
myCodeSystemDao.create(codeSystem, mySrd);
52-
53-
// test
54-
IOperationUntypedWithInputAndPartialOutput<Parameters> respParam = myClient
55-
.operation()
56-
.onType(CodeSystem.class)
57-
.named(JpaConstants.OPERATION_LOOKUP)
58-
.withParameter(Parameters.class, "code", new CodeType(ourCode))
59-
.andParameter("system", new UriType(ourCodeSystemUrl));
60-
61-
theLookupProperties.forEach(p -> respParam.andParameter("property", new CodeType(p)));
62-
Parameters parameters = respParam.execute();
63-
64-
Iterator<Parameters.ParametersParameterComponent> paramIterator = parameters.getParameter().iterator();
65-
Parameters.ParametersParameterComponent parameter = null;
66-
while (paramIterator.hasNext()) {
67-
Parameters.ParametersParameterComponent currentParameter = paramIterator.next();
68-
if (currentParameter.getName().equals("property")) {
69-
parameter = currentParameter;
70-
break;
71-
}
72-
}
73-
74-
if (theExpectedReturnedProperties.isEmpty()) {
75-
assertNull(parameter);
76-
return;
77-
}
78-
79-
Iterator<CodeSystem.ConceptPropertyComponent> propertyIterator = concept.getProperty().stream()
80-
.filter(property -> theExpectedReturnedProperties.contains(property.getCode())).iterator();
81-
82-
while (propertyIterator.hasNext()) {
83-
CodeSystem.ConceptPropertyComponent property = propertyIterator.next();
84-
assertNotNull(parameter);
85-
86-
Iterator<Parameters.ParametersParameterComponent> parameterPartIterator = parameter.getPart().iterator();
87-
88-
parameter = parameterPartIterator.next();
89-
assertEquals("code", parameter.getName());
90-
assertEquals(property.getCode(), ((CodeType)parameter.getValue()).getValue());
91-
92-
parameter = parameterPartIterator.next();
93-
assertEquals("value", parameter.getName());
94-
assertTrue(property.getValue().equalsShallow(parameter.getValue()));
95-
96-
if (paramIterator.hasNext()) {
97-
parameter = paramIterator.next();
98-
}
99-
}
100-
}
41+
public static Stream<Arguments> parametersLookup() {
42+
return CodeSystemLookupWithPropertiesUtil.parametersLookupWithProperties();
43+
}
44+
45+
@ParameterizedTest
46+
@MethodSource(value = "parametersLookup")
47+
public void testLookup_withProperties_returnsCorrectParameters(List<String> theLookupProperties, List<String> theExpectedReturnedProperties) {
48+
// setup
49+
CodeSystem codeSystem = new CodeSystem();
50+
codeSystem.setId(ourCodeSystemId);
51+
codeSystem.setUrl(ourCodeSystemUrl);
52+
ConceptDefinitionComponent concept = codeSystem.addConcept().setCode(ourCode)
53+
.addProperty(new ConceptPropertyComponent().setCode(ourPropertyA).setValue(new StringType(ourPropertyValueA)))
54+
.addProperty(new ConceptPropertyComponent().setCode(ourPropertyB).setValue(new StringType(ourPropertyValueB)))
55+
.addProperty(new ConceptPropertyComponent().setCode(ourPropertyC).setValue(new Coding(propertyCodeSystem, propertyCode, propertyDisplay)));
56+
myCodeSystemDao.create(codeSystem, mySrd);
57+
58+
// test
59+
IOperationUntypedWithInputAndPartialOutput<Parameters> respParam = myClient
60+
.operation()
61+
.onType(CodeSystem.class)
62+
.named(JpaConstants.OPERATION_LOOKUP)
63+
.withParameter(Parameters.class, "code", new CodeType(ourCode))
64+
.andParameter("system", new UriType(ourCodeSystemUrl));
65+
66+
theLookupProperties.forEach(p -> respParam.andParameter("property", new CodeType(p)));
67+
Parameters parameters = respParam.execute();
68+
69+
Iterator<ParametersParameterComponent> paramIterator = parameters.getParameter().iterator();
70+
ParametersParameterComponent parameter = null;
71+
while (paramIterator.hasNext()) {
72+
ParametersParameterComponent currentParameter = paramIterator.next();
73+
if (currentParameter.getName().equals("property")) {
74+
parameter = currentParameter;
75+
break;
76+
}
77+
}
78+
79+
// verify
80+
if (theExpectedReturnedProperties.isEmpty()) {
81+
assertNull(parameter);
82+
return;
83+
}
84+
85+
Iterator<ConceptPropertyComponent> propertyIterator = concept.getProperty().stream()
86+
.filter(property -> theExpectedReturnedProperties.contains(property.getCode())).iterator();
87+
88+
while (propertyIterator.hasNext()) {
89+
ConceptPropertyComponent property = propertyIterator.next();
90+
assertNotNull(parameter);
91+
92+
Iterator<ParametersParameterComponent> parameterPartIterator = parameter.getPart().iterator();
93+
94+
parameter = parameterPartIterator.next();
95+
assertEquals("code", parameter.getName());
96+
assertEquals(property.getCode(), ((CodeType) parameter.getValue()).getValue());
97+
98+
parameter = parameterPartIterator.next();
99+
assertEquals("value", parameter.getName());
100+
assertTrue(property.getValue().equalsShallow(parameter.getValue()));
101+
102+
if (paramIterator.hasNext()) {
103+
parameter = paramIterator.next();
104+
}
105+
}
106+
}
101107
}

0 commit comments

Comments
 (0)