Repository: networknt/json-schema-validator
Pull request #555 (2022-04-14 10:17:39)

Setting default value even if that value is null
Hi Team,

We have a use case as mention in this issue https://github.com/networknt/json-schema-validator/issues/553 which is causing failures in downstream systems where "x":null has different meaning than non existance of "x". can you please evaluate if this change can go in future release else we would have to put some hacky code before calling json-schema-validator which modifies the json which is going to be validated and thereby breaks the purpose of this library.

Please help  

stevehu: @harishvashistha I think this PR covers the previous one. Could we close that one and only leave this one open? Thanks.
harishvashistha: PR https://github.com/networknt/json-schema-validator/pull/554/files has changes related to file

`src/main/java/com/networknt/schema/utils/JsonNodeUtil.java`

so that nullable fields succeeds when handleNullable is true and null value supplied. This current PR does not have that change.
This PR is related to setting null as default value when `"default": "null"` is specified in schema definition.

Do you want me to merge these two PRs into this one for tracking?
stevehu: No necessary. You are using the same branch to track both PRs so they merged already automatically. If you look at this PR, there are three commits. 
harishvashistha: @stevehu apologies for confusion, but this PR does not have code from another PR as i reverted that code manually while creating this branch, as I wanted both the PRs to show relevant changes only for the issue they are targeting and mentioned in subject. Please consider both the PRs with respect to the file changes they have. 
When can we expect these two PRs in official master as we are planning our project release to production soon?
stevehu: The changes look good to me. I just want other developers to have some input on the changes as they are more familiar with this feature. Once merged, we are expecting a release soon. 
harishvashistha: @SiemelNaran I have fixed all the review comments. Please try to release this code with PR https://github.com/networknt/json-schema-validator/pull/554 as we urgently need these fixes for our Tuesday release next week in production.
harishvashistha: @SiemelNaran @stevehu @prashanthjos can we get these changes in master release please. This PR is urgently required with PR https://github.com/networknt/json-schema-validator/pull/554 as we are planning a relase on Tuesday.
I hope you would understand the urgency for these changes.
stevehu: @harishvashistha I am not familiar with the feature and depend on other team members to review the changes. Once the PR is reviewed and merged, we will have a release.
harishvashistha: @stevehu thanks for the update, may be if I can ask @SiemelNaran  and @prashanthjos if you guys can help me with these two PRs, it would be a great gesture on your side.
I am fully dependent on this library to move to production with our project. Please help with these PRs: https://github.com/networknt/json-schema-validator/pull/554 , https://github.com/networknt/json-schema-validator/pull/555


SiemelNaran: This PR looks good to me.  I don't know if I have the ability to merge to master.  I will try now.

Patch:
diff --git a/pom.xml b/pom.xml
index feaa95f..e5eac5f 100644
--- a/pom.xml
+++ b/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>
diff --git a/src/main/java/com/networknt/schema/ItemsValidator.java b/src/main/java/com/networknt/schema/ItemsValidator.java
index ea3cd04..4af2b1b 100644
--- a/src/main/java/com/networknt/schema/ItemsValidator.java
+++ b/src/main/java/com/networknt/schema/ItemsValidator.java
@@ -123,9 +123,6 @@ public class ItemsValidator extends BaseJsonValidator implements JsonValidator {
             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) {
diff --git a/src/main/java/com/networknt/schema/PropertiesValidator.java b/src/main/java/com/networknt/schema/PropertiesValidator.java
index 14d8a55..f09e52e 100644
--- a/src/main/java/com/networknt/schema/PropertiesValidator.java
+++ b/src/main/java/com/networknt/schema/PropertiesValidator.java
@@ -132,17 +132,23 @@ public class PropertiesValidator extends BaseJsonValidator implements JsonValida
         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();


Test patch:
diff --git a/src/test/java/com/networknt/schema/JsonWalkApplyDefaultsTest.java b/src/test/java/com/networknt/schema/JsonWalkApplyDefaultsTest.java
index 4afe9a5..c63655a 100644
--- a/src/test/java/com/networknt/schema/JsonWalkApplyDefaultsTest.java
+++ b/src/test/java/com/networknt/schema/JsonWalkApplyDefaultsTest.java
@@ -18,10 +18,10 @@ import org.junit.jupiter.params.provider.ValueSource;
 
 class JsonWalkApplyDefaultsTest {
 
-    @AfterEach
+   /* @AfterEach
     void cleanup() {
         CollectorContext.getInstance().reset();
-    }
+    }*/
 
     @ParameterizedTest
     @ValueSource(booleans = { true, false})
@@ -33,14 +33,15 @@ class JsonWalkApplyDefaultsTest {
         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 @@ class JsonWalkApplyDefaultsTest {
         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 @@ class JsonWalkApplyDefaultsTest {
                    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);
     }
 
diff --git a/src/test/resources/schema/walk-schema-default.json b/src/test/resources/schema/walk-schema-default.json
index a320913..e26cd83 100644
--- a/src/test/resources/schema/walk-schema-default.json
+++ b/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": [