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 6260e7d80..10a0e17f2 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -99,6 +99,7 @@ import org.springframework.util.ObjectUtils; * * @author Christoph Strobl * @author Mark Paluch + * @author Andrey Muchnik * @since 1.7 */ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter @@ -245,6 +246,11 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter connection.expire(phantomKey, rdo.getTimeToLive() + PHANTOM_KEY_TTL); } + boolean isNoExpire = rdo.getTimeToLive() == null || rdo.getTimeToLive() != null && rdo.getTimeToLive() < 0; + if (isNoExpire && !isNew && keepShadowCopy()) { + connection.del(ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); + } + connection.sAdd(toBytes(rdo.getKeyspace()), key); IndexWriter indexWriter = new IndexWriter(connection, converter); @@ -487,7 +493,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter } else { connection.persist(redisKey); - connection.persist(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); + connection.del(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); } } @@ -736,6 +742,10 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter } } + private boolean keepShadowCopy() { + return this.expirationListener.get() != null; + } + /** * {@link MessageListener} implementation used to capture Redis keyspace notifications. Tries to read a previously * created phantom key {@code keyspace:id:phantom} to provide the expired object as part of the published @@ -777,7 +787,8 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter byte[] key = message.getBody(); - byte[] phantomKey = ByteUtils.concat(key, converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class)); + byte[] phantomKey = ByteUtils.concat(key, + converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class)); Map hash = ops.execute((RedisCallback>) connection -> { @@ -794,7 +805,8 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter byte[] channelAsBytes = message.getChannel(); String channel = !ObjectUtils.isEmpty(channelAsBytes) - ? converter.getConversionService().convert(channelAsBytes, String.class) : null; + ? converter.getConversionService().convert(channelAsBytes, String.class) + : null; RedisKeyExpiredEvent event = new RedisKeyExpiredEvent(channel, key, value); diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java index ecd95b7d6..ac9ac33eb 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java @@ -60,6 +60,7 @@ import org.springframework.data.redis.core.mapping.RedisMappingContext; /** * @author Christoph Strobl * @author Mark Paluch + * @author Andrey Muchnik */ @RunWith(Parameterized.class) public class RedisKeyValueAdapterTests { @@ -687,6 +688,41 @@ public class RedisKeyValueAdapterTests { assertThat(updatedLocation.getY()).isCloseTo(18D, offset(0.005)); } + @Test // DATAREDIS-1955 + public void phantomKeyIsDeletedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() { + ExpiringPerson rand = new ExpiringPerson(); + rand.id = "1"; + rand.ttl = 3000L; + + adapter.put("1", rand, "persons"); + + assertThat(template.getExpire("persons:1:phantom")).isPositive(); + + rand.ttl = -1L; + + adapter.put("1", rand, "persons"); + + assertThat(template.hasKey("persons:1:phantom")).isFalse(); + } + + @Test // DATAREDIS-1955 + public void updateWithRefreshTtlAndWithoutPositiveTtlShouldDeletePhantomKey() { + ExpiringPerson person = new ExpiringPerson(); + person.id = "1"; + person.ttl = 100L; + + adapter.put("1", person, "persons"); + + assertThat(template.getExpire("persons:1:phantom")).isPositive(); + + PartialUpdate update = new PartialUpdate<>("1", ExpiringPerson.class) // + .refreshTtl(true); + + adapter.update(update); + + assertThat(template.hasKey("persons:1:phantom")).isFalse(); + } + /** * Wait up to 5 seconds until {@code key} is no longer available in Redis. * @@ -776,6 +812,11 @@ public class RedisKeyValueAdapterTests { } + static class ExpiringPerson extends Person { + + @TimeToLive Long ttl; + } + @KeySpace("locations") static class Location {