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

Matt Benson mbenson at apache.org
Wed May 17 11:32:51 EDT 2017 

On Wed, May 17, 2017 at 8:33 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
> 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).

Like you, I couldn't think of any others, which is why I proposed the question.

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

I can't think of any real advantage; it was just a thought since
OptionalInt is defined in terms of int rather than Integer.

Matt

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