tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: svn commit: r583858 - /tomcat/tc6.0.x/trunk/STATUS
Date Fri, 12 Oct 2007 14:05:10 GMT
jean-frederic clere wrote:
> Filip Hanik - Dev Lists wrote:
>   
>> to be clear, let me address your statements
>>
>> remm -1: removes support for the production APR connectors
>> Absolutely not, doesn't change the way connectors are evaluated.
>>
>> remm -1: no real benefit of the patch
>> The NIO connector attributes are now supported, the implementation is
>> simpler and more flexible, and can be expanded to other components not
>> supported today
>>     
>
> I think I will -1:
> There is the following in
> java/org/apache/tomcat/util/IntrospectionUtils.java
> +++
>             if (setPropertyMethod != null) {
>                 Object params[] = new Object[2];
>                 params[0] = name;
>                 params[1] = value;
>                 setPropertyMethod.invoke(o, params);
>                 return true;
>             }
> +++
> Can't we try to check the Object returned by setPropertyMethod.invoke
> and set the return code according to the value of the Object?
>   
actually, I want the boolean setProperty to have preferential treatment, 
ie, if it exists, then use it before the void.

so what I mean, if you have the following in an object,

    public void setProperty(String name, String value) {
    public boolean setProperty(Object name, Object value) {

then the  method with a boolean return value should be called.

however, you do bring up one point, if we have the following scenario

    public void setProperty(String name, String value) {
    public boolean setProperty(int name, int value) {

the void method should be called, I have modified the patch to catch the 
IllegalArgumentException and try the other method if existing.

 > @@ -334,17 +335,37 @@
60c60,71
< +                    return (Boolean) setPropertyMethodBool.invoke(o, 
params);
---
 > +                    try {
 > +                        return (Boolean) 
setPropertyMethodBool.invoke(o, params);
 > +                    }catch (IllegalArgumentException biae) {
 > +                        //the boolean method had the wrong
 > +                        //parameter types. lets try the other
 > +                        if (setPropertyMethodVoid!=null) {
 > +                            setPropertyMethodVoid.invoke(o, params);
 > +                            return true;
 > +                        }else {
 > +                            throw biae;
 > +                        }
 > +                    }


your suggestion, would mean the method that actually gets called, would 
be at random, depending on what order the method array comes back in 
from the reflection API.

the intro spector can of course be even more enhanced to check for 
argument types, but that is outside the scope of this patch.

Filip
> Cheers
>
> Jean-Frederic
>
>
>   
>> furthermore, there are no exception cases anymore, like the
>> o.a.c.connector.Connector object, is treated same way as before
>>
>> Filip
>>
>> Filip Hanik - Dev Lists wrote:
>>     
>>> the patch doesn't change the behavior for the APR connector, can you
>>> please explain what you mean?
>>> apply the patch, try it out on your APR connector, and it should work
>>> just like before
>>> Filip
>>>
>>> remm@apache.org wrote:
>>>       
>>>> Author: remm
>>>> Date: Thu Oct 11 08:44:34 2007
>>>> New Revision: 583858
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=583858&view=rev
>>>> Log:
>>>> - Vote.
>>>>
>>>> Modified:
>>>>     tomcat/tc6.0.x/trunk/STATUS
>>>>
>>>> Modified: tomcat/tc6.0.x/trunk/STATUS
>>>> URL:
>>>> http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS?rev=583858&r1=583857&r2=583858&view=diff
>>>>
>>>> ==============================================================================
>>>>
>>>> --- tomcat/tc6.0.x/trunk/STATUS (original)
>>>> +++ tomcat/tc6.0.x/trunk/STATUS Thu Oct 11 08:44:34 2007
>>>> @@ -50,4 +50,4 @@
>>>>  * to accept or reject the property
>>>>   
>>>> http://people.apache.org/~fhanik/patches/digester-attribute-warnings.patch
>>>>
>>>>    +1: fhanik
>>>> -  -1: +  -1: remm (removes support for the production APR
>>>> connectors, no real benefit of the patch)
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>>
>>>>
>>>>
>>>>   
>>>>         
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>
>>>
>>>
>>>       
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
>>     
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
>   


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message