Skip to content

Commit 57c294e

Browse files
christophstroblchristophstrobl
committed
Polishing.
Move reading shadow copy to dedicated method, update GH issue references, add test and fix formatting. Original Pull Request: spring-projects#2955
1 parent 3e80819 commit 57c294e

File tree

2 files changed

+122
-76
lines changed

2 files changed

+122
-76
lines changed

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

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,7 @@ public byte[] createKey(String keyspace, String id) {
572572
* Convert given source to binary representation using the underlying {@link ConversionService}.
573573
*/
574574
public byte[] toBytes(Object source) {
575-
return source instanceof byte[] bytes ? bytes
576-
: getConverter().getConversionService().convert(source, byte[].class);
575+
return source instanceof byte[] bytes ? bytes : getConverter().getConversionService().convert(source, byte[].class);
577576
}
578577

579578
private String toString(Object value) {
@@ -764,6 +763,7 @@ static class MappingExpirationListener extends KeyExpirationEventMessageListener
764763
private final RedisOperations<?, ?> ops;
765764
private final RedisConverter converter;
766765
private final ShadowCopy shadowCopy;
766+
767767
/**
768768
* Creates new {@link MappingExpirationListener}.
769769
*/
@@ -784,26 +784,7 @@ public void onMessage(Message message, @Nullable byte[] pattern) {
784784
}
785785

786786
byte[] key = message.getBody();
787-
Object value = null;
788-
789-
if (shadowCopy != ShadowCopy.OFF) {
790-
byte[] phantomKey = ByteUtils.concat(key,
791-
converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
792-
793-
Map<byte[], byte[]> hash = ops.execute((RedisCallback<Map<byte[], byte[]>>) connection -> {
794-
795-
Map<byte[], byte[]> phantomValue = connection.hGetAll(phantomKey);
796-
797-
if (!CollectionUtils.isEmpty(phantomValue)) {
798-
connection.del(phantomKey);
799-
}
800-
801-
return phantomValue;
802-
});
803-
804-
value = CollectionUtils.isEmpty(hash) ? null : converter.read(Object.class, new RedisData(hash));
805-
}
806-
787+
Object value = readShadowCopyIfEnabled(key);
807788
byte[] channelAsBytes = message.getChannel();
808789

809790
String channel = !ObjectUtils.isEmpty(channelAsBytes)
@@ -825,6 +806,35 @@ public void onMessage(Message message, @Nullable byte[] pattern) {
825806
private boolean isKeyExpirationMessage(Message message) {
826807
return BinaryKeyspaceIdentifier.isValid(message.getBody());
827808
}
809+
810+
@Nullable
811+
private Object readShadowCopyIfEnabled(byte[] key) {
812+
813+
if (shadowCopy == ShadowCopy.OFF) {
814+
return null;
815+
}
816+
return readShadowCopy(key);
817+
}
818+
819+
@Nullable
820+
private Object readShadowCopy(byte[] key) {
821+
822+
byte[] phantomKey = ByteUtils.concat(key,
823+
converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
824+
825+
Map<byte[], byte[]> hash = ops.execute((RedisCallback<Map<byte[], byte[]>>) connection -> {
826+
827+
Map<byte[], byte[]> phantomValue = connection.hGetAll(phantomKey);
828+
829+
if (!CollectionUtils.isEmpty(phantomValue)) {
830+
connection.del(phantomKey);
831+
}
832+
833+
return phantomValue;
834+
});
835+
836+
return CollectionUtils.isEmpty(hash) ? null : converter.read(Object.class, new RedisData(hash));
837+
}
828838
}
829839

830840
private boolean keepShadowCopy() {

‎src/test/java/org/springframework/data/redis/core/MappingExpirationListenerTest.java‎

Lines changed: 90 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,110 @@
1+
/*
2+
* Copyright 2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
116
package org.springframework.data.redis.core;
217

3-
import org.junit.jupiter.api.BeforeEach;
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.mockito.ArgumentMatchers.any;
20+
import static org.mockito.ArgumentMatchers.eq;
21+
import static org.mockito.Mockito.times;
22+
import static org.mockito.Mockito.verify;
23+
import static org.mockito.Mockito.when;
24+
25+
import java.util.ArrayList;
26+
import java.util.List;
27+
428
import org.junit.jupiter.api.Test;
529
import org.junit.jupiter.api.extension.ExtendWith;
630
import org.mockito.Mock;
7-
import org.mockito.MockitoAnnotations;
31+
import org.mockito.Mockito;
832
import org.mockito.junit.jupiter.MockitoExtension;
933
import org.mockito.junit.jupiter.MockitoSettings;
1034
import org.mockito.quality.Strictness;
11-
import org.springframework.context.ApplicationEvent;
12-
import org.springframework.context.ApplicationEventPublisher;
1335
import org.springframework.core.convert.ConversionService;
1436
import org.springframework.data.redis.connection.Message;
1537
import org.springframework.data.redis.core.convert.RedisConverter;
1638
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
1739

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.*;
2440
/**
2541
* @author Lucian Torje
42+
* @author Christoph Strobl
2643
*/
2744
@ExtendWith(MockitoExtension.class)
2845
@MockitoSettings(strictness = Strictness.LENIENT)
2946
class MappingExpirationListenerTest {
3047

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-
}
48+
@Mock private RedisOperations<?, ?> redisOperations;
49+
@Mock private RedisConverter redisConverter;
50+
@Mock private RedisMessageListenerContainer listenerContainer;
51+
@Mock private Message message;
52+
53+
private RedisKeyValueAdapter.MappingExpirationListener listener;
54+
55+
@Test // GH-2954
56+
void testOnNonKeyExpiration() {
57+
58+
byte[] key = "testKey".getBytes();
59+
when(message.getBody()).thenReturn(key);
60+
listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter,
61+
RedisKeyValueAdapter.ShadowCopy.ON);
62+
63+
listener.onMessage(message, null);
64+
65+
verify(redisOperations, times(0)).execute(any(RedisCallback.class));
66+
}
67+
68+
@Test // GH-2954
69+
void testOnValidKeyExpirationWithShadowCopiesDisabled() {
70+
71+
List<Object> eventList = new ArrayList<>();
72+
73+
byte[] key = "abc:testKey".getBytes();
74+
when(message.getBody()).thenReturn(key);
75+
76+
listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter,
77+
RedisKeyValueAdapter.ShadowCopy.OFF);
78+
listener.setApplicationEventPublisher(eventList::add);
79+
listener.onMessage(message, null);
80+
81+
verify(redisOperations, times(1)).execute(any(RedisCallback.class));
82+
assertThat(eventList).hasSize(1);
83+
assertThat(eventList.get(0)).isInstanceOf(RedisKeyExpiredEvent.class);
84+
assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getKeyspace()).isEqualTo("abc");
85+
assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getId()).isEqualTo("testKey".getBytes());
86+
}
87+
88+
@Test // GH-2954
89+
void testOnValidKeyExpirationWithShadowCopiesEnabled() {
90+
91+
ConversionService conversionService = Mockito.mock(ConversionService.class);
92+
List<Object> eventList = new ArrayList<>();
93+
94+
byte[] key = "abc:testKey".getBytes();
95+
when(message.getBody()).thenReturn(key);
96+
when(redisConverter.getConversionService()).thenReturn(conversionService);
97+
when(conversionService.convert(any(), eq(byte[].class))).thenReturn("foo".getBytes());
98+
99+
listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter,
100+
RedisKeyValueAdapter.ShadowCopy.ON);
101+
listener.setApplicationEventPublisher(eventList::add);
102+
listener.onMessage(message, null);
103+
104+
verify(redisOperations, times(2)).execute(any(RedisCallback.class)); // delete entry in index, delete phantom key
105+
assertThat(eventList).hasSize(1);
106+
assertThat(eventList.get(0)).isInstanceOf(RedisKeyExpiredEvent.class);
107+
assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getKeyspace()).isEqualTo("abc");
108+
assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getId()).isEqualTo("testKey".getBytes());
109+
}
110+
}

0 commit comments

Comments
 (0)