Skip to content

Commit

Permalink
[ssl/format] Tabs to spaces and add new ssl params for redis 2.9.0
Browse files Browse the repository at this point in the history
  • Loading branch information
hazendaz committed Nov 20, 2016
1 parent 350ebb3 commit e7d4f07
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 195 deletions.
7 changes: 4 additions & 3 deletions src/main/java/org/mybatis/caches/redis/RedisCache.java
Expand Up @@ -42,9 +42,10 @@ public RedisCache(final String id) {
}
this.id = id;
RedisConfig redisConfig = RedisConfigurationBuilder.getInstance().parseConfiguration();
pool = new JedisPool(redisConfig, redisConfig.getHost(), redisConfig.getPort(),
redisConfig.getConnectionTimeout(), redisConfig.getSoTimeout(), redisConfig.getPassword(),
redisConfig.getDatabase(), redisConfig.getClientName());
pool = new JedisPool(redisConfig, redisConfig.getHost(), redisConfig.getPort(),
redisConfig.getConnectionTimeout(), redisConfig.getSoTimeout(), redisConfig.getPassword(),
redisConfig.getDatabase(), redisConfig.getClientName(), redisConfig.isSsl(),
redisConfig.getSslSocketFactory(), redisConfig.getSslParameters(), redisConfig.getHostnameVerifier());
}

private Object execute(RedisCallback callback) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/mybatis/caches/redis/RedisCallback.java
Expand Up @@ -19,5 +19,5 @@

public interface RedisCallback {

Object doWithRedis(Jedis jedis);
Object doWithRedis(Jedis jedis);
}
184 changes: 112 additions & 72 deletions src/main/java/org/mybatis/caches/redis/RedisConfig.java
Expand Up @@ -15,82 +15,122 @@
*/
package org.mybatis.caches.redis;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocketFactory;

import redis.clients.jedis.JedisPoolConfig;
import redis.clients.jedis.Protocol;

public class RedisConfig extends JedisPoolConfig {

private String host = Protocol.DEFAULT_HOST;
private int port = Protocol.DEFAULT_PORT;
private int connectionTimeout = Protocol.DEFAULT_TIMEOUT;
private int soTimeout = Protocol.DEFAULT_TIMEOUT;
private String password;
private int database = Protocol.DEFAULT_DATABASE;
private String clientName;

public String getHost() {
return host;
}

public void setHost(String host) {
if (host == null || "".equals(host)) {
host = Protocol.DEFAULT_HOST;
}
this.host = host;
}

public int getPort() {
return port;
}

public void setPort(int port) {
this.port = port;
}

public String getPassword() {
return password;
}

public void setPassword(String password) {
if ("".equals(password)) {
password = null;
}
this.password = password;
}

public int getDatabase() {
return database;
}

public void setDatabase(int database) {
this.database = database;
}

public String getClientName() {
return clientName;
}

public void setClientName(String clientName) {
if ("".equals(clientName)) {
clientName = null;
}
this.clientName = clientName;
}

public int getConnectionTimeout() {
return connectionTimeout;
}

public void setConnectionTimeout(int connectionTimeout) {
this.connectionTimeout = connectionTimeout;
}

public int getSoTimeout() {
return soTimeout;
}

public void setSoTimeout(int soTimeout) {
this.soTimeout = soTimeout;
}
private String host = Protocol.DEFAULT_HOST;
private int port = Protocol.DEFAULT_PORT;
private int connectionTimeout = Protocol.DEFAULT_TIMEOUT;
private int soTimeout = Protocol.DEFAULT_TIMEOUT;
private String password;
private int database = Protocol.DEFAULT_DATABASE;
private String clientName;
private boolean ssl;
private SSLSocketFactory sslSocketFactory;
private SSLParameters sslParameters;
private HostnameVerifier hostnameVerifier;

public boolean isSsl() {
return ssl;
}

public void setSsl(boolean ssl) {
this.ssl = ssl;
}

public SSLSocketFactory getSslSocketFactory() {
return sslSocketFactory;
}

public void setSslSocketFactory(SSLSocketFactory sslSocketFactory) {
this.sslSocketFactory = sslSocketFactory;
}

public SSLParameters getSslParameters() {
return sslParameters;
}

public void setSslParameters(SSLParameters sslParameters) {
this.sslParameters = sslParameters;
}

public HostnameVerifier getHostnameVerifier() {
return hostnameVerifier;
}

public void setHostnameVerifier(HostnameVerifier hostnameVerifier) {
this.hostnameVerifier = hostnameVerifier;
}

public String getHost() {
return host;
}

public void setHost(String host) {
if (host == null || "".equals(host)) {
host = Protocol.DEFAULT_HOST;
}
this.host = host;
}

public int getPort() {
return port;
}

public void setPort(int port) {
this.port = port;
}

public String getPassword() {
return password;
}

public void setPassword(String password) {
if ("".equals(password)) {
password = null;
}
this.password = password;
}

public int getDatabase() {
return database;
}

public void setDatabase(int database) {
this.database = database;
}

public String getClientName() {
return clientName;
}

public void setClientName(String clientName) {
if ("".equals(clientName)) {
clientName = null;
}
this.clientName = clientName;
}

public int getConnectionTimeout() {
return connectionTimeout;
}

public void setConnectionTimeout(int connectionTimeout) {
this.connectionTimeout = connectionTimeout;
}

public int getSoTimeout() {
return soTimeout;
}

public void setSoTimeout(int soTimeout) {
this.soTimeout = soTimeout;
}

}

10 comments on commit e7d4f07

@harawata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new options are not simple types and RedisConfigurationBuilder throws an exception if they are specified.
The test is passing on Travis maybe because the configuration file is not correctly picked up?

org.apache.ibatis.cache.CacheException: Unsupported property type: 'hostnameVerifier' of type interface javax.net.ssl.HostnameVerifier
	at org.mybatis.caches.redis.RedisConfigurationBuilder.setConfigProperties(RedisConfigurationBuilder.java:130)
	at org.mybatis.caches.redis.RedisConfigurationBuilder.parseConfiguration(RedisConfigurationBuilder.java:100)
	at org.mybatis.caches.redis.RedisConfigurationBuilder.parseConfiguration(RedisConfigurationBuilder.java:67)
	at org.mybatis.caches.redis.RedisCache.<init>(RedisCache.java:46)
	at org.mybatis.caches.redis.RedisTestCase.newCache(RedisTestCase.java:38)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:678)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

@hazendaz
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harawata hmmm...I agree they are complex types. I don't have redis locally to test. Travis CI appears to be working. It says all 6 tests ran. Maybe because I left all values in test as null? In your testing, did you run as-is or specifiy values? I'm sure this is now a bug, basically supporting 2.9.x but not working as one would expect.

@hazendaz
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harawata I'm wondering if we can the settings on this for the complex types. Looking at just hostnameVerifier, jedis is internally creating one here. Basically they do it all so wonder if that is a good enough approach for now so it's at least supported at a basic level. I otherwise know zero with redis in general as I've never used it.

@harawata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hazendaz

In your testing, did you run as-is or specify values?

as-is. The test passes if 1) the config file does not exist or 2) the new keys don't exist in the file, so I suspected that Travis couldn't see the file for some reason.

Anyway, basic level support should be fine while it's in beta.

@harawata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I just started to see if it's possible to add the frequently-requested 'flushInterval' option.

@hazendaz
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harawata So I have something mostly picked from redis tests. It needs the peer certificates to get loaded before it might work. Not the best solution but probably a good starting point here

@harawata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, @hazendaz !
I don't have Redis + SSL setup to try the change, unfortunately.
We may have to release beta3 with it and wait for a feedback from users who actually use this feature, I guess.

@hazendaz
Copy link
Member Author

@hazendaz hazendaz commented on e7d4f07 Jul 8, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harawata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to release b3 if SSL stuff is not ready yet. Let's keep it as a SNAPSHOT for now.

@hazendaz
Copy link
Member Author

@hazendaz hazendaz commented on e7d4f07 Jul 11, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.