[jboss-cvs] JBossAS SVN: r63817 - trunk/tomcat/src/main/org/jboss/web/tomcat/service/session.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Wed Jul 4 00:26:25 EDT 2007


Author: bstansberry at jboss.com
Date: 2007-07-04 00:26:25 -0400 (Wed, 04 Jul 2007)
New Revision: 63817

Modified:
   trunk/tomcat/src/main/org/jboss/web/tomcat/service/session/JvmRouteValve.java
Log:
Code clarity -- reorg code a bit, rename vars, comments
Avoid duplicate session id comparisons

Modified: trunk/tomcat/src/main/org/jboss/web/tomcat/service/session/JvmRouteValve.java
===================================================================
--- trunk/tomcat/src/main/org/jboss/web/tomcat/service/session/JvmRouteValve.java	2007-07-04 04:23:15 UTC (rev 63816)
+++ trunk/tomcat/src/main/org/jboss/web/tomcat/service/session/JvmRouteValve.java	2007-07-04 04:26:25 UTC (rev 63817)
@@ -35,12 +35,17 @@
 
 /**
  * Web request valve to specifically handle Tomcat jvmRoute using mod_jk(2)
- * module. We assume that the session is set by cookie only for now, i.e., no
- * support of that from URL. Furthermore, the session id has a format of
- * id.jvmRoute where jvmRoute is used by JK module to determine sticky session
- * during load balancing.
+ * module. We assume that the session id has a format of id.jvmRoute where 
+ * jvmRoute is used by JK module to determine sticky session during 
+ * load balancing. Checks for failover by matching session and request jvmRoute
+ * to the session manager's, updating the session and session cookie if a 
+ * failover is detected.
  *
+ * This valve is inserted in the pipeline only when mod_jk is configured.
+ * 
  * @author Ben Wang
+ * @author Brian Stansberry
+ * 
  * @version $Revision: 58922 $
  */
 public class JvmRouteValve extends ValveBase implements Lifecycle
@@ -75,32 +80,32 @@
 
    public void invoke(Request request, Response response) throws IOException, ServletException
    {
-
-      // Need to check it before let it through. This is ok because this 
-      // valve is inserted only when mod_jk option is configured.
+      // Need to check it before let request through.
       checkJvmRoute(request, response);
 
-      // let the servlet invokation go through
+      // let the servlet invocation go through
       getNext().invoke(request, response);
    }
 
    public void checkJvmRoute(Request req, Response res)
       throws IOException, ServletException
    {
-      String oldsessionId = req.getRequestedSessionId();
+      String requestedId = req.getRequestedSessionId();
       HttpSession session = req.getSession(false);
       if (session != null)
       {
          String sessionId = session.getId();
 
          // Obtain JvmRoute
+         // FIXME do this once in constructor
          String jvmRoute = manager_.getJvmRoute();
-         if (log_.isDebugEnabled())
+         if (log_.isTraceEnabled())
          {
-            log_.debug("checkJvmRoute(): check if need to re-route based on JvmRoute. Session id: " +
+            log_.trace("checkJvmRoute(): check if need to re-route based on JvmRoute. Session id: " +
                sessionId + " jvmRoute: " + jvmRoute);
          }
-
+         
+         // FIXME do this once in the session manager's start()
          if (jvmRoute == null)
          {
             throw new RuntimeException("JvmRouteValve.checkJvmRoute(): Tomcat JvmRoute is null. " +
@@ -109,36 +114,33 @@
 
          // Check if incoming session id has JvmRoute appended. If not, append it.
          boolean setCookie = !req.isRequestedSessionIdFromURL();
-         handleJvmRoute(oldsessionId, sessionId, jvmRoute, res, setCookie);
+         handleJvmRoute(requestedId, sessionId, jvmRoute, res, setCookie);
       }
    }
 
-   protected void handleJvmRoute(String oldsessionId,
+   protected void handleJvmRoute(String requestedId,
                                  String sessionId, 
                                  String jvmRoute, 
                                  HttpServletResponse response,
                                  boolean setCookie)
-   {
-      // Get requested jvmRoute.
+         throws IOException
+   {      
+      // The new id we'll give the session if we detect a failover
+      String newId = null;
+      
+      // First, check if the session object's jvmRoute matches ours      
+      
       // TODO. The current format is assumed to be id.jvmRoute. Can be generalized later.
-      String receivedJvmRoute = null;
-      int index = oldsessionId.lastIndexOf(".");
+      String sessionJvmRoute = null;
+      int index = sessionId.lastIndexOf(".");
       if (index > 0)
       {
-         receivedJvmRoute = oldsessionId.substring(index + 1, sessionId.length());
+         sessionJvmRoute = sessionId.substring(index + 1, sessionId.length());
       }
-
-      String requestedJvmRoute = null;
-      index = sessionId.lastIndexOf(".");
-      if (index > 0)
+      
+      if (!jvmRoute.equals(sessionJvmRoute))
       {
-         requestedJvmRoute = sessionId.substring(index + 1, sessionId.length());
-      }
-
-      String newId = null;
-      if (!jvmRoute.equals(requestedJvmRoute))
-      {
-         if (requestedJvmRoute == null)
+         if (sessionJvmRoute == null)
          {
             // If this valve is turned on, we assume we have an appendix of jvmRoute. 
             // So this request is new.
@@ -148,63 +150,76 @@
          {
             // We just had a failover since jvmRoute does not match. 
             // We will replace the old one with the new one.         
-            if (log_.isDebugEnabled())
+            if (log_.isTraceEnabled())
             {
-               log_.debug("handleJvmRoute(): We have detected a failover with different jvmRoute." +
-                  " old one: " + requestedJvmRoute + " new one: " + jvmRoute + ". Will reset the session id.");
+               log_.trace("handleJvmRoute(): We have detected a failover with different jvmRoute." +
+                  " old one: " + sessionJvmRoute + " new one: " + jvmRoute + ". Will reset the session id.");
             }
             
             String base = sessionId.substring(0, index);
             newId = base + "." + jvmRoute;
          }
          
-         resetSessionId(sessionId, newId);
-         
+         // Fix the session's id
+         resetSessionId(sessionId, newId);         
       }
-      /* Also check the jvmRoute received (via req.getRequestedSessionId()) */
-      if (!jvmRoute.equals(receivedJvmRoute))
+      
+      // Now we know the session object has a correct id
+      // Also need to ensure any session cookie is correct
+      if (setCookie)
       {
-            if (log_.isDebugEnabled())
+         // Check if the jvmRoute of the requested session id matches ours.
+         // Only bother if we haven't already spotted a mismatch above
+         if (newId == null)
+         {
+            String requestedJvmRoute = null;
+            index = requestedId.lastIndexOf(".");
+            if (index > 0)
             {
-               log_.debug("handleJvmRoute(): We have detected a failover with different jvmRoute." +
-                  " received one: " + receivedJvmRoute + " new one: " + jvmRoute + ". Will resent the session id.");
+               requestedJvmRoute = requestedId.substring(index + 1, requestedId.length());
             }
-            String base = sessionId.substring(0, index);
-            newId = base + "." + jvmRoute;
+            
+            if (!jvmRoute.equals(requestedJvmRoute))
+            {
+               if (log_.isTraceEnabled())
+               {
+                  log_.trace("handleJvmRoute(): We have detected a failover with different jvmRoute." +
+                     " received one: " + requestedJvmRoute + " new one: " + jvmRoute + ". Will resent the session id.");
+               }
+               String base = sessionId.substring(0, index);
+               newId = base + "." + jvmRoute;
+            }
+         }
+
+         /* Change the sessionid cookie if needed */
+         if (newId != null)
+            manager_.setNewSessionCookie(newId, response);
       }
-
-      /* Change the sessionid cookie if needed */
-      if (setCookie && newId != null)
-         manager_.setNewSessionCookie(newId, response);
    }
    
+   /** 
+    * Update the id of the given session
+    * 
+    *  @param oldId id of the session to change
+    *  @param newId new session id the session object should have
+    */
    private void resetSessionId(String oldId, String newId)
+         throws IOException
    {
-      try
+      ClusteredSession session = (ClusteredSession)manager_.findSession(oldId);
+      // change session id with the new one using local jvmRoute.
+      if( session != null )
       {
-         ClusteredSession session = (ClusteredSession)manager_.findSession(oldId);
-         // change session id with the new one using local jvmRoute.
-         if( session != null )
+         // Note this will trigger a session remove from the super Tomcat class.
+         session.resetIdWithRouteInfo(newId);
+         if (log_.isTraceEnabled())
          {
-            // Note this will trigger a session remove from the super Tomcat class.
-            session.resetIdWithRouteInfo(newId);
-            if (log_.isDebugEnabled())
-            {
-               log_.debug("resetSessionId(): changed catalina session to= [" + newId + "] old one= [" + oldId + "]");
-            }
+            log_.trace("resetSessionId(): changed catalina session to= [" + newId + "] old one= [" + oldId + "]");
          }
-         else if (log_.isDebugEnabled())
-         {
-            log_.debug("resetSessionId(): no session with id " + newId + " found");
-         }
       }
-      catch (IOException e)
+      else if (log_.isTraceEnabled())
       {
-         if (log_.isDebugEnabled())
-         {
-            log_.debug("resetSessionId(): manager_.findSession() unable to find session= [" + oldId + "]", e);
-         }
-         throw new RuntimeException("JvmRouteValve.resetSessionId(): cannot find session [" + oldId + "]", e);
+         log_.trace("resetSessionId(): no session with id " + newId + " found");
       }
    }
 




More information about the jboss-cvs-commits mailing list