Hi,

We would like to discuss with you the node structure of the ConstraintViolation path in the case of value extraction (2 cases really: cascaded validation and type argument constraints).

This is not an easy one! And we will also digress on the string representation while we're at it even if it's not part of the spec.

= 1. Type parameter information

== 1.1 Should we add the type parameter to the node?

Assume the example of a map:
private Map<@Valid City (1), @Valid Statistics (2)> map;

With the current way of doing things, you end up with the following paths:
(1) (name = map, type = PROPERTY) -> (name = name, type = PROPERTY, isInIterable = true, key = city)
(2) (name = map, type = PROPERTY) -> (name = count, type = PROPERTY, isInIterable = true, key = city)

So you wouldn't be able to differentiate if the violations is coming from City or Statistics.

One of the ideas we had is to integrate the TypeVariable<?> type parameter info into the last node. In the case of (1), you would have 2 nodes:
(1) (name = map, type = PROPERTY) -> (name = name, type = PROPERTY, isInIterable = true, key = city, typeParameter = K)
(2) (name = map, type = PROPERTY) -> (name = count, type = PROPERTY, isInIterable = true, key = city, typeParameter = V)

WDYT?

== 1.2 If we add this information, what should it be?

At first, Gunnar thought about using java.lang.reflect.TypeVariable for this type parameter information but we have an issue with it: it is not serializable and the path is supposed to be.

Thus we need to either use a String with the name of the type parameter or introduce our own serializable structure.

What's your take on this? If we go the structure route, which information should this structure contain apart from the name? java.lang.reflect.TypeVariable also has the generic declaration information.

Do you foresee issues if we are not using java.lang.reflect.TypeVariable? Typically it would be harder to do additional reflection things.

= 2. Type argument constraints

So, the discussion above also applies to type argument constraints but there are some specific questions for them.

== 2.1 New node type

Type argument constraints cover the following case, ZipCode being a constraint:
Map<@ZipCode String, String> map;

In this case, we envision the following node structure (assuming we would add the typeParameter discussed in 1.1):
(name = map, type = property) -> (name = '<map key>', type = TYPE_ARGUMENT, isInIterable = true, key = myKey, typeParameter = K)

TYPE_ARGUMENT is a new type introduced in javax.validation.ElementKind.

Does it make sense?

== 2.2 Default node names

The default extractors define the following node names for the added TYPE_ARGUMENT node:
- arrays and Iterables (List included): <iterable element>
- Map key: <map key>
- Map value: <map value>

This is similar to the nodes we created for "<return value>" or "<cross-parameter>" constraints.

Question: should they have a node name? should it be the type parameter name instead (so E or K or V for instance)?

Note that this might have consequences in the string representation we will discuss later.

== 2.3 Optional and ObservableValue

In these 2 cases, we won't append a node.

Note that while in the ObservableValue case, it might feel more natural as the constraint will probably be used like:
@NotBlank StringProperty property;
to apply a NotBlank constraint on the wrapped value so it's rather logical to not have a node.

Just to be clear, for Optional, on the other hand, with our proposal, we won't have a node whereas the code looks like:
Optional<@NotBlank String> optional;

= 3 String representation

Note: this is implementation specific but we thought it might be interesting to discuss it here anyway.

The Path toString() is included in ConstraintViolationException.getMessage().

If we consider what we proposed above and the following property:
Map<@NotNull @Valid Address, String> nicksByAddress;

We would end up with:
map<K>[address.toString()].street //error in address.street
map<K>[address.toString()].<map key> //error in type param of address
map<K>[address.toString()] //error in class level address - there is a bean node which we don't show

If we consider the following property:
Map<String, @NotNull @Valid Address> addressesByNick;

And we decided to be consistent with the fact that we add the type parameter information, we would end up with:
map<V>[address.toString()].street //error in address.street
map<V>[address.toString()].<map key> //error in type param of address
map<V>[address.toString()] //error in class level address - there is a bean node which we don't show

Note that this breaks the previous behavior of HV 5.x as we used to only support constraints on the value and we did not have the '<V>' information.

The issue becomes a bit more pregnant in the case of Lists. Let's assume we have the following property:
List<@NotNull @Valid Address> addresses;

We would end up with:
addresses<E>[0].street //error in address.street
addresses<E>[0].<iterable element> //error in type param of address
addresses<E>[0] //error in class level address - there is a bean node which we don't show

Question that becomes obvious here: if it were possible to only display the type parameter if we have > 1 type parameters in the original generic declaration, would it make sense to omit it? Basically, we have to discuss consistency vs readability.

To completely illustrate the discussion, let's take a look at what happens with nested type argument constraints (it is not in the spec yet but we experimented with it in HV - we'll discuss this in a future email):
Map<String, List<@NotNull @Valid Address>> listOfAddressesByNick;

In this case, the envisioned string representation would be:
listOfAddressesByNick<V>[myKey].<map value><E>[0].street //error in address.street
listOfAddressesByNick<V>[myKey].<map value><E>[0].<iterable element> //error in type param of address
listOfAddressesByNick<V>[myKey].<map value><E>[0] //error in class level address - there is a bean node which we don't show

So, as you can see, the readability can become an issue.

Thoughts? Ideas?

--
Guillaume