[bv-dev] Updated proposal for container value extraction

[Matt Benson]

On Thu, Jan 12, 2017 at 9:15 AM, Emmanuel Bernard <emmanuel at hibernate.org>
wrote:

> Hey, thanks for the feedback Matt
>
> > On 11 Jan 2017, at 17:45, Matt Benson <mbenson at apache.org> wrote:
> >
> > I generally like this; I think the proposal is in quite good shape. My
> reactions to the open questions:
> >
> > 1. Should nested containers be supported?
> >   Given the Optional<List<@Email String>> example, it's probably a good
> idea (certainly it adds some complexity, but anything worth doing is worth
> doing right, as the saying goes).
> >
> > 2. Should @ConstraintsApplyTo be usable per-constraint?
> >   Doing such seems like it could be a bit clumsy, but it might be okay
> if @ConstraintsApplyTo were repeatable and included Class<? extends
> Annotation>[] constraintTypes() default {} element which, if non-empty,
> could differentiate which constraints applied to the wrapper vs. the
> extracted value.
>
> Do you mean?
>
> @ConstraintAppliesTo(target=WRAPPED_VALUE, constraintTypes=Min.class)
> @Min(3)
> List<String> nicknames;
>
> This approach cannot cover a case were the Min constraint it used one for
> the container and one for the wrapped value.
>

That is what I was postulating, yes. Min might not be the best example.
NotNull might be a better example of a constraint that one would want to
apply both to the wrapper and the extracted value.

>
> But it makes me think that we sorta addresssed a similar class of problem
> with groups.
> I haven’t explored at all but could we something similar by subverting
> groups. Let’s define two special groups: OnContainer OnWrappedValue
>
> @Min(value3, groups=OnWrappedValue.class)
> List<String> nicknames;
> // note that these examples are simplifications and should really be
> written List<@Min String> nicknames;
> // but pretend we have a subclass of List<String> with no way to put a
> TYPE_USE constraint
>
>
The idea is kind of cute (for lack of a better word), but doesn't this
complicate or prevent "normal" use of validation groups?


> This proposal would not address the case of multiple nestsed containers
> List<List<String>>
>
>
I agree that specifying WRAPPED_VALUE per-property is ambiguous for
List<List<String>>. Maybe that is an argument against allowing #8. What if
the rule were that @ConstraintsApplyTo(WRAPPED_VALUE) is valid on
TypeExtractor class definitions, while
@ConstraintsApplyTo(ANNOTATED_ELEMENT) is valid elsewhere, serving only to
override the behavior specified by the extractor? Then you could have
@Size(min=5) @ConstraintsApplyTo(ANNOTATED_ELEMENT) List<@Size(min=3)
@ConstraintsApplyTo(ANNOTATED_ELEMENT) List<String>> . This still doesn't
solve #2, but I think the problems are orthogonal.


> >
> > 3. Should @ConstraintsApplyTo also be used for tagging extractors
> triggering "auto-extraction"?
> >   I'm not sure I understand this question. Is this referring to
> classpath scanning, or "inline" specification of extractors? In either case
> a separate annotation might be warranted, particularly if the annotation is
> expanded for other reasons (see #2).
>
> No it’s to define that a given extractor prefers to target the wrapped
> value vs targeting the container
>
> @ConstraintsApplyTo(WRAPPED_VALUE)
> class OptionalExtractor … {}
>
> @Min(2) Optional<String> foo;
> // equivalent to @Min @ConstraintsApplyTo(WRAPPED_VALUE) Optional<String>
> foo;
>
> This behavior is hardcoded for Optional in the spec for info.
>
> Are you saying that question #3 is just asking for validation of this idea
as outlined in the current draft? Then my answer is yes.


> >
> > 4. Should a path node be added for type argument constraints of Optional
> and similar types?
> >   I don't personally feel this is necessary, but I wouldn't oppose it if
> others strongly desired it.
> >
> > 5. Should value extractors be discoverable via ServiceLoader?
> >   I would personally really like to see this be possible. WRT being able
> to override these, it seems simple enough that value extractors registered
> programmatically or in validation.xml could preempt any discovered via
> ServiceLoader.
> >
> > 6. What to return from PropertyDescriptor#getElementClass() if field
> type does not match method RT?
> >   The Javadoc of PropertyDescriptor references "Java Beans." Java Bean
> properties are defined by method signatures rather than fields, so it seems
> an easy decision to choose the method RT over the field signature.
> >
> > 7. Should the presence of type argument constraints alone trigger nested
> validation?
> >   I can appreciate the sense of the consistency argument to requiring
> @Valid, but in practice it seems reasonable to me that @Valid be implied.
> It probably would be much simpler to require @Valid from an implementation
> perspective, however.
>
> I personally am a bit reluctant. Do we really think that this is the
> default behavior people will want? Because you cannot negate a @Valid today
> so that’s a definitive decision. It seems to be that many containers are
> not beans per se and don’t want their properties to be validated, just the
> extacted stuff,.
>
> Are you saying that type argument constraints will be validated using
extraction, but that only validation of the Map object *itself* would
depend on @Valid to trigger validation of Map values per BV 1.x? So if
Address had validation metadata, you could have:

Map<AddressType, @NotNull Address>

And BV would validate that the values were non-null, but would not invoke
Address validation without @Valid on the Map (BV 1.x) or on the Address
type parameter? That makes sense to me. You could then combine:

Map<AddressType, @NotNull @Valid Address>

Matt


> [more reading later]
>
>
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/beanvalidation-dev/attachments/20170112/d27faf3b/attachment.html