Skip to content

Commit 616a708

Browse files
authored
chore: Handle hsetex options in any order (#4754)
NX and KEEPTTL can be specified in any order, but only one of each or both must be specified. Signed-off-by: Abhijat Malviya <[email protected]>
1 parent 70e757f commit 616a708

File tree

2 files changed

+25
-2
lines changed

2 files changed

+25
-2
lines changed

src/server/hset_family.cc

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -728,8 +728,26 @@ void HSetEx(CmdArgList args, const CommandContext& cmd_cntx) {
728728
string_view key = parser.Next();
729729
OpSetParams op_sp;
730730

731-
op_sp.skip_if_exists = parser.Check("NX");
732-
op_sp.keepttl = parser.Check("KEEPTTL");
731+
const auto option_already_set = [&cmd_cntx] {
732+
return cmd_cntx.rb->SendError(WrongNumArgsError(cmd_cntx.conn_cntx->cid->name()));
733+
};
734+
735+
while (true) {
736+
if (parser.Check("NX")) {
737+
if (op_sp.skip_if_exists) {
738+
return option_already_set();
739+
}
740+
op_sp.skip_if_exists = true;
741+
} else if (parser.Check("KEEPTTL")) {
742+
if (op_sp.keepttl) {
743+
return option_already_set();
744+
}
745+
op_sp.keepttl = true;
746+
} else {
747+
break;
748+
}
749+
}
750+
733751
op_sp.ttl = parser.Next<uint32_t>();
734752

735753
if (parser.HasError()) {

src/server/hset_family_test.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,11 @@ TEST_F(HSetFamilyTest, HSetEx) {
405405
EXPECT_EQ(Run({"HSET", "k", "nottl", "val"}), 1);
406406
EXPECT_EQ(Run({"HSETEX", "k", "KEEPTTL", long_time, "nottl", "newval"}), 0);
407407
EXPECT_EQ(Run({"FIELDTTL", "k", "nottl"}).GetInt(), 100);
408+
409+
EXPECT_THAT(Run({"HSETEX", "k", "NX", "KEEPTTL", "NX", "1", "v", "v2"}),
410+
ErrArg("ERR wrong number of arguments for 'hsetex' command"));
411+
EXPECT_THAT(Run({"HSETEX", "k", "KEEPTTL", "KEEPTTL", "1", "v", "v2"}),
412+
ErrArg("ERR wrong number of arguments for 'hsetex' command"));
408413
}
409414

410415
TEST_F(HSetFamilyTest, TriggerConvertToStrMap) {

0 commit comments

Comments
 (0)