Hi all, I and a few others have been working on a proposal to support things like Collection<@Email String> and Optional<@Email String>. This is more complicated that it seems at first glance. Instead of doing an ad-hoc support for the various collection types, Optional and the JavaFX Properties, we quickly decided to define the notion of container and the ability to declare constraints on contained elements to validate them. This lead to two main proposals that you can read at http://beanvalidation.org/proposals/BVAL-508/ This is a relatively long read, you can start by ignoring "alternative" options for your first pass. We are very interested in feedback at this stage as we have been pushing these proposal very far already and they would need to become part of the spec as next step. Let me know of what you think, questions, remarks etc. In particular, I'm interested in what you think of the following. The capability to define custom containers. The extractor approach vs its alternative. The concepts of @ConstraintsAppliesTo(COMTAINED) used for JavaFX and for subclasses of containers. @Valid, in particular the legacy and new forms and how to handle the transition. And finally, but a big one, what do you think of proposal 1 vs proposal 2. The latter being more generic but with more open questions (and a less elaborated at this stage). Emmanuel _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

+1 If everyone could take a look at this one, that'd be great! It's a bit more complex, so the more eyes we get on this one, the better. Thanks! 2016-09-19 13:28 GMT+02:00 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: > This is probably going to be most visible feature of Bean Validation > 2.0. We particularly need your feedback and involvement on this one. > > Emmanuel > > On Tue 2016-09-06 18:12, Emmanuel Bernard wrote: > >Hi all, > > > >I and a few others have been working on a proposal to support things > >like Collection<@Email String> and Optional<@Email String>. This is > >more complicated that it seems at first glance. > > > >Instead of doing an ad-hoc support for the various collection types, > >Optional and the JavaFX Properties, we quickly decided to define the > >notion of container and the ability to declare constraints on contained > >elements to validate them. > > > >This lead to two main proposals that you can read at > >http://beanvalidation.org/proposals/BVAL-508/ > > > >This is a relatively long read, you can start by ignoring "alternative" > >options for your first pass. We are very interested in feedback at this > >stage as we have been pushing these proposal very far already and they > >would need to become part of the spec as next step. > > > >Let me know of what you think, questions, remarks etc. > > > >In particular, I'm interested in what you think of the following. > > > >The capability to define custom containers. > > > >The extractor approach vs its alternative. > > > >The concepts of @ConstraintsAppliesTo(COMTAINED) used for JavaFX and for > >subclasses of containers. > > > >@Valid, in particular the legacy and new forms and how to handle the > >transition. > > > >And finally, but a big one, what do you think of proposal 1 vs proposal > >2. The latter being more generic but with more open questions (and a > >less elaborated at this stage). > > > >Emmanuel > >_______________________________________________ > >beanvalidation-dev mailing list > &gt;beanvalidation-dev(a)lists.jboss.org > >https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

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. Good question. I reckon we'll know more after experimenting with this in

1. 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? Optional will be part of it by default. It should be stated somewhere, I

1. 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. 2. 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. Same here. Personally I'm not a huge fan of annotating the type parameter

1. The concept of @ConstraintsApplyTo looks fine to me and makes a lot of sense. Especially being able to use the annotation on the extractor AND at the container declaration site. That's my first batch of feedback. Hope this helps.

2016-09-19 16:29 GMT+02:00 Otávio Gonçalves de Santana < otaviopolianasantana(a)gmail.com&gt;: > That's fine to me. > > On 19 Sep 2016 4:31 a.m., "Gunnar Morling" <gunnar(a)hibernate.org&gt; wrote: > >> +1 >> >> If everyone could take a look at this one, that'd be great! It's a bit >> more complex, so the more eyes we get on this one, the better. >> >> Thanks! >> >> 2016-09-19 13:28 GMT+02:00 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: >> >>> This is probably going to be most visible feature of Bean Validation >>> 2.0. We particularly need your feedback and involvement on this one. >>> >>> Emmanuel >>> >>> On Tue 2016-09-06 18:12, Emmanuel Bernard wrote: >>> >Hi all, >>> > >>> >I and a few others have been working on a proposal to support things >>> >like Collection<@Email String> and Optional<@Email String>. This is >>> >more complicated that it seems at first glance. >>> > >>> >Instead of doing an ad-hoc support for the various collection types, >>> >Optional and the JavaFX Properties, we quickly decided to define the >>> >notion of container and the ability to declare constraints on contained >>> >elements to validate them. >>> > >>> >This lead to two main proposals that you can read at >>> >http://beanvalidation.org/proposals/BVAL-508/ >>> > >>> >This is a relatively long read, you can start by ignoring "alternative" >>> >options for your first pass. We are very interested in feedback at this >>> >stage as we have been pushing these proposal very far already and they >>> >would need to become part of the spec as next step. >>> > >>> >Let me know of what you think, questions, remarks etc. >>> > >>> >In particular, I'm interested in what you think of the following. >>> > >>> >The capability to define custom containers. >>> > >>> >The extractor approach vs its alternative. >>> > >>> >The concepts of @ConstraintsAppliesTo(COMTAINED) used for JavaFX and >>> for >>> >subclasses of containers. >>> > >>> >@Valid, in particular the legacy and new forms and how to handle the >>> >transition. >>> > >>> >And finally, but a big one, what do you think of proposal 1 vs proposal >>> >2. The latter being more generic but with more open questions (and a >>> >less elaborated at this stage). >>> > >>> >Emmanuel >>> >_______________________________________________ >>> >beanvalidation-dev mailing list >>> &gt;beanvalidation-dev(a)lists.jboss.org >>> >https://lists.jboss.org/mailman/listinfo/beanvalidation-dev >>> _______________________________________________ >>> beanvalidation-dev mailing list >>> beanvalidation-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev >>> >> >> >> _______________________________________________ >> beanvalidation-dev mailing list >> beanvalidation-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev >> > > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > -- Christian Kaltepoth Blog: http://blog.kaltepoth.de/ Twitter: http://twitter.com/chkal GitHub: https://github.com/chkal _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

Am 30.09.2016 um 14:35 schrieb Christian Kaltepoth <christian(a)kaltepoth.de&gt;: Hey Gunnar, 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. Good question. I reckon we'll know more after experimenting with this in the RI. I agree. The proposal mentions the possibility of having different functional rules and better performance for the single value case as reasons for the distinction. I just think that the latter one won't actually be a real problem. 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? Optional will be part of it by default. It should be stated somewhere, I hope :) The document explicitly states: Bean Validation will have out of the box support for containers Iterable and Map. I was expecting Optional to be listed here too. That's why I mentioned it. :-) Christian -- Christian Kaltepoth Blog: http://blog.kaltepoth.de/ <http://blog.kaltepoth.de/> Twitter: http://twitter.com/chkal <http://twitter.com/chkal> GitHub: https://github.com/chkal <https://github.com/chkal> _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

On 17 Nov 2016, at 09:19, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: No feedback? :( Wonder if I missed something or if you think that my thoughts go in the right direction / show real problems with the discussed approach. Cheers, Hendrik > Am 07.11.2016 um 23:42 schrieb Hendrik Ebbers <hendrik.ebbers(a)me.com <mailto:hendrik.ebbers@me.com>>: > > Hi all, > I want to give some feedback to this proposal. > > Let’s start with a small example like this: > > @NotEmpty > private List<String> values; > > Here I assume that we have a @NotEmpty constraint that can be used of several types (like collections or Strings). In the example above the annotation defines some metadata for the field. By doing so you will read it as „The list should not be empty - The List should contain at least one element“. > I think this is the same syntax as it was done before and everyone will understand it. > > Here is the next example: > > private List<@NotEmpty String> desc; > > By defining it this way we have a list that can contain 0…n elements but each element must be a string with a length > 0. Since the @NotEmpty annotation now marks the generic type of the list it is related to the content of the list. > > The next example contains a mix: > > @NotEmpty > private List<@NotEmpty String> desc; > > Here 2 @NotEmpty annotations are used. In the end the field will be valid if the list contains at least one String and all Strings of the list have a length > 0. > > Based on this we could create a field that looks like this: > > private List<List<@NotEmpty List<StringProperty>>> spooky; > > Even here we can say that the field must contain a list with 0…n elements where each element contains a list that must not be empty. > > Until now I only worked with lists and here the approach is quite nice. Sadly it won’t work always. By adding an constraints annotation to the generic type of the list we know that the annotation mentions the content of the list. But this is only working because of one point: The generic type of a collection defines the type of the collection content. This works fine for collections (and for example JavaFX properties) but in other cases this will end in problems. > > Let’s say we have the following 2 interfaces: > > public interface CachedValue<V> { > V getCachedValue(); > } > > public interface RealValue<V> { > V getRealValue(); > } > > Based on this interfaces we can easily create a new class that implements both interfaces: > > public class CachableValue<V> implements CachedValue<V>, RealValue<V> { > > private V cachedValue; > > @Override > public V getCachedValue() { > return cachedValue; > } > > @Override > public V getRealValue() { > V realValue = receiveValueFromServer(); > cachedValue = realValue; > return realValue; > } > > private V receiveValueFromServer() { > return ServerConnector.getCurrentValue(); //Some fake code > } > } > > Let’s try to add a constraint annotation to validate the content of such a CachableValue: > > private CachableValue<@NotEmpty String> myValue; > > Based on this definition you have absolutely no idea if the @NotEmpty annotation is defined for the real value, the cached value or both values. From my point of view this is a big problem. Until now its was always easy to see how the validation of a model should work based on the validation information (annotations) in the model. With this new approach you have no idea what will happen. > > The most simple solution would be to add the support only to some special container / wrapper classes like collections, JavaFX properties, etc. I think this is a bad idea since new default might come to future versions of JavaSE and JavaEE (or maybe Spring) and that won’t be supported. Based on this I think that it will be a must to support all wrapper types. > > In addition I think that simply adding a constraints annotation to the generic type is wrong. All examples that we have mentioned are just perfect examples in that the generic type defines the content. Let’s say we have a class like this: > > public abstract class ConvertableInteger<V> { > > private int value; > > public void setValue(int value) { this.value = value; } > > public int getValue() { return value; } > > public abstract V getConvertedValue(); > } > > If you want to validate an instance of this class you normally want to validate the integer and not the converted value: > > private ConvertableInteger<@Min(2_000_000) Date> myValue; > > Such an expression looks like if you want to validate a Date object. In this case the better solution would be something like this: > > @Min(2_000_000) > private ConvertableInteger<Date> myValue; > > And now we end in 2 different ways how we annotation value wrappers: > > private CachableValue<@NotEmpty String> myCachedValue; > > @Min(2_000_000) > private ConvertableInteger<Date> myIntegerValue; > > And this is not the only example. We already have some classes in JavaSE that will end in such problems: > The basic Property class in JavaFX can be used like this: > > private Property<String> myStringProperty; > > Based on the current approach we should add a constraint annotation like this: > > private Property<@NotNull String> myStringProperty; > > But next to the property class JavaFX contains the class StringProperty that is a specific implementation of „Property<String>„. If we want to use this class we can not add a annotation to a generic type since we don’t have such a generic type. Here we need to do it this way: > > @NotNull > private StringProperty myStringProperty2; > > And again we have 2 different approaches: > > private Property<@NotNull String> myStringProperty; > > @NotNull > private StringProperty myStringProperty2; > > Based on this I do not think that it’s a good idea to use the constraint annotation for generic types to validate the content of a wrapper. To be true I do not have the ultimative solution for this problem. Currently I trying to add some metadata to the annotations to define it’s goal. Here is an example: > > @NotEmpty > private List<String> names1; > > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) > private List<String> names2; > > @NotEmpty > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) > private List<String> names3; > > Here the „applyTo“ value was added to the @NotEmpty annotation (like bean validation already does with the groups). By using this value a developer can specify if he wants to validate the value of the field or its internals. By doing so „names1“ is valid if the list contains at leas 1 element, „names2“ is valid if all elements in the list have at least a length of 1 (an empty list is allowed) and in „names3“ the list must contain at least one element and all elements must have a length > 0. > > A similar approach is already defined in the proposal and I understand why this is not the perfect solution since you need to define it whenever you do not want to valide a plain value: > > @NotNull(applyTo = ConstraintsApplyTo.INTERNALS) > private StringProperty myStringProperty2; > > Extracting this information to an additional (optional) annotation will be a problem since this can end in such an expression: > > @NotEmpty > @ConstraintsApplyTo(CONTAINED_VALUES) > @NotEmpty > private List<String> names3; > > By doing so you can not define the same constraints type for the container and for the containing value. In the example above the same is working. > > Next to the „applyTo“ value all the constraint annotations needs additional information to really validate all the given examples. In addition a plugin must be present that defines how we can get the internal value(s) of a class (in this case „StringProperty“). > Here are some code snippets that use additional information in the annotations to define the validation: > > > @NotEmpty > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2) > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3) > private List<List<List<StringProperty>>> spooky; > > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, valueContainerClass = RealValue.class) > private RealValue<String> myValue; > > The „applyToInternalDepth“ value is used to define how often the validator should get the internal value (depth). In the given example „@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)“ would validate the content of the „StringProperty“. In addition we need some information for the container if the container implements several interfaces. This could be defined in the annotation, too. In the example this is defined by the „valueContainerClass“ value. > > Based on this a constraint annotation needs this additional fields: > > ConstraintsApplyTo applyTo() default ConstraintsApplyTo.VALUE; // VALUE or INTERNALS > > int applyToInternalDepth() default 1; > > Class<?> valueContainerClass() default Object.class; > > As I said I don’t have a good solution :( But I think that based on the given issues simply adding annotations to the generic types isn’t the best idea. > > I think it’s quite important to have support for all the wrapper classes. Validation collections and (JavaFX) properties will be a big benefit. > > > I don’t think that support for Optional is that important since having a field of type Optional looks like an anti-pattern to me. Normally Optional should be used as a method return type and not as a type for a field. Supporting it in the bean validation might end in strange model classes. > > Example: > > The following model looks good to me: > > public class Model { > > @NotNull > private String name; > > public String getName() { return name; } > > public void setName(String name) { this.name = name; } > > public Optional<String> name() { return Optional.ofNullable(name);} > > } > > On the other hand this looks like an anti-pattern to me: > > public class Model { > > @NotNull > private Optional<String> name; > > public Optional<String> getName() { return name; } > > public void setName(String name) { this.name = Optional.ofNullable(name); } > > } > > I now that some of the code samples will not compile (for example several annotations of same type for element) but I think the code is more easy to read by doing it that way ;) > > Cheers, > > Hendrik > > >> Am 30.09.2016 um 14:35 schrieb Christian Kaltepoth <christian(a)kaltepoth.de <mailto:christian@kaltepoth.de>>: >> >> Hey Gunnar, >> >> 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. >> Good question. I reckon we'll know more after experimenting with this in the RI. >> >> I agree. The proposal mentions the possibility of having different functional rules and better performance for the single value case as reasons for the distinction. I just think that the latter one won't actually be a real problem. >> >> 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? >> Optional will be part of it by default. It should be stated somewhere, I hope :) >> >> The document explicitly states: >> >> Bean Validation will have out of the box support for containers Iterable and Map. >> >> I was expecting Optional to be listed here too. That's why I mentioned it. :-) >> >> >> Christian >> >> >> >> -- >> Christian Kaltepoth >> Blog: http://blog.kaltepoth.de/ <http://blog.kaltepoth.de/> >> Twitter: http://twitter.com/chkal <http://twitter.com/chkal> >> GitHub: https://github.com/chkal <https://github.com/chkal> >> >> _______________________________________________ >> beanvalidation-dev mailing list >> beanvalidation-dev(a)lists.jboss.org <mailto:beanvalidation-dev@lists.jboss.org> >> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev <https://lists.jboss.org/mailman/listinfo/beanvalidation-dev> > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org <mailto:beanvalidation-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: Hi all, I want to give some feedback to this proposal. Let’s start with a small example like this: @NotEmpty private List<String> values; Here I assume that we have a @NotEmpty constraint that can be used of several types (like collections or Strings). In the example above the annotation defines some metadata for the field. By doing so you will read it as „The list should not be empty - The List should contain at least one element“. I think this is the same syntax as it was done before and everyone will understand it. Here is the next example: private List<@NotEmpty String> desc; By defining it this way we have a list that can contain 0…n elements but each element must be a string with a length > 0. Since the @NotEmpty annotation now marks the generic type of the list it is related to the content of the list. The next example contains a mix: @NotEmpty private List<@NotEmpty String> desc; Here 2 @NotEmpty annotations are used. In the end the field will be valid if the list contains at least one String and all Strings of the list have a length > 0. Based on this we could create a field that looks like this: private List<List<@NotEmpty List<StringProperty>>> spooky; Even here we can say that the field must contain a list with 0…n elements where each element contains a list that must not be empty. Until now I only worked with lists and here the approach is quite nice. Sadly it won’t work always. By adding an constraints annotation to the generic type of the list we know that the annotation mentions the content of the list. But this is only working because of one point: The generic type of a collection defines the type of the collection content. This works fine for collections (and for example JavaFX properties) but in other cases this will end in problems. Let’s say we have the following 2 interfaces: public interface CachedValue<V> { V getCachedValue(); } public interface RealValue<V> { V getRealValue(); } Based on this interfaces we can easily create a new class that implements both interfaces: public class CachableValue<V> implements CachedValue<V>, RealValue<V> { private V cachedValue; @Override public V getCachedValue() { return cachedValue; } @Override public V getRealValue() { V realValue = receiveValueFromServer(); cachedValue = realValue; return realValue; } private V receiveValueFromServer() { return ServerConnector.getCurrentValue(); //Some fake code } } Let’s try to add a constraint annotation to validate the content of such a CachableValue: private CachableValue<@NotEmpty String> myValue; Based on this definition you have absolutely no idea if the @NotEmpty annotation is defined for the real value, the cached value or both values. From my point of view this is a big problem. Until now its was always easy to see how the validation of a model should work based on the validation information (annotations) in the model. With this new approach you have no idea what will happen. The most simple solution would be to add the support only to some special container / wrapper classes like collections, JavaFX properties, etc. I think this is a bad idea since new default might come to future versions of JavaSE and JavaEE (or maybe Spring) and that won’t be supported. Based on this I think that it will be a must to support all wrapper types. In addition I think that simply adding a constraints annotation to the generic type is wrong. All examples that we have mentioned are just perfect examples in that the generic type defines the content. Let’s say we have a class like this: public abstract class ConvertableInteger<V> { private int value; public void setValue(int value) { this.value = value; } public int getValue() { return value; } public abstract V getConvertedValue(); } If you want to validate an instance of this class you normally want to validate the integer and not the converted value: private ConvertableInteger<@Min(2_000_000) Date> myValue; Such an expression looks like if you want to validate a Date object. In this case the better solution would be something like this: @Min(2_000_000) private ConvertableInteger<Date> myValue; And now we end in 2 different ways how we annotation value wrappers: private CachableValue<@NotEmpty String> myCachedValue; @Min(2_000_000) private ConvertableInteger<Date> myIntegerValue; And this is not the only example. We already have some classes in JavaSE that will end in such problems: The basic Property class in JavaFX can be used like this: private Property<String> myStringProperty; Based on the current approach we should add a constraint annotation like this: private Property<@NotNull String> myStringProperty; But next to the property class JavaFX contains the class StringProperty that is a specific implementation of „Property<String>„. If we want to use this class we can not add a annotation to a generic type since we don’t have such a generic type. Here we need to do it this way: @NotNull private StringProperty myStringProperty2; And again we have 2 different approaches: private Property<@NotNull String> myStringProperty; @NotNull private StringProperty myStringProperty2; Based on this I do not think that it’s a good idea to use the constraint annotation for generic types to validate the content of a wrapper. To be true I do not have the ultimative solution for this problem. Currently I trying to add some metadata to the annotations to define it’s goal. Here is an example: @NotEmpty private List<String> names1; @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) private List<String> names2; @NotEmpty @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) private List<String> names3; Here the „applyTo“ value was added to the @NotEmpty annotation (like bean validation already does with the groups). By using this value a developer can specify if he wants to validate the value of the field or its internals. By doing so „names1“ is valid if the list contains at leas 1 element, „names2“ is valid if all elements in the list have at least a length of 1 (an empty list is allowed) and in „names3“ the list must contain at least one element and all elements must have a length > 0. A similar approach is already defined in the proposal and I understand why this is not the perfect solution since you need to define it whenever you do not want to valide a plain value: @NotNull(applyTo = ConstraintsApplyTo.INTERNALS) private StringProperty myStringProperty2; Extracting this information to an additional (optional) annotation will be a problem since this can end in such an expression: @NotEmpty @ConstraintsApplyTo(CONTAINED_VALUES) @NotEmpty private List<String> names3; By doing so you can not define the same constraints type for the container and for the containing value. In the example above the same is working. Next to the „applyTo“ value all the constraint annotations needs additional information to really validate all the given examples. In addition a plugin must be present that defines how we can get the internal value(s) of a class (in this case „StringProperty“). Here are some code snippets that use additional information in the annotations to define the validation: @NotEmpty @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2) @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3) private List<List<List<StringProperty>>> spooky; @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, valueContainerClass = RealValue.class) private RealValue<String> myValue; The „applyToInternalDepth“ value is used to define how often the validator should get the internal value (depth). In the given example „@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)“ would validate the content of the „StringProperty“. In addition we need some information for the container if the container implements several interfaces. This could be defined in the annotation, too. In the example this is defined by the „valueContainerClass“ value. Based on this a constraint annotation needs this additional fields: ConstraintsApplyTo applyTo() default ConstraintsApplyTo.VALUE; // VALUE or INTERNALS int applyToInternalDepth() default 1; Class<?> valueContainerClass() default Object.class; As I said I don’t have a good solution :( But I think that based on the given issues simply adding annotations to the generic types isn’t the best idea. I think it’s quite important to have support for all the wrapper classes. Validation collections and (JavaFX) properties will be a big benefit. I don’t think that support for Optional is that important since having a field of type Optional looks like an anti-pattern to me. Normally Optional should be used as a method return type and not as a type for a field. Supporting it in the bean validation might end in strange model classes. Example: The following model looks good to me: public class Model { @NotNull private String name; public String getName() { return name; } public void setName(String name) { this.name = name; } public Optional<String> name() { return Optional.ofNullable(name);} } On the other hand this looks like an anti-pattern to me: public class Model { @NotNull private Optional<String> name; public Optional<String> getName() { return name; } public void setName(String name) { this.name = Optional.ofNullable(name); } } I now that some of the code samples will not compile (for example several annotations of same type for element) but I think the code is more easy to read by doing it that way ;) Cheers, Hendrik > Am 30.09.2016 um 14:35 schrieb Christian Kaltepoth <christian(a)kaltepoth.de <mailto:christian@kaltepoth.de>>: > > Hey Gunnar, > > 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. > Good question. I reckon we'll know more after experimenting with this in the RI. > > I agree. The proposal mentions the possibility of having different functional rules and better performance for the single value case as reasons for the distinction. I just think that the latter one won't actually be a real problem. > > 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? > Optional will be part of it by default. It should be stated somewhere, I hope :) > > The document explicitly states: > > Bean Validation will have out of the box support for containers Iterable and Map. > > I was expecting Optional to be listed here too. That's why I mentioned it. :-) > > > Christian > > > > -- > Christian Kaltepoth > Blog: http://blog.kaltepoth.de/ <http://blog.kaltepoth.de/> > Twitter: http://twitter.com/chkal <http://twitter.com/chkal> > GitHub: https://github.com/chkal <https://github.com/chkal> > > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org <mailto:beanvalidation-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

On 21 Nov 2016, at 14:07, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: > On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com <mailto:hendrik.ebbers@me.com>> wrote: > > Let’s say we have the following 2 interfaces: > > public interface CachedValue<V> { > V getCachedValue(); > } > > public interface RealValue<V> { > V getRealValue(); > } > > Based on this interfaces we can easily create a new class that implements both interfaces: > > public class CachableValue<V> implements CachedValue<V>, RealValue<V> { > > private V cachedValue; > > @Override > public V getCachedValue() { > return cachedValue; > } > > @Override > public V getRealValue() { > V realValue = receiveValueFromServer(); > cachedValue = realValue; > return realValue; > } > > private V receiveValueFromServer() { > return ServerConnector.getCurrentValue(); //Some fake code > } > } > > Let’s try to add a constraint annotation to validate the content of such a CachableValue: > > private CachableValue<@NotEmpty String> myValue; > > Based on this definition you have absolutely no idea if the @NotEmpty annotation is defined for the real value, the cached value or both values. From my point of view this is a big problem. Until now its was always easy to see how the validation of a model should work based on the validation information (annotations) in the model. With this new approach you have no idea what will happen. > > The most simple solution would be to add the support only to some special container / wrapper classes like collections, JavaFX properties, etc. I think this is a bad idea since new default might come to future versions of JavaSE and JavaEE (or maybe Spring) and that won’t be supported. Based on this I think that it will be a must to support all wrapper types. Interesting. I did think about extractors of subtypes but not extractors of parallel types. I have been thinking about it and explored a few paths: - validating all parallel extractors - use one - validate none - fail In the end, I think the cleanest solution is to follow what Ceylon and Java do for default methods, they don’t allow ambiguous use and force a redefinition. http://stackoverflow.com/questions/16764791/how-does-java-8-new-default-i... <http://stackoverflow.com/questions/16764791/how-does-java-8-new-default-i... In practice for us, if there are extractors for CachedValue and for RealValue, then CachableValue should fail and require the definition of an explicit CachableValue extractor. What is interesting in this case is that CachableValue extractor can be see as either: - a ManyContainerValuesExtractor that will return getCachedValue and getRealValue just like a Collection would return get(0) and get(1). This approach does not reflect the path difference though - or two SingleContainerValueExtractor from the same container type and the same value type (*) (*) this is something we do not allow for the moment, we have a unique extractor per container + value type. We need to think about how to make that work and still allow people to override extractors. So back to your original concern of uncertainty, I think forcing a more specific extractor solves the uncertainty that you mentioned. Granted, the user would have to know the extractor(s) behavior but in many ways, people do need to know about that regardless. Thoughts?

In Therian there is a custom annotation to specify a method that provides a Type value at runtime. Specifically It allows one to designate a method that returns an instance of the Typed interface from Apache Commons Lang 3.x. Annotation: https://github.com/mbenson/therian/blob/master/core/src/ main/java/therian/BindTypeVariable.java Example: https://github.com/mbenson/therian/blob/master/core/src/ main/java/therian/operation/Transform.java Utilities: https://github.com/mbenson/therian/blob/master/core/src/ main/java/therian/util/Types.java#L88 An approach like this would of course put some onus onto users with the payoff of being very explicit. In any case, it might provide food for thought on the subject. Matt On Tue, Nov 22, 2016 at 1:15 PM, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: > > On 21 Nov 2016, at 14:07, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: > > > On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: > > Let’s say we have the following 2 interfaces: > > public interface CachedValue<V> { > V getCachedValue(); > } > > public interface RealValue<V> { > V getRealValue(); > } > > Based on this interfaces we can easily create a new class that implements > both interfaces: > > public class CachableValue<V> implements CachedValue<V>, RealValue<V> { > > private V cachedValue; > > @Override > public V getCachedValue() { > return cachedValue; > } > > @Override > public V getRealValue() { > V realValue = receiveValueFromServer(); > cachedValue = realValue; > return realValue; > } > > private V receiveValueFromServer() { > return ServerConnector.getCurrentValue(); //Some fake code > } > } > > Let’s try to add a constraint annotation to validate the content of such a > CachableValue: > > private CachableValue<@NotEmpty String> myValue; > > Based on this definition you have absolutely no idea if the @NotEmpty > annotation is defined for the real value, the cached value or both values. > From my point of view this is a big problem. Until now its was always easy > to see how the validation of a model should work based on the validation > information (annotations) in the model. With this new approach you have no > idea what will happen. > > The most simple solution would be to add the support only to some special > container / wrapper classes like collections, JavaFX properties, etc. I > think this is a bad idea since new default might come to future versions of > JavaSE and JavaEE (or maybe Spring) and that won’t be supported. Based on > this I think that it will be a must to support all wrapper types. > > > Interesting. > > I did think about extractors of subtypes but not extractors of parallel > types. > I have been thinking about it and explored a few paths: > - validating all parallel extractors > - use one > - validate none > - fail > > In the end, I think the cleanest solution is to follow what Ceylon and Java > do for default methods, they don’t allow ambiguous use and force a > redefinition. > http://stackoverflow.com/questions/16764791/how-does- java-8-new-default-interface-model-works-incl-diamond- multiple-inhe#16788295 > > In practice for us, if there are extractors for CachedValue and for > RealValue, then CachableValue should fail and require the definition of an > explicit CachableValue extractor. > What is interesting in this case is that CachableValue extractor can be see > as either: > - a ManyContainerValuesExtractor that will return getCachedValue and > getRealValue just like a Collection would return get(0) and get(1). This > approach does not reflect the path difference though > - or two SingleContainerValueExtractor from the same container type and the > same value type (*) > > (*) this is something we do not allow for the moment, we have a unique > extractor per container + value type. We need to think about how to make > that work and still allow people to override extractors. > > So back to your original concern of uncertainty, I think forcing a more > specific extractor solves the uncertainty that you mentioned. Granted, the > user would have to know the extractor(s) behavior but in many ways, people > do need to know about that regardless. > > Thoughts? > > > Gunnar and I discussed this problem further and the proposal at large, in > particular in the light of his alternative proposal (section 3). > > The main question we have is how common are constraints on user defined type > like: > > - CachableValue<V> // CacheableValue<@Email String> value; > - Tuple<V1,V2> // Tuple<@Email String, @Valid Address> userIdAndAddress; > - … > > Here I am excluding: > - Optional > - Collection and Map (and all Iterable really) > - even Collections of non Java languages (Scala, Ceylon, Kotlin if they have > their own collection type, etc) > > If constraints on the user defined types described above are common, then we > need a more systematic solution than imposing to write an extractor for each > of these cases. > If they are uncommon and the only containers are Optional, Collection, Map, > JavaFX containers and other language collections, or more generally > framework provided containers, then a requiring an extractor implementation > is not too problematic. > > > To put some context, the overall BVAL-508 proposal addresses several things: > - how to extract value from a container > - where to express constraints on the values of a container > - how and when the cascading logic should work > > In my proposal. extractors are explicit implementations (with built-in as > well as custom implementations). The extractors: > - extract the value(s) > - express the link between the extractor and the type parameter (and thus > the parameterized type) > - must be implemented explicitly for each tuple extractor / type parameter > > Gunnar has an alternative proposal or rather a proposal that builds on top > of the existing one but provide a default generic extractor logic. > > If no explicit extractor exists, the following will happen > > Assuming > > Tuple<@Email String, @Valid Address> userIdAndAddress; > > class Tuple<V1, V2> { > V1 v1; > V2 v2; > > V1 getV1() { return v1; }; > V2 getV2() { return v2; }; > } > > The default extractor would look for all getters that return the > parameterized type(s) and use the getter as extractor. It then would apply > the constraint. > In our case, the code would validate getV1() against @Email and getV2() will > be cascaded. > > This is elegant and extremely regular. But it has a very ugly angle: should > we validate the getters or the fields or the getters then the fields? > > The rule proposed by Gunnar is to validate the getters first because he > likes them better and then look for fields with no corresponding getter and > validate them. > > Assuming > > Tuple<@Email String, @Valid Address> userIdAndAddress; > > class Tuple<V1, V2> { > V1 v1; > V1 v1b; > V2 v2; > > V1 getV1() { return v1; }; > V2 getV2() { return v2; }; > } > > In this example, the code would validate getV1() against @Email, getV2() > will be cascaded and v1b will be validated against @Email. > > This alternative proposal has two advantages: > - it reduces the number of necessary extractor implementations (assuming the > relevant elements have getters) > - it makes the problem described by Hendrik with CacheableValue fully > deterministic > - it makes the logic of cascading very regular (for non collection at least) > - it still requires the ability to write custom extractors for collection > type containers > > I have to admit I hate the arbitrary logic of choosing getters and then > fields. > > So my questions to you are: > - how common do you see the Tuple, CacheableValue and other user defined > type needing validation? > - would you be satisfied by the getter approach as an extractor? > - what do you think of the getter priority logic? > > What do you think? > > Emmanuel > > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: In addition I think that simply adding a constraints annotation to the generic type is wrong. All examples that we have mentioned are just perfect examples in that the generic type defines the content. Let’s say we have a class like this: public abstract class ConvertableInteger<V> { private int value; public void setValue(int value) { this.value = value; } public int getValue() { return value; } public abstract V getConvertedValue(); } If you want to validate an instance of this class you normally want to validate the integer and not the converted value: private ConvertableInteger<@Min(2_000_000) Date> myValue; Such an expression looks like if you want to validate a Date object. In this case the better solution would be something like this: @Min(2_000_000) private ConvertableInteger<Date> myValue; And now we end in 2 different ways how we annotation value wrappers: private CachableValue<@NotEmpty String> myCachedValue; @Min(2_000_000) private ConvertableInteger<Date> myIntegerValue;

Just replying to this one: > == Constraint on non parameterized containers > I am liking option 3 less and less and as far as I can remember, it does not rely on an explicitly > identified use case. Who is wanting to support that usage, vs restrict the container feature to > parameterized containers? I'm not sure where this requirement is coming from. Wouldn't it simply be addressed by providing a validator for @Min + IntegerContainer? I.e. do we need any explicit support for this at all compared to what's possible today?

Also the JavaFX case (your 2nd example around StringProperty) could be satisfied by just adding explicit validators for StringProperty, IntegerProperty etc. Agreed it's not as elegant/generic and wouldn't work for custom types. May still be a way forward, though.

On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: I don’t think that support for Optional is that important since having a field of type Optional looks like an anti-pattern to me. Normally Optional should be used as a method return type and not as a type for a field. Supporting it in the bean validation might end in strange model classes. Example: The following model looks good to me: public class Model { @NotNull private String name; public String getName() { return name; } public void setName(String name) { this.name = name; } public Optional<String> name() { return Optional.ofNullable(name);} } On the other hand this looks like an anti-pattern to me: public class Model { @NotNull private Optional<String> name; public Optional<String> getName() { return name; } public void setName(String name) { this.name = Optional.ofNullable(name); } } Yes Optional on a property is deemed an anti pattern by the JDK team but since Bean Validation supports contraints on method parameters and return values, this is still a valid use case Optional<@Email String> getUserEmail(@NotNull UUID userId); Emmanuel _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

Am 21.11.2016 um 15:51 schrieb Matt Benson <mbenson(a)apache.org&gt;: On Mon, Nov 21, 2016 at 8:18 AM, Gunnar Morling <gunnar(a)hibernate.org <mailto:gunnar@hibernate.org>> wrote: > Yes, probably we should not support Optional for fields. Also parameters are > dubious (the JDK team discourages that afaik). But supporting it for return > values (and getter-based properties) seems reasonable. +1 Matt (with apologies for not having participated earlier in the spec 2.0 process) > > 2016-11-21 14:32 GMT+01:00 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: >> >> >> On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: >> >> I don’t think that support for Optional is that important since having a >> field of type Optional looks like an anti-pattern to me. Normally Optional >> should be used as a method return type and not as a type for a field. >> Supporting it in the bean validation might end in strange model classes. >> >> Example: >> >> The following model looks good to me: >> >> public class Model { >> >> @NotNull >> private String name; >> >> public String getName() { return name; } >> >> public void setName(String name) { this.name = name; } >> >> public Optional<String> name() { return Optional.ofNullable(name);} >> >> } >> >> On the other hand this looks like an anti-pattern to me: >> >> public class Model { >> >> @NotNull >> private Optional<String> name; >> >> public Optional<String> getName() { return name; } >> >> public void setName(String name) { this.name = Optional.ofNullable(name); >> } >> >> } >> >> >> Yes Optional on a property is deemed an anti pattern by the JDK team but >> since Bean Validation supports contraints on method parameters and return >> values, this is still a valid use case >> >> Optional<@Email String> getUserEmail(@NotNull UUID userId); >> >> Emmanuel >> >> >> _______________________________________________ >> beanvalidation-dev mailing list >> beanvalidation-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > > > > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org <mailto:beanvalidation-dev@lists.jboss.org> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev <https://lists.jboss.org/mailman/listinfo/beanvalidation-dev>

What’s the difference between a field and a getter based property? It is the same for all intended purposes in Bean Validation.

> On 21 Nov 2016, at 15:51, Matt Benson <mbenson(a)apache.org&gt; wrote: > > On Mon, Nov 21, 2016 at 8:18 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: >> Yes, probably we should not support Optional for fields. Also parameters are >> dubious (the JDK team discourages that afaik). But supporting it for return >> values (and getter-based properties) seems reasonable. > > +1 > > Matt (with apologies for not having participated earlier in the spec > 2.0 process) > >> >> 2016-11-21 14:32 GMT+01:00 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: >>> >>> >>> On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: >>> >>> I don’t think that support for Optional is that important since having a >>> field of type Optional looks like an anti-pattern to me. Normally Optional >>> should be used as a method return type and not as a type for a field. >>> Supporting it in the bean validation might end in strange model classes. >>> >>> Example: >>> >>> The following model looks good to me: >>> >>> public class Model { >>> >>> @NotNull >>> private String name; >>> >>> public String getName() { return name; } >>> >>> public void setName(String name) { this.name = name; } >>> >>> public Optional<String> name() { return Optional.ofNullable(name);} >>> >>> } >>> >>> On the other hand this looks like an anti-pattern to me: >>> >>> public class Model { >>> >>> @NotNull >>> private Optional<String> name; >>> >>> public Optional<String> getName() { return name; } >>> >>> public void setName(String name) { this.name = Optional.ofNullable(name); >>> } >>> >>> } >>> >>> >>> Yes Optional on a property is deemed an anti pattern by the JDK team but >>> since Bean Validation supports contraints on method parameters and return >>> values, this is still a valid use case >>> >>> Optional<@Email String> getUserEmail(@NotNull UUID userId); >>> >>> Emmanuel >>> >>> >>> _______________________________________________ >>> beanvalidation-dev mailing list >>> beanvalidation-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev >> >> >> >> _______________________________________________ >> beanvalidation-dev mailing list >> beanvalidation-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

Agreed that it can be made work for all cases (the RI does it already more or less). Question is whether by supporting e.g. Optional fields we encourage modelling styles that are discouraged by the JDK team. But maybe that's just a question of documentation/wording, i.e. we could support it everywhere but only show usage on return values / getter props in the spec.

On 21 Nov 2016, at 17:25, Christian Kaltepoth <christian(a)kaltepoth.de&gt; wrote: Not sure I like the idea of supporting Optional<T> for return types but not for fields and method parameters. Is there any technical reason for this restriction? I agree that using Optional<T> for fields is an anti-pattern, but I don't see any reason for not supporting it nevertheless. Am I missing something here? 2016-11-21 15:18 GMT+01:00 Gunnar Morling <gunnar(a)hibernate.org <mailto:gunnar@hibernate.org>>: Yes, probably we should not support Optional for fields. Also parameters are dubious (the JDK team discourages that afaik). But supporting it for return values (and getter-based properties) seems reasonable. 2016-11-21 14:32 GMT+01:00 Emmanuel Bernard <emmanuel(a)hibernate.org <mailto:emmanuel@hibernate.org>>: > On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com <mailto:hendrik.ebbers@me.com>> wrote: > > I don’t think that support for Optional is that important since having a field of type Optional looks like an anti-pattern to me. Normally Optional should be used as a method return type and not as a type for a field. Supporting it in the bean validation might end in strange model classes. > > Example: > > The following model looks good to me: > > public class Model { > > @NotNull > private String name; > > public String getName() { return name; } > > public void setName(String name) { this.name <http://this.name/> = name; } > > public Optional<String> name() { return Optional.ofNullable(name);} > > } > > On the other hand this looks like an anti-pattern to me: > > public class Model { > > @NotNull > private Optional<String> name; > > public Optional<String> getName() { return name; } > > public void setName(String name) { this.name <http://this.name/> = Optional.ofNullable(name); } > > } Yes Optional on a property is deemed an anti pattern by the JDK team but since Bean Validation supports contraints on method parameters and return values, this is still a valid use case Optional<@Email String> getUserEmail(@NotNull UUID userId); Emmanuel _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org <mailto:beanvalidation-dev@lists.jboss.org> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev <https://lists.jboss.org/mailman/listinfo/beanvalidation-dev> _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org <mailto:beanvalidation-dev@lists.jboss.org> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev <https://lists.jboss.org/mailman/listinfo/beanvalidation-dev> -- Christian Kaltepoth Blog: http://blog.kaltepoth.de/ <http://blog.kaltepoth.de/> Twitter: http://twitter.com/chkal <http://twitter.com/chkal> GitHub: https://github.com/chkal <https://github.com/chkal> _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: And this is not the only example. We already have some classes in JavaSE that will end in such problems: The basic Property class in JavaFX can be used like this: private Property<String> myStringProperty; Based on the current approach we should add a constraint annotation like this: private Property<@NotNull String> myStringProperty; But next to the property class JavaFX contains the class StringProperty that is a specific implementation of „Property<String>„. If we want to use this class we can not add a annotation to a generic type since we don’t have such a generic type. Here we need to do it this way: @NotNull private StringProperty myStringProperty2; And again we have 2 different approaches: private Property<@NotNull String> myStringProperty; @NotNull private StringProperty myStringProperty2; Based on this I do not think that it’s a good idea to use the constraint annotation for generic types to validate the content of a wrapper. General comment on you entire email. I think you are throwing the baby out with the bath water. Granted, more complicated cases don’t scale as nicely as we would hope but let’s get back tot he basics. 95%+ of this feature usage will be done for collections and maps. Based on this, I’d be a bit unhappy at giving up parameterized type use location for landing that annotations. This is sooooo natural. Our job is to make the common case simple and ideally make it scale in a not too ugly way and not close doors to the most complex use cases. And if one thing has to break, we can drop some of the less common cases.

parameterized type use but offers a backup plan for when the parameterized type use location is not present (like specialized subclasses of Java FX). Java FX is a clear cut as the wrapper is purely technical and all of the constraints do apply to the wrapped value. But everywhere we have a parameterized type use, we should have the app developer make use of them instead of using the @ConstraintAppliesTo workaround. To be true I do not have the ultimative solution for this problem. Currently I trying to add some metadata to the annotations to define it’s goal. Here is an example: @NotEmpty private List<String> names1; @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) private List<String> names2; @NotEmpty @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) private List<String> names3; Here the „applyTo“ value was added to the @NotEmpty annotation (like bean validation already does with the groups). By using this value a developer can specify if he wants to validate the value of the field or its internals. By doing so „names1“ is valid if the list contains at leas 1 element, „names2“ is valid if all elements in the list have at least a length of 1 (an empty list is allowed) and in „names3“ the list must contain at least one element and all elements must have a length > 0. A similar approach is already defined in the proposal and I understand why this is not the perfect solution since you need to define it whenever you do not want to valide a plain value: @NotNull(applyTo = ConstraintsApplyTo.INTERNALS) private StringProperty myStringProperty2; Extracting this information to an additional (optional) annotation will be a problem since this can end in such an expression: @NotEmpty @ConstraintsApplyTo(CONTAINED_VALUES) @NotEmpty private List<String> names3; By doing so you can not define the same constraints type for the container and for the containing value. In the example above the same is working. We did explore per annotation location overriding. This is a question in the initial proposal. The reason it did not make the first cut is as you have seen: - you need a lot of information on side that it not trivial to digest - each existing constraint annotation will have to be changed to add these new attributes - on that front, the spec has declared that attribute names starting with “valid” are reserved for the spec, so your proposal would have to have attributes prefixed with “valid” not to break existing code - at the end of the day, the feature is really for code using type use, so we are twisting it for edge cases But I am very open to the idea of doing per annotation location overriding if people think that they are an important feature. Next to the „applyTo“ value all the constraint annotations needs additional information to really validate all the given examples. In addition a plugin must be present that defines how we can get the internal value(s) of a class (in this case „StringProperty“). Here are some code snippets that use additional information in the annotations to define the validation: @NotEmpty @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2) @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3) private List<List<List<StringProperty>>> spooky; @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, valueContainerClass = RealValue.class) private RealValue<String> myValue; The „applyToInternalDepth“ value is used to define how often the validator should get the internal value (depth). In the given example „@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)“ would validate the content of the „StringProperty“. In addition we need some information for the container if the container implements several interfaces. This could be defined in the annotation, too. In the example this is defined by the „valueContainerClass“ value. Hopefully we can converge on my idea not to have to add valueContainerClass (see the other emails). But you do need a way to identify the correct type parameter targeted. This is equivalent to the @ExtractedValue which today has typeParameterHost and either typeParameterIndex or typeParameterName to uniquely identify them. On the depth approach, your example would be more easily rewritten as @NotEmpty List<@NotEmpty List<@NotEmpty List<@NotEmpty StringProperty>>> spooky; which clarifies everything. But let’s assume we have a type ListOfListOfListOfStringProperty type. do we really want to support @NotEmpty @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2) @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3) private ListOfListOfListOfStringProperty spooky; or even @NotEmpty // support nested levels by positioning ConstraintsApplyTo at the right level private List<@NotEmpty List<@NotEmpty @NotEmpty(applyTo = ConstraintsApplyTo.CONTAINED_VALUES) ListOfStringProperty>> spooky; Or do we throw the towel and say that nested levels are not supported for non parameterized type use locations? What I am realizing now is that this option is available essentially to support containers that do not offer parameterized type. And in a previous email, I am questioning that use case.

Based on this a constraint annotation needs this additional fields: ConstraintsApplyTo applyTo() default ConstraintsApplyTo.VALUE; // VALUE or INTERNALS int applyToInternalDepth() default 1; Class<?> valueContainerClass() default Object.class; As I said I don’t have a good solution :( But I think that based on the given issues simply adding annotations to the generic types isn’t the best idea. I think it’s quite important to have support for all the wrapper classes. Validation collections and (JavaFX) properties will be a big benefit. _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

On 23 Nov 2016, at 09:06, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: 2016-11-21 17:08 GMT+01:00 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: > > On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com&gt; wrote: > > Next to the „applyTo“ value all the constraint annotations needs > additional information to really validate all the given examples. In > addition a plugin must be present that defines how we can get the internal > value(s) of a class (in this case „StringProperty“). > Here are some code snippets that use additional information in the > annotations to define the validation: > > > @NotEmpty > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = > 2) > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = > 3) > private List<List<List<StringProperty>>> spooky; > > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, valueContainerClass = > RealValue.class) > private RealValue<String> myValue; > > The „applyToInternalDepth“ value is used to define how often the > validator should get the internal value (depth). In the given example > „@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth > = 3)“ would validate the content of the „StringProperty“. In addition we > need some information for the container if the container implements several > interfaces. This could be defined in the annotation, too. In the example > this is defined by the „valueContainerClass“ value. > > > Hopefully we can converge on my idea not to have to > add valueContainerClass (see the other emails). But you do need a way to > identify the correct type parameter targeted. This is equivalent to the > @ExtractedValue which today has typeParameterHost and > either typeParameterIndex or typeParameterName to uniquely identify them. > > On the depth approach, your example would be more easily rewritten as > > @NotEmpty List<@NotEmpty List<@NotEmpty List<@NotEmpty > StringProperty>>> spooky; > > which clarifies everything. But let’s assume we have a type > ListOfListOfListOfStringProperty type. do we really want to support > > @NotEmpty > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS) > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = > 2) > @NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = > 3) > private ListOfListOfListOfStringProperty spooky; > > or even > > @NotEmpty > // support nested levels by positioning ConstraintsApplyTo at the right > level > private List<@NotEmpty List<@NotEmpty @NotEmpty(applyTo = > ConstraintsApplyTo.CONTAINED_VALUES) ListOfStringProperty>> spooky; > > Or do we throw the towel and say that nested levels are not supported for > non parameterized type use locations? What I am realizing now is that this > option is available essentially to support containers that do not offer > parameterized type. And in a previous email, I am questioning that use case. > I'm also not convinced of this. In a way it seems to me like breaking encapsulation. When dealing with a type such as ListOfStringListsProperty, you don't know it's inner works from a variable declaration. Hence I'm doubting that you should be allowed to apply constraints to them from the outside. It's getting more apparent when thinking of more real-world domain-specific types: public class CustomerContacts { List<List<String>> contacts; // one single inner list contains the contacts valid at a given time } I don't think it makes sense to allow application of constraints "from the outside" when dealing with a property of CustomerContacts. It makes assumptions on the implementation of that type which is a dangerous thing to do. What if the implementation of CustomerContacts changes to be map-based? Existing constraint configurations given at the usage side would have to be adapted then. Instead the constraints should be declared within CustomerContacts itself. It's different for generic types. When dealing with a type like List<List<String>>, its "inner workings" are apparent to the user, i.e. it's reasonable to allow them to apply constraints to the elements of the inner list for instance. Ah interesting point of view. I’ve never thought of it that way. I think what I had in mind when I said containers with no parameterized type is the Java 4 style collections that only have raw type support. @Email(validApplyTo=ConsgraintsApplyTo.CONTAINED_VALUE) Collection emails; But back to the question, is that even a valid use case in 2016? _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

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.

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?

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.

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.

This is *super* complex. I wonder if we could get Alex Buckley and/or Michael Ernst to take a look at it to make sure we're going on the right direction. What do you think? PS: If they can't, I'll need a few hours of detailed reading to provide any meaningful feedback. Regards, Michael On Mon, Sep 19, 2016 at 8:31 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > +1 > > If everyone could take a look at this one, that'd be great! It's a bit more > complex, so the more eyes we get on this one, the better. > > Thanks! > > 2016-09-19 13:28 GMT+02:00 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: >> >> This is probably going to be most visible feature of Bean Validation >> 2.0. We particularly need your feedback and involvement on this one. >> >> Emmanuel >> >> On Tue 2016-09-06 18:12, Emmanuel Bernard wrote: >> >Hi all, >> > >> >I and a few others have been working on a proposal to support things >> >like Collection<@Email String> and Optional<@Email String>. This is >> >more complicated that it seems at first glance. >> > >> >Instead of doing an ad-hoc support for the various collection types, >> >Optional and the JavaFX Properties, we quickly decided to define the >> >notion of container and the ability to declare constraints on contained >> >elements to validate them. >> > >> >This lead to two main proposals that you can read at >> >http://beanvalidation.org/proposals/BVAL-508/ >> > >> >This is a relatively long read, you can start by ignoring "alternative" >> >options for your first pass. We are very interested in feedback at this >> >stage as we have been pushing these proposal very far already and they >> >would need to become part of the spec as next step. >> > >> >Let me know of what you think, questions, remarks etc. >> > >> >In particular, I'm interested in what you think of the following. >> > >> >The capability to define custom containers. >> > >> >The extractor approach vs its alternative. >> > >> >The concepts of @ConstraintsAppliesTo(COMTAINED) used for JavaFX and for >> >subclasses of containers. >> > >> >@Valid, in particular the legacy and new forms and how to handle the >> >transition. >> > >> >And finally, but a big one, what do you think of proposal 1 vs proposal >> >2. The latter being more generic but with more open questions (and a >> >less elaborated at this stage). >> > >> >Emmanuel >> >_______________________________________________ >> >beanvalidation-dev mailing list >> &gt;beanvalidation-dev(a)lists.jboss.org >> >https://lists.jboss.org/mailman/listinfo/beanvalidation-dev >> _______________________________________________ >> beanvalidation-dev mailing list >> beanvalidation-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > > > > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev