fix: prevent negative TTLs from insta-killing @document annotated ent…

…ities with save/saveAll (resolves gh-429)
redis · May 14, 2024 · 221ffdf · 221ffdf
1 parent 49d999e
commit 221ffdf
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 4 deletions.
diff --git a/redis-om-spring/src/main/java/com/redis/om/spring/RedisJSONKeyValueAdapter.java b/redis-om-spring/src/main/java/com/redis/om/spring/RedisJSONKeyValueAdapter.java
@@ -110,9 +110,7 @@ public Object put(Object id, Object item, String keyspace) {
     processReferences(key, item);
 
     redisOperations.execute((RedisCallback<Object>) connection -> {
-
-      maybeTtl.ifPresent(aLong -> connection.keyCommands().expire(toBytes(key), aLong));
-
+      maybeTtl.ifPresent(ttl -> { if (ttl > 0) connection.keyCommands().expire(toBytes(key), ttl); });
       return null;
     });
 

diff --git a/...g/src/main/java/com/redis/om/spring/repository/support/SimpleRedisDocumentRepository.java b/...g/src/main/java/com/redis/om/spring/repository/support/SimpleRedisDocumentRepository.java
@@ -199,7 +199,7 @@ public <S extends T> List<S> saveAll(Iterable<S> entities) {
 
         processReferenceAnnotations(objectKey, entity, pipeline);
 
-        maybeTtl.ifPresent(aLong -> pipeline.expire(objectKey, aLong));
+        maybeTtl.ifPresent(ttl -> { if (ttl > 0) pipeline.expire(objectKey, ttl); });
 
         saved.add(entity);
       }

diff --git a/redis-om-spring/src/test/java/com/redis/om/spring/annotations/document/DocumentTTLTests.java b/redis-om-spring/src/test/java/com/redis/om/spring/annotations/document/DocumentTTLTests.java
@@ -7,6 +7,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
@@ -119,4 +120,28 @@ void testExpiredEntitiesAreNotFound() throws InterruptedException {
     assertThat(withTTLAnnotationRepository.findOneByName("Konrad Zuse")).isNotPresent();
     assertThat(withTTLAnnotationRepository.findOneByName("Steve Wozniak")).isNotPresent();
   }
+
+  @Test
+  void testNegativeTimeToLiveAnnotationWithSave() {
+    ExpiringPerson mWoodger = ExpiringPerson.of("Mike Woodger", -1L);
+    withTTLAnnotationRepository.save(mWoodger);
+
+    Long expire = withTTLAnnotationRepository.getExpiration(mWoodger.getId());
+    boolean exists = withTTLAnnotationRepository.existsById(mWoodger.getId());
+
+    assertThat(expire).isEqualTo(-1L);
+    assertThat(exists).isTrue();
+  }
+
+  @Test
+  void testNegativeTimeToLiveAnnotationWithSaveAll() {
+    ExpiringPerson mWoodger = ExpiringPerson.of("Mike Woodger", -1L);
+    withTTLAnnotationRepository.saveAll(Set.of(mWoodger));
+
+    Long expire = withTTLAnnotationRepository.getExpiration(mWoodger.getId());
+    boolean exists = withTTLAnnotationRepository.existsById(mWoodger.getId());
+
+    assertThat(expire).isEqualTo(-1L);
+    assertThat(exists).isTrue();
+  }
 }
diff --git a/redis-om-spring/src/test/java/com/redis/om/spring/annotations/hash/RedisHashTTLTests.java b/redis-om-spring/src/test/java/com/redis/om/spring/annotations/hash/RedisHashTTLTests.java
@@ -8,6 +8,7 @@
 import org.springframework.data.redis.core.StringRedisTemplate;
 
 import java.util.List;
+import java.util.Set;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -103,4 +104,28 @@ void testTimeToLiveAnnotationWithDifferentTimeUnitOnSaveAll() {
     assertThat(jWilkinsonExpiration).isEqualTo(7L*24*60*60);
     assertThat(sDummontExpiration).isEqualTo(7L*24*60*60);
   }
+
+  @Test
+  void testNegativeTimeToLiveAnnotationUsingSave() {
+    ExpiringPerson mWoodger = ExpiringPerson.of("Mike Woodger", -1L);
+    withTTLAnnotationRepository.save(mWoodger);
+
+    Long expire = withTTLAnnotationRepository.getExpiration(mWoodger.getId());
+    boolean exists = withTTLAnnotationRepository.existsById(mWoodger.getId());
+
+    assertThat(expire).isEqualTo(-1L);
+    assertThat(exists).isTrue();
+  }
+
+  @Test
+  void testNegativeTimeToLiveAnnotationUsingSaveAll() {
+    ExpiringPerson mWoodger = ExpiringPerson.of("Mike Woodger", -1L);
+    withTTLAnnotationRepository.saveAll(Set.of(mWoodger));
+
+    Long expire = withTTLAnnotationRepository.getExpiration(mWoodger.getId());
+    boolean exists = withTTLAnnotationRepository.existsById(mWoodger.getId());
+
+    assertThat(expire).isEqualTo(-1L);
+    assertThat(exists).isTrue();
+  }
 }