geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anita kulshreshtha <a_kuls...@yahoo.com>
Subject Re: svn commit: r485321 - in /geronimo/server/trunk/modules: geronimo-kernel/src/main/java/org/apache/geronimo/gbean/ geronimo-kernel/src/main/java/org/apache/geronimo/gbean/runtime/ geronimo-kernel/src/test/java/org/apache/geronimo/gbean/ geronimo-k
Date Tue, 12 Dec 2006 13:32:28 GMT
 Thanks Gianny, David J and Dain for providing feedback. I could have
certainly used it before commiting rev 485321. IIUC, here is what needs
to be done:
1. Revert rev 485321. Make the necessary code changes to
GBeanInfoBuilder while keeping the signatures of addOpeartion methods
same.
2. GoperationInfo must contain a field named targetClass
(declaringClass?). There are 2 options:
   a.. Just add the field and maintain backward compatibility
   b.. Clean the code, i.e. remove unused field methodName and break
the compatibility.
     I am leaning towards b.
3. The fact that in some cases GBeanInfoBuilder.addInterfaces() Line
295 ends up with two methods with same name and signatures but
different return type, it is not by design. Most of the time it is due
to a badly overridden getter/setter/operation, and it is not the
responsibility of GBeanInfoBuilder to flag that as an error. After the
modification proposed in 2, the two operations will have different
GOpearationInfo. AFAIK, this is not an issue when operations are added
using addOparation.
  I am going to revert rev 485321 today.
Thanks
Anita

--- David Jencks <david_jencks@yahoo.com> wrote:

> 
> On Dec 11, 2006, at 7:18 AM, anita kulshreshtha wrote:
> 
> >   When we add inteface using addInterface(..), we can end up with
> two
> > methods like this.
> 
> You can't have a class that implements both interfaces if they have  
> methods whose signature differs only in the return type.  If there  
> are problems with trying to expose management interfaces that are not
>  
> actually implemented by the underlying bean maybe we should look for 
> 
> other ways of dealing with the management aspects.  Are there actual 
> 
> problems with existing gbeans extracting the return type for  
> operations?  If so, please indicate which gbean and which  
> operation.... I don't want to get lost in too much abstraction as I  
> so often do :-)
> 
> I very much share Gianni's concerns and was about to write a similar 
> 
> note.
> 
> thanks
> david jencks
> 
> > This is not a very good example because the
> > objectName will end up as an attribute in GBeanInfoBuilder, not an
> > operation.
> >
> > thanks
> > Anita
> >
> > --- Vamsavardhana Reddy <c1vamsi1c@gmail.com> wrote:
> >
> >> It is not allowed to have public Object getObjectName() and public
> >> String
> >> getObjectName() simultaneously.
> >>
> >> --vamsi
> >>
> >> On 12/11/06, anita kulshreshtha <a_kulshre@yahoo.com> wrote:
> >>>
> >>> Gianny,
> >>>     Thanks for looking into this. I did consider the easy way
> out.
> >> But
> >>> the retrun type is part of the method signature. What happens if
> we
> >>> have
> >>>
> >>> public class Myclass {
> >>> public Object getObjectName()
> >>> public String getObjectName()
> >>> ..........................
> >>> }
> >>>     If JMXUtil was patched which one should it return?
> >>>
> >>> Thanks
> >>> Anita
> >>>
> >>> --- Gianny Damour <gianny.damour@optusnet.com.au> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> I am quickly scanning this commit and I would like to know if it
> >> was
> >>>>
> >>>> not a little bit less intrusive to keep the existing
> addOperation
> >> and
> >>>>
> >>>> search for the return type of the added operations against the
> >> target
> >>>>
> >>>> gbeanType. This way, developers do not need to specify the
> return
> >>>> type of the operations (also, the migration of the existing
> >> GBeanInfo
> >>>>
> >>>> could have been avoided).
> >>>>
> >>>> After reading GERONIMO-2607, it seems that the goal of the
> change
> >> was
> >>>>
> >>>> to have a return type defined within JConsole for the GBean
> >>>> operations. It seems that patching JMXUtil.toMBeanInfo would
> have
> >>>> been another implementation approach: while getting the exposed
> >>>> operations, the GBean class could be searched for returned
> types.
> >> One
> >>>>
> >>>> of the advantages would have been to keep backward
> compatibility.
> >>>>
> >>>> Thanks,
> >>>> Gianny
> >>>>
> >>>> On 11/12/2006, at 11:14 AM, akulshreshtha@apache.org wrote:
> >>>>
> >>>>> Author: akulshreshtha
> >>>>> Date: Sun Dec 10 16:14:46 2006
> >>>>> New Revision: 485321
> >>>>>
> >>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=485321
> >>>>> Log:
> >>>>> GERONIMO-2607 Added returnType to GOperationInfo, This modifies
> >>>>> GBeanInfoBuilder and breaks backward compatibility
> >>>>>
> >>>>> Modified:
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/
> >>>>> apache/geronimo/gbean/DynamicGOperationInfo.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/
> >>>>> apache/geronimo/gbean/GBeanInfoBuilder.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/
> >>>>> apache/geronimo/gbean/GOperationInfo.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/
> >>>>> apache/geronimo/gbean/runtime/GBeanOperation.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/test/java/org/
> >>>>> apache/geronimo/gbean/GBeanInfoTest.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/test/java/org/
> >>>>> apache/geronimo/kernel/MockGBean.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/test/java/org/
> >>>>> apache/geronimo/kernel/config/MyGBean.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-system/src/main/java/org/
> >>>>> apache/geronimo/system/jmx/JMXUtil.java
> >>>>>
> >>>> geronimo/server/trunk/modules/geronimo-system/src/main/java/org/
> >>>>> apache/geronimo/system/logging/log4j/Log4jService.java
> >>>>>
> >>>>> Modified:
> >> geronimo/server/trunk/modules/geronimo-kernel/src/main/
> >>>>> java/org/apache/geronimo/gbean/DynamicGOperationInfo.java
> >>>>> URL:
> >> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> >>>>> geronimo-kernel/src/main/java/org/apache/geronimo/gbean/
> >>>>>
> >>
> DynamicGOperationInfo.java?view=diff&rev=485321&r1=485320&r2=485321
> >>>>>
> >>>>
> >>>
> >>
> >
>
======================================================================
> >>>>
> >>>>> ========
> >>>>> ---
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/
> >>>>> apache/geronimo/gbean/DynamicGOperationInfo.java (original)
> >>>>> +++
> >>>> geronimo/server/trunk/modules/geronimo-kernel/src/main/java/org/
> >>>>> apache/geronimo/gbean/DynamicGOperationInfo.java Sun Dec 10
> >>>>> 16:14:46 2006
> >>>>> @@ -24,14 +24,14 @@
> >>>>>   */
> >>>>>  public class DynamicGOperationInfo extends GOperationInfo {
> >>>>>      public DynamicGOperationInfo(String name) {
> >>>>> -        super(name);
> >>>>> +        super(name, "java.lang.Object");
> >>>>>      }
> >>>>>
> >>>>>      public DynamicGOperationInfo(String name, String[]
> >> paramTypes)
> >>>> {
> >>>>> -        super(name, paramTypes);
> >>>>> +        super(name, paramTypes, "java.lang.Object");
> >>>>>      }
> >>>>>
> >>>>>      public DynamicGOperationInfo(String name, List parameters)
> >> {
> >>>>> -        super(name, parameters);
> >>>>> +        super(name, parameters, "java.lang.Object");
> >>>>>      }
> >>>>>  }
> >>>>>
> >>>>> Modified:
> >> geronimo/server/trunk/modules/geronimo-kernel/src/main/
> >>>>> java/org/apache/geronimo/gbean/GBeanInfoBuilder.java
> >>>>> URL:
> >> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> >>>>> geronimo-kernel/src/main/java/org/apache/geronimo/gbean/
> >>>>> GBeanInfoBuilder.java?view=diff&rev=485321&r1=485320&r2=485321
> >>>>>
> >>>>
> >>>
> >>
> >
>
======================================================================
> >>>>
> 
=== message truncated ===



 
____________________________________________________________________________________
Do you Yahoo!?
Everyone is raving about the all-new Yahoo! Mail beta.
http://new.mail.yahoo.com

Mime
View raw message