Setting default value even if that value is null (#555)

* removed unnecessary check

* should always set default value even if default=null

* test case added which sets value as null when default=null specified in schema

* improved apply property default logic to accomodate applyPropertyDefaultsIfNull

* updating as per review comment by modifing code structure

* removed unnecessary test case data

* removed unnecessary schema cases

* review comment fixed to set nullnode even if nullNode is present in array

* apply default null even if null is provided in data when shouldApplyPropertyDefaultsIfNull is true

Co-authored-by: Vashist <[email protected]>

Author: harishvashistha

pom.xml

@@ -20,7 +20,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>com.networknt</groupId>
     <artifactId>json-schema-validator</artifactId>
-    <version>1.0.68</version>
+    <version>1.0.68-SNAPSHOT2</version>
     <packaging>bundle</packaging>
     <description>A json schema validator that supports draft v4, v6, v7 and v2019-09</description>
     <url>https://github.com/networknt/json-schema-validator</url>

src/main/java/com/networknt/schema/ItemsValidator.java

@@ -123,9 +123,6 @@ public Set<ValidationMessage> walk(JsonNode node, JsonNode rootNode, String at,
             JsonNode defaultNode = null;
             if (applyDefaultsStrategy.shouldApplyArrayDefaults() && schema != null) {
                 defaultNode = schema.getSchemaNode().get("default");
-                if (defaultNode != null && defaultNode.isNull()) {
-                    defaultNode = null;
-                }
             }
             int i = 0;
             for (JsonNode n : arrayNode) {

src/main/java/com/networknt/schema/PropertiesValidator.java

@@ -132,17 +132,23 @@ private void applyPropertyDefaults(ObjectNode node) {
         for (Map.Entry<String, JsonSchema> entry : schemas.entrySet()) {
             JsonNode propertyNode = node.get(entry.getKey());
 
-            if (propertyNode == null || (applyDefaultsStrategy.shouldApplyPropertyDefaultsIfNull() && propertyNode.isNull())) {
-                JsonSchema propertySchema = entry.getValue();
-                JsonNode defaultNode = propertySchema.getSchemaNode().get("default");
-                if (defaultNode != null && !defaultNode.isNull()) {
-                    // mutate the input json
-                    node.set(entry.getKey(), defaultNode);
-                }
+            JsonNode defaultNode = getDefaultNode(entry);
+            if (defaultNode == null) {
+                continue;
+            }
+            boolean applyDefault = propertyNode == null
+                    || (propertyNode.isNull() && applyDefaultsStrategy.shouldApplyPropertyDefaultsIfNull());
+            if (applyDefault) {
+                node.set(entry.getKey(), defaultNode);
             }
         }
     }
 
+    private JsonNode getDefaultNode(final Map.Entry<String, JsonSchema> entry) {
+        JsonSchema propertySchema = entry.getValue();
+        return propertySchema.getSchemaNode().get("default");
+    }
+
     private void walkSchema(Map.Entry<String, JsonSchema> entry, JsonNode node, JsonNode rootNode, String at,
                             boolean shouldValidateSchema, Set<ValidationMessage> validationMessages, WalkListenerRunner propertyWalkListenerRunner) {
         JsonSchema propertySchema = entry.getValue();

src/test/java/com/networknt/schema/JsonWalkApplyDefaultsTest.java

@@ -18,10 +18,10 @@
 
 class JsonWalkApplyDefaultsTest {
 
-    @AfterEach
+   /* @AfterEach
     void cleanup() {
         CollectorContext.getInstance().reset();
-    }
+    }*/
 
     @ParameterizedTest
     @ValueSource(booleans = { true, false})
@@ -33,14 +33,15 @@ void testApplyDefaults3(boolean shouldValidateSchema) throws IOException {
         if (shouldValidateSchema) {
             assertThat(result.getValidationMessages().stream().map(ValidationMessage::getMessage).collect(Collectors.toList()),
                        Matchers.containsInAnyOrder("$.outer.mixedObject.intValue_missingButError: string found, integer expected",
-                                                   "$.outer.badArray[1]: integer found, string expected"));
+                                                   "$.outer.badArray[1]: integer found, string expected",
+                               "$.outer.reference.stringValue_missing_with_default_null: null found, string expected"));
         } else {
             assertThat(result.getValidationMessages(), Matchers.empty());
         }
         // TODO: In Java 14 use text blocks
         assertEquals(
                 objectMapper.readTree(
-                        "{\"outer\":{\"mixedObject\":{\"intValue_present\":8,\"intValue_missing\":15,\"intValue_missing_notRequired\":25,\"intValue_null\":35,\"intValue_missingButError\":\"forty-five\"},\"goodArray\":[\"hello\",\"five\"],\"badArray\":[\"hello\",5],\"reference\":{\"stringValue_missing\":\"hello\"}}}"),
+                        "{\"outer\":{\"mixedObject\":{\"intValue_present\":8,\"intValue_null\":35,\"intValue_missingButError\":\"forty-five\",\"intValue_missing\":15,\"intValue_missing_notRequired\":25},\"goodArray\":[\"hello\",\"five\"],\"badArray\":[\"hello\",5],\"reference\":{\"stringValue_missing_with_default_null\":null,\"stringValue_missing\":\"hello\"}}}"),
                 inputNode);
     }
 
@@ -53,10 +54,11 @@ void testApplyDefaults2() throws IOException {
         assertThat(result.getValidationMessages().stream().map(ValidationMessage::getMessage).collect(Collectors.toList()),
                    Matchers.containsInAnyOrder("$.outer.mixedObject.intValue_missingButError: string found, integer expected",
                                                "$.outer.goodArray[1]: null found, string expected",
-                                               "$.outer.badArray[1]: null found, string expected"));
+                                               "$.outer.badArray[1]: null found, string expected",
+                           "$.outer.reference.stringValue_missing_with_default_null: null found, string expected"));
         assertEquals(
                 objectMapper.readTree(
-                        "{\"outer\":{\"mixedObject\":{\"intValue_present\":8,\"intValue_missing\":15,\"intValue_missing_notRequired\":25,\"intValue_null\":35,\"intValue_missingButError\":\"forty-five\"},\"goodArray\":[\"hello\",null],\"badArray\":[\"hello\",null],\"reference\":{\"stringValue_missing\":\"hello\"}}}"),
+                        "{\"outer\":{\"mixedObject\":{\"intValue_present\":8,\"intValue_null\":35,\"intValue_missingButError\":\"forty-five\",\"intValue_missing\":15,\"intValue_missing_notRequired\":25},\"goodArray\":[\"hello\",null],\"badArray\":[\"hello\",null],\"reference\":{\"stringValue_missing_with_default_null\":null,\"stringValue_missing\":\"hello\"}}}"),
                 inputNode);
     }
 
@@ -70,10 +72,11 @@ void testApplyDefaults1() throws IOException {
                    Matchers.containsInAnyOrder("$.outer.mixedObject.intValue_null: null found, integer expected",
                                                "$.outer.mixedObject.intValue_missingButError: string found, integer expected",
                                                "$.outer.goodArray[1]: null found, string expected",
-                                               "$.outer.badArray[1]: null found, string expected"));
+                                               "$.outer.badArray[1]: null found, string expected",
+                           "$.outer.reference.stringValue_missing_with_default_null: null found, string expected"));
         assertEquals(
                 objectMapper.readTree(
-                        "{\"outer\":{\"mixedObject\":{\"intValue_present\":8,\"intValue_missing\":15,\"intValue_missing_notRequired\":25,\"intValue_null\":null,\"intValue_missingButError\":\"forty-five\"},\"goodArray\":[\"hello\",null],\"badArray\":[\"hello\",null],\"reference\":{\"stringValue_missing\":\"hello\"}}}"),
+                        "{\"outer\":{\"mixedObject\":{\"intValue_present\":8,\"intValue_null\":null,\"intValue_missingButError\":\"forty-five\",\"intValue_missing\":15,\"intValue_missing_notRequired\":25},\"goodArray\":[\"hello\",null],\"badArray\":[\"hello\",null],\"reference\":{\"stringValue_missing_with_default_null\":null,\"stringValue_missing\":\"hello\"}}}"),
                 inputNode);
     }
 

src/test/resources/schema/walk-schema-default.json

@@ -10,6 +10,15 @@
           "description": "the test data does not have this attribute, so default should be applied if the ApplyDefaultsStrategy requires it",
           "type": "string",
           "default": "hello"
+        },
+        "stringValue_missing_with_default_null": {
+          "description": "the test data does not have this attribute, so default as null should be applied if the ApplyDefaultsStrategy requires it",
+          "type": "string",
+          "default": null
+        },
+        "stringValue_missing_with_no_default": {
+          "description": "the test data does not have this attribute, no default value should be applied",
+          "type": "string"
         }
       },
       "required": [