From bf712957f6c5329e948e90bfe03f4f1c8ab124c9 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 14 Jul 2017 22:10:31 +0200 Subject: [PATCH] Polish WebSession support code --- .../session/CookieWebSessionIdResolver.java | 11 ++- .../session/DefaultWebSessionManager.java | 97 ++++++++++--------- .../server/session/WebSessionIdResolver.java | 13 ++- .../web/server/session/WebSessionManager.java | 14 +-- .../web/server/session/WebSessionStore.java | 16 +-- .../DefaultWebSessionManagerTests.java | 8 ++ 6 files changed, 90 insertions(+), 69 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java b/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java index 32984c0eb9..738d07351b 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java +++ b/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 cookieMap = exchange.getResponse().getCookies(); cookieMap.set(name, ResponseCookie.from(name, id).maxAge(maxAge).httpOnly(true).secure(secure).build()); diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java index 18b128d464..b2f0d95327 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java +++ b/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. - *

By default {@link CookieWebSessionIdResolver} is used. - * @param sessionIdResolver the resolver + * Configure the id resolution strategy. + *

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. - *

By default {@link InMemoryWebSessionStore} is used. - * @param sessionStore the persistence strategy + * Configure the persistence strategy. + *

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. - *

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. + *

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. + *

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 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 validateSession(ServerWebExchange exchange, WebSession session) { + private Mono retrieveSession(ServerWebExchange exchange) { + return Flux.fromIterable(getSessionIdResolver().resolveSessionIds(exchange)) + .concatMap(this.sessionStore::retrieveSession) + .next(); + } + + private Mono removeSessionIfExpired(ServerWebExchange exchange, WebSession session) { if (session.isExpired()) { this.sessionIdResolver.setSessionId(exchange, ""); - return this.sessionStore.removeSession(session.getId()).cast(WebSession.class); + return this.sessionStore.removeSession(session.getId()).then(Mono.empty()); } - else { - return Mono.just(session); - } - } - - protected Mono 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 createSession() { + return Mono.fromSupplier(() -> + new DefaultWebSession(UUID.randomUUID().toString(), getClock())); } - protected Mono saveSession(ServerWebExchange exchange, WebSession session) { - + private Mono 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 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 ids = getSessionIdResolver().resolveSessionIds(exchange); + return ids.isEmpty() || !session.getId().equals(ids.get(0)); } } diff --git a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionIdResolver.java b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionIdResolver.java index b6f7a0939e..c7f1536166 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionIdResolver.java +++ b/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 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); + } diff --git a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionManager.java b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionManager.java index 1882592da9..2286590014 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionManager.java +++ b/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. - * - *

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 getSession(ServerWebExchange exchange); diff --git a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java index 2b80efdf24..e6461c311d 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java +++ b/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 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 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 removeSession(String sessionId); diff --git a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java index aa855a8e43..65373f0986 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java +++ b/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 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; + } } }