Skip to content

Commit

Permalink
Fix: Fix redis plugin socket error (#4791)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
sumitsum authored May 31, 2021
1 parent f75ec78 commit e2f4fd5
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
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);
Expand Down Expand Up @@ -106,6 +106,17 @@ public Mono<ActionExecutionResult> execute(Jedis jedis,
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);
}

Expand Down Expand Up @@ -178,9 +189,6 @@ public Set<String> validateDatasource(DatasourceConfiguration datasourceConfigur
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void itShouldValidateDatasourceWithInvalidEndpoint() {
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));
}

Expand All @@ -95,7 +95,8 @@ public void itShouldValidateDatasourceWithEmptyPort() {
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
Expand All @@ -112,7 +113,7 @@ public void itShouldValidateDatasourceWithInvalidAuth() {
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)
);
}
Expand Down

0 comments on commit e2f4fd5

Please sign in to comment.