Polish WebSession support code

spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java

@@ -87,8 +87,17 @@ public class CookieWebSessionIdResolver implements WebSessionIdResolver {
 
 	@Override
 	public void setSessionId(ServerWebExchange exchange, String id) {
+		Assert.notNull(id, "'id' is required");
+		setSessionCookie(exchange, id, getCookieMaxAge());
+	}
+
+	@Override
+	public void expireSession(ServerWebExchange exchange) {
+		setSessionCookie(exchange, "", Duration.ofSeconds(0));
+	}
+
+	private void setSessionCookie(ServerWebExchange exchange, String id, Duration maxAge) {
 		String name = getCookieName();
-		Duration maxAge = (StringUtils.hasText(id) ? getCookieMaxAge() : Duration.ofSeconds(0));
 		boolean secure = "https".equalsIgnoreCase(exchange.getRequest().getURI().getScheme());
 		MultiValueMap<String, ResponseCookie> cookieMap = exchange.getResponse().getCookies();
 		cookieMap.set(name, ResponseCookie.from(name, id).maxAge(maxAge).httpOnly(true).secure(secure).build());

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

@@ -30,8 +30,9 @@ import org.springframework.web.server.WebSession;
 
 
 /**
- * Default implementation of {@link WebSessionManager} with a cookie-based web
- * session id resolution strategy and simple in-memory session persistence.
+ * Default implementation of {@link WebSessionManager} delegating to a
+ * {@link WebSessionIdResolver} for session id resolution and to a
+ * {@link WebSessionStore}
  *
  * @author Rossen Stoyanchev
  * @since 5.0
@@ -46,12 +47,12 @@ public class DefaultWebSessionManager implements WebSessionManager {
 
 
 	/**
-	 * Configure the session id resolution strategy to use.
-	 * <p>By default {@link CookieWebSessionIdResolver} is used.
-	 * @param sessionIdResolver the resolver
+	 * Configure the id resolution strategy.
+	 * <p>By default an instance of {@link CookieWebSessionIdResolver}.
+	 * @param sessionIdResolver the resolver to use
 	 */
 	public void setSessionIdResolver(WebSessionIdResolver sessionIdResolver) {
-		Assert.notNull(sessionIdResolver, "'sessionIdResolver' is required.");
+		Assert.notNull(sessionIdResolver, "WebSessionIdResolver is required.");
 		this.sessionIdResolver = sessionIdResolver;
 	}
 
@@ -63,12 +64,12 @@ public class DefaultWebSessionManager implements WebSessionManager {
 	}
 
 	/**
-	 * Configure the session persistence strategy to use.
-	 * <p>By default {@link InMemoryWebSessionStore} is used.
-	 * @param sessionStore the persistence strategy
+	 * Configure the persistence strategy.
+	 * <p>By default an instance of {@link InMemoryWebSessionStore}.
+	 * @param sessionStore the persistence strategy to use
 	 */
 	public void setSessionStore(WebSessionStore sessionStore) {
-		Assert.notNull(sessionStore, "'sessionStore' is required.");
+		Assert.notNull(sessionStore, "WebSessionStore is required.");
 		this.sessionStore = sessionStore;
 	}
 
@@ -80,10 +81,12 @@ public class DefaultWebSessionManager implements WebSessionManager {
 	}
 
 	/**
-	 * Configure the {@link Clock} for access to current time. During tests you
-	 * may use {code Clock.offset(clock, Duration.ofMinutes(-31))} to set the
-	 * clock back for example to test changes after sessions expire.
-	 * <p>By default {@code Clock.system(ZoneId.of("GMT"))} is used.
+	 * Configure the {@link Clock} to use to set lastAccessTime on every created
+	 * session and to calculate if it is expired.
+	 * <p>This may be useful to align to different timezone or to set the clock
+	 * back in a test, e.g. {@code Clock.offset(clock, Duration.ofMinutes(-31))}
+	 * in order to simulate session expiration.
+	 * <p>By default this is {@code Clock.system(ZoneId.of("GMT"))}.
 	 * @param clock the clock to use
 	 */
 	public void setClock(Clock clock) {
@@ -92,7 +95,7 @@ public class DefaultWebSessionManager implements WebSessionManager {
 	}
 
 	/**
-	 * Return the configured clock for access to current time.
+	 * Return the configured clock for session lastAccessTime calculations.
 	 */
 	public Clock getClock() {
 		return this.clock;
@@ -102,48 +105,45 @@ public class DefaultWebSessionManager implements WebSessionManager {
 	@Override
 	public Mono<WebSession> getSession(ServerWebExchange exchange) {
 		return Mono.defer(() ->
-				Flux.fromIterable(getSessionIdResolver().resolveSessionIds(exchange))
-						.concatMap(this.sessionStore::retrieveSession)
-						.next()
-						.flatMap(session -> validateSession(exchange, session))
-						.switchIfEmpty(createSession(exchange))
-						.map(session -> extendSession(exchange, session)));
+				retrieveSession(exchange)
+						.flatMap(session -> removeSessionIfExpired(exchange, session))
+						.switchIfEmpty(createSession())
+						.doOnNext(session -> {
+							if (session instanceof ConfigurableWebSession) {
+								ConfigurableWebSession configurable = (ConfigurableWebSession) session;
+								configurable.setSaveOperation(() -> saveSession(exchange, session));
+								configurable.setLastAccessTime(Instant.now(getClock()));
+							}
+							exchange.getResponse().beforeCommit(session::save);
+						}));
 	}
 
-	protected Mono<WebSession> validateSession(ServerWebExchange exchange, WebSession session) {
+	private Mono<WebSession> retrieveSession(ServerWebExchange exchange) {
+		return Flux.fromIterable(getSessionIdResolver().resolveSessionIds(exchange))
+				.concatMap(this.sessionStore::retrieveSession)
+				.next();
+	}
+
+	private Mono<WebSession> removeSessionIfExpired(ServerWebExchange exchange, WebSession session) {
 		if (session.isExpired()) {
 			this.sessionIdResolver.setSessionId(exchange, "");
-			return this.sessionStore.removeSession(session.getId()).cast(WebSession.class);
-		}
-		else {
-			return Mono.just(session);
+			return this.sessionStore.removeSession(session.getId()).then(Mono.empty());
 		}
-	}
-
-	protected Mono<WebSession> createSession(ServerWebExchange exchange) {
-		String sessionId = UUID.randomUUID().toString();
-		WebSession session = new DefaultWebSession(sessionId, getClock());
 		return Mono.just(session);
 	}
 
-	protected WebSession extendSession(ServerWebExchange exchange, WebSession session) {
-		if (session instanceof ConfigurableWebSession) {
-			ConfigurableWebSession managed = (ConfigurableWebSession) session;
-			managed.setSaveOperation(() -> saveSession(exchange, session));
-			managed.setLastAccessTime(Instant.now(getClock()));
-		}
-		exchange.getResponse().beforeCommit(session::save);
-		return session;
+	private Mono<DefaultWebSession> createSession() {
+		return Mono.fromSupplier(() ->
+				new DefaultWebSession(UUID.randomUUID().toString(), getClock()));
 	}
 
-	protected Mono<Void> saveSession(ServerWebExchange exchange, WebSession session) {
-
+	private Mono<Void> saveSession(ServerWebExchange exchange, WebSession session) {
 		if (session.isExpired()) {
 			return Mono.error(new IllegalStateException(
 					"Sessions are checked for expiration and have their " +
-					"access time updated when first accessed during request processing. " +
+					"lastAccessTime updated when first accessed during request processing. " +
 					"However this session is expired meaning that maxIdleTime elapsed " +
-					"since then and before the call to session.save()."));
+					"before the call to session.save()."));
 		}
 
 		if (!session.isStarted()) {
@@ -153,11 +153,16 @@ public class DefaultWebSessionManager implements WebSessionManager {
 		// Force explicit start
 		session.start();
 
-		List<String> requestedIds = getSessionIdResolver().resolveSessionIds(exchange);
-		if (requestedIds.isEmpty() || !session.getId().equals(requestedIds.get(0))) {
+		if (hasNewSessionId(exchange, session)) {
 			this.sessionIdResolver.setSessionId(exchange, session.getId());
 		}
-		return this.sessionStore.storeSession(session);
+
+ 		return this.sessionStore.storeSession(session);
+	}
+
+	private boolean hasNewSessionId(ServerWebExchange exchange, WebSession session) {
+		List<String> ids = getSessionIdResolver().resolveSessionIds(exchange);
+		return ids.isEmpty() || !session.getId().equals(ids.get(0));
 	}
 
 }

spring-web/src/main/java/org/springframework/web/server/session/WebSessionIdResolver.java

@@ -22,8 +22,8 @@ import org.springframework.web.server.ServerWebExchange;
 
 /**
  * Contract for session id resolution strategies. Allows for session id
- * resolution through the request and for sending the session id to the
- * client through the response.
+ * resolution through the request and for sending the session id or expiring
+ * the session through the response.
  *
  * @author Rossen Stoyanchev
  * @since 5.0
@@ -39,11 +39,16 @@ public interface WebSessionIdResolver {
 	List<String> resolveSessionIds(ServerWebExchange exchange);
 
 	/**
-	 * Send the given session id to the client or if the session id is "null"
-	 * instruct the client to end the current session.
+	 * Send the given session id to the client.
 	 * @param exchange the current exchange
 	 * @param sessionId the session id
 	 */
 	void setSessionId(ServerWebExchange exchange, String sessionId);
 
+	/**
+	 * Instruct the client to end the current session.
+	 * @param exchange the current exchange
+	 */
+	void expireSession(ServerWebExchange exchange);
+
 }

spring-web/src/main/java/org/springframework/web/server/session/WebSessionManager.java

@@ -21,13 +21,7 @@ import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.WebSession;
 
 /**
- * Main contract abstracting support for access to {@link WebSession} instances
- * associated with HTTP requests as well as the subsequent management such as
- * persistence and others.
- *
- * <p>The {@link DefaultWebSessionManager} implementation in turn delegates to
- * {@link WebSessionIdResolver} and {@link WebSessionStore} which abstract
- * underlying concerns related to the management of web sessions.
+ * Main class for for access to the {@link WebSession} for an HTTP request.
  *
  * @author Rossen Stoyanchev
  * @since 5.0
@@ -39,10 +33,10 @@ public interface WebSessionManager {
 	/**
 	 * Return the {@link WebSession} for the given exchange. Always guaranteed
 	 * to return an instance either matching to the session id requested by the
-	 * client, or with a new session id either because the client did not
-	 * specify one or because the underlying session had expired.
+	 * client, or a new session either because the client did not specify one
+	 * or because the underlying session expired.
 	 * @param exchange the current exchange
-	 * @return {@code Mono} for async access to the session
+	 * @return promise for the WebSession
 	 */
 	Mono<WebSession> getSession(ServerWebExchange exchange);
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,23 +28,23 @@ import org.springframework.web.server.WebSession;
 public interface WebSessionStore {
 
 	/**
-	 * Store the given session.
+	 * Store the given WebSession.
 	 * @param session the session to store
-	 * @return {@code Mono} for completion notification
+	 * @return a completion notification (success or error)
 	 */
 	Mono<Void> storeSession(WebSession session);
 
 	/**
-	 * Load the session for the given session id.
+	 * Return the WebSession for the given id.
 	 * @param sessionId the session to load
-	 * @return {@code Mono} for async access to the loaded session
+	 * @return the session, or an empty {@code Mono}.
 	 */
 	Mono<WebSession> retrieveSession(String sessionId);
 
 	/**
-	 * Remove the session with the given id.
-	 * @param sessionId the session to remove
-	 * @return {@code Mono} for completion notification
+	 * Remove the WebSession for the specified id.
+	 * @param sessionId the id of the session to remove
+	 * @return a completion notification (success or error)
 	 */
 	Mono<Void> removeSession(String sessionId);
 

spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java

@@ -27,6 +27,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.http.codec.ServerCodecConfigurer;
+import org.springframework.lang.Nullable;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
 import org.springframework.web.server.ServerWebExchange;
@@ -139,6 +140,7 @@ public class DefaultWebSessionManagerTests {
 
 		private List<String> idsToResolve = new ArrayList<>();
 
+		@Nullable
 		private String id = null;
 
 
@@ -146,6 +148,7 @@ public class DefaultWebSessionManagerTests {
 			this.idsToResolve = idsToResolve;
 		}
 
+		@Nullable
 		public String getSavedId() {
 			return this.id;
 		}
@@ -159,6 +162,11 @@ public class DefaultWebSessionManagerTests {
 		public void setSessionId(ServerWebExchange exchange, String sessionId) {
 			this.id = sessionId;
 		}
+
+		@Override
+		public void expireSession(ServerWebExchange exchange) {
+			this.id = null;
+		}
 	}
 
 }