<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 22, 2017 at 1:10 PM, Guillaume Smet <span dir="ltr">&lt;<a href="mailto:guillaume.smet@gmail.com" target="_blank">guillaume.smet@gmail.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div>= 1. Type parameter information<br></div></div><div><br></div>== 1.1 Should we add the type parameter to the node?<br><br></div>Assume the example of a map:<br></div>private Map&lt;@Valid City (1), @Valid Statistics (2)&gt; map;<br><br></div><div>With the current way of doing things, you end up with the following paths:<br></div><div><div><div><div><div><div><div>(1) (name = map, type = PROPERTY) -&gt; (name = name, type = PROPERTY, isInIterable = true, key = city)<br></div><div>(2) (name = map, type = PROPERTY) -&gt; (name = count, type = PROPERTY, isInIterable = true, key = city)<br></div><div><br></div><div>So you wouldn&#39;t be able to differentiate if the violations is coming from City or Statistics.<br><br></div><div>One of the ideas we had is to integrate the TypeVariable&lt;?&gt; type parameter info into the last node. In the case of (1), you would have 2 nodes:<br></div><div>(1) (name = map, type = PROPERTY) -&gt; (name = name, type = PROPERTY, isInIterable = true, key = city, typeParameter = K)<br></div><div>(2) (name = map, type = PROPERTY) -&gt; (name = count, type = PROPERTY, isInIterable = true, key = city, typeParameter = V)<br><br></div><div>WDYT?<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Looks bad, type parameter names in Java are considered documentation and not something you cannot change during class evolution. For Map, it won&#39;t ever change, but for other domain classes, it can change.</div><div><br></div><div>Type parameter *order*, though, cannot change without change meaning, so that&#39;s the best option we have left :-(</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div>== 1.2 If we add this information, what should it be?<br><br></div><div>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.<br><br></div><div>Thus we need to either use a String with the name of the type parameter or introduce our own serializable structure.<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Introduce a serializable structure. :-(</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div>What&#39;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.<br><br></div><div>Do you foresee issues if we are not using java.lang.reflect.<wbr>TypeVariable? Typically it would be harder to do additional reflection things.<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Yeah, as the generics model evolve, we might paint ourselves in the corner.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div>= 2. Type argument constraints<br><br></div><div>So, the discussion above also applies to type argument constraints but there are some specific questions for them.<br></div><div><br></div><div>== 2.1 New node type<br><br><div>Type argument constraints cover the following case, ZipCode being a constraint:<br></div>Map&lt;@ZipCode String, String&gt; map;</div><div><br></div><div>In this case, we envision the following node structure (assuming we would add the typeParameter discussed in 1.1):<br></div><div>(name = map, type = property) -&gt; (name = &#39;&lt;map key&gt;&#39;, type = TYPE_ARGUMENT, isInIterable = true, key = myKey, typeParameter = K)<br><br></div><div>TYPE_ARGUMENT is a new type introduced in javax.validation.ElementKind.<br><br></div><div>Does it make sense?<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>My answer from above applies here as well. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div>== 2.2 Default node names<br><br></div><div>The default extractors define the following node names for the added TYPE_ARGUMENT node:<br></div><div>- arrays and Iterables (List included): &lt;iterable element&gt;<br></div><div>- Map key: &lt;map key&gt;<br></div><div>- Map value: &lt;map value&gt;<br><br></div><div>This is similar to the nodes we created for &quot;&lt;return value&gt;&quot; or &quot;&lt;cross-parameter&gt;&quot; constraints.<br><br></div><div>Question: should they have a node name? should it be the type parameter name instead (so E or K or V for instance)?<br></div><div><br></div><div>Note that this might have consequences in the string representation we will discuss later.<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>If they must have a name, it will probably have to be named after the types they apply to and type parameter order :-(</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div>== 2.3 Optional and ObservableValue<br><br></div><div>In these 2 cases, we won&#39;t append a node.<br><br></div><div>Note that while in the ObservableValue case, it might feel more natural as the constraint will probably be used like:<br></div><div>@NotBlank StringProperty property;<br></div><div>to apply a NotBlank constraint on the wrapped value so it&#39;s rather logical to not have a node.<br><br></div><div>Just to be clear, for Optional, on the other hand, with our proposal, we won&#39;t have a node whereas the code looks like:<br></div><div>Optional&lt;@NotBlank String&gt; optional;<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Couldn&#39;t quite understand what comments I could make about it, so ok :-) </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div></div><div><br></div><div>= 3 String representation </div></div></div></div></div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div><br></div><div>Note: this is implementation specific but we thought it might be interesting to discuss it here anyway.<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Sorry, too many nuances and I don&#39;t have a strong opinion here.</div><div><br></div><div>Regards,</div><div>Michael</div></div></div></div>