From 832b1c9b649f69e09d0b68a82bb084c6811f45af Mon Sep 17 00:00:00 2001 From: Andy Dunstall Date: Mon, 22 May 2023 20:29:41 +0100 Subject: [PATCH] fix: remove xread required arguments (#1263) --- src/server/stream_family.cc | 19 +------------------ src/server/stream_family_test.cc | 12 ------------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/server/stream_family.cc b/src/server/stream_family.cc index 4d89da08e..1d595263e 100644 --- a/src/server/stream_family.cc +++ b/src/server/stream_family.cc @@ -1197,7 +1197,6 @@ void StreamFamily::XRead(CmdArgList args, ConnectionContext* cntx) { size_t streams_arg = 0; uint32_t count = kuint32max; - bool count_found = false; // Parse the arguments. for (size_t id_indx = 0; id_indx < args.size(); ++id_indx) { @@ -1213,7 +1212,6 @@ void StreamFamily::XRead(CmdArgList args, ConnectionContext* cntx) { if (!absl::SimpleAtoi(arg, &count)) { return (*cntx)->SendError(kSyntaxErr); } - count_found = true; } else if (arg == "STREAMS" && remaining_args > 0) { streams_arg = id_indx + 1; @@ -1235,15 +1233,6 @@ void StreamFamily::XRead(CmdArgList args, ConnectionContext* cntx) { return (*cntx)->SendError(kSyntaxErr); } - // TODO NB: Currently require 2 streams and a COUNT option due to the - // transaction expecting stream keys at positions 4 and 5. - if (!count_found) { - return (*cntx)->SendError("requires COUNT option", kSyntaxErr); - } - if (streams_count != 2) { - return (*cntx)->SendError("requires 2 streams", kSyntaxErr); - } - ReadOpts read_opts; read_opts.count = count; @@ -1439,13 +1428,7 @@ void StreamFamily::Register(CommandRegistry* registry) { << CI{"XLEN", CO::READONLY | CO::FAST, 2, 1, 1, 1}.HFUNC(XLen) << CI{"XRANGE", CO::READONLY, -4, 1, 1, 1}.HFUNC(XRange) << CI{"XREVRANGE", CO::READONLY, -4, 1, 1, 1}.HFUNC(XRevRange) - // TODO NB: Assuming: - // * We always have a COUNT parameter - // * We always have two streams - // * Don't support BLOCK - // Therefore the command has format: - // XREAD COUNT STREAMS - // Where the keys are and . + // TODO NB: Doesn't support BLOCK << CI{"XREAD", CO::READONLY | CO::REVERSE_MAPPING | CO::VARIADIC_KEYS, -3, 3, 3, 1} .HFUNC(XRead) << CI{"XSETID", CO::WRITE | CO::DENYOOM, 3, 1, 1, 1}.HFUNC(XSetId) diff --git a/src/server/stream_family_test.cc b/src/server/stream_family_test.cc index 67409892f..7c182cc82 100644 --- a/src/server/stream_family_test.cc +++ b/src/server/stream_family_test.cc @@ -175,18 +175,6 @@ TEST_F(StreamFamilyTest, XReadInvalidArgs) { // Unbalanced list of streams. resp = Run({"xread", "count", "invalid", "streams", "s1", "s2", "s3", "0", "0"}); EXPECT_THAT(resp, ErrArg("syntax error")); - - // Missing COUNT option. - // TODO Remove once support optional COUNT option. - resp = Run({"xread", "streams", "s1", "s2", "0", "0"}); - EXPECT_THAT(resp, ErrArg("requires COUNT option")); - - // Less/more than two streams. - // TODO Remove once support a variable number of streams. - resp = Run({"xread", "count", "5", "streams", "s1", "0"}); - EXPECT_THAT(resp, ErrArg("requires 2 streams")); - resp = Run({"xread", "count", "5", "streams", "s1", "s2", "s3", "0", "0", "0"}); - EXPECT_THAT(resp, ErrArg("requires 2 streams")); } TEST_F(StreamFamilyTest, Issue854) {