[bv-dev] Support for OptionalInt, OptionalLong and OptionalDouble
Gunnar Morling
gunnar at hibernate.org
Wed May 17 09:33:34 EDT 2017
Hi,
2017-05-17 14:42 GMT+02:00 Matt Benson <mbenson at apache.org>:
> Nice. I presume the definition would look like:
>
> implements ValueExtractor<@ExtractedValue (type=Integer.class) OptionalInt>
Yes, exactly (plus being decorated with @UnwrapByDefault).
That's the definition in the RI:
https://github.com/hibernate/hibernate-validator/blob/master/engine/src/main/java/org/hibernate/validator/internal/engine/cascading/OptionalIntValueExtractor.java
>
> * Should we be really brave and rename or alias that member to value()?
I thought about it, but value() reads poorly if other members are
added down the road (I cannot think of any, though). But given that
@ExtractedValue is only used by BV providers and a very few authors of
custom extractors, I don't mind the few extra characters of type() for
the sake of more explicitness.
What do others think?
> * Should we permit to specify as int.class and force the implementation to
> wrap? (I haven't tried it, and also haven't been awake very long, but I
> can't think why it wouldn't work)
Would there be any advantage to that? During constraint validator
resolution (where this ends up being used), "primitive types are
considered equivalent to their respective primitive wrapper class", so
it shouldn't make a difference.
>
> Thanks,
> Matt
The spec change is here:
https://github.com/beanvalidation/beanvalidation-spec/pull/177.
As we need to hand in the Public Draft update today, the change has
already been merged and we'll hit the button for cutting the release
in a bit. Sorry for the rush, but I preferred to have this change in
today's spec update. As said, we can still fine-tune details later on,
if needed.
Thanks,
--Gunnar
>
>
> On May 17, 2017 5:30 AM, "Gunnar Morling" <gunnar at hibernate.org> wrote:
>
> Hi,
>
> Guillaume has looked into adding the type() member to @ExtractedValue
> so OptionalInt et al. can be handled via value extractors. All looks
> good and the change to the API has already been merged
> (https://github.com/beanvalidation/beanvalidation-api/pull/107).
>
> I'll send a PR with the corresponding spec change in a bit. It should
> be part of the update to the Public Review Draft which I've planned to
> submit to the JCP today. I originally thought I'd have a few more days
> time for that, but it turned out that contradictory information was
> given on jcp.org (they are going to fix that) and that an update must
> not be sent later than 10 days before the PD phase's end.
>
> Should any issues come up around this change (or anything else, for
> that matter) we can address them as part of the Proposed Final Draft,
> which we'll need to submit in a bit more than one month from now
> (around June 20th).
>
> Cheers,
>
> --Gunnar
>
>
>
>
>
> 2017-05-11 18:42 GMT+02:00 Matt Benson <mbenson at apache.org>:
>> On Thu, May 11, 2017 at 2:31 AM, Gunnar Morling <gunnar at hibernate.org>
>> wrote:
>>> 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).
>>
>> Fair enough. :)
>>
>>>
>>> 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?
>>
>> The spec says that #keyedValue() will be called twice per Map entry:
>> once per key and once per value, but the method signature includes
>> both the key and value along with the node name, so AFAICT you'd have
>> to consider whether the node name is "<map key>" or "<map value>" to
>> know which parameter was referenced. Or is the intent that the call
>> for the key repeat the key as both the key and value parameters? That
>> could make more sense, but I don't find the spec to be crystal clear
>> on that point if that is indeed the intent.
>>
>> Matt
>>
>>>
>>>>
>>>> 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
>>> _______________________________________________
>>> 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