Implement the command hello

[mapleFU]

This patch tries to fix this issue: #753

It's a WIP because:

1. I don't know if I need to implement the logic about authentication.
2. I don't know how to write some returning code like "sentinel", "version"

Also, I update some implementions in Redis::Array, it will suffer from lots of unnecessary memory allocations

[mapleFU]

@git-hulk mind take a look? I need some help here

[git-hulk]

Hi, @mapleFU, many thanks for your contribution.

I don't know if I need to implement the logic about authentication.

Yes, but there's a bit different in Kvrocks since it has NOT username, so we can always ignore the username. For the auth logic can see redis_cmd.cc#L78.

I don't know how to write some returning code like "sentinel", "version"

Can ignore those sentinel fields since we are never running in sentinel mode.

[mapleFU]

@git-hulk Now I extract the setting logic from CommandAuth, and implement it in HELLO.

But I wonder how can I testing hello, and would it be ambigious if setName is called when conn is an admin? And should we update the tokens here? And should I add some extra return values?

[git-hulk]

But I wonder how can I testing hello

Do you mean where to add the test case? if yes, can do it in gocase/unit/auth

would it be ambigious if setName is called when conn is an admin? And should we update the tokens here? And should I add some extra return values?

I didn't get this point. For SetName will only add an alias name for connection without other side effects. And no need to update any token/password here.

[mapleFU]

Hi, I'm trying to adding unit tests for hello, but here I come cross a problem:

ADD_CMD("hello", -1,  "read-only ok-loading", 0, 0, 0, CommandHello),

After adding the ADD_CMD above, the auth test would failed:

=== RUN   TestAuth
--- FAIL: TestAuth (2.01s)
=== RUN   TestAuth/AUTH_fails_when_a_wrong_password_is_given
    auth_test.go:56: 
        	Error Trace:	/Users/fuxuwei/workspace/CMakeLibs/incubator-kvrocks/tests/gocase/unit/auth/auth_test.go:56
        	Error:      	Error "NOAUTH Authentication required." does not contain "invalid password"
        	Test:       	TestAuth/AUTH_fails_when_a_wrong_password_is_given
    --- FAIL: TestAuth/AUTH_fails_when_a_wrong_password_is_given (0.00s)

=== RUN   TestAuth/Arbitrary_command_gives_an_error_when_AUTH_is_required
    --- PASS: TestAuth/Arbitrary_command_gives_an_error_when_AUTH_is_required (0.00s)
=== RUN   TestAuth/AUTH_succeeds_when_the_right_password_is_given
    auth_test.go:66: 
        	Error Trace:	/Users/fuxuwei/workspace/CMakeLibs/incubator-kvrocks/tests/gocase/unit/auth/auth_test.go:66
        	Error:      	Not equal: 
        	            	expected: string("OK")
        	            	actual  : <nil>(<nil>)
        	Test:       	TestAuth/AUTH_succeeds_when_the_right_password_is_given
    --- FAIL: TestAuth/AUTH_succeeds_when_the_right_password_is_given (0.00s)


Expected :string("OK")
Actual   :<nil>(<nil>)
<Click to see difference>

Even leaving an empty implements in CommandHello, or not changing the logic of CommandAuth, the test would failed. I think maybe I miss some details in redis_cmd, do you have any ideas? @git-hulk

[mapleFU]

It will be no problem when i change ADD_CMD("hello" ... to ADD_CMD("HELLO", ... or "ciallo" , which really makes me mad...

[mapleFU]

Oh, finally I find out the reason. go-redis will send hello 3 to kvrocks, and it will be filtered by auth checking...

[mapleFU]

[TIMEOUT]: clients state report follows.

maybe I should rerun the test?

[git-hulk]

[TIMEOUT]: clients state report follows.

maybe I should rerun the test?

Yes, this failure isn't caused by the current PR, let's rerun.

[tanruixiang]

LGTM. Thanks for your contribution.

[PragmaTwice]

LGTM

[git-hulk]

Thanks all, will merge this PR after the CI becomes green.

[mapleFU]

@git-hulk it's able to be merged now （*＾3＾）

[git-hulk]

Thanks @mapleFU again