Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align the expire option behavior of redis #1017

Merged
merged 5 commits into from
Oct 23, 2022
Merged

Align the expire option behavior of redis #1017

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d9bc5c7
align expire option behavior
tanruixiang Oct 20, 2022
24d9a11
Use simpler test code
tanruixiang Oct 21, 2022
0197cc6
Merge branch 'unstable' into align_expire_option_behavior
tisonkun Oct 21, 2022
2d685a1
use second and enhance test
tanruixiang Oct 21, 2022
262c361
adjust code style
tanruixiang Oct 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/commands/redis_cmd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "server/server.h"
#include "stats/disk_stats.h"
#include "stats/log_collector.h"
#include "status.h"
#include "storage/redis_db.h"
#include "storage/redis_pubsub.h"
#include "storage/scripting.h"
Expand Down Expand Up @@ -103,24 +104,28 @@ Status ParseTTL(const std::vector<std::string> &args, std::unordered_map<std::st
int ttl = 0;
int64_t expire = 0;
bool last_arg = false;
int ex_exist = 0, exat_exist = 0, pxat_exist = 0, px_exist = 0;
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
git-hulk marked this conversation as resolved.
Show resolved Hide resolved
for (size_t i = 0; i < args.size(); i++) {
last_arg = (i == args.size() - 1);
std::string opt = Util::ToLower(args[i]);
if (opt == "ex" && !ttl && !last_arg) {
if (opt == "ex" && !last_arg) {
ex_exist = 1;
auto parse_result = ParseInt<int>(args[++i], 10);
if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInteger);
}
ttl = *parse_result;
if (ttl <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
} else if (opt == "exat" && !ttl && !expire && !last_arg) {
} else if (opt == "exat" && !last_arg) {
exat_exist = 1;
auto parse_result = ParseInt<int64_t>(args[++i], 10);
if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInteger);
}
expire = *parse_result;
if (expire <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
} else if (opt == "pxat" && !ttl && !expire && !last_arg) {
} else if (opt == "pxat" && !last_arg) {
pxat_exist = 1;
auto parse_result = ParseInt<uint64_t>(args[++i], 10);
if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInteger);
Expand All @@ -132,7 +137,8 @@ Status ParseTTL(const std::vector<std::string> &args, std::unordered_map<std::st
} else {
expire = static_cast<int64_t>(expire_ms / 1000);
}
} else if (opt == "px" && !ttl && !last_arg) {
} else if (opt == "px" && !last_arg) {
px_exist = 1;
int64_t ttl_ms = 0;
auto parse_result = ParseInt<int64_t>(args[++i], 10);
if (!parse_result) {
Expand All @@ -154,6 +160,9 @@ Status ParseTTL(const std::vector<std::string> &args, std::unordered_map<std::st
}
}
}
if (px_exist + ex_exist + exat_exist + pxat_exist >= 2) {
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
return Status(Status::NotOK, errInvalidSyntax);
}
if (!ttl && expire) {
int64_t now;
rocksdb::Env::Default()->GetCurrentTime(&now);
Expand Down Expand Up @@ -829,10 +838,12 @@ class CommandCAS : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
bool last_arg;
bool ex_exist = false, px_exist = false;
for (size_t i = 4; i < args.size(); i++) {
last_arg = (i == args.size() - 1);
std::string opt = Util::ToLower(args[i]);
if (opt == "ex") {
ex_exist = true;
if (last_arg) return Status(Status::NotOK, errWrongNumOfArguments);
auto parse_result = ParseInt<int>(args_[++i].c_str(), 10);
if (!parse_result) {
Expand All @@ -841,6 +852,7 @@ class CommandCAS : public Commander {
ttl_ = *parse_result;
if (ttl_ <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
} else if (opt == "px") {
px_exist = true;
if (last_arg) return Status(Status::NotOK, errWrongNumOfArguments);
auto parse_result = ParseInt<int>(args[++i].c_str(), 10);
if (!parse_result) {
Expand All @@ -858,6 +870,9 @@ class CommandCAS : public Commander {
return Status(Status::NotOK, errInvalidSyntax);
}
}
if (ex_exist && px_exist) {
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
return Status(Status::NotOK, errInvalidSyntax);
}
return Commander::Parse(args);
}

Expand Down
105 changes: 104 additions & 1 deletion tests/gocase/unit/type/strings/strings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,33 +141,75 @@ func TestString(t *testing.T) {
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second, 10*time.Second)
})

t.Run("GETEX Duplicate EX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.GetEx(ctx, "foo", 10*time.Second).Err())
require.NoError(t, rdb.Do(ctx, "getex", "foo", "ex", 1, "ex", 10).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second, 10*time.Second)
})

t.Run("GETEX PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.GetEx(ctx, "foo", 10000*time.Millisecond).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5000*time.Millisecond, 10000*time.Millisecond)
})

t.Run("GETEX Duplicate PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.Do(ctx, "getex", "foo", "px", 1, "px", 10000).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5000*time.Millisecond, 10000*time.Millisecond)
})

t.Run("GETEX EXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.Do(ctx, "getex", "foo", "exat", time.Now().Add(10*time.Second).Unix()).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second, 10*time.Second)
})

t.Run("GETEX Duplicate EXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.Do(ctx, "getex", "foo", "exat", time.Now().Add(100*time.Second).Unix(), "exat", time.Now().Add(10*time.Second).Unix()).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second, 10*time.Second)
})

t.Run("GETEX PXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.Do(ctx, "getex", "foo", "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli()).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5000*time.Millisecond, 10000*time.Millisecond)
})

t.Run("GETEX Duplicate PXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
require.NoError(t, rdb.Do(ctx, "getex", "foo", "pxat", time.Now().Add(1000000*time.Millisecond).UnixMilli(), "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli()).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5000*time.Millisecond, 10000*time.Millisecond)
})

t.Run("GETEX PERSIST option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 10*time.Second).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second, 10*time.Second)
require.NoError(t, rdb.Do(ctx, "getex", "foo", "persist").Err())
require.EqualValues(t, -1, rdb.TTL(ctx, "foo").Val())
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10, "persist").Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 10000, "persist").Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli(), "persist").Err(), "syntax err")
git-hulk marked this conversation as resolved.
Show resolved Hide resolved
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "exat", time.Now().Add(100*time.Second).Unix(), "persist").Err(), "syntax err")
})

t.Run("GETEX with incorrect use of multi options should result in syntax err", func(t *testing.T) {
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 100, "ex", 10).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 100, "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 100, "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10, "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10, "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli(), "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
})

t.Run("GETEX no option", func(t *testing.T) {
Expand Down Expand Up @@ -445,13 +487,27 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET Duplicate EX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "ex", "1", "ex", "2", "ex", "10").Val())
ttl := rdb.TTL(ctx, "foo").Val()
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "px", "10000").Val())
ttl := rdb.TTL(ctx, "foo").Val()
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET Duplicate PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "px", "100", "px", "1000", "px", "10000").Val())
ttl := rdb.TTL(ctx, "foo").Val()
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET EXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())

Expand All @@ -461,6 +517,16 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET Duplicate EXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())

expireFirst := strconv.FormatInt(time.Now().Add(1*time.Second).Unix(), 10)
expireSecond := strconv.FormatInt(time.Now().Add(10*time.Second).Unix(), 10)
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "exat", expireFirst, "exat", expireSecond).Val())
ttl := rdb.TTL(ctx, "foo").Val()
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET EXAT option with expired timestamp", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "exat", "1").Val())
Expand All @@ -484,6 +550,16 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET Duplicate PXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())

expireFirst := strconv.FormatInt(time.Now().Add(1*time.Second).UnixMilli(), 10)
expireSecond := strconv.FormatInt(time.Now().Add(10*time.Second).UnixMilli(), 10)
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "pxat", expireFirst, "pxat", expireSecond).Val())
ttl := rdb.TTL(ctx, "foo").Val()
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})

t.Run("Extended SET PXAT option with expired timestamp", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "pxat", "1").Val())
Expand All @@ -497,7 +573,12 @@ func TestString(t *testing.T) {
})

t.Run("Extended SET with incorrect use of multi options should result in syntax err", func(t *testing.T) {
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex", "10", "px", "10000").Err(), "syntax err")
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "px", 100, "ex", 10).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "px", 100, "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "px", 100, "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex", 10, "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex", 10, "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "pxat", time.Now().Add(10000*time.Millisecond).UnixMilli(), "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "NX", "XX").Err(), "syntax err")
})

Expand Down Expand Up @@ -558,6 +639,28 @@ func TestString(t *testing.T) {
require.Equal(t, "", rdb.Get(ctx, "cas_key").Val())
})

t.Run("CAS expire Duplicate EX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "cas_key").Err())
require.NoError(t, rdb.Set(ctx, "cas_key", "123", 0).Err())
require.EqualValues(t, 1, rdb.Do(ctx, "CAS", "cas_key", "123", "234", "ex", 100, "ex", 10).Val())
util.BetweenValues(t, rdb.TTL(ctx, "cas_key").Val(), 5*time.Second, 10*time.Second)
})

t.Run("CAS expire Duplicate PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "cas_key").Err())
require.NoError(t, rdb.Set(ctx, "cas_key", "123", 0).Err())
require.EqualValues(t, 1, rdb.Do(ctx, "CAS", "cas_key", "123", "234", "px", 1000, "px", 10000).Val())
util.BetweenValues(t, rdb.TTL(ctx, "cas_key").Val(), 5000*time.Millisecond, 10000*time.Millisecond)
})

t.Run("CAS expire PX option and EX option exist at the same time", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "cas_key").Err())
require.NoError(t, rdb.Set(ctx, "cas_key", "123", 0).Err())
require.ErrorContains(t, rdb.Do(ctx, "CAS", "cas_key", "123", "234", "ex", 100, "px", 100000).Err(), "syntax error")
require.ErrorContains(t, rdb.Do(ctx, "CAS", "cas_key", "123", "234", "ex", 100, "ex", 10, "px", 10000).Err(), "syntax error")
require.ErrorContains(t, rdb.Do(ctx, "CAS", "cas_key", "123", "234", "px", 10000, "ex", 100, "ex", 10, "px", 10000).Err(), "syntax error")
})

t.Run("CAD normal case", func(t *testing.T) {
require.EqualValues(t, -1, rdb.Do(ctx, "CAD", "cad_key", "123").Val())
require.NoError(t, rdb.Set(ctx, "cad_key", "123", 0).Err())
Expand Down