tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jean-frederic clere <jfcl...@gmail.com>
Subject Re: svn commit: r583858 - /tomcat/tc6.0.x/trunk/STATUS
Date Sat, 13 Oct 2007 08:46:42 GMT
Filip Hanik - Dev Lists wrote:
> 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.

Sure but what I am just telling is that we should try to return the
Boolean value in case a Boolean is returned.

> 
> 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.

I don't think that we will ever have a code like the above and below
examples...

> 
> 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.

Yes we both agree we need a better patch here, no?

Cheers

Jean-Frederic

> 
> 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
> 
> 


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


Mime
View raw message