7:06 a.m.
On Fri, Jun 16, 2017 at 2:43 AM, Gunnar Morling <gunnar(a)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(a)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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev