diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java index fe17d513..8b42e2a0 100644 --- a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java +++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java @@ -111,10 +111,10 @@ class RedisIndexedSessionRepositoryITests extends AbstractRedisITests { this.repository.save(toSave); assertThat(this.registry.receivedEvent(toSave.getId())).isTrue(); - assertThat(this.registry.getEvent(toSave.getId())).isInstanceOf(SessionCreatedEvent.class); assertThat(this.redis.boundSetOps(usernameSessionKey).members()).contains(toSave.getId()); - Session session = this.repository.findById(toSave.getId()); + SessionCreatedEvent createdEvent = this.registry.getEvent(toSave.getId()); + Session session = createdEvent.getSession(); assertThat(session.getId()).isEqualTo(toSave.getId()); assertThat(session.getAttributeNames()).isEqualTo(toSave.getAttributeNames()); 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 f0a00b25..89083294 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 @@ -462,11 +462,6 @@ public class RedisIndexedSessionRepository @Override public void save(RedisSession session) { session.save(); - if (session.isNew) { - String sessionCreatedKey = getSessionCreatedChannel(session.getId()); - this.sessionRedisOperations.convertAndSend(sessionCreatedKey, session.delta); - session.isNew = false; - } } public void cleanUpExpiredSessions() { @@ -507,7 +502,7 @@ public class RedisIndexedSessionRepository if (entries.isEmpty()) { return null; } - MapSession loaded = loadSession(id, entries); + MapSession loaded = new RedisSessionMapper(id).apply(entries); if (!allowExpired && loaded.isExpired()) { return null; } @@ -516,26 +511,6 @@ public class RedisIndexedSessionRepository return result; } - private MapSession loadSession(String id, Map entries) { - MapSession loaded = new MapSession(id); - for (Map.Entry entry : entries.entrySet()) { - String key = entry.getKey(); - if (RedisSessionMapper.CREATION_TIME_KEY.equals(key)) { - loaded.setCreationTime(Instant.ofEpochMilli((long) entry.getValue())); - } - else if (RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY.equals(key)) { - loaded.setMaxInactiveInterval(Duration.ofSeconds((int) entry.getValue())); - } - else if (RedisSessionMapper.LAST_ACCESSED_TIME_KEY.equals(key)) { - loaded.setLastAccessedTime(Instant.ofEpochMilli((long) entry.getValue())); - } - else if (key.startsWith(RedisSessionMapper.ATTRIBUTE_PREFIX)) { - loaded.setAttribute(key.substring(RedisSessionMapper.ATTRIBUTE_PREFIX.length()), entry.getValue()); - } - } - return loaded; - } - @Override public void deleteById(String sessionId) { RedisSession session = getSession(sessionId, true); @@ -568,9 +543,13 @@ public class RedisIndexedSessionRepository if (ByteUtils.startsWith(messageChannel, this.sessionCreatedChannelPrefixBytes)) { // TODO: is this thread safe? + String channel = new String(messageChannel); + String sessionId = channel.substring(channel.lastIndexOf(":") + 1); @SuppressWarnings("unchecked") - Map loaded = (Map) this.defaultSerializer.deserialize(message.getBody()); - handleCreated(loaded, new String(messageChannel)); + Map entries = (Map) this.defaultSerializer.deserialize(message.getBody()); + MapSession loaded = new RedisSessionMapper(sessionId).apply(entries); + RedisSession session = new RedisSession(loaded, false); + handleCreated(session); return; } @@ -618,9 +597,7 @@ public class RedisIndexedSessionRepository } } - private void handleCreated(Map loaded, String channel) { - String id = channel.substring(channel.lastIndexOf(":") + 1); - Session session = loadSession(id, loaded); + private void handleCreated(RedisSession session) { publishEvent(new SessionCreatedEvent(this, session)); } @@ -874,9 +851,12 @@ public class RedisIndexedSessionRepository .add(sessionId); } } - + if (this.isNew) { + String sessionCreatedKey = getSessionCreatedChannel(getId()); + RedisIndexedSessionRepository.this.sessionRedisOperations.convertAndSend(sessionCreatedKey, this.delta); + this.isNew = false; + } this.delta = new HashMap<>(this.delta.size()); - Long originalExpiration = (this.originalLastAccessTime != null) ? this.originalLastAccessTime.plus(getMaxInactiveInterval()).toEpochMilli() : null; RedisIndexedSessionRepository.this.expirationPolicy.onExpirationUpdated(originalExpiration, this); 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 c1403d3f..697749e6 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 @@ -311,7 +311,6 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void delete() { String attrName = "attrName"; MapSession expected = new MapSession(); @@ -319,10 +318,11 @@ class RedisIndexedSessionRepositoryTests { expected.setAttribute(attrName, "attrValue"); given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); - Map map = map(RedisIndexedSessionRepository.getSessionAttrNameKey(attrName), expected.getAttribute(attrName), - RedisSessionMapper.CREATION_TIME_KEY, expected.getCreationTime().toEpochMilli(), - RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) expected.getMaxInactiveInterval().getSeconds(), - RedisSessionMapper.LAST_ACCESSED_TIME_KEY, expected.getLastAccessedTime().toEpochMilli()); + Map map = map(RedisIndexedSessionRepository.getSessionAttrNameKey(attrName), + expected.getAttribute(attrName), RedisSessionMapper.CREATION_TIME_KEY, + expected.getCreationTime().toEpochMilli(), RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, + (int) expected.getMaxInactiveInterval().getSeconds(), RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + expected.getLastAccessedTime().toEpochMilli()); given(this.boundHashOperations.entries()).willReturn(map); given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); @@ -345,7 +345,6 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void getSessionNotFound() { String id = "abc"; given(this.redisOperations.boundHashOps(getKey(id))).willReturn(this.boundHashOperations); @@ -355,7 +354,6 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void getSessionFound() { String attribute1 = "attribute1"; String attribute2 = "attribute2"; @@ -365,7 +363,7 @@ class RedisIndexedSessionRepositoryTests { expected.setAttribute(attribute2, null); given(this.redisOperations.boundHashOps(getKey(expected.getId()))) .willReturn(this.boundHashOperations); - Map map = map(RedisIndexedSessionRepository.getSessionAttrNameKey(attribute1), + Map map = map(RedisIndexedSessionRepository.getSessionAttrNameKey(attribute1), expected.getAttribute(attribute1), RedisIndexedSessionRepository.getSessionAttrNameKey(attribute2), expected.getAttribute(attribute2), RedisSessionMapper.CREATION_TIME_KEY, expected.getCreationTime().toEpochMilli(), RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, @@ -386,12 +384,12 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void getSessionExpired() { String expiredId = "expired-id"; given(this.redisOperations.boundHashOps(getKey(expiredId))) .willReturn(this.boundHashOperations); - Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, Instant.now().minus(5, ChronoUnit.MINUTES).toEpochMilli()); given(this.boundHashOperations.entries()).willReturn(map); @@ -399,14 +397,14 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void findByPrincipalNameExpired() { String expiredId = "expired-id"; given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); given(this.boundSetOperations.members()).willReturn(Collections.singleton(expiredId)); given(this.redisOperations.boundHashOps(getKey(expiredId))) .willReturn(this.boundHashOperations); - Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, Instant.now().minus(5, ChronoUnit.MINUTES).toEpochMilli()); given(this.boundHashOperations.entries()).willReturn(map); @@ -416,7 +414,6 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void findByPrincipalName() { Instant lastAccessed = Instant.now().minusMillis(10); Instant createdTime = lastAccessed.minusMillis(10); @@ -426,7 +423,7 @@ class RedisIndexedSessionRepositoryTests { given(this.boundSetOperations.members()).willReturn(Collections.singleton(sessionId)); given(this.redisOperations.boundHashOps(getKey(sessionId))) .willReturn(this.boundHashOperations); - Map map = map(RedisSessionMapper.CREATION_TIME_KEY, createdTime.toEpochMilli(), + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, createdTime.toEpochMilli(), RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) maxInactive.getSeconds(), RedisSessionMapper.LAST_ACCESSED_TIME_KEY, lastAccessed.toEpochMilli()); given(this.boundHashOperations.entries()).willReturn(map); @@ -468,7 +465,10 @@ class RedisIndexedSessionRepositoryTests { String channel = "spring:session:event:0:created:" + session.getId(); JdkSerializationRedisSerializer defaultSerailizer = new JdkSerializationRedisSerializer(); this.redisRepository.setDefaultSerializer(defaultSerailizer); - byte[] body = defaultSerailizer.serialize(new HashMap()); + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 0, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5)); + byte[] body = defaultSerailizer.serialize(map); DefaultMessage message = new DefaultMessage(channel.getBytes(StandardCharsets.UTF_8), body); this.redisRepository.setApplicationEventPublisher(this.publisher); @@ -485,7 +485,10 @@ class RedisIndexedSessionRepositoryTests { byte[] pattern = "".getBytes(StandardCharsets.UTF_8); byte[] body = new byte[0]; String channel = "spring:session:event:0:created:" + session.getId(); - given(this.defaultSerializer.deserialize(body)).willReturn(new HashMap()); + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 0, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5)); + given(this.defaultSerializer.deserialize(body)).willReturn(map); DefaultMessage message = new DefaultMessage(channel.getBytes(StandardCharsets.UTF_8), body); this.redisRepository.setApplicationEventPublisher(this.publisher); @@ -497,12 +500,12 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void onMessageDeletedSessionFound() { String deletedId = "deleted-id"; given(this.redisOperations.boundHashOps(getKey(deletedId))) .willReturn(this.boundHashOperations); - Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 0, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 0, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5)); given(this.boundHashOperations.entries()).willReturn(map); @@ -525,7 +528,6 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void onMessageDeletedSessionNotFound() { String deletedId = "deleted-id"; given(this.redisOperations.boundHashOps(getKey(deletedId))) @@ -549,12 +551,12 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void onMessageExpiredSessionFound() { String expiredId = "expired-id"; given(this.redisOperations.boundHashOps(getKey(expiredId))) .willReturn(this.boundHashOperations); - Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, + Map map = map(RedisSessionMapper.CREATION_TIME_KEY, Instant.EPOCH.toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY, System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5)); given(this.boundHashOperations.entries()).willReturn(map); @@ -577,7 +579,6 @@ class RedisIndexedSessionRepositoryTests { } @Test - @SuppressWarnings("unchecked") void onMessageExpiredSessionNotFound() { String expiredId = "expired-id"; given(this.redisOperations.boundHashOps(getKey(expiredId))) @@ -813,7 +814,7 @@ class RedisIndexedSessionRepositoryTests { MapSession session = this.cached; String channel = "spring:session:event:created:1:" + session.getId(); - byte[] body = serializer.serialize(new HashMap()); + byte[] body = serializer.serialize(new HashMap<>()); DefaultMessage message = new DefaultMessage(channel.getBytes(StandardCharsets.UTF_8), body); this.redisRepository.onMessage(message, "".getBytes(StandardCharsets.UTF_8)); @@ -913,7 +914,7 @@ class RedisIndexedSessionRepositoryTests { return "spring:session:sessions:" + id; } - private Map map(Object... objects) { + private Map map(Object... objects) { Map result = new HashMap<>(); if (objects == null) { return result;