From 180cf6275fb1bebaecde49d8432292863869cbff Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Tue, 6 Jun 2023 14:00:25 -0300 Subject: [PATCH] Improve handling of failed rename operation Consider messages that starts with "ERR no such key". Some Redis clients like Redisson might change the message to include more details Closes gh-1743 --- .../RedisIndexedSessionRepositoryITests.java | 36 ++++++++++++++++++- .../redis/RedisIndexedSessionRepository.java | 6 ++-- 2 files changed, 39 insertions(+), 3 deletions(-) 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 08745fab..821929a7 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 @@ -1,5 +1,5 @@ /* - * Copyright 2014-2021 the original author or authors. + * Copyright 2014-2023 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. @@ -20,6 +20,7 @@ import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.UUID; +import io.lettuce.core.RedisCommandExecutionException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -27,6 +28,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.RedisSystemException; import org.springframework.data.redis.connection.DefaultMessage; import org.springframework.data.redis.core.RedisOperations; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -45,8 +47,13 @@ import org.springframework.session.events.SessionDestroyedEvent; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.willThrow; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.spy; /** * Integration tests for {@link RedisIndexedSessionRepository}. @@ -682,6 +689,33 @@ class RedisIndexedSessionRepositoryITests extends AbstractRedisITests { assertThat(this.repository.findById(copy2.getId())).isNull(); } + // gh-1743 + @Test + @SuppressWarnings("unchecked") + void saveChangeSessionIdWhenFailedRenameOperationExceptionContainsMoreDetailsThenIgnoreError() { + RedisOperations sessionRedisOperations = (RedisOperations) ReflectionTestUtils + .getField(this.repository, "sessionRedisOperations"); + RedisOperations spyOperations = spy(sessionRedisOperations); + ReflectionTestUtils.setField(this.repository, "sessionRedisOperations", spyOperations); + + RedisSession toSave = this.repository.createSession(); + String sessionId = toSave.getId(); + + this.repository.save(toSave); + RedisIndexedSessionRepository.RedisSession session = this.repository.findById(sessionId); + this.repository.deleteById(sessionId); + String newSessionId = session.changeSessionId(); + + RedisSystemException redisSystemException = new RedisSystemException(null, + new RedisCommandExecutionException("ERR no such key. channel: [id: 0xec125091,...")); + willThrow(redisSystemException).given(spyOperations).rename(any(), any()); + + this.repository.save(session); + assertThat(this.repository.findById(sessionId)).isNull(); + assertThat(this.repository.findById(newSessionId)).isNull(); + reset(spyOperations); + } + private String getSecurityName() { return this.context.getAuthentication().getName(); } 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 19f3e0a2..163f9089 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 @@ -1,5 +1,5 @@ /* - * Copyright 2014-2021 the original author or authors. + * Copyright 2014-2023 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. @@ -52,6 +52,7 @@ import org.springframework.session.events.SessionDestroyedEvent; import org.springframework.session.events.SessionExpiredEvent; import org.springframework.session.web.http.SessionRepositoryFilter; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** *

@@ -870,7 +871,8 @@ public class RedisIndexedSessionRepository } private void handleErrNoSuchKeyError(NonTransientDataAccessException ex) { - if (!"ERR no such key".equals(NestedExceptionUtils.getMostSpecificCause(ex).getMessage())) { + String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage(); + if (!StringUtils.startsWithIgnoreCase(message, "ERR no such key")) { throw ex; } }