Pass-thru byte arrays in entities into Bucket.

MappingRedisConverter now accepts data for byte arrays as single property to avoid flattening out binary data into a collection form. We still accept data in a collection form to retain compatibility.

Closes: #1981
Original Pull Request: #2015

src/main/asciidoc/new-features.adoc

@@ -3,6 +3,11 @@
 
 This section briefly covers items that are new and noteworthy in the latest releases.
 
+[[new-in-2.5.0]]
+== New in Spring Data Redis 2.5
+
+* `MappingRedisConverter` no longer converts <<redis.repositories.mapping,byte arrays to a collection>> representation.
+
 [[new-in-2.4.0]]
 == New in Spring Data Redis 2.4
 

src/main/asciidoc/reference/redis-repositories.adoc

@@ -131,6 +131,10 @@ The following table describes the default mapping rules:
 | String firstname = "rand";
 | firstname = "rand"
 
+| Byte array (`byte[]`)
+| byte[] image = "rand".getBytes();
+| image = "rand"
+
 | Complex Type +
 (for example, Address)
 | Address address = new Address("emond's field");

src/main/java/org/springframework/data/redis/core/convert/Bucket.java

@@ -97,7 +97,7 @@ public class Bucket {
 	/**
 	 * Get value assigned with path.
 	 *
-	 * @param path path must not be {@literal null} or {@link String#isEmpty()}.
+	 * @param path must not be {@literal null} or {@link String#isEmpty()}.
 	 * @return {@literal null} if not set.
 	 */
 	@Nullable
@@ -107,6 +107,17 @@ public class Bucket {
 		return data.get(path);
 	}
 
+	/**
+	 * Return whether {@code path} is associated with a non-{@code null} value.
+	 *
+	 * @param path must not be {@literal null} or {@link String#isEmpty()}.
+	 * @return {@literal true} if the {@code path} is associated with a non-{@code null} value.
+	 * @since 2.5
+	 */
+	public boolean hasValue(String path) {
+		return get(path) != null;
+	}
+
 	/**
 	 * A set view of the mappings contained in this bucket.
 	 *

src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java

@@ -283,8 +283,18 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 
 		if (typeInformation.isCollectionLike()) {
 
-			return readCollectionOrArray(currentPath, typeInformation.getType(),
-					typeInformation.getRequiredComponentType().getActualType().getType(), source.getBucket());
+			if (isByteArray(typeInformation)) {
+
+				// accept collection form for byte[] if there's no dedicated key
+				if (!source.getBucket().hasValue(currentPath) && isByteArray(typeInformation)) {
+
+					return readCollectionOrArray(currentPath, typeInformation.getType(),
+							typeInformation.getRequiredComponentType().getActualType().getType(), source.getBucket());
+				}
+			} else {
+				return readCollectionOrArray(currentPath, typeInformation.getType(),
+						typeInformation.getRequiredComponentType().getActualType().getType(), source.getBucket());
+			}
 		}
 
 		if (persistentProperty.isEntity()
@@ -309,7 +319,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 			return sourceBytes == null ? fromBytes(sourceBytes, typeInformation.getActualType().getType()) : source.getId();
 		}
 
-		Class<?> typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getActualType());
+		Class<?> typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getType());
 		return fromBytes(sourceBytes, typeToUse);
 	}
 
@@ -508,7 +518,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 					sink.getBucket().put(pUpdate.getPropertyPath(), toBytes(ref.getKeySpace() + ":" + refId));
 				}
 			}
-		} else if (targetProperty.isCollectionLike()) {
+		} else if (targetProperty.isCollectionLike() && !isByteArray(targetProperty)) {
 
 			Collection<?> collection = pUpdate.getValue() instanceof Collection ? (Collection<?>) pUpdate.getValue()
 					: Collections.singleton(pUpdate.getValue());
@@ -595,6 +605,11 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 			return;
 		}
 
+		if (value instanceof byte[]) {
+			sink.getBucket().put(StringUtils.hasText(path) ? path : "_raw", (byte[]) value);
+			return;
+		}
+
 		if (value.getClass() != typeHint.getType()) {
 			typeMapper.writeType(value.getClass(), sink.getBucket().getPropertyPath(path));
 		}
@@ -620,7 +635,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 				if (propertyValue != null) {
 					writeMap(keyspace, propertyStringPath, persistentProperty.getMapValueType(), (Map<?, ?>) propertyValue, sink);
 				}
-			} else if (persistentProperty.isCollectionLike()) {
+			} else if (persistentProperty.isCollectionLike() && !isByteArray(persistentProperty)) {
 
 				if (propertyValue == null) {
 					writeCollection(keyspace, propertyStringPath, null,
@@ -753,6 +768,11 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 			return;
 		}
 
+		if (value instanceof byte[]) {
+			sink.getBucket().put(path, toBytes(value));
+			return;
+		}
+
 		if (customConversions.hasCustomWriteTarget(value.getClass())) {
 
 			Optional<Class<?>> targetType = customConversions.getCustomWriteTarget(value.getClass());
@@ -969,6 +989,11 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 	 * @throws ConverterNotFoundException
 	 */
 	public <T> T fromBytes(byte[] source, Class<T> type) {
+
+		if (type.isInstance(source)) {
+			return type.cast(source);
+		}
+
 		return conversionService.convert(source, type);
 	}
 
@@ -1055,6 +1080,14 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 		customConversions.registerConvertersIn(conversionService);
 	}
 
+	private static boolean isByteArray(RedisPersistentProperty property) {
+		return property.getType().equals(byte[].class);
+	}
+
+	private static boolean isByteArray(TypeInformation<?> type) {
+		return type.getType().equals(byte[].class);
+	}
+
 	/**
 	 * @author Christoph Strobl
 	 * @author Mark Paluch

src/test/java/org/springframework/data/redis/core/convert/ConversionTestEntities.java

@@ -203,6 +203,7 @@ public class ConversionTestEntities {
 		String[] arrayOfSimpleTypes;
 		Species[] arrayOfCompexTypes;
 		int[] arrayOfPrimitives;
+		byte[] avatar;
 	}
 
 	static class TypeWithObjectValueTypes {

src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.java

@@ -52,7 +52,6 @@ import org.springframework.data.convert.WritingConverter;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.redis.core.PartialUpdate;
 import org.springframework.data.redis.core.convert.KeyspaceConfiguration.KeyspaceSettings;
-import org.springframework.data.redis.core.convert.ConversionTestEntities.*;
 import org.springframework.data.redis.core.mapping.RedisMappingContext;
 import org.springframework.data.redis.serializer.Jackson2JsonRedisSerializer;
 import org.springframework.data.redis.test.util.RedisTestData;
@@ -1273,6 +1272,40 @@ class MappingRedisConverterUnitTests {
 		assertThat(target.arrayOfSimpleTypes).isEqualTo(new String[] { "rand", "mat", "perrin" });
 	}
 
+	@Test // GH-1981
+	void readHandlesByteArrays() {
+
+		Map<String, String> source = new LinkedHashMap<>();
+		source.put("avatar", "foo-bar-baz");
+		source.put("otherAvatar", "foo-bar-baz");
+
+		WithArrays target = read(WithArrays.class, source);
+
+		assertThat(target.avatar).isEqualTo("foo-bar-baz".getBytes());
+	}
+
+	@Test // GH-1981
+	void writeHandlesByteArrays() {
+
+		WithArrays withArrays = new WithArrays();
+		withArrays.avatar = "foo-bar-baz".getBytes();
+
+		assertThat(write(withArrays)).containsEntry("avatar", "foo-bar-baz");
+	}
+
+	@Test // GH-1981
+	void readHandlesByteArraysUsingCollectionRepresentation() {
+
+		Map<String, String> source = new LinkedHashMap<>();
+		source.put("avatar.[0]", "102");
+		source.put("avatar.[1]", "111");
+		source.put("avatar.[2]", "111");
+
+		WithArrays target = read(WithArrays.class, source);
+
+		assertThat(target.avatar).isEqualTo("foo".getBytes());
+	}
+
 	@Test // DATAREDIS-492
 	void writeHandlesArraysOfComplexTypeProperly() {
 
@@ -1496,6 +1529,26 @@ class MappingRedisConverterUnitTests {
 		assertThat(write(update)).containsEntry("firstname", "rand").containsEntry("age", "24");
 	}
 
+	@Test // GH-1981
+	void writeShouldWritePartialUpdateFromEntityByteArrayValueCorrectly() {
+
+		WithArrays value = new WithArrays();
+		value.avatar = "foo-bar-baz".getBytes();
+
+		PartialUpdate<WithArrays> update = new PartialUpdate<>("123", value);
+
+		assertThat(write(update)).containsEntry("avatar", "foo-bar-baz");
+	}
+
+	@Test // GH-1981
+	void writeShouldWritePartialUpdateFromSetByteArrayValueCorrectly() {
+
+		PartialUpdate<WithArrays> update = PartialUpdate.newPartialUpdate(42, WithArrays.class).set("avatar",
+				"foo-bar-baz".getBytes());
+
+		assertThat(write(update)).containsEntry("avatar", "foo-bar-baz");
+	}
+
 	@Test // DATAREDIS-471
 	void writeShouldWritePartialUpdatePathWithSimpleValueCorrectly() {