Skip to content

Commit 3e80819

Browse files
ltorjechristophstrobl
authored andcommitted
Do not attempt to delete phantom key if ShadowCopy is OFF
This commit makes sure to consider the ShadowCopy flag for expiring key events and prevents delete attempts if shadow copy is off. Closes: #2954 Original Pull Request: #2955
1 parent c1521b6 commit 3e80819

File tree

2 files changed

+92
-14
lines changed

2 files changed

+92
-14
lines changed

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
* @author Mark Paluch
100100
* @author Andrey Muchnik
101101
* @author John Blum
102+
* @author Lucian Torje
102103
* @since 1.7
103104
*/
104105
public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
@@ -735,9 +736,8 @@ private void initMessageListenerContainer() {
735736
private void initKeyExpirationListener() {
736737

737738
if (this.expirationListener.get() == null) {
738-
739739
MappingExpirationListener listener = new MappingExpirationListener(this.messageListenerContainer, this.redisOps,
740-
this.converter);
740+
this.converter, this.shadowCopy);
741741

742742
listener.setKeyspaceNotificationsConfigParameter(keyspaceNotificationsConfigParameter);
743743

@@ -763,16 +763,17 @@ static class MappingExpirationListener extends KeyExpirationEventMessageListener
763763

764764
private final RedisOperations<?, ?> ops;
765765
private final RedisConverter converter;
766-
766+
private final ShadowCopy shadowCopy;
767767
/**
768768
* Creates new {@link MappingExpirationListener}.
769769
*/
770770
MappingExpirationListener(RedisMessageListenerContainer listenerContainer, RedisOperations<?, ?> ops,
771-
RedisConverter converter) {
771+
RedisConverter converter, ShadowCopy shadowCopy) {
772772

773773
super(listenerContainer);
774774
this.ops = ops;
775775
this.converter = converter;
776+
this.shadowCopy = shadowCopy;
776777
}
777778

778779
@Override
@@ -783,22 +784,25 @@ public void onMessage(Message message, @Nullable byte[] pattern) {
783784
}
784785

785786
byte[] key = message.getBody();
787+
Object value = null;
786788

787-
byte[] phantomKey = ByteUtils.concat(key,
788-
converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
789+
if (shadowCopy != ShadowCopy.OFF) {
790+
byte[] phantomKey = ByteUtils.concat(key,
791+
converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
789792

790-
Map<byte[], byte[]> hash = ops.execute((RedisCallback<Map<byte[], byte[]>>) connection -> {
793+
Map<byte[], byte[]> hash = ops.execute((RedisCallback<Map<byte[], byte[]>>) connection -> {
791794

792-
Map<byte[], byte[]> phantomValue = connection.hGetAll(phantomKey);
795+
Map<byte[], byte[]> phantomValue = connection.hGetAll(phantomKey);
793796

794-
if (!CollectionUtils.isEmpty(phantomValue)) {
795-
connection.del(phantomKey);
796-
}
797+
if (!CollectionUtils.isEmpty(phantomValue)) {
798+
connection.del(phantomKey);
799+
}
797800

798-
return phantomValue;
799-
});
801+
return phantomValue;
802+
});
800803

801-
Object value = CollectionUtils.isEmpty(hash) ? null : converter.read(Object.class, new RedisData(hash));
804+
value = CollectionUtils.isEmpty(hash) ? null : converter.read(Object.class, new RedisData(hash));
805+
}
802806

803807
byte[] channelAsBytes = message.getChannel();
804808

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package org.springframework.data.redis.core;
2+
3+
import org.junit.jupiter.api.BeforeEach;
4+
import org.junit.jupiter.api.Test;
5+
import org.junit.jupiter.api.extension.ExtendWith;
6+
import org.mockito.Mock;
7+
import org.mockito.MockitoAnnotations;
8+
import org.mockito.junit.jupiter.MockitoExtension;
9+
import org.mockito.junit.jupiter.MockitoSettings;
10+
import org.mockito.quality.Strictness;
11+
import org.springframework.context.ApplicationEvent;
12+
import org.springframework.context.ApplicationEventPublisher;
13+
import org.springframework.core.convert.ConversionService;
14+
import org.springframework.data.redis.connection.Message;
15+
import org.springframework.data.redis.core.convert.RedisConverter;
16+
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
21+
import static org.assertj.core.api.Assertions.assertThat;
22+
import static org.mockito.ArgumentMatchers.any;
23+
import static org.mockito.Mockito.*;
24+
/**
25+
* @author Lucian Torje
26+
*/
27+
@ExtendWith(MockitoExtension.class)
28+
@MockitoSettings(strictness = Strictness.LENIENT)
29+
class MappingExpirationListenerTest {
30+
31+
@Mock
32+
private RedisOperations<?, ?> redisOperations;
33+
@Mock
34+
private RedisConverter redisConverter;
35+
@Mock
36+
private RedisMessageListenerContainer listenerContainer;
37+
@Mock
38+
private Message message;
39+
@Mock
40+
private RedisKeyExpiredEvent<?> event;
41+
@Mock
42+
private ConversionService conversionService;
43+
44+
private RedisKeyValueAdapter.MappingExpirationListener listener;
45+
46+
@Test
47+
void testOnNonKeyExpiration() {
48+
byte[] key = "testKey".getBytes();
49+
when(message.getBody()).thenReturn(key);
50+
listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, RedisKeyValueAdapter.ShadowCopy.ON);
51+
52+
listener.onMessage(message, null);
53+
54+
verify(redisOperations, times(0)).execute(any(RedisCallback.class));
55+
}
56+
57+
@Test
58+
void testOnValidKeyExpiration() {
59+
List<Object> eventList = new ArrayList<>();
60+
61+
byte[] key = "abc:testKey".getBytes();
62+
when(message.getBody()).thenReturn(key);
63+
64+
listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, RedisKeyValueAdapter.ShadowCopy.OFF);
65+
listener.setApplicationEventPublisher(eventList::add);
66+
listener.onMessage(message, null);
67+
68+
verify(redisOperations, times(1)).execute(any(RedisCallback.class));
69+
assertThat(eventList).hasSize(1);
70+
assertThat(eventList.get(0)).isInstanceOf(RedisKeyExpiredEvent.class);
71+
assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getKeyspace()).isEqualTo("abc");
72+
assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getId()).isEqualTo("testKey".getBytes());
73+
}
74+
}

0 commit comments

Comments
 (0)