From ee09a9e863ad28eae559d04dd00451773177aa92 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 17 Nov 2015 10:47:42 -0600 Subject: [PATCH] Add ExpiringSession setLastAccessedTime Previously SessionRepository had to update the lastAccessTime when it was loaded. This prevented inspecting the last access time. For example, listing all the sessions for a specific user. Furthermore, it is unintuitive that a read operation would update attributes on the domain model. This change introduces ExpiringSession setLastAccessedTime to allow setting the expiration on the interface. This means that the SessionRepositoryFilter can update the last accessed time. Fixes gh-272 --- .../springframework/session/ExpiringSession.java | 7 +++++++ .../session/MapSessionRepository.java | 4 +--- .../springframework/session/SessionRepository.java | 6 ------ .../redis/RedisOperationsSessionRepository.java | 2 -- .../session/web/http/SessionRepositoryFilter.java | 13 +++++++++++-- .../springframework/session/MapSessionTests.java | 4 ++++ .../RedisOperationsSessionRepositoryTests.java | 2 +- .../web/http/SessionRepositoryFilterTests.java | 2 +- 8 files changed, 25 insertions(+), 15 deletions(-) diff --git a/spring-session/src/main/java/org/springframework/session/ExpiringSession.java b/spring-session/src/main/java/org/springframework/session/ExpiringSession.java index d371330..7e9e8e7 100644 --- a/spring-session/src/main/java/org/springframework/session/ExpiringSession.java +++ b/spring-session/src/main/java/org/springframework/session/ExpiringSession.java @@ -30,6 +30,13 @@ public interface ExpiringSession extends Session { */ long getCreationTime(); + /** + * Sets the last accessed time in milliseconds since midnight of 1/1/1970 GMT + * + * @param lastAccessedTime the last accessed time in milliseconds since midnight of 1/1/1970 GMT + */ + void setLastAccessedTime(long lastAccessedTime); + /** * Gets the last time this {@link Session} was accessed expressed in milliseconds since midnight of 1/1/1970 GMT * diff --git a/spring-session/src/main/java/org/springframework/session/MapSessionRepository.java b/spring-session/src/main/java/org/springframework/session/MapSessionRepository.java index 88e3e7f..15a96f4 100644 --- a/spring-session/src/main/java/org/springframework/session/MapSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/MapSessionRepository.java @@ -81,9 +81,7 @@ public class MapSessionRepository implements SessionRepository delete(saved.getId()); return null; } - MapSession result = new MapSession(saved); - result.setLastAccessedTime(System.currentTimeMillis()); - return result; + return new MapSession(saved); } public void delete(String id) { diff --git a/spring-session/src/main/java/org/springframework/session/SessionRepository.java b/spring-session/src/main/java/org/springframework/session/SessionRepository.java index 66da4f1..c8ea352 100644 --- a/spring-session/src/main/java/org/springframework/session/SessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/SessionRepository.java @@ -49,12 +49,6 @@ public interface SessionRepository { /** * Gets the {@link Session} by the {@link Session#getId()} or null if no {@link Session} is found. * - *

- * If the {@link Session} extends {@link ExpiringSession}, then {@link ExpiringSession#getLastAccessedTime()} will be - * updated on the returned object. In order to persist this change, {@link #save(Session)} must be invoked on the returned - * instance. - *

- * * @param id the {@link org.springframework.session.Session#getId()} to lookup * @return the {@link Session} by the {@link Session#getId()} or null if no {@link Session} is found. */ diff --git a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java index c838469..3d85438 100644 --- a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java @@ -365,7 +365,6 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess for(Object id : sessionIds) { RedisSession session = getSession((String) id); if(session != null) { - session.setLastAccessedTime(session.originalLastAccessTime); sessions.put(session.getId(), session); } } @@ -392,7 +391,6 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess } RedisSession result = new RedisSession(loaded); result.originalLastAccessTime = loaded.getLastAccessedTime(); - result.setLastAccessedTime(System.currentTimeMillis()); return result; } diff --git a/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java b/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java index 39d6a32..a5b298c 100644 --- a/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java +++ b/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java @@ -242,7 +242,7 @@ public class SessionRepositoryFilter extends OncePerR public boolean isRequestedSessionIdValid() { if(requestedSessionIdValid == null) { String sessionId = getRequestedSessionId(); - S session = sessionId == null ? null : sessionRepository.getSession(sessionId); + S session = sessionId == null ? null : getSession(sessionId); return isRequestedSessionIdValid(session); } @@ -260,6 +260,15 @@ public class SessionRepositoryFilter extends OncePerR return getCurrentSession() == null && requestedSessionInvalidated; } + private S getSession(String sessionId) { + S session = sessionRepository.getSession(sessionId); + if(session == null) { + return null; + } + session.setLastAccessedTime(System.currentTimeMillis()); + return session; + } + @Override public HttpSessionWrapper getSession(boolean create) { HttpSessionWrapper currentSession = getCurrentSession(); @@ -268,7 +277,7 @@ public class SessionRepositoryFilter extends OncePerR } String requestedSessionId = getRequestedSessionId(); if(requestedSessionId != null) { - S session = sessionRepository.getSession(requestedSessionId); + S session = getSession(requestedSessionId); if(session != null) { this.requestedSessionIdValid = true; currentSession = new HttpSessionWrapper(session, getServletContext()); diff --git a/spring-session/src/test/java/org/springframework/session/MapSessionTests.java b/spring-session/src/test/java/org/springframework/session/MapSessionTests.java index 42873f5..811c9cb 100644 --- a/spring-session/src/test/java/org/springframework/session/MapSessionTests.java +++ b/spring-session/src/test/java/org/springframework/session/MapSessionTests.java @@ -94,6 +94,10 @@ public class MapSessionTests { return "id"; } + public void setLastAccessedTime(long lastAccessedTime) { + throw new UnsupportedOperationException(); + } + public long getLastAccessedTime() { return 0; } diff --git a/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java b/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java index 603a487..80574d4 100644 --- a/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java +++ b/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java @@ -319,7 +319,7 @@ public class RedisOperationsSessionRepositoryTests { assertThat(session.getAttribute(attrName)).isEqualTo(expected.getAttribute(attrName)); assertThat(session.getCreationTime()).isEqualTo(expected.getCreationTime()); assertThat(session.getMaxInactiveIntervalInSeconds()).isEqualTo(expected.getMaxInactiveIntervalInSeconds()); - assertThat(session.getLastAccessedTime()).isGreaterThanOrEqualTo(now); + assertThat(session.getLastAccessedTime()).isEqualTo(expected.getLastAccessedTime()); } diff --git a/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java b/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java index e8dce10..5a60881 100644 --- a/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java +++ b/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java @@ -126,7 +126,7 @@ public class SessionRepositoryFilterTests { } }); - Thread.sleep(10L); + Thread.sleep(1L); nextRequest(); doFilter(new DoInFilter() {