From f49b7639a7e479ca1780c66fc28ee910d835d8f4 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Thu, 20 Jul 2023 15:27:05 +0800 Subject: [PATCH] Avoid to retain SessionIdGenerator in session object session id generation shouldn't be the responsibility of session object, since SessionIdGenerator held by session object is only used by changeSessionId(), we could introduce overload method passing it as parameter to avoid that. --- .../springframework/session/MapSession.java | 33 +++---------- .../session/MapSessionRepository.java | 11 ++--- .../session/ReactiveMapSessionRepository.java | 2 - .../org/springframework/session/Session.java | 21 ++++++-- .../session/SessionIdGenerator.java | 16 ++++++ .../http/SpringHttpSessionConfiguration.java | 12 ++++- .../server/SpringWebSessionConfiguration.java | 14 +++++- .../web/http/SessionRepositoryFilter.java | 17 ++++++- .../session/SpringSessionWebSessionStore.java | 13 +++-- .../session/MapSessionTests.java | 17 +------ .../ReactiveMapSessionRepositoryTests.java | 21 ++++++-- .../http/SessionRepositoryFilterTests.java | 15 +++++- .../SpringSessionWebSessionStoreTests.java | 5 +- .../mongo/MongoIndexedSessionRepository.java | 5 +- .../session/data/mongo/MongoSession.java | 49 +++---------------- .../mongo/ReactiveMongoSessionRepository.java | 6 +-- .../MongoIndexedSessionRepositoryTest.java | 24 +++++++-- .../ReactiveMongoSessionRepositoryTest.java | 12 +++-- .../redis/ReactiveRedisSessionRepository.java | 5 +- .../redis/RedisIndexedSessionRepository.java | 7 +-- .../data/redis/RedisSessionRepository.java | 6 +-- .../ReactiveRedisSessionRepositoryTests.java | 12 +++-- .../RedisIndexedSessionRepositoryTests.java | 21 ++++++-- .../redis/RedisSessionRepositoryTests.java | 23 +++++++-- .../HazelcastIndexedSessionRepository.java | 7 +-- ...azelcastIndexedSessionRepositoryTests.java | 23 +++++++-- .../jdbc/JdbcIndexedSessionRepository.java | 12 ++--- .../JdbcIndexedSessionRepositoryTests.java | 22 +++++++-- 28 files changed, 265 insertions(+), 166 deletions(-) diff --git a/spring-session-core/src/main/java/org/springframework/session/MapSession.java b/spring-session-core/src/main/java/org/springframework/session/MapSession.java index eed90b841..6d35a943f 100644 --- a/spring-session-core/src/main/java/org/springframework/session/MapSession.java +++ b/spring-session-core/src/main/java/org/springframework/session/MapSession.java @@ -43,6 +43,7 @@ * * @author Rob Winch * @author Vedran Pavic + * @author Yanming Zhou * @since 1.0 */ public final class MapSession implements Session, Serializable { @@ -73,8 +74,6 @@ public final class MapSession implements Session, Serializable { */ private Duration maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL; - private transient SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT; - /** * Creates a new instance with identifier generated by * {@link SessionIdGenerator#DEFAULT}. @@ -83,17 +82,6 @@ public MapSession() { this(SessionIdGenerator.DEFAULT.generate()); } - /** - * Creates a new instance using the specified {@link SessionIdGenerator} to generate - * the session id. - * @param sessionIdGenerator the {@link SessionIdGenerator} to use. - * @since 3.2 - */ - public MapSession(SessionIdGenerator sessionIdGenerator) { - this(sessionIdGenerator.generate()); - this.sessionIdGenerator = sessionIdGenerator; - } - /** * Creates a new instance with the specified id. This is preferred to the default * constructor when the id is known to prevent unnecessary consumption on entropy @@ -146,17 +134,17 @@ public String getId() { /** * Get the original session id. * @return the original session id - * @see #changeSessionId() + * @see #changeSessionId(SessionIdGenerator) */ public String getOriginalId() { return this.originalId; } @Override - public String changeSessionId() { - String changedId = this.sessionIdGenerator.generate(); - setId(changedId); - return changedId; + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); + setId(newSessionId); + return newSessionId; } @Override @@ -241,15 +229,6 @@ public int hashCode() { return this.id.hashCode(); } - /** - * Sets the {@link SessionIdGenerator} to use when generating a new session id. - * @param sessionIdGenerator the {@link SessionIdGenerator} to use. - * @since 3.2 - */ - public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) { - this.sessionIdGenerator = sessionIdGenerator; - } - private static final long serialVersionUID = 7160779239673823561L; } diff --git a/spring-session-core/src/main/java/org/springframework/session/MapSessionRepository.java b/spring-session-core/src/main/java/org/springframework/session/MapSessionRepository.java index df2e44162..d0bdbb789 100644 --- a/spring-session-core/src/main/java/org/springframework/session/MapSessionRepository.java +++ b/spring-session-core/src/main/java/org/springframework/session/MapSessionRepository.java @@ -35,6 +35,7 @@ *

* * @author Rob Winch + * @author Yanming Zhou * @since 1.0 */ public class MapSessionRepository implements SessionRepository { @@ -73,9 +74,7 @@ public void save(MapSession session) { if (!session.getId().equals(session.getOriginalId())) { this.sessions.remove(session.getOriginalId()); } - MapSession saved = new MapSession(session); - saved.setSessionIdGenerator(this.sessionIdGenerator); - this.sessions.put(session.getId(), saved); + this.sessions.put(session.getId(), new MapSession(session)); } @Override @@ -88,9 +87,7 @@ public MapSession findById(String id) { deleteById(saved.getId()); return null; } - MapSession result = new MapSession(saved); - result.setSessionIdGenerator(this.sessionIdGenerator); - return result; + return new MapSession(saved); } @Override @@ -100,7 +97,7 @@ public void deleteById(String id) { @Override public MapSession createSession() { - MapSession result = new MapSession(this.sessionIdGenerator); + MapSession result = new MapSession(this.sessionIdGenerator.generate()); result.setMaxInactiveInterval(this.defaultMaxInactiveInterval); return result; } diff --git a/spring-session-core/src/main/java/org/springframework/session/ReactiveMapSessionRepository.java b/spring-session-core/src/main/java/org/springframework/session/ReactiveMapSessionRepository.java index 54956e4da..92d51c3f7 100644 --- a/spring-session-core/src/main/java/org/springframework/session/ReactiveMapSessionRepository.java +++ b/spring-session-core/src/main/java/org/springframework/session/ReactiveMapSessionRepository.java @@ -88,7 +88,6 @@ public Mono findById(String id) { return Mono.defer(() -> Mono.justOrEmpty(this.sessions.get(id)) .filter((session) -> !session.isExpired()) .map(MapSession::new) - .doOnNext((session) -> session.setSessionIdGenerator(this.sessionIdGenerator)) .switchIfEmpty(deleteById(id).then(Mono.empty()))); // @formatter:on } @@ -107,7 +106,6 @@ public Mono createSession() { .map((sessionId) -> { MapSession result = new MapSession(sessionId); result.setMaxInactiveInterval(this.defaultMaxInactiveInterval); - result.setSessionIdGenerator(this.sessionIdGenerator); return result; }); // @formatter:on diff --git a/spring-session-core/src/main/java/org/springframework/session/Session.java b/spring-session-core/src/main/java/org/springframework/session/Session.java index 6be3445cc..e143b7200 100644 --- a/spring-session-core/src/main/java/org/springframework/session/Session.java +++ b/spring-session-core/src/main/java/org/springframework/session/Session.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 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. @@ -26,6 +26,7 @@ * * @author Rob Winch * @author Vedran Pavic + * @author Yanming Zhou * @since 1.0 */ public interface Session { @@ -37,11 +38,23 @@ public interface Session { String getId(); /** - * Changes the session id. After invoking the {@link #getId()} will return a new - * identifier. + * Changes the session id with the {@link SessionIdGenerator#DEFAULT}. After invoking + * the {@link #getId()} will return a new identifier. * @return the new session id which {@link #getId()} will now return + * @deprecated use {@link #changeSessionId(SessionIdGenerator)} instead */ - String changeSessionId(); + @Deprecated + default String changeSessionId() { + return changeSessionId(SessionIdGenerator.DEFAULT); + } + + /** + * Changes the session id with the supplied {@link SessionIdGenerator}. After invoking + * the {@link #getId()} will return a new identifier. + * @param sessionIdGenerator the {@link SessionIdGenerator} to use + * @return the new session id which {@link #getId()} will now return + */ + String changeSessionId(SessionIdGenerator sessionIdGenerator); /** * Gets the Object associated with the specified name or null if no Object is diff --git a/spring-session-core/src/main/java/org/springframework/session/SessionIdGenerator.java b/spring-session-core/src/main/java/org/springframework/session/SessionIdGenerator.java index cd8057848..0ae10d7cf 100644 --- a/spring-session-core/src/main/java/org/springframework/session/SessionIdGenerator.java +++ b/spring-session-core/src/main/java/org/springframework/session/SessionIdGenerator.java @@ -22,6 +22,7 @@ * An interface for specifying a strategy for generating session identifiers. * * @author Marcus da Coregio + * @author Yanming Zhou * @since 3.2 */ public interface SessionIdGenerator { @@ -31,7 +32,22 @@ public interface SessionIdGenerator { */ SessionIdGenerator DEFAULT = UuidSessionIdGenerator.getInstance(); + /** + * Generate identifier for creating new session. + * @return the generated session identifier + */ @NonNull String generate(); + /** + * Generate identifier for changing id of existing session. + * @param session the existing {@link Session} object + * @return the generated session identifier + * @see Session#changeSessionId(SessionIdGenerator) + */ + @NonNull + default String regenerate(Session session) { + return generate(); + } + } diff --git a/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/http/SpringHttpSessionConfiguration.java b/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/http/SpringHttpSessionConfiguration.java index 74456681a..28f865993 100644 --- a/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/http/SpringHttpSessionConfiguration.java +++ b/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/http/SpringHttpSessionConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2022 the original author or authors. + * Copyright 2014-2023 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. @@ -34,6 +34,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.SessionRepository; import org.springframework.session.events.SessionCreatedEvent; import org.springframework.session.events.SessionDestroyedEvent; @@ -87,6 +88,7 @@ * * @author Rob Winch * @author Vedran Pavic + * @author Yanming Zhou * @since 1.1 * @see EnableSpringHttpSession */ @@ -107,6 +109,8 @@ public class SpringHttpSessionConfiguration implements InitializingBean, Applica private List httpSessionListeners = new ArrayList<>(); + private SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT; + @Override public void afterPropertiesSet() { CookieSerializer cookieSerializer = (this.cookieSerializer != null) ? this.cookieSerializer @@ -124,6 +128,7 @@ public SessionRepositoryFilter springSess SessionRepository sessionRepository) { SessionRepositoryFilter sessionRepositoryFilter = new SessionRepositoryFilter<>(sessionRepository); sessionRepositoryFilter.setHttpSessionIdResolver(this.httpSessionIdResolver); + sessionRepositoryFilter.setSessionIdGenerator(this.sessionIdGenerator); return sessionRepositoryFilter; } @@ -155,6 +160,11 @@ public void setHttpSessionListeners(List listeners) { this.httpSessionListeners = listeners; } + @Autowired(required = false) + public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) { + this.sessionIdGenerator = sessionIdGenerator; + } + private CookieSerializer createDefaultCookieSerializer() { DefaultCookieSerializer cookieSerializer = new DefaultCookieSerializer(); if (this.servletContext != null) { diff --git a/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/server/SpringWebSessionConfiguration.java b/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/server/SpringWebSessionConfiguration.java index 00d559977..a978e92ea 100644 --- a/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/server/SpringWebSessionConfiguration.java +++ b/spring-session-core/src/main/java/org/springframework/session/config/annotation/web/server/SpringWebSessionConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 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. @@ -21,6 +21,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.session.ReactiveSessionRepository; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.web.server.session.SpringSessionWebSessionStore; import org.springframework.web.server.adapter.WebHttpHandlerBuilder; import org.springframework.web.server.session.DefaultWebSessionManager; @@ -33,6 +34,7 @@ * * @author Greg Turnquist * @author Rob Winch + * @author Yanming Zhou * @since 2.0 * @see EnableSpringWebSession */ @@ -41,11 +43,18 @@ public class SpringWebSessionConfiguration { private WebSessionIdResolver webSessionIdResolver; + private SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT; + @Autowired(required = false) public void setWebSessionIdResolver(WebSessionIdResolver webSessionIdResolver) { this.webSessionIdResolver = webSessionIdResolver; } + @Autowired(required = false) + public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) { + this.sessionIdGenerator = sessionIdGenerator; + } + /** * Configure a {@link WebSessionManager} using a provided * {@link ReactiveSessionRepository}. @@ -55,7 +64,8 @@ public void setWebSessionIdResolver(WebSessionIdResolver webSessionIdResolver) { */ @Bean(WebHttpHandlerBuilder.WEB_SESSION_MANAGER_BEAN_NAME) public WebSessionManager webSessionManager(ReactiveSessionRepository repository) { - SpringSessionWebSessionStore sessionStore = new SpringSessionWebSessionStore<>(repository); + SpringSessionWebSessionStore sessionStore = new SpringSessionWebSessionStore<>(repository, + SpringWebSessionConfiguration.this.sessionIdGenerator); DefaultWebSessionManager manager = new DefaultWebSessionManager(); manager.setSessionStore(sessionStore); diff --git a/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java b/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java index d4a119a53..a2385abc3 100644 --- a/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java +++ b/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java @@ -36,6 +36,7 @@ import org.springframework.core.annotation.Order; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.SessionRepository; /** @@ -106,6 +107,8 @@ public class SessionRepositoryFilter extends OncePerRequestFi private HttpSessionIdResolver httpSessionIdResolver = new CookieHttpSessionIdResolver(); + private SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT; + /** * Creates a new instance. * @param sessionRepository the SessionRepository to use. Cannot be null. @@ -130,6 +133,18 @@ public void setHttpSessionIdResolver(HttpSessionIdResolver httpSessionIdResolver this.httpSessionIdResolver = httpSessionIdResolver; } + /** + * Sets the {@link SessionIdGenerator} to be used. The default is a + * {@link SessionIdGenerator#DEFAULT}. + * @param sessionIdGenerator the {@link SessionIdGenerator} to use. Cannot be null. + */ + public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) { + if (sessionIdGenerator == null) { + throw new IllegalArgumentException("sessionIdGenerator cannot be null"); + } + this.sessionIdGenerator = sessionIdGenerator; + } + @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { @@ -259,7 +274,7 @@ public String changeSessionId() { "Cannot change session ID. There is no session associated with this request."); } - return getCurrentSession().getSession().changeSessionId(); + return getCurrentSession().getSession().changeSessionId(SessionRepositoryFilter.this.sessionIdGenerator); } @Override diff --git a/spring-session-core/src/main/java/org/springframework/session/web/server/session/SpringSessionWebSessionStore.java b/spring-session-core/src/main/java/org/springframework/session/web/server/session/SpringSessionWebSessionStore.java index 97e0d9103..636a2d05d 100644 --- a/spring-session-core/src/main/java/org/springframework/session/web/server/session/SpringSessionWebSessionStore.java +++ b/spring-session-core/src/main/java/org/springframework/session/web/server/session/SpringSessionWebSessionStore.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 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. @@ -35,6 +35,7 @@ import org.springframework.lang.Nullable; import org.springframework.session.ReactiveSessionRepository; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.util.Assert; import org.springframework.web.server.WebSession; import org.springframework.web.server.session.WebSessionStore; @@ -47,17 +48,23 @@ * @param the {@link Session} type * @author Rob Winch * @author Vedran Pavic + * @author Yanming Zhou * @since 2.0 */ public class SpringSessionWebSessionStore implements WebSessionStore { private final ReactiveSessionRepository sessions; + private final SessionIdGenerator sessionIdGenerator; + private Clock clock = Clock.system(ZoneOffset.UTC); - public SpringSessionWebSessionStore(ReactiveSessionRepository reactiveSessionRepository) { + public SpringSessionWebSessionStore(ReactiveSessionRepository reactiveSessionRepository, + SessionIdGenerator sessionIdGenerator) { Assert.notNull(reactiveSessionRepository, "reactiveSessionRepository cannot be null"); + Assert.notNull(sessionIdGenerator, "sessionIdGenerator cannot be null"); this.sessions = reactiveSessionRepository; + this.sessionIdGenerator = sessionIdGenerator; } /** @@ -134,7 +141,7 @@ public String getId() { @Override public Mono changeSessionId() { return Mono.defer(() -> { - this.session.changeSessionId(); + this.session.changeSessionId(SpringSessionWebSessionStore.this.sessionIdGenerator); return save(); }); } diff --git a/spring-session-core/src/test/java/org/springframework/session/MapSessionTests.java b/spring-session-core/src/test/java/org/springframework/session/MapSessionTests.java index ce6d2cd36..56777132d 100644 --- a/spring-session-core/src/test/java/org/springframework/session/MapSessionTests.java +++ b/spring-session-core/src/test/java/org/springframework/session/MapSessionTests.java @@ -43,12 +43,6 @@ void constructorNullSession() { .withMessage("session cannot be null"); } - @Test - void constructorWhenSessionIdGeneratorThenUsesStrategy() { - MapSession session = new MapSession(new FixedSessionIdGenerator("my-id")); - assertThat(session.getId()).isEqualTo("my-id"); - } - @Test void constructorWhenDefaultThenUuid() { String id = this.session.getId(); @@ -157,15 +151,6 @@ void getAttributeNamesAndRemove() { assertThat(this.session.getAttributeNames()).isEmpty(); } - @Test - void changeSessionIdWhenSessionIdStrategyThenUsesStrategy() { - MapSession session = new MapSession(new IncrementalSessionIdGenerator()); - String idBeforeChange = session.getId(); - String idAfterChange = session.changeSessionId(); - assertThat(idBeforeChange).isEqualTo("1"); - assertThat(idAfterChange).isEqualTo("2"); - } - static class FixedSessionIdGenerator implements SessionIdGenerator { private final String id; @@ -200,7 +185,7 @@ public Instant getCreationTime() { } @Override - public String changeSessionId() { + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { throw new UnsupportedOperationException(); } diff --git a/spring-session-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java b/spring-session-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java index 7e4933c3c..3f8e452e7 100644 --- a/spring-session-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java +++ b/spring-session-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java @@ -33,6 +33,7 @@ * Tests for {@link ReactiveMapSessionRepository}. * * @author Rob Winch + * @author Yanming Zhou * @since 2.0 */ class ReactiveMapSessionRepositoryTests { @@ -154,10 +155,21 @@ void getAttributeNamesAndRemove() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = new SessionIdGenerator() { + @Override + public String generate() { + return "test"; + } + + @Override + public String regenerate(Session session) { + return "test2"; + } + }; + this.repository.setSessionIdGenerator(generator); MapSession session = this.repository.createSession().block(); assertThat(session.getId()).isEqualTo("test"); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test2"); } @Test @@ -168,7 +180,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.repository.setSessionIdGenerator(generator); MapSession session = this.repository.createSession().block(); this.repository.save(session).block(); @@ -176,7 +189,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { MapSession savedSession = this.repository.findById("test").block(); assertThat(savedSession.getId()).isEqualTo("test"); - assertThat(savedSession.changeSessionId()).isEqualTo("test"); + assertThat(savedSession.changeSessionId(generator)).isEqualTo("test"); } } diff --git a/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java b/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java index f9de42330..f89349d5c 100644 --- a/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java +++ b/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java @@ -55,6 +55,7 @@ import org.springframework.session.MapSession; import org.springframework.session.MapSessionRepository; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.SessionRepository; import org.springframework.test.util.ReflectionTestUtils; @@ -447,6 +448,18 @@ public void doFilter(HttpServletRequest wrappedRequest) { // gh-152 @Test void doFilterChangeSessionId() throws Exception { + this.filter.setSessionIdGenerator(new SessionIdGenerator() { + @Override + public String generate() { + return SessionIdGenerator.DEFAULT.generate(); + } + + @Override + public String regenerate(Session session) { + return session.getId() + '$'; + } + }); + final String ATTR = "ATTRIBUTE"; final String VALUE = "VALUE"; @@ -468,7 +481,7 @@ public void doFilter(HttpServletRequest wrappedRequest) { assertThat(originalSession.getId()).isEqualTo(originalSessionId); String changeSessionId = ReflectionTestUtils.invokeMethod(wrappedRequest, "changeSessionId"); - assertThat(changeSessionId).isNotEqualTo(originalSessionId); + assertThat(changeSessionId).isEqualTo(originalSessionId + '$'); // gh-227 assertThat(originalSession.getId()).isEqualTo(changeSessionId); } diff --git a/spring-session-core/src/test/java/org/springframework/session/web/server/session/SpringSessionWebSessionStoreTests.java b/spring-session-core/src/test/java/org/springframework/session/web/server/session/SpringSessionWebSessionStoreTests.java index 6f14e514b..065228c2a 100644 --- a/spring-session-core/src/test/java/org/springframework/session/web/server/session/SpringSessionWebSessionStoreTests.java +++ b/spring-session-core/src/test/java/org/springframework/session/web/server/session/SpringSessionWebSessionStoreTests.java @@ -30,6 +30,7 @@ import org.springframework.session.ReactiveSessionRepository; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.web.server.WebSession; import static org.assertj.core.api.Assertions.assertThat; @@ -60,14 +61,14 @@ class SpringSessionWebSessionStoreTests { @BeforeEach void setup() { - this.webSessionStore = new SpringSessionWebSessionStore<>(this.sessionRepository); + this.webSessionStore = new SpringSessionWebSessionStore<>(this.sessionRepository, SessionIdGenerator.DEFAULT); given(this.sessionRepository.findById(any())).willReturn(Mono.just(this.findByIdSession)); given(this.sessionRepository.createSession()).willReturn(Mono.just(this.createSession)); } @Test void constructorWhenNullRepositoryThenThrowsIllegalArgumentException() { - assertThatIllegalArgumentException().isThrownBy(() -> new SpringSessionWebSessionStore(null)) + assertThatIllegalArgumentException().isThrownBy(() -> new SpringSessionWebSessionStore(null, null)) .withMessage("reactiveSessionRepository cannot be null"); } diff --git a/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoIndexedSessionRepository.java b/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoIndexedSessionRepository.java index c8b93f73b..2eef74462 100644 --- a/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoIndexedSessionRepository.java +++ b/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoIndexedSessionRepository.java @@ -51,6 +51,7 @@ * @author Jakub Kubrynski * @author Greg Turnquist * @author Vedran Pavic + * @author Yanming Zhou * @since 2.2.0 */ public class MongoIndexedSessionRepository @@ -91,7 +92,7 @@ public MongoIndexedSessionRepository(MongoOperations mongoOperations) { @Override public MongoSession createSession() { - MongoSession session = new MongoSession(this.sessionIdGenerator); + MongoSession session = new MongoSession(this.sessionIdGenerator.generate()); session.setMaxInactiveInterval(this.defaultMaxInactiveInterval); @@ -125,7 +126,6 @@ public MongoSession findById(String id) { deleteById(id); return null; } - session.setSessionIdGenerator(this.sessionIdGenerator); } return session; @@ -146,7 +146,6 @@ public Map findByIndexNameAndIndexValue(String indexName, .map((query) -> this.mongoOperations.find(query, Document.class, this.collectionName)) .orElse(Collections.emptyList()).stream() .map((dbSession) -> MongoSessionUtils.convertToSession(this.mongoSessionConverter, dbSession)) - .peek((session) -> session.setSessionIdGenerator(this.sessionIdGenerator)) .collect(Collectors.toMap(MongoSession::getId, (mapSession) -> mapSession)); } diff --git a/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoSession.java b/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoSession.java index 1856b7c1d..cd95f8619 100644 --- a/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoSession.java +++ b/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoSession.java @@ -29,13 +29,13 @@ import org.springframework.session.MapSession; import org.springframework.session.Session; import org.springframework.session.SessionIdGenerator; -import org.springframework.util.Assert; /** * Session object providing additional information about the datetime of expiration. * * @author Jakub Kubrynski * @author Greg Turnquist + * @author Yanming Zhou * @since 1.2 */ class MongoSession implements Session { @@ -47,9 +47,7 @@ class MongoSession implements Session { * NOTE: This was originally stored in unicode format. Delomboking the code caused it * to get converted to another encoding, which isn't supported on all systems, so we * migrated back to unicode. The same character is being represented ensuring binary - * compatibility. - * - * See https://www.compart.com/en/unicode/U+F607 + * compatibility. See https://www.compart.com/en/unicode/U+F607 */ private static final char DOT_COVER_CHAR = '\uF607'; @@ -67,8 +65,6 @@ class MongoSession implements Session { private Map attrs = new HashMap<>(); - private transient SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT; - /** * Constructs a new instance using the provided session id. * @param sessionId the session id to use @@ -94,28 +90,6 @@ class MongoSession implements Session { setLastAccessedTime(Instant.ofEpochMilli(this.createdMillis)); } - /** - * Constructs a new instance using the provided {@link SessionIdGenerator}. - * @param sessionIdGenerator the {@link SessionIdGenerator} to use - * @since 3.2 - */ - MongoSession(SessionIdGenerator sessionIdGenerator) { - this(sessionIdGenerator.generate(), MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS); - this.sessionIdGenerator = sessionIdGenerator; - } - - /** - * Constructs a new instance using the provided {@link SessionIdGenerator} and max - * inactive interval. - * @param sessionIdGenerator the {@link SessionIdGenerator} to use - * @param maxInactiveIntervalInSeconds the max inactive interval in seconds - * @since 3.2 - */ - MongoSession(SessionIdGenerator sessionIdGenerator, long maxInactiveIntervalInSeconds) { - this(sessionIdGenerator.generate(), maxInactiveIntervalInSeconds); - this.sessionIdGenerator = sessionIdGenerator; - } - static String coverDot(String attributeName) { return attributeName.replace('.', DOT_COVER_CHAR); } @@ -125,11 +99,10 @@ static String uncoverDot(String attributeName) { } @Override - public String changeSessionId() { - - String changedId = this.sessionIdGenerator.generate(); - this.id = changedId; - return changedId; + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); + setId(newSessionId); + return newSessionId; } @Override @@ -242,14 +215,4 @@ void setId(String id) { this.id = id; } - /** - * Sets the {@link SessionIdGenerator} to use. - * @param sessionIdGenerator the {@link SessionIdGenerator} to use - * @since 3.2 - */ - void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) { - Assert.notNull(sessionIdGenerator, "sessionIdGenerator cannot be null"); - this.sessionIdGenerator = sessionIdGenerator; - } - } diff --git a/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java b/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java index c73334459..94177c743 100644 --- a/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java +++ b/spring-session-data-mongodb/src/main/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepository.java @@ -45,6 +45,7 @@ * * @author Greg Turnquist * @author Vedran Pavic + * @author Yanming Zhou * @since 2.2.0 */ public class ReactiveMongoSessionRepository @@ -101,10 +102,8 @@ public Mono createSession() { return Mono.fromSupplier(() -> this.sessionIdGenerator.generate()) .map(MongoSession::new) .doOnNext((mongoSession) -> mongoSession.setMaxInactiveInterval(this.defaultMaxInactiveInterval)) - .doOnNext( - (mongoSession) -> mongoSession.setSessionIdGenerator(this.sessionIdGenerator)) .doOnNext((mongoSession) -> publishEvent(new SessionCreatedEvent(this, mongoSession))) - .switchIfEmpty(Mono.just(new MongoSession(this.sessionIdGenerator))) + .switchIfEmpty(Mono.just(new MongoSession(this.sessionIdGenerator.generate()))) .subscribeOn(Schedulers.boundedElastic()) .publishOn(Schedulers.parallel()); // @formatter:on @@ -137,7 +136,6 @@ public Mono findById(String id) { return findSession(id) // .map((document) -> MongoSessionUtils.convertToSession(this.mongoSessionConverter, document)) // .filter((mongoSession) -> !mongoSession.isExpired()) // - .doOnNext((mongoSession) -> mongoSession.setSessionIdGenerator(this.sessionIdGenerator)) .switchIfEmpty(Mono.defer(() -> this.deleteById(id).then(Mono.empty()))); } diff --git a/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/MongoIndexedSessionRepositoryTest.java b/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/MongoIndexedSessionRepositoryTest.java index a9e86ad58..06d5fe80e 100644 --- a/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/MongoIndexedSessionRepositoryTest.java +++ b/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/MongoIndexedSessionRepositoryTest.java @@ -34,6 +34,7 @@ import org.springframework.data.mongodb.core.query.Query; import org.springframework.session.FindByIndexNameSessionRepository; import org.springframework.session.MapSession; +import org.springframework.session.Session; import org.springframework.session.SessionIdGenerator; import static org.assertj.core.api.Assertions.assertThat; @@ -51,6 +52,7 @@ * @author Jakub Kubrynski * @author Vedran Pavic * @author Greg Turnquist + * @author Yanming Zhou */ @ExtendWith(MockitoExtension.class) public class MongoIndexedSessionRepositoryTest { @@ -213,10 +215,21 @@ void shouldReturnEmptyMapForNotSupportedIndex() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.repository.setSessionIdGenerator(new FixedSessionIdGenerator("123")); + SessionIdGenerator generator = new SessionIdGenerator() { + @Override + public String generate() { + return "test"; + } + + @Override + public String regenerate(Session session) { + return "test2"; + } + }; + this.repository.setSessionIdGenerator(generator); MongoSession session = this.repository.createSession(); - assertThat(session.getId()).isEqualTo("123"); - assertThat(session.changeSessionId()).isEqualTo("123"); + assertThat(session.getId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test2"); } @Test @@ -226,7 +239,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.repository.setSessionIdGenerator(new FixedSessionIdGenerator("456")); + SessionIdGenerator generator = new FixedSessionIdGenerator("456"); + this.repository.setSessionIdGenerator(generator); Document sessionDocument = new Document(); @@ -240,7 +254,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { MongoSession retrievedSession = this.repository.findById("123"); assertThat(retrievedSession.getId()).isEqualTo("123"); - String newSessionId = retrievedSession.changeSessionId(); + String newSessionId = retrievedSession.changeSessionId(generator); assertThat(newSessionId).isEqualTo("456"); } diff --git a/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepositoryTest.java b/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepositoryTest.java index 9e15d382f..001889380 100644 --- a/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepositoryTest.java +++ b/spring-session-data-mongodb/src/test/java/org/springframework/session/data/mongo/ReactiveMongoSessionRepositoryTest.java @@ -37,6 +37,7 @@ import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.index.IndexOperations; import org.springframework.session.MapSession; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.events.SessionDeletedEvent; import static org.assertj.core.api.Assertions.assertThat; @@ -54,6 +55,7 @@ * @author Jakub Kubrynski * @author Vedran Pavic * @author Greg Turnquist + * @author Yanming Zhou */ @ExtendWith(MockitoExtension.class) public class ReactiveMongoSessionRepositoryTest { @@ -213,11 +215,12 @@ void shouldInvokeMethodToCreateIndexesImperatively() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.repository.setSessionIdGenerator(generator); this.repository.createSession().as(StepVerifier::create).assertNext((mongoSession) -> { assertThat(mongoSession.getId()).isEqualTo("test"); - assertThat(mongoSession.changeSessionId()).isEqualTo("test"); + assertThat(mongoSession.changeSessionId(generator)).isEqualTo("test"); }).verifyComplete(); } @@ -229,7 +232,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.repository.setSessionIdGenerator(generator); String sessionId = UUID.randomUUID().toString(); Document sessionDocument = new Document(); @@ -244,7 +248,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { this.repository.findById(sessionId).as(StepVerifier::create).assertNext((mongoSession) -> { String oldId = mongoSession.getId(); - String newId = mongoSession.changeSessionId(); + String newId = mongoSession.changeSessionId(generator); assertThat(oldId).isEqualTo(sessionId); assertThat(newId).isEqualTo("test"); }).verifyComplete(); diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java index fad5c58e2..5811df160 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java @@ -42,6 +42,7 @@ * * @author Vedran Pavic * @author Kai Zhao + * @author Yanming Zhou * @since 2.2.0 */ public class ReactiveRedisSessionRepository @@ -223,8 +224,8 @@ public String getId() { } @Override - public String changeSessionId() { - String newSessionId = ReactiveRedisSessionRepository.this.sessionIdGenerator.generate(); + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); this.cached.setId(newSessionId); return newSessionId; } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java index a222a61a2..3d0fe8ff7 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java @@ -254,6 +254,7 @@ * * @author Rob Winch * @author Vedran Pavic + * @author Yanming Zhou * @since 2.2.0 */ public class RedisIndexedSessionRepository @@ -550,7 +551,7 @@ public void deleteById(String sessionId) { @Override public RedisSession createSession() { - MapSession cached = new MapSession(this.sessionIdGenerator); + MapSession cached = new MapSession(this.sessionIdGenerator.generate()); cached.setMaxInactiveInterval(this.defaultMaxInactiveInterval); RedisSession session = new RedisSession(cached, true); session.flushImmediateIfNecessary(); @@ -792,8 +793,8 @@ public String getId() { } @Override - public String changeSessionId() { - String newSessionId = RedisIndexedSessionRepository.this.sessionIdGenerator.generate(); + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); this.cached.setId(newSessionId); return newSessionId; } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java index df84355eb..ee58a4ba4 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java @@ -109,7 +109,7 @@ public void setSaveMode(SaveMode saveMode) { @Override public RedisSession createSession() { - MapSession cached = new MapSession(this.sessionIdGenerator); + MapSession cached = new MapSession(this.sessionIdGenerator.generate()); cached.setMaxInactiveInterval(this.defaultMaxInactiveInterval); RedisSession session = new RedisSession(cached, true); session.flushIfRequired(); @@ -210,8 +210,8 @@ public String getId() { } @Override - public String changeSessionId() { - String newSessionId = RedisSessionRepository.this.sessionIdGenerator.generate(); + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); this.cached.setId(newSessionId); return newSessionId; } diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryTests.java index baa6d14d2..948d695fe 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryTests.java @@ -33,6 +33,7 @@ import org.springframework.data.redis.core.ReactiveRedisOperations; import org.springframework.session.MapSession; import org.springframework.session.SaveMode; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.data.redis.ReactiveRedisSessionRepository.RedisSession; import org.springframework.test.util.ReflectionTestUtils; @@ -50,6 +51,7 @@ * * @author Vedran Pavic * @author Kai Zhao + * @author Yanming Zhou */ class ReactiveRedisSessionRepositoryTests { @@ -440,11 +442,12 @@ void saveWithSaveModeAlways() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.repository.setSessionIdGenerator(generator); this.repository.createSession().as(StepVerifier::create).assertNext((redisSession) -> { assertThat(redisSession.getId()).isEqualTo("test"); - assertThat(redisSession.changeSessionId()).isEqualTo("test"); + assertThat(redisSession.changeSessionId(generator)).isEqualTo("test"); }).verifyComplete(); } @@ -457,7 +460,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test @SuppressWarnings("unchecked") void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.repository.setSessionIdGenerator(() -> "changed-session-id"); + SessionIdGenerator generator = () -> "changed-session-id"; + this.repository.setSessionIdGenerator(generator); given(this.redisOperations.opsForHash()).willReturn(this.hashOperations); String attribute1 = "attribute1"; String attribute2 = "attribute2"; @@ -474,7 +478,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { StepVerifier.create(this.repository.findById("test")).consumeNextWith((session) -> { assertThat(session.getId()).isEqualTo(expected.getId()); - assertThat(session.changeSessionId()).isEqualTo("changed-session-id"); + assertThat(session.changeSessionId(generator)).isEqualTo("changed-session-id"); }).verifyComplete(); } diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java index f580a543a..30aca72a4 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java @@ -50,6 +50,7 @@ import org.springframework.session.MapSession; import org.springframework.session.SaveMode; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.data.redis.RedisIndexedSessionRepository.RedisSession; import org.springframework.session.events.AbstractSessionEvent; @@ -912,10 +913,21 @@ void saveWithSaveModeAlways() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.redisRepository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = new SessionIdGenerator() { + @Override + public String generate() { + return "test"; + } + + @Override + public String regenerate(Session session) { + return "test2"; + } + }; + this.redisRepository.setSessionIdGenerator(generator); RedisSession session = this.redisRepository.createSession(); assertThat(session.getId()).isEqualTo("test"); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test2"); } @Test @@ -926,7 +938,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.redisRepository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.redisRepository.setSessionIdGenerator(generator); String attribute1 = "attribute1"; String attribute2 = "attribute2"; MapSession expected = new MapSession("original"); @@ -945,7 +958,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { RedisSession session = this.redisRepository.findById(expected.getId()); String oldSessionId = session.getId(); - String newSessionId = session.changeSessionId(); + String newSessionId = session.changeSessionId(generator); assertThat(oldSessionId).isEqualTo("original"); assertThat(newSessionId).isEqualTo("test"); } diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionRepositoryTests.java index 6f930b727..d46f60864 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionRepositoryTests.java @@ -36,6 +36,8 @@ import org.springframework.session.FlushMode; import org.springframework.session.MapSession; import org.springframework.session.SaveMode; +import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.data.redis.RedisSessionRepository.RedisSession; import org.springframework.test.util.ReflectionTestUtils; @@ -51,6 +53,7 @@ * Tests for {@link RedisSessionRepository}. * * @author Vedran Pavic + * @author Yanming Zhou */ @ExtendWith(MockitoExtension.class) class RedisSessionRepositoryTests { @@ -375,10 +378,21 @@ void getSessionRedisOperations__ShouldReturnRedisOperations() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.sessionRepository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = new SessionIdGenerator() { + @Override + public String generate() { + return "test"; + } + + @Override + public String regenerate(Session session) { + return "test2"; + } + }; + this.sessionRepository.setSessionIdGenerator(generator); RedisSessionRepository.RedisSession session = this.sessionRepository.createSession(); assertThat(session.getId()).isEqualTo("test"); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test2"); } @Test @@ -389,7 +403,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.sessionRepository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.sessionRepository.setSessionIdGenerator(generator); Instant now = Instant.now().truncatedTo(ChronoUnit.MILLIS); given(this.sessionHashOperations.entries(eq(TEST_SESSION_KEY))) .willReturn(mapOf(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), @@ -398,7 +413,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { RedisSessionMapper.ATTRIBUTE_PREFIX + "attribute1", "value1")); RedisSession session = this.sessionRepository.findById(TEST_SESSION_ID); assertThat(session.getId()).isEqualTo(TEST_SESSION_ID); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test"); } private static String getSessionKey(String sessionId) { diff --git a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepository.java b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepository.java index c9416c3f6..b99574915 100644 --- a/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepository.java +++ b/spring-session-hazelcast/src/main/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepository.java @@ -111,6 +111,7 @@ * @author Mark Anderson * @author Aleksandar Stojsavljevic * @author Eleftheria Stein + * @author Yanming Zhou * @since 2.2.0 */ public class HazelcastIndexedSessionRepository @@ -248,7 +249,7 @@ public void setSaveMode(SaveMode saveMode) { @Override public HazelcastSession createSession() { - MapSession cached = new MapSession(this.sessionIdGenerator); + MapSession cached = new MapSession(this.sessionIdGenerator.generate()); cached.setMaxInactiveInterval(this.defaultMaxInactiveInterval); HazelcastSession session = new HazelcastSession(cached, true); session.flushImmediateIfNecessary(); @@ -417,8 +418,8 @@ public String getId() { } @Override - public String changeSessionId() { - String newSessionId = HazelcastIndexedSessionRepository.this.sessionIdGenerator.generate(); + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); this.delegate.setId(newSessionId); this.sessionIdChanged = true; return newSessionId; diff --git a/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepositoryTests.java b/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepositoryTests.java index 5865ce8cf..11ef7db9b 100644 --- a/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepositoryTests.java +++ b/spring-session-hazelcast/src/test/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepositoryTests.java @@ -39,6 +39,8 @@ import org.springframework.session.FlushMode; import org.springframework.session.MapSession; import org.springframework.session.SaveMode; +import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.hazelcast.HazelcastIndexedSessionRepository.HazelcastSession; import org.springframework.test.util.ReflectionTestUtils; @@ -60,6 +62,7 @@ * * @author Vedran Pavic * @author Aleksandar Stojsavljevic + * @author Yanming Zhou */ class HazelcastIndexedSessionRepositoryTests { @@ -467,10 +470,21 @@ void saveWithSaveModeAlways() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = new SessionIdGenerator() { + @Override + public String generate() { + return "test"; + } + + @Override + public String regenerate(Session session) { + return "test2"; + } + }; + this.repository.setSessionIdGenerator(generator); HazelcastSession session = this.repository.createSession(); assertThat(session.getId()).isEqualTo("test"); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test2"); } @Test @@ -481,7 +495,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.repository.setSessionIdGenerator(generator); MapSession saved = new MapSession("original"); saved.setAttribute("savedName", "savedValue"); given(this.sessions.get(eq(saved.getId()))).willReturn(saved); @@ -489,7 +504,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { HazelcastSession session = this.repository.findById(saved.getId()); assertThat(session.getId()).isEqualTo(saved.getId()); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test"); } } diff --git a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java index ffaa8ac09..75f241966 100644 --- a/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java +++ b/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.UUID; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -135,6 +134,7 @@ * * @author Vedran Pavic * @author Craig Andrews + * @author Yanming Zhou * @since 2.2.0 */ public class JdbcIndexedSessionRepository implements @@ -464,9 +464,9 @@ public void setCleanupCron(String cleanupCron) { @Override public JdbcSession createSession() { - MapSession delegate = new MapSession(this.sessionIdGenerator); + MapSession delegate = new MapSession(this.sessionIdGenerator.generate()); delegate.setMaxInactiveInterval(this.defaultMaxInactiveInterval); - JdbcSession session = new JdbcSession(delegate, UUID.randomUUID().toString(), true); + JdbcSession session = new JdbcSession(delegate, delegate.getId(), true); session.flushIfRequired(); return session; } @@ -784,10 +784,10 @@ public String getId() { } @Override - public String changeSessionId() { - this.changed = true; - String newSessionId = JdbcIndexedSessionRepository.this.sessionIdGenerator.generate(); + public String changeSessionId(SessionIdGenerator sessionIdGenerator) { + String newSessionId = sessionIdGenerator.regenerate(this); this.delegate.setId(newSessionId); + this.changed = true; return newSessionId; } diff --git a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java index 159e5982a..059ff15e1 100644 --- a/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java +++ b/spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java @@ -47,6 +47,7 @@ import org.springframework.session.MapSession; import org.springframework.session.SaveMode; import org.springframework.session.Session; +import org.springframework.session.SessionIdGenerator; import org.springframework.session.jdbc.JdbcIndexedSessionRepository.JdbcSession; import org.springframework.transaction.support.TransactionOperations; @@ -69,6 +70,7 @@ * * @author Vedran Pavic * @author Craig Andrews + * @author Yanming Zhou */ @ExtendWith(MockitoExtension.class) class JdbcIndexedSessionRepositoryTests { @@ -777,10 +779,21 @@ void saveAndFreeTemporaryLob() { @Test void createSessionWhenSessionIdGeneratorThenUses() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = new SessionIdGenerator() { + @Override + public String generate() { + return "test"; + } + + @Override + public String regenerate(Session session) { + return "test2"; + } + }; + this.repository.setSessionIdGenerator(generator); JdbcSession session = this.repository.createSession(); assertThat(session.getId()).isEqualTo("test"); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test2"); } @Test @@ -791,7 +804,8 @@ void setSessionIdGeneratorWhenNullThenThrowsException() { @Test void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { - this.repository.setSessionIdGenerator(() -> "test"); + SessionIdGenerator generator = () -> "test"; + this.repository.setSessionIdGenerator(generator); Session saved = this.repository.new JdbcSession(new MapSession(), "primaryKey", false); saved.setAttribute("savedName", "savedValue"); given(this.jdbcOperations.query(isA(String.class), isA(PreparedStatementSetter.class), @@ -800,7 +814,7 @@ void findByIdWhenChangeSessionIdThenUsesSessionIdGenerator() { JdbcSession session = this.repository.findById(saved.getId()); assertThat(session.getId()).isEqualTo(saved.getId()); - assertThat(session.changeSessionId()).isEqualTo("test"); + assertThat(session.changeSessionId(generator)).isEqualTo("test"); } }