<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Jun 21, 2017 2:58 AM, "Gunnar Morling" <<a href="mailto:gunnar@hibernate.org">gunnar@hibernate.org</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="quoted-text">> My thinking was to make it possible for a user to configure container<br>
> element constraints for various levels of the inheritance hierarchy<br>
> for a given e.g. method return type. So continuing the lookup example,<br>
> perhaps we want to constrain the key of a LookupTable only for<br>
> LookupImpl and not for every implementation of ILookup.<br>
<br>
</div>Ah, got you.<br>
<br>
I don't think it's something we should support really, as it'd abandon<br>
the symmetry of capabilities of annotations and XML. For such use<br>
case, LookupImpl#getLookup() should declare a return type of Map<...,<br>
...> itself, so container element constraints can be applied via<br>
<beanClass class="LookupImpl">...</<wbr>beanClass>.<br>
<div class="elided-text"></div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">I suppose allowing a particular XML class configuration to target a superclass method in the suggested manner would confer an advantage not available to the annotation based config. Let's finish it up then. :-)</div><div dir="auto"><br></div><div dir="auto">Matt</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text"><br>
2017-06-20 22:36 GMT+02:00 Matt Benson <<a href="mailto:mbenson@apache.org">mbenson@apache.org</a>>:<br>
> On Tue, Jun 20, 2017 at 8:04 AM, Gunnar Morling <<a href="mailto:gunnar@hibernate.org">gunnar@hibernate.org</a>> wrote:<br>
>> 2017-06-19 17:50 GMT+02:00 Matt Benson <<a href="mailto:mbenson@apache.org">mbenson@apache.org</a>>:<br>
>>> On Mon, Jun 19, 2017 at 8:04 AM, Gunnar Morling <<a href="mailto:gunnar@hibernate.org">gunnar@hibernate.org</a>> wrote:<br>
>>>> Hi,<br>
>>>><br>
>>>> Putting constraints to type parameters of generic types or methods as<br>
>>>> well as putting constraints to type arguments in extends/implements<br>
>>>> clauses isn't supported as of BV 2.0. I thought that's mentioned<br>
>>>> somewhere in the spec, but couldn't find it during a quick check. If<br>
>>>> it's indeed missing, I'll add it. We can look into this for 2.1 of<br>
>>>> course.<br>
>>><br>
>>> I'm pretty sure wildcards are explicitly mentioned as not being<br>
>>> supported, and bounds are mentioned there. I don't remember type vars<br>
>>> being included in that. You say "as of BV 2.0." Can I take that to<br>
>>> mean that you think it's at least possible that this might be<br>
>>> something the spec should support in the future? Is this basically a<br>
>>> case of limiting feature creep so that we can finish this iteration<br>
>>> ASAP?<br>
>><br>
>> Yes, it's definitely something we can consider to support in a future<br>
>> revision. But for BV 2.0 it's too late to add it, as I'll have to<br>
>> submit the Proposed Final Draft tomorrow or say latest Thursday. While<br>
>> we can do some more adjustments until Final Approval Ballot, this may<br>
>> only be corrections and clarifications of already added stuff at this<br>
>> point unfortunately.<br>
>><br>
>> For sure we should explore this on the implementation level, so we<br>
>> have some experiences already when starting to work on the next spec<br>
>> revision.<br>
>><br>
>><br>
>>><br>
>>>><br>
>>>>> I also suspect that it will make sense to introduce a <container><br>
>>>>> element to the XML mapping schema with an optional type, to<br>
>>>>> (optionally?) house the <container-element-type> elements.<br>
>>>><br>
>>><br>
>>> If we've established that the ContainerElementType needs the container<br>
>>> type to be certain of providing enough information to locate the<br>
>>> relevant value extractor, then consider the XML mapping. Say we have:<br>
>>><br>
>>> interface ILookup {<br>
>>> Map<String, String> getLookup();<br>
>>> }<br>
>>><br>
>>> class LookupTable extends Hashtable<String, String> {<br>
>>> }<br>
>>><br>
>>> class LookupImpl implements ILookup {<br>
>>> private LookupTable lookupTable;<br>
>>><br>
>>> LookupTable getLookup() {<br>
>>> return lookupTable;<br>
>>> }<br>
>>> }<br>
>>><br>
>>> And we want to constrain the type parameters of the Map in XML:<br>
>>><br>
>>> <bean class="ILookup"><br>
>>> <getter name="getLookup"><br>
>>> <!-- TBD --><br>
>>> </getter><br>
>>> </bean><br>
>>><br>
>>> We could do:<br>
>>><br>
>>> <container-element-type container-type="java.util.Map" type-argument-index="0"><br>
>>> <constraint annotation="NotNull" /><br>
>>> <constraint annotation="NotBlank" /><br>
>>> </container-element-type><br>
>>> <container-element-type container-type="java.util.Map" type-argument-index="1"><br>
>>> <constraint annotation="NotBlank" /><br>
>>> </container-element-type><br>
>><br>
>> I don't think there's a need to specify the container type in this<br>
>> case, as this can be obtained from the configured field, method etc.<br>
>> So for configuring ILookup#getLookup(), it'd look like this with the<br>
>> currently proposed schema:<br>
>><br>
>> <bean class="ILookup"><br>
>> <getter name="lookup"><br>
>> <container-element-type type-argument-index="0"><br>
>> <constraint annotation="NotNull" /><br>
>> <constraint annotation="NotBlank" /><br>
>> </container-element-type><br>
>> <container-element-type type-argument-index="1"><br>
>> <constraint annotation="NotBlank" /><br>
>> </container-element-type><br>
>> </getter><br>
>> </bean><br>
>><br>
>> I.e. this is about Map<String, String> as that's the type of<br>
>> ILookup#getLookup().<br>
>><br>
>>><br>
>>> ... and for simplicity's sake, since the typical container likely has<br>
>>> a single element type, we probably should continue to support this.<br>
>>> But should we also allow, e.g.:<br>
>>><br>
>>> <container type="java.util.Map"><br>
>>> <container-element-type type-argument-index="0"><br>
>>> <constraint annotation="NotNull" /><br>
>>> <constraint annotation="NotBlank" /><br>
>>> </container-element-type><br>
>>> <container-element-type type-argument-index="1"><br>
>>> <constraint annotation="NotBlank" /><br>
>>> </container-element-type><br>
>>> </container-type><br>
>><br>
>> I think it's answered by my previous comment?<br>
>><br>
><br>
> My thinking was to make it possible for a user to configure container<br>
> element constraints for various levels of the inheritance hierarchy<br>
> for a given e.g. method return type. So continuing the lookup example,<br>
> perhaps we want to constrain the key of a LookupTable only for<br>
> LookupImpl and not for every implementation of ILookup. Given the time<br>
> issue though we can leave this alone for now.<br>
><br>
>>><br>
>>> WRT the optionality of the type argument index, it's clear that for<br>
>>> containers with a single element type, that could be omitted. Is it<br>
>>> further intended that where the list of container-element-type<br>
>>> elements has a length matching the number of type variables declared<br>
>>> on the container type, these should be understood to specify a 1:1<br>
>>> ordered correspondence? If so, is there a place (and hopefully a more<br>
>>> intelligible way ;) ) to clarify that in the spec?<br>
>><br>
>> That behaviour is not intended. I don't like the idea of relying on<br>
>> ordering, as it'd require you to specify (empty) container element<br>
>> type nodes for all type arguments, also if only one is to be<br>
>> constrained.<br>
>><br>
>> That's what we currently say in the spec:<br>
>><br>
>> "The type-argument-index is used to specify the index of the<br>
>> configured type argument. The type-argument-index can be omitted, if<br>
>> the container element type has exactly one type argument. [...] If an<br>
>> invalid container type element configuration is detected, a<br>
>> ValidationException is raised. This includes the following<br>
>> configuration errors:<br>
>><br>
>> * The type of the surrounding element (field, getter etc.) has no<br>
>> type arguments.<br>
>> * The type of the surrounding element has no type argument with<br>
>> the index given via type-argument-index.<br>
>> * The type of the surrounding element has multiple type arguments<br>
>> and no index is given via type-argument-index.<br>
>> * The same type argument of the surrounding element is configured<br>
>> multiple times."<br>
>><br>
>> Do you think this is clear enough or should we change the wording in some way?<br>
>><br>
><br>
> Seems okay to me.<br>
><br>
> Matt<br>
><br>
>><br>
>>><br>
>>> Matt<br>
>>><br>
>>>> Could you elaborate on that point a bit? What exactly is the use case for this?<br>
>>>><br>
>>>> --Gunnar<br>
>>>><br>
>>>><br>
>>>><br>
>>>> 2017-06-18 17:34 GMT+02:00 Matt Benson <<a href="mailto:mbenson@apache.org">mbenson@apache.org</a>>:<br>
>>>>> More thoughts in this vein:<br>
>>>>><br>
>>>>> At first I thought that method overrides were the only place you might<br>
>>>>> have > 1 container class represented, but why wouldn't we support<br>
>>>>> e.g.:<br>
>>>>><br>
>>>>> class Roles implements Set<@NotEmpty String> {<br>
>>>>> ...<br>
>>>>> }<br>
>>>>><br>
>>>>> ? Then it'd be possible to extract container elements even at the bean level.<br>
>>>>><br>
>>>>> Next, consider parameters. the spec currently says that wildcards<br>
>>>>> aren't supported, but what about:<br>
>>>>><br>
>>>>> <X extends Set<@NotEmpty String> & Iterable<@NotNull String>> void foo(X);<br>
>>>>><br>
>>>>> In this way a parameter could expose multiple container types (pretend<br>
>>>>> I used unrelated interfaces if that helps). We could save generic<br>
>>>>> extends support for a future iteration of the spec, but semantically<br>
>>>>> it makes sense to me so I'd expect we'll eventually have to do it in<br>
>>>>> any case. I suppose you could even do fields if you annotated some<br>
>>>>> type parameter of the bean.<br>
>>>>><br>
>>>>> I also suspect that it will make sense to introduce a <container><br>
>>>>> element to the XML mapping schema with an optional type, to<br>
>>>>> (optionally?) house the <container-element-type> elements.<br>
>>>>><br>
>>>>> Matt<br>
>>>>><br>
>>>>><br>
>>>>> On Fri, Jun 16, 2017 at 1:38 PM, Gunnar Morling <<a href="mailto:gunnar@hibernate.org">gunnar@hibernate.org</a>> wrote:<br>
>>>>>> 2017-06-16 14:06 GMT+02:00 Matt Benson <<a href="mailto:mbenson@apache.org">mbenson@apache.org</a>>:<br>
>>>>>>> On Fri, Jun 16, 2017 at 2:43 AM, Gunnar Morling <<a href="mailto:gunnar@hibernate.org">gunnar@hibernate.org</a>> wrote:<br>
>>>>>>>> Hi Matt,<br>
>>>>>>>><br>
>>>>>>>> Thanks for bringing this up!<br>
>>>>>>>><br>
>>>>>>>> I think adding ContainerElementTypeDescriptor<wbr>#getContainerClass() is<br>
>>>>>>>> one thing needed, but it's not enough. Currently we have<br>
>>>>>>>> List<<wbr>ContainerElementTypeDescriptor<wbr>> getContainerElementTypes() on<br>
>>>>>>>> PropertyDescriptor etc. which returns the container element<br>
>>>>>>>> descriptors in the order of type arguments. This also doesn't really<br>
>>>>>>>> fit for User#getRoles().<br>
>>>>>>>><br>
>>>>>>>> So we could make this a Set<<wbr>ContainerElementTypeDescriptor<wbr>> instead,<br>
>>>>>>>> and then each element from that set exposes containerClass and<br>
>>>>>>>> typeArgumentIndex as you suggest.<br>
>>>>>>>><br>
>>>>>>>> It's getting a bit tricky though to get the effective set of all<br>
>>>>>>>> container element constraints. Eg. say we have:<br>
>>>>>>>><br>
>>>>>>>> public interface IUserBase {<br>
>>>>>>>> Set<@NotBlank String> getRoles();<br>
>>>>>>>> }<br>
>>>>>>>><br>
>>>>>>>> public interface IUser extends IUserBase {<br>
>>>>>>>> Set<@NotEmpty String> getRoles();<br>
>>>>>>>> }<br>
>>>>>>>><br>
>>>>>>>> public class UserImpl implements IUser {<br>
>>>>>>>> Roles getRoles();<br>
>>>>>>>> }<br>
>>>>>>>><br>
>>>>>>>> Then when querying<br>
>>>>>>>><br>
>>>>>>>> validator.<wbr>getConstraintsForClass( IUser.class )<br>
>>>>>>>> .getConstraintsForProperty( "roles" )<br>
>>>>>>>> .getContainerElementTypes();<br>
>>>>>>>><br>
>>>>>>>> We'd get a set with these elements:<br>
>>>>>>>><br>
>>>>>>>> - ContainerElementDescriptor(<wbr>containerClass=IUser.class,<br>
>>>>>>>> typeArgumentIndex=0, constraints={(type=NotEmpty,<br>
>>>>>>>> defined=DEFINED_LOCALLY), (type=NotBlank,<br>
>>>>>>>> defined=DEFINED_IN_HIERARCHY)}<br>
>>>>>>>> - ContainerElementDescriptor(<wbr>containerClass=User.class,<br>
>>>>>>>> typeArgumentIndex=0, constraints={(type=NotBlank, defined=<br>
>>>>>>>> DEFINED_LOCALLY)}<br>
>>>>>>>><br>
>>>>>>><br>
>>>>>>> I was thinking that the containerClass would be Set.class.<br>
>>>>>><br>
>>>>>> Yes, you are right of course.<br>
>>>>>><br>
>>>>>>><br>
>>>>>>>> The descriptor for IUser would have to return the inherited @NotBlank<br>
>>>>>>>> as per the defined semantics of getConstraintDescriptors(). So one<br>
>>>>>>>> would have to examine all ContainerElementDescriptors and just collect<br>
>>>>>>>> the ones that are DEFINED_LOCALLY to have the effective set without<br>
>>>>>>>> duplicates.<br>
>>>>>>>><br>
>>>>>>><br>
>>>>>>> I agree that the set of ContainerElementTypeDescriptor<wbr>s needs to be<br>
>>>>>>> reduced to unique type parameters, but I'm not sure I'm following your<br>
>>>>>>> argument above. My suggestion above (containerClass=Set.class) would<br>
>>>>>>> give an easy way to identify what needs to be collapsed where: onto<br>
>>>>>>> Set<[0]>. Consider this:<br>
>>>>>>><br>
>>>>>>> public class Roles<S extends System> implements Set<String> {<br>
>>>>>>> public S getSystem() {<br>
>>>>>>> ...<br>
>>>>>>> }<br>
>>>>>>> ...<br>
>>>>>>> }<br>
>>>>>>><br>
>>>>>>> public class UserImpl implements IUser {<br>
>>>>>>> public Roles<@ValidSystem UserSystem> getRoles() {<br>
>>>>>>> ...<br>
>>>>>>> }<br>
>>>>>>> }<br>
>>>>>>><br>
>>>>>>> Here Roles would implement a custom container type on top of Iterable<br>
>>>>>>> via Set, so the method would return separate ContainerTypeElements for<br>
>>>>>>> Roles<[0]> and Set<[0]>. Does that make sense?<br>
>>>>>><br>
>>>>>> Yes, it does.<br>
>>>>>><br>
>>>>>> And in my previous example we'd have getContainerElementTypes() return<br>
>>>>>> a set with one descriptor:<br>
>>>>>><br>
>>>>>> ContainerElementDescriptor(<wbr>containerClass=Set.class,<br>
>>>>>> typeArgumentIndex=0, constraints={(type=NotEmpty, defined=<br>
>>>>>> DEFINED_IN_HIERARCHY), (type=NotBlank, defined=DEFINED_IN_HIERARCHY)}<br>
>>>>>><br>
>>>>>> (both constraints are defined in the hierarchy from the perspective of UserImpl)<br>
>>>>>><br>
>>>>>>><br>
>>>>>>> Matt<br>
>>>>>>><br>
>>>>>>>> The alternative would be to explicitly redeclare<br>
>>>>>>>> getConstraintDescriptors() for ContainerElementDescriptor and say that<br>
>>>>>>>> it doesn't expose constraints from the hierarchy. But this would make<br>
>>>>>>>> it very difficult for the user to find all those constraints applying<br>
>>>>>>>> to a given container element type (esp. as type arguments could switch<br>
>>>>>>>> their order in the hierarchy). So that'd not be my preferred route.<br>
>>>>>>>><br>
>>>>>>>> WDYT?<br>
>>>>>>>><br>
>>>>>>>> I've filed <a href="https://hibernate.atlassian.net/browse/BVAL-655" rel="noreferrer" target="_blank">https://hibernate.atlassian.<wbr>net/browse/BVAL-655</a> for this issue.<br>
>>>>>>>><br>
>>>>>>>> --Gunnar<br>
>>>>>>>><br>
>>>>>>>><br>
>>>>>>>> 2017-06-15 17:38 GMT+02:00 Matt Benson <<a href="mailto:mbenson@apache.org">mbenson@apache.org</a>>:<br>
>>>>>>>>> The various descriptor types return constraint information from the<br>
>>>>>>>>> entire hierarchy by default. However, in thinking about this I quickly<br>
>>>>>>>>> encountered a puzzle which I cannot solve using the current version of<br>
>>>>>>>>> the specification. Consider:<br>
>>>>>>>>><br>
>>>>>>>>> public interface User {<br>
>>>>>>>>> Set<@NotEmpty String> getRoles();<br>
>>>>>>>>> }<br>
>>>>>>>>><br>
>>>>>>>>> public class Roles implements Set<String> {<br>
>>>>>>>>> ...<br>
>>>>>>>>> }<br>
>>>>>>>>><br>
>>>>>>>>> public class UserImpl implements User {<br>
>>>>>>>>> @Override<br>
>>>>>>>>> public Roles getRoles();<br>
>>>>>>>>> }<br>
>>>>>>>>><br>
>>>>>>>>> What should the metadata of UserImpl provide? The Iterable value<br>
>>>>>>>>> extractor is applicable to Roles, but because the return value of the<br>
>>>>>>>>> getter has been narrowed, there is no type parameter at this point in<br>
>>>>>>>>> the hierarchy. It seems strange for a PropertyDescriptor of type Roles<br>
>>>>>>>>> to return a ContainerElementTypeDescriptor with index 0. This example<br>
>>>>>>>>> would AFAICT apply to e.g. javafx StringProperty as well, so it's<br>
>>>>>>>>> probably a question we need to resolve, unless I have missed something<br>
>>>>>>>>> that should be obvious. I think this could be solved if the CETD were<br>
>>>>>>>>> to explicitly specify the type with which its argument index is<br>
>>>>>>>>> associated. The alternative would be to require a custom<br>
>>>>>>>>> ValueExtractor? This seems cumbersome and unnecessary.<br>
>>>>>>>>><br>
>>>>>>>>> Thanks,<br>
>>>>>>>>> Matt<br>
>>>>>>>>> ______________________________<wbr>_________________<br>
>>>>>>>>> beanvalidation-dev mailing list<br>
>>>>>>>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>>>>>>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>>>>>>>> ______________________________<wbr>_________________<br>
>>>>>>>> beanvalidation-dev mailing list<br>
>>>>>>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>>>>>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>>>>>>> ______________________________<wbr>_________________<br>
>>>>>>> beanvalidation-dev mailing list<br>
>>>>>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>>>>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>>>>>> ______________________________<wbr>_________________<br>
>>>>>> beanvalidation-dev mailing list<br>
>>>>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>>>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>>>>> ______________________________<wbr>_________________<br>
>>>>> beanvalidation-dev mailing list<br>
>>>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>>>> ______________________________<wbr>_________________<br>
>>>> beanvalidation-dev mailing list<br>
>>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>>> ______________________________<wbr>_________________<br>
>>> beanvalidation-dev mailing list<br>
>>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
>> ______________________________<wbr>_________________<br>
>> beanvalidation-dev mailing list<br>
>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
> ______________________________<wbr>_________________<br>
> beanvalidation-dev mailing list<br>
> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
______________________________<wbr>_________________<br>
beanvalidation-dev mailing list<br>
<a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.<wbr>jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/<wbr>mailman/listinfo/<wbr>beanvalidation-dev</a><br>
</div></blockquote></div><br></div></div></div>