geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dain Sundstrom <d...@iq80.com>
Subject Re: Web Console: Portlet Review
Date Sat, 23 Jul 2005 15:40:26 GMT
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) {
>


Mime
View raw message