[bv-dev] Node#getElementDescriptor() for user-created nodes

[Hardy Ferentschik]

+1 for option  #1. I never likes the fact that you could add arbitrary nodes (even though I have seen code doing it).

If we not go for #1, I prefer #2 - null from getElementDescriptor(). I don't see the point intruding another virtual descriptor 
where not all methods are clearly defined or need to be defined with constructed values. In this case I prefer to return null
directly which of course means that there is one more null check if you are going to use this API. On the other hand, I don't
see this API used much by actual end users. It is probably more important for integrators of BV. In this case an additional null
check is acceptable imo.

--Hardy 


On 9 Jan 2013, at 3:01 PM, Sebastian Thomschke <sebastian.thomschke at web.de> wrote:

> I agree with Gunnar. I'm for option #1 as well.
> 
> seb
> 
> On 09.02.2013 12:12, Gunnar Morling wrote:
>> My personal preference would be #1 as well.
>> 
>> The docs don't say that non-existent nodes are allowed, so IMO it's ok to clarify now that it is not allowed. If code relied on adding non-existent nodes, it relied on behavior not guaranteed explicitly by the spec.
>> 
>> --Gunnar
>> 
>> 
>> 
>> 2013/2/8 Emmanuel Bernard <emmanuel at hibernate.org>
>> I had the same reserve as you but then reading the JavaDoc made me
>> realize that the doc mentions properties so it is technically not as
>> vague as I initially thought.
>> 
>> Emmanuel
>> 
>> On Fri 2013-02-08 11:48, Matt Benson wrote:
>> > Personally my reason for eschewing option #1 is the possibility of breaking
>> > existing code.
>> >
>> > Matt
>> >
>> >
>> > On Fri, Feb 8, 2013 at 11:22 AM, Emmanuel Bernard <emmanuel at hibernate.org>wrote:
>> >
>> > > At this time of day I do tend to like option #1 (ie making virtual nodes
>> > > illegal). Otherwise my preferred option is #4
>> > > (VirtualElementDescriptor).
>> > >
>> > > Out of curiosity for people not preferring option #1. What use case do
>> > > you see in having virtual nodes?
>> > >
>> > > Emmanuel
>> > >
>> > > On Fri 2013-02-08 11:08, Matt Benson wrote:
>> > > > Hmm, a third option for the elementClass of such a descriptor might be
>> > > > Void.class.
>> > > >
>> > > > I prefer option #3, followed by #4.
>> > > >
>> > > > Matt
>> > > >
>> > > >
>> > > > On Fri, Feb 8, 2013 at 5:17 AM, Gunnar Morling <gunnar at hibernate.org>
>> > > wrote:
>> > > >
>> > > > > Hi experts,
>> > > > >
>> > > > > One of the remaining issues on our way to 1.1 is BVAL-336 [1], which is
>> > > > > about what to return from Node#getElementDescriptor() for nodes added
>> > > by
>> > > > > the user via the ConstraintViolationBuilder API.
>> > > > >
>> > > > > Note that as per the spec only sub paths can be created by the user,
>> > > i.e.
>> > > > > user added nodes always represent properties but e.g. no methods or
>> > > > > parameters. Based on that, it seems right to return the correct
>> > > > > PropertyDescriptor in case a user adds a node for an existing property.
>> > > > >
>> > > > > Things get tricky though when nodes are added for non-existent
>> > > properties.
>> > > > > Emmanuel, Hardy and I identified the following options to address this
>> > > > > situation:
>> > > > >
>> > > > > #1 Disallow adding non-existent nodes
>> > > > >
>> > > > > In case a user adds a node but no property with that names exist, an
>> > > > > exception is thrown. The problem is elegantly avoided that way, but we
>> > > > > might break some 1.0 user code (currently the spec is not really clear
>> > > > > whether added nodes must exist or not).
>> > > > >
>> > > > > #2 return null from getElementDescriptor()
>> > > > >
>> > > > > When invoking getElementDescriptor() on a Node representing a
>> > > non-existent
>> > > > > property, null could be returned. This seems consistent (there is no
>> > > > > element), but causes additional null checking when traversing a path.
>> > > > >
>> > > > > #3 return a PropertyDescriptor from getElementDescriptor()
>> > > > >
>> > > > > Since all user-added nodes represent properties, a PropertyDescriptor
>> > > > > could be returned. hasConstraints() would return false,
>> > > > > getConstraintDescriptors() the empty set etc. This would allow to
>> > > handle
>> > > > > all nodes and their descriptors uniformly when traversing a path.
>> > > Question
>> > > > > is what to return from PropertyDescriptor#getElementClass(). Options
>> > > are to
>> > > > > return null (signaling that the property is non-existent) or
>> > > Object.class.
>> > > > >
>> > > > > #4 return a VirtualDescriptor from getElementDescriptor()
>> > > > >
>> > > > > We could create a new Kind, VIRTUAL, and an accompanying type
>> > > > > VirtualDescriptor and return an instance of this. Behavior of the
>> > > methods
>> > > > > on the descriptor would be basically the same as in #3; getKind()
>> > > returning
>> > > > > Kind.VIRTUAL would allow for a very explicit checking whether the node
>> > > > > exists or not.
>> > > > >
>> > > > > Any feedback on the options (ideally with arguments pro/con) or other
>> > > > > alternatives are highly welcome. As life goes, Emmanuel's, Hardy's and
>> > > my
>> > > > > preferences are equally distributed between these options :)
>> > > > >
>> > > > > Thanks,
>> > > > >
>> > > > > --Gunnar
>> > > > >
>> > > > > [1] https://hibernate.onjira.com/browse/BVAL-336
>> > > > >
>> > > > >
> 
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev