Skip to content
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

Use Bean Validation (JSR 303) annotations for resolving JSON schema validation constraints #32

Merged
merged 4 commits into from Dec 19, 2014

Conversation

Projects
None yet
9 participants
@cponomaryov
Copy link
Contributor

cponomaryov commented Feb 7, 2014

Use Bean Validation (JSR 303) annotations:

  • @Size for setting
    • ArraySchema's minItems and maxItems properties
    • StringSchema's minLength and maxLength properties
  • @Min and @DecimalMin for setting
    • NumberSchema's minimum property
  • @Max and @DecimalMax for setting
    • NumberSchema's maximum property
@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Feb 7, 2014

First of all, thank you for this contribution! This is a very interesting idea, and seems useful.
Since it adds a new dependency, change needs to go in 2.4. And I think it makes sense to discuss this on the mailing list, just in case. It may not be a big deal, but generally we try to be conservative with adding new dependencies. I will send an email, see what community thinks, and we can proceed from there.

@cponomaryov

This comment has been minimized.

Copy link
Contributor Author

cponomaryov commented Feb 7, 2014

Sure, I'm ready to discuss this on the mailing list. Just want to say, that Bean Validation annotations is the most standardized and logical way of validation constraints resolving for JSON schema. So I think it will not be a problem to add a small standard java dependency.

@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Feb 7, 2014

@cponomaryov I agree -- and as I said, I think this is a great idea. A brief discussion will also have the benefit of making more devs aware of this new feature; one challenge I have seen has been that there are so many new things that it is easy to miss good improvements.

I also think this could be really nice for services, since Bean Validation annotations are used by things like DropWizard to validate configurations.

@yogeshgadge

This comment has been minimized.

Copy link

yogeshgadge commented Jun 25, 2014

Wondering where is this going. With 2.4 I am thinking that this could be done using SchemaFactoryWrapper as custom properties like like the HyperJsonSchema. Seems like this needs a rewrite.

@cponomaryov

This comment has been minimized.

Copy link
Contributor Author

cponomaryov commented Jun 28, 2014

Makes sense. Give me a couple of days to rewrite this.

@cponomaryov

This comment has been minimized.

Copy link
Contributor Author

cponomaryov commented Jun 29, 2014

I have refactored this to use SchemaFactoryWrapper.

@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Jul 11, 2014

Unfortunately since this did not make it in 2.4, I need to wait until we create 2.4 branch, and then merge this for 2.5. But I hope this is not a long delay.

@yogeshgadge

This comment has been minimized.

Copy link

yogeshgadge commented Jul 11, 2014

Some branch should be fine for me.
On Jul 11, 2014 3:51 PM, "Tatu Saloranta" notifications@github.com wrote:

Unfortunately since this did not make it in 2.4, I need to wait until we
create 2.4 branch, and then merge this for 2.5. But I hope this is not a
long delay.


Reply to this email directly or view it on GitHub
#32 (comment)
.

@steveblackmon

This comment has been minimized.

Copy link

steveblackmon commented Oct 12, 2014

Look forward to seeing this in jackson. Have been trying to validate jsonschema2pojo beans with hibernate, cryptic errors and ugly maven dependency exclusions required

@TheNitek

This comment has been minimized.

Copy link

TheNitek commented Nov 13, 2014

Any chance this is merged soon? I am realy looking forward to this!

@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Nov 13, 2014

@cponomaryov Ok, before merging, one thing I need to make sure is to have the filled Contributor License Agreement (CLA). If I haven't yet asked for one (I don't think I have a copy, looking at ones I have?), could you get this one:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and print, sign, scan and email to info at fasterxml dot com? I will also send a question on dev list, double-checking addition of the new dependency is fine.

@ccleve

This comment has been minimized.

Copy link

ccleve commented Nov 14, 2014

This is good, but we really need a way to handle all of the other constraints in JSON Schema.

This handles:
maxLength
minLength
maximum
minimum
maxItems
minItems
exclusiveMaximum (I think, through @DecimalMax(inclusive=))
exclusiveMinimum (I think, through @DecimalMin(inclusive=))

It does not handle:
format; // int32, int64, float, double, byte, date, date-time or custom
collectionFormat; // csv, ssv, tsv, pipes
default; // default value
pattern;
uniqueItems;
enum;
multipleOf;
required;

Hibernate could handle "required" with @NotNull and "pattern" with @pattern. The rest of them would require some custom annotations.

Would love to hear your thoughts on doing this in some kind of standards-compliant way.

I wonder if it would make sense to create a bunch of new Jackson-specific annotations that map to all of the JSON Schema constraints? (There's got to be a better way, but I haven't found it.)

@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Nov 14, 2014

@ccleve Good point. I think there are two starting points here:

  1. What already exists from other sources (like Bean Validation annotations)
  2. What is needed by JSON Schema

and I think there is validity to both. Doing (1) often gets you to first part of 80/20.
But at some point adding new sources will not get you significantly closer to the goal.

I don't have strong opinion on @NotNull or @Pattern, but one interesting thing to note is that we actually have another source for these:

  1. AnnotationIntrospector supports finding required (by default, from @JsonProperty(required=true), but not limited)
  2. @JsonFormat has a few properties, including pattern.

So I think that perhaps these could/should be piped through existing method of AnnotationIntrospector. Question there would then be where to implement custom AnnotationIntrospector.

Eventually we may also bump into somewhat deeper problem with JSON Schema -- it focuses a lot on kinds of things JSON might express, but not enough (IMO) on how object models of programming languages (like Java) work. I think that former is much less important than latter; and focus with JSON Schema is bit sub-optimal. But I hope there are many things we can improve before hitting the limits.

@mjustin

This comment has been minimized.

Copy link

mjustin commented Nov 17, 2014

This should also handle constraint composition -- constraints that are defined purely by the composition of other constraints. If this was supported, any constraints from other libraries that are defined purely by composition, such as NotEmpty in Hibernate Validator, would be automatically supported without having any dependencies on those libraries.

@Vad1mo

This comment has been minimized.

Copy link

Vad1mo commented Nov 19, 2014

it would be great if we had the basic part out of the door, then the community (me included) will build upon it and add more features or is the goal to do big bang and provide all futures right from the start.

@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Nov 19, 2014

@cponomaryov @Vad1mo At this point I am just waiting for CLA, as per my earlier comment. After that I can merge this PR.

@yogeshgadge

This comment has been minimized.

Copy link

yogeshgadge commented Dec 15, 2014

The two sides: Model side Bean Validation and Client side JSON validation will not converge until a common set of specifications are set forth i.e. Bean validation and JSON schema are married into one specifications.

I would personally like JSON validation move towards something like http://annotatorjs.org/ and use Annotations and let the Annotator plugins do its thing. FYI - AngularJS community is moving towards annotation for dependency injections etc albeit for different things.

Having said all these things I will just get to the following idea (which I am using at my work):-

Let the jackson module extract out all annotations on a Bean and its properties into JSON schema as additional properties. (This is where a standard Annotation property is worth while in JSON schema but for now this could be "additionalProperties": [] schema property)

Let this module be configurable to include global filters - excludes, includes by package patterns.

Such generic solution will eliminate the need do anything for JSR 303 or any future versions.

@cowtowncoder

This comment has been minimized.

Copy link
Member

cowtowncoder commented Dec 19, 2014

Ok, enough procrastination, will proceed with merge.

cowtowncoder added a commit that referenced this pull request Dec 19, 2014

Merge pull request #32 from cponomaryov/master
Use Bean Validation (JSR 303) annotations for resolving JSON schema validation constraints

@cowtowncoder cowtowncoder merged commit b333f23 into FasterXML:master Dec 19, 2014

@Vad1mo

This comment has been minimized.

Copy link

Vad1mo commented Dec 19, 2014

very good!

@cowtowncoder cowtowncoder added this to the 2.5.0 milestone Dec 19, 2014

cowtowncoder added a commit that referenced this pull request Dec 19, 2014

@sjz

This comment has been minimized.

Copy link

sjz commented Jan 8, 2015

Just my two cents:
@pattern and @NotNull provide meaning and are already handled by processes independently of Jackson.
So it would be extremely useful to handle these annotations because they are 'standard' already. However maybe this should be an 'opt-in' as, given there is already some provision for this with @json* annotations, it might not be the desired behaviour for all users

chrisphe added a commit to GroupCDG/jackson-schema-maven-plugin that referenced this pull request Feb 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.