From d85624c58e5f6f995f1762961af3711659b99e10 Mon Sep 17 00:00:00 2001
From: Vladimir Tsanev <tsachev@gmail.com>
Date: Thu, 26 Nov 2015 14:43:31 +0200
Subject: [PATCH] Don't send expirations del events when expiration time does
 not change.

Expiration time is rounded up, so in case of many request per second from a
single session we can skip sending of delete event to reduce the traffic.
This is good because actually at the moment the redis repository is subscribed
for this events, even though it ignores them.

Fix gh-315
---
 .../data/redis/RedisSessionExpirationPolicy.java  | 10 ++++++----
 .../redis/RedisSessionExpirationPolicyTests.java  | 15 ++++++++++++++-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java b/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java
index 8d8e1a4..17e8804 100644
--- a/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java
+++ b/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java
@@ -68,14 +68,16 @@ final class RedisSessionExpirationPolicy {
 
 	public void onExpirationUpdated(Long originalExpirationTimeInMilli, ExpiringSession session) {
 		String keyToExpire = "expires:" + session.getId();
+		long toExpire = roundUpToNextMinute(expiresInMillis(session));
+
 		if(originalExpirationTimeInMilli != null) {
 			long originalRoundedUp = roundUpToNextMinute(originalExpirationTimeInMilli);
-			String expireKey = getExpirationKey(originalRoundedUp);
-			redis.boundSetOps(expireKey).remove(keyToExpire);
+			if(toExpire != originalRoundedUp) {
+				String expireKey = getExpirationKey(originalRoundedUp);
+				redis.boundSetOps(expireKey).remove(keyToExpire);
+			}
 		}
 
-		long toExpire = roundUpToNextMinute(expiresInMillis(session));
-
 		String expireKey = getExpirationKey(toExpire);
 		BoundSetOperations<Object, Object> expireOperations = redis.boundSetOps(expireKey);
 		expireOperations.add(keyToExpire);
diff --git a/spring-session/src/test/java/org/springframework/session/data/redis/RedisSessionExpirationPolicyTests.java b/spring-session/src/test/java/org/springframework/session/data/redis/RedisSessionExpirationPolicyTests.java
index a0c3cd9..0912cd7 100644
--- a/spring-session/src/test/java/org/springframework/session/data/redis/RedisSessionExpirationPolicyTests.java
+++ b/spring-session/src/test/java/org/springframework/session/data/redis/RedisSessionExpirationPolicyTests.java
@@ -59,7 +59,7 @@ public class RedisSessionExpirationPolicyTests {
 		RedisOperationsSessionRepository repository = new RedisOperationsSessionRepository(sessionRedisOperations);
 		policy = new RedisSessionExpirationPolicy(sessionRedisOperations, repository);
 		session = new MapSession();
-		session.setLastAccessedTime(1429116694665L);
+		session.setLastAccessedTime(1429116694675L);
 		session.setId("12345");
 
 		when(sessionRedisOperations.boundSetOps(anyString())).thenReturn(setOperations);
@@ -81,6 +81,19 @@ public class RedisSessionExpirationPolicyTests {
 		verify(setOperations).remove("expires:"+ session.getId());
 	}
 
+	@Test
+	public void onExpirationUpdatedDoNotSendDeleteWhenExpirationTimeDoesNotChange() throws Exception {
+		long originalExpirationTimeInMs = RedisSessionExpirationPolicy.expiresInMillis(session) - 10;
+		long originalRoundedToNextMinInMs = RedisSessionExpirationPolicy.roundUpToNextMinute(originalExpirationTimeInMs);
+		String originalExpireKey = policy.getExpirationKey(originalRoundedToNextMinInMs);
+
+		policy.onExpirationUpdated(originalExpirationTimeInMs, session);
+
+		// verify the original is not removed
+		verify(sessionRedisOperations).boundSetOps(originalExpireKey);
+		verify(setOperations, never()).remove("expires:"+ session.getId());
+	}
+
 	@Test
 	public void onExpirationUpdatedAddsExpirationTimeRoundedUp() throws Exception {
 		long expirationTimeInMs = RedisSessionExpirationPolicy.expiresInMillis(session);