Move expires check to dedicated method.

Introduce helper method to unify expiry check and simplify control flow.

Original Pull Request: #1961
This commit is contained in:
Christoph Strobl
2021-02-10 12:47:41 +01:00
parent e46246721f
commit 4df8e44818
2 changed files with 40 additions and 22 deletions

View File

@@ -236,26 +236,28 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
connection.hMSet(objectKey, rdo.getBucket().rawMap()); connection.hMSet(objectKey, rdo.getBucket().rawMap());
if (rdo.getTimeToLive() != null && rdo.getTimeToLive() > 0) { if(isNew) {
connection.sAdd(toBytes(rdo.getKeyspace()), key);
}
if (expires(rdo)) {
connection.expire(objectKey, rdo.getTimeToLive()); connection.expire(objectKey, rdo.getTimeToLive());
}
if (keepShadowCopy()) { // add phantom key so values can be restored if (keepShadowCopy()) { // add phantom key so values can be restored
byte[] phantomKey = ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX);
if (expires(rdo)) {
byte[] phantomKey = ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX);
connection.del(phantomKey); connection.del(phantomKey);
connection.hMSet(phantomKey, rdo.getBucket().rawMap()); connection.hMSet(phantomKey, rdo.getBucket().rawMap());
connection.expire(phantomKey, rdo.getTimeToLive() + PHANTOM_KEY_TTL); connection.expire(phantomKey, rdo.getTimeToLive() + PHANTOM_KEY_TTL);
} else if (!isNew) {
connection.del(phantomKey);
} }
} }
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); IndexWriter indexWriter = new IndexWriter(connection, converter);
if (isNew) { if (isNew) {
indexWriter.createIndexes(key, rdo.getIndexedData()); indexWriter.createIndexes(key, rdo.getIndexedData());
@@ -349,11 +351,14 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
connection.sRem(binKeyspace, binId); connection.sRem(binKeyspace, binId);
new IndexWriter(connection, converter).removeKeyFromIndexes(asString(keyspace), binId); new IndexWriter(connection, converter).removeKeyFromIndexes(asString(keyspace), binId);
RedisPersistentEntity<?> persistentEntity = converter.getMappingContext().getPersistentEntity(type); if(RedisKeyValueAdapter.this.keepShadowCopy()) {
if (persistentEntity != null && persistentEntity.isExpiring()) {
byte[] phantomKey = ByteUtils.concat(keyToDelete, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX); RedisPersistentEntity<?> persistentEntity = converter.getMappingContext().getPersistentEntity(type);
connection.del(phantomKey); if (persistentEntity != null && persistentEntity.isExpiring()) {
byte[] phantomKey = ByteUtils.concat(keyToDelete, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX);
connection.del(phantomKey);
}
} }
return null; return null;
}); });
@@ -484,7 +489,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
if (update.isRefreshTtl()) { if (update.isRefreshTtl()) {
if (rdo.getTimeToLive() != null && rdo.getTimeToLive() > 0) { if (expires(rdo)) {
connection.expire(redisKey, rdo.getTimeToLive()); connection.expire(redisKey, rdo.getTimeToLive());
@@ -498,7 +503,10 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
} else { } else {
connection.persist(redisKey); connection.persist(redisKey);
connection.del(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX));
if(keepShadowCopy()) {
connection.del(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX));
}
} }
} }
@@ -655,6 +663,16 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
return target; return target;
} }
/**
* @return {@literal true} if {@link RedisData#getTimeToLive()} has a positive value.
*
* @param data must not be {@literal null}.
* @since 2.3.7
*/
private boolean expires(RedisData data) {
return data.getTimeToLive() != null && data.getTimeToLive() > 0;
}
/** /**
* Configure usage of {@link KeyExpirationEventMessageListener}. * Configure usage of {@link KeyExpirationEventMessageListener}.
* *

View File

@@ -706,10 +706,10 @@ public class RedisKeyValueAdapterTests {
assertThat(template.hasKey("persons:1:phantom")).isTrue(); assertThat(template.hasKey("persons:1:phantom")).isTrue();
} }
@Test // DATAREDIS-1955 @Test // GH-1955
void phantomKeyIsDeletedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() { void phantomKeyIsDeletedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() {
ExpiringPerson rand = new ExpiringPerson(); ExpiringPerson rand = new ExpiringPerson();
rand.id = "1";
rand.ttl = 3000L; rand.ttl = 3000L;
adapter.put("1", rand, "persons"); adapter.put("1", rand, "persons");
@@ -723,21 +723,21 @@ public class RedisKeyValueAdapterTests {
assertThat(template.hasKey("persons:1:phantom")).isFalse(); assertThat(template.hasKey("persons:1:phantom")).isFalse();
} }
@Test // DATAREDIS-1955 @Test // GH-1955
void updateWithRefreshTtlAndWithoutPositiveTtlShouldDeletePhantomKey() { void updateWithRefreshTtlAndWithoutPositiveTtlShouldDeletePhantomKey() {
ExpiringPerson person = new ExpiringPerson(); ExpiringPerson person = new ExpiringPerson();
person.id = "1";
person.ttl = 100L; person.ttl = 100L;
adapter.put("1", person, "persons"); adapter.put("1", person, "persons");
assertThat(template.getExpire("persons:1:phantom")).isPositive(); assertThat(template.getExpire("persons:1:phantom")).isPositive();
PartialUpdate<ExpiringPerson> update = new PartialUpdate<>("1", ExpiringPerson.class) // PartialUpdate<ExpiringPerson> update = new PartialUpdate<>("1", ExpiringPerson.class) //
.refreshTtl(true); .set("ttl", -1L).refreshTtl(true);
adapter.update(update); adapter.update(update);
assertThat(template.getExpire("persons:1")).isNotPositive();
assertThat(template.hasKey("persons:1:phantom")).isFalse(); assertThat(template.hasKey("persons:1:phantom")).isFalse();
} }