Re: [bv-dev] Value extraction open issue #13 and #14: Path and nodes

Gunnar, Maybe I'm missing the point here, but as far as I understand, names added to the paths will be significant - for instance, it will influence how messages will end up being formatted, because keys will now depend on this additional information, right? If this is wrong, then disregard what I wrote below because it's based on that assumption. When you rename a property or a class name, client code will very likely fail to compile. You're releasing an incompatible change - you probably need to bump your major version in some cases and write thorough release notes in order to do that. Now, we're saying that to BeanValidation - and so far, in JCP standards, the only one AFAIK -, type parameter *names* are relevant. If you rename one, your code will compile but you might end up having a runtime exception or a poorly formatted error message because of that, unless you explicitly test them for type parameter name change - the only change that will not break other parts of your code. Whereas if you really on order, a change either breaks compilation or completely changes the semantics of existing code and has to follow the incompatible change process. But, as I said, what I said above is only valid if the names are significant, i.e., used somewhere else in a meaningful way. Regards, Michael On Wed, Feb 22, 2017 at 2:13 PM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > > > Looks bad, type parameter names in Java are considered documentation and > > not > > something you cannot change during class evolution. For Map, it won't > > ever > > change, but for other domain classes, it can change. > > Property paths reflect on a model in a given state. If your model > evolves, e.g. because you rename a bean property, paths obtained > previously may not match any element in the model's current state > anymore. > > I.e. I'm failing to see the how the situation is fundamentally > different for type parameters? > > 2017-02-22 17:46 GMT+01:00 Michael Nascimento <misterm(a)gmail.com&gt;: > > On Wed, Feb 22, 2017 at 1:10 PM, Guillaume Smet > > <guillaume.smet(a)gmail.com&gt; > > wrote: > >> > >> = 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? > > > > > > Looks bad, type parameter names in Java are considered documentation and > > not > > something you cannot change during class evolution. For Map, it won't > > ever > > change, but for other domain classes, it can change. > > > > Type parameter *order*, though, cannot change without change meaning, so > > that's the best option we have left :-( > > > >> > >> == 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. > > > > > > Introduce a 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. > > > > > > Yeah, as the generics model evolve, we might paint ourselves in the > > corner. > > > >> = 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? > > > > > > My answer from above applies here as well. > > > >> == 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. > > > > > > If they must have a name, it will probably have to be named after the > > types > > they apply to and type parameter order :-( > > > >> > >> == 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; > > > > > > Couldn't quite understand what comments I could make about it, so ok :-) > >> > >> > >> = 3 String representation > >> > >> > >> Note: this is implementation specific but we thought it might be > >> interesting to discuss it here anyway. > > > > > > Sorry, too many nuances and I don't have a strong opinion here. > > > > Regards, > > Michael > > > > _______________________________________________ > > beanvalidation-dev mailing list > > beanvalidation-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev > _______________________________________________ > beanvalidation-dev mailing list > beanvalidation-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev _______________________________________________ beanvalidation-dev mailing list beanvalidation-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/beanvalidation-dev