Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle null values in fhir-server-config and throw for exceptional case #2822

Merged
merged 4 commits into from
Oct 5, 2021
Merged

Handle null values in fhir-server-config and throw for exceptional case #2822

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d82cc6
Handle null values in fhir-server-config and throw for exceptional case
lmsurpre Oct 1, 2021
37fb951
Update PropertyGroup.getProperties to omit properties with null values
lmsurpre Oct 4, 2021
2938c85
Update PropertyGroup.getJsonValue to normalize null and missing props
lmsurpre Oct 4, 2021
645211e
Update fhir-config/src/main/java/com/ibm/fhir/config/PropertyGroup.java
lmsurpre Oct 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions fhir-config/src/main/java/com/ibm/fhir/config/PropertyGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
import java.util.List;
import java.util.Map;

import com.ibm.fhir.core.FHIRUtilities;

import jakarta.json.JsonArray;
import jakarta.json.JsonNumber;
import jakarta.json.JsonObject;
import jakarta.json.JsonString;
import jakarta.json.JsonValue;

import com.ibm.fhir.core.FHIRUtilities;

/**
* This class represents a collection of properties - a property group. This could be the entire set of properties
* resulting from loading the configuration, or it could be just a sub-structure within the overall config hierarchy, as
Expand Down Expand Up @@ -107,7 +107,7 @@ public String getStringProperty(String propertyName, String defaultValue) throws
/**
* This is a convenience function that will retrieve an array property, then convert it
* to a list of Strings by calling toString() on each array element.
*
*
* @param propertyName the name of the property to retrieve
* @return a List<String> containing the elements from the JSON array property; possibly null
* @throws Exception
Expand Down Expand Up @@ -237,12 +237,16 @@ public Object[] getArrayProperty(String propertyName) throws Exception {
/**
* Returns the properties contained in the PropertyGroup in the form of a list of
* PropertyEntry instances. If no properties exist, then an empty list will be returned.
* Properties with a value of null will be omitted from the list.
* @throws Exception
*/
public List<PropertyEntry> getProperties() throws Exception {
List<PropertyEntry> results = new ArrayList<>();
for (Map.Entry<String, JsonValue> entry : jsonObj.entrySet()) {
results.add(new PropertyEntry(entry.getKey(), convertJsonValue(entry.getValue())));
Object jsonValue = convertJsonValue(entry.getValue());
if (jsonValue != null) {
results.add(new PropertyEntry(entry.getKey(), convertJsonValue(entry.getValue())));
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
}
}
return results;
}
Expand All @@ -262,7 +266,7 @@ public String toString() {
/**
* Converts the specified JsonValue into the appropriate java.lang.* type.
* @param jsonValue the JsonValue instance to be converted
* @return an instance of Boolean, Integer, String, PropertyGroup, or List<Object>
* @return either null or an instance of Boolean, Integer, String, PropertyGroup, or List<Object>
* @throws Exception
*/
public static Object convertJsonValue(JsonValue jsonValue) throws Exception {
Expand Down Expand Up @@ -292,6 +296,8 @@ public static Object convertJsonValue(JsonValue jsonValue) throws Exception {
case FALSE:
result = Boolean.FALSE;
break;
case NULL:
punktilious marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
throw new IllegalStateException("Unexpected JSON value type: " + jsonValue.getValueType().name());
}
Expand All @@ -317,6 +323,7 @@ private static Object[] convertJsonArray(JsonArray jsonArray) throws Exception {
*
* @param propertyName
* the possibly hierarchical property name.
* @return the property value as a JsonValue or null if the property is either missing or has a null value
*/
public JsonValue getJsonValue(String propertyName) {
String[] pathElements = getPathElements(propertyName);
Expand All @@ -325,7 +332,7 @@ public JsonValue getJsonValue(String propertyName) {
if (subGroup != null) {
result = subGroup.get(pathElements[pathElements.length - 1]);
}
return result;
return result == JsonValue.NULL ? null : result;
}

/**
Expand Down
123 changes: 80 additions & 43 deletions fhir-config/src/test/java/com/ibm/fhir/config/test/FHIRConfigHelperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
/*
* (C) Copyright IBM Corp. 2017, 2020
* (C) Copyright IBM Corp. 2017, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/

package com.ibm.fhir.config.test;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.AssertJUnit.assertEquals;
import static org.testng.AssertJUnit.assertNotNull;
import static org.testng.AssertJUnit.assertTrue;
import static org.testng.Assert.assertTrue;

import java.io.PrintWriter;
import java.util.Arrays;
Expand All @@ -34,6 +34,9 @@ public class FHIRConfigHelperTest {
String[] array2 = { "token3", "token4" };
List<String> expectedList2 = Arrays.asList(array2);

String[] arrayWithNull = { "token1", null, "token2" };
List<String> expectedListWithNull = Arrays.asList(arrayWithNull);

// String used to write out json config file on the fly.
String tenant3ConfigTemplate =
"{ \"fhirServer\": { \"property1\": \"__A__\", \"property2\": \"__B__\" }}";
Expand Down Expand Up @@ -62,39 +65,72 @@ public void testDefaultConfig1() throws Exception {
Double d;
s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp1", null);
assertNotNull(s);
assertEquals("defaultValue1", s);
assertEquals(s, "defaultValue1");

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp2", null);
assertNotNull(s);
assertEquals("defaultValue2", s);
assertEquals(s, "defaultValue2");

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp1", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp2", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

l = FHIRConfigHelper.getStringListProperty("collection/groupB/stringList1");
assertNotNull(l);
assertEquals(expectedList1, l);
assertEquals(l, expectedList1);

s = FHIRConfigHelper.getStringProperty("collection/groupB/boolProp1", null);
assertNotNull(s);
assertEquals("false", s);
assertEquals(s, "false");

i = FHIRConfigHelper.getIntProperty("collection/groupC/intProp1", null);
assertNotNull(i);
assertEquals(12345, i.intValue());
assertEquals(i.intValue(), 12345);

i = FHIRConfigHelper.getIntProperty("collection/groupC/intProp2", null);
assertNotNull(i);
assertEquals(12345, i.intValue());
assertEquals(i.intValue(), 12345);

d = FHIRConfigHelper.getDoubleProperty("collection/groupC/doubleProp2", null);
assertNotNull(d);
assertEquals(12345.001, d.doubleValue());
assertEquals(d.doubleValue(), 12345.001);
}

@Test
public void testNullValues() throws Exception {
String tenant = FHIRConfigHelper.getStringProperty("collection/tenant", null);
assertNotNull(tenant);
assertEquals("default", tenant);

String s;
Boolean b;
Integer i;
Double d;
List<String> l;

s = FHIRConfigHelper.getStringProperty("collection/groupD/nullProp", "defaultValue1");
assertNotNull(s);
assertEquals(s, "defaultValue1");

b = FHIRConfigHelper.getBooleanProperty("collection/groupD/nullProp", false);
assertNotNull(b);
assertEquals(b, Boolean.FALSE);

i = FHIRConfigHelper.getIntProperty("collection/groupD/nullProp", 12345);
assertNotNull(i);
assertEquals(i.intValue(), 12345);

d = FHIRConfigHelper.getDoubleProperty("collection/groupD/nullProp", 12345.001);
assertNotNull(d);
assertEquals(d.doubleValue(), 12345.001);

l = FHIRConfigHelper.getStringListProperty("collection/groupD/stringListWithNullProp");
assertNotNull(l);
assertEquals(l, expectedListWithNull);
}

@Test
Expand All @@ -110,23 +146,23 @@ public void testTenant1() throws Exception {
Boolean b;
s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp1", null);
assertNotNull(s);
assertEquals("tenant1Value1", s);
assertEquals(s, "tenant1Value1");

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp2", null);
assertNotNull(s);
assertEquals("defaultValue2", s);
assertEquals(s, "defaultValue2");

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp1", null);
assertNotNull(b);
assertEquals(Boolean.TRUE, b);
assertEquals(b, Boolean.TRUE);

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp2", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

l = FHIRConfigHelper.getStringListProperty("collection/groupB/stringList1");
assertNotNull(l);
assertEquals(expectedList2, l);
assertEquals(l, expectedList2);
}

@Test
Expand All @@ -140,25 +176,26 @@ public void testTenant2() throws Exception {
List<String> l;
String s;
Boolean b;

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp1", null);
assertNotNull(s);
assertEquals("defaultValue1", s);
assertEquals(s, "defaultValue1");

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp2", null);
assertNotNull(s);
assertEquals("tenant2Value2", s);
assertEquals(s, "tenant2Value2");

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp1", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp2", null);
assertNotNull(b);
assertEquals(Boolean.TRUE, b);
assertEquals(b, Boolean.TRUE);

l = FHIRConfigHelper.getStringListProperty("collection/groupB/stringList1");
assertNotNull(l);
assertEquals(0, l.size());
assertEquals(l.size(), 0);
}

@Test
Expand All @@ -178,11 +215,11 @@ public void testTenant3() throws Exception {
// Load tenant3's config and check the initial property values.
String s = FHIRConfigHelper.getStringProperty("fhirServer/property1", null);
assertNotNull(s);
assertEquals("property1Value1", s);
assertEquals(s, "property1Value1");

s = FHIRConfigHelper.getStringProperty("fhirServer/property2", null);
assertNotNull(s);
assertEquals("property2Value1", s);
assertEquals(s, "property2Value1");

// Sleep for just a bit to make sure the updated config file has a newer timestamp.
Thread.sleep(1000);
Expand All @@ -196,11 +233,11 @@ public void testTenant3() throws Exception {

s = FHIRConfigHelper.getStringProperty("fhirServer/property1", null);
assertNotNull(s);
assertEquals("property1Value2", s);
assertEquals(s, "property1Value2");

s = FHIRConfigHelper.getStringProperty("fhirServer/property2", null);
assertNotNull(s);
assertEquals("property2Value2", s);
assertEquals(s, "property2Value2");
}

@Test
Expand All @@ -218,36 +255,36 @@ public void testTenant4() throws Exception {
Boolean b;
s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp1", null);
assertNotNull(s);
assertEquals("defaultValue1", s);
assertEquals(s, "defaultValue1");

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp2", null);
assertNotNull(s);
assertEquals("defaultValue2", s);
assertEquals(s, "defaultValue2");

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp1", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp2", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

l = FHIRConfigHelper.getStringListProperty("collection/groupB/stringList1");
assertNotNull(l);
assertEquals(expectedList1, l);
assertEquals(l, expectedList1);

Double d = FHIRConfigHelper.getDoubleProperty("collection/groupC/doubleProp1", 1.0);
assertNotNull(d);
assertEquals(12345.001, d);
assertEquals(d.doubleValue(), 12345.001);

d = FHIRConfigHelper.getDoubleProperty("collection/groupC/doubleProp2", 1.0);
assertNotNull(d);
assertEquals(12345.001, d);
assertEquals(d.doubleValue(), 12345.001);

d = FHIRConfigHelper.getDoubleProperty("collection/groupC/doubleProp3", 1.0);
assertNotNull(d);
assertEquals(1.0, d);
assertEquals(d.doubleValue(), 1.0);

assertNotNull(FHIRConfiguration.getInstance().loadConfiguration().toString());
assertFalse(FHIRConfiguration.getInstance().loadConfiguration().toString().isEmpty());
}
Expand All @@ -268,31 +305,31 @@ public void testTenant5() throws Exception {
Boolean b;
s = FHIRConfigHelper.getStringProperty("collection/groupA/newProp1", null);
assertNotNull(s);
assertEquals("newValue1", s);
assertEquals(s, "newValue1");

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp1", null);
assertNotNull(s);
assertEquals("defaultValue1", s);
assertEquals(s, "defaultValue1");

s = FHIRConfigHelper.getStringProperty("collection/groupA/stringProp2", null);
assertNotNull(s);
assertEquals("defaultValue2", s);
assertEquals(s, "defaultValue2");

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp1", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/boolProp2", null);
assertNotNull(b);
assertEquals(Boolean.FALSE, b);
assertEquals(b, Boolean.FALSE);

b = FHIRConfigHelper.getBooleanProperty("collection/groupB/newBoolProp1", null);
assertNotNull(b);
assertEquals(Boolean.TRUE, b);
assertEquals(b, Boolean.TRUE);

l = FHIRConfigHelper.getStringListProperty("collection/groupB/stringList1");
assertNotNull(l);
assertEquals(expectedList1, l);
assertEquals(l, expectedList1);
}

@Test
Expand Down
Loading