Re: [bv-dev] Spec question on ContainerElementTypeDescriptor

Hi, Putting constraints to type parameters of generic types or methods as well as putting constraints to type arguments in extends/implements clauses isn't supported as of BV 2.0. I thought that's mentioned somewhere in the spec, but couldn't find it during a quick check. If it's indeed missing, I'll add it. We can look into this for 2.1 of course.

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

Could you elaborate on that point a bit? What exactly is the use case for this? --Gunnar 2017-06-18 17:34 GMT+02:00 Matt Benson <mbenson(a)apache.org&gt;: > 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(a)hibernate.org&gt; wrote: >> 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 > _______________________________________________ > 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