DATAREDIS-1032 - Polishing.

Add early return in cache key is a String. Convert convertCollectionLikeOrMapKey to non-nullable method by throwing an exception on a non-collection-like and Map-like type.

Original pull request: #475.

Author: mp911de

src/main/java/org/springframework/data/redis/cache/RedisCache.java

@@ -286,6 +286,10 @@ protected String createCacheKey(Object key) {
 	 */
 	protected String convertKey(Object key) {
 
+		if (key instanceof String) {
+			return (String) key;
+		}
+
 		TypeDescriptor source = TypeDescriptor.forObject(key);
 
 		if (conversionService.canConvert(source, TypeDescriptor.valueOf(String.class))) {
@@ -310,20 +314,21 @@ protected String convertKey(Object key) {
 
 		throw new IllegalStateException(String.format(
 				"Cannot convert cache key %s to String. Please register a suitable Converter via 'RedisCacheConfiguration.configureKeyConverters(...)' or override '%s.toString()'.",
-				source, key != null ? key.getClass().getSimpleName() : "Object"));
+				source, key.getClass().getSimpleName()));
 	}
 
-	@Nullable
 	private String convertCollectionLikeOrMapKey(Object key, TypeDescriptor source) {
 
 		if (source.isMap()) {
 
-			String target = "{";
+			StringBuilder target = new StringBuilder("{");
+
 			for (Entry<?, ?> entry : ((Map<?, ?>) key).entrySet()) {
-				target += (convertKey(entry.getKey()) + "=" + convertKey(entry.getValue()));
+				target.append(convertKey(entry.getKey())).append("=").append(convertKey(entry.getValue()));
 			}
-			target += "}";
-			return target;
+			target.append("}");
+
+			return target.toString();
 		} else if (source.isCollection() || source.isArray()) {
 
 			StringJoiner sj = new StringJoiner(",");
@@ -336,7 +341,8 @@ private String convertCollectionLikeOrMapKey(Object key, TypeDescriptor source)
 			}
 			return "[" + sj.toString() + "]";
 		}
-		return null;
+
+		throw new IllegalArgumentException(String.format("Cannot convert cache key %s to String.", key));
 	}
 
 	private boolean isCollectionLikeOrMap(TypeDescriptor source) {

src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java

@@ -311,7 +311,7 @@ public ConversionService getConversionService() {
 	 *
 	 * @param cacheKeyConverter
 	 * @throws IllegalStateException if {@link #getConversionService()} does not allow converter registration.
-	 * @since 2.2
+	 * @since 2.1.11
 	 */
 	public void addCacheKeyConverter(Converter<?, String> cacheKeyConverter) {
 		configureKeyConverters(it -> it.addConverter(cacheKeyConverter));
@@ -322,7 +322,7 @@ public void addCacheKeyConverter(Converter<?, String> cacheKeyConverter) {
 	 *
 	 * @param registryConsumer never {@literal null}.
 	 * @throws IllegalStateException if {@link #getConversionService()} does not allow converter registration.
-	 * @since 2.2
+	 * @since 2.1.11
 	 */
 	public void configureKeyConverters(Consumer<ConverterRegistry> registryConsumer) {
 

src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java

@@ -24,6 +24,7 @@
 
 import java.io.Serializable;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
@@ -57,7 +58,7 @@ public class RedisCacheTests {
 
 	String key = "key-1";
 	String cacheKey = "cache::" + key;
-	byte[] binaryCacheKey = cacheKey.getBytes(Charset.forName("UTF-8"));
+	byte[] binaryCacheKey = cacheKey.getBytes(StandardCharsets.UTF_8);
 
 	Person sample = new Person("calmity", new Date());
 	byte[] binarySample;
@@ -281,15 +282,15 @@ public void computePrefixCreatesCacheKeyCorrectly() {
 
 		doWithConnection(connection -> {
 
-			assertThat(connection.stringCommands().get("_cache_key-1".getBytes(Charset.forName("UTF-8"))))
+			assertThat(connection.stringCommands().get("_cache_key-1".getBytes(StandardCharsets.UTF_8)))
 					.isEqualTo(binarySample);
 		});
 	}
 
 	@Test // DATAREDIS-715
 	public void fetchKeyWithComputedPrefixReturnsExpectedResult() {
 
-		doWithConnection(connection -> connection.set("_cache_key-1".getBytes(Charset.forName("UTF-8")), binarySample));
+		doWithConnection(connection -> connection.set("_cache_key-1".getBytes(StandardCharsets.UTF_8), binarySample));
 
 		RedisCache cacheWithCustomPrefix = new RedisCache("cache", new DefaultRedisCacheWriter(connectionFactory),
 				RedisCacheConfiguration.defaultCacheConfig().serializeValuesWith(SerializationPair.fromSerializer(serializer))
@@ -307,18 +308,19 @@ public void cacheShouldAllowListKeyCacheKeysOfSimpleTypes() {
 		Object key = SimpleKeyGenerator.generateKey(Collections.singletonList("my-cache-key-in-a-list"));
 		cache.put(key, sample);
 
-		Object target = cache.get(SimpleKeyGenerator.generateKey(Collections.singletonList("my-cache-key-in-a-list")));
-		assertThat(((ValueWrapper) target).get()).isEqualTo(sample);
+		ValueWrapper target = cache
+				.get(SimpleKeyGenerator.generateKey(Collections.singletonList("my-cache-key-in-a-list")));
+		assertThat(target.get()).isEqualTo(sample);
 	}
 
 	@Test // DATAREDIS-1032
 	public void cacheShouldAllowArrayKeyCacheKeysOfSimpleTypes() {
 
-		Object key = SimpleKeyGenerator.generateKey(new String[] { "my-cache-key-in-an-array" });
+		Object key = SimpleKeyGenerator.generateKey("my-cache-key-in-an-array");
 		cache.put(key, sample);
 
-		Object target = cache.get(SimpleKeyGenerator.generateKey(new String[] { "my-cache-key-in-an-array" }));
-		assertThat(((ValueWrapper) target).get()).isEqualTo(sample);
+		ValueWrapper target = cache.get(SimpleKeyGenerator.generateKey("my-cache-key-in-an-array"));
+		assertThat(target.get()).isEqualTo(sample);
 	}
 
 	@Test // DATAREDIS-1032
@@ -328,9 +330,9 @@ public void cacheShouldAllowListCacheKeysOfComplexTypes() {
 				.generateKey(Collections.singletonList(new ComplexKey(sample.getFirstame(), sample.getBirthdate())));
 		cache.put(key, sample);
 
-		Object target = cache.get(SimpleKeyGenerator
+		ValueWrapper target = cache.get(SimpleKeyGenerator
 				.generateKey(Collections.singletonList(new ComplexKey(sample.getFirstame(), sample.getBirthdate()))));
-		assertThat(((ValueWrapper) target).get()).isEqualTo(sample);
+		assertThat(target.get()).isEqualTo(sample);
 	}
 
 	@Test // DATAREDIS-1032
@@ -340,9 +342,9 @@ public void cacheShouldAllowMapCacheKeys() {
 				.generateKey(Collections.singletonMap("map-key", new ComplexKey(sample.getFirstame(), sample.getBirthdate())));
 		cache.put(key, sample);
 
-		Object target = cache.get(SimpleKeyGenerator
+		ValueWrapper target = cache.get(SimpleKeyGenerator
 				.generateKey(Collections.singletonMap("map-key", new ComplexKey(sample.getFirstame(), sample.getBirthdate()))));
-		assertThat(((ValueWrapper) target).get()).isEqualTo(sample);
+		assertThat(target.get()).isEqualTo(sample);
 	}
 
 	@Test // DATAREDIS-1032