<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">It’s all my fault. I’ve been (un)focused on a lot of non related Bean Validation workload. and all feedback from Sept 25th onward is still in my queue to process.<div class="">Working hard to come back to this one , Gunnar is also putting pressure on me :)</div><div class=""><br class=""></div><div class="">Emmanuel</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 17 Nov 2016, at 09:19, Hendrik Ebbers <<a href="mailto:hendrik.ebbers@me.com" class="">hendrik.ebbers@me.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">No feedback? :(<div class="">Wonder if I missed something or if you think that my thoughts go in the right direction / show real problems with the discussed approach.</div><div class=""><br class=""></div><div class="">Cheers,</div><div class=""><br class=""></div><div class="">Hendrik</div><div class=""><br class=""></div><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">Am 07.11.2016 um 23:42 schrieb Hendrik Ebbers <<a href="mailto:hendrik.ebbers@me.com" class="">hendrik.ebbers@me.com</a>>:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi all,<br class="">I want to give some feedback to this proposal. <br class=""><br class="">Let’s start with a small example like this:<br class=""><br class=""><font face="Courier New" class="">@NotEmpty<br class="">private List<String> values;</font><br class=""><br class="">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“.<br class="">I think this is the same syntax as it was done before and everyone will understand it.<br class=""><br class="">Here is the next example:<br class=""><br class=""><font face="Courier New" class="">private List<@NotEmpty String> desc;</font><br class=""><br class="">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.<br class=""><br class="">The next example contains a mix:<br class=""><br class=""><font face="Courier New" class="">@NotEmpty<br class="">private List<@NotEmpty String> desc;</font><br class=""><br class="">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.<br class=""><br class="">Based on this we could create a field that looks like this:<br class=""><br class=""><font face="Courier New" class="">private List<List<@NotEmpty List<StringProperty>>> spooky;</font><br class=""><br class="">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.<br class=""><br class="">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.<br class=""><br class="">Let’s say we have the following 2 interfaces:<br class=""><br class=""><font face="Courier New" class="">public interface CachedValue<V> {<br class=""> V getCachedValue();<br class="">}<br class=""><br class="">public interface RealValue<V> {<br class=""> V getRealValue();<br class="">}</font><br class=""><br class="">Based on this interfaces we can easily create a new class that implements both interfaces:<br class=""><br class=""><font face="Courier New" class="">public class CachableValue<V> implements CachedValue<V>, RealValue<V> {<br class=""><br class=""> private V cachedValue;<br class=""><br class=""> @Override<br class=""> public V getCachedValue() {<br class=""> return cachedValue;<br class=""> }<br class=""><br class=""> @Override<br class=""> public V getRealValue() {<br class=""> V realValue = receiveValueFromServer();<br class=""> cachedValue = realValue;<br class=""> return realValue;<br class=""> }<br class=""><br class=""> private V receiveValueFromServer() {<br class=""> return ServerConnector.getCurrentValue(); //Some fake code<br class=""> }<br class="">}</font><br class=""><br class="">Let’s try to add a constraint annotation to validate the content of such a CachableValue:<br class=""><br class=""><font face="Courier New" class="">private CachableValue<@NotEmpty String> myValue;</font><br class=""><br class="">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.<br class=""><br class="">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. <br class=""><br class="">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:<br class=""><br class=""><font face="Courier New" class="">public abstract class ConvertableInteger<V> {<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>private int value;<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public void setValue(int value) { this.value = value; }<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public int getValue() { return value; }<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public abstract V getConvertedValue();<br class="">}</font><br class=""><br class="">If you want to validate an instance of this class you normally want to validate the integer and not the converted value:<br class=""><br class=""><font face="Courier New" class="">private ConvertableInteger<@Min(2_000_000) Date> myValue;</font><br class=""><br class="">Such an expression looks like if you want to validate a Date object. In this case the better solution would be something like this:<br class=""><br class=""><font face="Courier New" class="">@Min(2_000_000)<br class="">private ConvertableInteger<Date> myValue;</font><br class=""><br class="">And now we end in 2 different ways how we annotation value wrappers:<br class=""><br class=""><font face="Courier New" class="">private CachableValue<@NotEmpty String> myCachedValue;<br class=""><br class="">@Min(2_000_000)<br class="">private ConvertableInteger<Date> myIntegerValue;</font><br class=""><br class="">And this is not the only example. We already have some classes in JavaSE that will end in such problems:<br class="">The basic Property class in JavaFX can be used like this:<br class=""><br class=""><font face="Courier New" class="">private Property<String> myStringProperty;</font><br class=""><br class="">Based on the current approach we should add a constraint annotation like this:<br class=""><font face="Courier New" class=""><br class="">private Property<@NotNull String> myStringProperty;</font><br class=""><br class="">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:<br class=""><br class=""><font face="Courier New" class="">@NotNull <br class="">private StringProperty myStringProperty2;</font><br class=""><br class="">And again we have 2 different approaches:<br class=""><br class=""><font face="Courier New" class="">private Property<@NotNull String> myStringProperty;<br class=""><br class="">@NotNull<br class="">private StringProperty myStringProperty2;</font><br class=""><br class="">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:<br class=""><br class=""><font face="Courier New" class="">@NotEmpty<br class="">private List<String> names1;<br class=""><br class="">@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)<br class="">private List<String> names2;<br class=""><br class="">@NotEmpty<br class="">@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)<br class="">private List<String> names3;</font><br class=""><br class="">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.<br class=""><br class="">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:<br class=""><br class=""><font face="Courier New" class="">@NotNull(applyTo = ConstraintsApplyTo.INTERNALS)<br class="">private StringProperty myStringProperty2;</font><div class=""><br class=""></div><div class="">Extracting this information to an additional (optional) annotation will be a problem since this can end in such an expression:</div><div class=""><br class=""></div><div class=""><span style="font-family: 'Courier New';" class="">@NotEmpty</span></div><div class=""><font face="Courier New" class="">@ConstraintsApplyTo(CONTAINED_VALUES)</font><br style="font-family: 'Courier New';" class=""><span style="font-family: 'Courier New';" class="">@NotEmpty</span><br style="font-family: 'Courier New';" class=""><span style="font-family: 'Courier New';" class="">private </span><span style="font-family: 'Courier New';" class="">List<String> </span><span style="font-family: 'Courier New';" class="">names3</span><span style="font-family: 'Courier New';" class="">;</span></div><div class=""><br class=""></div><div class="">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. </div><div class=""><br class="">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“).<br class="">Here are some code snippets that use additional information in the annotations to define the validation:<br class=""><br class=""><br class=""><font face="Courier New" class="">@NotEmpty<br class="">@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)<br class="">@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2)<br class="">@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)<br class="">private List<List<List<StringProperty>>> spooky;<br class=""><br class="">@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, valueContainerClass = RealValue.class)<br class="">private RealValue<String> myValue;</font><br class=""><br class="">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.<br class=""><br class="">Based on this a constraint annotation needs this additional fields:<br class=""><br class=""><font face="Courier New" class="">ConstraintsApplyTo applyTo() default ConstraintsApplyTo.VALUE; // VALUE or INTERNALS<br class=""><br class="">int applyToInternalDepth() default 1;<br class=""><br class="">Class<?> valueContainerClass() default Object.class;</font><br class=""><br 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.<br class=""><br class="">I think it’s quite important to have support for all the wrapper classes. Validation collections and (JavaFX) properties will be a big benefit. <br class=""><br class=""><br class="">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. <br class=""><br class="">Example:<br class=""><br class="">The following model looks good to me:<br class=""><br class=""><font face="Courier New" class="">public class Model {<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>@NotNull<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>private String name; <br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public String getName() { return name; }<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public void setName(String name) { this.name = name; }<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public Optional<String> name() { return Optional.ofNullable(name);}<br class=""><br class="">}</font><br class=""><br class="">On the other hand this looks like an anti-pattern to me:<br class=""><br class=""><font face="Courier New" class="">public class Model {<br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>@NotNull<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>private Optional<String> name; <br class=""><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public Optional<String> getName() { return name; }<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span><br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>public void setName(String name) { this.name = Optional.ofNullable(name); }<br class=""><br class="">}<br class=""></font><br class="">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 ;)<br class=""><br class="">Cheers,<br class=""><br class="">Hendrik<br class=""><br class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">Am 30.09.2016 um 14:35 schrieb Christian Kaltepoth <<a href="mailto:christian@kaltepoth.de" class="">christian@kaltepoth.de</a>>:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hey Gunnar,<div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div class=""><ol class=""><li class="">I was wondering whether the distinction between <font face="monospace, monospace" class="">SingleContainerValueEx<wbr class="">tractor</font> and <font face="monospace, monospace" class="">ManyContainerValuesExtract<wbr class="">or</font> 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 <font face="monospace, monospace" class="">Iterator</font>/<font face="monospace, monospace" class="">Iterable</font> contract in any case. Not sure how others feel about that.<br class=""></li></ol></div></div></blockquote></span><div class="">Good question. I reckon we'll know more after experimenting with this in the RI. </div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div class=""><ol class=""><li class="">Section 2.1 states that BV will support <font face="monospace, monospace" class="">Iterable</font> and <font face="monospace, monospace" class="">Map</font> by default. What about <font face="monospace, monospace" class="">Optional</font>? We will support the JDK8 date/time API, so why don't we plan to support <font face="monospace, monospace" class="">Optional</font>?</li></ol></div></div></blockquote></span><div class="">Optional will be part of it by default. It should be stated somewhere, I hope :) </div></div></div></div></blockquote><div class=""><br class=""></div><div class="">The document explicitly states:</div><div class=""><br class=""></div><div class=""><i class=""> Bean Validation will have out of the box support for containers Iterable and Map.</i><br class=""></div><div class=""><br class=""></div><div class="">I was expecting Optional to be listed here too. That's why I mentioned it. :-)</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Christian</div><div class=""><br class=""></div><div class=""><br class=""></div></div><div class=""><br class=""></div>-- <br class=""><div class="gmail_signature"><div class="">Christian Kaltepoth</div><div class="">Blog: <a href="http://blog.kaltepoth.de/" target="_blank" class="">http://blog.kaltepoth.de/</a></div><div class="">Twitter: <a href="http://twitter.com/chkal" target="_blank" class="">http://twitter.com/chkal</a></div><div class="">GitHub: <a href="https://github.com/chkal" target="_blank" class="">https://github.com/chkal</a></div><div class=""><br class=""></div></div>
</div></div></div>
_______________________________________________<br class="">beanvalidation-dev mailing list<br class=""><a href="mailto:beanvalidation-dev@lists.jboss.org" class="">beanvalidation-dev@lists.jboss.org</a><br class=""><a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" class="">https://lists.jboss.org/mailman/listinfo/beanvalidation-dev</a></div></blockquote></div><br class=""></div></div>_______________________________________________<br class="">beanvalidation-dev mailing list<br class=""><a href="mailto:beanvalidation-dev@lists.jboss.org" class="">beanvalidation-dev@lists.jboss.org</a><br class=""><a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" class="">https://lists.jboss.org/mailman/listinfo/beanvalidation-dev</a></div></blockquote></div><br class=""></div></div>_______________________________________________<br class="">beanvalidation-dev mailing list<br class=""><a href="mailto:beanvalidation-dev@lists.jboss.org" class="">beanvalidation-dev@lists.jboss.org</a><br class="">https://lists.jboss.org/mailman/listinfo/beanvalidation-dev</div></blockquote></div><br class=""></div></body></html>