DATAREDIS-719 - Rework FutureResult and implementations for Jedis/Lettuce.

We now encapsulate deferred results for pipelining and transactions entirely within FutureResult and its subtypes. FutureResult accepts a Supplier<T> for default values, if operations return null and reports whether its result requires conversion. JedisResult and LettuceResult are now top-level classes and no longer inner classes of their connection factories.

Original pull request: #289.

Author: christophstrobl

src/main/java/org/springframework/data/redis/connection/DefaultStringRedisConnection.java

@@ -15,8 +15,17 @@
  */
 package org.springframework.data.redis.connection;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.Queue;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.logging.Log;
@@ -725,7 +734,7 @@ public List<byte[]> mGet(byte[]... keys) {
 	 */
 	@Override
 	public Boolean mSet(Map<byte[], byte[]> tuple) {
-		return delegate.mSet(tuple);
+		return convertAndReturn(delegate.mSet(tuple), identityConverter);
 	}
 
 	/*
@@ -923,7 +932,7 @@ public void select(int dbIndex) {
 	 */
 	@Override
 	public Boolean set(byte[] key, byte[] value) {
-		return delegate.set(key, value);
+		return convertAndReturn(delegate.set(key, value), identityConverter);
 	}
 
 	/*
@@ -932,7 +941,7 @@ public Boolean set(byte[] key, byte[] value) {
 	 */
 	@Override
 	public Boolean set(byte[] key, byte[] value, Expiration expiration, SetOption option) {
-		return delegate.set(key, value, expiration, option);
+		return convertAndReturn(delegate.set(key, value, expiration, option), identityConverter);
 	}
 
 	/*
@@ -959,7 +968,7 @@ public void setConfig(String param, String value) {
 	 */
 	@Override
 	public Boolean setEx(byte[] key, long seconds, byte[] value) {
-		return delegate.setEx(key, seconds, value);
+		return convertAndReturn(delegate.setEx(key, seconds, value), identityConverter);
 	}
 
 	/*
@@ -968,7 +977,7 @@ public Boolean setEx(byte[] key, long seconds, byte[] value) {
 	 */
 	@Override
 	public Boolean pSetEx(byte[] key, long milliseconds, byte[] value) {
-		return delegate.pSetEx(key, milliseconds, value);
+		return convertAndReturn(delegate.pSetEx(key, milliseconds, value), identityConverter);
 	}
 
 	/*
@@ -2106,7 +2115,7 @@ public Boolean mSetNXString(Map<String, String> tuple) {
 	 */
 	@Override
 	public Boolean mSetString(Map<String, String> tuple) {
-		return delegate.mSet(serialize(tuple));
+		return mSet(serialize(tuple));
 	}
 
 	/*
@@ -2241,7 +2250,7 @@ public Long sDiffStore(String destKey, String... keys) {
 	 */
 	@Override
 	public Boolean set(String key, String value) {
-		return delegate.set(serialize(key), serialize(value));
+		return set(serialize(key), serialize(value));
 	}
 
 	/*
@@ -2268,7 +2277,7 @@ public Boolean setBit(String key, long offset, boolean value) {
 	 */
 	@Override
 	public Boolean setEx(String key, long seconds, String value) {
-		return delegate.setEx(serialize(key), seconds, serialize(value));
+		return setEx(serialize(key), seconds, serialize(value));
 	}
 
 	/*
@@ -3441,7 +3450,7 @@ public Set<byte[]> zRangeByLex(byte[] key, Range range, Limit limit) {
 	 */
 	@Override
 	public Set<String> zRangeByLex(String key) {
-		return this.zRangeByLex(key, Range.unbounded());
+		return zRangeByLex(key, Range.unbounded());
 	}
 
 	/*
@@ -3450,7 +3459,7 @@ public Set<String> zRangeByLex(String key) {
 	 */
 	@Override
 	public Set<String> zRangeByLex(String key, Range range) {
-		return this.zRangeByLex(key, range, null);
+		return zRangeByLex(key, range, null);
 	}
 
 	/*

src/main/java/org/springframework/data/redis/connection/FutureResult.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.redis.connection;
 
+import java.util.function.Supplier;
+
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.lang.Nullable;
 
@@ -28,23 +30,60 @@
  */
 public abstract class FutureResult<T> {
 
-	protected T resultHolder;
+	private T resultHolder;
+	private final Supplier<?> defaultConversionResult;
 
-	protected boolean status = false;
+	private boolean status = false;
 
 	@SuppressWarnings("rawtypes") //
 	protected @Nullable Converter converter;
 
+	/**
+	 * Create new {@link FutureResult} for given object actually holding the result itself.
+	 *
+	 * @param resultHolder must not be {@literal null}.
+	 */
 	public FutureResult(T resultHolder) {
-		this.resultHolder = resultHolder;
+		this(resultHolder, val -> val);
 	}
 
+	/**
+	 * Create new {@link FutureResult} for given object actually holding the result itself and a converter capable of
+	 * transforming the result via {@link #convert(Object)}.
+	 *
+	 * @param resultHolder must not be {@literal null}.
+	 * @param converter can be {@literal null} and will be defaulted to an identity converter {@code value -> value} to
+	 *          preserve the original value.
+	 */
+	@SuppressWarnings("rawtypes")
+	public FutureResult(T resultHolder, @Nullable Converter converter) {
+		this(resultHolder, converter, () -> null);
+	}
+
+	/**
+	 * Create new {@link FutureResult} for given object actually holding the result itself and a converter capable of
+	 * transforming the result via {@link #convert(Object)}.
+	 *
+	 * @param resultHolder must not be {@literal null}.
+	 * @param converter can be {@literal null} and will be defaulted to an identity converter {@code value -> value} to
+	 *          preserve the original value.
+	 * @param defaultConversionResult must not be {@literal null}.
+	 * @since 2.1
+	 */
 	@SuppressWarnings("rawtypes")
-	public FutureResult(T resultHolder, Converter converter) {
+	public FutureResult(T resultHolder, @Nullable Converter converter, Supplier<?> defaultConversionResult) {
+
 		this.resultHolder = resultHolder;
-		this.converter = converter;
+		this.converter = converter != null ? converter : val -> val;
+		this.defaultConversionResult = defaultConversionResult;
 	}
 
+	/**
+	 * Get the object holding the actual result.
+	 *
+	 * @return never {@literal null}.
+	 * @since 1.1
+	 */
 	public T getResultHolder() {
 		return resultHolder;
 	}
@@ -60,14 +99,18 @@ public T getResultHolder() {
 	public Object convert(@Nullable Object result) {
 
 		if (result == null) {
-			return null;
+			return computeDefaultResult(result);
 		}
 
-		return (converter != null) ? converter.convert(result) : result;
+		return computeDefaultResult(converter.convert(result));
 	}
 
-	@SuppressWarnings("rawtypes")
 	@Nullable
+	private Object computeDefaultResult(@Nullable Object source) {
+		return source != null ? source : defaultConversionResult.get();
+	}
+
+	@SuppressWarnings("rawtypes")
 	public Converter getConverter() {
 		return converter;
 	}
@@ -93,4 +136,12 @@ public void setStatus(boolean status) {
 	 */
 	@Nullable
 	public abstract Object get();
+
+	/**
+	 * Indicate whether or not the actual result needs to be {@link #convert(Object) converted} before handing over.
+	 *
+	 * @return
+	 * @since 2.1
+	 */
+	public abstract boolean seeksConversion();
 }

src/main/java/org/springframework/data/redis/connection/convert/TransactionResultConverter.java

@@ -71,7 +71,7 @@ public List<Object> convert(List<Object> execResults) {
 						: new RedisSystemException("Error reading future result.", source);
 			}
 			if (!(futureResult.isStatus())) {
-				convertedResults.add(futureResult.convert(result));
+				convertedResults.add(futureResult.seeksConversion() ? futureResult.convert(result) : result);
 			}
 		}