From c0f4c7f38134ca3e9c29525e8ca1aa3d5e93ca6a Mon Sep 17 00:00:00 2001
From: Vedran Pavic <vedran.pavic@gmail.com>
Date: Fri, 5 Jan 2018 12:21:39 +0100
Subject: [PATCH] Fix reactive Redis change session id handling

This commit updates logic around changing session id in `ReactiveRedisOperationsSessionRepository` to properly handle updates for new sessions i.e. ones that haven't been saved yet.

Previously, the logic skipped both Redis rename operation and replacement of session id within the current session holder object, which led to no such key errors on subsequent save operation which still observed the session id as changed.

Closes gh-954
---
 ...edisOperationsSessionRepositoryITests.java | 17 +++++++++++-
 ...ctiveRedisOperationsSessionRepository.java | 26 ++++++++++++-------
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryITests.java
index ce41fefd..69861eb8 100644
--- a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryITests.java
+++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepositoryITests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2017 the original author or authors.
+ * Copyright 2014-2018 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.
@@ -176,6 +176,21 @@ public class ReactiveRedisOperationsSessionRepositoryITests extends AbstractRedi
 		assertThat(this.repository.findById(originalId).block()).isNull();
 	}
 
+	// gh-954
+	@Test
+	public void changeSessionIdSaveTwice() {
+		ReactiveRedisOperationsSessionRepository.RedisSession toSave = this.repository
+				.createSession().block();
+		String originalId = toSave.getId();
+		toSave.changeSessionId();
+
+		this.repository.save(toSave).block();
+		this.repository.save(toSave).block();
+
+		assertThat(this.repository.findById(toSave.getId()).block()).isNotNull();
+		assertThat(this.repository.findById(originalId).block()).isNull();
+	}
+
 	@Configuration
 	@EnableRedisWebSession
 	static class Config extends BaseConfig {
diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java
index 8fd21604..a4c117d5 100644
--- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java
+++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisOperationsSessionRepository.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2017 the original author or authors.
+ * Copyright 2014-2018 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.
@@ -24,6 +24,7 @@ import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import org.reactivestreams.Publisher;
 import reactor.core.publisher.Mono;
 
 import org.springframework.data.redis.core.ReactiveRedisOperations;
@@ -319,18 +320,25 @@ public class ReactiveRedisOperationsSessionRepository implements
 		}
 
 		private Mono<Void> saveChangeSessionId(String sessionId) {
-			if (isNew() || sessionId.equals(this.originalSessionId)) {
+			if (sessionId.equals(this.originalSessionId)) {
 				return Mono.empty();
 			}
 
-			String originalSessionKey = getSessionKey(this.originalSessionId);
-			String sessionKey = getSessionKey(sessionId);
+			Publisher<Void> replaceSessionId = s -> {
+				this.originalSessionId = sessionId;
+				s.onComplete();
+			};
 
-			return ReactiveRedisOperationsSessionRepository.this.sessionRedisOperations
-					.rename(originalSessionKey, sessionKey).and(s -> {
-						this.originalSessionId = sessionId;
-						s.onComplete();
-					});
+			if (isNew()) {
+				return Mono.from(replaceSessionId);
+			}
+			else {
+				String originalSessionKey = getSessionKey(this.originalSessionId);
+				String sessionKey = getSessionKey(sessionId);
+
+				return ReactiveRedisOperationsSessionRepository.this.sessionRedisOperations
+						.rename(originalSessionKey, sessionKey).and(replaceSessionId);
+			}
 		}
 
 	}