Update com.networknt:json-schema-validator to 2.0.1#3344
Conversation
5184376 to
99da018
Compare
|
Seems this is the reason we didn't upgrade right away: |
…r 2.x Signed-off-by: Pedro Boado <pedro.boado@datadoghq.com>
| Dialect.builder(dialectFor(schemaVersion.toVersionFlag())) | ||
| .keyword(new NonValidationKeyword("nullable")) |
There was a problem hiding this comment.
what is the .keyword method call doing exactly? the tests seem to pass without it.
There was a problem hiding this comment.
Hi, I'm following the migration notes here
`The `com.networknt.schema.SchemaValidatorsConfig` file has been replaced by either `com.networknt.schema.SchemaRegistryConfig` or `com.networknt.schema.walk.WalkConfig` or moved to `com.networknt.schema.ExecutionConfig` and can no longer be configured on a per schema basis.
Concretely, the migration action for this is
| Name | Migration |
| ------------------------------------- | --------------------------------------------------------------------------------------- |
| `nullableKeywordEnabled` | Removed. Dialect must contain a `nullable` keyword.
nullableKeywordEnabled is the new definition of config.setHandleNullableField(true); introduced with version 1.4.1 , never applied to wiremock.
In version 2.0.x it seems nullable keyword is only supported in OpenAPI 3.0 . What we are doing is adding that keyword nullable to the draft dialects in order to keep the same behaviour .
This is tested in MatchesJsonSchemaPatternTest.nullValueMatchesWhenSchemaDeclaresNullable() :
- The test passes on master branch , running with version
1.5.6ofjson-schema-validator. - The test fails after migrating to 2.0.1 right after the first commit in this PR.
- Adding the
nullablekeyword to the draft dialects allows it to pass again.
There was a problem hiding this comment.
I think there a legitimate question as to whether we actually want the nullable keyword in here, since it's technically more correct not to support it (we're doing pure JSON Schema here, not OpenAPI), and 4.x is still in beta and thus breaking changes are possible.
There was a problem hiding this comment.
@pedroboado sorry, you're right, the tests do fail without it. i was just looking at the wrong tests 🤦
There was a problem hiding this comment.
Having done a bit of data analysis I think this would be pretty nasty break, so I think we should keep this. Will merge.
…JsonSchemaVersion Signed-off-by: Pedro Boado <pedro.boado@datadoghq.com>
|
@pedroboado please could you run |
Done, apologies I missed running it with the last commit. |
|
There's an architecture test failing now. Could you take a quick look at that and push a fix? |
ae9ad2b to
e00e5b2
Compare
…s and arch tests were not detecting their use. Replacing with chained if sequence. Signed-off-by: Pedro Boado <pedro.boado@datadoghq.com>
e00e5b2 to
1616139
Compare
|
Thanks @pedroboado ! |
The dependency
json-schema-validatorhas seen a major API rework after1.5.6, making keeping it up-to-date a tad more challenging. This PR addresses the required code changes to unblock the dependency update from the now discontinued branch1.5.x.json-schema-validator:2.0.1retains Java 8 and Jackson 2 requirements.Migrate from the removed 1.x API to the 2.x equivalents:
References
Submitter checklist
#help-contributingor a project-specific channel like#wiremock-java