You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by gr...@apache.org on 2003/08/13 04:12:40 UTC

cvs commit: incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/context TransactionInterceptor.java

gregw       2003/08/12 19:12:40

  Modified:    modules/core/src/java/org/apache/geronimo/common
                        AbstractComponent.java AbstractInterceptor.java
                        Component.java State.java
               modules/core/src/java/org/apache/geronimo/ejb
                        EJBProxyFactoryManager.java
               modules/core/src/java/org/apache/geronimo/ejb/cache
                        EnterpriseContextInstancePool.java
               modules/core/src/java/org/apache/geronimo/ejb/container
                        ContainerImpl.java
               modules/core/src/java/org/apache/geronimo/ejb/context
                        TransactionInterceptor.java
  Log:
  Updated the Component class to support the JSR77 state model.
  Implemented the base behaviour in AbstractComponent
  Implemented recursive behaviour in ContainerImpl - but should be moved to
  a AbstractContainer class.
  Moved create nehaviour to doStart methods
  Left destroy methods hanging with todo comments to resolve that issue.
  
  Submitted by:	Greg Wilkins
  Reviewed by:	Posted to geronimo-dev and coredevelopers.net mailing lists
  
  Revision  Changes    Path
  1.3       +175 -46   incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/AbstractComponent.java
  
  Index: AbstractComponent.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/AbstractComponent.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- AbstractComponent.java	11 Aug 2003 17:59:10 -0000	1.2
  +++ AbstractComponent.java	13 Aug 2003 02:12:40 -0000	1.3
  @@ -55,7 +55,6 @@
    */
   package org.apache.geronimo.common;
   
  -
   import org.apache.commons.logging.Log;
   import org.apache.commons.logging.LogFactory;
   
  @@ -65,59 +64,189 @@
    *
    * @version $Revision$ $Date$
    */
  -public class AbstractComponent implements Component {
  -    private State state = State.NOT_CREATED;
  +public class AbstractComponent implements Component
  +{
  +    private State state= State.STOPPED;
  +    private long startTime;
       private Container container;
  -    protected Log log = LogFactory.getLog(getClass());
  +    protected Log log= LogFactory.getLog(getClass());
   
  -    public State getState() {
  +    public State getState()
  +    {
           return state;
       }
   
  -    public final Container getContainer() {
  -        return container;
  -    }
  -
  -    public final void setContainer(Container container) {
  -        if (state != State.NOT_CREATED && state != State.DESTROYED) {
  -            throw new IllegalStateException("Set container can only be called while in the not-created or destroyed states: state=" + state);
  -        }
  -        if (state == State.NOT_CREATED && container == null) {
  -            throw new IllegalArgumentException("Interceptor has not been created; container must be NOT null.");
  -        }
  -        if (state == State.DESTROYED && container != null) {
  -            throw new IllegalArgumentException("Interceptor has been destroyed; container must be null.");
  -        }
  -        this.container = container;
  +    /**
  +     * Set the Component state.
  +     * @param newState
  +     * @throws IllegalStateException Thrown if the transition is not supported by the JSR77 lifecycle.
  +     */
  +    protected void setState(State newState) throws IllegalStateException
  +    {
  +        switch (state.getIndex())
  +        {
  +            case State.STOPPED_INDEX :
  +                {
  +                    switch (state.getIndex())
  +                    {
  +                        case State.STARTING_INDEX :
  +                            break;
  +                        case State.STOPPED_INDEX :
  +                        case State.RUNNING_INDEX :
  +                        case State.STOPPING_INDEX :
  +                        case State.FAILED_INDEX :
  +                            throw new IllegalStateException(
  +                                "Can not transition to " + newState + " state from " + state);
  +                    }
  +                    break;
  +                }
  +
  +            case State.STARTING_INDEX :
  +                {
  +                    switch (state.getIndex())
  +                    {
  +                        case State.RUNNING_INDEX :
  +                        case State.FAILED_INDEX :
  +                        case State.STOPPING_INDEX :
  +                            break;
  +                        case State.STOPPED_INDEX :
  +                        case State.STARTING_INDEX :
  +                            throw new IllegalStateException(
  +                                "Can not transition to " + newState + " state from " + state);
  +                    }
  +                    break;
  +                }
  +
  +            case State.RUNNING_INDEX :
  +                {
  +                    switch (state.getIndex())
  +                    {
  +                        case State.STOPPING_INDEX :
  +                        case State.FAILED_INDEX :
  +                            break;
  +                        case State.STOPPED_INDEX :
  +                        case State.STARTING_INDEX :
  +                        case State.RUNNING_INDEX :
  +                            throw new IllegalStateException(
  +                                "Can not transition to " + newState + " state from " + state);
  +
  +                    }
  +                    break;
  +                }
  +
  +            case State.STOPPING_INDEX :
  +                {
  +                    switch (state.getIndex())
  +                    {
  +                        case State.STOPPED_INDEX :
  +                        case State.FAILED_INDEX :
  +                            break;
  +                        case State.STARTING_INDEX :
  +                        case State.RUNNING_INDEX :
  +                        case State.STOPPING_INDEX :
  +                            throw new IllegalStateException(
  +                                "Can not transition to " + newState + " state from " + state);
  +                    }
  +                    break;
  +                }
  +
  +            case State.FAILED_INDEX :
  +                {
  +                    switch (state.getIndex())
  +                    {
  +                        case State.STARTING_INDEX :
  +                        case State.STOPPING_INDEX :
  +                            break;
  +                        case State.STOPPED_INDEX :
  +                        case State.RUNNING_INDEX :
  +                        case State.FAILED_INDEX :
  +                            throw new IllegalStateException(
  +                                "Can not transition to " + newState + " state from " + state);
  +                    }
  +                    break;
  +                }
  +        }
  +        log.debug("State changed from " + state + " to " + newState);
  +        if (newState==State.RUNNING)
  +            startTime= System.currentTimeMillis();
  +        state= newState;
  +
  +    }
  +
  +    public long getStartTime()
  +    {
  +        return startTime;
       }
   
  -    public void create() throws Exception {
  -        if (state != State.NOT_CREATED) {
  -            throw new IllegalStateException("Can not transition to created state from " + state);
  -        }
  -        state = State.STOPPED;
  +    public final Container getContainer()
  +    {
  +        return container;
       }
   
  -    public void start() throws Exception {
  -        if (state != State.STOPPED) {
  -            throw new IllegalStateException("Can not transition to started state from " + state);
  -        }
  -        state = State.STARTED;
  +    public final void setContainer(Container container)
  +    {
  +        if (state != State.STOPPED)
  +        {
  +            throw new IllegalStateException(
  +                "Set container can only be called while in the stopped state: state=" + state);
  +        }
  +        this.container= container;
  +    }
  +
  +    public void start() throws Exception
  +    {
  +        try
  +        {
  +            setState(State.STARTING);
  +            doStart();
  +            setState(State.RUNNING);
  +        }
  +        finally
  +        {
  +            if (state != State.RUNNING)
  +                setState(State.FAILED);
  +        }
  +    }
  +
  +    public void startRecursive() throws Exception
  +    {
  +        start();
  +    }
  +
  +    /**
  +     * Do the start tasks for the component.  Called in the STARTING state by 
  +     * the start() and startRecursive() methods to perform the tasks required to 
  +     * start the component. The default implementation does nothing.
  +     * @throws Exception
  +     */
  +    public void doStart() throws Exception
  +    {
  +    }
  +
  +    public void stop()
  +    {
  +        // Do the actual stop tasks
  +        try
  +        {
  +            setState(State.STOPPING);
  +            doStop();
  +            setState(State.STOPPED);
  +        }
  +        catch (Exception e)
  +        {
  +            log.warn("Stop failed", e);
  +            setState(State.FAILED);
  +        }
  +    }
  +
  +    /**
  +     * Do the stop tasks for the component.  Called in the STOPPING state by the stop()
  +     * method to perform the tasks required to stop the component.
  +     * This implementation does nothing.
  +     * @throws Exception
  +     */
  +    public void doStop() throws Exception
  +    {
       }
   
  -    public void stop() {
  -        if (state == State.NOT_CREATED || state == State.DESTROYED) {
  -            throw new IllegalStateException("Can not transition to started state from " + state);
  -        } else if (state == State.STOPPED) {
  -            log.warn("Stop called on an already stopped component; no exception will be thrown but this is a programming error.");
  -        }
  -        state = State.STOPPED;
  -    }
  -
  -    public void destroy() {
  -        if (state != State.STOPPED) {
  -            log.warn("Destroy called on an component in the " + state + " state; no exception will be thrown but this is a programming error.");
  -        }
  -        state = State.DESTROYED;
  -    }
   }
  
  
  
  1.3       +3 -9      incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/AbstractInterceptor.java
  
  Index: AbstractInterceptor.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/AbstractInterceptor.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- AbstractInterceptor.java	11 Aug 2003 17:59:10 -0000	1.2
  +++ AbstractInterceptor.java	13 Aug 2003 02:12:40 -0000	1.3
  @@ -70,14 +70,8 @@
   
       public final void setNext(Interceptor nextInterceptor) {
           State state = getState();
  -        if (state != State.NOT_CREATED && state != State.DESTROYED) {
  -            throw new IllegalStateException("setNext can only be called while in the not-created or destroyed states: state=" + state);
  -        }
  -        if (state == State.NOT_CREATED && nextInterceptor == null) {
  -            throw new IllegalArgumentException("Interceptor has not been created; nextInterceptor must be NOT null.");
  -        }
  -        if (state == State.DESTROYED && nextInterceptor != null) {
  -            throw new IllegalArgumentException("Interceptor has been destroyed; nextInterceptor must be null.");
  +        if (state != State.STOPPED) {
  +            throw new IllegalStateException("setNext can only be called while in the stopped state: state=" + state);
           }
           next = nextInterceptor;
       }
  
  
  
  1.3       +32 -30    incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/Component.java
  
  Index: Component.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/Component.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- Component.java	11 Aug 2003 17:59:10 -0000	1.2
  +++ Component.java	13 Aug 2003 02:12:40 -0000	1.3
  @@ -57,7 +57,7 @@
   
   
   /**
  - *
  + * Implements the JSR 77 state model
    *
    *
    * @version $Revision$ $Date$
  @@ -69,6 +69,12 @@
        */
       State getState();
   
  +	/**
  +	 * Gets the start time of this component
  +	 * @return time in milliseonds since epoch that this component was started.
  +	 */
  +	long getStartTime();
  +	
       /**
        * Gets the container to which this component belongs.
        * @return the container for which invocations will be intercepted
  @@ -87,20 +93,7 @@
       void setContainer(Container container) throws IllegalStateException, IllegalArgumentException;
   
       /**
  -     * Transitions the component to the stopped state.  This method has access to the
  -     * container. Once an component has been created setContainer() will throw IllegalStateException.
  -     *
  -     * Normally a component uses this to acquire references to other componets of the container. The
  -     * components will all have been registered at this stage, but not necessarily
  -     * created so methods should not be invoked on the reference until the start method.
  -     *
  -     * @throws java.lang.Exception if a problem occurs during the transition
  -     * @throws java.lang.IllegalStateException if this interceptor is not in the not-created state
  -     */
  -    void create() throws Exception, IllegalStateException;
  -
  -    /**
  -     * Transitions the component to the started state.  This method has access to the
  +     * Transitions the component to the starting state.  This method has access to the
        * container.
        *
        * Normally a component uses this to cache data from other components. The other components will
  @@ -108,29 +101,38 @@
        * invoked on them.
        *
        * @throws java.lang.Exception if a problem occurs during the transition
  -     * @throws java.lang.IllegalStateException if this interceptor is not in the stopped state
  +     * @throws java.lang.IllegalStateException if this interceptor is not in the stopped or failed state
        */
       void start() throws Exception, IllegalStateException;
   
  +	/**
  +	 * Transitions the component to the starting state.  This method has access to the
  +	 * container.
  +	 * 
  +	 * If this Component is a Container, then startRecursive is called on all child Components
  +	 * that are in the STOPPED or FAILED state.
  +	 * Normally a component uses this to cache data from other components. The other components will
  +	 * have been created at this stage, but not necessairly started and may not be ready to have methods
  +	 * invoked on them.
  +	 *
  +	 * @throws java.lang.Exception if a problem occurs during the transition
  +	 * @throws java.lang.IllegalStateException if this interceptor is not in the STOPPED or FAILED state
  +	 */
  +	void startRecursive() throws Exception, IllegalStateException;
  +
       /**
  -     * Transitions the component to the stopped state.  This method has access to the
  +     * Transitions the component to the stopping state.  This method has access to the
        * container.
        *
  +	 * If this is Component is a Container, then all its child components must be in the 
  +	 * STOPPED or FAILED State.
  +	 * 
        * Normally a component uses this to drop references to data cached in the start method.
        * The other components will not necessairly have been stopped at this stage and may not be ready
        * to have methods invoked on them.
        */
  -    void stop();
  +    void stop() throws IllegalStateException;
  +    
  +
   
  -    /**
  -     * Transitions the component to the destroyed state.  This method has access to the
  -     * container.  Once a component has been destroyed, it can not be recreated with the i
  -     * create method.  If the create method is called on an destroyed component an
  -     * IllegalStateException will be thrown.
  -     *
  -     * Normally a component uses this to drop references to components cached in the create method.
  -     * The other components will not necessairly have been destroyed at this stage, but may have been
  -     * stopped somethods should not be called on other components.
  -     */
  -    void destroy();
   }
  
  
  
  1.3       +22 -8     incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/State.java
  
  Index: State.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/common/State.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- State.java	11 Aug 2003 17:59:10 -0000	1.2
  +++ State.java	13 Aug 2003 02:12:40 -0000	1.3
  @@ -62,17 +62,31 @@
    * @version $Revision$ $Date$
    */
   public final class State {
  -    public static final State NOT_CREATED = new State("not-created");
  -    public static final State STOPPED = new State("stopped");
  -    public static final State STARTED = new State("started");
  -    public static final State DESTROYED = new State("destroyed");
  -
  +	
  +	public static final int STARTING_INDEX=0;
  +	public static final int RUNNING_INDEX=1;
  +	public static final int STOPPING_INDEX=2;
  +	public static final int STOPPED_INDEX=3;
  +	public static final int FAILED_INDEX=4;
  +	
  +    public static final State STARTING = new State("starting",STARTING_INDEX);
  +    public static final State RUNNING = new State("running",RUNNING_INDEX);
  +	public static final State STOPPING = new State("stopping",STOPPING_INDEX);
  +	public static final State STOPPED = new State("stopped",STOPPED_INDEX);
  +	public static final State FAILED = new State("failed",FAILED_INDEX);
  +	
       private final String name;
  -
  -    private State(String name) {
  +	private final int index;
  +	
  +    private State(String name, int index) {
           this.name = name;
  +        this.index = index;
       }
   
  +	public int getIndex() {	
  +		return index;
  +	}
  +	
       public String toString() {
           return name;
       }
  
  
  
  1.4       +11 -27    incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/EJBProxyFactoryManager.java
  
  Index: EJBProxyFactoryManager.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/EJBProxyFactoryManager.java,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -r1.3 -r1.4
  --- EJBProxyFactoryManager.java	11 Aug 2003 17:59:11 -0000	1.3
  +++ EJBProxyFactoryManager.java	13 Aug 2003 02:12:40 -0000	1.4
  @@ -73,22 +73,11 @@
       private Map proxies = new HashMap();
       private ThreadLocal threadEJBProxyFactory = new ThreadLocal();
   
  -    public void create() throws Exception {
  -        log.debug("Creating EJBProxyFactoryManager: ejbName=" + EJBPlugins.getEJBMetadata(getContainer()).getName());
  -        super.create();
  -        for (Iterator iterator = proxies.keySet().iterator(); iterator.hasNext();) {
  -            String proxyName = (String) iterator.next();
  -            EJBProxyFactory ejbProxyFactory = getEJBProxyFactory(proxyName);
  -            // @todo uncomment this when the ejb proxy factories are rewriten to be full components
  -            //ejbProxyFactory.setContainer(getContainer());
  -            log.debug("Creating EJBProxyFactory: proxyName=" + proxyName + " ejbName=" + EJBPlugins.getEJBMetadata(getContainer()).getName());
  -            ejbProxyFactory.create();
  -        }
  -    }
   
  -    public void start() throws Exception {
  +    public void doStart() throws Exception {
           log.debug("Starting EJBProxyFactoryManager: ejbName=" + EJBPlugins.getEJBMetadata(getContainer()).getName());
  -        super.start();
  +
  +        // @todo This should be handled by startRecursive()
           for (Iterator iterator = proxies.keySet().iterator(); iterator.hasNext();) {
               String proxyName = (String) iterator.next();
               EJBProxyFactory ejbProxyFactory = getEJBProxyFactory(proxyName);
  @@ -97,9 +86,10 @@
           }
       }
   
  -    public void stop() {
  +    public void doStop() {
           log.debug("Stopping EJBProxyFactoryManager: ejbName=" + EJBPlugins.getEJBMetadata(getContainer()).getName());
  -        super.stop();
  + 
  +        // @todo This will not work when this is a container, as children must be stopped before stop() is called.
           for (Iterator iterator = proxies.keySet().iterator(); iterator.hasNext();) {
               String proxyName = (String) iterator.next();
               EJBProxyFactory ejbProxyFactory = getEJBProxyFactory(proxyName);
  @@ -108,17 +98,11 @@
           }
       }
   
  +
  +    /**
  +     * @todo Currently not part of the Component lifecycle (due to jsr77)
  +     */
       public void destroy() {
  -        log.debug("Destroying EJBProxyFactoryManager: ejbName=" + EJBPlugins.getEJBMetadata(getContainer()).getName());
  -        super.destroy();
  -        for (Iterator iterator = proxies.keySet().iterator(); iterator.hasNext();) {
  -            String proxyName = (String) iterator.next();
  -            EJBProxyFactory ejbProxyFactory = getEJBProxyFactory(proxyName);
  -            log.debug("Destroying EJBProxyFactory: proxyName=" + proxyName + " ejbName=" + EJBPlugins.getEJBMetadata(getContainer()).getName());
  -            ejbProxyFactory.destroy();
  -            // @todo uncomment this when the ejb proxy factories are rewriten to be full components
  -            //ejbProxyFactory.setContainer(null);
  -        }
           proxies.clear();
       }
   
  
  
  
  1.4       +8 -11     incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/cache/EnterpriseContextInstancePool.java
  
  Index: EnterpriseContextInstancePool.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/cache/EnterpriseContextInstancePool.java,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -r1.3 -r1.4
  --- EnterpriseContextInstancePool.java	11 Aug 2003 17:59:11 -0000	1.3
  +++ EnterpriseContextInstancePool.java	13 Aug 2003 02:12:40 -0000	1.4
  @@ -79,17 +79,16 @@
       private int maxSize = 100;
       private boolean hardLimit = false;
   
  -    public void create() throws Exception {
  -        super.create();
  -        pool = new SimpleInstancePool(EJBPlugins.getInstanceFactory(getContainer()), maxSize, hardLimit);
  -        discardQueue = new DiscardQueue();
  -    }
  -
  -    public void start() throws Exception {
  -        super.start();
  +    public void doStart() throws Exception {
  +        if (pool==null)
  +            pool = new SimpleInstancePool(EJBPlugins.getInstanceFactory(getContainer()), maxSize, hardLimit);
  +        if (discardQueue==null)
  +            discardQueue = new DiscardQueue();
  +        
           pool.fill();
       }
   
  +    // @todo Destroy not part of jsr77 lifecycle
       public void destroy() {
           List contexts = pool.stopPooling();
           pool = null;
  @@ -104,8 +103,6 @@
   
           discardQueue.stop();
           discardQueue = null;
  -
  -        super.destroy();
       }
   
       /**
  
  
  
  1.4       +113 -105  incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/container/ContainerImpl.java
  
  Index: ContainerImpl.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/container/ContainerImpl.java,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -r1.3 -r1.4
  --- ContainerImpl.java	11 Aug 2003 17:59:11 -0000	1.3
  +++ ContainerImpl.java	13 Aug 2003 02:12:40 -0000	1.4
  @@ -73,155 +73,163 @@
   /**
    *
    *
  - *
  + * @todo Currently this class implements the startRecursive method of 
  + * the JSR77 lifecycle. This should be moved to an AbstractContainer class
  + * @todo The stop method is implemented as stopRecursive, which should be moved
  + * to an abstractContainer class
    * @version $Revision$ $Date$
    */
  -public class ContainerImpl extends AbstractComponent implements Container {
  -    private final Map plugins = new LinkedHashMap();
  -    private final Map pluginObjects = new LinkedHashMap();
  -    private final LinkedList interceptors = new LinkedList();
  +public class ContainerImpl extends AbstractComponent implements Container
  +{
  +    private final Map plugins= new LinkedHashMap();
  +    private final Map pluginObjects= new LinkedHashMap();
  +    private final LinkedList interceptors= new LinkedList();
       // for efficency keep a reference to the first interceptor
       private Interceptor firstInterceptor;
   
  -    public InvocationResult invoke(Invocation invocation) throws Exception {
  +    public InvocationResult invoke(Invocation invocation) throws Exception
  +    {
           return firstInterceptor.invoke(invocation);
       }
   
  -    public void addInterceptor(Interceptor interceptor) {
  -        if (firstInterceptor == null) {
  -            firstInterceptor = interceptor;
  +    public void addInterceptor(Interceptor interceptor)
  +    {
  +        if (firstInterceptor == null)
  +        {
  +            firstInterceptor= interceptor;
               interceptors.addLast(interceptor);
  -        } else {
  -            Interceptor lastInterceptor = (Interceptor) interceptors.getLast();
  +        }
  +        else
  +        {
  +            Interceptor lastInterceptor= (Interceptor)interceptors.getLast();
               lastInterceptor.setNext(interceptor);
               interceptors.addLast(interceptor);
           }
       }
   
  -    public void create() throws Exception {
  -        super.create();
  -        // Create all the interceptors in forward insertion order
  -        for (Iterator iterator = pluginObjects.values().iterator(); iterator.hasNext();) {
  -            Object object = iterator.next();
  -            if (object instanceof Component) {
  -                Component component = (Component) object;
  -                component.setContainer(this);
  -                component.create();
  +    public void startRecursive() throws Exception
  +    {
  +        try
  +        {
  +            // start this component
  +            setState(State.STARTING);
  +
  +            // Start all the components in forward insertion order
  +            for (Iterator iterator= pluginObjects.values().iterator(); iterator.hasNext();)
  +            {
  +                Object object= iterator.next();
  +                if (object instanceof Component)
  +                {
  +                    Component component= (Component)object;
  +                    component.startRecursive();
  +                }
               }
  -        }
   
  -        // Create all the plugins in forward insertion order
  -        for (Iterator iterator = interceptors.iterator(); iterator.hasNext();) {
  -            Interceptor interceptor = (Interceptor) iterator.next();
  -            interceptor.setContainer(this);
  -            interceptor.create();
  -        }
  -    }
  +            // Start this component
  +            doStart();
   
  -    public void start() throws Exception {
  -        super.start();
  -        // Start all the interceptors in forward insertion order
  -        for (Iterator iterator = pluginObjects.values().iterator(); iterator.hasNext();) {
  -            Object object = iterator.next();
  -            if (object instanceof Component) {
  -                Component component = (Component) object;
  -                component.start();
  -            }
  +            setState(State.RUNNING);
  +        }
  +        finally
  +        {
  +            if (getState() != State.RUNNING)
  +                setState(State.FAILED);
           }
  +    }
   
  +    public void doStart() throws Exception
  +    {
           // Start all the plugins in forward insertion order
  -        for (Iterator iterator = interceptors.iterator(); iterator.hasNext();) {
  -            Interceptor interceptor = (Interceptor) iterator.next();
  +        for (Iterator iterator= interceptors.iterator(); iterator.hasNext();)
  +        {
  +            Interceptor interceptor= (Interceptor)iterator.next();
               interceptor.start();
           }
       }
   
  -    public void stop() {
  -        // Stop all the interceptors in reverse insertion order
  -        for (ListIterator iterator = interceptors.listIterator(interceptors.size()); iterator.hasPrevious();) {
  -            Interceptor interceptor = (Interceptor) iterator.previous();
  -            interceptor.stop();
  -        }
  -
  -        // Stop all the plugins in reverse insertion order
  -        LinkedList list = new LinkedList();
  -        for (Iterator iterator = pluginObjects.values().iterator(); iterator.hasNext();) {
  -            Object object = iterator.next();
  -            if (object instanceof Component) {
  -                list.addFirst(object);
  +    public void stop()
  +    {
  +        try
  +        {
  +            setState(State.STOPPING);
  +
  +            // @todo this is actually a stopRecursive, which is not supported
  +            // by JSR77
  +            // Stop all the plugins in reverse insertion order
  +            LinkedList list= new LinkedList();
  +            for (Iterator iterator= pluginObjects.values().iterator(); iterator.hasNext();)
  +            {
  +                Object object= iterator.next();
  +                if (object instanceof Component)
  +                {
  +                    list.addFirst(object);
  +                }
  +            }
  +            for (Iterator iterator= list.iterator(); iterator.hasNext();)
  +            {
  +                Component component= (Component)iterator.next();
  +                component.stop();
               }
  -        }
  -        for (Iterator iterator = list.iterator(); iterator.hasNext();) {
  -            Component component = (Component) iterator.next();
  -            component.stop();
  -        }
   
  -        super.stop();
  -    }
  +            // Stop this component
  +            doStop();
   
  -    public void destroy() {
  -        // Destroy all the interceptors in reverse insertion order
  -        for (ListIterator iterator = interceptors.listIterator(interceptors.size()); iterator.hasPrevious();) {
  -            Interceptor interceptor = (Interceptor) iterator.previous();
  -            interceptor.destroy();
  -            interceptor.setContainer(null);
  +            setState(State.STOPPED);
           }
  -
  -        // Destroy all the plugins in reverse insertion order
  -        LinkedList list = new LinkedList();
  -        for (Iterator iterator = pluginObjects.values().iterator(); iterator.hasNext();) {
  -            Object object = iterator.next();
  -            if (object instanceof Component) {
  -                list.addFirst(object);
  -            }
  +        finally
  +        {
  +            if (getState() != State.STOPPED)
  +                setState(State.FAILED);
           }
  -        for (Iterator iterator = list.iterator(); iterator.hasNext();) {
  -            Component component = (Component) iterator.next();
  -            component.destroy();
  -            component.setContainer(null);
  +    }
  +
  +    public void doStop()
  +    {
  +        // Stop all the interceptors in reverse insertion order
  +        for (ListIterator iterator= interceptors.listIterator(interceptors.size()); iterator.hasPrevious();)
  +        {
  +            Interceptor interceptor= (Interceptor)iterator.previous();
  +            interceptor.stop();
           }
  +    }
   
  +    // @todo destroy not supported in JSR77 lifecycle, needs to be
  +    // integrated or removed.
  +    public void destroy()
  +    {
           plugins.clear();
           pluginObjects.clear();
  -        super.destroy();
       }
   
  -    public ObjectName getPlugin(String logicalPluginName) {
  -        return (ObjectName) plugins.get(logicalPluginName);
  +    public ObjectName getPlugin(String logicalPluginName)
  +    {
  +        return (ObjectName)plugins.get(logicalPluginName);
       }
   
  -    public void putPlugin(String logicalPluginName, ObjectName objectName) {
  -        State state = getState();
  -        if (state != State.NOT_CREATED && state != State.DESTROYED) {
  -            throw new IllegalStateException("putPluginObject can only be called while in the not-created or destroyed states: state=" + state);
  +    public void putPlugin(String logicalPluginName, ObjectName objectName)
  +    {
  +        State state= getState();
  +        if (state != State.STOPPED)
  +        {
  +            throw new IllegalStateException(
  +                "putPluginObject can only be called while in the stopped state: state=" + state);
           }
  -        if (state == State.NOT_CREATED && objectName == null) {
  -            throw new IllegalArgumentException("Container has not been created; objectName must be NOT null.");
  -        }
  -        if (state == State.DESTROYED && objectName != null) {
  -            throw new IllegalArgumentException("Container has been destroyed; objectName must be null.");
  -        }
  -
           plugins.put(logicalPluginName, objectName);
       }
   
  -    public Object getPluginObject(String logicalPluginName) {
  +    public Object getPluginObject(String logicalPluginName)
  +    {
           return pluginObjects.get(logicalPluginName);
       }
   
  -    public void putPluginObject(String logicalPluginName, Object plugin) {
  -        State state = getState();
  -        if (state != State.NOT_CREATED && state != State.DESTROYED) {
  -            throw new IllegalStateException("putPluginObject can only be called while in the not-created or destroyed states: state=" + state);
  -        }
  -        if (state == State.NOT_CREATED && plugin == null) {
  -            throw new IllegalArgumentException("Container has not been created; plugin must be NOT null.");
  +    public void putPluginObject(String logicalPluginName, Object plugin)
  +    {
  +        State state= getState();
  +        if (state != State.STOPPED)
  +        {
  +            throw new IllegalStateException(
  +                "putPluginObject can only be called while in the not-created or destroyed states: state=" + state);
           }
  -        if (state == State.DESTROYED && plugin != null) {
  -            throw new IllegalArgumentException("Container has been destroyed; plugin must be null.");
  -        }
  -
           pluginObjects.put(logicalPluginName, plugin);
       }
   }
  -
  
  
  
  1.4       +11 -7     incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/context/TransactionInterceptor.java
  
  Index: TransactionInterceptor.java
  ===================================================================
  RCS file: /home/cvs/incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/context/TransactionInterceptor.java,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -r1.3 -r1.4
  --- TransactionInterceptor.java	11 Aug 2003 17:59:12 -0000	1.3
  +++ TransactionInterceptor.java	13 Aug 2003 02:12:40 -0000	1.4
  @@ -78,6 +78,10 @@
           return transactionInterceptor.getState();
       }
   
  +    public long getStartTime(){
  +        return transactionInterceptor.getStartTime();
  +    }
  +
       public final Container getContainer() {
           return container;
       }
  @@ -94,7 +98,7 @@
           this.nextInterceptor = nextInterceptor;
       }
   
  -    public void create() throws Exception {
  +    public void start() throws Exception {
           EJBMetadata ejbMetadata = EJBPlugins.getEJBMetadata(container);
           if (ejbMetadata.getTransactionDemarcation().isContainer()) {
               transactionInterceptor = new CMTInterceptor();
  @@ -103,19 +107,19 @@
           }
           transactionInterceptor.setContainer(container);
           transactionInterceptor.setNext(nextInterceptor);
  -        transactionInterceptor.create();
  -    }
  -
  -    public void start() throws Exception {
           transactionInterceptor.start();
       }
  +    
  +    public void startRecursive() throws Exception {
  +        transactionInterceptor.startRecursive();
  +    }
   
       public void stop() {
           transactionInterceptor.stop();
       }
   
  +    // @todo not supported by JSR77 lifecycle 
       public void destroy() {
  -        transactionInterceptor.destroy();
           transactionInterceptor.setContainer(null);
           transactionInterceptor = null;
       }
  
  
  

Re: cvs commit: incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/context TransactionInterceptor.java

Posted by Jason Dillon <ja...@coredevelopers.net>.
BTW, thanks for the feedback on commit messages.  I find it very 
helpful, as no one has ever really commented on them before, and it 
helps to have some feedback ;-)

--jason


On Thursday, August 14, 2003, at 04:16  AM, Greg Stein wrote:

> On Wed, Aug 13, 2003 at 02:12:40AM -0000, gregw@apache.org wrote:
>> ...
>>   Log:
>>   Updated the Component class to support the JSR77 state model.
>>   Implemented the base behaviour in AbstractComponent
>>   Implemented recursive behaviour in ContainerImpl - but should be 
>> moved to
>>   a AbstractContainer class.
>>   Moved create nehaviour to doStart methods
>>   Left destroy methods hanging with todo comments to resolve that 
>> issue.
>
> From this commit message, it sounds like you're mixing semantics in a 
> single
> commit, which should really be done in multiple commits. By breaking 
> down
> the commits into logical groups, it makes the commits reviewable. And
> reviewable commits is, um, "highly desirable" :-)
>
>>   Submitted by:	Greg Wilkins
>>   Reviewed by:	Posted to geronimo-dev and coredevelopers.net mailing 
>> lists
>
> Let's please keep all discussion about Geronimo on geronimo-dev rather 
> than
> private mailing lists.
>
> [ actually, on other projects, people have raised concerns about 
> "decisions"
>   made off-list, such as in IRC or coding sprints or other gatherings; 
> the
>   best thing to do for those cases is bring the resulting consensus to 
> the
>   list for a closure discussion ]
>
> Cheers,
> -g
>
> -- 
> Greg Stein, http://www.lyra.org/
>


Re: cvs commit: incubator-geronimo/modules/core/src/java/org/apache/geronimo/ejb/context TransactionInterceptor.java

Posted by Greg Stein <gs...@lyra.org>.
On Wed, Aug 13, 2003 at 02:12:40AM -0000, gregw@apache.org wrote:
>...
>   Log:
>   Updated the Component class to support the JSR77 state model.
>   Implemented the base behaviour in AbstractComponent
>   Implemented recursive behaviour in ContainerImpl - but should be moved to
>   a AbstractContainer class.
>   Moved create nehaviour to doStart methods
>   Left destroy methods hanging with todo comments to resolve that issue.

>From this commit message, it sounds like you're mixing semantics in a single
commit, which should really be done in multiple commits. By breaking down
the commits into logical groups, it makes the commits reviewable. And
reviewable commits is, um, "highly desirable" :-)

>   Submitted by:	Greg Wilkins
>   Reviewed by:	Posted to geronimo-dev and coredevelopers.net mailing lists

Let's please keep all discussion about Geronimo on geronimo-dev rather than
private mailing lists.

[ actually, on other projects, people have raised concerns about "decisions"
  made off-list, such as in IRC or coding sprints or other gatherings; the
  best thing to do for those cases is bring the resulting consensus to the
  list for a closure discussion ]

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/