[bv-dev] Spec question on ContainerElementTypeDescriptor
Matt Benson
mbenson at apache.org
Mon Jun 19 11:50:32 EDT 2017
On Mon, Jun 19, 2017 at 8:04 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
> 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'm pretty sure wildcards are explicitly mentioned as not being
supported, and bounds are mentioned there. I don't remember type vars
being included in that. You say "as of BV 2.0." Can I take that to
mean that you think it's at least possible that this might be
something the spec should support in the future? Is this basically a
case of limiting feature creep so that we can finish this iteration
ASAP?
>
>> 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.
>
If we've established that the ContainerElementType needs the container
type to be certain of providing enough information to locate the
relevant value extractor, then consider the XML mapping. Say we have:
interface ILookup {
Map<String, String> getLookup();
}
class LookupTable extends Hashtable<String, String> {
}
class LookupImpl implements ILookup {
private LookupTable lookupTable;
LookupTable getLookup() {
return lookupTable;
}
}
And we want to constrain the type parameters of the Map in XML:
<bean class="ILookup">
<getter name="getLookup">
<!-- TBD -->
</getter>
</bean>
We could do:
<container-element-type container-type="java.util.Map" type-argument-index="0">
<constraint annotation="NotNull" />
<constraint annotation="NotBlank" />
</container-element-type>
<container-element-type container-type="java.util.Map" type-argument-index="1">
<constraint annotation="NotBlank" />
</container-element-type>
... and for simplicity's sake, since the typical container likely has
a single element type, we probably should continue to support this.
But should we also allow, e.g.:
<container type="java.util.Map">
<container-element-type type-argument-index="0">
<constraint annotation="NotNull" />
<constraint annotation="NotBlank" />
</container-element-type>
<container-element-type type-argument-index="1">
<constraint annotation="NotBlank" />
</container-element-type>
</container-type>
WRT the optionality of the type argument index, it's clear that for
containers with a single element type, that could be omitted. Is it
further intended that where the list of container-element-type
elements has a length matching the number of type variables declared
on the container type, these should be understood to specify a 1:1
ordered correspondence? If so, is there a place (and hopefully a more
intelligible way ;) ) to clarify that in the spec?
Matt
> 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 at apache.org>:
>> 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
>> _______________________________________________
>> 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
More information about the beanvalidation-dev
mailing list