[bv-dev] Updated proposal for container value extraction

Matt Benson mbenson at apache.org
Wed Jan 11 11:45:43 EST 2017


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.

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

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.

8. The core question seems to me to be whether there should be multiple
valid mechanisms for specifying constraints on wrapped elements.
  Since it's necessary to support the inverse for wrapped elements of
non-parameterized types, the effort required to support this is probably
similar to that required to disallow it.

9. Should we allow extractors to be defined for specific parameterized
types?
  It's tempting to support this, extends bounds and all, just to feel that
the feature is as complete as possible. At the same time I can't make a
compelling case why such a feature would really be *necessary*. This
suggests support for things like: class FooToBarMapValueExtractor
implements ValueExtractor<Map<? extends Foo, @ExtractedValue ? extends
Bar>> . In this case it might even be reasonable to equivalently support:
class FToBMapValueExtractor<F extends Foo, B extends Bar> implements
ValueExtractor<Map<F, @ExtractedValue B>> (i.e. BV would not care whether
the bounds are specified on the type parameter of the extractor class or on
the argument to the wrapper type).  Not to say that these might not be
valid use cases, but we should enter any such complexity with our eyes
open, to be sure.

10. I have no suggestions for alternatives to "type argument constraints."
"Type constraints" suggests the argument to a type parameter itself. "Type
use constraints" is fine with me, though. If people don't know, they can
learn. ;)

11. No input on this from me (for now, anyway).

12. I don't see why it would be necessary for an extractor to refer to a
parameter from a super type. The super type parameter can(/should) be
mapped to a type parameter from the extractor type. Or am I missing
something?

13/14. I am in favor of the variant proposals that would permit the type
parameter to be included as part of a Path node.

Additional notes:

* I think we ought to consider whether to provide ValueReceiver methods
that omit the nodeName parameter rather than having ValueExtractor
implementations pass null. We could potentially then require non-blank
values when the nodeName-bearing method variants are called. This would
make the API more explicit and we're not talking about an interface that
will conceivably have a lot of implementations.

* Some of the features that might come under debate, e.g.
ServiceLoader-discovered ValueExtractors, could be enabled via
implementation-specific configuration parameters or e.g. CDI extensions,
etc., if the community can't reach a consensus or decides to omit them.


Matt


On Fri, Jan 6, 2017 at 9:43 AM, Gunnar Morling <gunnar at hibernate.org> wrote:

> Hi all,
>
> I've pushed the consolidated proposal on the validation of container
> elements as an appendix to the spec draft now:
>
>     http://beanvalidation.org/latest-draft/spec/#appendix-value-extraction
>
> Can you please give it a read and raise your opinion on it. I know that
> Hendrik and Christian had some remarks/concerns originally, I'd hope the
> new one can address them appropriately. Apart from the more formal
> specification I recommend you take a look at the examples [1] to get a
> feeling for the indented semantics.
>
> Towards the end of the appendix there's a list of open questions [2]. Any
> input on those is highly welcome. E.g. 1, 2 and 5 would deserve some more
> opinions. Also what's your take on the ValueExtractor API in general? I'll
> wait a few more days to let you form some thoughts and then start a
> specific thread per open question.
>
> In the meantime we'll continue with implementing this proposal in the RI
> as far as it's defined.
>
> Thanks,
>
> --Gunnar
>
> [1] http://beanvalidation.org/latest-draft/spec/#_examples_11
> [2] http://beanvalidation.org/latest-draft/spec/#_open_questions
>
>
>
>
>
> 2016-12-22 17:15 GMT+01:00 Gunnar Morling <gunnar at hibernate.org>:
>
>> Hi,
>>
>> It has been silent on the surface, but that doesn't mean we haven't been
>> busy :)
>>
>> I've spent some time working on a proof of concept implementation of
>> the value extraction proposal (BVAL-508) which has been added to the
>> RI earlier this week [1]. This effort brought a fair bit of insights
>> and clarity which is reflected in an updated proposal that you can
>> find at [2].
>>
>> So if you can spend some time and review it, that'd be much
>> appreciated. It's a bit more formalized than the original proposal and
>> also takes an opinion on some of the original questions. I tried to
>> cut down a bit on the many loose ends to advance the matter a bit, but
>> if you are doubtful about any of the directions taken, please speak
>> up.
>>
>> Also the ValueExtractor contract has evolved a bit. It now allows for
>> a nicely uniform, yet efficient implementation of the single value and
>> multi value case. Refer to the pull request for the details. There are
>> some open questions at the end of the document. Feedback on these (and
>> of course any other parts of the document) are highly welcome. I've
>> checked on the previous threads and discussions of the matter (e.g.
>> Hendrik's extensive feedback) and hope the proposal covers all the
>> essential items. Please let me know if anything is missing.
>>
>> The idea is that we add this version of the proposal as an appendix to
>> the spec, adjust the RI to conform with it (it already does more or
>> less) so we can aim for a first alpha release of spec and RI very
>> early next year. That will allow people to get their hands on this
>> feature and play around with it a bit, hopefully resulting in some
>> more feedback from the outside, too.
>>
>> My main remaining questions are:
>>
>> * Is allowing to put constraints to an element but automatically
>> applying them to the wrapped value the right thing to do? I can see
>> the concerns about lacking comprehensibility (it's working based on
>> the presence of an automatically unwrapping value extractor), but then
>> it's needed to support "@Email StringProperty email"
>> * Should we support nested collections (e.g. List<Map<String, @NotNull
>> String>> addressesByType). It hasn't been supported before, but I can
>> see how it's more appealing with type parameter constraints. But it
>> adds complexity, too.
>>
>> Looking forward to your feedback,
>>
>> --Gunnar
>>
>> [1] https://github.com/hibernate/hibernate-validator/pull/592
>> [2] https://github.com/beanvalidation/beanvalidation-spec/pull/116
>>
>
>
> _______________________________________________
> 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/20170111/fb34bb28/attachment-0001.html 


More information about the beanvalidation-dev mailing list