Skip to content

Commit 87616b4

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 16cd1ef commit 87616b4

File tree

7 files changed

+435
-44
lines changed

7 files changed

+435
-44
lines changed

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015 the original author or authors.
2+
* Copyright 2015-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -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.ArrayList;
@@ -61,6 +64,7 @@
6164
import org.springframework.data.redis.core.mapping.RedisMappingContext;
6265
import org.springframework.data.redis.core.mapping.RedisPersistentEntity;
6366
import org.springframework.data.redis.core.mapping.RedisPersistentProperty;
67+
import org.springframework.data.redis.util.ByteUtils;
6468
import org.springframework.data.util.ClassTypeInformation;
6569
import org.springframework.data.util.TypeInformation;
6670
import org.springframework.lang.Nullable;
@@ -317,9 +321,14 @@ private void readAssociation(String path, RedisData source, RedisPersistentEntit
317321
for (Entry<String, byte[]> entry : bucket.entrySet()) {
318322

319323
String referenceKey = fromBytes(entry.getValue(), String.class);
320-
String[] args = referenceKey.split(":");
321324

322-
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(args[1], args[0]);
325+
if (!KeyspaceIdentifier.isValid(referenceKey)) {
326+
continue;
327+
}
328+
329+
KeyspaceIdentifier identifier = KeyspaceIdentifier.of(referenceKey);
330+
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(identifier.getId(),
331+
identifier.getKeyspace());
323332

324333
if (!CollectionUtils.isEmpty(rawHash)) {
325334
target.add(read(association.getInverse().getActualType(), new RedisData(rawHash)));
@@ -335,15 +344,18 @@ private void readAssociation(String path, RedisData source, RedisPersistentEntit
335344
return;
336345
}
337346

338-
String key = fromBytes(binKey, String.class);
347+
String referenceKey = fromBytes(binKey, String.class);
348+
if (KeyspaceIdentifier.isValid(referenceKey)) {
339349

340-
String[] args = key.split(":");
350+
KeyspaceIdentifier identifier = KeyspaceIdentifier.of(referenceKey);
341351

342-
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(args[1], args[0]);
352+
Map<byte[], byte[]> rawHash = referenceResolver.resolveReference(identifier.getId(),
353+
identifier.getKeyspace());
343354

344-
if (!CollectionUtils.isEmpty(rawHash)) {
345-
accessor.setProperty(association.getInverse(),
346-
read(association.getInverse().getActualType(), new RedisData(rawHash)));
355+
if (!CollectionUtils.isEmpty(rawHash)) {
356+
accessor.setProperty(association.getInverse(),
357+
read(association.getInverse().getActualType(), new RedisData(rawHash)));
358+
}
347359
}
348360
}
349361
});
@@ -443,9 +455,10 @@ private void writePartialPropertyUpdate(PartialUpdate<?> update, PropertyUpdate
443455
targetProperty = getTargetPropertyOrNullForPath(path.replaceAll("\\.\\[.*\\]", ""), update.getTarget());
444456

445457
TypeInformation<?> ti = targetProperty == null ? ClassTypeInformation.OBJECT
446-
: (targetProperty.isMap() ? (targetProperty.getTypeInformation().getMapValueType() != null
447-
? targetProperty.getTypeInformation().getRequiredMapValueType()
448-
: ClassTypeInformation.OBJECT) : targetProperty.getTypeInformation().getActualType());
458+
: (targetProperty.isMap()
459+
? (targetProperty.getTypeInformation().getMapValueType() != null
460+
? targetProperty.getTypeInformation().getRequiredMapValueType() : ClassTypeInformation.OBJECT)
461+
: targetProperty.getTypeInformation().getActualType());
449462

450463
writeInternal(entity.getKeySpace(), pUpdate.getPropertyPath(), pUpdate.getValue(), ti, sink);
451464
return;
@@ -1159,4 +1172,177 @@ public int compareTo(Part that) {
11591172
}
11601173
}
11611174

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

0 commit comments

Comments
 (0)