Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

WebSessionStore updates lastAccessTime on retrieve

Browse Source 
Now that WebSessionStore is in charge of expiration checks on retrieve
it makes sense to also update the lastAccessTime on retrieve at the
same time, saving the need to call it after a retrieve.

Issue: SPR-15963
This commit is contained in:
Rossen Stoyanchev
2017-09-26 23:11:25 -04:00
parent cb2deccb2d
commit 6da3518a66
4 changed files with 49 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java
View File

@@ -78,11 +78,9 @@ public class DefaultWebSessionManager implements WebSessionManager {
@Override
public Mono<WebSession> getSession(ServerWebExchange exchange) {
return Mono.defer(() ->
retrieveSession(exchange)
.flatMap(this.getSessionStore()::updateLastAccessTime)
.switchIfEmpty(this.sessionStore.createWebSession())
.doOnNext(session -> exchange.getResponse().beforeCommit(() -> save(exchange, session))));
return Mono.defer(() -> retrieveSession(exchange)
.switchIfEmpty(this.sessionStore.createWebSession())
.doOnNext(session -> exchange.getResponse().beforeCommit(() -> save(exchange, session))));
}
private Mono<WebSession> retrieveSession(ServerWebExchange exchange) {
@@ -93,11 +91,7 @@ public class DefaultWebSessionManager implements WebSessionManager {
private Mono<Void> save(ServerWebExchange exchange, WebSession session) {
if (session.isExpired()) {
return Mono.error(new IllegalStateException(
"Sessions are checked for expiration and have their " +
"lastAccessTime updated when first accessed during request processing. " +
"However this session is expired meaning that maxIdleTime elapsed " +
"before the call to session.save()."));
return Mono.error(new IllegalStateException("Session='" + session.getId() + "' expired."));
}
if (!session.isStarted()) {

52
spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java
View File

@@ -45,7 +45,7 @@ public class InMemoryWebSessionStore implements WebSessionStore {
private Clock clock = Clock.system(ZoneId.of("GMT"));
private final Map<String, WebSession> sessions = new ConcurrentHashMap<>();
private final Map<String, InMemoryWebSession> sessions = new ConcurrentHashMap<>();
/**
@@ -77,7 +77,7 @@ public class InMemoryWebSessionStore implements WebSessionStore {
@Override
public Mono<WebSession> retrieveSession(String id) {
WebSession session = this.sessions.get(id);
InMemoryWebSession session = this.sessions.get(id);
if (session == null) {
return Mono.empty();
}
@@ -86,6 +86,7 @@ public class InMemoryWebSessionStore implements WebSessionStore {
return Mono.empty();
}
else {
session.updateLastAccessTime();
return Mono.just(session);
}
}
@@ -98,27 +99,14 @@ public class InMemoryWebSessionStore implements WebSessionStore {
public Mono<WebSession> updateLastAccessTime(WebSession webSession) {
return Mono.fromSupplier(() -> {
Assert.isInstanceOf(InMemoryWebSession.class, webSession);
InMemoryWebSession session = (InMemoryWebSession) webSession;
Instant lastAccessTime = Instant.now(getClock());
return new InMemoryWebSession(session, lastAccessTime);
session.updateLastAccessTime();
return session;
});
}
// Private methods for InMemoryWebSession
private Mono<Void> changeSessionId(String oldId, WebSession session) {
this.sessions.remove(oldId);
this.sessions.put(session.getId(), session);
return Mono.empty();
}
private Mono<Void> storeSession(WebSession session) {
this.sessions.put(session.getId(), session);
return Mono.empty();
}
private class InMemoryWebSession implements WebSession {
private final AtomicReference<String> id;
@@ -127,12 +115,13 @@ public class InMemoryWebSessionStore implements WebSessionStore {
private final Instant creationTime;
private final Instant lastAccessTime;
private volatile Instant lastAccessTime;
private volatile Duration maxIdleTime;
private volatile boolean started;
InMemoryWebSession() {
this.id = new AtomicReference<>(String.valueOf(idGenerator.generateId()));
this.attributes = new ConcurrentHashMap<>();
@@ -141,14 +130,6 @@ public class InMemoryWebSessionStore implements WebSessionStore {
this.maxIdleTime = Duration.ofMinutes(30);
}
InMemoryWebSession(InMemoryWebSession existingSession, Instant lastAccessTime) {
this.id = existingSession.id;
this.attributes = existingSession.attributes;
this.creationTime = existingSession.creationTime;
this.lastAccessTime = lastAccessTime;
this.maxIdleTime = existingSession.maxIdleTime;
this.started = existingSession.isStarted(); // Use method (explicit or implicit start)
}
@Override
public String getId() {
@@ -192,15 +173,22 @@ public class InMemoryWebSessionStore implements WebSessionStore {
@Override
public Mono<Void> changeSessionId() {
String oldId = this.id.get();
String currentId = this.id.get();
if (InMemoryWebSessionStore.this.sessions.remove(currentId) == null) {
return Mono.error(new IllegalStateException(
"Failed to change session id: " + currentId +
" because the Session is no longer present in the store."));
}
String newId = String.valueOf(idGenerator.generateId());
this.id.set(newId);
return InMemoryWebSessionStore.this.changeSessionId(oldId, this).doOnError(ex -> this.id.set(oldId));
InMemoryWebSessionStore.this.sessions.put(this.getId(), this);
return Mono.empty();
}
@Override
public Mono<Void> save() {
return InMemoryWebSessionStore.this.storeSession(this);
InMemoryWebSessionStore.this.sessions.put(this.getId(), this);
return Mono.empty();
}
@Override
@@ -208,6 +196,10 @@ public class InMemoryWebSessionStore implements WebSessionStore {
return (isStarted() && !this.maxIdleTime.isNegative() &&
Instant.now(getClock()).minus(this.maxIdleTime).isAfter(this.lastAccessTime));
}
private void updateLastAccessTime() {
this.lastAccessTime = Instant.now(getClock());
}
}
}

3
spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java
View File

@@ -41,7 +41,8 @@ public interface WebSessionStore {
/**
* Return the WebSession for the given id.
* <p><strong>Note:</strong> This method should perform an expiration check,
* remove the session if it has expired and return empty.
* and if it has expired remove the session and return empty. This method
* should also update the lastAccessTime of retrieved sessions.
* @param sessionId the session to load
* @return the session, or an empty {@code Mono} .
*/

44
spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java
View File

@@ -17,6 +17,7 @@ package org.springframework.web.server.session;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import org.junit.Test;
@@ -28,7 +29,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
/**
* Unit tests.
* Unit tests for {@link InMemoryWebSessionStore}.
* @author Rob Winch
*/
public class InMemoryWebSessionStoreTests {
@@ -36,28 +37,6 @@ public class InMemoryWebSessionStoreTests {
private InMemoryWebSessionStore store = new InMemoryWebSessionStore();
@Test
public void constructorWhenImplicitStartCopiedThenCopyIsStarted() {
WebSession original = this.store.createWebSession().block();
assertNotNull(original);
original.getAttributes().put("foo", "bar");
WebSession copy = this.store.updateLastAccessTime(original).block();
assertNotNull(copy);
assertTrue(copy.isStarted());
}
@Test
public void constructorWhenExplicitStartCopiedThenCopyIsStarted() {
WebSession original = this.store.createWebSession().block();
assertNotNull(original);
original.start();
WebSession copy = this.store.updateLastAccessTime(original).block();
assertNotNull(copy);
assertTrue(copy.isStarted());
}
@Test
public void startsSessionExplicitly() {
WebSession session = this.store.createWebSession().block();
@@ -87,8 +66,27 @@ public class InMemoryWebSessionStoreTests {
assertNotNull(retrieved);
assertSame(session, retrieved);
// Fast-forward 31 minutes
this.store.setClock(Clock.offset(this.store.getClock(), Duration.ofMinutes(31)));
WebSession retrievedAgain = this.store.retrieveSession(id).block();
assertNull(retrievedAgain);
}
@Test
public void lastAccessTimeIsUpdatedOnRetrieve() throws Exception {
WebSession session1 = this.store.createWebSession().block();
assertNotNull(session1);
String id = session1.getId();
Instant time1 = session1.getLastAccessTime();
session1.save();
// Fast-forward a few seconds
this.store.setClock(Clock.offset(this.store.getClock(), Duration.ofSeconds(5)));
WebSession session2 = this.store.retrieveSession(id).block();
assertNotNull(session2);
assertSame(session1, session2);
Instant time2 = session2.getLastAccessTime();
assertTrue(time1.isBefore(time2));
}
}