[bv-dev] Support for OptionalInt, OptionalLong and OptionalDouble

Thu May 11 03:31:42 EDT 2017

2017-05-10 20:39 GMT+02:00 Matt Benson <mbenson at apache.org>:
> On Wed, May 10, 2017 at 11:34 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
>> Hi Matt,
>>
>> Putting the specific spec wording aside for a moment, how would such
>> extractor implementation make it possible to tell the wrapped type?
>>
>> When validating a constraint on an element of type OptionalInt, the
>> validation engine needs to know that this "behaves as an int", so it
>> can determine the applicable constraint validators. For something like
>> List<@Email String> we can obtain that information from the type
>> argument (so we know we need to look for validators on String), but
>> it's not doable for OptionalInt which doesn't bear any hint that it is
>> about int. Hence the idea of extending the @ExtractedValue annotation
>> to convey that information in such case.
>>
>
> That's the thing: I can see that perhaps as an implementation detail,
> but I can't actually see how the spec communicates that it's
> absolutely necessary that the implementation know up-front what the
> expected type is. A different perspective could come away with the
> expectation that the ValueExtractor pass objects to the ValueReceiver
> and *then* the implementation inspects the values received to
> determine which constraints to apply.

It's the spirit of the spec that the declared type of a constrained
element (property, parameter, etc., and now also container elements)
is the basis for constraint validator resolution (see 5.7.4).

It also mentions the possibility to create an annotation processor for
checking that constraints are only put to elements as supported by
available constraint validators ("rules can be statically checked").

Basing validator resolution on the received type as you suggest would
deviate from this, which is why I think we shouldn't do it.

> The only place where this is
> problematic is on the #keyedValue() calls, because you'd have to
> inspect the nodeName to know which argument to apply constraints to.

I'm not sure I'm following on this one. Can you elaborate?

>
> Matt
>
>> --Gunnar
>>
>>
>> 2017-05-10 17:33 GMT+02:00 Matt Benson <mbenson at apache.org>:
>>> On Wed, May 10, 2017 at 9:17 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
>>>> 2017-05-10 3:59 GMT+02:00 Matt Benson <mbenson at apache.org>:
>>>>> On Tue, May 9, 2017 at 8:36 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I'm curious about your take on supporting the types in ${subject}
>>>>>> (BVAL-579 [1]).
>>>>>>
>>>>>> They are non-generic wrappers for int, long and double. Should we
>>>>>> support them with the numeric constraints such as @Min et al.? The
>>>>>> easiest way to do so would be to just mandate support in the JavaDoc
>>>>>> of the numeric constraint types.
>>>>>>
>>>>>> The only thing I can see speaking against this is that we may migrate
>>>>>> to an extractor-based approach in a future revision. Currently
>>>>>> extractors cannot be used, as those types don't have any type
>>>>>> parameter which could be extracted. But assuming we extend the
>>>>>> extractor API in a future revision to deal with non-generic types,
>>>>>> too, we could then rather mandate built-in extractors for those types.
>>>>>> Which will allow to put *any* constraint applying to int also to
>>>>>> OptionalInt.
>>>>>
>>>>> Wait... the current draft of the spec still mentions implicit
>>>>> unwrapping of containers. Is that not staying? Why can't we have e.g.
>>>>> an extractor of OptionalInt to Integer, returning an absent value as
>>>>> null?
>>>>
>>>> Implicit unwrapping is staying, but value extraction cannot be used as
>>>> is, as we'd lack a way for obtaining the wrapped type from a value
>>>> extractor for a non-generic type:
>>>>
>>>>     OptionalIntExtractor implements ValueExtractor<@ExtractedValue
>>>> OptionalInt> {
>>>>
>>>>     }
>>>>
>>>> There is no type parameter/argument from which the wrapped type (int)
>>>> could be obtained (which is needed for finding the right constraint
>>>> validator). We could add an attribute to @ExtractedValue for such
>>>> cases: @ExtractedValue(wrappedType=int.class).
>>>>
>>>> We could do this in a future revision or if we are confident about it
>>>> in the Proposed Final draft. We are going to explore it in the RI (by
>>>> using a separate annotation next to @ExtractedValue for the time
>>>> being).
>>>
>>> Okay, I hadn't investigated the JavaFX APIs and hadn't realized that
>>> e.g. StringProperty and IntegerProperty did have a generic type
>>> parameter up their hierarchy. I still don't know that I believe the
>>> specification is fully consistent in this regard then, because section
>>> 5.5.1 refers to these as "non-generic property types." I see that the
>>> unwrapping for these types is intended to be fulfilled by
>>> ValueExtractor<ObservableValue<@ExtractedValue T>>, but the
>>> "non-generic property types" wording, to me, is suggestive of, and I
>>> think this has been discussed, something like a ValueExtractor that:
>>>
>>> * does *not* bear an @ExtractedValue annotation, and
>>> * *must* be marked as @UnwrapByDefault
>>>
>>> Then it seems an implementation would provide something like:
>>>
>>> @UnwrapByDefault
>>> public class OptionalIntExtractor implements ValueExtractor<OptionalInt> {
>>>   void extractValues(OptionalInt originalValue, ValueReceiver receiver) {
>>>     if (originalValue.isPresent()) {
>>>       receiver.value(null, originalValue.getAsInt());
>>>     }
>>>   }
>>> }
>>>
>>> This seems so simple that perhaps I am missing something; what is it? :)
>>>
>>> Matt
>>>
>>>>
>>>>>
>>>>> Matt
>>>>>
>>>>>>
>>>>>> Should we do such change in a future revision, I don't think anything
>>>>>> would change for users. Only providers would have to implement support
>>>>>> for these types via extractors instead of dedicated constraint
>>>>>> validators. I think such change is acceptable.
>>>>>>
>>>>>> What do others think?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> --Gunnar
>>>>>>
>>>>>> [1] https://hibernate.atlassian.net/browse/BVAL-579
>>>>>> _______________________________________________
>>>>>> 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