From 730dea42fe1adea1b0fd251aada1365f1027e6b9 Mon Sep 17 00:00:00 2001 From: HandSonic <8078023+handsonic@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:46:04 +0800 Subject: [PATCH] fix(redis): persist no-expiration TTL updates --- .../plugin/redis/RedisScriptExecutor.java | 15 ++-- .../plugin/redis/constant/RedisConstants.java | 1 + .../redis/RedisScriptExecutorUpdateTest.java | 88 ++++++++++++++++++- 3 files changed, 96 insertions(+), 8 deletions(-) diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/RedisScriptExecutor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/RedisScriptExecutor.java index 174208ea3f..497aaeb516 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/RedisScriptExecutor.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/RedisScriptExecutor.java @@ -336,11 +336,16 @@ public ExecuteResponse update(RedisKey oldKey, RedisKey newKey) { scripts.addAll(script); } } - if (newKey != null && newKey.getTtl() != null && newKey.getTtl() > 0) { - StringBuilder stringBuilder = new StringBuilder(); - stringBuilder.append(RedisConstants.COMMAND_EXPIRE_KEY_PREFIX).append(getRedisValue(newKey.getName())) - .append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(newKey.getTtl()); - scripts.add(stringBuilder.toString()); + if (newKey != null && newKey.getTtl() != null) { + if (newKey.getTtl() > 0) { + StringBuilder stringBuilder = new StringBuilder(); + stringBuilder.append(RedisConstants.COMMAND_EXPIRE_KEY_PREFIX).append(getRedisValue(newKey.getName())) + .append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(newKey.getTtl()); + scripts.add(stringBuilder.toString()); + } else if (!typeChanged && newKey.getTtl() == -1L && oldKey != null + && oldKey.getTtl() != null && oldKey.getTtl() >= 0) { + scripts.add(RedisConstants.COMMAND_PERSIST_KEY_PREFIX + getRedisValue(newKey.getName())); + } } ExecuteResponse executeResult = new ExecuteResponse(); for (String s : scripts) { diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/constant/RedisConstants.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/constant/RedisConstants.java index 2bc30ed161..903537ea64 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/constant/RedisConstants.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/main/java/ai/chat2db/plugin/redis/constant/RedisConstants.java @@ -13,6 +13,7 @@ public final class RedisConstants { public static final String COMMAND_EXISTS_KEY = "EXISTS %s"; public static final String COMMAND_EXPIRE_ARGUMENT_PREFIX = " EX "; public static final String COMMAND_EXPIRE_KEY_PREFIX = "EXPIRE "; + public static final String COMMAND_PERSIST_KEY_PREFIX = "PERSIST "; public static final String COMMAND_GET_KEY_PREFIX = "GET "; public static final String COMMAND_HASH_DELETE_PREFIX = "HDEL "; public static final String COMMAND_HASH_GET_ALL_PREFIX = "HGETALL "; diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/test/java/ai/chat2db/plugin/redis/RedisScriptExecutorUpdateTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/test/java/ai/chat2db/plugin/redis/RedisScriptExecutorUpdateTest.java index 344538784e..f96cbfab90 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/test/java/ai/chat2db/plugin/redis/RedisScriptExecutorUpdateTest.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redis/src/test/java/ai/chat2db/plugin/redis/RedisScriptExecutorUpdateTest.java @@ -1,24 +1,33 @@ package ai.chat2db.plugin.redis; +import ai.chat2db.community.domain.api.config.DriverConfig; import ai.chat2db.community.domain.api.model.result.ExecuteResponse; import ai.chat2db.plugin.redis.model.RedisKey; +import ai.chat2db.spi.model.datasource.ConnectInfo; +import ai.chat2db.spi.sql.Chat2DBContext; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import java.lang.reflect.Proxy; import java.sql.Connection; import java.sql.PreparedStatement; +import java.util.ArrayList; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; /** - * Covers update() paths that must return before touching a connection. - * These tests run without a Chat2DBContext, so reaching command execution - * would fail, which is exactly the regression they guard against. + * Covers update() command generation and paths that must return before touching a connection. */ class RedisScriptExecutorUpdateTest { + @AfterEach + void tearDown() { + Chat2DBContext.close(); + } + @Test void updateReturnsEmptyResponseWhenBothKeysAreNull() { assertNotNull(RedisScriptExecutor.getInstance().update(null, null)); @@ -32,6 +41,42 @@ void typeChangeAbortsInsteadOfDeletingWhenNewTypeHasNothingToWrite() { assertNotNull(RedisScriptExecutor.getInstance().update(oldKey, newKey)); } + @Test + void updatePersistsKeyWhenTtlChangesFromExpiringToNoExpiration() { + List commands = captureUpdateCommands(); + + RedisScriptExecutor.getInstance().update(stringKey("k", 60L), stringKey("k", -1L)); + + assertEquals(List.of("PERSIST 'k'"), commands); + } + + @Test + void updateDoesNotPersistKeyWhenTtlAlreadyHasNoExpiration() { + List commands = captureUpdateCommands(); + + RedisScriptExecutor.getInstance().update(stringKey("k", -1L), stringKey("k", -1L)); + + assertTrue(commands.isEmpty()); + } + + @Test + void updatePersistsRenamedKeyUsingQuotedNewName() { + List commands = captureUpdateCommands(); + + RedisScriptExecutor.getInstance().update(stringKey("old key", 60L), stringKey("new key's", -1L)); + + assertEquals(List.of("RENAME 'old key' 'new key\\'s'\n", "PERSIST 'new key\\'s'"), commands); + } + + @Test + void updateKeepsExpireCommandForPositiveTtl() { + List commands = captureUpdateCommands(); + + RedisScriptExecutor.getInstance().update(stringKey("k", 60L), stringKey("k", 120L)); + + assertEquals(List.of("EXPIRE 'k' 120"), commands); + } + @Test void executeUpdatePublishesExecutionMetrics() { PreparedStatement statement = (PreparedStatement) Proxy.newProxyInstance( @@ -51,4 +96,41 @@ void executeUpdatePublishesExecutionMetrics() { result.getExecutionMetrics().getExecuteDurationMs()); assertTrue(result.getExecutionMetrics().getExecuteDurationMs() >= 0L); } + + private RedisKey stringKey(String name, Long ttl) { + return RedisKey.builder() + .name(name) + .type("string") + .value("value") + .ttl(ttl) + .build(); + } + + private List captureUpdateCommands() { + List commands = new ArrayList<>(); + Connection connection = (Connection) Proxy.newProxyInstance( + Connection.class.getClassLoader(), new Class[]{Connection.class}, + (proxy, method, args) -> { + if ("prepareStatement".equals(method.getName())) { + commands.add((String) args[0]); + return updateStatement(); + } + if ("isClosed".equals(method.getName())) { + return false; + } + return null; + }); + ConnectInfo connectInfo = new ConnectInfo(); + connectInfo.setDbType("REDIS"); + connectInfo.setDriverConfig(new DriverConfig()); + connectInfo.setConnection(connection); + Chat2DBContext.putContext(connectInfo); + return commands; + } + + private PreparedStatement updateStatement() { + return (PreparedStatement) Proxy.newProxyInstance( + PreparedStatement.class.getClassLoader(), new Class[]{PreparedStatement.class}, + (proxy, method, args) -> "executeUpdate".equals(method.getName()) ? 1 : null); + } }