DATAREDIS-719 - Polishing.

Fix generics for accepted Future, Supplier and Converter. Introduce generics for status responses. Add Javadoc, use synchronous API for exists call.

Original pull request: #289.

Author: mp911de

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

@@ -26,7 +26,8 @@
  * @author Jennifer Hickey
  * @author Christoph Strobl
  * @author Mark Paluch
- * @param <T> The data type of the object that holds the future result (usually of type Future)
+ * @param <T> The data type of the object that holds the future result (usually type of the
+ *          {@link java.util.concurrent.Future} or response wrapper).
  */
 public abstract class FutureResult<T> {
 
@@ -36,7 +37,7 @@ public abstract class FutureResult<T> {
 	private boolean status = false;
 
 	@SuppressWarnings("rawtypes") //
-	protected @Nullable Converter converter;
+	protected Converter converter;
 
 	/**
 	 * Create new {@link FutureResult} for given object actually holding the result itself.
@@ -70,7 +71,6 @@ public FutureResult(T resultHolder, @Nullable Converter converter) {
 	 * @param defaultConversionResult must not be {@literal null}.
 	 * @since 2.1
 	 */
-	@SuppressWarnings("rawtypes")
 	public FutureResult(T resultHolder, @Nullable Converter converter, Supplier<?> defaultConversionResult) {
 
 		this.resultHolder = resultHolder;
@@ -99,7 +99,7 @@ public T getResultHolder() {
 	public Object convert(@Nullable Object result) {
 
 		if (result == null) {
-			return computeDefaultResult(result);
+			return computeDefaultResult(null);
 		}
 
 		return computeDefaultResult(converter.convert(result));
@@ -140,8 +140,8 @@ public void setStatus(boolean status) {
 	/**
 	 * Indicate whether or not the actual result needs to be {@link #convert(Object) converted} before handing over.
 	 *
-	 * @return
+	 * @return {@literal true} if result conversion is required.
 	 * @since 2.1
 	 */
-	public abstract boolean seeksConversion();
+	public abstract boolean conversionRequired();
 }

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

@@ -16,7 +16,6 @@
 package org.springframework.data.redis.connection.convert;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Queue;
 
@@ -31,13 +30,13 @@
  *
  * @author Jennifer Hickey
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @param <T> The type of {@link FutureResult} of the individual tx operations
  */
 public class TransactionResultConverter<T> implements Converter<List<Object>, List<Object>> {
 
-	private Queue<FutureResult<T>> txResults = new LinkedList<>();
-
-	private Converter<Exception, DataAccessException> exceptionConverter;
+	private final Queue<FutureResult<T>> txResults;
+	private final Converter<Exception, DataAccessException> exceptionConverter;
 
 	public TransactionResultConverter(Queue<FutureResult<T>> txResults,
 			Converter<Exception, DataAccessException> exceptionConverter) {
@@ -71,7 +70,7 @@ public List<Object> convert(List<Object> execResults) {
 						: new RedisSystemException("Error reading future result.", source);
 			}
 			if (!(futureResult.isStatus())) {
-				convertedResults.add(futureResult.seeksConversion() ? futureResult.convert(result) : result);
+				convertedResults.add(futureResult.conversionRequired() ? futureResult.convert(result) : result);
 			}
 		}
 

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

@@ -453,7 +453,7 @@ private List<Object> convertPipelineResults() {
 				Object data = result.get();
 
 				if (!result.isStatus()) {
-					results.add(result.seeksConversion() ? result.convert(data) : data);
+					results.add(result.conversionRequired() ? result.convert(data) : data);
 				}
 			} catch (JedisDataException e) {
 				DataAccessException dataAccessException = convertJedisAccessException(e);
@@ -618,16 +618,16 @@ JedisResult newJedisResult(Response<?> response) {
 		return JedisResultBuilder.forResponse(response).build();
 	}
 
-	<T> JedisResult newJedisResult(Response<T> response, Converter<T, ?> converter) {
+	<T, R> JedisResult newJedisResult(Response<T> response, Converter<T, R> converter) {
 
-		return JedisResultBuilder.forResponse(response).mappedWith(converter)
+		return JedisResultBuilder.<T, R> forResponse(response).mappedWith(converter)
 				.convertPipelineAndTxResults(convertPipelineAndTxResults).build();
 	}
 
-	<T> JedisResult newJedisResult(Response<T> response, Converter<T, ?> converter, Supplier<?> defaultValue) {
+	<T, R> JedisResult newJedisResult(Response<T> response, Converter<T, R> converter, Supplier<R> defaultValue) {
 
-		return JedisResultBuilder.forResponse(response).mappedWith(converter)
-				.convertPipelineAndTxResults(convertPipelineAndTxResults).defaultNullTo(defaultValue).build();
+		return JedisResultBuilder.<T, R> forResponse(response).mappedWith(converter)
+				.convertPipelineAndTxResults(convertPipelineAndTxResults).mapNullTo(defaultValue).build();
 	}
 
 	JedisStatusResult newStatusResult(Response<?> response) {

src/main/java/org/springframework/data/redis/connection/jedis/JedisResult.java

@@ -30,21 +30,23 @@
  * @author Jennifer Hickey
  * @author Christoph Strobl
  * @author Mark Paluch
+ * @param <T> The data type of the object that holds the future result (usually of type Future).
+ * @param <R> The data type of the result type.
  * @since 2.1
  */
-class JedisResult<T, S> extends FutureResult<Response<?>> {
+class JedisResult<T, R> extends FutureResult<Response<?>> {
 
 	private final boolean convertPipelineAndTxResults;
 
-	<T> JedisResult(Response<T> resultHolder) {
+	JedisResult(Response<T> resultHolder) {
 		this(resultHolder, false, null);
 	}
 
-	<T> JedisResult(Response<T> resultHolder, boolean convertPipelineAndTxResults, @Nullable Converter<T, ?> converter) {
-		this(resultHolder, null, convertPipelineAndTxResults, converter);
+	JedisResult(Response<T> resultHolder, boolean convertPipelineAndTxResults, @Nullable Converter<T, ?> converter) {
+		this(resultHolder, () -> null, convertPipelineAndTxResults, converter);
 	}
 
-	<T> JedisResult(Response<T> resultHolder, Supplier<S> defaultReturnValue, boolean convertPipelineAndTxResults,
+	JedisResult(Response<T> resultHolder, Supplier<R> defaultReturnValue, boolean convertPipelineAndTxResults,
 			@Nullable Converter<T, ?> converter) {
 
 		super(resultHolder, converter, defaultReturnValue);
@@ -58,25 +60,26 @@ <T> JedisResult(Response<T> resultHolder, Supplier<S> defaultReturnValue, boolea
 	 */
 	@Nullable
 	@Override
+	@SuppressWarnings("unchecked")
 	public T get() {
 		return (T) getResultHolder().get();
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.redis.connection.FutureResult#seeksConversion()
-	 * @return
+	 * @see org.springframework.data.redis.connection.FutureResult#conversionRequired()
 	 */
-	public boolean seeksConversion() {
-		return convertPipelineAndTxResults && converter != null;
+	public boolean conversionRequired() {
+		return convertPipelineAndTxResults;
 	}
 
 	/**
 	 * Jedis specific {@link FutureResult} implementation of a throw away status result.
 	 */
-	static class JedisStatusResult extends JedisResult {
+	static class JedisStatusResult<T, R> extends JedisResult<T, R> {
 
-		<T> JedisStatusResult(Response<T> resultHolder, Converter<T, ?> converter) {
+		@SuppressWarnings("unchecked")
+		JedisStatusResult(Response<T> resultHolder, Converter<T, R> converter) {
 
 			super(resultHolder, false, converter);
 			setStatus(true);
@@ -87,54 +90,77 @@ <T> JedisStatusResult(Response<T> resultHolder, Converter<T, ?> converter) {
 	 * Builder for constructing {@link JedisResult}.
 	 *
 	 * @param <T>
-	 * @param <S>
+	 * @param <R>
 	 * @since 2.1
 	 */
-	static class JedisResultBuilder<T, S> {
+	static class JedisResultBuilder<T, R> {
 
 		private final Response<T> response;
-		private Converter<T, ?> converter;
+		private Converter<T, R> converter;
 		private boolean convertPipelineAndTxResults = false;
-		private Supplier<?> nullValueDefault = () -> null;
+		private Supplier<R> nullValueDefault = () -> null;
 
+		@SuppressWarnings("unchecked")
 		JedisResultBuilder(Response<T> response) {
 
 			this.response = response;
-			this.converter = (source) -> source;
+			this.converter = (source) -> (R) source;
 		}
 
-		static <T> JedisResultBuilder<T, ?> forResponse(Response<T> response) {
+		/**
+		 * Create a new {@link JedisResultBuilder} given {@link Response}.
+		 * 
+		 * @param response must not be {@literal null}.
+		 * @param <T> native response type.
+		 * @param <R> resulting response type.
+		 * @return the new {@link JedisResultBuilder}.
+		 */
+		static <T, R> JedisResultBuilder<T, R> forResponse(Response<T> response) {
 			return new JedisResultBuilder<>(response);
 		}
 
-		<S> JedisResultBuilder<T, S> mappedWith(Converter<T, S> converter) {
+		/**
+		 * Configure a {@link Converter} to convert between {@code T} and {@code R} types.
+		 * 
+		 * @param converter must not be {@literal null}.
+		 * @return {@code this} builder.
+		 */
+		JedisResultBuilder<T, R> mappedWith(Converter<T, R> converter) {
 
 			this.converter = converter;
-			return (JedisResultBuilder<T, S>) this;
-		}
-
-		<S> JedisResultBuilder<T, S> defaultNullTo(S value) {
-			return (defaultNullTo(() -> value));
+			return this;
 		}
 
-		<S> JedisResultBuilder<T, S> defaultNullTo(Supplier<S> value) {
+		/**
+		 * Configure a {@link Supplier} to map {@literal null} responses to a different value.
+		 * 
+		 * @param supplier must not be {@literal null}.
+		 * @return {@code this} builder.
+		 */
+		JedisResultBuilder<T, R> mapNullTo(Supplier<R> supplier) {
 
-			this.nullValueDefault = value;
-			return (JedisResultBuilder<T, S>) this;
+			this.nullValueDefault = supplier;
+			return this;
 		}
 
-		JedisResultBuilder<T, S> convertPipelineAndTxResults(boolean flag) {
+		JedisResultBuilder<T, R> convertPipelineAndTxResults(boolean flag) {
 
 			convertPipelineAndTxResults = flag;
 			return this;
 		}
 
-		JedisResult<T, S> build() {
-			return new JedisResult(response, nullValueDefault, convertPipelineAndTxResults, converter);
+		/**
+		 * @return a new {@link JedisResult} wrapper with configuration applied from this builder.
+		 */
+		JedisResult<T, R> build() {
+			return new JedisResult<>(response, nullValueDefault, convertPipelineAndTxResults, converter);
 		}
 
+		/**
+		 * @return a new {@link JedisStatusResult} wrapper for status results with configuration applied from this builder.
+		 */
 		JedisStatusResult buildStatusResult() {
-			return new JedisStatusResult(response, converter);
+			return new JedisStatusResult<>(response, converter);
 		}
 	}
 }