fix: Make restore accept ttl in ms (#1724)

Signed-off-by: Vladislav Oleshko <[email protected]>

Author: dranikpg

src/server/generic_family.cc

@@ -41,12 +41,6 @@ using VersionBuffer = std::array<char, sizeof(uint16_t)>;
 using CrcBuffer = std::array<char, sizeof(uint64_t)>;
 constexpr size_t DUMP_FOOTER_SIZE = sizeof(uint64_t) + sizeof(uint16_t);  // version number and crc
 
-int64_t CalculateExpirationTime(bool seconds, bool absolute, int64_t ts, int64_t now_msec) {
-  int64_t msec = seconds ? ts * 1000 : ts;
-  int64_t rel_msec = absolute ? msec - now_msec : msec;
-  return rel_msec;
-}
-
 VersionBuffer MakeRdbVersion() {
   VersionBuffer buf;
   buf[0] = RDB_SER_VERSION & 0xff;
@@ -188,12 +182,13 @@ class RestoreArgs {
     return replace_;
   }
 
-  constexpr int64_t ExpirationTime() const {
+  uint64_t ExpirationTime() const {
+    DCHECK_GE(expiration_, 0);
     return expiration_;
   }
 
   [[nodiscard]] constexpr bool Expired() const {
-    return ExpirationTime() < 0;
+    return expiration_ < 0;
   }
 
   [[nodiscard]] constexpr bool HasExpiration() const {
@@ -207,14 +202,11 @@ class RestoreArgs {
 
 [[nodiscard]] bool RestoreArgs::UpdateExpiration(int64_t now_msec) {
   if (HasExpiration()) {
-    auto new_ttl = CalculateExpirationTime(!abs_time_, abs_time_, expiration_, now_msec);
-    if (new_ttl > kMaxExpireDeadlineSec * 1000) {
+    int64_t ttl = abs_time_ ? expiration_ - now_msec : expiration_;
+    if (ttl > kMaxExpireDeadlineSec * 1000)
       return false;
-    }
-    expiration_ = new_ttl;
-    if (new_ttl > 0) {
-      expiration_ += now_msec;
-    }
+
+    expiration_ = ttl < 0 ? -1 : ttl + now_msec;
   }
   return true;
 }

src/server/generic_family_test.cc

@@ -474,11 +474,11 @@ TEST_F(GenericFamilyTest, Restore) {
                                  0x75, 0x59, 0x6d, 0x10, 0x04, 0x3f, 0x5c};
   auto resp = Run({"set", "exiting-key", "1234"});
   EXPECT_EQ(resp, "OK");
-  // try to restore into existing key - this should failed
+  // try to restore into existing key - this should fail
   ASSERT_THAT(Run({"restore", "exiting-key", "0", ToSV(STRING_DUMP_REDIS)}),
               ArgType(RespExpr::ERROR));
 
-  // Try restore while setting expiration into the pass
+  // Try restore while setting expiration into the past
   // note that value for expiration is just some valid unix time stamp from the pass
   resp = Run(
       {"restore", "exiting-key", "1665476212900", ToSV(STRING_DUMP_REDIS), "ABSTTL", "REPLACE"});
@@ -518,11 +518,11 @@ TEST_F(GenericFamilyTest, Restore) {
   resp = Run({"dump", "string-key"});
   dump = resp.GetBuf();
   // this will change the value from "hello world" to "1234"
-  resp = Run({"restore", "string-key", "7", ToSV(STRING_DUMP_REDIS), "REPLACE"});
+  resp = Run({"restore", "string-key", "7000", ToSV(STRING_DUMP_REDIS), "REPLACE"});
   resp = Run({"get", "string-key"});
   EXPECT_EQ("1234", resp);
   // check TTL validity
-  EXPECT_EQ(CheckedInt({"ttl", "string-key"}), 7);
+  EXPECT_EQ(CheckedInt({"pttl", "string-key"}), 7000);
 
   // Make check about ttl with abs time, restoring back to "hello world"
   resp = Run({"restore", "string-key", absl::StrCat(TEST_current_time_ms + 2000), ToSV(dump),