Merge remote-tracking branch 'origin/jetty-7' into jetty-8

jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java

@@ -30,7 +30,9 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Random;
 import java.util.Timer;
@@ -81,7 +83,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
     protected String _createSessionIdTable;
     protected String _createSessionTable;
                                             
-    protected String _selectExpiredSessions;
+    protected String _selectBoundedExpiredSessions;
     protected String _deleteOldExpiredSessions;
 
     protected String _insertId;
@@ -96,6 +98,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
     
     protected DatabaseAdaptor _dbAdaptor;
 
+    private String _selectExpiredSessions;
+
     
     /**
      * DatabaseAdaptor
@@ -114,7 +118,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
         String _dbName;
         boolean _isLower;
         boolean _isUpper;
-
+       
         
         
         public DatabaseAdaptor (DatabaseMetaData dbMeta)
@@ -123,7 +127,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
             _dbName = dbMeta.getDatabaseProductName().toLowerCase(); 
             LOG.debug ("Using database {}",_dbName);
             _isLower = dbMeta.storesLowerCaseIdentifiers();
-            _isUpper = dbMeta.storesUpperCaseIdentifiers();
+            _isUpper = dbMeta.storesUpperCaseIdentifiers();            
         }
         
         /**
@@ -455,6 +459,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
             inUse = _sessionIds.contains(clusterId);
         }
         
+        
         if (inUse)
             return true; //optimisation - if this session is one we've been managing, we can check locally
 
@@ -514,7 +519,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
         try
         {            
             initializeDatabase();
-            prepareTables();        
+            prepareTables();   
+            cleanExpiredSessions();
             super.doStart();
             if (LOG.isDebugEnabled()) 
                 LOG.debug("Scavenging interval = "+getScavengeInterval()+" sec");
@@ -542,6 +548,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
                 _timer.cancel();
             _timer=null;
         }
+        _sessionIds.clear();
         super.doStop();
     }
   
@@ -559,31 +566,9 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
         else
             return DriverManager.getConnection(_connectionUrl);
     }
-
     
-    private void initializeDatabase ()
-    throws Exception
-    {
-        if (_datasource != null)
-            return; //already set up
-        
-        if (_jndiName!=null)
-        {
-            InitialContext ic = new InitialContext();
-            _datasource = (DataSource)ic.lookup(_jndiName);
-        }
-        else if ( _driver != null && _connectionUrl != null )
-        {
-            DriverManager.registerDriver(_driver);
-        }
-        else if (_driverClassName != null && _connectionUrl != null)
-        {
-            Class.forName(_driverClassName);
-        }
-        else
-            throw new IllegalStateException("No database configured for sessions");
-    }
     
+   
     
     
     /**
@@ -594,7 +579,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
     throws SQLException
     {
         _createSessionIdTable = "create table "+_sessionIdTable+" (id varchar(120), primary key(id))";
-        _selectExpiredSessions = "select * from "+_sessionTable+" where expiryTime >= ? and expiryTime <= ?";
+        _selectBoundedExpiredSessions = "select * from "+_sessionTable+" where expiryTime >= ? and expiryTime <= ?";
+        _selectExpiredSessions = "select * from "+_sessionTable+" where expiryTime >0 and expiryTime <= ?";
         _deleteOldExpiredSessions = "delete from "+_sessionTable+" where expiryTime >0 and expiryTime <= ?";
 
         _insertId = "insert into "+_sessionIdTable+" (id)  values (?)";
@@ -794,7 +780,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
                 connection = getConnection();
                 connection.setAutoCommit(true);
                 //"select sessionId from JettySessions where expiryTime > (lastScavengeTime - scanInterval) and expiryTime < lastScavengeTime";
-                PreparedStatement statement = connection.prepareStatement(_selectExpiredSessions);
+                PreparedStatement statement = connection.prepareStatement(_selectBoundedExpiredSessions);
                 long lowerBound = (_lastScavengeTime - _scavengeIntervalMs);
                 long upperBound = _lastScavengeTime;
                 if (LOG.isDebugEnabled()) 
@@ -833,7 +819,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
                     if (LOG.isDebugEnabled()) LOG.debug("Deleting old expired sessions expired before "+upperBound);
                     statement = connection.prepareStatement(_deleteOldExpiredSessions);
                     statement.setLong(1, upperBound);
-                    statement.executeUpdate();
+                    int rows = statement.executeUpdate();
+                    if (LOG.isDebugEnabled()) LOG.debug("Deleted "+rows+" rows");
                 }
             }
         }
@@ -861,4 +848,121 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
             }
         }
     }
+    
+    /**
+     * Get rid of sessions and sessionids from sessions that have already expired
+     * @throws Exception
+     */
+    private void cleanExpiredSessions ()
+    throws Exception
+    {
+        Connection connection = null;
+        List<String> expiredSessionIds = new ArrayList<String>();
+        try
+        {     
+            connection = getConnection();
+            connection.setTransactionIsolation(Connection.TRANSACTION_SERIALIZABLE);
+            connection.setAutoCommit(false);
+
+            PreparedStatement statement = connection.prepareStatement(_selectExpiredSessions);
+            long now = System.currentTimeMillis();
+            if (LOG.isDebugEnabled()) LOG.debug ("Searching for sessions expired before {}", now);
+
+            statement.setLong(1, now);
+            ResultSet result = statement.executeQuery();
+            while (result.next())
+            {
+                String sessionId = result.getString("sessionId");
+                expiredSessionIds.add(sessionId);
+                if (LOG.isDebugEnabled()) LOG.debug ("Found expired sessionId={}", sessionId); 
+            }
+            
+            Statement sessionsTableStatement = null;
+            Statement sessionIdsTableStatement = null;
+
+            if (!expiredSessionIds.isEmpty())
+            {
+                sessionsTableStatement = connection.createStatement();
+                sessionsTableStatement.executeUpdate(createCleanExpiredSessionsSql("delete from "+_sessionTable+" where sessionId in ", expiredSessionIds));
+                sessionIdsTableStatement = connection.createStatement();
+                sessionIdsTableStatement.executeUpdate(createCleanExpiredSessionsSql("delete from "+_sessionIdTable+" where id in ", expiredSessionIds));
+            }
+            connection.commit();
+
+            synchronized (_sessionIds)
+            {
+                _sessionIds.removeAll(expiredSessionIds); //in case they were in our local cache of session ids
+            }
+        }
+        catch (Exception e)
+        {
+            if (connection != null)
+                connection.rollback();
+            throw e;
+        }
+        finally
+        {
+            try
+            {
+                if (connection != null)
+                    connection.close();
+            }
+            catch (SQLException e)
+            {
+                LOG.warn(e);
+            }
+        }
+    }
+    
+    
+    /**
+     * 
+     * @param sql
+     * @param connection
+     * @param expiredSessionIds
+     * @throws Exception
+     */
+    private String createCleanExpiredSessionsSql (String sql,Collection<String> expiredSessionIds)
+    throws Exception
+    {
+        StringBuffer buff = new StringBuffer();
+        buff.append(sql);
+        buff.append("(");
+        Iterator<String> itor = expiredSessionIds.iterator();
+        while (itor.hasNext())
+        {
+            buff.append("'"+(itor.next())+"'");
+            if (itor.hasNext())
+                buff.append(",");
+        }
+        buff.append(")");
+        
+        if (LOG.isDebugEnabled()) LOG.debug("Cleaning expired sessions with: {}", buff);
+        return buff.toString();
+    }
+    
+    private void initializeDatabase ()
+    throws Exception
+    {
+        if (_datasource != null)
+            return; //already set up
+        
+        if (_jndiName!=null)
+        {
+            InitialContext ic = new InitialContext();
+            _datasource = (DataSource)ic.lookup(_jndiName);
+        }
+        else if ( _driver != null && _connectionUrl != null )
+        {
+            DriverManager.registerDriver(_driver);
+        }
+        else if (_driverClassName != null && _connectionUrl != null)
+        {
+            Class.forName(_driverClassName);
+        }
+        else
+            throw new IllegalStateException("No database configured for sessions");
+    }
+    
+   
 }

jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java

@@ -542,7 +542,7 @@ public class JDBCSessionManager extends AbstractSessionManager
                         //if the session has no expiry, or it is not already expired
                         if (data._expiryTime <= 0 || data._expiryTime > now)
                         {
-                            LOG.debug("getSession("+idInCluster+"): lastNode="+data.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName());
+                            if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): lastNode="+data.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName());
                             data.setLastNode(getSessionIdManager().getWorkerName());
                             //session last used on a different node, or we don't have it in memory
                             session = new Session(now,data);
@@ -553,18 +553,21 @@ public class JDBCSessionManager extends AbstractSessionManager
                             updateSessionNode(data);
                         }
                         else
-                            if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): Session has expired");
+                        {
+                            LOG.debug("getSession ({}): Session has expired", idInCluster);
+                            
+                        }
 
                     }
                     else
-                        if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): Session not stale "+session._data);
+                       LOG.debug("getSession({}): Session not stale {}", idInCluster,session._data);
                     //session in db shares same id, but is not for this context
                 }
                 else
                 {
                     //No session in db with matching id and context path.
                     session=null;
-                    if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): No session in database matching id="+idInCluster);
+                    LOG.debug("getSession({}): No session in database matching id={}",idInCluster,idInCluster);
                 }
 
                 return session;
@@ -608,6 +611,7 @@ public class JDBCSessionManager extends AbstractSessionManager
         _jdbcSessionIdMgr = (JDBCSessionIdManager)_sessionIdManager;
         
         _sessions = new ConcurrentHashMap<String, AbstractSession>();
+
         super.doStart();
     }
 

tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/SessionExpiryTest.java

@@ -0,0 +1,53 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+package org.eclipse.jetty.server.session;
+
+import org.junit.Test;
+
+/**
+ * SessionExpiryTest
+ *
+ *
+ *
+ */
+public class SessionExpiryTest extends AbstractSessionExpiryTest
+{
+
+    /** 
+     * @see org.eclipse.jetty.server.session.AbstractSessionExpiryTest#createServer(int, int, int)
+     */
+    @Override
+    public AbstractTestServer createServer(int port, int max, int scavenge)
+    {
+        return new JdbcTestServer(port,max,scavenge);
+    }
+
+    @Test
+    public void testSessionExpiry() throws Exception
+    {
+        super.testSessionExpiry();
+    }
+
+    @Test
+    public void testSessionNotExpired() throws Exception
+    {
+        super.testSessionNotExpired();
+    }
+
+    
+}

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionExpiryTest.java

@@ -0,0 +1,210 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+package org.eclipse.jetty.server.session;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+
+import org.eclipse.jetty.client.ContentExchange;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.http.HttpMethods;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.SessionManager;
+import org.eclipse.jetty.server.session.AbstractTestServer;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+
+
+/**
+ * AbstractSessionExpiryTest
+ *
+ *
+ *
+ */
+public abstract class AbstractSessionExpiryTest
+{
+    public abstract AbstractTestServer createServer(int port, int max, int scavenge);
+
+    public void pause(int scavengePeriod)
+    {
+        try
+        {
+            Thread.sleep(scavengePeriod * 2500L);
+        }
+        catch (InterruptedException e)
+        {
+            e.printStackTrace();
+        }
+    }
+    
+    @Test
+    public void testSessionNotExpired() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 10;
+        int scavengePeriod = 10;
+        AbstractTestServer server1 = createServer(0, inactivePeriod, scavengePeriod);
+        TestServlet servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(servlet);
+        server1.addContext(contextPath).addServlet(holder, servletMapping);
+        server1.start();
+        int port1 = server1.getPort();
+
+        try
+        {
+            HttpClient client = new HttpClient();
+            client.setConnectorType(HttpClient.CONNECTOR_SOCKET);
+            client.start();
+            String url = "http://localhost:" + port1 + contextPath + servletMapping;
+
+            //make a request to set up a session on the server
+            ContentExchange exchange1 = new ContentExchange(true);
+            exchange1.setMethod(HttpMethods.GET);
+            exchange1.setURL(url + "?action=init");
+            client.send(exchange1);
+            exchange1.waitForDone();
+            assertEquals(HttpServletResponse.SC_OK,exchange1.getResponseStatus());
+            String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie");
+            assertTrue(sessionCookie != null);
+            // Mangle the cookie, replacing Path with $Path, etc.
+            sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+            
+            //now stop the server
+            server1.stop();
+            
+            //start the server again, before the session times out
+            server1.start();            
+            port1 = server1.getPort();
+            url = "http://localhost:" + port1 + contextPath + servletMapping;
+            
+            //make another request, the session should not have expired
+            ContentExchange exchange2 = new ContentExchange(true);
+            exchange2.setMethod(HttpMethods.GET);
+            exchange2.setURL(url + "?action=notexpired");
+            exchange2.getRequestFields().add("Cookie", sessionCookie);
+            client.send(exchange2);
+            exchange2.waitForDone();
+            assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus());
+
+        }
+        finally
+        {
+            server1.stop();
+        }
+    }
+
+    @Test
+    public void testSessionExpiry() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 2;
+        int scavengePeriod = 10;
+        AbstractTestServer server1 = createServer(0, inactivePeriod, scavengePeriod);
+        TestServlet servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(servlet);
+        server1.addContext(contextPath).addServlet(holder, servletMapping);
+        server1.start();
+        int port1 = server1.getPort();
+
+        try
+        {
+            HttpClient client = new HttpClient();
+            client.setConnectorType(HttpClient.CONNECTOR_SOCKET);
+            client.start();
+            String url = "http://localhost:" + port1 + contextPath + servletMapping;
+
+            //make a request to set up a session on the server
+            ContentExchange exchange1 = new ContentExchange(true);
+            exchange1.setMethod(HttpMethods.GET);
+            exchange1.setURL(url + "?action=init");
+            client.send(exchange1);
+            exchange1.waitForDone();
+            assertEquals(HttpServletResponse.SC_OK,exchange1.getResponseStatus());
+            String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie");
+            assertTrue(sessionCookie != null);
+            // Mangle the cookie, replacing Path with $Path, etc.
+            sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+            
+            //now stop the server
+            server1.stop();
+            
+            //and wait until the expiry time has passed
+            pause(inactivePeriod);
+            
+            //restart the server
+            server1.start();            
+            port1 = server1.getPort();
+            url = "http://localhost:" + port1 + contextPath + servletMapping;
+            
+            //make another request, the session should have expired
+            ContentExchange exchange2 = new ContentExchange(true);
+            exchange2.setMethod(HttpMethods.GET);
+            exchange2.setURL(url + "?action=test");
+            exchange2.getRequestFields().add("Cookie", sessionCookie);
+            client.send(exchange2);
+            exchange2.waitForDone();
+            assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus());
+        }
+        finally
+        {
+            server1.stop();
+        }
+    }
+
+    public static class TestServlet extends HttpServlet
+    {
+        public String originalId = null;
+        public String testId = null;
+        public String checkId = null;
+
+        @Override
+        protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException
+        {
+            String action = request.getParameter("action");
+            if ("init".equals(action))
+            {
+                HttpSession session = request.getSession(true);
+                session.setAttribute("test", "test");
+                originalId = session.getId();
+            }
+            else if ("test".equals(action))
+            {
+                HttpSession session = request.getSession(true);
+                assertTrue(session != null);
+                assertTrue(!originalId.equals(session.getId()));
+            }
+            else if ("notexpired".equals(action))
+            {
+                HttpSession session = request.getSession(false);
+                assertTrue(session != null);
+                assertTrue(originalId.equals(session.getId()));
+            }
+           
+        }
+    }
+}