From 7463592988b96d9c76b4ab70860ba6a87c5eea75 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 28 Jan 2016 21:38:18 -0600 Subject: [PATCH] Fix RedisOperationsSessionRepository.findByPrincipalName index Previously, the index was not properly cleaned up when it was changed. This commit ensures that the index is removed when the index name changes. Fixes gh-343 --- ...edisOperationsSessionRepositoryITests.java | 134 ++++++++++++++++++ .../RedisOperationsSessionRepository.java | 17 ++- 2 files changed, 146 insertions(+), 5 deletions(-) diff --git a/spring-session/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java b/spring-session/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java index 3a9939e..a44d06c 100644 --- a/spring-session/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java +++ b/spring-session/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java @@ -26,6 +26,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.connection.DefaultMessage; import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; import org.springframework.data.redis.core.RedisOperations; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -145,6 +146,139 @@ public class RedisOperationsSessionRepositoryITests { assertThat(findByPrincipalName.keySet()).doesNotContain(toSave.getId()); } + @Test + public void findByPrincipalNameExpireRemovesIndex() throws Exception { + String principalName = "findByPrincipalNameExpireRemovesIndex" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + String body = "spring:session:RedisOperationsSessionRepositoryITests:sessions:expires:" + toSave.getId(); + String channel = ":expired"; + DefaultMessage message = new DefaultMessage(channel.getBytes("UTF-8"), body.getBytes("UTF-8")); + byte[] pattern = new byte[] {}; + repository.onMessage(message , pattern); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + + assertThat(findByPrincipalName).hasSize(0); + assertThat(findByPrincipalName.keySet()).doesNotContain(toSave.getId()); + } + + @Test + public void findByPrincipalNameNoPrincipalNameChange() throws Exception { + String principalName = "findByPrincipalNameNoPrincipalNameChange" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + toSave.setAttribute("other", "value"); + repository.save(toSave); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + + assertThat(findByPrincipalName).hasSize(1); + assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId()); + } + + @Test + public void findByPrincipalNameNoPrincipalNameChangeReload() throws Exception { + String principalName = "findByPrincipalNameNoPrincipalNameChangeReload" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + toSave = repository.getSession(toSave.getId()); + + toSave.setAttribute("other", "value"); + repository.save(toSave); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + + assertThat(findByPrincipalName).hasSize(1); + assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId()); + } + + @Test + public void findByDeletedPrincipalName() throws Exception { + String principalName = "findByDeletedPrincipalName" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, null); + repository.save(toSave); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + + assertThat(findByPrincipalName).isEmpty(); + } + + @Test + public void findByChangedPrincipalName() throws Exception { + String principalName = "findByChangedPrincipalName" + UUID.randomUUID(); + String principalNameChanged = "findByChangedPrincipalName" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalNameChanged); + repository.save(toSave); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + assertThat(findByPrincipalName).isEmpty(); + + findByPrincipalName = repository.findByPrincipalName(principalNameChanged); + + assertThat(findByPrincipalName).hasSize(1); + assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId()); + } + + @Test + public void findByDeletedPrincipalNameReload() throws Exception { + String principalName = "findByDeletedPrincipalName" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + RedisSession getSession = repository.getSession(toSave.getId()); + getSession.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, null); + repository.save(getSession); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + + assertThat(findByPrincipalName).isEmpty(); + } + + @Test + public void findByChangedPrincipalNameReload() throws Exception { + String principalName = "findByChangedPrincipalName" + UUID.randomUUID(); + String principalNameChanged = "findByChangedPrincipalName" + UUID.randomUUID(); + RedisSession toSave = repository.createSession(); + toSave.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalName); + + repository.save(toSave); + + RedisSession getSession = repository.getSession(toSave.getId()); + + getSession.setAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME, principalNameChanged); + repository.save(getSession); + + Map findByPrincipalName = repository.findByPrincipalName(principalName); + assertThat(findByPrincipalName).isEmpty(); + + findByPrincipalName = repository.findByPrincipalName(principalNameChanged); + + assertThat(findByPrincipalName).hasSize(1); + assertThat(findByPrincipalName.keySet()).containsOnly(toSave.getId()); + } + @Configuration @EnableRedisHttpSession(redisNamespace = "RedisOperationsSessionRepositoryITests") static class Config { diff --git a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java index 8e9e166..c2ee9fe 100644 --- a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java @@ -602,6 +602,7 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess private Long originalLastAccessTime; private Map delta = new HashMap(); private boolean isNew; + private String originalPrincipalName; /** * Creates a new instance ensuring to mark all of the new attributes to be persisted in the next save operation. @@ -614,8 +615,6 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess this.isNew = true; } - - /** * Creates a new instance from the provided {@link MapSession} * @@ -624,6 +623,7 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess RedisSession(MapSession cached) { Assert.notNull("MapSession cannot be null"); this.cached = cached; + this.originalPrincipalName = cached.getAttribute(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME); } public void setNew(boolean isNew) { @@ -691,9 +691,16 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess getSessionBoundHashOperations(sessionId).putAll(delta); String key = getSessionAttrNameKey(Session.PRINCIPAL_NAME_ATTRIBUTE_NAME); if(delta.containsKey(key)) { - Object principal = delta.get(key); - String principalKey = getPrincipalKey((String) principal); - sessionRedisOperations.boundSetOps(principalKey).add(sessionId); + if(originalPrincipalName != null) { + String originalPrincipalKey = getPrincipalKey((String) originalPrincipalName); + sessionRedisOperations.boundSetOps(originalPrincipalKey).remove(sessionId); + } + String principal = (String) delta.get(key); + originalPrincipalName = principal; + if(principal != null) { + String principalKey = getPrincipalKey( principal); + sessionRedisOperations.boundSetOps(principalKey).add(sessionId); + } } delta = new HashMap(delta.size());