[bv-dev] Updated proposal for container value extraction

[Emmanuel Bernard]

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.

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

This proposal would not address the case of multiple nestsed containers List<List<String>>

> 
> 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.

> 
> 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,.

[more reading later]