From 0cb8ebc667e5d19c314511be1d7082e88d3b99cc Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 5 Aug 2025 13:16:49 +0200 Subject: [PATCH 1/3] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9a9e56021c..036aa79717 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 4.0.0-SNAPSHOT + 4.0.x-GH-3191-SNAPSHOT Spring Data Redis Spring Data module for Redis From 6ac045dd1767cfee10a55d8aa3bd049f75b22593 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 5 Aug 2025 14:29:34 +0200 Subject: [PATCH 2/3] Update RedisCommands. --- .../data/redis/core/RedisCommand.java | 91 +++++++++++++++++-- .../redis/core/RedisCommandUnitTests.java | 2 +- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/RedisCommand.java b/src/main/java/org/springframework/data/redis/core/RedisCommand.java index 4d9caeb4bc..90b794c22a 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisCommand.java +++ b/src/main/java/org/springframework/data/redis/core/RedisCommand.java @@ -45,25 +45,43 @@ public enum RedisCommand { // -- A APPEND("rw", 2, 2), // AUTH("rw", 1, 1), // + // -- B BGREWRITEAOF("r", 0, 0, "bgwriteaof"), // BGSAVE("r", 0, 0), // BITCOUNT("r", 1, 3), // + BITFIELD("rw", 1), // + BITFIELD_RO("r", 1), BITOP("rw", 3), // BITPOS("r", 2, 4), // + BLMOVE("rw", 4), // + BLMPOP("rw", 4), // BLPOP("rw", 2), // BRPOP("rw", 2), // BRPOPLPUSH("rw", 3), // + BZMPOP("rw", 3), // + BZPOPMAX("rw", 2), // + BZPOPMIN("rw", 2), // + // -- C + CLIENT_GETREDIR("r", 0, 0), // + CLIENT_ID("r", 0, 0), // + CLIENT_INFO("r", 0, 0), // CLIENT_KILL("rw", 1, 1), // CLIENT_LIST("r", 0, 0), // CLIENT_GETNAME("r", 0, 0), // CLIENT_PAUSE("rw", 1, 1), // + CLIENT_SETINFO("w", 1), // CLIENT_SETNAME("w", 1, 1), // + CLIENT_NO_EVICT("w", 1, 1, "client no-evict"), // + CLIENT_NO_TOUCH("w", 1, 1, "client no-touch"), // + CLIENT_TRACKING("rw", 1), // CONFIG_GET("r", 1, 1, "getconfig"), // CONFIG_REWRITE("rw", 0, 0), // CONFIG_SET("w", 2, 2, "setconfig"), // CONFIG_RESETSTAT("w", 0, 0, "resetconfigstats"), // + COPY("rw", 2), // + // -- D DBSIZE("r", 0, 0), // DECR("w", 1, 1), // @@ -71,39 +89,62 @@ public enum RedisCommand { DEL("rw", 1), // DISCARD("rw", 0, 0), // DUMP("r", 1, 1), // + // -- E ECHO("r", 1, 1), // EVAL("rw", 2), // + EVAL_RO("r", 2), // EVALSHA("rw", 2), // + EVALSHA_RO("r", 2), // EXEC("rw", 0, 0), // EXISTS("r", 1, 1), // EXPIRE("rw", 2), // EXPIREAT("rw", 2), // + EXPIRETIME("r", 1), // + // -- F + FCALL("rw", 2), // + FCALL_RO("r", 2), // FLUSHALL("w", 0, 0), // FLUSHDB("w", 0, 0), // + FUNCTION_DELETE("w", 1), // + FUNCTION_DUMP("w", 0, 0), // + FUNCTION_FLUSH("w", 0, 0), // + FUNCTION_KILL("w", 0, 0), // + // -- G GET("r", 1, 1), // GETBIT("r", 2, 2), // + GETDEL("rw", 1), // + GETEX("rw", 1), // GETRANGE("r", 3, 3), // GETSET("rw", 2, 2), // GEOADD("w", 3), // GEODIST("r", 2), // GEOHASH("r", 2), // GEOPOS("r", 2), // - GEORADIUS("r", 4), // - GEORADIUSBYMEMBER("r", 3), // + GEORADIUS("rw", 4), // + GEORADIUS_RO("r", 4), // + GEORADIUSBYMEMBER("rw", 3), // + GEORADIUSBYMEMBER_RO("r", 3), // + GEOSEARCH("r", 1), // + GEOSEARCH_STORE("rw", 1), // + // -- H HDEL("rw", 2), // + HELLO("rw", 0, 0), // HEXISTS("r", 2, 2), // HGET("r", 2, 2), // HGETALL("r", 1, 1), // + HGETDEL("rw", 2), // + HGETEX("rw", 2), // HINCRBY("rw", 3, 3), // HINCBYFLOAT("rw", 3, 3), // HKEYS("r", 1), // HLEN("r", 1), // HMGET("r", 2), // HMSET("w", 3), // + HPOP("rw", 3), HSET("w", 3, 3), // HSETNX("w", 3, 3), // HVALS("r", 1, 1), // @@ -111,27 +152,39 @@ public enum RedisCommand { HEXPIREAT("w", 5), // HPEXPIRE("w", 5), // HPEXPIREAT("w", 5), // + HPEXPIRETIME("r", 4), // HPERSIST("w", 4), // HTTL("r", 4), // HPTTL("r", 4), // + HSCAN("r", 2), // + HSTRLEN("r", 2), // + // -- I INCR("rw", 1), // + INCRBY("rw", 2, 2), // INCRBYFLOAT("rw", 2, 2), // INFO("r", 0), // + // -- K KEYS("r", 1), // + // -- L + LCS("r", 2), // LASTSAVE("r", 0), // LINDEX("r", 2, 2), // LINSERT("rw", 4, 4), // LLEN("r", 1, 1), // + LMOVE("rw", 2), // + LMPOP("rw", 2), // LPOP("rw", 1, 1), // + LPOS("r", 2), // LPUSH("rw", 2), // LPUSHX("rw", 2), // LRANGE("r", 3, 3), // LREM("rw", 3, 3), // LSET("w", 3, 3), // LTRIM("w", 3, 3), // + // -- M MGET("r", 1), // MIGRATE("rw", 0), // @@ -140,19 +193,26 @@ public enum RedisCommand { MSET("w", 2), // MSETNX("w", 2), // MULTI("rw", 0, 0), // + // -- P PERSIST("rw", 1, 1), // PEXPIRE("rw", 2), // PEXPIREAT("rw", 2), // + PEXPIRETIME("r", 1), // + PFADD("w", 10), // + PFCOUNT("r", 1), // + PFMERGE("rw", 2), // PING("r", 0, 0), // PSETEX("w", 3), // PSUBSCRIBE("r", 1), // PTTL("r", 1, 1), // // -- Q QUIT("rw", 0, 0), // + // -- R RANDOMKEY("r", 0, 0), // - + READONLY("w", 0, 0), // + READWRITE("w", 0, 0), // RENAME("w", 2, 2), // RENAMENX("w", 2, 2), // REPLICAOF("w", 2), // @@ -161,9 +221,11 @@ public enum RedisCommand { RPOPLPUSH("rw", 2, 2), // RPUSH("rw", 2), // RPUSHX("rw", 2, 2), // + // -- S SADD("rw", 2), // SAVE("rw", 0, 0), // + SCAN("r", 1), // SCARD("r", 1, 1), // SCRIPT_EXISTS("r", 1), // SCRIPT_FLUSH("rw", 0, 0), // @@ -179,6 +241,7 @@ public enum RedisCommand { SETRANGE("rw", 3, 3), // SHUTDOWN("rw", 0), // SINTER("r", 1), // + SINTERCARD("r", 1), // SINTERSTORE("rw", 2), // SISMEMBER("r", 2), // SLAVEOF("w", 2), // @@ -186,21 +249,39 @@ public enum RedisCommand { SMEMBERS("r", 1, 1), // SMOVE("rw", 3, 3), // SORT("rw", 1), // + SORT_RO("r", 1), // SPOP("rw", 1, 1), // SRANDMEMBER("r", 1, 1), // SREM("rw", 2), // + SSCAN("r", 1), // STRLEN("r", 1, 1), // SUBSCRIBE("rw", 1), // + SUBSTR("r", 3), // SUNION("r", 1), // SUNIONSTORE("rw ", 2), // SYNC("rw", 0, 0), // + // -- T TIME("r", 0, 0), // TTL("r", 1, 1), // TYPE("r", 1, 1), // + // -- U + UNLINK("w", 1), // UNSUBSCRIBE("rw", 0), // UNWATCH("rw", 0, 0), // + + // -- V + VADD("w", 3), // + VCARD("r", 1), // + VDIM("r", 1), // + VEMB("r", 2), // + VISMEMBER("r", 2), // + VLINKS("r", 2, 3), // + VRANDMEMBER("r", 1, 2), // + VREM("w", 2), // + VSIM("w", 1), // + // -- W WATCH("rw", 1), // // -- Z @@ -220,10 +301,8 @@ public enum RedisCommand { ZREVRANK("r", 2, 2), // ZSCORE("r", 2, 2), // ZUNIONSTORE("rw", 3), // - SCAN("r", 1), // - SSCAN("r", 2), // - HSCAN("r", 2), // ZSCAN("r", 2), // + // -- UNKNOWN / DEFAULT UNKNOWN("rw", -1); diff --git a/src/test/java/org/springframework/data/redis/core/RedisCommandUnitTests.java b/src/test/java/org/springframework/data/redis/core/RedisCommandUnitTests.java index e4d376de82..5634301274 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisCommandUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisCommandUnitTests.java @@ -135,7 +135,7 @@ void commandRequiresExactNumberOfArgumentsIsCorrect() { Arrays.stream(RedisCommand.values()) .forEach(command -> assertThat(command.requiresExactNumberOfArguments()) - .describedAs("Redis command [%s] failed requires exact arguments check").isEqualTo( + .describedAs("Redis command [%s] failed requires exact arguments check".formatted(command.name())).isEqualTo( ReflectionTestUtils.getField(command, "minArgs") == ReflectionTestUtils.getField(command, "maxArgs"))); } From 565f701c99edcf0e84f7e49db2ea519bfa75bd39 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 6 Aug 2025 12:13:34 +0200 Subject: [PATCH 3/3] Remove connection wrapping in StringRedisTemplate --- .../data/redis/core/StringRedisTemplate.java | 5 - .../core/RedisTemplateIntegrationTests.java | 5 +- ...TransactionalStringRedisTemplateTests.java | 141 ++++++++++++++++++ 3 files changed, 144 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/springframework/data/redis/core/TransactionalStringRedisTemplateTests.java diff --git a/src/main/java/org/springframework/data/redis/core/StringRedisTemplate.java b/src/main/java/org/springframework/data/redis/core/StringRedisTemplate.java index e2197f38b7..124404b1b8 100644 --- a/src/main/java/org/springframework/data/redis/core/StringRedisTemplate.java +++ b/src/main/java/org/springframework/data/redis/core/StringRedisTemplate.java @@ -55,9 +55,4 @@ public StringRedisTemplate(RedisConnectionFactory connectionFactory) { setConnectionFactory(connectionFactory); afterPropertiesSet(); } - - protected RedisConnection preProcessConnection(RedisConnection connection, boolean existingConnection) { - return new DefaultStringRedisConnection(connection); - } - } diff --git a/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java b/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java index 44753946d2..0c6b837ff6 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisTemplateIntegrationTests.java @@ -37,6 +37,7 @@ import org.springframework.data.redis.Person; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.DataType; +import org.springframework.data.redis.connection.DefaultStringRedisConnection; import org.springframework.data.redis.connection.ExpirationOptions; import org.springframework.data.redis.connection.RedisClusterConnection; import org.springframework.data.redis.connection.RedisConnection; @@ -161,7 +162,7 @@ void testTemplateNotInitialized() throws Exception { void testStringTemplateExecutesWithStringConn() { assumeThat(redisTemplate instanceof StringRedisTemplate).isTrue(); String value = redisTemplate.execute((RedisCallback) connection -> { - StringRedisConnection stringConn = (StringRedisConnection) connection; + StringRedisConnection stringConn = new DefaultStringRedisConnection(connection); stringConn.set("test", "it"); return stringConn.get("test"); }); @@ -307,7 +308,7 @@ void testExecutePipelinedCustomSerializer() { assumeThat(redisTemplate instanceof StringRedisTemplate).isTrue(); List results = redisTemplate.executePipelined((RedisCallback) connection -> { - StringRedisConnection stringRedisConn = (StringRedisConnection) connection; + StringRedisConnection stringRedisConn = new DefaultStringRedisConnection(connection); stringRedisConn.set("foo", "5"); stringRedisConn.get("foo"); stringRedisConn.rPush("foolist", "10"); diff --git a/src/test/java/org/springframework/data/redis/core/TransactionalStringRedisTemplateTests.java b/src/test/java/org/springframework/data/redis/core/TransactionalStringRedisTemplateTests.java new file mode 100644 index 0000000000..4b2ffe55f2 --- /dev/null +++ b/src/test/java/org/springframework/data/redis/core/TransactionalStringRedisTemplateTests.java @@ -0,0 +1,141 @@ +/* + * Copyright 2025 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.stream.Stream; + +import javax.sql.DataSource; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.AfterParameterizedClassInvocation; +import org.junit.jupiter.params.BeforeParameterizedClassInvocation; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.aggregator.ArgumentsAccessor; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mockito; +import org.springframework.context.Lifecycle; +import org.springframework.data.redis.SettingsUtils; +import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; +import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; +import org.springframework.jdbc.datasource.DataSourceTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +/** + * @author Christoph Strobl + */ +@ParameterizedClass +@MethodSource("argumentsStream") +public class TransactionalStringRedisTemplateTests { + + RedisConnectionFactory redisConnectionFactory; + StringRedisTemplate stringTemplate; + + TransactionalStringRedisTemplateTests(RedisConnectionFactory redisConnectionFactory) { + this.redisConnectionFactory = redisConnectionFactory; + + if (redisConnectionFactory instanceof Lifecycle lifecycleBean) { + lifecycleBean.start(); + } + } + + @BeforeEach + void beforeEach() { + + stringTemplate = new StringRedisTemplate(redisConnectionFactory); + + // explicitly enable transaction support + stringTemplate.setEnableTransactionSupport(true); + stringTemplate.afterPropertiesSet(); + + stringTemplate.execute((RedisCallback) con -> { + con.flushDb(); + return null; + }); + } + + @AfterEach + void afterEach() { + redisConnectionFactory.getConnection().flushAll(); + } + + @Test // GH-3191 + void visibilityDuringManagedTransaction() throws SQLException { + + stringTemplate.opsForSet().add("myset", "outside"); + + DataSource ds = Mockito.mock(DataSource.class); + Mockito.when(ds.getConnection()).thenReturn(Mockito.mock(Connection.class)); + + DataSourceTransactionManager txMgr = new DataSourceTransactionManager(ds); + + TransactionTemplate txTemplate = new TransactionTemplate(txMgr); + txTemplate.afterPropertiesSet(); + Map result = txTemplate.execute(x -> { + + Map operationAndOutput = new LinkedHashMap<>(); + // visible since set outside of tx + operationAndOutput.put("isMember(outside)", stringTemplate.opsForSet().isMember("myset", "outside")); + + // add happens inside multi/exec + operationAndOutput.put("add", stringTemplate.opsForSet().add("myset", "inside")); + + // changes not visible though inside of tx, but command is not part of multi/exec block + operationAndOutput.put("isMember(inside)", stringTemplate.opsForSet().isMember("myset", "inside")); + + return operationAndOutput; + }); + + assertThat(result).containsEntry("isMember(outside)", true).containsEntry("add", null) + .containsEntry("isMember(inside)", false); + } + + static Stream argumentsStream() { + + LettuceConnectionFactory lcf = new LettuceConnectionFactory(SettingsUtils.standaloneConfiguration()); + lcf.afterPropertiesSet(); + + JedisConnectionFactory jcf = new JedisConnectionFactory(SettingsUtils.standaloneConfiguration()); + jcf.afterPropertiesSet(); + + return Stream.of(Arguments.of(lcf), Arguments.of(jcf)); + } + + @AfterParameterizedClassInvocation + static void afterInvocation(ArgumentsAccessor accessor) { + Object o = accessor.get(0); + if (o instanceof Lifecycle lifecycle) { + lifecycle.stop(); + } + } + + @BeforeParameterizedClassInvocation + static void beforeInvocation(ArgumentsAccessor accessor) { + Object o = accessor.get(0); + if (o instanceof Lifecycle lifecycle) { + lifecycle.start(); + } + } +}