Re: [bv-dev] Extractors vs cascading logic (was Re: Support for constraints on container values (e.g. Collection<@Email String>))

On 22 Nov 2016, at 20:15, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: > On 21 Nov 2016, at 14:07, Emmanuel Bernard <emmanuel(a)hibernate.org <mailto:emmanuel@hibernate.org>> 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? 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