You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Aaron Mulder <am...@alumni.princeton.edu> on 2005/07/23 07:11:50 UTC

Web Console: Portlet Review

	Well, I actually worked with a couple portlets today.  For
starters, I think the diff at the bottom of this message speaks volumes
about why a management API based on interfaces is nice.  It takes out
Strings, constants to hold the Strings, the Kernel variable, and various
casts, and instead stores things like the current Server and JVM in the
session and queries them to get the data it needs.  The code is smaller 
and more reliable.  :)

	As for the old code, I'm not thrilled with it even besides the 
kernel invocations, for a couple reasons (and I just looked at the two 
portlets on the front Server Info page so far):

 - The portlets that need to use a Kernel keep one in a local variable, 
and populate it by manually looking up a kernel during initialization.  I 
would really have preferred to see common "get kernel" logic at least, if 
not a common utility to hold the current kernel.

 - The portlets assume they are running within the server in question, 
doing things like calling System.getProperties to get the "server's" 
system properties (and also using a local kernel, of course).  While I 
have to agree that it works for now, I don't think it's any harder to 
write this so it would work against a remote server, and it ends up being 
cleaner anyway.

 - The portlets store data in static variables, even though they are not 
reused from request to request.  My J2EE application multithreading 
sensors are going off, though I don't know for sure whether a portlet 
instance can service more than one request at a time.  But I also don't 
like using static variables for what is otherwise a totally stateless 
class, and there's especially no reason to if they are reassigned at the 
beginning of every request.

 - Many of the kernel invocations returning objects are just popped into a 
collection without casting, which is almost as bad as casting, because 
then there's no assurance that if the JSP needed a particular type to be 
there, the value would be of the right type.  Of course the JSPs I looked 
just print objects and don't care about type.

 - The JSP for printing system properties, for example, has separate lines 
for every individual property, and manually sets the background style for 
each, to achieve the alternating row color.  There are just so many better 
ways to do this, that take advantage of, say, a loop, and don't involve 
needing to manually restyle every row after inserting a new row, and so 
on.

	Anyway, I think the portlets are going to need a bit of attention.  
That's OK I guess, since I'd like to migrate most of them to the new
management API anyway, so we'll have occasion to touch them, but I figured
I'd give my thoughts on where things stand.

Thanks,
	Aaron

Index: console-standard/src/java/org/apache/geronimo/console/infomanager/ServerInfoPortlet.java
===================================================================
--- console-standard/src/java/org/apache/geronimo/console/infomanager/ServerInfoPortlet.java	(revision 224404)
+++ console-standard/src/java/org/apache/geronimo/console/infomanager/ServerInfoPortlet.java	(working copy)
@@ -22,7 +22,6 @@
 import java.util.HashMap;
 import java.util.Map;
 
-import javax.management.ObjectName;
 import javax.portlet.ActionRequest;
 import javax.portlet.ActionResponse;
 import javax.portlet.GenericPortlet;
@@ -33,56 +32,22 @@
 import javax.portlet.RenderResponse;
 import javax.portlet.WindowState;
 
-import org.apache.geronimo.console.util.ObjectNameConstants;
-import org.apache.geronimo.kernel.Kernel;
-import org.apache.geronimo.kernel.KernelRegistry;
+import org.apache.geronimo.console.util.PortletManager;
+import org.apache.geronimo.j2ee.management.geronimo.JVM;
 
+/**
+ * Calculates various information about the server to display in the server
+ * info portlet view (on of several JSPs depending on the portlet state).
+ *
+ * @version $Rev: 46019 $ $Date: 2004-09-14 05:56:06 -0400 (Tue, 14 Sep 2004) $
+ */
 public class ServerInfoPortlet extends GenericPortlet {
-
-    public static final String SVRINFO_BASEDIR = "baseDirectory";
-
-    public static final String SVRINFO_VERSION = "version";
-
-    public static final String SVRINFO_BUILDDATE = "buildDate";
-
-    public static final String SVRINFO_BUILDTIME = "buildTime";
-
-    public static final String SVRINFO_COPYRIGHT = "copyright";
-
-    public static final String SVRINFO_PLATFORMARCH = "platformArch";
-
-    public static final String SVRINFO_GERONIMO_BUILD_VERSION = "geronimoBuildVersion";
-
-    public static final String SVRINFO_GERONIMO_SPEC_VERSION = "geronimoSpecVersion";
-
-    public static final String SVRINFO_PORTAL_CORE_VERSION = "portalCoreVersion";
-
-    public static final String JVMIMPL_JAVAVER = "javaVersion";
-
-    public static final String JVMIMPL_JAVAVENDOR = "javaVendor";
-
-    public static final String JVMIMPL_NODE = "node";
-
-    public static final String JVMIMPL_FREEMEM = "freeMemory";
-
-    public static final String JVMIMPL_MAXMEM = "maxMemory";
-
-    public static final String JVMIMPL_TOTALMEM = "totalMemory";
-
-    public static final String JVMIMPL_AVAILABLEPROCS = "availableProcessors";
-
     private static final String NORMALVIEW_JSP = "/WEB-INF/view/infomanager/svrInfoNormal.jsp";
 
     private static final String MAXIMIZEDVIEW_JSP = "/WEB-INF/view/infomanager/svrInfoMaximized.jsp";
 
     private static final String HELPVIEW_JSP = "/WEB-INF/view/infomanager/svrInfoHelp.jsp";
 
-    private static Map svrProps = new HashMap();
-
-    private static Map jvmProps = new HashMap();
-
-    private Kernel kernel;
-
     private PortletRequestDispatcher normalView;
 
     private PortletRequestDispatcher maximizedView;
@@ -99,64 +64,27 @@
             return;
         }
 
-        try {
-            Object o = null;
+        Map svrProps = new HashMap();
+        Map jvmProps = new HashMap();
 
-            // Kernel boot time
-            Date bootDate = kernel.getBootTime();
-            long bootTime = bootDate.getTime() / 1000;
-            long currentTime = System.currentTimeMillis() / 1000;
-            long elapsedTime = currentTime - bootTime;
-            svrProps.put("Kernel Boot Time", bootDate);
-            svrProps.put("Kernel Up Time", calcElapsedTime(elapsedTime));
+        JVM jvm = PortletManager.getCurrentJVM(renderRequest);
 
-            // Server info
-            /*
-             * ObjectName svrInfo = new
-             * ObjectName(ObjectNameConstants.SERVER_INFO_OBJECT_NAME); o =
-             * kernel.getAttribute(svrInfo, SVRINFO_BASEDIR); svrProps.put("Base
-             * Directory", o); ObjectName joeSvrInfo = new
-             * ObjectName(ObjectNameConstants.SE_SERVER_INFO_NAME); o =
-             * kernel.getAttribute(joeSvrInfo, SVRINFO_PLATFORMARCH);
-             * svrProps.put("Platform Architecture", o); o =
-             * kernel.getAttribute(joeSvrInfo, SVRINFO_VERSION);
-             * svrProps.put("Version", o); o = kernel.getAttribute(joeSvrInfo,
-             * SVRINFO_GERONIMO_BUILD_VERSION); svrProps.put("Apache Geronimo
-             * Build Version", o); o = kernel.getAttribute(joeSvrInfo,
-             * SVRINFO_GERONIMO_SPEC_VERSION); svrProps.put("J2EE Specifications
-             * Version", o); o = kernel.getAttribute(joeSvrInfo,
-             * SVRINFO_PORTAL_CORE_VERSION); svrProps.put("JSR 168 Portal
-             * Version", o); o = kernel.getAttribute(joeSvrInfo,
-             * SVRINFO_BUILDDATE); svrProps.put("Build Date", o); o =
-             * kernel.getAttribute(joeSvrInfo, SVRINFO_BUILDTIME);
-             * svrProps.put("Build Time", o); o =
-             * kernel.getAttribute(joeSvrInfo, SVRINFO_COPYRIGHT);
-             * svrProps.put("Copyright", o);
-             */
-            renderRequest.setAttribute("svrProps", svrProps);
+        Date bootDate = jvm.getKernelBootTime();
+        long bootTime = bootDate.getTime() / 1000;
+        long currentTime = System.currentTimeMillis() / 1000;
+        long elapsedTime = currentTime - bootTime;
+        svrProps.put("Kernel Boot Time", bootDate);
+        svrProps.put("Kernel Up Time", calcElapsedTime(elapsedTime));
+        renderRequest.setAttribute("svrProps", svrProps);
 
-            // JVM info
-            ObjectName jvmImpl = new ObjectName(
-                    ObjectNameConstants.JVM_IMPL_NAME);
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_JAVAVER);
-            jvmProps.put("Java Version", o);
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_JAVAVENDOR);
-            jvmProps.put("Java Vendor", o);
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_NODE);
-            jvmProps.put("Node", o);
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_MAXMEM);
-            jvmProps.put("Max Memory", calcMemory((Long) o));
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_TOTALMEM);
-            jvmProps.put("Total Memory", calcMemory((Long) o));
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_FREEMEM);
-            jvmProps.put("Free Memory", calcMemory((Long) o));
-            o = kernel.getAttribute(jvmImpl, JVMIMPL_AVAILABLEPROCS);
-            jvmProps.put("Available Processors", o);
-            renderRequest.setAttribute("jvmProps", jvmProps);
-        } catch (Exception e) {
-            e.printStackTrace();
-            //throw new PortletException(e);
-        }
+        jvmProps.put("Java Version", jvm.getJavaVersion());
+        jvmProps.put("Java Vendor", jvm.getJavaVendor());
+        jvmProps.put("Node", jvm.getNode());
+        jvmProps.put("Max Memory", calcMemory(jvm.getMaxMemory()));
+        jvmProps.put("Total Memory", calcMemory(jvm.getTotalMemory()));
+        jvmProps.put("Free Memory", calcMemory(jvm.getFreeMemory()));
+        jvmProps.put("Available Processors", new Integer(jvm.getAvailableProcessors()));
+        renderRequest.setAttribute("jvmProps", jvmProps);
 
         if (WindowState.NORMAL.equals(renderRequest.getWindowState())) {
             normalView.include(renderRequest, renderResponse);
@@ -172,7 +100,6 @@
 
     public void init(PortletConfig portletConfig) throws PortletException {
         super.init(portletConfig);
-        kernel = KernelRegistry.getSingleKernel();
         normalView = portletConfig.getPortletContext().getRequestDispatcher(
                 NORMALVIEW_JSP);
         maximizedView = portletConfig.getPortletContext().getRequestDispatcher(
@@ -182,22 +109,13 @@
     }
 
     public void destroy() {
-        kernel = null;
         normalView = null;
         maximizedView = null;
         helpView = null;
         super.destroy();
     }
 
-    public static Map getServerInfo() {
-        return svrProps;
-    }
-
-    public static Map getJVMInfo() {
-        return jvmProps;
-    }
-
-    private String calcElapsedTime(long timeInSeconds) {
+    private static String calcElapsedTime(long timeInSeconds) {
         long days, hrs, mins, secs;
         days = timeInSeconds / 86400;
         timeInSeconds = timeInSeconds - (days * 86400);
@@ -220,9 +138,8 @@
         return sb.toString();
     }
 
-    private String calcMemory(Long memory) {
+    private static String calcMemory(long mem) {
         long mb, kb;
-        long mem = memory.longValue();
         mb = mem / 1048576;
         // If less than 10MB return as KB
         if (mb < 10) {

Re: Web Console: Portlet Review

Posted by Aaron Mulder <am...@alumni.princeton.edu>.
On Sat, 23 Jul 2005, Dain Sundstrom wrote:
> For the initial kernel reference you can get it from the JNDI ENC.   
> Just add an env entry of type "org.apache.geronimo.kernel.Kernel"  
> with no value and the deployment code will automatically put the  
> kernel there.

The kernel trick sounds neat.  However, when I tried it, I got this:

    Error: Operation failed: Error parsing web.xml caused by Invalid
    deployment descriptor: [error: cvc-enumeration-valid: string value
    'org.apache.geronimo.kernel.Kernel' is not a valid enumeration value
    for env-entry-type-valuesType in namespace
    http://java.sun.com/xml/ns/j2ee]

for

    <env-entry>
        <env-entry-name>GeronimoKernel</env-entry-name>
        <env-entry-type>org.apache.geronimo.kernel.Kernel</env-entry-type>
    </env-entry>

Any ideas?

Thanks,
	Aaron

> 
>   -dain
> 
> On Jul 22, 2005, at 10:11 PM, Aaron Mulder wrote:
> 
> >     Well, I actually worked with a couple portlets today.  For
> > starters, I think the diff at the bottom of this message speaks  
> > volumes
> > about why a management API based on interfaces is nice.  It takes out
> > Strings, constants to hold the Strings, the Kernel variable, and  
> > various
> > casts, and instead stores things like the current Server and JVM in  
> > the
> > session and queries them to get the data it needs.  The code is  
> > smaller
> > and more reliable.  :)
> >
> >     As for the old code, I'm not thrilled with it even besides the
> > kernel invocations, for a couple reasons (and I just looked at the two
> > portlets on the front Server Info page so far):
> >
> >  - The portlets that need to use a Kernel keep one in a local  
> > variable,
> > and populate it by manually looking up a kernel during  
> > initialization.  I
> > would really have preferred to see common "get kernel" logic at  
> > least, if
> > not a common utility to hold the current kernel.
> >
> >  - The portlets assume they are running within the server in question,
> > doing things like calling System.getProperties to get the "server's"
> > system properties (and also using a local kernel, of course).  While I
> > have to agree that it works for now, I don't think it's any harder to
> > write this so it would work against a remote server, and it ends up  
> > being
> > cleaner anyway.
> >
> >  - The portlets store data in static variables, even though they  
> > are not
> > reused from request to request.  My J2EE application multithreading
> > sensors are going off, though I don't know for sure whether a portlet
> > instance can service more than one request at a time.  But I also  
> > don't
> > like using static variables for what is otherwise a totally stateless
> > class, and there's especially no reason to if they are reassigned  
> > at the
> > beginning of every request.
> >
> >  - Many of the kernel invocations returning objects are just popped  
> > into a
> > collection without casting, which is almost as bad as casting, because
> > then there's no assurance that if the JSP needed a particular type  
> > to be
> > there, the value would be of the right type.  Of course the JSPs I  
> > looked
> > just print objects and don't care about type.
> >
> >  - The JSP for printing system properties, for example, has  
> > separate lines
> > for every individual property, and manually sets the background  
> > style for
> > each, to achieve the alternating row color.  There are just so many  
> > better
> > ways to do this, that take advantage of, say, a loop, and don't  
> > involve
> > needing to manually restyle every row after inserting a new row,  
> > and so
> > on.
> >
> >     Anyway, I think the portlets are going to need a bit of attention.
> > That's OK I guess, since I'd like to migrate most of them to the new
> > management API anyway, so we'll have occasion to touch them, but I  
> > figured
> > I'd give my thoughts on where things stand.
> >
> > Thanks,
> >     Aaron
> >
> > Index: console-standard/src/java/org/apache/geronimo/console/ 
> > infomanager/ServerInfoPortlet.java
> > ===================================================================
> > --- console-standard/src/java/org/apache/geronimo/console/ 
> > infomanager/ServerInfoPortlet.java    (revision 224404)
> > +++ console-standard/src/java/org/apache/geronimo/console/ 
> > infomanager/ServerInfoPortlet.java    (working copy)
> > @@ -22,7 +22,6 @@
> >  import java.util.HashMap;
> >  import java.util.Map;
> >
> > -import javax.management.ObjectName;
> >  import javax.portlet.ActionRequest;
> >  import javax.portlet.ActionResponse;
> >  import javax.portlet.GenericPortlet;
> > @@ -33,56 +32,22 @@
> >  import javax.portlet.RenderResponse;
> >  import javax.portlet.WindowState;
> >
> > -import org.apache.geronimo.console.util.ObjectNameConstants;
> > -import org.apache.geronimo.kernel.Kernel;
> > -import org.apache.geronimo.kernel.KernelRegistry;
> > +import org.apache.geronimo.console.util.PortletManager;
> > +import org.apache.geronimo.j2ee.management.geronimo.JVM;
> >
> > +/**
> > + * Calculates various information about the server to display in  
> > the server
> > + * info portlet view (on of several JSPs depending on the portlet  
> > state).
> > + *
> > + * @version $Rev: 46019 $ $Date: 2004-09-14 05:56:06 -0400 (Tue,  
> > 14 Sep 2004) $
> > + */
> >  public class ServerInfoPortlet extends GenericPortlet {
> > -
> > -    public static final String SVRINFO_BASEDIR = "baseDirectory";
> > -
> > -    public static final String SVRINFO_VERSION = "version";
> > -
> > -    public static final String SVRINFO_BUILDDATE = "buildDate";
> > -
> > -    public static final String SVRINFO_BUILDTIME = "buildTime";
> > -
> > -    public static final String SVRINFO_COPYRIGHT = "copyright";
> > -
> > -    public static final String SVRINFO_PLATFORMARCH = "platformArch";
> > -
> > -    public static final String SVRINFO_GERONIMO_BUILD_VERSION =  
> > "geronimoBuildVersion";
> > -
> > -    public static final String SVRINFO_GERONIMO_SPEC_VERSION =  
> > "geronimoSpecVersion";
> > -
> > -    public static final String SVRINFO_PORTAL_CORE_VERSION =  
> > "portalCoreVersion";
> > -
> > -    public static final String JVMIMPL_JAVAVER = "javaVersion";
> > -
> > -    public static final String JVMIMPL_JAVAVENDOR = "javaVendor";
> > -
> > -    public static final String JVMIMPL_NODE = "node";
> > -
> > -    public static final String JVMIMPL_FREEMEM = "freeMemory";
> > -
> > -    public static final String JVMIMPL_MAXMEM = "maxMemory";
> > -
> > -    public static final String JVMIMPL_TOTALMEM = "totalMemory";
> > -
> > -    public static final String JVMIMPL_AVAILABLEPROCS =  
> > "availableProcessors";
> > -
> >      private static final String NORMALVIEW_JSP = "/WEB-INF/view/ 
> > infomanager/svrInfoNormal.jsp";
> >
> >      private static final String MAXIMIZEDVIEW_JSP = "/WEB-INF/view/ 
> > infomanager/svrInfoMaximized.jsp";
> >
> >      private static final String HELPVIEW_JSP = "/WEB-INF/view/ 
> > infomanager/svrInfoHelp.jsp";
> >
> > -    private static Map svrProps = new HashMap();
> > -
> > -    private static Map jvmProps = new HashMap();
> > -
> > -    private Kernel kernel;
> > -
> >      private PortletRequestDispatcher normalView;
> >
> >      private PortletRequestDispatcher maximizedView;
> > @@ -99,64 +64,27 @@
> >              return;
> >          }
> >
> > -        try {
> > -            Object o = null;
> > +        Map svrProps = new HashMap();
> > +        Map jvmProps = new HashMap();
> >
> > -            // Kernel boot time
> > -            Date bootDate = kernel.getBootTime();
> > -            long bootTime = bootDate.getTime() / 1000;
> > -            long currentTime = System.currentTimeMillis() / 1000;
> > -            long elapsedTime = currentTime - bootTime;
> > -            svrProps.put("Kernel Boot Time", bootDate);
> > -            svrProps.put("Kernel Up Time", calcElapsedTime 
> > (elapsedTime));
> > +        JVM jvm = PortletManager.getCurrentJVM(renderRequest);
> >
> > -            // Server info
> > -            /*
> > -             * ObjectName svrInfo = new
> > -             * ObjectName 
> > (ObjectNameConstants.SERVER_INFO_OBJECT_NAME); o =
> > -             * kernel.getAttribute(svrInfo, SVRINFO_BASEDIR);  
> > svrProps.put("Base
> > -             * Directory", o); ObjectName joeSvrInfo = new
> > -             * ObjectName 
> > (ObjectNameConstants.SE_SERVER_INFO_NAME); o =
> > -             * kernel.getAttribute(joeSvrInfo, SVRINFO_PLATFORMARCH);
> > -             * svrProps.put("Platform Architecture", o); o =
> > -             * kernel.getAttribute(joeSvrInfo, SVRINFO_VERSION);
> > -             * svrProps.put("Version", o); o = kernel.getAttribute 
> > (joeSvrInfo,
> > -             * SVRINFO_GERONIMO_BUILD_VERSION); svrProps.put 
> > ("Apache Geronimo
> > -             * Build Version", o); o = kernel.getAttribute 
> > (joeSvrInfo,
> > -             * SVRINFO_GERONIMO_SPEC_VERSION); svrProps.put("J2EE  
> > Specifications
> > -             * Version", o); o = kernel.getAttribute(joeSvrInfo,
> > -             * SVRINFO_PORTAL_CORE_VERSION); svrProps.put("JSR 168  
> > Portal
> > -             * Version", o); o = kernel.getAttribute(joeSvrInfo,
> > -             * SVRINFO_BUILDDATE); svrProps.put("Build Date", o); o =
> > -             * kernel.getAttribute(joeSvrInfo, SVRINFO_BUILDTIME);
> > -             * svrProps.put("Build Time", o); o =
> > -             * kernel.getAttribute(joeSvrInfo, SVRINFO_COPYRIGHT);
> > -             * svrProps.put("Copyright", o);
> > -             */
> > -            renderRequest.setAttribute("svrProps", svrProps);
> > +        Date bootDate = jvm.getKernelBootTime();
> > +        long bootTime = bootDate.getTime() / 1000;
> > +        long currentTime = System.currentTimeMillis() / 1000;
> > +        long elapsedTime = currentTime - bootTime;
> > +        svrProps.put("Kernel Boot Time", bootDate);
> > +        svrProps.put("Kernel Up Time", calcElapsedTime(elapsedTime));
> > +        renderRequest.setAttribute("svrProps", svrProps);
> >
> > -            // JVM info
> > -            ObjectName jvmImpl = new ObjectName(
> > -                    ObjectNameConstants.JVM_IMPL_NAME);
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_JAVAVER);
> > -            jvmProps.put("Java Version", o);
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_JAVAVENDOR);
> > -            jvmProps.put("Java Vendor", o);
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_NODE);
> > -            jvmProps.put("Node", o);
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_MAXMEM);
> > -            jvmProps.put("Max Memory", calcMemory((Long) o));
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_TOTALMEM);
> > -            jvmProps.put("Total Memory", calcMemory((Long) o));
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_FREEMEM);
> > -            jvmProps.put("Free Memory", calcMemory((Long) o));
> > -            o = kernel.getAttribute(jvmImpl, JVMIMPL_AVAILABLEPROCS);
> > -            jvmProps.put("Available Processors", o);
> > -            renderRequest.setAttribute("jvmProps", jvmProps);
> > -        } catch (Exception e) {
> > -            e.printStackTrace();
> > -            //throw new PortletException(e);
> > -        }
> > +        jvmProps.put("Java Version", jvm.getJavaVersion());
> > +        jvmProps.put("Java Vendor", jvm.getJavaVendor());
> > +        jvmProps.put("Node", jvm.getNode());
> > +        jvmProps.put("Max Memory", calcMemory(jvm.getMaxMemory()));
> > +        jvmProps.put("Total Memory", calcMemory(jvm.getTotalMemory 
> > ()));
> > +        jvmProps.put("Free Memory", calcMemory(jvm.getFreeMemory()));
> > +        jvmProps.put("Available Processors", new Integer 
> > (jvm.getAvailableProcessors()));
> > +        renderRequest.setAttribute("jvmProps", jvmProps);
> >
> >          if (WindowState.NORMAL.equals(renderRequest.getWindowState 
> > ())) {
> >              normalView.include(renderRequest, renderResponse);
> > @@ -172,7 +100,6 @@
> >
> >      public void init(PortletConfig portletConfig) throws  
> > PortletException {
> >          super.init(portletConfig);
> > -        kernel = KernelRegistry.getSingleKernel();
> >          normalView = portletConfig.getPortletContext 
> > ().getRequestDispatcher(
> >                  NORMALVIEW_JSP);
> >          maximizedView = portletConfig.getPortletContext 
> > ().getRequestDispatcher(
> > @@ -182,22 +109,13 @@
> >      }
> >
> >      public void destroy() {
> > -        kernel = null;
> >          normalView = null;
> >          maximizedView = null;
> >          helpView = null;
> >          super.destroy();
> >      }
> >
> > -    public static Map getServerInfo() {
> > -        return svrProps;
> > -    }
> > -
> > -    public static Map getJVMInfo() {
> > -        return jvmProps;
> > -    }
> > -
> > -    private String calcElapsedTime(long timeInSeconds) {
> > +    private static String calcElapsedTime(long timeInSeconds) {
> >          long days, hrs, mins, secs;
> >          days = timeInSeconds / 86400;
> >          timeInSeconds = timeInSeconds - (days * 86400);
> > @@ -220,9 +138,8 @@
> >          return sb.toString();
> >      }
> >
> > -    private String calcMemory(Long memory) {
> > +    private static String calcMemory(long mem) {
> >          long mb, kb;
> > -        long mem = memory.longValue();
> >          mb = mem / 1048576;
> >          // If less than 10MB return as KB
> >          if (mb < 10) {
> >
> 
> 

Re: Web Console: Portlet Review

Posted by Dain Sundstrom <da...@iq80.com>.
Cool stuff.

For the initial kernel reference you can get it from the JNDI ENC.   
Just add an env entry of type "org.apache.geronimo.kernel.Kernel"  
with no value and the deployment code will automatically put the  
kernel there.

  -dain

On Jul 22, 2005, at 10:11 PM, Aaron Mulder wrote:

>     Well, I actually worked with a couple portlets today.  For
> starters, I think the diff at the bottom of this message speaks  
> volumes
> about why a management API based on interfaces is nice.  It takes out
> Strings, constants to hold the Strings, the Kernel variable, and  
> various
> casts, and instead stores things like the current Server and JVM in  
> the
> session and queries them to get the data it needs.  The code is  
> smaller
> and more reliable.  :)
>
>     As for the old code, I'm not thrilled with it even besides the
> kernel invocations, for a couple reasons (and I just looked at the two
> portlets on the front Server Info page so far):
>
>  - The portlets that need to use a Kernel keep one in a local  
> variable,
> and populate it by manually looking up a kernel during  
> initialization.  I
> would really have preferred to see common "get kernel" logic at  
> least, if
> not a common utility to hold the current kernel.
>
>  - The portlets assume they are running within the server in question,
> doing things like calling System.getProperties to get the "server's"
> system properties (and also using a local kernel, of course).  While I
> have to agree that it works for now, I don't think it's any harder to
> write this so it would work against a remote server, and it ends up  
> being
> cleaner anyway.
>
>  - The portlets store data in static variables, even though they  
> are not
> reused from request to request.  My J2EE application multithreading
> sensors are going off, though I don't know for sure whether a portlet
> instance can service more than one request at a time.  But I also  
> don't
> like using static variables for what is otherwise a totally stateless
> class, and there's especially no reason to if they are reassigned  
> at the
> beginning of every request.
>
>  - Many of the kernel invocations returning objects are just popped  
> into a
> collection without casting, which is almost as bad as casting, because
> then there's no assurance that if the JSP needed a particular type  
> to be
> there, the value would be of the right type.  Of course the JSPs I  
> looked
> just print objects and don't care about type.
>
>  - The JSP for printing system properties, for example, has  
> separate lines
> for every individual property, and manually sets the background  
> style for
> each, to achieve the alternating row color.  There are just so many  
> better
> ways to do this, that take advantage of, say, a loop, and don't  
> involve
> needing to manually restyle every row after inserting a new row,  
> and so
> on.
>
>     Anyway, I think the portlets are going to need a bit of attention.
> That's OK I guess, since I'd like to migrate most of them to the new
> management API anyway, so we'll have occasion to touch them, but I  
> figured
> I'd give my thoughts on where things stand.
>
> Thanks,
>     Aaron
>
> Index: console-standard/src/java/org/apache/geronimo/console/ 
> infomanager/ServerInfoPortlet.java
> ===================================================================
> --- console-standard/src/java/org/apache/geronimo/console/ 
> infomanager/ServerInfoPortlet.java    (revision 224404)
> +++ console-standard/src/java/org/apache/geronimo/console/ 
> infomanager/ServerInfoPortlet.java    (working copy)
> @@ -22,7 +22,6 @@
>  import java.util.HashMap;
>  import java.util.Map;
>
> -import javax.management.ObjectName;
>  import javax.portlet.ActionRequest;
>  import javax.portlet.ActionResponse;
>  import javax.portlet.GenericPortlet;
> @@ -33,56 +32,22 @@
>  import javax.portlet.RenderResponse;
>  import javax.portlet.WindowState;
>
> -import org.apache.geronimo.console.util.ObjectNameConstants;
> -import org.apache.geronimo.kernel.Kernel;
> -import org.apache.geronimo.kernel.KernelRegistry;
> +import org.apache.geronimo.console.util.PortletManager;
> +import org.apache.geronimo.j2ee.management.geronimo.JVM;
>
> +/**
> + * Calculates various information about the server to display in  
> the server
> + * info portlet view (on of several JSPs depending on the portlet  
> state).
> + *
> + * @version $Rev: 46019 $ $Date: 2004-09-14 05:56:06 -0400 (Tue,  
> 14 Sep 2004) $
> + */
>  public class ServerInfoPortlet extends GenericPortlet {
> -
> -    public static final String SVRINFO_BASEDIR = "baseDirectory";
> -
> -    public static final String SVRINFO_VERSION = "version";
> -
> -    public static final String SVRINFO_BUILDDATE = "buildDate";
> -
> -    public static final String SVRINFO_BUILDTIME = "buildTime";
> -
> -    public static final String SVRINFO_COPYRIGHT = "copyright";
> -
> -    public static final String SVRINFO_PLATFORMARCH = "platformArch";
> -
> -    public static final String SVRINFO_GERONIMO_BUILD_VERSION =  
> "geronimoBuildVersion";
> -
> -    public static final String SVRINFO_GERONIMO_SPEC_VERSION =  
> "geronimoSpecVersion";
> -
> -    public static final String SVRINFO_PORTAL_CORE_VERSION =  
> "portalCoreVersion";
> -
> -    public static final String JVMIMPL_JAVAVER = "javaVersion";
> -
> -    public static final String JVMIMPL_JAVAVENDOR = "javaVendor";
> -
> -    public static final String JVMIMPL_NODE = "node";
> -
> -    public static final String JVMIMPL_FREEMEM = "freeMemory";
> -
> -    public static final String JVMIMPL_MAXMEM = "maxMemory";
> -
> -    public static final String JVMIMPL_TOTALMEM = "totalMemory";
> -
> -    public static final String JVMIMPL_AVAILABLEPROCS =  
> "availableProcessors";
> -
>      private static final String NORMALVIEW_JSP = "/WEB-INF/view/ 
> infomanager/svrInfoNormal.jsp";
>
>      private static final String MAXIMIZEDVIEW_JSP = "/WEB-INF/view/ 
> infomanager/svrInfoMaximized.jsp";
>
>      private static final String HELPVIEW_JSP = "/WEB-INF/view/ 
> infomanager/svrInfoHelp.jsp";
>
> -    private static Map svrProps = new HashMap();
> -
> -    private static Map jvmProps = new HashMap();
> -
> -    private Kernel kernel;
> -
>      private PortletRequestDispatcher normalView;
>
>      private PortletRequestDispatcher maximizedView;
> @@ -99,64 +64,27 @@
>              return;
>          }
>
> -        try {
> -            Object o = null;
> +        Map svrProps = new HashMap();
> +        Map jvmProps = new HashMap();
>
> -            // Kernel boot time
> -            Date bootDate = kernel.getBootTime();
> -            long bootTime = bootDate.getTime() / 1000;
> -            long currentTime = System.currentTimeMillis() / 1000;
> -            long elapsedTime = currentTime - bootTime;
> -            svrProps.put("Kernel Boot Time", bootDate);
> -            svrProps.put("Kernel Up Time", calcElapsedTime 
> (elapsedTime));
> +        JVM jvm = PortletManager.getCurrentJVM(renderRequest);
>
> -            // Server info
> -            /*
> -             * ObjectName svrInfo = new
> -             * ObjectName 
> (ObjectNameConstants.SERVER_INFO_OBJECT_NAME); o =
> -             * kernel.getAttribute(svrInfo, SVRINFO_BASEDIR);  
> svrProps.put("Base
> -             * Directory", o); ObjectName joeSvrInfo = new
> -             * ObjectName 
> (ObjectNameConstants.SE_SERVER_INFO_NAME); o =
> -             * kernel.getAttribute(joeSvrInfo, SVRINFO_PLATFORMARCH);
> -             * svrProps.put("Platform Architecture", o); o =
> -             * kernel.getAttribute(joeSvrInfo, SVRINFO_VERSION);
> -             * svrProps.put("Version", o); o = kernel.getAttribute 
> (joeSvrInfo,
> -             * SVRINFO_GERONIMO_BUILD_VERSION); svrProps.put 
> ("Apache Geronimo
> -             * Build Version", o); o = kernel.getAttribute 
> (joeSvrInfo,
> -             * SVRINFO_GERONIMO_SPEC_VERSION); svrProps.put("J2EE  
> Specifications
> -             * Version", o); o = kernel.getAttribute(joeSvrInfo,
> -             * SVRINFO_PORTAL_CORE_VERSION); svrProps.put("JSR 168  
> Portal
> -             * Version", o); o = kernel.getAttribute(joeSvrInfo,
> -             * SVRINFO_BUILDDATE); svrProps.put("Build Date", o); o =
> -             * kernel.getAttribute(joeSvrInfo, SVRINFO_BUILDTIME);
> -             * svrProps.put("Build Time", o); o =
> -             * kernel.getAttribute(joeSvrInfo, SVRINFO_COPYRIGHT);
> -             * svrProps.put("Copyright", o);
> -             */
> -            renderRequest.setAttribute("svrProps", svrProps);
> +        Date bootDate = jvm.getKernelBootTime();
> +        long bootTime = bootDate.getTime() / 1000;
> +        long currentTime = System.currentTimeMillis() / 1000;
> +        long elapsedTime = currentTime - bootTime;
> +        svrProps.put("Kernel Boot Time", bootDate);
> +        svrProps.put("Kernel Up Time", calcElapsedTime(elapsedTime));
> +        renderRequest.setAttribute("svrProps", svrProps);
>
> -            // JVM info
> -            ObjectName jvmImpl = new ObjectName(
> -                    ObjectNameConstants.JVM_IMPL_NAME);
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_JAVAVER);
> -            jvmProps.put("Java Version", o);
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_JAVAVENDOR);
> -            jvmProps.put("Java Vendor", o);
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_NODE);
> -            jvmProps.put("Node", o);
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_MAXMEM);
> -            jvmProps.put("Max Memory", calcMemory((Long) o));
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_TOTALMEM);
> -            jvmProps.put("Total Memory", calcMemory((Long) o));
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_FREEMEM);
> -            jvmProps.put("Free Memory", calcMemory((Long) o));
> -            o = kernel.getAttribute(jvmImpl, JVMIMPL_AVAILABLEPROCS);
> -            jvmProps.put("Available Processors", o);
> -            renderRequest.setAttribute("jvmProps", jvmProps);
> -        } catch (Exception e) {
> -            e.printStackTrace();
> -            //throw new PortletException(e);
> -        }
> +        jvmProps.put("Java Version", jvm.getJavaVersion());
> +        jvmProps.put("Java Vendor", jvm.getJavaVendor());
> +        jvmProps.put("Node", jvm.getNode());
> +        jvmProps.put("Max Memory", calcMemory(jvm.getMaxMemory()));
> +        jvmProps.put("Total Memory", calcMemory(jvm.getTotalMemory 
> ()));
> +        jvmProps.put("Free Memory", calcMemory(jvm.getFreeMemory()));
> +        jvmProps.put("Available Processors", new Integer 
> (jvm.getAvailableProcessors()));
> +        renderRequest.setAttribute("jvmProps", jvmProps);
>
>          if (WindowState.NORMAL.equals(renderRequest.getWindowState 
> ())) {
>              normalView.include(renderRequest, renderResponse);
> @@ -172,7 +100,6 @@
>
>      public void init(PortletConfig portletConfig) throws  
> PortletException {
>          super.init(portletConfig);
> -        kernel = KernelRegistry.getSingleKernel();
>          normalView = portletConfig.getPortletContext 
> ().getRequestDispatcher(
>                  NORMALVIEW_JSP);
>          maximizedView = portletConfig.getPortletContext 
> ().getRequestDispatcher(
> @@ -182,22 +109,13 @@
>      }
>
>      public void destroy() {
> -        kernel = null;
>          normalView = null;
>          maximizedView = null;
>          helpView = null;
>          super.destroy();
>      }
>
> -    public static Map getServerInfo() {
> -        return svrProps;
> -    }
> -
> -    public static Map getJVMInfo() {
> -        return jvmProps;
> -    }
> -
> -    private String calcElapsedTime(long timeInSeconds) {
> +    private static String calcElapsedTime(long timeInSeconds) {
>          long days, hrs, mins, secs;
>          days = timeInSeconds / 86400;
>          timeInSeconds = timeInSeconds - (days * 86400);
> @@ -220,9 +138,8 @@
>          return sb.toString();
>      }
>
> -    private String calcMemory(Long memory) {
> +    private static String calcMemory(long mem) {
>          long mb, kb;
> -        long mem = memory.longValue();
>          mb = mem / 1048576;
>          // If less than 10MB return as KB
>          if (mb < 10) {
>