Setting default value even if that value is null

[harishvashistha]

Hi Team,

We have a use case as mention in this issue #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, out of three commits one commit is related to revert of other PR changes, 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 #554 as we urgently need these fixes for our Tuesday release next week in production.

[SiemelNaran]

Approved after minor comment addressed. I'll review again afterwards.

[harishvashistha]

@SiemelNaran @stevehu @prashanthjos can we get these changes in master release please. This PR is urgently required with PR #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 so that I can get them in master release: #554 , #555

[SiemelNaran]

This PR looks good to me. I cannot merge to master as the button Merge pull request button (I normally use squash merge though) is grayed out and there is text "You’re not authorized to merge this pull request." next to it.