Spring/spring-session
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix SessionCreatedEvent handling in RedisIndexedSessionRepository

Browse Source 
At present, RedisIndexedSessionRepository publishes a SessionCreatedEvent backed by an empty MapSession instance. This happens because session delta has been cleared before publishing a message to the session created channel.

Fixes gh-1338
This commit is contained in:
Vedran Pavic
2022-09-27 18:38:13 +02:00
committed by Rob Winch
parent 6d74cf5f35
commit 2406ec8302
3 changed files with 40 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java
View File

@@ -111,10 +111,10 @@ class RedisIndexedSessionRepositoryITests extends AbstractRedisITests {
this.repository.save(toSave);
assertThat(this.registry.receivedEvent(toSave.getId())).isTrue();
assertThat(this.registry.<SessionCreatedEvent>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());

46
spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java
View File

@@ -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<String, Object> entries) {
MapSession loaded = new MapSession(id);
for (Map.Entry<String, Object> 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<String, Object> loaded = (Map<String, Object>) this.defaultSerializer.deserialize(message.getBody());
handleCreated(loaded, new String(messageChannel));
Map<String, Object> entries = (Map<String, Object>) 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<String, Object> 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);

49
spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java
View File

@@ -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.<String, Object>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<String, Object> 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.<String, Object>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.<String, Object>boundHashOps(getKey(expected.getId())))
.willReturn(this.boundHashOperations);
Map map = map(RedisIndexedSessionRepository.getSessionAttrNameKey(attribute1),
Map<String, Object> 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.<String, Object>boundHashOps(getKey(expiredId)))
.willReturn(this.boundHashOperations);
Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY,
Map<String, Object> 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.<String, Object>boundHashOps(getKey(expiredId)))
.willReturn(this.boundHashOperations);
Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY,
Map<String, Object> 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.<String, Object>boundHashOps(getKey(sessionId)))
.willReturn(this.boundHashOperations);
Map map = map(RedisSessionMapper.CREATION_TIME_KEY, createdTime.toEpochMilli(),
Map<String, Object> 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<String, Object> 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<String, Object>());
Map<String, Object> 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.<String, Object>boundHashOps(getKey(deletedId)))
.willReturn(this.boundHashOperations);
Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 0, RedisSessionMapper.LAST_ACCESSED_TIME_KEY,
Map<String, Object> 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.<String, Object>boundHashOps(getKey(deletedId)))
@@ -549,12 +551,12 @@ class RedisIndexedSessionRepositoryTests {
}
@Test
@SuppressWarnings("unchecked")
void onMessageExpiredSessionFound() {
String expiredId = "expired-id";
given(this.redisOperations.<String, Object>boundHashOps(getKey(expiredId)))
.willReturn(this.boundHashOperations);
Map map = map(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1, RedisSessionMapper.LAST_ACCESSED_TIME_KEY,
Map<String, Object> 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.<String, Object>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<String, Object> map(Object... objects) {
Map<String, Object> result = new HashMap<>();
if (objects == null) {
return result;