[bv-dev] Validator#validateValue() with incompatible Object specified

[Gunnar Morling]

Hey Matt,

I think you're giving too much bonus to the original authors of that test
method and their well-meaning intentions ;) IMO the test is simply
incorrect, for sure the intention was not that a single Order should
implicitly be handled as a single-element Set<Order>.

The reason it doesn't fail with the RI is simply the fact that the RI is
actually doing what the test seeks to ensure: @Valid isn't applied by
validateValue(), i.e. the algorithm never gets to the point where it'd make
use of the given value to recursively validate the constraints of the
"orders" property. Were validateValue() be invoked for another property
with a local constraint, a ClassCastException would occur when trying to
feed the non-matching value to the ConstraintValidator.

Can you open a BVTCK issue for fixing this test? A Set<Order> containing
the Order object should be passed to validateValue().

Regarding the expected behaviour, indeed the spec doesn't mandate something
specifically. I don't think it's a big problem, though, because the
non-matching value will cause an exception in one or another way anyways.
Theoretically we could mandate a specific IllegalArgumentException in that
case, but that'd require an instanceof check up front, and I don't think I
wanted to impose this to implementations. I'd rather leave it as is; of
course an implementation is free to do such check, as Apache Bval was doing
it.

As far as ExecutableValidator is concerned, there it says to raise an
exception if "parameters don't match with each other". This leaves some
leeway on the exact checks to implement and also here I'd prefer to keep it
that way, instead of enforcing potentially costly checks at the spec level.

--Gunnar

2018-03-30 17:33 GMT+02:00 Matt Benson <mbenson at apache.org>:

> I would add that other recent discussions have borne out my
> interpretation of the ExecutableValidator javadoc in saying that
> incompatible parameter/return values should raise
> IllegalArgumentExceptions. I would see this as a precedent in the case
> of #validateValue().
>
> Matt
>
> On Fri, Mar 30, 2018 at 10:17 AM, Matt Benson <mbenson at apache.org> wrote:
> > Hi, this comes from TCK
> > ValidateValueTest#testValidIsNotHonoredValidateValue() . The test
> > asserts that no constraint violations (and no exceptions) will be
> > generated when specifying an Order as a proposed value for a property
> > that includes a Set of Orders. The name of the test suggests to me,
> > however, that the Validator is expected to transparently treat the
> > Order as a single-element Set and simply show that validation of that
> > set is not cascaded. Is this the case, or is the test actually showing
> > that any incompatible value will be treated as raising no violations
> > (and is therefore imperfectly named)? In either case, where does the
> > specification dictate this behavior? Apache BVal has always raised an
> > IllegalArgumentException if the specified value is not compatible, but
> > its justification for having done so appears to be an artifact of an
> > incorrect interpretation of the Javadoc of #validateValue(): "not a
> > valid object property" was mistakenly applied to the "value" parameter
> > instead of to the "propertyName" parameter as a careful reading
> > reveals (IMO) is proper. But this exposes what seems to be an omission
> > in the spec: what *is* the proper behavior when validation of an
> > incompatible value is requested? And let's not forget the unclear
> > nature of the test in question: is "order" a compatible value by
> > virtue of being checked against a Set of its type (and, if so, why?),
> > or is "order" an incompatible value that the Validator quietly
> > abstains from checking at all?
> >
> > Thanks,
> > Matt
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/beanvalidation-dev/attachments/20180331/79ac7e74/attachment.html