Refactor Redis secondary-index key formatting.

Redis secondary-index keys are now formatted using helper methods instead of using chains of concatAll(toBytes(…)) sequences for more readability.

Original pull request: #2795
Closes: #2794

Author: junghoon-vans

src/main/java/org/springframework/data/redis/core/IndexWriter.java

@@ -33,23 +33,26 @@
 
 /**
  * {@link IndexWriter} takes care of writing <a href="https://redis.io/topics/indexes">secondary index</a> structures to
- * Redis. Depending on the type of {@link IndexedData} it uses eg. Sets with specific names to add actually referenced
- * keys to. While doing so {@link IndexWriter} also keeps track of all indexes associated with the root types key, which
+ * Redis. Depending on the type of {@link IndexedData}, it uses Sets with specific names to add actually referenced keys
+ * to. While doing so {@link IndexWriter} also keeps track of all indexes associated with the root types key, which
  * allows to remove the root key from all indexes in case of deletion.
  *
  * @author Christoph Strobl
  * @author Rob Winch
+ * @author Mark Paluch
  * @since 1.7
  */
 class IndexWriter {
 
+	private static final byte[] SEPARATOR = ":".getBytes();
+	private static final byte[] IDX = "idx".getBytes();
+
 	private final RedisConnection connection;
 	private final RedisConverter converter;
 
 	/**
 	 * Creates new {@link IndexWriter}.
 	 *
-	 * @param keyspace The key space to write index values to. Must not be {@literal null}.
 	 * @param connection must not be {@literal null}.
 	 * @param converter must not be {@literal null}.
 	 */
@@ -127,7 +130,7 @@ public void removeKeyFromIndexes(String keyspace, Object key) {
 		Assert.notNull(key, "Key must not be null");
 
 		byte[] binKey = toBytes(key);
-		byte[] indexHelperKey = ByteUtils.concatAll(toBytes(keyspace + ":"), binKey, toBytes(":idx"));
+		byte[] indexHelperKey = createIndexKey(keyspace, binKey);
 
 		for (byte[] indexKey : connection.sMembers(indexHelperKey)) {
 
@@ -147,10 +150,10 @@ public void removeKeyFromIndexes(String keyspace, Object key) {
 	 */
 	public void removeAllIndexes(String keyspace) {
 
-		Set<byte[]> potentialIndex = connection.keys(toBytes(keyspace + ":*"));
+		Set<byte[]> potentialIndex = connection.keys(createIndexKey(keyspace, "*"));
 
 		if (!potentialIndex.isEmpty()) {
-			connection.del(potentialIndex.toArray(new byte[potentialIndex.size()][]));
+			connection.del(potentialIndex.toArray(new byte[0][]));
 		}
 	}
 
@@ -162,7 +165,7 @@ private void removeKeyFromExistingIndexes(byte[] key, Iterable<IndexedData> inde
 	}
 
 	/**
-	 * Remove given key from all indexes matching {@link IndexedData#getIndexName()}:
+	 * Remove given key from all indexes matching {@link IndexedData#getIndexName()}.
 	 *
 	 * @param key
 	 * @param indexedData
@@ -171,8 +174,7 @@ protected void removeKeyFromExistingIndexes(byte[] key, IndexedData indexedData)
 
 		Assert.notNull(indexedData, "IndexedData must not be null");
 
-		Set<byte[]> existingKeys = connection
-				.keys(toBytes(indexedData.getKeyspace() + ":" + indexedData.getIndexName() + ":*"));
+		Set<byte[]> existingKeys = connection.keys(createIndexKey(indexedData.getKeyPrefix(), "*"));
 
 		if (!CollectionUtils.isEmpty(existingKeys)) {
 			for (byte[] existingKey : existingKeys) {
@@ -216,30 +218,66 @@ protected void addKeyToIndex(byte[] key, IndexedData indexedData) {
 				return;
 			}
 
-			byte[] indexKey = toBytes(indexedData.getKeyspace() + ":" + indexedData.getIndexName() + ":");
+			byte[] indexKey = toBytes(indexedData.getKeyPrefix(), SEPARATOR);
 			indexKey = ByteUtils.concat(indexKey, toBytes(value));
 			connection.sAdd(indexKey, key);
 
 			// keep track of indexes used for the object
-			connection.sAdd(ByteUtils.concatAll(toBytes(indexedData.getKeyspace() + ":"), key, toBytes(":idx")), indexKey);
+			connection.sAdd(createIndexKey(indexedData.getKeyspace(), key), indexKey);
 		} else if (indexedData instanceof GeoIndexedPropertyValue propertyValue) {
 
 			Object value = propertyValue.getValue();
 			if (value == null) {
 				return;
 			}
 
-			byte[] indexKey = toBytes(indexedData.getKeyspace() + ":" + indexedData.getIndexName());
+			byte[] indexKey = toBytes(indexedData.getKeyPrefix());
 			connection.geoAdd(indexKey, propertyValue.getPoint(), key);
 
 			// keep track of indexes used for the object
-			connection.sAdd(ByteUtils.concatAll(toBytes(indexedData.getKeyspace() + ":"), key, toBytes(":idx")), indexKey);
+			connection.sAdd(createIndexKey(indexedData.getKeyspace(), key), indexKey);
 		} else {
 			throw new IllegalArgumentException(
 					String.format("Cannot write index data for unknown index type %s", indexedData.getClass()));
 		}
 	}
 
+	private byte[] createIndexKey(String keyspace, byte[] key) {
+		return createIndexKey(keyspace, key, IDX);
+	}
+
+	private byte[] createIndexKey(Object... items) {
+
+		Object[] elements = new Object[items.length + (items.length - 1)];
+
+		int j = 0;
+		for (int i = 0; i < items.length; i++) {
+
+			elements[j++] = items[i];
+			if (items.length - 1 > i) {
+				elements[j++] = SEPARATOR;
+			}
+		}
+
+		return toBytes(elements);
+	}
+
+	private byte[] toBytes(Object... values) {
+
+		byte[][] arrays = new byte[values.length][];
+
+		for (int i = 0; i < values.length; i++) {
+
+			if (values[i] instanceof byte[] bb) {
+				arrays[i] = bb;
+			} else {
+				arrays[i] = toBytes(values[i]);
+			}
+		}
+
+		return ByteUtils.concatAll(arrays);
+	}
+
 	private byte[] toBytes(@Nullable Object source) {
 
 		if (source == null) {

src/main/java/org/springframework/data/redis/core/RedisQueryEngine.java

@@ -25,6 +25,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.springframework.core.convert.ConversionService;
 import org.springframework.data.geo.Circle;
 import org.springframework.data.geo.GeoResult;
 import org.springframework.data.geo.GeoResults;
@@ -81,8 +82,8 @@ private RedisQueryEngine(CriteriaAccessor<RedisOperationChain> criteriaAccessor,
 
 	@Override
 	@SuppressWarnings("unchecked")
-	public <T> List<T> execute(RedisOperationChain criteria, Comparator<?> sort, long offset, int rows,
-			String keyspace, Class<T> type) {
+	public <T> List<T> execute(RedisOperationChain criteria, Comparator<?> sort, long offset, int rows, String keyspace,
+			Class<T> type) {
 		List<T> result = doFind(criteria, offset, rows, keyspace, type);
 
 		if (sort != null) {
@@ -199,8 +200,7 @@ private List<byte[]> findKeys(RedisOperationChain criteria, int rows, String key
 	}
 
 	@Override
-	public List<?> execute(RedisOperationChain criteria, Comparator<?> sort, long offset, int rows,
-			String keyspace) {
+	public List<?> execute(RedisOperationChain criteria, Comparator<?> sort, long offset, int rows, String keyspace) {
 		return execute(criteria, sort, offset, rows, keyspace, Object.class);
 	}
 
@@ -229,14 +229,13 @@ public long count(RedisOperationChain criteria, String keyspace) {
 
 	private byte[][] keys(String prefix, Collection<PathAndValue> source) {
 
+		ConversionService conversionService = getRequiredAdapter().getConverter().getConversionService();
 		byte[][] keys = new byte[source.size()][];
 		int i = 0;
 		for (PathAndValue pathAndValue : source) {
 
-			byte[] convertedValue = getRequiredAdapter().getConverter().getConversionService()
-					.convert(pathAndValue.getFirstValue(), byte[].class);
-			byte[] fullPath = getRequiredAdapter().getConverter().getConversionService()
-					.convert(prefix + pathAndValue.getPath() + ":", byte[].class);
+			byte[] convertedValue = conversionService.convert(pathAndValue.getFirstValue(), byte[].class);
+			byte[] fullPath = conversionService.convert(prefix + pathAndValue.getPath() + ":", byte[].class);
 
 			keys[i] = ByteUtils.concat(fullPath, convertedValue);
 			i++;

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

@@ -38,4 +38,13 @@ public interface IndexedData {
 	 */
 	String getKeyspace();
 
+	/**
+	 * Return the key prefix for usage in Redis.
+	 *
+	 * @return concatenated form of the keyspace and the index name.
+	 * @since 3.3.4
+	 */
+	default String getKeyPrefix() {
+		return getKeyspace() + ":" + getIndexName();
+	}
 }