bval-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carlos Vara <bashfl...@gmail.com>
Subject Re: graduating the guice integration from sandbox
Date Thu, 13 May 2010 11:20:08 GMT
Hi Simone,

I took a look at your new interceptor. Looking very nice and clean to
my eyes, good job there!

Arrivederci (I'm afraid my italian skills are nowhere close to your
spanish ones),
Carlos

On Tue, May 11, 2010 at 4:46 PM, Simone Tripodi
<simone.tripodi@gmail.com> wrote:
> Hi all,
> I just integrated Carlos Vara's suggestions (thanks a lot!!!) and
> tests seem working nicely. I'd ask once again a little review and, if
> it is ok for you, I'd start moving the module from the sandbox to the
> top level. Please let me know!!! :)
> Best regards, have a nice day,
> Simo
>
> http://people.apache.org/~simonetripodi/
>
>
>
> On Mon, May 10, 2010 at 10:37 PM, Donald Woods <dwoods@apache.org> wrote:
>> Great review Carlos!
>>
>> Simone, I haven't had time to look thoroughly at the code, but it looks
>> like it uses BVAL correctly and is a good complement module.  I was able
>> to build the module and it passed rat:check, so I'd vote for bringing it
>> over into trunk.
>>
>>
>> -Donald
>>
>>
>> On 5/9/10 6:10 PM, Carlos Vara wrote:
>>> 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 <simone.tripodi@gmail.com>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/>
>>>>
>>>
>>
>

Mime
View raw message