SEC-484: fixed concurrency issue

core/src/main/java/org/acegisecurity/concurrent/SessionRegistryImpl.java

@@ -34,7 +34,6 @@ import java.util.Set;
 
 import javax.servlet.http.HttpSession;
 
-
 /**
  * Base implementation of {@link org.acegisecurity.concurrent.SessionRegistry}
  * which also listens for {@link
@@ -47,113 +46,123 @@ import javax.servlet.http.HttpSession;
  * <code>web.xml</code> so that this class is notified of sessions that
  * expire.
  * </p>
- *
+ * 
  * @author Ben Alex
- * @version $Id${date}
+ * @version $Id: SessionRegistryImpl.java 1862 2007-05-25 01:38:42Z benalex
+ *          ${date}
  */
 public class SessionRegistryImpl implements SessionRegistry,
-    ApplicationListener {
-    //~ Instance fields ========================================================
-
-    private Map principals = Collections.synchronizedMap(new HashMap()); // <principal:Object,SessionIdSet>
-    private Map sessionIds = Collections.synchronizedMap(new HashMap()); // <sessionId:Object,SessionInformation>
-
-    //~ Methods ================================================================
-
-    public Object[] getAllPrincipals() {
-        return principals.keySet().toArray();
-    }
-
-    public SessionInformation[] getAllSessions(Object principal,
-        boolean includeExpiredSessions) {
-        Set sessionsUsedByPrincipal = (Set) principals.get(principal);
-
-        if (sessionsUsedByPrincipal == null) {
-            return null;
-        }
-
-        List list = new ArrayList();
-        Iterator iter = sessionsUsedByPrincipal.iterator();
-
-        while (iter.hasNext()) {
-            synchronized (sessionsUsedByPrincipal) {
-                String sessionId = (String) iter.next();
-                SessionInformation sessionInformation = getSessionInformation(sessionId);
-
-                if (includeExpiredSessions || !sessionInformation.isExpired()) {
-                    list.add(sessionInformation);
-                }
-            }
-        }
-
-        return (SessionInformation[]) list.toArray(new SessionInformation[] {});
-    }
-
-    public SessionInformation getSessionInformation(String sessionId) {
-        Assert.hasText(sessionId, "SessionId required as per interface contract");
-
-        return (SessionInformation) sessionIds.get(sessionId);
-    }
-
-    public void onApplicationEvent(ApplicationEvent event) {
-        if (event instanceof HttpSessionDestroyedEvent) {
-            String sessionId = ((HttpSession) event.getSource()).getId();
-            removeSessionInformation(sessionId);
-        }
-    }
-
-    public void refreshLastRequest(String sessionId) {
-        Assert.hasText(sessionId, "SessionId required as per interface contract");
-
-        SessionInformation info = getSessionInformation(sessionId);
-
-        if (info != null) {
-            info.refreshLastRequest();
-        }
-    }
-
-    public synchronized void registerNewSession(String sessionId, Object principal) {
-        Assert.hasText(sessionId, "SessionId required as per interface contract");
-        Assert.notNull(principal, "Principal required as per interface contract");
-
-        if (getSessionInformation(sessionId) != null) {
-            removeSessionInformation(sessionId);
-        }
-
-        sessionIds.put(sessionId,
-            new SessionInformation(principal, sessionId, new Date()));
-
-        Set sessionsUsedByPrincipal = (Set) principals.get(principal);
-
-        if (sessionsUsedByPrincipal == null) {
-            sessionsUsedByPrincipal = Collections.synchronizedSet(new HashSet());
-        }
-
-        sessionsUsedByPrincipal.add(sessionId);
-
-        principals.put(principal, sessionsUsedByPrincipal);
-    }
-
-    public void removeSessionInformation(String sessionId) {
-        Assert.hasText(sessionId, "SessionId required as per interface contract");
-
-        SessionInformation info = getSessionInformation(sessionId);
-
-        if (info != null) {
-            sessionIds.remove(sessionId);
+		ApplicationListener {
+	// ~ Instance fields
+	// ========================================================
 
-            Set sessionsUsedByPrincipal = (Set) principals.get(info.getPrincipal());
+	private Map principals = Collections.synchronizedMap(new HashMap()); // <principal:Object,SessionIdSet>
+	private Map sessionIds = Collections.synchronizedMap(new HashMap()); // <sessionId:Object,SessionInformation>
 
-            if (sessionsUsedByPrincipal != null) {
-                synchronized (sessionsUsedByPrincipal) {
-                    sessionsUsedByPrincipal.remove(sessionId);
+	// ~ Methods
+	// ================================================================
 
-                    if (sessionsUsedByPrincipal.size() == 0) {
-                        // No need to keep object in principals Map anymore 
-                        principals.remove(info.getPrincipal());
-                    }
-                }
-            }
-        }
-    }
+	public Object[] getAllPrincipals() {
+		return principals.keySet().toArray();
+	}
+
+	public SessionInformation[] getAllSessions(Object principal,
+			boolean includeExpiredSessions) {
+		Set sessionsUsedByPrincipal = (Set) principals.get(principal);
+		if (sessionsUsedByPrincipal == null) {
+			return null;
+		}
+
+		List list = new ArrayList();
+
+		synchronized (sessionsUsedByPrincipal) {
+			for (Iterator iter = sessionsUsedByPrincipal.iterator(); iter
+					.hasNext();) {
+				String sessionId = (String) iter.next();
+				SessionInformation sessionInformation = getSessionInformation(sessionId);
+
+				if (includeExpiredSessions || !sessionInformation.isExpired()) {
+					list.add(sessionInformation);
+				}
+			}
+		}
+
+		return (SessionInformation[]) list.toArray(new SessionInformation[] {});
+	}
+
+	public SessionInformation getSessionInformation(String sessionId) {
+		Assert.hasText(sessionId,
+				"SessionId required as per interface contract");
+
+		return (SessionInformation) sessionIds.get(sessionId);
+	}
+
+	public void onApplicationEvent(ApplicationEvent event) {
+		if (event instanceof HttpSessionDestroyedEvent) {
+			String sessionId = ((HttpSession) event.getSource()).getId();
+			removeSessionInformation(sessionId);
+		}
+	}
+
+	public void refreshLastRequest(String sessionId) {
+		Assert.hasText(sessionId,
+				"SessionId required as per interface contract");
+
+		SessionInformation info = getSessionInformation(sessionId);
+
+		if (info != null) {
+			info.refreshLastRequest();
+		}
+	}
+
+	public synchronized void registerNewSession(String sessionId,
+			Object principal) {
+		Assert.hasText(sessionId,
+				"SessionId required as per interface contract");
+		Assert.notNull(principal,
+				"Principal required as per interface contract");
+
+		if (getSessionInformation(sessionId) != null) {
+			removeSessionInformation(sessionId);
+		}
+
+		sessionIds.put(sessionId, new SessionInformation(principal, sessionId,
+				new Date()));
+
+		Set sessionsUsedByPrincipal = (Set) principals.get(principal);
+
+		if (sessionsUsedByPrincipal == null) {
+			sessionsUsedByPrincipal = Collections
+					.synchronizedSet(new HashSet());
+		}
+
+		sessionsUsedByPrincipal.add(sessionId);
+
+		principals.put(principal, sessionsUsedByPrincipal);
+	}
+
+	public void removeSessionInformation(String sessionId) {
+		Assert.hasText(sessionId,
+				"SessionId required as per interface contract");
+
+		SessionInformation info = getSessionInformation(sessionId);
+
+		if (info != null) {
+			sessionIds.remove(sessionId);
+
+			Set sessionsUsedByPrincipal = (Set) principals.get(info
+					.getPrincipal());
+
+			if (sessionsUsedByPrincipal != null) {
+				synchronized (sessionsUsedByPrincipal) {
+					sessionsUsedByPrincipal.remove(sessionId);
+
+					if (sessionsUsedByPrincipal.size() == 0) {
+						// No need to keep object in principals Map anymore
+						principals.remove(info.getPrincipal());
+					}
+				}
+			}
+		}
+	}
 }

core/src/test/java/org/acegisecurity/concurrent/SessionRegistryImplTests.java

@@ -23,137 +23,153 @@ import org.springframework.mock.web.MockHttpSession;
 
 import java.util.Date;
 
-
 /**
  * Tests {@link SessionRegistryImpl}.
- *
+ * 
  * @author Ben Alex
  * @version $Id$
  */
 public class SessionRegistryImplTests extends TestCase {
-    //~ Methods ========================================================================================================
-
-    public void testEventPublishing() {
-        MockHttpSession httpSession = new MockHttpSession();
-        Object principal = "Some principal object";
-        String sessionId = httpSession.getId();
-        assertNotNull(sessionId);
-
-        SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
+	// ~ Methods
+	// ========================================================================================================
+
+	public void testEventPublishing() {
+		MockHttpSession httpSession = new MockHttpSession();
+		Object principal = "Some principal object";
+		String sessionId = httpSession.getId();
+		assertNotNull(sessionId);
+
+		SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
+
+		// Register new Session
+		sessionRegistry.registerNewSession(sessionId, principal);
+
+		// Deregister session via an ApplicationEvent
+		sessionRegistry.onApplicationEvent(new HttpSessionDestroyedEvent(
+				httpSession));
+
+		// Check attempts to retrieve cleared session return null
+		assertNull(sessionRegistry.getSessionInformation(sessionId));
+	}
+
+	public void testMultiplePrincipals() throws Exception {
+		Object principal1 = "principal_1";
+		Object principal2 = "principal_2";
+		String sessionId1 = "1234567890";
+		String sessionId2 = "9876543210";
+		String sessionId3 = "5432109876";
+
+		SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
+
+		sessionRegistry.registerNewSession(sessionId1, principal1);
+		sessionRegistry.registerNewSession(sessionId2, principal1);
+		sessionRegistry.registerNewSession(sessionId3, principal2);
+
+		assertEquals(principal1, sessionRegistry.getAllPrincipals()[0]);
+		assertEquals(principal2, sessionRegistry.getAllPrincipals()[1]);
+	}
+
+	public void testSessionInformationLifecycle() throws Exception {
+		Object principal = "Some principal object";
+		String sessionId = "1234567890";
+		SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
+
+		// Register new Session
+		sessionRegistry.registerNewSession(sessionId, principal);
+
+		// Retrieve existing session by session ID
+		Date currentDateTime = sessionRegistry.getSessionInformation(sessionId)
+				.getLastRequest();
+		assertEquals(principal, sessionRegistry
+				.getSessionInformation(sessionId).getPrincipal());
+		assertEquals(sessionId, sessionRegistry
+				.getSessionInformation(sessionId).getSessionId());
+		assertNotNull(sessionRegistry.getSessionInformation(sessionId)
+				.getLastRequest());
+
+		// Retrieve existing session by principal
+		assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
+
+		// Sleep to ensure SessionRegistryImpl will update time
+		Thread.sleep(1000);
+
+		// Update request date/time
+		sessionRegistry.refreshLastRequest(sessionId);
+
+		Date retrieved = sessionRegistry.getSessionInformation(sessionId)
+				.getLastRequest();
+		assertTrue(retrieved.after(currentDateTime));
+
+		// Check it retrieves correctly when looked up via principal
+		assertEquals(retrieved, sessionRegistry
+				.getAllSessions(principal, false)[0].getLastRequest());
+
+		// Clear session information
+		sessionRegistry.removeSessionInformation(sessionId);
+
+		// Check attempts to retrieve cleared session return null
+		assertNull(sessionRegistry.getSessionInformation(sessionId));
+		assertNull(sessionRegistry.getAllSessions(principal, false));
+	}
+
+	public void testTwoSessionsOnePrincipalExpiring() throws Exception {
+		Object principal = "Some principal object";
+		String sessionId1 = "1234567890";
+		String sessionId2 = "9876543210";
+		SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
+
+		// Register new Session
+		sessionRegistry.registerNewSession(sessionId1, principal);
+		assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
+		assertEquals(sessionId1, sessionRegistry.getAllSessions(principal,
+				false)[0].getSessionId());
+
+		// Register new Session
+		sessionRegistry.registerNewSession(sessionId2, principal);
+		assertEquals(2, sessionRegistry.getAllSessions(principal, false).length);
+		assertEquals(sessionId2, sessionRegistry.getAllSessions(principal,
+				false)[1].getSessionId());
+
+		// Expire one session
+		SessionInformation session = sessionRegistry
+				.getSessionInformation(sessionId2);
+		session.expireNow();
+
+		// Check retrieval still correct
+		assertTrue(sessionRegistry.getSessionInformation(sessionId2)
+				.isExpired());
+		assertFalse(sessionRegistry.getSessionInformation(sessionId1)
+				.isExpired());
+	}
+
+	public void testTwoSessionsOnePrincipalHandling() throws Exception {
+		Object principal = "Some principal object";
+		String sessionId1 = "1234567890";
+		String sessionId2 = "9876543210";
+		SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
+
+		// Register new Session
+		sessionRegistry.registerNewSession(sessionId1, principal);
+		assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
+		assertEquals(sessionId1, sessionRegistry.getAllSessions(principal,
+				false)[0].getSessionId());
+
+		// Register new Session
+		sessionRegistry.registerNewSession(sessionId2, principal);
+		assertEquals(2, sessionRegistry.getAllSessions(principal, false).length);
+		assertEquals(sessionId2, sessionRegistry.getAllSessions(principal,
+				false)[1].getSessionId());
+
+		// Clear session information
+		sessionRegistry.removeSessionInformation(sessionId1);
+		assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
+		assertEquals(sessionId2, sessionRegistry.getAllSessions(principal,
+				false)[0].getSessionId());
+
+		// Clear final session
+		sessionRegistry.removeSessionInformation(sessionId2);
+		assertNull(sessionRegistry.getSessionInformation(sessionId2));
+		assertNull(sessionRegistry.getAllSessions(principal, false));
+	}
 
-        // Register new Session
-        sessionRegistry.registerNewSession(sessionId, principal);
-
-        // Deregister session via an ApplicationEvent
-        sessionRegistry.onApplicationEvent(new HttpSessionDestroyedEvent(httpSession));
-
-        // Check attempts to retrieve cleared session return null
-        assertNull(sessionRegistry.getSessionInformation(sessionId));
-    }
-
-    public void testMultiplePrincipals() throws Exception {
-        Object principal1 = "principal_1";
-        Object principal2 = "principal_2";
-        String sessionId1 = "1234567890";
-        String sessionId2 = "9876543210";
-        String sessionId3 = "5432109876";
-
-        SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
-
-        sessionRegistry.registerNewSession(sessionId1, principal1);
-        sessionRegistry.registerNewSession(sessionId2, principal1);
-        sessionRegistry.registerNewSession(sessionId3, principal2);
-
-        assertEquals(principal1, sessionRegistry.getAllPrincipals()[0]);
-        assertEquals(principal2, sessionRegistry.getAllPrincipals()[1]);
-    }
-
-    public void testSessionInformationLifecycle() throws Exception {
-        Object principal = "Some principal object";
-        String sessionId = "1234567890";
-        SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
-
-        // Register new Session
-        sessionRegistry.registerNewSession(sessionId, principal);
-
-        // Retrieve existing session by session ID
-        Date currentDateTime = sessionRegistry.getSessionInformation(sessionId).getLastRequest();
-        assertEquals(principal, sessionRegistry.getSessionInformation(sessionId).getPrincipal());
-        assertEquals(sessionId, sessionRegistry.getSessionInformation(sessionId).getSessionId());
-        assertNotNull(sessionRegistry.getSessionInformation(sessionId).getLastRequest());
-
-        // Retrieve existing session by principal
-        assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
-
-        // Sleep to ensure SessionRegistryImpl will update time
-        Thread.sleep(1000);
-
-        // Update request date/time
-        sessionRegistry.refreshLastRequest(sessionId);
-
-        Date retrieved = sessionRegistry.getSessionInformation(sessionId).getLastRequest();
-        assertTrue(retrieved.after(currentDateTime));
-
-        // Check it retrieves correctly when looked up via principal
-        assertEquals(retrieved, sessionRegistry.getAllSessions(principal, false)[0].getLastRequest());
-
-        // Clear session information
-        sessionRegistry.removeSessionInformation(sessionId);
-
-        // Check attempts to retrieve cleared session return null
-        assertNull(sessionRegistry.getSessionInformation(sessionId));
-        assertNull(sessionRegistry.getAllSessions(principal, false));
-    }
-
-    public void testTwoSessionsOnePrincipalExpiring() throws Exception {
-        Object principal = "Some principal object";
-        String sessionId1 = "1234567890";
-        String sessionId2 = "9876543210";
-        SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
-
-        // Register new Session
-        sessionRegistry.registerNewSession(sessionId1, principal);
-        assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
-        assertEquals(sessionId1, sessionRegistry.getAllSessions(principal, false)[0].getSessionId());
-
-        // Register new Session
-        sessionRegistry.registerNewSession(sessionId2, principal);
-        assertEquals(2, sessionRegistry.getAllSessions(principal, false).length);
-        assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, false)[1].getSessionId());
-
-        // Expire one session
-        SessionInformation session = sessionRegistry.getSessionInformation(sessionId2);
-        session.expireNow();
-
-        // Check retrieval still correct
-        assertTrue(sessionRegistry.getSessionInformation(sessionId2).isExpired());
-        assertFalse(sessionRegistry.getSessionInformation(sessionId1).isExpired());
-    }
-
-    public void testTwoSessionsOnePrincipalHandling() throws Exception {
-        Object principal = "Some principal object";
-        String sessionId1 = "1234567890";
-        String sessionId2 = "9876543210";
-        SessionRegistryImpl sessionRegistry = new SessionRegistryImpl();
-
-        // Register new Session
-        sessionRegistry.registerNewSession(sessionId1, principal);
-        assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
-        assertEquals(sessionId1, sessionRegistry.getAllSessions(principal, false)[0].getSessionId());
-
-        // Register new Session
-        sessionRegistry.registerNewSession(sessionId2, principal);
-        assertEquals(2, sessionRegistry.getAllSessions(principal, false).length);
-        assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, false)[1].getSessionId());
-
-        // Clear session information
-        sessionRegistry.removeSessionInformation(sessionId1);
-        assertEquals(1, sessionRegistry.getAllSessions(principal, false).length);
-        assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, false)[0].getSessionId());
-
-        // Clear final session
-        sessionRegistry.removeSessionInformation(sessionId2);
-        assertNull(sessionRegistry.getSessionInformation(sessionId2));
-        assertNull(sessionRegistry.getAllSessions(principal, false));
-    }
 }