[bv-dev] BVAL-221 Constraint violation builder cannot put constraint on a top level map key

[Hardy Ferentschik]

Hi,

Thanks for the clarification. Indeed I was forgetting about the special meaning of null in this case (should have read section 5.2 of the spec).
In this context adding addNode(null) is indeed "natural" in the sense that it strictly follows the spec. That said, it still looks awkward to me.
Couldn't there be for example a constant so that you could do something like addNode(CLASS_LEVEL). I find this much more telling.
In this light option 4 - adding a different method - of your proposed solution on http://beanvalidation.org/proposals/BVAL-221/ sounds more appealing again.
What about adding addClassLevelNode()? 

--Hardy



On 4 Jan 2012, at 4:42 PM, Emmanuel Bernard <emmanuel at hibernate.org> wrote:

> These are good questions
> 
> ## Natural way to do it?
> 
> Assuming
> 
> @HasCorrectHomeAddressFromTop
> class Order {
>  @HasCorrectHomeAddressFromUser
>  User user;
> }
> 
> class User {
>  @CollectionNotEmpty
>  @HasCorrectHomeAddressFromAddresses
>  Map<String,Address> addresses;
> }
> 
> When `HasCorrectHomeAddressFromTop` is validated, you can attach a
> `ConstraintViolation` to Order.user.addresses["home"] with the
> following call and it works in BV 1.0
> This is how the API is supposed to be used.
> 
>    context.buildConstraintViolationWithTemplate( "Incorrect home address" )
>        .addNode( "user" )
>        .addNode( "addresses" )
>            .inIterable().atKey( "home" )
>        .addConstraintViolation();
> 
> following this logic, when `HasCorrectHomeAddressFromUser` is validated,
> you can attach a `ConstraintViolation` with the following call but that
> does not work in BV 1.0.
> 
>    context.buildConstraintViolationWithTemplate( "Incorrect home address" )
>        .addNode( "addresses" )
>            .inIterable().atKey( "home" )
>        .addConstraintViolation();
> 
> When `HasCorrectHomeAddressFromAddresses` is validated, you can attach a
> `ConstraintViolation` with the following call but that does not work
> in BV 1.0.
> 
>    context.buildConstraintViolationWithTemplate( "Incorrect home address" )
>        .addNode( null )
>            .inIterable().atKey( "home" )
>        .addConstraintViolation();
> 
> Not that we use null as we add a class level constraint violation to the
> element in `addresses["home"]`.
> 
> For you info, when `CollectionNotEmpty` is validated, you can attach a
> `ConstraintViolation` with the following call.
> 
>    context.buildConstraintViolationWithTemplate( "collection empty" )
>        .addConstraintViolation();
> 
> ## Loss in type-safety
> 
> If we add `inIterable()` to `NodeBuilderDefinedContext` we solve our
> problem but we introduce a leak in the type safety. I could do the
> following calls
> 
>    context.buildConstraintViolationWithTemplate( "Incorrect home address" )
>        .addNode( null )
>            .inIterable().atKey( "home" )
>                .inIterable().atKey( "wc" )
>            .addConstraintViolation();
> 
> This call is not correct as no node has been (explicitly) created
> between the two inIterable() calls. The correct call would be
> 
>    context.buildConstraintViolationWithTemplate( "Incorrect WC location" )
>        .addNode( null )
>            .inIterable().atKey( "home" )
>        .addNode( null )
>            .inIterable().atKey( "wc" )
>        .addConstraintViolation();
> 
> Here we are assuming that the `addresses` elements contain map.
> 
> We could add in the spec that subsequent calls to `inIterable()`
> implicitly create a node without name.
> 
> The problem is not as critical as I imagined but that would require
> adding some magic rules wrt node creation.
> 
> Also imagine a user willing to validate a property of Address, could he
> forget to call the node creation method?
> 
> Emmanuel
> 
> On Mon 2012-11-26 13:07, Hardy Ferentschik wrote:
>> 
>> On 23 Jan 2012, at 2:48 PM, Emmanuel Bernard <emmanuel at hibernate.org> wrote:
>> 
>>> The question is do we want to do that or lean towards one of the
>>> alternative methods:
>>> 
>>> - an explicit new method deprecating addNode
>>> - relaxing NodeBuilderDefinedContext and adding the new contract on it
>> 
>> In the proposal you are saying about relaxing NodeBuilderDefinedContext:  "But that would break the contextual 
>> and safety of the API as this interface is also returned by atKey and atIndex." 
>> Can you provide an example?
>> 
>> Also in the proposal you are saying that:
>> 
>> context.buildConstraintViolationWithTemplate("oh noes")
>>        .addNode(null) //ie the contained element
>>            inIterable().atKey(someKey)
>>        .addConstraintViolation();
>> 
>> would be the natural solution. Why is ".addNode(null)" natural and why could we not have inIterable() directly 
>> on ConstraintViolationBuilder?
>> 
>> --Hardy
>> 
>> 
>> _______________________________________________
>> beanvalidation-dev mailing list
>> beanvalidation-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev