Hi Simone,
I took a look at the code, very good work there!
I'm afraid I can't comment much about the guice part as I'm more of a spring
man myself, but I can comment a little on the validation part.
First, just to check that I understood what it does right:
- It creates an interceptor for method calls annotated with @Validate and
validates all the arguments. In case any validation error is found, it
throws a CVE.
If the above is right, a few pointers on the validation part:
- If you validate the arguments individually, maybe having an extra
annotation in them can help improve the speed of the code. Take care that
calling validator.validate(..) for all basic types (Strings, Integers, etc.)
will always output an empty set, and those arguments are very common.
You can use @Valid as a marker for the arguments that you want to validate,
this way, a validated method call would be like this:
@Validate
public String myMethod(String notValidated, @Valid Person validatedPerson) {
... }
You may decide to validate all arguments in case @Validate is present and no
argument is annotated with @Valid.
- Although it is not portable, you may also want to take a look at the
method validation API that the Apache implementation has. It works quite
well in this scenario. I posted a quick implementation I did with AspectJ
here:
http://carinae.net/2010/04/automatic-validation-method-calls-with-jsr-303-appendix-c/
If you are interested, you could do a second interceptor that takes
advantage of it in case the JSR-303 implementation is the apache one.
- One last thing, in ValidateMethodInterceptor.invoke(), you may want to
also validate the return value before returning.
Oh, and my non-binding opinion about moving it out of the sandbox would be a
yes. All the above comments are just possible improvements, IMHO code uses
the validation API correctly to solve an interesting problem.
Regards,
Carlos
On Sat, May 8, 2010 at 1:07 PM, Simone Tripodi <[email protected]>wrote:
> Hi all guys,
> I'd like to ask you what you think about moving the actual google
> guice integration support from sandbox to current modules.
> I just integrated the groups support in the AOP interceptor and it
> seems working nicely, at least for me, I'd really would like if
> someone could provide feedbacks.
> Every kind of help will be more than appreciated, thanks in advance!
> Simo
>
> http://people.apache.org/~simonetripodi/<http://people.apache.org/%7Esimonetripodi/>
>
