Pass-thru byte arrays in entities into Bucket

pom.xml

@@ -1,11 +1,13 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 
 	<modelVersion>4.0.0</modelVersion>
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>2.5.0-SNAPSHOT</version>
+	<version>2.5.0-GH-1981-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 

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 void remove(String path) {
 	/**
 	 * 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 byte[] get(String path) {
 		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

@@ -61,6 +61,7 @@
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.comparator.NullSafeComparator;
 
@@ -283,18 +284,28 @@ protected Object readProperty(String path, RedisData source, RedisPersistentProp
 
 		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().getType(), source.getBucket());
+				}
+			} else {
+				return readCollectionOrArray(currentPath, typeInformation.getType(),
+						typeInformation.getRequiredComponentType().getType(), source.getBucket());
+			}
 		}
 
 		if (persistentProperty.isEntity()
-				&& !conversionService.canConvert(byte[].class, typeInformation.getActualType().getType())) {
+				&& !conversionService.canConvert(byte[].class, typeInformation.getRequiredActualType().getType())) {
 
 			Bucket bucket = source.getBucket().extract(currentPath + ".");
 
 			RedisData newBucket = new RedisData(bucket);
 			TypeInformation<?> typeToRead = typeMapper.readType(bucket.getPropertyPath(currentPath),
-					typeInformation.getActualType());
+					typeInformation);
 
 			return readInternal(currentPath, typeToRead.getType(), newBucket);
 		}
@@ -305,11 +316,15 @@ protected Object readProperty(String path, RedisData source, RedisPersistentProp
 			return null;
 		}
 
-		if (persistentProperty.isIdProperty() && StringUtils.isEmpty(path.isEmpty())) {
-			return sourceBytes == null ? fromBytes(sourceBytes, typeInformation.getActualType().getType()) : source.getId();
+		if (persistentProperty.isIdProperty() && ObjectUtils.isEmpty(path.isEmpty())) {
+			return sourceBytes != null ? fromBytes(sourceBytes, typeInformation.getType()) : source.getId();
 		}
 
-		Class<?> typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getActualType());
+		if (sourceBytes == null) {
+			return null;
+		}
+
+		Class<?> typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getType());
 		return fromBytes(sourceBytes, typeToUse);
 	}
 
@@ -508,7 +523,7 @@ private void writePartialPropertyUpdate(PartialUpdate<?> update, PropertyUpdate
 					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 +610,11 @@ private void writeInternal(@Nullable String keyspace, String path, @Nullable Obj
 			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 +640,7 @@ private void writeInternal(@Nullable String keyspace, String path, @Nullable Obj
 				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 +773,11 @@ private void writeToBucket(String path, @Nullable Object value, RedisData sink,
 			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 +994,11 @@ public byte[] toBytes(Object source) {
 	 * @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 +1085,14 @@ private void initializeConverters() {
 		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 static class WithArrays {
 		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.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 @@ void readHandlesArraysOfSimpleTypeProperly() {
 		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 @@ void writeShouldWritePartialUpdateSimpleValueCorrectly() {
 		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() {