Introduce SessionIdGenerator::regenerate for Session::changeSessionId

spring-projects · Aug 10, 2023 · 26c5755 · 26c5755
1 parent 242b854
commit 26c5755
Show file tree

Hide file tree

Showing 15 changed files with 113 additions and 21 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
@@ -142,7 +142,7 @@ public String getOriginalId() {
 
 	@Override
 	public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-		String newSessionId = sessionIdGenerator.generate();
+		String newSessionId = sessionIdGenerator.regenerate(this);
 		setId(newSessionId);
 		return newSessionId;
 	}

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
@@ -22,11 +22,27 @@
  * An interface for specifying a strategy for generating session identifiers.
  *
  * @author Marcus da Coregio
+ * @author Yanming Zhou
  * @since 3.2
  */
 public interface SessionIdGenerator {
 
+	/**
+	 * 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/...ion-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java b/...ion-core/src/test/java/org/springframework/session/ReactiveMapSessionRepositoryTests.java
@@ -155,11 +155,21 @@ void getAttributeNamesAndRemove() {
 
 	@Test
 	void createSessionWhenSessionIdGeneratorThenUses() {
-		SessionIdGenerator generator = () -> "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(generator)).isEqualTo("test");
+		assertThat(session.changeSessionId(generator)).isEqualTo("test2");
 	}
 
 	@Test

diff --git a/...core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java b/...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 Session.DEFAULT_SESSION_ID_GENERATOR.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/...ssion-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoSession.java b/...ssion-data-mongodb/src/main/java/org/springframework/session/data/mongo/MongoSession.java
@@ -100,7 +100,7 @@ static String uncoverDot(String attributeName) {
 
 	@Override
 	public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-		String newSessionId = sessionIdGenerator.generate();
+		String newSessionId = sessionIdGenerator.regenerate(this);
 		setId(newSessionId);
 		return newSessionId;
 	}

diff --git a/...c/test/java/org/springframework/session/data/mongo/MongoIndexedSessionRepositoryTest.java b/...c/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;
@@ -214,11 +215,21 @@ void shouldReturnEmptyMapForNotSupportedIndex() {
 
 	@Test
 	void createSessionWhenSessionIdGeneratorThenUses() {
-		SessionIdGenerator generator = 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(generator)).isEqualTo("123");
+		assertThat(session.getId()).isEqualTo("test");
+		assertThat(session.changeSessionId(generator)).isEqualTo("test2");
 	}
 
 	@Test

diff --git a/.../src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java b/.../src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java
@@ -220,7 +220,7 @@ public String getId() {
 
 		@Override
 		public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-			String newSessionId = sessionIdGenerator.generate();
+			String newSessionId = sessionIdGenerator.regenerate(this);
 			this.cached.setId(newSessionId);
 			return newSessionId;
 		}

diff --git a/...s/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java b/...s/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java
@@ -794,7 +794,7 @@ public String getId() {
 
 		@Override
 		public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-			String newSessionId = sessionIdGenerator.generate();
+			String newSessionId = sessionIdGenerator.regenerate(this);
 			this.cached.setId(newSessionId);
 			return newSessionId;
 		}

diff --git a/...ta-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java b/...ta-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java
@@ -211,7 +211,7 @@ public String getId() {
 
 		@Override
 		public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-			String newSessionId = sessionIdGenerator.generate();
+			String newSessionId = sessionIdGenerator.regenerate(this);
 			this.cached.setId(newSessionId);
 			return newSessionId;
 		}

diff --git a/.../test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java b/.../test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java
@@ -913,11 +913,21 @@ void saveWithSaveModeAlways() {
 
 	@Test
 	void createSessionWhenSessionIdGeneratorThenUses() {
-		SessionIdGenerator generator = () -> "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(generator)).isEqualTo("test");
+		assertThat(session.changeSessionId(generator)).isEqualTo("test2");
 	}
 
 	@Test

diff --git a/...dis/src/test/java/org/springframework/session/data/redis/RedisSessionRepositoryTests.java b/...dis/src/test/java/org/springframework/session/data/redis/RedisSessionRepositoryTests.java
@@ -36,6 +36,7 @@
 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;
@@ -377,11 +378,21 @@ void getSessionRedisOperations__ShouldReturnRedisOperations() {
 
 	@Test
 	void createSessionWhenSessionIdGeneratorThenUses() {
-		SessionIdGenerator generator = () -> "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(generator)).isEqualTo("test");
+		assertThat(session.changeSessionId(generator)).isEqualTo("test2");
 	}
 
 	@Test

diff --git a/...rc/main/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepository.java b/...rc/main/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepository.java
@@ -419,7 +419,7 @@ public String getId() {
 
 		@Override
 		public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-			String newSessionId = sessionIdGenerator.generate();
+			String newSessionId = sessionIdGenerator.regenerate(this);
 			this.delegate.setId(newSessionId);
 			this.sessionIdChanged = true;
 			return newSessionId;

diff --git a/...st/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepositoryTests.java b/...st/java/org/springframework/session/hazelcast/HazelcastIndexedSessionRepositoryTests.java
@@ -39,6 +39,7 @@
 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;
@@ -469,11 +470,21 @@ void saveWithSaveModeAlways() {
 
 	@Test
 	void createSessionWhenSessionIdGeneratorThenUses() {
-		SessionIdGenerator generator = () -> "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(generator)).isEqualTo("test");
+		assertThat(session.changeSessionId(generator)).isEqualTo("test2");
 	}
 
 	@Test

diff --git a/...ion-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java b/...ion-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java
@@ -785,7 +785,7 @@ public String getId() {
 
 		@Override
 		public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
-			String newSessionId = sessionIdGenerator.generate();
+			String newSessionId = sessionIdGenerator.regenerate(this);
 			this.delegate.setId(newSessionId);
 			this.changed = true;
 			return newSessionId;

diff --git a/...dbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java b/...dbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java
@@ -779,11 +779,21 @@ void saveAndFreeTemporaryLob() {
 
 	@Test
 	void createSessionWhenSessionIdGeneratorThenUses() {
-		SessionIdGenerator generator = () -> "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(generator)).isEqualTo("test");
+		assertThat(session.changeSessionId(generator)).isEqualTo("test2");
 	}
 
 	@Test