-
Notifications
You must be signed in to change notification settings - Fork 280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
definition schemas #466
base: master
Are you sure you want to change the base?
definition schemas #466
Conversation
603fd2e
to
bad6a5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for raising the PR. Added a couple of notes.
One more question: do you think it is possible to change the SchemaLoader
so that the subschemas appearing under "$defs"
are added to Schema#definitionSchemas
? I know it won't be a trivial code change, but maybe it is feasible?
@@ -227,6 +227,13 @@ private void describePropertyDependencies(Map<String, Set<String>> propertyDepen | |||
visit(propertyNameSchema); | |||
} | |||
|
|||
@Override void visitDefinitionSchemas(Map<String, Schema> definitionSchemas) { | |||
if (!definitionSchemas.isEmpty()) { | |||
writer.key("definitions"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better to use $defs
since that is the currently recommended keyword according to the spec?
@@ -177,6 +181,12 @@ void visitPatternProperties(Map<Regexp, Schema> patternProperties) { | |||
} | |||
} | |||
|
|||
void visitDefinitionSchemas(Map<String, Schema> definitionSchemas) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this makes sense? It may trigger unnecessary validation. For example if the schema is
{
"type": "object",
"properties": {
"A": { "type": "string" }
},
"required": ["A"']
"$defs": {
"A": { "type": "integer" }
}
}
Then I suspect no instance would pass the validation (the "A" prop would be validated against both {"type":"string"}
and { "type": "integer" }
).
Adds the ability to configure definition schemas.