fix: null handling in config mapper (#897)

* fix: core config validation * fix: core config validation * fix: PR comments * fix: PR comments * fix: test * fix: startup test * fix: using ConfigMapper * fix: test * fix: config mapper * fix: core config * fix: null handling * fix: test defaults
supertokens · Dec 6, 2023 · 4a3a974 · 4a3a974
1 parent 16293c1
commit 4a3a974
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 20 deletions.
diff --git a/src/main/java/io/supertokens/config/Config.java b/src/main/java/io/supertokens/config/Config.java
@@ -19,6 +19,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
 import com.google.gson.JsonObject;
 import io.supertokens.Main;
 import io.supertokens.ProcessState;
@@ -51,7 +52,7 @@ private Config(Main main, String configFilePath) throws InvalidConfigException,
         this.main = main;
         final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
         Object configObj = mapper.readValue(new File(configFilePath), Object.class);
-        JsonObject jsonConfig = new Gson().toJsonTree(configObj).getAsJsonObject();
+        JsonObject jsonConfig = new GsonBuilder().serializeNulls().create().toJsonTree(configObj).getAsJsonObject();
         CoreConfig config = ConfigMapper.mapConfig(jsonConfig, CoreConfig.class);
         config.normalizeAndValidate(main, true);
         this.core = config;
@@ -91,7 +92,7 @@ public static JsonObject getBaseConfigAsJsonObject(Main main) throws IOException
         // omit them from the output json.
         ObjectMapper yamlReader = new ObjectMapper(new YAMLFactory());
         Object obj = yamlReader.readValue(new File(getConfigFilePath(main)), Object.class);
-        return new Gson().toJsonTree(obj).getAsJsonObject();
+        return new GsonBuilder().serializeNulls().create().toJsonTree(obj).getAsJsonObject();
     }
 
     private static String getConfigFilePath(Main main) {

diff --git a/src/main/java/io/supertokens/utils/ConfigMapper.java b/src/main/java/io/supertokens/utils/ConfigMapper.java
@@ -72,21 +72,9 @@ private static <T> Field findField(Class<T> clazz, String key) {
     }
 
     private static <T> void setValue(T object, Field field, JsonElement value) throws InvalidConfigException {
-        boolean foundAnnotation = false;
-        for (Annotation a : field.getAnnotations()) {
-            if (a.toString().contains("JsonProperty")) {
-                foundAnnotation = true;
-                break;
-            }
-        }
-
-        if (!foundAnnotation) {
-            return;
-        }
-
         field.setAccessible(true);
         Object convertedValue = convertJsonElementToTargetType(value, field.getType(), field.getName());
-        if (convertedValue != null) {
+        if (convertedValue != null || isNullable(field.getType())) {
             try {
                 field.set(object, convertedValue);
             } catch (IllegalAccessException e) {
@@ -95,6 +83,10 @@ private static <T> void setValue(T object, Field field, JsonElement value) throw
         }
     }
 
+    private static boolean isNullable(Class<?> type) {
+        return !type.isPrimitive();
+    }
+
     private static Object convertJsonElementToTargetType(JsonElement value, Class<?> targetType, String fieldName)
             throws InvalidConfigException {
         // If the value is JsonNull, return null for any type

diff --git a/src/test/java/io/supertokens/test/ConfigMapperTest.java b/src/test/java/io/supertokens/test/ConfigMapperTest.java
@@ -29,26 +29,40 @@ public class ConfigMapperTest {
 
     public static class DummyConfig {
         @JsonProperty
-        int int_property;
+        int int_property = -1;
 
         @JsonProperty
-        long long_property;
+        long long_property = -1;
 
         @JsonProperty
-        float float_property;
+        float float_property = -1;
 
         @JsonProperty
-        double double_property;
+        double double_property = -1;
 
         @JsonProperty
-        String string_property;
+        String string_property = "default_string";
 
         @JsonProperty
         boolean bool_property;
+
+        @JsonProperty
+        Long nullable_long_property = new Long(-1);
     }
 
     @Test
     public void testAllValidConversions() throws Exception {
+        // Test defaults
+        {
+            JsonObject config = new JsonObject();
+            assertEquals(-1, ConfigMapper.mapConfig(config, DummyConfig.class).int_property);
+            assertEquals(-1, ConfigMapper.mapConfig(config, DummyConfig.class).long_property);
+            assertEquals(-1, ConfigMapper.mapConfig(config, DummyConfig.class).float_property, 0.0001);
+            assertEquals(-1, ConfigMapper.mapConfig(config, DummyConfig.class).double_property, 0.0001);
+            assertEquals("default_string", ConfigMapper.mapConfig(config, DummyConfig.class).string_property);
+            assertEquals(new Long(-1), ConfigMapper.mapConfig(config, DummyConfig.class).nullable_long_property);
+        }
+
         // valid for int
         {
             JsonObject config = new JsonObject();
@@ -171,6 +185,25 @@ public void testAllValidConversions() throws Exception {
             config.addProperty("string_property", "hello");
             assertEquals("hello", ConfigMapper.mapConfig(config, DummyConfig.class).string_property);
         }
+
+        {
+            JsonObject config = new JsonObject();
+            config.add("string_property", null);
+            assertEquals(null, ConfigMapper.mapConfig(config, DummyConfig.class).string_property);
+        }
+
+        // valid for nullable long
+        {
+            JsonObject config = new JsonObject();
+            config.add("nullable_long_property", null);
+            assertEquals(null, ConfigMapper.mapConfig(config, DummyConfig.class).nullable_long_property);
+        }
+
+        {
+            JsonObject config = new JsonObject();
+            config.addProperty("nullable_long_property", 100);
+            assertEquals(new Long(100), ConfigMapper.mapConfig(config, DummyConfig.class).nullable_long_property);
+        }
     }
 
     @Test