You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by sk...@apache.org on 2008/01/11 12:17:25 UTC
svn commit: r611144 - in
/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra:
conversation/ conversation/servlet/ frameworkAdapter/
frameworkAdapter/basic/ frameworkAdapter/local/
Author: skitching
Date: Fri Jan 11 03:17:19 2008
New Revision: 611144
URL: http://svn.apache.org/viewvc?rev=611144&view=rev
Log:
ORCHESTRA-10 Change handling of conversation timeouts, support containers where Session.getId can throw exception.
This reverts the recent changes to FrameworkAdapter and subclasses, to keep that interface to a minimum
(as portable as possible). It also removes the "id" property from ConversationManager. ConversationWiperThread
now uses a set of ConversationManager rather than a hashmap keyed by session-id, meaning callers now need to
pass a ConversationManager instance to remove rather than a session-id. The ContextListener now also handles
session activate/passivate.
Note that this introduces a binary incompatibility in
ConversationWiperThread. However this is very unlikely to have been
subclassed or directly invoked by users.
Modified:
myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationManager.java
myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationWiperThread.java
myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/servlet/ConversationManagerSessionListener.java
myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/FrameworkAdapter.java
myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/basic/BasicFrameworkAdapter.java
myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/local/LocalFrameworkAdapter.java
Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationManager.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationManager.java?rev=611144&r1=611143&r2=611144&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationManager.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationManager.java Fri Jan 11 03:17:19 2008
@@ -62,14 +62,9 @@
// This member must always be accessed with a lock held on the parent ConverstationManager instance;
// a HashMap is not thread-safe and this class must be thread-safe.
private final Map conversationContexts = new HashMap();
-
- // holds the id of the actual session used to for unique identification of this conversationManager instance
- private String conversationManagerId;
-
- protected ConversationManager(String conversationManagerId)
+
+ protected ConversationManager()
{
- this.conversationManagerId = conversationManagerId;
- // conversationMessager = createMessager();
}
/**
@@ -111,7 +106,7 @@
{
// TODO: do not call new directly here, as it makes it impossible to configure
// an alternative ConversationManager instance. This is IOC and test unfriendly.
- conversationManager = new ConversationManager(frameworkAdapter.getSessionId());
+ conversationManager = new ConversationManager();
// initialize environmental systems
RequestParameterProviderManager.getInstance().register(new ConversationRequestParameterProvider());
@@ -158,14 +153,6 @@
return conversationContextId;
}
-
- /**
- *
- * @return a unique id for this conversationManger instance
- */
- public String getConversationManagerId() {
- return conversationManagerId;
- }
/**
* Get the conversation context for the given id
Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationWiperThread.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationWiperThread.java?rev=611144&r1=611143&r2=611144&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationWiperThread.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/ConversationWiperThread.java Fri Jan 11 03:17:19 2008
@@ -18,12 +18,12 @@
*/
package org.apache.myfaces.orchestra.conversation;
+import java.util.HashSet;
+import java.util.Set;
+
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
-import java.util.HashMap;
-import java.util.Map;
-
/**
* The ConversationWiperThread will trigger the conversation timeout check.
* <p>
@@ -37,7 +37,7 @@
private final long checkTime;
- private Map conversationManagers = new HashMap();
+ private Set conversationManagers = new HashSet();
/**
* Constructor.
@@ -55,40 +55,40 @@
/**
* Add a ConversationManager to check.
- * <p>
- * If there is already a ConversationManager associated with the given
- * <code>id</code>, the old ConversationManager will be replaced.
+ *
+ * @since 1.1
*/
- public void addConversationManager(String id, ConversationManager conversationManager)
+ public void addConversationManager(ConversationManager cm)
{
synchronized (conversationManagers)
{
- conversationManagers.put(id, conversationManager);
+ conversationManagers.add(cm);
}
}
/**
* Remove a ConversationManager from the list to check.
+ *
+ * @since 1.1
*/
- public void removeConversationManager(String id)
+ public void removeConversationManager(ConversationManager cm)
{
synchronized (conversationManagers)
{
- conversationManagers.remove(id);
+ boolean found = conversationManagers.remove(cm);
+ if (!found)
+ {
+ // sanity check: this should not happen.
+ log.error("Conversation Manager not found in remove");
+ }
}
}
public void run()
{
- if (log.isDebugEnabled())
- {
- log.debug("ConversationWiperThread startup"); // NON-NLS
- }
+ log.debug("ConversationWiperThread startup"); // NON-NLS
_run();
- if (log.isInfoEnabled())
- {
- log.debug("ConversationWiperThread shtudown"); // NON-NLS
- }
+ log.debug("ConversationWiperThread shtudown"); // NON-NLS
}
private void _run()
@@ -99,7 +99,12 @@
synchronized (conversationManagers)
{
managersArray = new ConversationManager[conversationManagers.size()];
- conversationManagers.values().toArray(managersArray);
+ conversationManagers.toArray(managersArray);
+ }
+
+ if (log.isDebugEnabled())
+ {
+ log.debug("ConversationWiperThread running against " + managersArray.length + " instances.");
}
for (int i = 0; i<managersArray.length; i++)
Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/servlet/ConversationManagerSessionListener.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/servlet/ConversationManagerSessionListener.java?rev=611144&r1=611143&r2=611144&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/servlet/ConversationManagerSessionListener.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/conversation/servlet/ConversationManagerSessionListener.java Fri Jan 11 03:17:19 2008
@@ -18,14 +18,19 @@
*/
package org.apache.myfaces.orchestra.conversation.servlet;
+import java.util.Enumeration;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.apache.myfaces.orchestra.conversation.ConversationManager;
import org.apache.myfaces.orchestra.conversation.ConversationWiperThread;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
+import javax.servlet.http.HttpSession;
+import javax.servlet.http.HttpSessionActivationListener;
import javax.servlet.http.HttpSessionAttributeListener;
import javax.servlet.http.HttpSessionBindingEvent;
-import javax.servlet.http.HttpSessionListener;
import javax.servlet.http.HttpSessionEvent;
/**
@@ -56,8 +61,9 @@
* for more details.
*/
public class ConversationManagerSessionListener
- implements HttpSessionAttributeListener, ServletContextListener
+ implements HttpSessionAttributeListener, HttpSessionActivationListener, ServletContextListener
{
+ private final Log log = LogFactory.getLog(ConversationManagerSessionListener.class);
private final static long DEFAULT_CHECK_TIME = 5 * 60 * 1000; // every 5 min
private final static String CHECK_TIME = "org.apache.myfaces.orchestra.WIPER_THREAD_CHECK_TIME"; // NON-NLS
@@ -75,6 +81,7 @@
conversationWiperThread = new ConversationWiperThread(checkTime);
conversationWiperThread.start();
+ log.debug("initialised");
}
public void contextDestroyed(ServletContextEvent event)
@@ -85,31 +92,92 @@
public void attributeAdded(HttpSessionBindingEvent event)
{
+ // Somebody has called session.setAttribute
if (event.getValue() instanceof ConversationManager)
{
- conversationWiperThread.addConversationManager(
- ((ConversationManager) event.getValue()).getConversationManagerId(),
- (ConversationManager) event.getValue());
+ ConversationManager cm = (ConversationManager) event.getValue();
+ conversationWiperThread.addConversationManager(cm);
}
}
public void attributeRemoved(HttpSessionBindingEvent event)
{
+ // Either someone has called session.removeAttribute, or the session has been invalidated.
+ // When an HttpSession is invalidated (including when it "times out"), this method is
+ // called once for every attribute in the session; note however that at that time the
+ // session is invalid so in some containers certain methods (including getId and
+ // getAttribute) throw IllegalStateException.
if (event.getValue() instanceof ConversationManager)
{
- conversationWiperThread.removeConversationManager(((ConversationManager) event.getValue()).getConversationManagerId());
+ ConversationManager cm = (ConversationManager) event.getValue();
+ conversationWiperThread.removeConversationManager(cm);
}
}
public void attributeReplaced(HttpSessionBindingEvent event)
{
+ // Note that this method is called *after* the attribute has been replaced,
+ // and that event.getValue contains the old object.
if (event.getValue() instanceof ConversationManager)
{
- String id = ((ConversationManager) event.getValue()).getConversationManagerId();
- ConversationManager newConversationManager = (ConversationManager) event.getSession().getAttribute(event.getName());
- conversationWiperThread.addConversationManager(
- id, newConversationManager);
+ ConversationManager oldConversationManager = (ConversationManager) event.getValue();
+ conversationWiperThread.removeConversationManager(oldConversationManager);
+ }
+
+ // The new object is already in the session and can be retrieved from there
+ HttpSession session = event.getSession();
+ String attrName = event.getName();
+ Object newObj = session.getAttribute(attrName);
+ if (newObj instanceof ConversationManager)
+ {
+ ConversationManager newConversationManager = (ConversationManager) newObj;
+ conversationWiperThread.addConversationManager(newConversationManager);
}
}
+ public void sessionDidActivate(HttpSessionEvent se)
+ {
+ // Reattach any ConversationManager objects in the session to the conversationWiperThread
+ HttpSession session = se.getSession();
+ Enumeration e = session.getAttributeNames();
+ while (e.hasMoreElements())
+ {
+ String attrName = (String) e.nextElement();
+ Object val = session.getAttribute(attrName);
+ if (val instanceof ConversationManager)
+ {
+ // TODO: maybe touch the "last accessed" stamp for the conversation manager
+ // and all its children? Without this, a conversation that has been passivated
+ // might almost immediately get cleaned up after being reactivated.
+ //
+ // Hmm..actually, we should make sure the wiper thread never cleans up anything
+ // associated with a session that is currently in use by a request. That should
+ // then be sufficient, as the timeouts will only apply after the end of the
+ // request that caused this activation to occur by which time any relevant
+ // timestamps have been restored.
+ ConversationManager cm = (ConversationManager) val;
+ conversationWiperThread.addConversationManager(cm);
+ }
+ }
+ }
+
+ public void sessionWillPassivate(HttpSessionEvent se)
+ {
+ // Detach all ConversationManager objects in the session from the conversationWiperThread.
+ // Without this, the ConversationManager and all its child objects would be kept in
+ // memory as well as being passivated to external storage. Of course this does mean
+ // that conversations in passivated sessions will not get timed out.
+ HttpSession session = se.getSession();
+ Enumeration e = session.getAttributeNames();
+ while (e.hasMoreElements())
+ {
+ String attrName = (String) e.nextElement();
+ Object val = session.getAttribute(attrName);
+ if (val instanceof ConversationManager)
+ {
+ ConversationManager cm = (ConversationManager) val;
+ conversationWiperThread.removeConversationManager(cm);
+ }
+ }
+ }
}
Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/FrameworkAdapter.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/FrameworkAdapter.java?rev=611144&r1=611143&r2=611144&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/FrameworkAdapter.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/FrameworkAdapter.java Fri Jan 11 03:17:19 2008
@@ -181,15 +181,11 @@
*/
public abstract Object getSessionAttribute(String key);
- public abstract void setSessionAttribute(String key, Object value);
+ public abstract void setSessionAttribute(String key, Object value);
public abstract boolean containsSessionAttribute(String key);
- /**
- * @return id of the actual session
- */
- public abstract String getSessionId();
- /**
+ /**
* Instruct the remote browser to fetch the specified URL.
*/
public abstract void redirect(String url) throws IOException;
Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/basic/BasicFrameworkAdapter.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/basic/BasicFrameworkAdapter.java?rev=611144&r1=611143&r2=611144&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/basic/BasicFrameworkAdapter.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/basic/BasicFrameworkAdapter.java Fri Jan 11 03:17:19 2008
@@ -225,19 +225,7 @@
throw new IllegalStateException(ISE_MESSAGE);
}
- /**
- * @see org.apache.myfaces.orchestra.frameworkAdapter.FrameworkAdapter#getSessionId()
- */
- public String getSessionId() {
- HttpServletRequest request = getRequest();
- if (request != null && request.getSession(true) != null)
- {
- return request.getSession(true).getId();
- }
- throw new IllegalStateException(ISE_MESSAGE);
- }
-
- protected String getRequestContextPath()
+ protected String getRequestContextPath()
{
HttpServletRequest request = getRequest();
if (request != null)
Modified: myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/local/LocalFrameworkAdapter.java
URL: http://svn.apache.org/viewvc/myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/local/LocalFrameworkAdapter.java?rev=611144&r1=611143&r2=611144&view=diff
==============================================================================
--- myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/local/LocalFrameworkAdapter.java (original)
+++ myfaces/orchestra/trunk/core/src/main/java/org/apache/myfaces/orchestra/frameworkAdapter/local/LocalFrameworkAdapter.java Fri Jan 11 03:17:19 2008
@@ -126,11 +126,7 @@
return sessionMap.containsKey(key);
}
- public String getSessionId() {
- return String.valueOf(sessionMap.hashCode());
- }
-
- protected ConfigurableApplicationContext getApplicationContext()
+ protected ConfigurableApplicationContext getApplicationContext()
{
return configurableApplicationContext;
}