admin@glassfish.java.net

Re: Code Review Request

From: Rajeshwar V Patil <Rajeshwar.Patil_at_Sun.COM>
Date: Thu, 01 Nov 2007 11:14:15 -0800

Thanks Llyod.

----- Original Message -----
From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Thursday, November 1, 2007 10:06 am
Subject: Re: Code Review Request
To: admin_at_glassfish.dev.java.net


> Rajeshwar,
>
> I have responded to admin_at_glassfish. Such change proposals should be
>
> publicly sent.
>
>
> Comments:
> 1. What's a "new software"? New software what? (The grammar is
> non-English). We have "number of modules eg "NumModules". But
> "number of software" doesn't make sense. Is it the number of
> software "modules" or what?
>
Yes, it is number of new modules. The existing API NumModules is essentially referring to new updates whereas new API is referring to new modules. We should have called existing method as NumUpdates and new method as NumModules. But, to keep changes to the minimum and to avoid changes to the tested code in this update release, I avoided renaming of existing method appropriately. But for the new method, I can change it to something else if any suggestions.

In UC we have Available Updates and Available Software tabs. Any new module hosted on server can show in either of these based on whether previous version of it is installed on users m/c or not.

> 2. Is there a build dependency such that SchedulerImpl cannot be
> imported? The reliance on Class.forName() and reflection is fragile,
>
> should the class change.
If you see the comments(by Satish) on existing API(getNumModules), its looks like a deliberate decision to use reflection. Just followed the way its done for the existing API.

Thanks
Rajeshwar

>
> Lloyd
>
> On Oct 31, 2007, at 6:01 PM, Rajeshwar V Patil wrote:
>
> > Hello Llyod/Nazrul,
> > I am planning to checkin following code changes to the admin/
> > appserv-api module. Please review the following code diffs.
> >
> > Brief context:
> > ---------------
> > Currently, on Admin GUI login, we report no of updates available if
>
> > any. Now, we also want to report number of new software available
>
> > if any(Bug- https://updatecenter.dev.java.net/issues/show_bug.cgi?
> > id=336). UC code is fixed. Need to make corresponding changes in GF
>
> > backend and Admin GUI.
> >
> >
> > What the change is:
> > ----------------------
> > I have added new api(and implementation) to provide the number of
> > new software available on update center server. Admin GUI code will
>
> > call this api on user login order to report any new software
> > available.
> >
> >
> > Code Diffs:
> > ------------
> >
> > Index: UpdateStatus.java
> > ===================================================================
> > RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/
> > management/ext/update/UpdateStatus.java,v
> > retrieving revision 1.2
> > diff -c -r1.2 UpdateStatus.java
> > *** UpdateStatus.java 5 May 2007 05:30:49 -0000 1.2
> > --- UpdateStatus.java 1 Nov 2007 00:36:37 -0000
> > ***************
> > *** 71,74 ****
> > --- 71,80 ----
> > */
> > public int getNumModules();
> >
> > + /**
> > + * Returns number of new software available on update
> > center server.
> > + * @returns int the number of new software available
> > since last invocation.
> > + */
> > + public int getNumNewSoftware();
> > +
> > }
> >
> >
> > Index: UpdateStatusImpl.java
> > ===================================================================
> > RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
> > enterprise/management/ext/update/UpdateStatusImpl.java,v
> > retrieving revision 1.2
> > diff -c -r1.2 UpdateStatusImpl.java
> > *** UpdateStatusImpl.java 5 May 2007 05:23:28 -0000 1.2
> > --- UpdateStatusImpl.java 1 Nov 2007 00:35:33 -0000
> > ***************
> > *** 66,73 ****
> > return( AMX.GROUP_UTILITY );
> > }
> >
> > ! public int
> > ! getNumModules() {
> >
> > // Even though Update Center jars are packaged with
> > Application Server.
> > // They could be un-installed. Doing a reflection based
> > implementation
> > --- 66,76 ----
> > return( AMX.GROUP_UTILITY );
> > }
> >
> > ! /**
> > ! * Returns number of new updates available on update center
>
> > server.
> > ! * @returns int the number of updates available since last
>
> > invocation.
> > ! */
> > ! public int getNumModules() {
> >
> > // Even though Update Center jars are packaged with
> > Application Server.
> > // They could be un-installed. Doing a reflection based
> > implementation
> > ***************
> > *** 97,100 ****
> > --- 100,131 ----
> > return num;
> > }
> >
> > + /**
> > + * Returns number of new software available on update center
>
> > server.
> > + * @returns int the number of new software available since
>
> > last invocation.
> > + */
> > + public int getNumNewSoftware() {
> > + int num = 0;
> > + try {
> > + final Class c = Class.forName(
> > + "com.sun.enterprise.update.schedule.SchedulerImpl");
> > + final Object s = c.newInstance();
> > +
> > + if (s != null) {
> > + // call runNewSoftwareCheck() with zero arguments
> > + final Class[] paramTypes = null;
> > + final Object[] argTypes = null;
> > +
> > + final Method m = c.getMethod
> > ("runNewSoftwareCheck", paramTypes);
> > + final Integer retval = (Integer) m.invoke(s,
> > argTypes);
> > + num = retval.intValue();
> > + }
> > + } catch(Throwable t) {
> > + getMBeanLogger().warning(t.getMessage());
> > + if(getMBeanLogger().isLoggable(Level.FINE))
> > + t.printStackTrace();
> > +
> > + }
> > + return num;
> > + }
> > }
> >
> >
> > Thanks
> > Rajeshwar
> > <new-files.zip>
> > <old-files.zip>
>
> ---
> Lloyd L Chambers
> lloyd.chambers_at_sun.com
> Sun Microsystems, Inc
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>