Spring/spring-data-redis
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAREDIS-603 - Fall back to RedisSystemException for non translateable exceptions during cluster execution.

Browse Source 
We now use a fallback exception translation strategy in JedisClusterConnection to map all non-mapped exceptions to RedisSystemException. This change prevents null pointer exceptions caused by potentially throwing null.

Original Pull Request: #275
This commit is contained in:
Mark Paluch
2017-09-06 16:07:19 +02:00
committed by Christoph Strobl
parent 2dd172e771
commit 06c37880f7
6 changed files with 92 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/main/java/org/springframework/data/redis/connection/ClusterCommandExecutor.java
View File

@@ -149,7 +149,7 @@ public class ClusterCommandExecutor implements DisposableBean {
return new NodeResult<T>(node, cmd.doInCluster(client));
} catch (RuntimeException ex) {
RuntimeException translatedException = convertToDataAccessExeption(ex);
RuntimeException translatedException = convertToDataAccessException(ex);
if (translatedException instanceof ClusterRedirectException) {
ClusterRedirectException cre = (ClusterRedirectException) translatedException;
return executeCommandOnSingleNode(cmd,
@@ -252,13 +252,13 @@ public class ClusterCommandExecutor implements DisposableBean {
}
} catch (ExecutionException e) {
RuntimeException ex = convertToDataAccessExeption((Exception) e.getCause());
RuntimeException ex = convertToDataAccessException((Exception) e.getCause());
exceptions.put(entry.getKey().getNode(), ex != null ? ex : e.getCause());
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
RuntimeException ex = convertToDataAccessExeption((Exception) e.getCause());
RuntimeException ex = convertToDataAccessException((Exception) e.getCause());
exceptions.put(entry.getKey().getNode(), ex != null ? ex : e.getCause());
break;
}
@@ -338,7 +338,7 @@ public class ClusterCommandExecutor implements DisposableBean {
return new NodeResult<T>(node, cmd.doInCluster(client, key), key);
} catch (RuntimeException ex) {
RuntimeException translatedException = convertToDataAccessExeption(ex);
RuntimeException translatedException = convertToDataAccessException(ex);
throw translatedException != null ? translatedException : ex;
} finally {
this.resourceProvider.returnResourceForSpecificNode(node, client);
@@ -349,7 +349,7 @@ public class ClusterCommandExecutor implements DisposableBean {
return this.topologyProvider.getTopology();
}
private DataAccessException convertToDataAccessExeption(Exception e) {
private DataAccessException convertToDataAccessException(Exception e) {
return exceptionTranslationStrategy.translate(e);
}

10
src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java
View File

@@ -44,7 +44,8 @@ import org.springframework.data.geo.Metric;
import org.springframework.data.geo.Point;
import org.springframework.data.redis.ClusterStateFailureException;
import org.springframework.data.redis.ExceptionTranslationStrategy;
import org.springframework.data.redis.PassThroughExceptionTranslationStrategy;
import org.springframework.data.redis.FallbackExceptionTranslationStrategy;
import org.springframework.data.redis.RedisSystemException;
import org.springframework.data.redis.connection.*;
import org.springframework.data.redis.connection.ClusterCommandExecutor.ClusterCommandCallback;
import org.springframework.data.redis.connection.ClusterCommandExecutor.MultiKeyClusterCommandCallback;
@@ -76,7 +77,7 @@ import org.springframework.util.ObjectUtils;
*/
public class JedisClusterConnection implements RedisClusterConnection {
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy(
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy(
JedisConverters.exceptionConverter());
private final Log log = LogFactory.getLog(getClass());
@@ -3980,7 +3981,10 @@ public class JedisClusterConnection implements RedisClusterConnection {
*/
protected DataAccessException convertJedisAccessException(Exception ex) {
return EXCEPTION_TRANSLATION.translate(ex);
DataAccessException translated = EXCEPTION_TRANSLATION.translate(ex);
return translated != null ? translated : new RedisSystemException(ex.getMessage(), ex);
}
/*

5
src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java
View File

@@ -20,8 +20,6 @@ import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -42,6 +40,7 @@ import org.springframework.data.geo.Point;
import org.springframework.data.redis.ExceptionTranslationStrategy;
import org.springframework.data.redis.FallbackExceptionTranslationStrategy;
import org.springframework.data.redis.RedisConnectionFailureException;
import org.springframework.data.redis.RedisSystemException;
import org.springframework.data.redis.connection.AbstractRedisConnection;
import org.springframework.data.redis.connection.DataType;
import org.springframework.data.redis.connection.FutureResult;
@@ -244,7 +243,7 @@ public class JedisConnection extends AbstractRedisConnection {
broken = true;
}
return exception;
return exception != null ? exception : new RedisSystemException(ex.getMessage(), ex);
}
public Object execute(String command, byte[]... args) {

33
src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java
View File

@@ -47,6 +47,7 @@ import org.springframework.data.geo.Point;
import org.springframework.data.redis.ExceptionTranslationStrategy;
import org.springframework.data.redis.FallbackExceptionTranslationStrategy;
import org.springframework.data.redis.RedisConnectionFailureException;
import org.springframework.data.redis.RedisSystemException;
import org.springframework.data.redis.connection.AbstractRedisConnection;
import org.springframework.data.redis.connection.DataType;
import org.springframework.data.redis.connection.FutureResult;
@@ -124,7 +125,7 @@ import com.lambdaworks.redis.sentinel.api.StatefulRedisSentinelConnection;
/**
* {@code RedisConnection} implementation on top of <a href="https://github.com/mp911de/lettuce">Lettuce</a> Redis
* client.
*
*
* @author Costin Leau
* @author Jennifer Hickey
* @author Christoph Strobl
@@ -181,7 +182,10 @@ public class LettuceConnection extends AbstractRedisConnection {
}
return resultHolder.getOutput().get();
} catch (Exception e) {
throw EXCEPTION_TRANSLATION.translate(e);
DataAccessException translated = EXCEPTION_TRANSLATION.translate(e);
throw translated != null ? translated : new RedisSystemException(e.getMessage(), e);
}
}
}
@@ -259,7 +263,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Instantiates a new lettuce connection.
*
*
* @param timeout The connection timeout (in milliseconds)
* @param client The {@link RedisClient} to use when instantiating a native connection
*/
@@ -269,7 +273,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Instantiates a new lettuce connection.
*
*
* @param timeout The connection timeout (in milliseconds) * @param client The {@link RedisClient} to use when
* instantiating a pub/sub connection
* @param pool The connection pool to use for all other native connections
@@ -280,7 +284,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Instantiates a new lettuce connection.
*
*
* @param sharedConnection A native connection that is shared with other {@link LettuceConnection}s. Will not be used
* for transactions or blocking operations
* @param timeout The connection timeout (in milliseconds)
@@ -292,7 +296,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Instantiates a new lettuce connection.
*
*
* @param sharedConnection A native connection that is shared with other {@link LettuceConnection}s. Should not be
* used for transactions or blocking operations
* @param timeout The connection timeout (in milliseconds)
@@ -332,7 +336,8 @@ public class LettuceConnection extends AbstractRedisConnection {
if (exception instanceof RedisConnectionFailureException) {
broken = true;
}
return exception;
return exception != null ? exception : new RedisSystemException(ex.getMessage(), ex);
}
@SuppressWarnings({ "rawtypes", "unchecked" })
@@ -352,7 +357,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* 'Native' or 'raw' execution of the given command along-side the given arguments.
*
*
* @see RedisCommands#execute(String, byte[]...)
* @param command Command to execute
* @param commandOutputTypeHint Type of Output to use, may be (may be {@literal null}).
@@ -1173,7 +1178,7 @@ public class LettuceConnection extends AbstractRedisConnection {
}
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisKeyCommands#ttl(byte[])
*/
@@ -1198,7 +1203,7 @@ public class LettuceConnection extends AbstractRedisConnection {
}
}
/*
/*
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisKeyCommands#ttl(byte[], java.util.concurrent.TimeUnit)
*/
@@ -3888,7 +3893,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Specifies if pipelined and transaction results should be converted to the expected data type. If false, results of
* {@link #closePipeline()} and {@link #exec()} will be of the type returned by the Lettuce driver
*
*
* @param convertPipelineAndTxResults Whether or not to convert pipeline and tx results
*/
public void setConvertPipelineAndTxResults(boolean convertPipelineAndTxResults) {
@@ -4161,7 +4166,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* {@link TypeHints} provide {@link CommandOutput} information for a given {@link CommandType}.
*
*
* @since 1.2.1
*/
static class TypeHints {
@@ -4322,7 +4327,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Returns the {@link CommandOutput} mapped for given {@link CommandType} or {@link ByteArrayOutput} as default.
*
*
* @param type
* @return {@link ByteArrayOutput} as default when no matching {@link CommandOutput} available.
*/
@@ -4333,7 +4338,7 @@ public class LettuceConnection extends AbstractRedisConnection {
/**
* Returns the {@link CommandOutput} mapped for given {@link CommandType} given {@link CommandOutput} as default.
*
*
* @param type
* @return
*/

18
src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionUnitTests.java
View File

@@ -16,6 +16,7 @@
package org.springframework.data.redis.connection.jedis;
import static org.hamcrest.Matchers.*;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.*;
import static org.mockito.Matchers.anyVararg;
import static org.mockito.Matchers.eq;
@@ -48,6 +49,7 @@ import org.mockito.invocation.InvocationOnMock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
import org.springframework.data.redis.ClusterStateFailureException;
import org.springframework.data.redis.RedisSystemException;
import org.springframework.data.redis.connection.ClusterInfo;
import org.springframework.data.redis.connection.RedisClusterCommands.AddSlots;
import org.springframework.data.redis.connection.RedisClusterNode;
@@ -108,7 +110,7 @@ public class JedisClusterConnectionUnitTests {
}
@Test // DATAREDIS-315
public void thowsExceptionWhenClusterCommandExecturorIsNull() {
public void throwsExceptionWhenClusterCommandExecutorIsNull() {
expectedException.expect(IllegalArgumentException.class);
@@ -353,6 +355,20 @@ public class JedisClusterConnectionUnitTests {
new JedisClusterTopologyProvider(clusterMock).getTopology();
}
@Test // DATAREDIS-603
public void translatesUnknownExceptions() {
IllegalArgumentException exception = new IllegalArgumentException("Aw, snap!");
expectedException.expect(RedisSystemException.class);
expectedException.expectMessage(exception.getMessage());
expectedException.expectCause(is(exception));
when(clusterMock.set("foo".getBytes(), "bar".getBytes())).thenThrow(exception);
connection.set("foo".getBytes(), "bar".getBytes());
}
static class StubJedisCluster extends JedisCluster {
JedisClusterConnectionHandler connectionHandler;

42
src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionUnitTestSuite.java
View File

@@ -15,6 +15,8 @@
*/
package org.springframework.data.redis.connection.lettuce;
import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.*;
@@ -25,6 +27,7 @@ import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Suite;
import org.mockito.Mockito;
import org.springframework.dao.InvalidDataAccessResourceUsageException;
import org.springframework.data.redis.connection.AbstractConnectionUnitTestBase;
import org.springframework.data.redis.connection.RedisServerCommands.ShutdownOption;
@@ -32,10 +35,12 @@ import org.springframework.data.redis.connection.lettuce.LettuceConnectionUnitTe
import org.springframework.data.redis.connection.lettuce.LettuceConnectionUnitTestSuite.LettucePipelineConnectionUnitTests;
import com.lambdaworks.redis.RedisClient;
import com.lambdaworks.redis.RedisFuture;
import com.lambdaworks.redis.api.StatefulRedisConnection;
import com.lambdaworks.redis.api.async.RedisAsyncCommands;
import com.lambdaworks.redis.api.sync.RedisCommands;
import com.lambdaworks.redis.codec.RedisCodec;
import com.lambdaworks.redis.protocol.RedisCommand;
/**
* @author Christoph Strobl
@@ -138,6 +143,43 @@ public class LettuceConnectionUnitTestSuite {
verify(syncCommandsMock, times(1)).select(1);
}
@Test // DATAREDIS-603
public void translatesUnknownExceptions() {
IllegalArgumentException exception = new IllegalArgumentException("Aw, snap!");
when(syncCommandsMock.set(any(), any())).thenThrow(exception);
connection = new LettuceConnection(null, 0, clientMock, null, 1);
try {
connection.set("foo".getBytes(), "bar".getBytes());
} catch (Exception e) {
assertThat(e.getMessage(), containsString(exception.getMessage()));
assertThat(e.getCause(), is((Throwable) exception));
}
}
@Test // DATAREDIS-603
public void translatesPipelineUnknownExceptions() throws Exception {
IllegalArgumentException exception = new IllegalArgumentException("Aw, snap!");
RedisCommand future = mock(RedisCommand.class, Mockito.withSettings().extraInterfaces(RedisFuture.class));
when(future.getOutput()).thenThrow(exception);
when(asyncCommandsMock.set(any(byte[].class), any(byte[].class))).thenReturn((RedisFuture) future);
connection = new LettuceConnection(null, 0, clientMock, null, 1);
connection.openPipeline();
try {
connection.set("foo".getBytes(), "bar".getBytes());
} catch (Exception e) {
assertThat(e.getMessage(), containsString(exception.getMessage()));
assertThat(e.getCause(), is((Throwable) exception));
}
}
}
public static class LettucePipelineConnectionUnitTests extends LettuceConnectionUnitTests {