Renaming ExecutableDescriptor#areParametersConstrained()?
by Gunnar Morling
Hi all,
Hardy and I were wondering whether the method
name ExecutableDescriptor#areParametersConstrained() is the best choice.
The name seems a bit unintuitive, when e.g. looking via auto-completion for
boolean getter methods which typically start with "is" or "has". Maybe
* "isParameterConstrained()" (as in "is constrained on at least one
parameter") or
* "hasConstrainedParameters()" would be better alternatives?
The latter would also nicely match with
BeanDescriptor#hasConstrainedExecutables().
WDYT?
--Gunnar
11 years, 2 months
Creating custom nodes in cross-parameter constraint validators
by Gunnar Morling
Hi,
We added the method addParameterNode() to the node builder API, allowing
for the creation of custom subpaths in cross-parameter constraint
validators.
Currently such a path will look like this (the first two nodes are the
default ones, the remain ones are custom created nodes):
METHOD -> CROSS_PARAMETER -> PARAMETER -> ...
I'm wondering whether it wouldn't be better to get rid of the
cross-parameter node in this case:
METHOD -> PARAMETER -> ...
E.g. when looking at the following example:
@OldAndNewPasswordDifferent
public void resetPassword(String new, String old) { ... }
Then I think the following path might be what constraint authors want:
MethodNode("resetPassword") -> ParameterNode("old", arg1)
As it seems, the RI does the same when custom nodes are added in
class-level constraint validators (i.e. no node for the class-level
constraint will be part of the path). Shortly looking in the spec, I
couldn't find an example mandating this behavior, though.
Any thoughts?
--Gunnar
11 years, 2 months
Spec review
by Gunnar Morling
Hi all,
As you know the proposed final draft of our spec is due in two days, on
Feb. 20th.
So anyone who can spare some time to do a review of the latest spec draft (
http://beanvalidation.org/latest-draft/spec/) is more than welcomed to give
feedback. Let us know if you see any issues, e.g. related to
comprehensibility, consistency, wording, ambiguities, typos, examples, you
get the picture :)
Thanks,
--Gunnar
11 years, 2 months
Specifying no validator for cross-param constraint
by Gunnar Morling
All,
I just noticed that it's currently not possible to specify *no* validator
type for cross-parameter constraints:
@CrossParameterConstraint(validatedBy=...)
MyCrossParamConstraint{ ... }
Not specifying a validator might be relevant for people who want to hide
their validator implementations and e.g. configure them via XML. Null is
not allowed as annotation member value, and it's also not possible to
specify ConstraintValidator.class as dummy value (doesn't compile).
Note that the problem doesn't exist with @Constraint where validatedBy()
expects an array of class objects, allowing empty arrays. The following
solutions come to my mind:
#1 Get rid of @CrossParameterConstraint, use @Validates on validator
implementations, as discussed as alternative before
#2 Offer a dummy NoOpValidator in the API, obviously rather inelegant
#3 Ignore the problem, maybe the use case is really uncommon? If required,
recommend people to use their own dummy validator and override it via XML
On a related note, if sticking to @CrossParameterConstraint, we may
consider to narrow the type of supported validators:
public @interface CrossParameterConstraint {
Class<? extends ConstraintValidator<?, Object[]>> validatedBy();
}
I.e., only validators for Object[] would be allowed (currently that's
demanded by the JavaDocs only).
Any thoughts?
--Gunnar
11 years, 2 months
Node#getElementDescriptor() for user-created nodes
by Gunnar Morling
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
11 years, 2 months
Representation of cross-parameter constraints
by Gunnar Morling
Hi all,
Could you give us your feedback on BVAL-370 [1]?
This is about how cross-parameter constraints should be represented in the
metadata and other APIs. Currently, cross-parameter constraints are
represented directly on the ExecutableDescriptor, contrary to parameter and
return value constraints, which are reachable via
ExecutableDescriptor#getParameterDescriptors() and
getReturnValueDescriptor().
A similar question arises when building property paths for cross-parameter
constraints. This is currently still undefined in the spec; The RI adds one
node representing the hosting executable, while another node is added for
parameter and return value violations.
The same with XML descriptors, where <cross-parameter-constraint>
element(s) are directly added to the method/constructor element, as opposed
to parameter/return value constraints which are added to the
<parameter>/<return-value> sub-elements.
The question is, whether we should add such an intermediary descriptor/node
also for cross-parameter constraints, e.g.
ExecutableDescriptor#getCrossParameterDescriptor(). This would make the API
more regular (all executable descriptors/constraints are retrieved by
stepping one level down into the model), possibly helping to avoid
confusions. By introducing an element CROSS_PARAMETER, also cross-parameter
path nodes can be recognized very explicitly (today this is indicated by
Kind.METHOD/CONSTRUCTOR on the leaf node).
On the other hand this is technically not strictly required (all
information can be retrieved unambiguously from the current API). Also
would be the question what to return from e.g.
ExecutableDescriptor#getConstraints() (which currently is re-defined to
return cross-param constraints only). One possibility would be to consider
all executable constraints and extend the constraint finder API to allow
filtering by ElementDescriptor.Kind.
Any thoughts?
Thanks,
--Gunnar
[1] https://hibernate.onjira.com/browse/BVAL-370
11 years, 2 months
Making @ValidateExecutable easier to use on methods
by Emmanuel Bernard
Santiago from the JAX-RS EG is concerned that we have created an
over-engineered solution to enable / disable method validation on a per
method basis.
We have good reasons for it but I am wondering if we can make it a bit
easier to enable or disable methods out of the box.
Remember, the default is to ignore getters. If we make
@ValidateExecutable.type default to ALL, enabling it on a getter would
be
@NotEmpty @ValidateExecutable
public String getAction() {...}
instead of
@NotEmpty @ValidateExecutable(type=ExecutableType.ALL)
public String getAction() {...}
We would still need the verbose approach to disable validation
@NotEmpty @ValidateExecutable(type=ExecutableType.NONE)
public String getAction() {...}
I am open to suggestions to make that easier on methods. I don't think
an additional annotation will make anything easier
Maybe renaming NONE with OFF might help? It's a bit weirder on a
class-level but that might work.
Thoughts?
Emmanuel
11 years, 2 months
Making ElementDescriptor.Kind a top-level type
by Gunnar Morling
Experts,
Currently the enumeration Kind (as returned by ElementDescriptor#getKind())
is an inner enum of the ElementDescriptor interface.
In order to allow for its future re-use in other contexts, independent from
element descriptors, it seems reasonable to make this a top-level type.
Unfortunately the natural names, ElementKind and ElementType are taken by
JDK types, and if possible we should avoid a collision with these.
Candidates are
* ConstrainedElementKind
* ValidatedElementKind
Any preferences or other suggestions?
Thanks,
--Gunnar
11 years, 2 months
Fwd: [jsr345-experts] Interceptors spec 1.2 DRAFT2 is available for review
by Emmanuel Bernard
Here is the interceptor spec changes BV uses when integrating with CDI for method interception.
Begin forwarded message:
> From: Marina Vatkina <marina.vatkina(a)oracle.com>
> Date: 12 février 2013 17:50:41 UTC+01:00
> To: Emmanuel Bernard <emmanuel(a)hibernate.org>
> Subject: Fwd: [jsr345-experts] Interceptors spec 1.2 DRAFT2 is available for review
> Reply-To: marina.vatkina(a)oracle.com
>
> Forgot to cc you...
>
> -marina
> -------- Original Message --------
> Subject: [jsr345-experts] Interceptors spec 1.2 DRAFT2 is available for review
> Date: Mon, 11 Feb 2013 12:19:44 -0800
> From: Marina Vatkina <marina.vatkina(a)oracle.com>
> Reply-To: marina.vatkina(a)oracle.com
> To: jsr345-experts(a)ejb-spec.java.net, users(a)interceptors-spec.java.net
>
> Experts,
>
> You can now review the 2nd draft of the Interceptors spec. This version
> incorporates most of the review comments and the AroundConstruct
> interceptor. The full list of changes is below (the change log at the
> end of the document is an accumulative list).
>
> The draft:
> http://java.net/projects/interceptors-spec/downloads/download/interceptor...
> The diff from draft1:
> http://java.net/projects/interceptors-spec/downloads/download/interceptor...
>
> Please review it (AroundConstruct) in particular ASAP.
>
> Pete, please forward it to the CDI EG.
>
> The list of changes as found in the revision history:
>
> - Fixed page numbers in the book
> - Minor editorial changes
> - Moved Section 5.6 to the end of Chapter 5
> - Moved rule on invocation order of interceptors with superclasses to
> Section 5.3
> - Replaced rules on the Nonbinding annotation with references to the CDI
> spec
> - Clarified that an around-invoke interceptor method, around-timeout
> interceptor method, and lifecycle callback interceptor methods for
> different lifecycle events may be defined on the same interceptor class
> - Added the AroundConstruct lifecycle callback interceptor
> - Extended InvocationContext with getConstructor method; adjusted rules
> on the InvocationContext.getTarget return value, and
> InvocationContext.proceed result
> - Added notes to the around-invoke and around-timeout about the throw
> Exception clause in their signatures
> - Added a not that transaction context of interceptors may be changed by
> transactional interceptors in the invocation chain
> - Explained how interceptors are enabled
>
> Happy reading,
> -marina
>
>
>
11 years, 2 months