[bv-dev] Spec question on ContainerElementTypeDescriptor

[Matt Benson]

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.

> 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?

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