From e48663b03ad42ae5737c1198b92b1daee8202e12 Mon Sep 17 00:00:00 2001 From: Nikolay Turpitko Date: Thu, 21 Jan 2016 22:07:36 +0600 Subject: [PATCH 1/3] RESP writer test --- server/client_resp.go | 4 ++ server/client_resp_test.go | 75 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 server/client_resp_test.go diff --git a/server/client_resp.go b/server/client_resp.go index 55e0db5..5719ad7 100644 --- a/server/client_resp.go +++ b/server/client_resp.go @@ -280,6 +280,10 @@ func (w *respWriter) writeArray(lst []interface{}) { w.writeBulk(nil) case int64: w.writeInteger(v) + case string: + w.writeStatus(v) + case error: + w.writeError(v) default: panic(fmt.Sprintf("invalid array type %T %v", lst[i], v)) } diff --git a/server/client_resp_test.go b/server/client_resp_test.go new file mode 100644 index 0000000..9a96104 --- /dev/null +++ b/server/client_resp_test.go @@ -0,0 +1,75 @@ +package server + +import ( + "bufio" + "bytes" + "errors" + "testing" +) + +func TestRespWriter(t *testing.T) { + for _, fixture := range []struct { + v interface{} + e string + }{ + { + v: errors.New("Some error"), + //e: "-Some error\r\n", // as described at http://redis.io/topics/protocol + e: "- Some error\r\n", // actual + }, + { + v: "Some status", + e: "+Some status\r\n", + }, + { + v: int64(42), + e: ":42\r\n", + }, + { + v: []byte("ultimate answer"), + e: "$15\r\nultimate answer\r\n", + }, + { + v: []interface{}{[]byte("aaa"), []byte("bbb"), int64(42)}, + e: "*3\r\n$3\r\naaa\r\n$3\r\nbbb\r\n:42\r\n", + }, + { + v: [][]byte{[]byte("test"), nil, []byte("zzz")}, + e: "*3\r\n$4\r\ntest\r\n$-1\r\n$3\r\nzzz\r\n", + }, + { + v: nil, + e: "$-1\r\n", + }, + { + v: []interface{}{[]interface{}{int64(1), int64(2), int64(3)}, []interface{}{"Foo", errors.New("Bar")}}, + //e: "*2\r\n*3\r\n:1\r\n:2\r\n:3\r\n*2\r\n+Foo\r\n-Bar\r\n", + e: "*2\r\n*3\r\n:1\r\n:2\r\n:3\r\n*2\r\n+Foo\r\n- Bar\r\n", + }, + } { + w := new(respWriter) + var b bytes.Buffer + w.buff = bufio.NewWriter(&b) + switch v := fixture.v.(type) { + case error: + w.writeError(v) + case string: + w.writeStatus(v) + case int64: + w.writeInteger(v) + case []byte: + w.writeBulk(v) + case []interface{}: + w.writeArray(v) + case [][]byte: + w.writeSliceArray(v) + default: + w.writeBulk(b.Bytes()) + } + w.flush() + if b.String() != fixture.e { + t.Errorf("respWriter, actual: %v, expected: %v", []byte(b.String()), []byte(fixture.e)) + } + } + +} From b2f90f832ad62268e8a0aabd77b7c4cc5dbde9a2 Mon Sep 17 00:00:00 2001 From: Nikolay Turpitko Date: Thu, 21 Jan 2016 23:48:19 +0600 Subject: [PATCH 2/3] Lua script, HMGET, value for absent key #219 --- server/script.go | 19 ++++++++++++++++++- server/script_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/server/script.go b/server/script.go index d15dfbd..8e6b14f 100644 --- a/server/script.go +++ b/server/script.go @@ -84,8 +84,25 @@ func (w *luaWriter) writeSliceArray(lst [][]byte) { w.l.CreateTable(len(lst), 0) for i, v := range lst { - w.l.PushString(hack.String(v)) + if v == nil { + w.l.PushBoolean(false) + //TNL: not sure what is "expected" behaviour here. + //Redis returns "false", but may be "nil" is more appropriate? + //For me both variants seems better, then empty string. + // + //w.l.PushNil() + } else { + w.l.PushString(hack.String(v)) + } w.l.RawSeti(-2, i+1) + + /* + //TNL: and instead of push nil we can just skip it + if v != nil { + w.l.PushString(hack.String(v)) + w.l.RawSeti(-2, i+1) + } + */ } } diff --git a/server/script_test.go b/server/script_test.go index 0446bb7..bae27ff 100644 --- a/server/script_test.go +++ b/server/script_test.go @@ -105,6 +105,15 @@ var testScript5 = ` return ledis.call("PING") ` +var testScript6 = ` + ledis.call('hmset', 'zzz', 1, 2, 5, 42) + local a = ledis.call('hmget', 'zzz', 1, 2, 5, 42) + for i = 1, 5 do + a[i] = type(a[i]) + end + return a +` + func TestLuaCall(t *testing.T) { cfg := config.NewConfigDefault() cfg.Addr = ":11188" @@ -187,5 +196,29 @@ func TestLuaCall(t *testing.T) { t.Fatal(fmt.Sprintf("%v %T", v, v)) } + err = app.script.l.DoString(testScript6) + if err != nil { + t.Fatal(err) + } + + v = luaReplyToLedisReply(l) + vv := v.([]interface{}) + expected := []string{ + "string", + "boolean", + "string", + "boolean", + "nil", + } + if len(expected) != len(vv) { + t.Fatalf("length different: %d, %d", len(expected), len(vv)) + } + for i, r := range vv { + s := string(r.([]byte)) + if s != expected[i] { + t.Errorf("reply[%d] expected: %s, actual: %s", i, expected[i], s) + } + } + luaClient.db = nil } From f08763cf374c27f6432ee5a7dc3835d1ddd3da2b Mon Sep 17 00:00:00 2001 From: Nikolay Turpitko Date: Fri, 22 Jan 2016 16:57:52 +0500 Subject: [PATCH 3/3] Update script.go OK then. I removed my comments, for bool code should work as is. --- server/script.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/server/script.go b/server/script.go index 8e6b14f..8fd22ec 100644 --- a/server/script.go +++ b/server/script.go @@ -86,23 +86,10 @@ func (w *luaWriter) writeSliceArray(lst [][]byte) { for i, v := range lst { if v == nil { w.l.PushBoolean(false) - //TNL: not sure what is "expected" behaviour here. - //Redis returns "false", but may be "nil" is more appropriate? - //For me both variants seems better, then empty string. - // - //w.l.PushNil() } else { w.l.PushString(hack.String(v)) } w.l.RawSeti(-2, i+1) - - /* - //TNL: and instead of push nil we can just skip it - if v != nil { - w.l.PushString(hack.String(v)) - w.l.RawSeti(-2, i+1) - } - */ } }