Re: [bv-dev] Spec question on ContainerElementTypeDescriptor

2017-06-16 14:06 GMT+02:00 Matt Benson <mbenson(a)apache.org&gt;: > On Fri, Jun 16, 2017 at 2:43 AM, Gunnar Morling <gunnar(a)hibernate.org&gt; 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(a)apache.org&gt;: >>> 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 > _______________________________________________ > 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