[jboss-cvs] JBossAS SVN: r64536 - in projects/microcontainer/trunk: kernel/src/main/org/jboss/kernel/plugins/dependency and 1 other directory.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Fri Aug 10 08:20:13 EDT 2007


Author: alesj
Date: 2007-08-10 08:20:13 -0400 (Fri, 10 Aug 2007)
New Revision: 64536

Added:
   projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/ScopedController.java
Modified:
   projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/AbstractController.java
   projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/AbstractKernelController.java
   projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/PreInstallAction.java
   projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/ScopedKernelController.java
Log:
Refactored ScopedKernelController + super controllers - removing public modifiers on the add/remove controller context methods.
Added write/read locks where missing.
Squeezed in ScopedController to provide appropriate modifiers for ScopedKernelController.

Modified: projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/AbstractController.java
===================================================================
--- projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/AbstractController.java	2007-08-10 08:53:47 UTC (rev 64535)
+++ projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/AbstractController.java	2007-08-10 12:20:13 UTC (rev 64536)
@@ -28,6 +28,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.HashMap;
+import java.util.Collections;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CopyOnWriteArraySet;
@@ -136,7 +137,6 @@
       }
    }
 
-   // TODO need tests for shutdown
    public void shutdown()
    {
       lockWrite();
@@ -211,20 +211,30 @@
       }
    }
 
-   // TODO This api looks broken and unsafe
-   //      1) It should not be public
-   //      2) There should be parameter checking for public methods
-   //      3) There should be locking when updating state
-   //      4) Error handling?
-   public void addControllerContext(ControllerContext context)
+   void addControllerContext(ControllerContext context)
    {
-      registerControllerContext(context);
+      lockWrite();
+      try
+      {
+         registerControllerContext(context);
+      }
+      finally
+      {
+         unlockWrite();
+      }
    }
 
-   // TODO This api looks broken and unsafe see above
-   public void removeControllerContext(ControllerContext context)
+   void removeControllerContext(ControllerContext context)
    {
-      unregisterControllerContext(context);
+      lockWrite();
+      try
+      {
+         unregisterControllerContext(context);
+      }
+      finally
+      {
+         unlockWrite();
+      }
    }
 
    protected AbstractController getParentController()
@@ -239,19 +249,41 @@
 
    public Set<AbstractController> getControllers()
    {
-      return childControllers;
+      lockRead();
+      try
+      {
+         return Collections.unmodifiableSet(childControllers);
+      }
+      finally
+      {
+         unlockRead();
+      }
    }
 
-   // no need for locking - we are already locked
-
    public boolean addController(AbstractController controller)
    {
-      return childControllers.add(controller);
+      lockWrite();
+      try
+      {
+         return childControllers.add(controller);
+      }
+      finally
+      {
+         unlockWrite();
+      }
    }
 
    public boolean removeController(AbstractController controller)
    {
-      return childControllers.remove(controller);
+      lockWrite();
+      try
+      {
+         return childControllers.remove(controller);
+      }
+      finally
+      {
+         unlockWrite();
+      }
    }
 
    /**
@@ -452,7 +484,6 @@
                }
                parent = parent.getParentController();
             }
-
          }
          else
          {

Added: projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/ScopedController.java
===================================================================
--- projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/ScopedController.java	                        (rev 0)
+++ projects/microcontainer/trunk/dependency/src/main/org/jboss/dependency/plugins/ScopedController.java	2007-08-10 12:20:13 UTC (rev 64536)
@@ -0,0 +1,93 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2006, JBoss Inc., and individual contributors as indicated
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* This is free software; you can redistribute it and/or modify it
+* under the terms of the GNU Lesser General Public License as
+* published by the Free Software Foundation; either version 2.1 of
+* the License, or (at your option) any later version.
+*
+* This software is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this software; if not, write to the Free
+* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+*/
+package org.jboss.dependency.plugins;
+
+import org.jboss.dependency.spi.ControllerContext;
+
+/**
+ * Scoped controller.
+ * Not other scoping logic except add/remove controller context.
+ * Subclasses should provide parent lookup after looking
+ * at the current scoped instance.
+ *
+ * @author <a href="ales.justin at jboss.com">Ales Justin</a>
+ */
+public abstract class ScopedController extends AbstractController
+{
+   private AbstractController underlyingController;
+
+   protected void setUnderlyingController(AbstractController underlyingController)
+   {
+      this.underlyingController = underlyingController;
+   }
+
+   /**
+    * Is controller scoped.
+    *
+    * @return true if scoped
+    */
+   protected boolean isScoped()
+   {
+      return underlyingController != null;
+   }
+
+   protected void addControllerContext(ControllerContext context)
+   {
+      if (isScoped())
+      {
+         lockWrite();
+         try
+         {
+            underlyingController.removeControllerContext(context);
+            context.setController(this);
+            registerControllerContext(context);
+         }
+         finally
+         {
+            unlockWrite();
+         }
+      }
+      else
+         super.addControllerContext(context);
+   }
+
+   protected void removeControllerContext(ControllerContext context)
+   {
+      if (isScoped())
+      {
+         lockWrite();
+         try
+         {
+            unregisterControllerContext(context);
+            context.setController(underlyingController);
+            underlyingController.addControllerContext(context);
+         }
+         finally
+         {
+            unlockWrite();
+         }
+      }
+      else
+         super.removeControllerContext(context);
+   }
+
+}

Modified: projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/AbstractKernelController.java
===================================================================
--- projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/AbstractKernelController.java	2007-08-10 08:53:47 UTC (rev 64535)
+++ projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/AbstractKernelController.java	2007-08-10 12:20:13 UTC (rev 64536)
@@ -31,7 +31,7 @@
 
 import org.jboss.beans.metadata.spi.BeanMetaData;
 import org.jboss.beans.metadata.spi.SupplyMetaData;
-import org.jboss.dependency.plugins.AbstractController;
+import org.jboss.dependency.plugins.ScopedController;
 import org.jboss.dependency.spi.ControllerContext;
 import org.jboss.dependency.spi.ControllerState;
 import org.jboss.kernel.Kernel;
@@ -51,7 +51,7 @@
  * @author <a href="adrian at jboss.com">Adrian Brock</a>
  * @version $Revision$
  */
-public class AbstractKernelController extends AbstractController implements KernelController, KernelRegistryPlugin
+public class AbstractKernelController extends ScopedController implements KernelController, KernelRegistryPlugin
 {
    /** The kernel */
    protected Kernel kernel;

Modified: projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/PreInstallAction.java
===================================================================
--- projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/PreInstallAction.java	2007-08-10 08:53:47 UTC (rev 64535)
+++ projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/PreInstallAction.java	2007-08-10 12:20:13 UTC (rev 64536)
@@ -170,7 +170,6 @@
             ((MutableMetaData)mdr).addMetaData(scopedController, ScopedKernelController.class);
          }
          scopedController.addControllerContext(context);
-         context.setController(scopedController);
       }
    }
 
@@ -195,7 +194,6 @@
          }
          ScopedKernelController scopedController = controllerItem.getValue();
          scopedController.removeControllerContext(context);
-         context.setController(scopedController.getUnderlyingController());
          if (scopedController.isActive() == false)
          {
             try

Modified: projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/ScopedKernelController.java
===================================================================
--- projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/ScopedKernelController.java	2007-08-10 08:53:47 UTC (rev 64535)
+++ projects/microcontainer/trunk/kernel/src/main/org/jboss/kernel/plugins/dependency/ScopedKernelController.java	2007-08-10 12:20:13 UTC (rev 64536)
@@ -52,7 +52,6 @@
 public class ScopedKernelController extends AbstractKernelController
 {
    protected Kernel parentKernel;
-   protected AbstractController underlyingController;
 
    public ScopedKernelController(Kernel parentKernel, AbstractController parentController) throws Exception
    {
@@ -60,7 +59,7 @@
       this.parentKernel = parentKernel;
       if (parentKernel.getController() instanceof AbstractController == false)
          throw new IllegalArgumentException("Underlying controller not AbstractController instance!");
-      this.underlyingController = (AbstractController)parentKernel.getController();
+      setUnderlyingController((AbstractController)parentKernel.getController());
       setParentController(parentController);
       KernelConfig config = new ScopedKernelConfig(System.getProperties());
       kernel = KernelFactory.newInstance(config);
@@ -81,29 +80,20 @@
 
    // Scoped helper methods 
 
-   public AbstractController getUnderlyingController()
+   void addControllerContext(KernelControllerContext context)
    {
-      return underlyingController;
+      super.addControllerContext(context);
    }
 
-   // TODO See comments on super implementation
-   public void addControllerContext(ControllerContext context)
+   void removeControllerContext(KernelControllerContext context)
    {
-      underlyingController.removeControllerContext(context);
-      registerControllerContext(context);
+      super.removeControllerContext(context);
    }
 
-   // TODO See comments on super implementation
-   public void removeControllerContext(ControllerContext context)
+   void release()
    {
-      unregisterControllerContext(context);
-      underlyingController.addControllerContext(context);
-   }
-
-   public void release()
-   {
       getParentController().removeController(this);
-      underlyingController = null;
+      setUnderlyingController(null);
       setParentController(null);
       parentKernel = null;
    }




More information about the jboss-cvs-commits mailing list