[bv-dev] Spec question on ContainerElementTypeDescriptor

[Matt Benson]

More thoughts in this vein:

At first I thought that method overrides were the only place you might
have > 1 container class represented, but why wouldn't we support
e.g.:

class Roles implements Set<@NotEmpty String> {
...
}

? Then it'd be possible to extract container elements even at the bean level.

Next, consider parameters. the spec currently says that wildcards
aren't supported, but what about:

<X extends Set<@NotEmpty String> & Iterable<@NotNull String>> void foo(X);

In this way a parameter could expose multiple container types (pretend
I used unrelated interfaces if that helps). We could save generic
extends support for a future iteration of the spec, but semantically
it makes sense to me so I'd expect we'll eventually have to do it in
any case. I suppose you could even do fields if you annotated some
type parameter of the bean.

I also suspect that it will make sense to introduce a <container>
element to the XML mapping schema with an optional type, to
(optionally?) house the <container-element-type> elements.

Matt


On Fri, Jun 16, 2017 at 1:38 PM, Gunnar Morling <gunnar at hibernate.org> wrote:
> 2017-06-16 14:06 GMT+02:00 Matt Benson <mbenson at apache.org>:
>> On Fri, Jun 16, 2017 at 2:43 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
>>> Hi Matt,
>>>
>>> Thanks for bringing this up!
>>>
>>> I think adding ContainerElementTypeDescriptor#getContainerClass() is
>>> one thing needed, but it's not enough. Currently we have
>>> List<ContainerElementTypeDescriptor> getContainerElementTypes() on
>>> PropertyDescriptor etc. which returns the container element
>>> descriptors in the order of type arguments. This also doesn't really
>>> fit for User#getRoles().
>>>
>>> So we could make this a Set<ContainerElementTypeDescriptor> instead,
>>> and then each element from that set exposes containerClass and
>>> typeArgumentIndex as you suggest.
>>>
>>> It's getting a bit tricky though to get the effective set of all
>>> container element constraints. Eg. say we have:
>>>
>>>     public interface IUserBase {
>>>         Set<@NotBlank String> getRoles();
>>>     }
>>>
>>>     public interface IUser extends IUserBase {
>>>         Set<@NotEmpty String> getRoles();
>>>     }
>>>
>>>     public class UserImpl implements IUser {
>>>         Roles getRoles();
>>>     }
>>>
>>> Then when querying
>>>
>>>     validator.getConstraintsForClass( IUser.class )
>>>         .getConstraintsForProperty( "roles" )
>>>         .getContainerElementTypes();
>>>
>>> We'd get a set with these elements:
>>>
>>>     - ContainerElementDescriptor(containerClass=IUser.class,
>>> typeArgumentIndex=0, constraints={(type=NotEmpty,
>>> defined=DEFINED_LOCALLY), (type=NotBlank,
>>> defined=DEFINED_IN_HIERARCHY)}
>>>     - ContainerElementDescriptor(containerClass=User.class,
>>> typeArgumentIndex=0, constraints={(type=NotBlank, defined=
>>> DEFINED_LOCALLY)}
>>>
>>
>> I was thinking that the containerClass would be Set.class.
>
> Yes, you are right of course.
>
>>
>>> The descriptor for IUser would have to return the inherited @NotBlank
>>> as per the defined semantics of getConstraintDescriptors(). So one
>>> would have to examine all ContainerElementDescriptors and just collect
>>> the ones that are DEFINED_LOCALLY to have the effective set without
>>> duplicates.
>>>
>>
>> I agree that the set of ContainerElementTypeDescriptors needs to be
>> reduced to unique type parameters, but I'm not sure I'm following your
>> argument above. My suggestion above (containerClass=Set.class) would
>> give an easy way to identify what needs to be collapsed where: onto
>> Set<[0]>. Consider this:
>>
>> public class Roles<S extends System> implements Set<String> {
>>   public S getSystem() {
>>     ...
>>   }
>>   ...
>> }
>>
>> public class UserImpl implements IUser {
>>   public Roles<@ValidSystem UserSystem> getRoles() {
>>     ...
>>   }
>> }
>>
>> Here Roles would implement a custom container type on top of Iterable
>> via Set, so the method would return separate ContainerTypeElements for
>> Roles<[0]> and Set<[0]>. Does that make sense?
>
> Yes, it does.
>
> And in my previous example we'd have getContainerElementTypes() return
> a set with one descriptor:
>
> ContainerElementDescriptor(containerClass=Set.class,
> typeArgumentIndex=0, constraints={(type=NotEmpty, defined=
> DEFINED_IN_HIERARCHY), (type=NotBlank, defined=DEFINED_IN_HIERARCHY)}
>
> (both constraints are defined in the hierarchy from the perspective of UserImpl)
>
>>
>> Matt
>>
>>> The alternative would be to explicitly redeclare
>>> getConstraintDescriptors() for ContainerElementDescriptor and say that
>>> it doesn't expose constraints from the hierarchy. But this would make
>>> it very difficult for the user to find all those constraints applying
>>> to a given container element type (esp. as type arguments could switch
>>> their order in the hierarchy). So that'd not be my preferred route.
>>>
>>> WDYT?
>>>
>>> I've filed https://hibernate.atlassian.net/browse/BVAL-655 for this issue.
>>>
>>> --Gunnar
>>>
>>>
>>> 2017-06-15 17:38 GMT+02:00 Matt Benson <mbenson at apache.org>:
>>>> The various descriptor types return constraint information from the
>>>> entire hierarchy by default. However, in thinking about this I quickly
>>>> encountered a puzzle which I cannot solve using the current version of
>>>> the specification. Consider:
>>>>
>>>> public interface User {
>>>>   Set<@NotEmpty String> getRoles();
>>>> }
>>>>
>>>> public class Roles implements Set<String> {
>>>>  ...
>>>> }
>>>>
>>>> public class UserImpl implements User {
>>>>   @Override
>>>>   public Roles getRoles();
>>>> }
>>>>
>>>> What should the metadata of UserImpl provide? The Iterable value
>>>> extractor is applicable to Roles, but because the return value of the
>>>> getter has been narrowed, there is no type parameter at this point in
>>>> the hierarchy. It seems strange for a PropertyDescriptor of type Roles
>>>> to return a ContainerElementTypeDescriptor with index 0. This example
>>>> would AFAICT apply to e.g. javafx StringProperty as well, so it's
>>>> probably a question we need to resolve, unless I have missed something
>>>> that should be obvious. I think this could be solved if the CETD were
>>>> to explicitly specify the type with which its argument index is
>>>> associated. The alternative would be to require a custom
>>>> ValueExtractor? This seems cumbersome and unnecessary.
>>>>
>>>> Thanks,
>>>> Matt
>>>> _______________________________________________
>>>> beanvalidation-dev mailing list
>>>> beanvalidation-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>>> _______________________________________________
>>> beanvalidation-dev mailing list
>>> beanvalidation-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>> _______________________________________________
>> beanvalidation-dev mailing list
>> beanvalidation-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev