7:40 p.m.
Unless anyone has concern, I plan to go for option 3 on
http://beanvalidation.org/proposals/BVAL-221/
12:57 p.m.
Hi,
Another approach would be to make NodeBuilderCustomizableContext a
sub
insterface of NodeBuilderDefinedContext.
I think changing the return type of ConstraintViolationBuilder#addNode()
to NodeBuilderCustomizableContext is a problem.
While that change is source compatible (code using addNode() can be
compiled against the new API version without having to be adapted) it is
not binary compatible. That is, code compiled against the previous version
will fail at runtime:
java.lang.NoSuchMethodError:
javax.validation.ConstraintValidatorContext$ConstraintViolationBuilder.addNode(Ljava/lang/String;)Ljavax/validation/ConstraintValidatorContext$ConstraintViolationBuilder$NodeBuilderDefinedContext;
This error occurs, no matter wether the result of addNode() is assigned to
a variable or not.
What we might try to do is to return an intersection type from
ConstraintViolationBuilder#addNode() like this:
interface ConstraintViolationBuilder {
<T extends NodeBuilderDefinedContext & NodeBuilderCustomizableContext>
T addNode(String name);
ConstraintValidatorContext addConstraintViolation();
}
This is source compatible and AFAICS it's also binary compatible because
the erasure of T is its leftmost bound, that is NodeBuilderDefinedContext.
That way code compiled against the old API can still dispatch the addNode()
method, while code written against the new version can invoke all methods
defined on NodeBuilderDefinedContext and NodeBuilderCustomizableContext.
Btw. I stumbled upon [1] which gives a very good overview on the
compatibility of all sorts of API changes.
--Gunnar
[1]
http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_Interfaces
2012/11/22 Emmanuel Bernard <emmanuel(a)hibernate.org>
Unless anyone has concern, I plan to go for option 3 on
http://beanvalidation.org/proposals/BVAL-221/
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
2:48 p.m.
I guess you always learn something new every day :)
This is indeed only working if we use interception types.
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
Emmanuel
On Fri 2012-11-23 12:57, Gunnar Morling wrote:
Hi,
> Another approach would be to make NodeBuilderCustomizableContext a sub
insterface of NodeBuilderDefinedContext.
I think changing the return type of ConstraintViolationBuilder#addNode()
to NodeBuilderCustomizableContext is a problem.
While that change is source compatible (code using addNode() can be
compiled against the new API version without having to be adapted) it is
not binary compatible. That is, code compiled against the previous version
will fail at runtime:
java.lang.NoSuchMethodError:
javax.validation.ConstraintValidatorContext$ConstraintViolationBuilder.addNode(Ljava/lang/String;)Ljavax/validation/ConstraintValidatorContext$ConstraintViolationBuilder$NodeBuilderDefinedContext;
This error occurs, no matter wether the result of addNode() is assigned to
a variable or not.
What we might try to do is to return an intersection type from
ConstraintViolationBuilder#addNode() like this:
interface ConstraintViolationBuilder {
<T extends NodeBuilderDefinedContext & NodeBuilderCustomizableContext>
T addNode(String name);
ConstraintValidatorContext addConstraintViolation();
}
This is source compatible and AFAICS it's also binary compatible because
the erasure of T is its leftmost bound, that is NodeBuilderDefinedContext.
That way code compiled against the old API can still dispatch the addNode()
method, while code written against the new version can invoke all methods
defined on NodeBuilderDefinedContext and NodeBuilderCustomizableContext.
Btw. I stumbled upon [1] which gives a very good overview on the
compatibility of all sorts of API changes.
--Gunnar
[1]
http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_Interfaces
2012/11/22 Emmanuel Bernard <emmanuel(a)hibernate.org>
> Unless anyone has concern, I plan to go for option 3 on
> http://beanvalidation.org/proposals/BVAL-221/
> _______________________________________________
> 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
3:21 p.m.
- relaxing NodeBuilderDefinedContext and adding the new contract on
it
I wouldn't really like that as it reduces safety of the API.
- an explicit new method deprecating addNode
Related to this is BVAL-336 [1]. We've added Node#getDescriptor(), so the
question is what should that return for nodes created by the user via the
node builder API?
We could have a new method
addNode(String name, ElementDescriptor descriptor) or even
addNode(ElementDescriptor descriptor)
Leaves the question where does the user get the descriptor from? One way
could be to provide access to BeanDescriptor via ConstraintValidatorContext.
For nodes added via the deprecated method getDescriptor() we would likely
have to return null.
--Gunnar
[1] https://hibernate.onjira.com/browse/BVAL-336
2012/11/23 Emmanuel Bernard <emmanuel(a)hibernate.org>
> I guess you always learn something new every day :)
> This is indeed only working if we use interception types.
>
> 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
>
> Emmanuel
>
> On Fri 2012-11-23 12:57, Gunnar Morling wrote:
> > Hi,
> >
> > > Another approach would be to make NodeBuilderCustomizableContext a sub
> > insterface of NodeBuilderDefinedContext.
> >
> > I think changing the return type of ConstraintViolationBuilder#addNode()
> > to NodeBuilderCustomizableContext is a problem.
> >
> > While that change is source compatible (code using addNode() can be
> > compiled against the new API version without having to be adapted) it is
> > not binary compatible. That is, code compiled against the previous
> version
> > will fail at runtime:
> >
> > java.lang.NoSuchMethodError:
> >
>
javax.validation.ConstraintValidatorContext$ConstraintViolationBuilder.addNode(Ljava/lang/String;)Ljavax/validation/ConstraintValidatorContext$ConstraintViolationBuilder$NodeBuilderDefinedContext;
> >
> > This error occurs, no matter wether the result of addNode() is assigned
> to
> > a variable or not.
> >
> > What we might try to do is to return an intersection type from
> > ConstraintViolationBuilder#addNode() like this:
> >
> > interface ConstraintViolationBuilder {
> >
> > <T extends NodeBuilderDefinedContext &
> NodeBuilderCustomizableContext>
> > T addNode(String name);
> >
> > ConstraintValidatorContext addConstraintViolation();
> > }
> >
> > This is source compatible and AFAICS it's also binary compatible because
> > the erasure of T is its leftmost bound, that is
> NodeBuilderDefinedContext.
> > That way code compiled against the old API can still dispatch the
> addNode()
> > method, while code written against the new version can invoke all methods
> > defined on NodeBuilderDefinedContext and NodeBuilderCustomizableContext.
> >
> > Btw. I stumbled upon [1] which gives a very good overview on the
> > compatibility of all sorts of API changes.
> >
> > --Gunnar
> >
> > [1]
> >
> http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_Interfaces
> >
> >
> > 2012/11/22 Emmanuel Bernard <emmanuel(a)hibernate.org>
> >
> > > Unless anyone has concern, I plan to go for option 3 on
> > > http://beanvalidation.org/proposals/BVAL-221/
> > > _______________________________________________
> > > 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
>
3:50 p.m.
On 23 Jan 2012, at 3:21 PM, Gunnar Morling <gunnar(a)hibernate.org> wrote:
> - relaxing NodeBuilderDefinedContext and adding the new contract
on it
I wouldn't really like that as it reduces safety of the API.
need to still make up my mind about this one
> - an explicit new method deprecating addNode
Related to this is BVAL-336 [1]. We've added Node#getDescriptor(), so the question is
what should that return for nodes created by the user via the node builder API?
We could have a new method
addNode(String name, ElementDescriptor descriptor) or even
addNode(ElementDescriptor descriptor)
Leaves the question where does the user get the descriptor from? One way could be to
provide access to BeanDescriptor via ConstraintValidatorContext. For nodes added via the
deprecated method getDescriptor() we would likely have to return null.
This is one of the reason I never found it such a good idea to "merry" the path
with the descriptors. In this context we also talked once about the need of a
DummyDescriptor. Using the
ConstraintValidatorContext you are not obliged to build a ConstraintViolation for an
actual property/method/etc. What is going to happen in this case?
I am wondering whether something like an ElementDescritorFinder (similar to
ConstraintFinder) wouldn't be a better alternative (on BeanDescriptor)?
--Hardy
3:56 p.m.
On Fri 2012-11-23 15:50, Hardy Ferentschik wrote:
On 23 Jan 2012, at 3:21 PM, Gunnar Morling <gunnar(a)hibernate.org> wrote:
> > - relaxing NodeBuilderDefinedContext and adding the new contract on it
>
> I wouldn't really like that as it reduces safety of the API.
need to still make up my mind about this one
> > - an explicit new method deprecating addNode
>
> Related to this is BVAL-336 [1]. We've added Node#getDescriptor(), so the
question is what should that return for nodes created by the user via the node builder
API?
>
> We could have a new method
>
> addNode(String name, ElementDescriptor descriptor) or even
> addNode(ElementDescriptor descriptor)
>
> Leaves the question where does the user get the descriptor from? One way could be to
provide access to BeanDescriptor via ConstraintValidatorContext. For nodes added via the
deprecated method getDescriptor() we would likely have to return null.
This is one of the reason I never found it such a good idea to "merry" the path
with the descriptors. In this context we also talked once about the need of a
DummyDescriptor. Using the
ConstraintValidatorContext you are not obliged to build a ConstraintViolation for an
actual property/method/etc. What is going to happen in this case?
I am wondering whether something like an ElementDescritorFinder (similar to
ConstraintFinder) wouldn't be a better alternative (on BeanDescriptor)?
Note that the main reason for having getElementDescriptor on Node is to
be able to know which parameter (index) a constraint belong to.
This was at the time our nicest approach but we can revisit if someone
finds a better approach.
3:54 p.m.
The problem with returning `null` is that it literally creates an
exception clause in your code.
if (eltdescriptor == nukk) {
/something
}
else {
switch(eltdescriptor.getKind()) {
case PARAMETER:
//do something
...
}
So we might want to explore the idea of adding a `UnknownDescriptor` or
`FreeNodeDescriptor` is a specific kind for ad-hoc created via the API.
Should we have the ability to provide the element descriptor when
creating a node? That's a good question. It woud be nice if you could
explore how the code would look like in a "real life" example and see
where getting the elementDescriptor makes the most sense.
Emmanuel
On Fri 2012-11-23 15:21, Gunnar Morling wrote:
> - relaxing NodeBuilderDefinedContext and adding the new contract
on it
I wouldn't really like that as it reduces safety of the API.
> - an explicit new method deprecating addNode
Related to this is BVAL-336 [1]. We've added Node#getDescriptor(), so the
question is what should that return for nodes created by the user via the
node builder API?
We could have a new method
addNode(String name, ElementDescriptor descriptor) or even
addNode(ElementDescriptor descriptor)
Leaves the question where does the user get the descriptor from? One way
could be to provide access to BeanDescriptor via ConstraintValidatorContext.
For nodes added via the deprecated method getDescriptor() we would likely
have to return null.
--Gunnar
[1] https://hibernate.onjira.com/browse/BVAL-336
2012/11/23 Emmanuel Bernard <emmanuel(a)hibernate.org>
> I guess you always learn something new every day :)
> This is indeed only working if we use interception types.
>
> 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
>
> Emmanuel
>
> On Fri 2012-11-23 12:57, Gunnar Morling wrote:
> > Hi,
> >
> > > Another approach would be to make NodeBuilderCustomizableContext a sub
> > insterface of NodeBuilderDefinedContext.
> >
> > I think changing the return type of ConstraintViolationBuilder#addNode()
> > to NodeBuilderCustomizableContext is a problem.
> >
> > While that change is source compatible (code using addNode() can be
> > compiled against the new API version without having to be adapted) it is
> > not binary compatible. That is, code compiled against the previous
> version
> > will fail at runtime:
> >
> > java.lang.NoSuchMethodError:
> >
>
javax.validation.ConstraintValidatorContext$ConstraintViolationBuilder.addNode(Ljava/lang/String;)Ljavax/validation/ConstraintValidatorContext$ConstraintViolationBuilder$NodeBuilderDefinedContext;
> >
> > This error occurs, no matter wether the result of addNode() is assigned
> to
> > a variable or not.
> >
> > What we might try to do is to return an intersection type from
> > ConstraintViolationBuilder#addNode() like this:
> >
> > interface ConstraintViolationBuilder {
> >
> > <T extends NodeBuilderDefinedContext &
> NodeBuilderCustomizableContext>
> > T addNode(String name);
> >
> > ConstraintValidatorContext addConstraintViolation();
> > }
> >
> > This is source compatible and AFAICS it's also binary compatible because
> > the erasure of T is its leftmost bound, that is
> NodeBuilderDefinedContext.
> > That way code compiled against the old API can still dispatch the
> addNode()
> > method, while code written against the new version can invoke all methods
> > defined on NodeBuilderDefinedContext and NodeBuilderCustomizableContext.
> >
> > Btw. I stumbled upon [1] which gives a very good overview on the
> > compatibility of all sorts of API changes.
> >
> > --Gunnar
> >
> > [1]
> >
> http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_Interfaces
> >
> >
> > 2012/11/22 Emmanuel Bernard <emmanuel(a)hibernate.org>
> >
> > > Unless anyone has concern, I plan to go for option 3 on
> > > http://beanvalidation.org/proposals/BVAL-221/
> > > _______________________________________________
> > > 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
>
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
1:07 p.m.
On 23 Jan 2012, at 2:48 PM, Emmanuel Bernard <emmanuel(a)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
4:42 p.m.
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(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
2:10 p.m.
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(a)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(a)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(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
4:46 p.m.
addNode(CLASS_LEVEL) is not a bad idea. The alternative is a full method
as proposed in BVAL-191.
Unfortunately, we cannot solve the problem with this method because of
the second use case I describe in the email below. This example does not
work even if does not make use of addNode(null) :(
Emmanuel
On Wed 2012-12-05 14:10, Hardy Ferentschik wrote:
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(a)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(a)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(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
12:41 a.m.
Regarding BVAL-221, wouldn't that be fixed by the intersection type
approach I suggested? Or do you think it we shouldn't pursue it because its
too magic or uncommon?
2012/12/5 Emmanuel Bernard <emmanuel(a)hibernate.org>
addNode(CLASS_LEVEL) is not a bad idea. The alternative is a full
method
as proposed in BVAL-191.
Unfortunately, we cannot solve the problem with this method because of
the second use case I describe in the email below. This example does not
work even if does not make use of addNode(null) :(
Emmanuel
On Wed 2012-12-05 14:10, Hardy Ferentschik wrote:
> 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(a)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(a)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(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
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
9:56 a.m.
Yes it would. That was my question earlier in this thread. Do we consider the intersection
trick acceptable in the specification.
On 7 déc. 2012, at 00:41, Gunnar Morling <gunnar(a)hibernate.org> wrote:
Regarding BVAL-221, wouldn't that be fixed by the intersection
type approach I suggested? Or do you think it we shouldn't pursue it because its too
magic or uncommon?
2012/12/5 Emmanuel Bernard <emmanuel(a)hibernate.org>
> addNode(CLASS_LEVEL) is not a bad idea. The alternative is a full method
> as proposed in BVAL-191.
>
> Unfortunately, we cannot solve the problem with this method because of
> the second use case I describe in the email below. This example does not
> work even if does not make use of addNode(null) :(
>
> Emmanuel
>
> On Wed 2012-12-05 14:10, Hardy Ferentschik wrote:
> > 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(a)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(a)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(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
> _______________________________________________
> 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
5:05 p.m.
If we test is and it works I am fine with it.
I had tried it out and from a first impression it looked ok. I'll brush up
the test a bit and push it to GH.
--Gunnar
2012/12/7 Hardy Ferentschik <hardy(a)hibernate.org>
>
> On 7 Jan 2012, at 9:56 AM, Emmanuel Bernard <emmanuel(a)hibernate.org>
> wrote:
>
> > Yes it would. That was my question earlier in this thread. Do we
> consider the intersection trick acceptable in the specification.
>
If we test is and it works I am fine with it.
>
> --Hardy
>
>
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
4163
days inactive
4181
days old
beanvalidation-dev@lists.jboss.org
14 comments
3 participants
participants (3)
-
Emmanuel Bernard -
Gunnar Morling -
Hardy Ferentschik