[bv-dev] Support for constraints on container values (e.g. Collection<@Email String>)

[Emmanuel Bernard]

Thanks for your input, some comments inline.

On Sun 16-09-25 11:22, Christian Kaltepoth wrote:
>Hey Emmanuel, Hey Gunnar,
>
>sorry for my late reply. Thank you very much for the enormous amount of
>work you already spent on this proposal. This topic seems to be quite
>complex and it looks like you spent many hours thinking about all this. It
>is a bit difficult to give good feedback without being so deep into this
>topic. But I'll try. ;-)
>
>   1. I was wondering whether the distinction between
>   SingleContainerValueExtractor and ManyContainerValuesExtractor is really
>   necessary. I'm not sure if the unnecessary instantiation of an iterator in
>   the single value case is really a problem in practice. I think extractor
>   implementations will provide some special implementation of the Iterator/
>   Iterable contract in any case. Not sure how others feel about that.

I thought about that but even with a special implementation of Iterator,
you are stuck with one extra object created for each single object
validated with no purpose.
We don't have in Java  way to decorate an object without extra
invocation.
The Bean Validation design is very sensible to performance in the
critical path of object validation.

But yes we should check how things react once we have a RI prototyle,
I'll add a note to that effect.

>   2. Section 2.1 states that BV will support Iterable and Map by default.
>   What about Optional? We will support the JDK8 date/time API, so why
>   don't we plan to support Optional?

Yes that's an oversight. Fixed now.

>   3. I like the idea that users can provide custom extractors for other
>   container types. Not sure if this is something the average developer will
>   do very often, but defining an SPI here seems like a good idea.

Yes, it felt a bit over engineered at first but with the multitude of
new languages on the VM, such a contract is very useful.

>   4. I'm not completely sure if the "one extractor per container" or the
>   "one extractor per container + value" approach makes more sense. That's a
>   very difficult questions. I think I need some more time to think about that.

Gunnar explained that he prefers "one extractor per container" to avoid
the need for for @ExtractedValue on the type parameter.

I personally find the "one extractor per container + value" nicer for a
couple of reasons:

* the actual implementations are straight to the point of extracting
  values instead of also handling type parameter references and having
  to implement `getNode` which is a bit of a mystery to me now (I used
  to know why).
* it leaves the side responsibilities (type param / node) to the BV
  engine
* it is inherently more composable. I can add a new extractor after the
  fact
* it only extracts objects that need to be validated (constrained ones)
  vs returning all objects (i.e. only return Map.value vs Map.key and
  value)

It's main drawback I think is that to read a Map which is constrained
on both key and values, it requires to walk through it twice. You can
work around it in the BV engine but that requires to be slight off the
standard semantic of this proposed SPI.