Skip to content

Commit 5de7390

Browse files
mp911dechristophstrobl
mp911de
authored and
christophstrobl
committed
DATAREDIS-744 - Support Redis hashes with colon in their id.
We now support Redis hashes via Repository support that contain colon in their id. We're using colons to split a composite id string into keyspace and id parts. Previously, we partially rejected processing of id's that don't exactly match the number of parts delimited by colon. This caused leftovers in secondary indexes. Original Pull Request: #298
1 parent 323cb32 commit 5de7390

File tree

7 files changed

+434
-43
lines changed

7 files changed

+434
-43
lines changed

‎src/main/java/org/springframework/data/redis/core/RedisKeyExpiredEvent.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@
1919
import java.nio.charset.StandardCharsets;
2020

2121
import org.springframework.context.ApplicationEvent;
22-
import org.springframework.data.redis.util.ByteUtils;
22+
import org.springframework.data.redis.core.convert.MappingRedisConverter.BinaryKeyspaceIdentifier;
2323
import org.springframework.lang.Nullable;
2424

2525
/**
2626
* {@link RedisKeyExpiredEvent} is Redis specific {@link ApplicationEvent} published when a specific key in Redis
2727
* expires. It might but must not hold the expired value itself next to the key.
2828
*
2929
* @author Christoph Strobl
30+
* @author Mark Paluch
3031
* @since 1.7
3132
*/
3233
public class RedisKeyExpiredEvent<T> extends RedisKeyspaceEvent {
@@ -36,7 +37,7 @@ public class RedisKeyExpiredEvent<T> extends RedisKeyspaceEvent {
3637
*/
3738
public static final Charset CHARSET = StandardCharsets.UTF_8;
3839

39-
private final byte[][] args;
40+
private final BinaryKeyspaceIdentifier objectId;
4041
private final @Nullable Object value;
4142

4243
/**
@@ -69,7 +70,12 @@ public RedisKeyExpiredEvent(byte[] key, @Nullable Object value) {
6970
public RedisKeyExpiredEvent(@Nullable String channel, byte[] key, @Nullable Object value) {
7071
super(channel, key);
7172

72-
args = ByteUtils.split(key, ':');
73+
if (BinaryKeyspaceIdentifier.isValid(key)) {
74+
this.objectId = BinaryKeyspaceIdentifier.of(key);
75+
} else {
76+
this.objectId = null;
77+
}
78+
7379
this.value = value;
7480
}
7581

@@ -79,12 +85,7 @@ public RedisKeyExpiredEvent(@Nullable String channel, byte[] key, @Nullable Obje
7985
* @return {@literal null} if it could not be determined.
8086
*/
8187
public String getKeyspace() {
82-
83-
if (args.length >= 2) {
84-
return new String(args[0], CHARSET);
85-
}
86-
87-
return null;
88+
return objectId != null ? new String(objectId.getKeyspace(), CHARSET) : null;
8889
}
8990

9091
/**
@@ -93,7 +94,7 @@ public String getKeyspace() {
9394
* @return
9495
*/
9596
public byte[] getId() {
96-
return args.length == 2 ? args[1] : args[0];
97+
return objectId != null ? objectId.getId() : getSource();
9798
}
9899

99100
/**

‎src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
import org.springframework.data.redis.core.convert.GeoIndexedPropertyValue;
4949
import org.springframework.data.redis.core.convert.KeyspaceConfiguration;
5050
import org.springframework.data.redis.core.convert.MappingRedisConverter;
51+
import org.springframework.data.redis.core.convert.MappingRedisConverter.BinaryKeyspaceIdentifier;
52+
import org.springframework.data.redis.core.convert.MappingRedisConverter.KeyspaceIdentifier;
5153
import org.springframework.data.redis.core.convert.PathIndexResolver;
5254
import org.springframework.data.redis.core.convert.RedisConverter;
5355
import org.springframework.data.redis.core.convert.RedisCustomConversions;
@@ -240,7 +242,7 @@ public Object put(final Object id, Object item, String keyspace) {
240242
connection.expire(objectKey, rdo.getTimeToLive());
241243

242244
// add phantom key so values can be restored
243-
byte[] phantomKey = ByteUtils.concat(objectKey, toBytes(":phantom"));
245+
byte[] phantomKey = ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX);
244246
connection.del(phantomKey);
245247
connection.hMSet(phantomKey, rdo.getBucket().rawMap());
246248
connection.expire(phantomKey, rdo.getTimeToLive() + 300);
@@ -471,14 +473,14 @@ public void update(PartialUpdate<?> update) {
471473
connection.expire(redisKey, rdo.getTimeToLive());
472474

473475
// add phantom key so values can be restored
474-
byte[] phantomKey = ByteUtils.concat(redisKey, toBytes(":phantom"));
476+
byte[] phantomKey = ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX);
475477
connection.hMSet(phantomKey, rdo.getBucket().rawMap());
476478
connection.expire(phantomKey, rdo.getTimeToLive() + 300);
477479

478480
} else {
479481

480482
connection.persist(redisKey);
481-
connection.persist(ByteUtils.concat(redisKey, toBytes(":phantom")));
483+
connection.persist(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX));
482484
}
483485
}
484486

@@ -763,7 +765,7 @@ public void onMessage(Message message, @Nullable byte[] pattern) {
763765

764766
byte[] key = message.getBody();
765767

766-
byte[] phantomKey = ByteUtils.concat(key, converter.getConversionService().convert(":phantom", byte[].class));
768+
byte[] phantomKey = ByteUtils.concat(key, converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
767769

768770
Map<byte[], byte[]> hash = ops.execute((RedisCallback<Map<byte[], byte[]>>) connection -> {
769771

@@ -799,9 +801,7 @@ private boolean isKeyExpirationMessage(Message message) {
799801
return false;
800802
}
801803

802-
byte[][] args = ByteUtils.split(message.getBody(), ':');
803-
804-
return args.length == 2;
804+
return BinaryKeyspaceIdentifier.isValid(message.getBody());
805805
}
806806
}
807807

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

Lines changed: 197 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
package org.springframework.data.redis.core.convert;
1717

1818
import lombok.RequiredArgsConstructor;
19+
import lombok.AccessLevel;
20+
import lombok.AllArgsConstructor;
21+
import lombok.Getter;
1922

2023
import java.lang.reflect.Array;
2124
import java.util.*;
@@ -52,6 +55,7 @@
5255
import org.springframework.data.redis.core.mapping.RedisMappingContext;
5356
import org.springframework.data.redis.core.mapping.RedisPersistentEntity;
5457
import org.springframework.data.redis.core.mapping.RedisPersistentProperty;
58+
import org.springframework.data.redis.util.ByteUtils;
5559
import org.springframework.data.util.ClassTypeInformation;
5660
import org.springframework.data.util.TypeInformation;
5761
import org.springframework.lang.Nullable;
@@ -308,9 +312,14 @@ private void readAssociation(String path, RedisData source, RedisPersistentEntit
308312
for (Entry<String, byte[]> entry : bucket.entrySet()) {
309313

310314
String referenceKey = fromBytes(entry.getValue(), String.class);
311-
String[] args = referenceKey.split(":");
312315

313-
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(args[1], args[0]);
316+
if (!KeyspaceIdentifier.isValid(referenceKey)) {
317+
continue;
318+
}
319+
320+
KeyspaceIdentifier identifier = KeyspaceIdentifier.of(referenceKey);
321+
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(identifier.getId(),
322+
identifier.getKeyspace());
314323

315324
if (!CollectionUtils.isEmpty(rawHash)) {
316325
target.add(read(association.getInverse().getActualType(), new RedisData(rawHash)));
@@ -326,15 +335,18 @@ private void readAssociation(String path, RedisData source, RedisPersistentEntit
326335
return;
327336
}
328337

329-
String key = fromBytes(binKey, String.class);
338+
String referenceKey = fromBytes(binKey, String.class);
339+
if (KeyspaceIdentifier.isValid(referenceKey)) {
330340

331-
String[] args = key.split(":");
341+
KeyspaceIdentifier identifier = KeyspaceIdentifier.of(referenceKey);
332342

333-
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(args[1], args[0]);
343+
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(identifier.getId(),
344+
identifier.getKeyspace());
334345

335-
if (!CollectionUtils.isEmpty(rawHash)) {
336-
accessor.setProperty(association.getInverse(),
337-
read(association.getInverse().getActualType(), new RedisData(rawHash)));
346+
if (!CollectionUtils.isEmpty(rawHash)) {
347+
accessor.setProperty(association.getInverse(),
348+
read(association.getInverse().getActualType(), new RedisData(rawHash)));
349+
}
338350
}
339351
}
340352
});
@@ -434,9 +446,10 @@ private void writePartialPropertyUpdate(PartialUpdate<?> update, PropertyUpdate
434446
targetProperty = getTargetPropertyOrNullForPath(path.replaceAll("\\.\\[.*\\]", ""), update.getTarget());
435447

436448
TypeInformation<?> ti = targetProperty == null ? ClassTypeInformation.OBJECT
437-
: (targetProperty.isMap() ? (targetProperty.getTypeInformation().getMapValueType() != null
438-
? targetProperty.getTypeInformation().getRequiredMapValueType()
439-
: ClassTypeInformation.OBJECT) : targetProperty.getTypeInformation().getActualType());
449+
: (targetProperty.isMap()
450+
? (targetProperty.getTypeInformation().getMapValueType() != null
451+
? targetProperty.getTypeInformation().getRequiredMapValueType() : ClassTypeInformation.OBJECT)
452+
: targetProperty.getTypeInformation().getActualType());
440453

441454
writeInternal(entity.getKeySpace(), pUpdate.getPropertyPath(), pUpdate.getValue(), ti, sink);
442455
return;
@@ -1160,4 +1173,177 @@ public int compareTo(Part that) {
11601173
}
11611174
}
11621175

1176+
/**
1177+
* Value object representing a Redis Hash/Object identifier composed from keyspace and object id in the form of
1178+
* {@literal keyspace:id}.
1179+
*
1180+
* @author Mark Paluch
1181+
* @since 1.8.10
1182+
*/
1183+
@AllArgsConstructor(access = AccessLevel.PRIVATE)
1184+
@Getter
1185+
public static class KeyspaceIdentifier {
1186+
1187+
public static final String PHANTOM = "phantom";
1188+
public static final String DELIMITTER = ":";
1189+
public static final String PHANTOM_SUFFIX = DELIMITTER + PHANTOM;
1190+
1191+
private String keyspace;
1192+
private String id;
1193+
private boolean phantomKey;
1194+
1195+
/**
1196+
* Parse a {@code key} into {@link KeyspaceIdentifier}.
1197+
*
1198+
* @param key the key representation.
1199+
* @return {@link BinaryKeyspaceIdentifier} for binary key.
1200+
*/
1201+
public static KeyspaceIdentifier of(String key) {
1202+
1203+
Assert.isTrue(isValid(key), String.format("Invalid key %s", key));
1204+
1205+
boolean phantomKey = key.endsWith(PHANTOM_SUFFIX);
1206+
int keyspaceEndIndex = key.indexOf(DELIMITTER);
1207+
String keyspace = key.substring(0, keyspaceEndIndex);
1208+
String id;
1209+
1210+
if (phantomKey) {
1211+
id = key.substring(keyspaceEndIndex + 1, key.length() - PHANTOM_SUFFIX.length());
1212+
} else {
1213+
id = key.substring(keyspaceEndIndex + 1);
1214+
}
1215+
1216+
return new KeyspaceIdentifier(keyspace, id, phantomKey);
1217+
}
1218+
1219+
/**
1220+
* Check whether the {@code key} is valid, in particular whether the key contains a keyspace and an id part in the
1221+
* form of {@literal keyspace:id}.
1222+
*
1223+
* @param key the key.
1224+
* @return {@literal true} if the key is valid.
1225+
*/
1226+
public static boolean isValid(String key) {
1227+
1228+
if (key == null) {
1229+
return false;
1230+
}
1231+
1232+
int keyspaceEndIndex = key.indexOf(DELIMITTER);
1233+
1234+
return keyspaceEndIndex > 0 && key.length() > keyspaceEndIndex;
1235+
}
1236+
}
1237+
1238+
/**
1239+
* Value object representing a binary Redis Hash/Object identifier composed from keyspace and object id in the form of
1240+
* {@literal keyspace:id}.
1241+
*
1242+
* @author Mark Paluch
1243+
* @since 1.8.10
1244+
*/
1245+
@AllArgsConstructor(access = AccessLevel.PRIVATE)
1246+
@Getter
1247+
public static class BinaryKeyspaceIdentifier {
1248+
1249+
public static final byte[] PHANTOM = KeyspaceIdentifier.PHANTOM.getBytes();
1250+
public static final byte DELIMITTER = ':';
1251+
public static final byte[] PHANTOM_SUFFIX = ByteUtils.concat(new byte[] { DELIMITTER }, PHANTOM);
1252+
1253+
private byte[] keyspace;
1254+
private byte[] id;
1255+
private boolean phantomKey;
1256+
1257+
/**
1258+
* Parse a binary {@code key} into {@link BinaryKeyspaceIdentifier}.
1259+
*
1260+
* @param key the binary key representation.
1261+
* @return {@link BinaryKeyspaceIdentifier} for binary key.
1262+
*/
1263+
public static BinaryKeyspaceIdentifier of(byte[] key) {
1264+
1265+
Assert.isTrue(isValid(key), String.format("Invalid key %s", new String(key)));
1266+
1267+
boolean phantomKey = startsWith(key, PHANTOM_SUFFIX, key.length - PHANTOM_SUFFIX.length);
1268+
1269+
int keyspaceEndIndex = find(key, DELIMITTER);
1270+
byte[] keyspace = getKeyspace(key, keyspaceEndIndex);
1271+
byte[] id = getId(key, phantomKey, keyspaceEndIndex);
1272+
1273+
return new BinaryKeyspaceIdentifier(keyspace, id, phantomKey);
1274+
}
1275+
1276+
/**
1277+
* Check whether the {@code key} is valid, in particular whether the key contains a keyspace and an id part in the
1278+
* form of {@literal keyspace:id}.
1279+
*
1280+
* @param key the key.
1281+
* @return {@literal true} if the key is valid.
1282+
*/
1283+
public static boolean isValid(byte[] key) {
1284+
1285+
if (key == null) {
1286+
return false;
1287+
}
1288+
1289+
int keyspaceEndIndex = find(key, DELIMITTER);
1290+
1291+
return keyspaceEndIndex > 0 && key.length > keyspaceEndIndex;
1292+
}
1293+
1294+
private static byte[] getId(byte[] key, boolean phantomKey, int keyspaceEndIndex) {
1295+
1296+
int idSize;
1297+
1298+
if (phantomKey) {
1299+
idSize = (key.length - PHANTOM_SUFFIX.length) - (keyspaceEndIndex + 1);
1300+
} else {
1301+
1302+
idSize = key.length - (keyspaceEndIndex + 1);
1303+
}
1304+
1305+
byte[] id = new byte[idSize];
1306+
System.arraycopy(key, keyspaceEndIndex + 1, id, 0, idSize);
1307+
1308+
return id;
1309+
}
1310+
1311+
private static byte[] getKeyspace(byte[] key, int keyspaceEndIndex) {
1312+
1313+
byte[] keyspace = new byte[keyspaceEndIndex];
1314+
System.arraycopy(key, 0, keyspace, 0, keyspaceEndIndex);
1315+
1316+
return keyspace;
1317+
}
1318+
1319+
private static boolean startsWith(byte[] haystack, byte[] prefix, int offset) {
1320+
1321+
int to = offset;
1322+
int prefixOffset = 0;
1323+
int prefixLength = prefix.length;
1324+
1325+
if ((offset < 0) || (offset > haystack.length - prefixLength)) {
1326+
return false;
1327+
}
1328+
1329+
while (--prefixLength >= 0) {
1330+
if (haystack[to++] != prefix[prefixOffset++]) {
1331+
return false;
1332+
}
1333+
}
1334+
1335+
return true;
1336+
}
1337+
1338+
private static int find(byte[] haystack, byte needle) {
1339+
1340+
for (int i = 0; i < haystack.length; i++) {
1341+
if (haystack[i] == needle) {
1342+
return i;
1343+
}
1344+
}
1345+
1346+
return -1;
1347+
}
1348+
}
11631349
}

0 commit comments

Comments
 (0)