fix(stream): add KeyRangeGen for XRead and XReadGroup

[LindaSummer]

Issue

Close #2625 .

Proposed Changes

• Refactor Parse function for XRead and XReadGroup.
• Add KeyRangeGen for XRead and XReadGroup.

[PragmaTwice]

Hmm actually from my side I don't think we need to reuse the code of Parse(), since:

• many fields in parsed_args is useless in key range gen, so we made lots of useless string copies.
• in key range gen we can assume that the syntax of the input args is valid, so we can omit many checks.

[LindaSummer]

Hi @PragmaTwice ,

Thanks for your suggestions! 😊

I have reverted the original Parse function and add a simple search function for parsing stream key counts.

Best Regards,
Edward

[PragmaTwice]

Suggested change

	static CommandKeyRangeGen KeyRangeGen() {
	static constexpr auto keyRangeGenFunc = [](const std::vector<std::string> &args) {
	static CommandKeyRange KeyRangeGen(const std::vector<std::string> &args) {

Also we don't need to make a function that returns a function here.

[LindaSummer]

Hi @PragmaTwice ,

Got it!

I have changed it to a static variable. 😊

Best Regards,
Edward

[PragmaTwice]

Suggested change

	auto stream_keyword_iter =
	std::find_if(args.cbegin(), args.cend(), [](const std::string &arg) { return util::ToLower(arg) == "streams"; });
	auto stream_keyword_iter =
	std::find_if(args.cbegin(), args.cend(), [](const std::string &arg) { return util::EqualICase(arg, "streams"); });

[LindaSummer]

Hi @PragmaTwice ,

Thanks very much for your suggestion! 😊

I have replaced the comparison operator to util::EqualICase.

Best Regards,
Edward

[PragmaTwice]

For XREADGROUP, I think we need to find from begin + 4, to avoid a group or consumer named streams.

refer to https://redis.io/docs/latest/commands/xreadgroup/ .

[LindaSummer]

Hi @PragmaTwice ,

Thanks very much for your review! 😊

I have updated the ParseStreamReadRange with a begin offset.

Best Regards,
Edward

[PragmaTwice]

The code looks good to me.

Would you like to write a go test case that calls redis command COMMAND GETKEYS XREAD/XREADGROUP ... to confirm the returned keys is correct?

You can add a new test like https://github.com/apache/kvrocks/blob/unstable/tests/gocase/unit/command/command_test.go#L125.

[LindaSummer]

The code looks good to me.

Would you like to write a go test case that calls redis command COMMAND GETKEYS XREAD/XREADGROUP ... to confirm the returned keys is correct?

You can add a new test like https://github.com/apache/kvrocks/blob/unstable/tests/gocase/unit/command/command_test.go#L125.

Of course!

I will add unittest later today.😄

Best Regards,
Edward

[LindaSummer]

Hi @PragmaTwice ,

I have added related unit tests for different combinations of XREAD and XREADGROUP. 😊

Best Regards,
Edward

[sonarqubecloud]

Quality Gate failed

Failed conditions
47.2% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud