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@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@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@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev