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 Sat, 13 Oct 2007 21:56:43 GMT
jean-frederic clere wrote:
> 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?
>   
I don't want to cram too much into one patch, this patch does
1. allows setProperty to return boolean, and this patch does return the 
value as you asked for
2. NIO connector checks for invalid attributes
3. Simplifies the Connector component to not be an exception case

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


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


Mime
View raw message