diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java index 0be695b4b..926f9d768 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -572,8 +572,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter * Convert given source to binary representation using the underlying {@link ConversionService}. */ public byte[] toBytes(Object source) { - return source instanceof byte[] bytes ? bytes - : getConverter().getConversionService().convert(source, byte[].class); + return source instanceof byte[] bytes ? bytes : getConverter().getConversionService().convert(source, byte[].class); } private String toString(Object value) { @@ -764,6 +763,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter private final RedisOperations ops; private final RedisConverter converter; private final ShadowCopy shadowCopy; + /** * Creates new {@link MappingExpirationListener}. */ @@ -784,26 +784,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter } byte[] key = message.getBody(); - Object value = null; - - if (shadowCopy != ShadowCopy.OFF) { - byte[] phantomKey = ByteUtils.concat(key, - converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class)); - - Map hash = ops.execute((RedisCallback>) connection -> { - - Map phantomValue = connection.hGetAll(phantomKey); - - if (!CollectionUtils.isEmpty(phantomValue)) { - connection.del(phantomKey); - } - - return phantomValue; - }); - - value = CollectionUtils.isEmpty(hash) ? null : converter.read(Object.class, new RedisData(hash)); - } - + Object value = readShadowCopyIfEnabled(key); byte[] channelAsBytes = message.getChannel(); String channel = !ObjectUtils.isEmpty(channelAsBytes) @@ -825,6 +806,35 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter private boolean isKeyExpirationMessage(Message message) { return BinaryKeyspaceIdentifier.isValid(message.getBody()); } + + @Nullable + private Object readShadowCopyIfEnabled(byte[] key) { + + if (shadowCopy == ShadowCopy.OFF) { + return null; + } + return readShadowCopy(key); + } + + @Nullable + private Object readShadowCopy(byte[] key) { + + byte[] phantomKey = ByteUtils.concat(key, + converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class)); + + Map hash = ops.execute((RedisCallback>) connection -> { + + Map phantomValue = connection.hGetAll(phantomKey); + + if (!CollectionUtils.isEmpty(phantomValue)) { + connection.del(phantomKey); + } + + return phantomValue; + }); + + return CollectionUtils.isEmpty(hash) ? null : converter.read(Object.class, new RedisData(hash)); + } } private boolean keepShadowCopy() { diff --git a/src/test/java/org/springframework/data/redis/core/MappingExpirationListenerTest.java b/src/test/java/org/springframework/data/redis/core/MappingExpirationListenerTest.java index b019fe309..ebb7aa53a 100644 --- a/src/test/java/org/springframework/data/redis/core/MappingExpirationListenerTest.java +++ b/src/test/java/org/springframework/data/redis/core/MappingExpirationListenerTest.java @@ -1,74 +1,110 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.data.redis.core; -import org.junit.jupiter.api.BeforeEach; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; -import org.springframework.context.ApplicationEvent; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.core.convert.ConversionService; import org.springframework.data.redis.connection.Message; import org.springframework.data.redis.core.convert.RedisConverter; import org.springframework.data.redis.listener.RedisMessageListenerContainer; -import java.util.ArrayList; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; /** * @author Lucian Torje + * @author Christoph Strobl */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) class MappingExpirationListenerTest { - @Mock - private RedisOperations redisOperations; - @Mock - private RedisConverter redisConverter; - @Mock - private RedisMessageListenerContainer listenerContainer; - @Mock - private Message message; - @Mock - private RedisKeyExpiredEvent event; - @Mock - private ConversionService conversionService; + @Mock private RedisOperations redisOperations; + @Mock private RedisConverter redisConverter; + @Mock private RedisMessageListenerContainer listenerContainer; + @Mock private Message message; - private RedisKeyValueAdapter.MappingExpirationListener listener; + private RedisKeyValueAdapter.MappingExpirationListener listener; - @Test - void testOnNonKeyExpiration() { - byte[] key = "testKey".getBytes(); - when(message.getBody()).thenReturn(key); - listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, RedisKeyValueAdapter.ShadowCopy.ON); + @Test // GH-2954 + void testOnNonKeyExpiration() { - listener.onMessage(message, null); + byte[] key = "testKey".getBytes(); + when(message.getBody()).thenReturn(key); + listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, + RedisKeyValueAdapter.ShadowCopy.ON); - verify(redisOperations, times(0)).execute(any(RedisCallback.class)); - } + listener.onMessage(message, null); - @Test - void testOnValidKeyExpiration() { - List eventList = new ArrayList<>(); + verify(redisOperations, times(0)).execute(any(RedisCallback.class)); + } - byte[] key = "abc:testKey".getBytes(); - when(message.getBody()).thenReturn(key); + @Test // GH-2954 + void testOnValidKeyExpirationWithShadowCopiesDisabled() { - listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, RedisKeyValueAdapter.ShadowCopy.OFF); - listener.setApplicationEventPublisher(eventList::add); - listener.onMessage(message, null); + List eventList = new ArrayList<>(); - verify(redisOperations, times(1)).execute(any(RedisCallback.class)); - assertThat(eventList).hasSize(1); - assertThat(eventList.get(0)).isInstanceOf(RedisKeyExpiredEvent.class); - assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getKeyspace()).isEqualTo("abc"); - assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getId()).isEqualTo("testKey".getBytes()); - } -} \ No newline at end of file + byte[] key = "abc:testKey".getBytes(); + when(message.getBody()).thenReturn(key); + + listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, + RedisKeyValueAdapter.ShadowCopy.OFF); + listener.setApplicationEventPublisher(eventList::add); + listener.onMessage(message, null); + + verify(redisOperations, times(1)).execute(any(RedisCallback.class)); + assertThat(eventList).hasSize(1); + assertThat(eventList.get(0)).isInstanceOf(RedisKeyExpiredEvent.class); + assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getKeyspace()).isEqualTo("abc"); + assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getId()).isEqualTo("testKey".getBytes()); + } + + @Test // GH-2954 + void testOnValidKeyExpirationWithShadowCopiesEnabled() { + + ConversionService conversionService = Mockito.mock(ConversionService.class); + List eventList = new ArrayList<>(); + + byte[] key = "abc:testKey".getBytes(); + when(message.getBody()).thenReturn(key); + when(redisConverter.getConversionService()).thenReturn(conversionService); + when(conversionService.convert(any(), eq(byte[].class))).thenReturn("foo".getBytes()); + + listener = new RedisKeyValueAdapter.MappingExpirationListener(listenerContainer, redisOperations, redisConverter, + RedisKeyValueAdapter.ShadowCopy.ON); + listener.setApplicationEventPublisher(eventList::add); + listener.onMessage(message, null); + + verify(redisOperations, times(2)).execute(any(RedisCallback.class)); // delete entry in index, delete phantom key + assertThat(eventList).hasSize(1); + assertThat(eventList.get(0)).isInstanceOf(RedisKeyExpiredEvent.class); + assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getKeyspace()).isEqualTo("abc"); + assertThat(((RedisKeyExpiredEvent) (eventList.get(0))).getId()).isEqualTo("testKey".getBytes()); + } +}