From 18275be7fd6906570e8883750c0d200e43a82f58 Mon Sep 17 00:00:00 2001 From: Sumit Kumar Date: Mon, 31 May 2021 11:55:47 +0530 Subject: [PATCH] Fix: Fix redis plugin socket error (#4791) * fix socket error * fix default port * remove invalid check for missing port, since default port would be supplied if user leaves the port field empty. (cherry picked from commit e2f4fd5e94a1ca34fc703b2a94e5c773ca454d63) --- .../java/com/external/plugins/RedisPlugin.java | 16 ++++++++++++---- .../com/external/plugins/RedisPluginTest.java | 7 ++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 4a5dfcd059..f5dca31ecb 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -36,7 +36,7 @@ import java.util.stream.Collectors; import static com.appsmith.external.constants.ActionConstants.ACTION_CONFIGURATION_BODY; public class RedisPlugin extends BasePlugin { - private static final Integer DEFAULT_PORT = 6379; + private static final Long DEFAULT_PORT = 6379L; public RedisPlugin(PluginWrapper wrapper) { super(wrapper); @@ -106,6 +106,17 @@ public class RedisPlugin extends BasePlugin { result.setRequest(request); return result; }) + .doFinally(signalType -> { + /* + * - For some reason, Jedis throws a socket error when kept idle for like 10 min when + * appsmith is setup via docker image. + * - APMU, jedis.close() should disconnect the connection, causing jedis to refresh connection + * during next execution. + * - This is a placeholder solution till better fix is available (would connection pool fix + * it ?) + */ + jedis.close(); + }) .subscribeOn(scheduler); } @@ -178,9 +189,6 @@ public class RedisPlugin extends BasePlugin { if (StringUtils.isNullOrEmpty(endpoint.getHost())) { invalids.add("Missing host for endpoint"); } - if (endpoint.getPort() == null) { - invalids.add("Missing port for endpoint"); - } } DBAuth auth = (DBAuth) datasourceConfiguration.getAuthentication(); diff --git a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java index 72198786c1..58274a14d2 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java +++ b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java @@ -83,7 +83,7 @@ public class RedisPluginTest { Endpoint endpoint = new Endpoint(); invalidDatasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - Assert.assertEquals(Set.of("Missing port for endpoint", "Missing host for endpoint"), + Assert.assertEquals(Set.of("Missing host for endpoint"), pluginExecutor.validateDatasource(invalidDatasourceConfiguration)); } @@ -95,7 +95,8 @@ public class RedisPluginTest { endpoint.setHost("test-host"); invalidDatasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - Assert.assertEquals(pluginExecutor.validateDatasource(invalidDatasourceConfiguration), Set.of("Missing port for endpoint")); + // Since default port is picked, set of invalids should be empty. + Assert.assertEquals(pluginExecutor.validateDatasource(invalidDatasourceConfiguration), Set.of()); } @Test @@ -112,7 +113,7 @@ public class RedisPluginTest { invalidDatasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); Assert.assertEquals( - Set.of("Missing port for endpoint", "Missing username for authentication.", "Missing password for authentication."), + Set.of("Missing username for authentication.", "Missing password for authentication."), pluginExecutor.validateDatasource(invalidDatasourceConfiguration) ); }