[
http://opensource.atlassian.com/projects/hibernate/browse/HV-296?page=com...
]
Hardy Ferentschik commented on HV-296:
--------------------------------------
Added transcript of discussion between Emmanuel, Gunnar and Hardy about how to best
implement this feature for HV:
{noformat}
9:05pm] epbernard: so let's get started and over with
[9:05pm] epbernard: Let me try and summarize
[9:05pm] hardy_: let's get the link up BVAL-202
[9:05pm] jbossbot: [BVAL-202] Apply constraints on the elements of an iterator [Open,
Major, Hardy Ferentschik]
http://opensource.atlassian.com/projects/hibernate/BVAL-202
[9:05pm] epbernard: (your new toy) the correct solution is to way for JDK 7 (hopefully)
and get finer grained annotations
[9:06pm] epbernard: but we wnat to offer the feature now to users
[9:06pm] epbernard: at least in Hibernate Validator
[9:06pm] epbernard: (do we have a corresponding HV issue number btw?)
[9:06pm] gunnar__1: yep ... are we sure that JSR 308 comes with Java 7?
[9:06pm] epbernard: no we are not sure
[9:06pm] gunnar__1: HV-296
[9:06pm] jbossbot: [HV-296] Apply constraints on the elements of an Iterable instance
[Open, Major, Hardy Ferentschik]
http://opensource.atlassian.com/projects/hibernate/HV-296
[9:06pm] epbernard: Considering that, we ahev several approaches
[9:07pm] epbernard: #! use a special group as a placeholder
[9:07pm] epbernard: #1bis use payload as a placeholder
[9:07pm] hardy_: personally using groups or payload feels wrong
[9:07pm] epbernard: #2 use a specific validElements attribute to store that (though not
allowed by the spec as valid is a reserved prefix)
[9:08pm] epbernard: #3 do some automagic and apply on elements for constraints not
applicable on Iterable<?> and subclasses
[9:08pm] gunnar__1: #3 fails for things like @size ...
[9:08pm] epbernard: #4, create a dedicated annotation for elements constraints
[9:09pm] hardy_: there is some sort of solution #5
[9:10pm] epbernard: am I missing a solution?
[9:10pm] epbernard: hardy_: hit us
[9:10pm] hardy_: let people implement custom validators where they need them and maybe
provide some our of the box where it makes sense
[9:10pm] epbernard: ah right
[9:10pm] gunnar__1: #4 splits into two solutions i think
[9:11pm] gunnar__1: having an inner anntation with all the attributes of the parent of
have an inner that holds instances of the parent
[9:11pm] hardy_: gunnar__1: right, that was your latest suggestion
[9:12pm] hardy_: regarding my #5 - HV-264 was the issue which triggered this discussion
[9:12pm] jbossbot: [HV-264] Implement ConstraintValidator<Email,
Collection<String>> [Open, Major, Hardy Ferentschik]
http://opensource.atlassian.com/projects/hibernate/HV-264
[9:12pm] gunnar__1: wouldn't like #5 too much as this problem seems like very common
so bascally many users would to the same
[9:12pm] epbernard: right #5 is out of the table for me
[9:12pm] epbernard: it's an existing but annoying solution
[9:13pm] gunnar__1: personally i feel it should be one from #4
[9:13pm] epbernard: #3 is pretty fragile and fails for a few like @NotNull and @Size
[9:13pm] hardy_: i am not sure, at least it is working with the current framework/spec and
we are waiting for a true solution
[9:13pm] epbernard: #1 is bad as groups have a very specific semantic
[9:14pm] hardy_: if not #5 I also tend to #4a or #4b
[9:14pm] epbernard: I kinda like #1bis as payloads are here for extensions
[9:14pm] epbernard: and #1bis does nto force people to write specific constraints
[9:14pm] epbernard: (ie the annotations wrappers)
[9:15pm] gunnar__1: yep, if #1 then with payload
[9:15pm] hardy_: agreed
[9:15pm] epbernard: the problem with #1 is that it will fail on another BV provider
[9:15pm] epbernard: Which is not idea
[9:15pm] gunnar__1: #2 seems pretty good if integrated into BV
[9:16pm] epbernard: As far as #4, it creates a bunch of work for us (including creating
the wrapper for all the built-in constraints
[9:16pm] hardy_: it would also avoid the problem that we somehow would have to get this
inner annotations into the constraints. How would be do that with the BV spec
constraints?
[9:16pm] sjacobs__ joined the chat room.
[9:16pm] epbernard: and leaves one question opened: how do I create my @ValidateEach
annotation?
[9:16pm] sjacobs__ left the chat room. (Read error: Connection reset by peer)
[9:17pm] hardy_: #2 btw has the same problem as #4 - how do we make this happen with the
BV constraints
[9:17pm] epbernard: is it by convention
[9:17pm] epbernard: or is the annotation meta annotated with some Hibernate Validator
specific annotations
[9:17pm] sjacobs__ joined the chat room.
[9:17pm] gunnar__1: yep #2 is the right thing when tackling this in the spec
[9:17pm] epbernard: #2 violates the spec. I'd rather avoid it
[9:17pm] sjacobs__ left the chat room. (Read error: Connection reset by peer)
[9:17pm] gunnar__1: dont see a good way to do this now for HV
[9:18pm] sjacobs__ joined the chat room.
[9:18pm] epbernard: gunnar__1: you don't see a good way at all or on a specific
problem?
[9:19pm] gunnar__1: i would like #2 to be part of the spec. for now I wouldnt know a good
way to put this new attribute into the existing BV annotatoins
[9:19pm] hardy_: it seems to me that the only solution which does not cause some spec
headache is the payload one (#1b)
[9:19pm] gunnar__1: without providing a pimped BV.jar of course
[9:19pm] hardy_: autsch
[9:20pm] epbernard:
http://pastebin.org/201668
[9:20pm] sjacobs_ left the chat room. (Ping timeout: 252 seconds)
[9:21pm] epbernard: gunnar__1: nan you can't change the bv.jar and spec wise, using a
vlid* attribute in a constraint is not legal (reserved to the EG)
[9:21pm] sjacobs joined the chat room.
[9:21pm] sjacobs__ left the chat room. (Read error: Operation timed out)
[9:21pm] hardy_: epbernard: so one ValidateEach for each constraint?
[9:21pm] epbernard: hardy_: that's the problem yes
[9:22pm] gunnar__1: yep, thats what i mean. solution #2 would be great if it was a part of
the official bv spec
[9:22pm] gunnar__1: but its out for an ad-hoc solutioin for HV i think
[9:22pm] gunnar__1: hardy_: yes unfortunately
[9:23pm] epbernard: reasonably we have #4
http://pastebin.org/201668 or #1bis
http://pastebin.org/201673
[9:23pm] gunnar__1: its really a pity that one cant have something like @interface
ValidateEach { Annotation[] value(); }
[9:23pm] gunnar__1: this would have made a great proposal for project coin
[9:24pm] epbernard: ok here is what we're going to do: propose #4 and #1bis to the
community via a blog and see the arguments
[9:24pm] epbernard: but before that, let's see the best approach for #4
[9:24pm] hardy_: we could have a single ValidateEach with one attribute for each
constraint type
[9:24pm] epbernard: hardy_: custom constraints?
[9:25pm] gunnar__1: we could provide this for the BV constraints
[9:25pm] hardy_: are in issue either way, right?
[9:25pm] gunnar__1: but what would the attributes names?
[9:25pm] hardy_: we could allow to provide a custom ValidateEach together with the custom
constraint
[9:26pm] hardy_: but the one HV would provide would contain attributes for all builtin
constraints
[9:26pm] epbernard: try it on pastebin
[9:26pm] epbernard: but I think it dpes not look good
[9:26pm] epbernard: and I am not a fan of multiple solutions for the same problem
[9:27pm] epbernard: and what about non built-in custom constraints not written by the app
developer?
[9:27pm] gunnar__1: dont see that as a problem. its in the interest of the constraint
developer to provide everything. same as with @List
[9:28pm] hardy_:
http://pastebin.com/08aUZri6
[9:29pm] epbernard:
http://pastebin.org/201696
[9:29pm] hardy_: it's not the nicest, but an alternative to the one ValidateEach
annotation for each
[9:30pm] gunnar__1: thats 4#a, isnt it?
[9:30pm] epbernard: gunnar__1: who are you talking to?
[9:30pm] gunnar__1: sorry. to you epbernar
[9:31pm] gunnar__1: I think this causes kind a lot of work for constraint developers
[9:31pm] epbernard: gunnar__1: that's #4a and #4b (two approaches are depicted)
[9:31pm] gunnar__1: yep. wanted to say "approach 2" from pastebin is #4a
[9:31pm] epbernard: I think approach 1 is less intrusive IMO
[9:32pm] epbernard: Though it's likely to be @ValidateEachSize(@Size)
@ValidateEachNotNull(@NotNull) etc
[9:33pm] epbernard: or we would need a different package name per built-in constraint
[9:33pm] hardy_: we probably would also need another marker annotation
[9:33pm] epbernard: hardy_: so hardy for your solution, what would be the approach for
custom constraints?
[9:33pm] hardy_: for custom constraints
[9:35pm] hardy_: somehow you would have to know that ValidateEachXYZ is a annotation
indication that XYX has to be applied to each element of an iterable
[9:35pm] epbernard: well not if these annotations are annotated with @ValidateElements
[9:36pm] gunnar__1: isnt that the same as @List
[9:36pm] gunnar__1: an annotation having an array-typed value of constraint annotations
[9:37pm] epbernard: gunnar__1: no @List does not need a meta annotation, the spec implies
that it's considered as all the annotations in the array
[9:37pm] epbernard: but that means that #4 is not 100% spec compliant either
[9:37pm] epbernard: both #1bis and #4 fail with another BV provider
[9:38pm] gunnar__1: oh yes ... it would be detected as multi-valued constraint
[9:38pm] epbernard: if we don't find a way out of this, I prefer the payload approach
[9:38pm] epbernard: much simpler
[9:38pm] gunnar__1: seems like that
[9:38pm] epbernard: I guess I scrwed up by not requiring a @List meta annotation
[9:39pm] hardy_: payload is less intrusive
[9:39pm] epbernard: lemme read the spec
[9:40pm] gunnar__1: and actually we'd be using it the intended way ... implementing
provider-specific extensions
[9:41pm] hardy_: hmm, except a payload means for me that a value is just carried through
the system without any specific menaing to it
[9:41pm] epbernard: ah sothe spec says that arrays of constraints whose attribute name is
value should be considered as a multivalue annotation
[9:41pm] hardy_: it's then the cient which applies meaning/semantics
[9:42pm] epbernard: @Apply(onElements=@Size(min=30)) is valid as per the spec
[9:42pm] gunnar__1: @hardy_: ok, but i wouldn't consider it to bad if HV adds a
meaning to payload
[9:43pm] hardy_: i guess i could live with it as well
[9:43pm] hardy_: it's just not 100% compliant with my understanding of a payload
[9:43pm] epbernard: hardy_: where does it say that actually?
[9:43pm] gunnar__1: well, its a payload for the provider
[9:43pm] hardy_:
[9:43pm] epbernard: the spec says Payloads are typically non-portable.
[9:43pm] hardy_: if you put it this way
[9:44pm] hardy_: epbernard: which is fair enough right? It is a HV specific feature
[9:44pm] gunnar__1: epbernard: the @Apply approach would work, but I think using it would
become pretty lengthy
[9:45pm] epbernard: hardy_: right. but to be honest I am a little bothered by the non
portability across providers as well
[9:45pm] epbernard: let's propose botha pproaches in a blog with detailed pros and
cons
[9:45pm] epbernard: and shoot based on feedback
[9:45pm] epbernard: hardy_: take the lead on that?
[9:45pm] hardy_: ok
[9:46pm] gunnar__1: sounds fine to me
[9:46pm] epbernard: cool
[9:46pm] hardy_: btw, how would any of the annotation based approaches be portable? They
are not either
[9:46pm] epbernard: @Apply is
[9:46pm] epbernard: ahh right
[9:46pm] epbernard: theya re not portable
[9:47pm] epbernard: but they don't *change* the meaning of existing annotations
[9:47pm] gunnar__1: you couldn't compile without HV
[9:47pm] epbernard: so some constraints would not be applied
[9:47pm] epbernard: but they would not change semantic
[9:47pm] hardy_: right
[9:47pm] gustavo joined the chat room.
[9:47pm] epbernard: (hum actually if HV is not in the CP, the payload class being
unresolvable, the constraint would not apply either. This needs testing)
[9:48pm] hardy_: for me this is just a tmp solution, since hopefully the true solution
would be JSR 308 further down the track
[9:48pm] epbernard: (but if HV and some other provider are both in the CP then we could
ahve different semantics for the same constraint)
[9:48pm] hardy_: so our approach should be as unintrusive as possible
[9:48pm] epbernard: I agree
{noformat}
Apply constraints on the elements of an Iterable instance
---------------------------------------------------------
Key: HV-296
URL:
http://opensource.atlassian.com/projects/hibernate/browse/HV-296
Project: Hibernate Validator
Issue Type: New Feature
Components: engine
Affects Versions: 4.0.2.GA
Reporter: Hardy Ferentschik
Assignee: Hardy Ferentschik
Fix For: 4.1.0
See BVAL-202
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://opensource.atlassian.com/projects/hibernate/secure/Administrators....
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira