fix(kernel): Correct null handling in JSON types (#523)

The JSON types were serialized "as-is" without touch, meaning if the value was `null`, it would be forwarded as `null`, instead of `undefined`. This caused some CDK usage patterns to crash at runtime. This changes the behavior so that top-level `null`s in JSON types turn into `undefined`, while `null`s nested in the JSON value will be preserved (they are valid JSON after all!). Additionally, tuned the Java code generator a little: The serialization of structs creates a jackson ObjectNode to pass down to the serialization layer, however those are turned to JSON without processing and are not honoring the inclusion setting to ignore nulls. Added some code in the Java generator to stop adding null values into the ObjectNode.
aws · Jun 7, 2019 · 7ffa98d · 7ffa98d
1 parent 09c4828
commit 7ffa98d
Show file tree

Hide file tree

Showing 12 changed files with 68 additions and 22 deletions.
diff --git a/packages/jsii-kernel/lib/serialization.ts b/packages/jsii-kernel/lib/serialization.ts
@@ -169,7 +169,9 @@ export const SERIALIZERS: {[k: string]: Serializer} = {
       // Just whatever. Dates will automatically serialize themselves to strings.
       return value;
     },
-    deserialize(value) {
+    deserialize(value, optionalValue) {
+      // /!\ Top-level "null" will turn to underfined, but any null nested in the value is valid JSON, so it'll stay!
+      if (nullAndOk(value, optionalValue)) { return undefined; }
       return value;
     },
   },

diff --git a/packages/jsii-pacmak/lib/targets/java.ts b/packages/jsii-pacmak/lib/targets/java.ts
@@ -890,7 +890,9 @@ class JavaGenerator extends Generator {
         this.code.line(`com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();`);
 
         for (const prop of props) {
+            if (prop.nullable) { this.code.openBlock(`if (this.get${prop.propName}() != null)`); }
             this.code.line(`obj.set(\"${prop.spec.name}\", om.valueToTree(this.get${prop.propName}()));`);
+            if (prop.nullable) { this.code.closeBlock(); }
         }
 
         this.code.line(`return obj;`);

diff --git a/...-calc-lib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/MyFirstStruct.java b/...-calc-lib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/MyFirstStruct.java
@@ -113,7 +113,9 @@ public java.util.List<java.lang.String> getFirstOptional() {
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
                     obj.set("anumber", om.valueToTree(this.getAnumber()));
                     obj.set("astring", om.valueToTree(this.getAstring()));
-                    obj.set("firstOptional", om.valueToTree(this.getFirstOptional()));
+                    if (this.getFirstOptional() != null) {
+                        obj.set("firstOptional", om.valueToTree(this.getFirstOptional()));
+                    }
                     return obj;
                 }
 

diff --git a/...java/src/main/java/software/amazon/jsii/tests/calculator/lib/StructWithOnlyOptionals.java b/...java/src/main/java/software/amazon/jsii/tests/calculator/lib/StructWithOnlyOptionals.java
@@ -114,9 +114,15 @@ public java.lang.Boolean getOptional3() {
                 public com.fasterxml.jackson.databind.JsonNode $jsii$toJson() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
-                    obj.set("optional1", om.valueToTree(this.getOptional1()));
-                    obj.set("optional2", om.valueToTree(this.getOptional2()));
-                    obj.set("optional3", om.valueToTree(this.getOptional3()));
+                    if (this.getOptional1() != null) {
+                        obj.set("optional1", om.valueToTree(this.getOptional1()));
+                    }
+                    if (this.getOptional2() != null) {
+                        obj.set("optional2", om.valueToTree(this.getOptional2()));
+                    }
+                    if (this.getOptional3() != null) {
+                        obj.set("optional3", om.valueToTree(this.getOptional3()));
+                    }
                     return obj;
                 }
 

diff --git a/...d.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/CalculatorProps.java b/...d.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/CalculatorProps.java
@@ -84,8 +84,12 @@ public java.lang.Number getMaximumValue() {
                 public com.fasterxml.jackson.databind.JsonNode $jsii$toJson() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
-                    obj.set("initialValue", om.valueToTree(this.getInitialValue()));
-                    obj.set("maximumValue", om.valueToTree(this.getMaximumValue()));
+                    if (this.getInitialValue() != null) {
+                        obj.set("initialValue", om.valueToTree(this.getInitialValue()));
+                    }
+                    if (this.getMaximumValue() != null) {
+                        obj.set("maximumValue", om.valueToTree(this.getMaximumValue()));
+                    }
                     return obj;
                 }
 

diff --git a/...ted.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java b/...ted.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DerivedStruct.java
@@ -237,12 +237,20 @@ public java.util.List<java.lang.String> getFirstOptional() {
                     obj.set("anotherRequired", om.valueToTree(this.getAnotherRequired()));
                     obj.set("bool", om.valueToTree(this.getBool()));
                     obj.set("nonPrimitive", om.valueToTree(this.getNonPrimitive()));
-                    obj.set("anotherOptional", om.valueToTree(this.getAnotherOptional()));
-                    obj.set("optionalAny", om.valueToTree(this.getOptionalAny()));
-                    obj.set("optionalArray", om.valueToTree(this.getOptionalArray()));
+                    if (this.getAnotherOptional() != null) {
+                        obj.set("anotherOptional", om.valueToTree(this.getAnotherOptional()));
+                    }
+                    if (this.getOptionalAny() != null) {
+                        obj.set("optionalAny", om.valueToTree(this.getOptionalAny()));
+                    }
+                    if (this.getOptionalArray() != null) {
+                        obj.set("optionalArray", om.valueToTree(this.getOptionalArray()));
+                    }
                     obj.set("anumber", om.valueToTree(this.getAnumber()));
                     obj.set("astring", om.valueToTree(this.getAstring()));
-                    obj.set("firstOptional", om.valueToTree(this.getFirstOptional()));
+                    if (this.getFirstOptional() != null) {
+                        obj.set("firstOptional", om.valueToTree(this.getFirstOptional()));
+                    }
                     return obj;
                 }
 

diff --git a/.../src/main/java/software/amazon/jsii/tests/calculator/EraseUndefinedHashValuesOptions.java b/.../src/main/java/software/amazon/jsii/tests/calculator/EraseUndefinedHashValuesOptions.java
@@ -82,8 +82,12 @@ public java.lang.String getOption2() {
                 public com.fasterxml.jackson.databind.JsonNode $jsii$toJson() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
-                    obj.set("option1", om.valueToTree(this.getOption1()));
-                    obj.set("option2", om.valueToTree(this.getOption2()));
+                    if (this.getOption1() != null) {
+                        obj.set("option1", om.valueToTree(this.getOption1()));
+                    }
+                    if (this.getOption2() != null) {
+                        obj.set("option2", om.valueToTree(this.getOption2()));
+                    }
                     return obj;
                 }
 

diff --git a/.../expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/Greetee.java b/.../expected.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/Greetee.java
@@ -64,7 +64,9 @@ public java.lang.String getName() {
                 public com.fasterxml.jackson.databind.JsonNode $jsii$toJson() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
-                    obj.set("name", om.valueToTree(this.getName()));
+                    if (this.getName() != null) {
+                        obj.set("name", om.valueToTree(this.getName()));
+                    }
                     return obj;
                 }
 

diff --git a/.../src/main/java/software/amazon/jsii/tests/calculator/LoadBalancedFargateServiceProps.java b/.../src/main/java/software/amazon/jsii/tests/calculator/LoadBalancedFargateServiceProps.java
@@ -195,11 +195,21 @@ public java.lang.Boolean getPublicTasks() {
                 public com.fasterxml.jackson.databind.JsonNode $jsii$toJson() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
-                    obj.set("containerPort", om.valueToTree(this.getContainerPort()));
-                    obj.set("cpu", om.valueToTree(this.getCpu()));
-                    obj.set("memoryMiB", om.valueToTree(this.getMemoryMiB()));
-                    obj.set("publicLoadBalancer", om.valueToTree(this.getPublicLoadBalancer()));
-                    obj.set("publicTasks", om.valueToTree(this.getPublicTasks()));
+                    if (this.getContainerPort() != null) {
+                        obj.set("containerPort", om.valueToTree(this.getContainerPort()));
+                    }
+                    if (this.getCpu() != null) {
+                        obj.set("cpu", om.valueToTree(this.getCpu()));
+                    }
+                    if (this.getMemoryMiB() != null) {
+                        obj.set("memoryMiB", om.valueToTree(this.getMemoryMiB()));
+                    }
+                    if (this.getPublicLoadBalancer() != null) {
+                        obj.set("publicLoadBalancer", om.valueToTree(this.getPublicLoadBalancer()));
+                    }
+                    if (this.getPublicTasks() != null) {
+                        obj.set("publicTasks", om.valueToTree(this.getPublicTasks()));
+                    }
                     return obj;
                 }
 

diff --git a/...c/main/java/software/amazon/jsii/tests/calculator/NullShouldBeTreatedAsUndefinedData.java b/...c/main/java/software/amazon/jsii/tests/calculator/NullShouldBeTreatedAsUndefinedData.java
@@ -81,7 +81,9 @@ public java.lang.Object getThisShouldBeUndefined() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
                     obj.set("arrayWithThreeElementsAndUndefinedAsSecondArgument", om.valueToTree(this.getArrayWithThreeElementsAndUndefinedAsSecondArgument()));
-                    obj.set("thisShouldBeUndefined", om.valueToTree(this.getThisShouldBeUndefined()));
+                    if (this.getThisShouldBeUndefined() != null) {
+                        obj.set("thisShouldBeUndefined", om.valueToTree(this.getThisShouldBeUndefined()));
+                    }
                     return obj;
                 }
 

diff --git a/...ed.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/OptionalStruct.java b/...ed.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/OptionalStruct.java
@@ -58,7 +58,9 @@ public java.lang.String getField() {
                 public com.fasterxml.jackson.databind.JsonNode $jsii$toJson() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
-                    obj.set("field", om.valueToTree(this.getField()));
+                    if (this.getField() != null) {
+                        obj.set("field", om.valueToTree(this.getField()));
+                    }
                     return obj;
                 }
 

diff --git a/...d.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/UnionProperties.java b/...d.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/UnionProperties.java
@@ -111,7 +111,9 @@ public java.lang.Object getFoo() {
                     com.fasterxml.jackson.databind.ObjectMapper om = software.amazon.jsii.JsiiObjectMapper.INSTANCE;
                     com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
                     obj.set("bar", om.valueToTree(this.getBar()));
-                    obj.set("foo", om.valueToTree(this.getFoo()));
+                    if (this.getFoo() != null) {
+                        obj.set("foo", om.valueToTree(this.getFoo()));
+                    }
                     return obj;
                 }