From ede1ce026942e87f2b2b4e4a002c4def9f572b1e Mon Sep 17 00:00:00 2001 From: tidwall Date: Fri, 23 Sep 2022 10:42:43 -0700 Subject: [PATCH] Better GET/DROP tests --- internal/server/crud.go | 137 ++++++++++++++++++++----------------- internal/server/scripts.go | 4 +- internal/server/server.go | 4 +- tests/keys_test.go | 48 ++++++++++--- 4 files changed, 117 insertions(+), 76 deletions(-) diff --git a/internal/server/crud.go b/internal/server/crud.go index 65702cd3..9d07ca73 100644 --- a/internal/server/crud.go +++ b/internal/server/crud.go @@ -107,54 +107,73 @@ func (s *Server) cmdTYPE(msg *Message) (resp.Value, error) { return resp.SimpleStringValue(typ), nil } -func (s *Server) cmdGet(msg *Message) (resp.Value, error) { +// GET key id [WITHFIELDS] [OBJECT|POINT|BOUNDS|(HASH geohash)] +func (s *Server) cmdGET(msg *Message) (resp.Value, error) { start := time.Now() - vs := msg.Args[1:] - var ok bool - var key, id, typ, sprecision string - if vs, key, ok = tokenval(vs); !ok || key == "" { - return NOMessage, errInvalidNumberOfArguments - } - if vs, id, ok = tokenval(vs); !ok || id == "" { - return NOMessage, errInvalidNumberOfArguments + // >> Args + + args := msg.Args + + if len(args) < 3 { + return retrerr(errInvalidNumberOfArguments) } + key, id := args[1], args[2] withfields := false - if _, peek, ok := tokenval(vs); ok && strings.ToLower(peek) == "withfields" { - withfields = true - vs = vs[1:] + kind := "object" + var precision int64 + for i := 3; i < len(args); i++ { + switch strings.ToLower(args[i]) { + case "withfields": + withfields = true + case "object": + kind = "object" + case "point": + kind = "point" + case "bounds": + kind = "bounds" + case "hash": + kind = "hash" + i++ + if i == len(args) { + return retrerr(errInvalidNumberOfArguments) + } + var err error + precision, err = strconv.ParseInt(args[i], 10, 64) + if err != nil || precision < 1 || precision > 12 { + return retrerr(errInvalidArgument(args[i])) + } + default: + return retrerr(errInvalidNumberOfArguments) + } } + // >> Operation + col, _ := s.cols.Get(key) if col == nil { if msg.OutputType == RESP { return resp.NullValue(), nil } - return NOMessage, errKeyNotFound + return retrerr(errKeyNotFound) } o := col.Get(id) - ok = o != nil - if !ok { + if o == nil { if msg.OutputType == RESP { return resp.NullValue(), nil } - return NOMessage, errIDNotFound + return retrerr(errIDNotFound) } + // >> Response + vals := make([]resp.Value, 0, 2) var buf bytes.Buffer if msg.OutputType == JSON { buf.WriteString(`{"ok":true`) } - vs, typ, ok = tokenval(vs) - typ = strings.ToLower(typ) - if !ok { - typ = "object" - } - switch typ { - default: - return NOMessage, errInvalidArgument(typ) + switch kind { case "object": if msg.OutputType == JSON { buf.WriteString(`,"object":`) @@ -183,16 +202,9 @@ func (s *Server) cmdGet(msg *Message) (resp.Value, error) { } } case "hash": - if vs, sprecision, ok = tokenval(vs); !ok || sprecision == "" { - return NOMessage, errInvalidNumberOfArguments - } if msg.OutputType == JSON { buf.WriteString(`,"hash":`) } - precision, err := strconv.ParseInt(sprecision, 10, 64) - if err != nil || precision < 1 || precision > 12 { - return NOMessage, errInvalidArgument(sprecision) - } center := o.Geo().Center() p := geohash.EncodeWithPrecision(center.Y, center.X, uint(precision)) if msg.OutputType == JSON { @@ -219,9 +231,6 @@ func (s *Server) cmdGet(msg *Message) (resp.Value, error) { } } - if len(vs) != 0 { - return NOMessage, errInvalidNumberOfArguments - } if withfields { nfields := o.Fields().Len() if nfields > 0 { @@ -250,20 +259,17 @@ func (s *Server) cmdGet(msg *Message) (resp.Value, error) { } } } - switch msg.OutputType { - case JSON: + if msg.OutputType == JSON { buf.WriteString(`,"elapsed":"` + time.Since(start).String() + "\"}") return resp.StringValue(buf.String()), nil - case RESP: - var oval resp.Value - if withfields { - oval = resp.ArrayValue(vals) - } else { - oval = vals[0] - } - return oval, nil } - return NOMessage, nil + var oval resp.Value + if withfields { + oval = resp.ArrayValue(vals) + } else { + oval = vals[0] + } + return oval, nil } // DEL key id [ERRON404] @@ -405,27 +411,32 @@ func (s *Server) cmdPDEL(msg *Message) (resp.Value, commandDetails, error) { return res, d, nil } -func (s *Server) cmdDrop(msg *Message) (res resp.Value, d commandDetails, err error) { +// DROP key +func (s *Server) cmdDROP(msg *Message) (resp.Value, commandDetails, error) { start := time.Now() - vs := msg.Args[1:] - var ok bool - if vs, d.key, ok = tokenval(vs); !ok || d.key == "" { - err = errInvalidNumberOfArguments - return + + // >> Args + + args := msg.Args + if len(args) != 2 { + return retwerr(errInvalidNumberOfArguments) } - if len(vs) != 0 { - err = errInvalidNumberOfArguments - return - } - col, _ := s.cols.Get(d.key) + key := args[1] + + // >> Operation + + col, _ := s.cols.Get(key) if col != nil { - s.cols.Delete(d.key) - d.updated = true - } else { - d.key = "" // ignore the details - d.updated = false + s.cols.Delete(key) } - s.groupDisconnectCollection(d.key) + s.groupDisconnectCollection(key) + + // >> Response + + var res resp.Value + var d commandDetails + d.key = key + d.updated = col != nil d.command = "drop" d.timestamp = time.Now() switch msg.OutputType { @@ -438,7 +449,7 @@ func (s *Server) cmdDrop(msg *Message) (res resp.Value, d commandDetails, err er res = resp.IntegerValue(0) } } - return + return res, d, nil } func (s *Server) cmdRename(msg *Message) (res resp.Value, d commandDetails, err error) { diff --git a/internal/server/scripts.go b/internal/server/scripts.go index 1c3a5fc5..82fed1aa 100644 --- a/internal/server/scripts.go +++ b/internal/server/scripts.go @@ -600,7 +600,7 @@ func (s *Server) commandInScript(msg *Message) ( case "pdel": res, d, err = s.cmdPDEL(msg) case "drop": - res, d, err = s.cmdDrop(msg) + res, d, err = s.cmdDROP(msg) case "expire": res, d, err = s.cmdEXPIRE(msg) case "rename": @@ -626,7 +626,7 @@ func (s *Server) commandInScript(msg *Message) ( case "bounds": res, err = s.cmdBOUNDS(msg) case "get": - res, err = s.cmdGet(msg) + res, err = s.cmdGET(msg) case "jget": res, err = s.cmdJget(msg) case "jset": diff --git a/internal/server/server.go b/internal/server/server.go index b27ac477..9842791a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1024,7 +1024,7 @@ func (s *Server) command(msg *Message, client *Client) ( case "pdel": res, d, err = s.cmdPDEL(msg) case "drop": - res, d, err = s.cmdDrop(msg) + res, d, err = s.cmdDROP(msg) case "flushdb": res, d, err = s.cmdFLUSHDB(msg) case "rename": @@ -1098,7 +1098,7 @@ func (s *Server) command(msg *Message, client *Client) ( case "bounds": res, err = s.cmdBOUNDS(msg) case "get": - res, err = s.cmdGet(msg) + res, err = s.cmdGET(msg) case "jget": res, err = s.cmdJget(msg) case "jset": diff --git a/tests/keys_test.go b/tests/keys_test.go index ed5b3998..ee0d92b5 100644 --- a/tests/keys_test.go +++ b/tests/keys_test.go @@ -75,15 +75,20 @@ func keys_DEL_test(mc *mockServer) error { } func keys_DROP_test(mc *mockServer) error { - return mc.DoBatch([][]interface{}{ - {"SET", "mykey", "myid1", "HASH", "9my5xp7"}, {"OK"}, - {"SET", "mykey", "myid2", "HASH", "9my5xp8"}, {"OK"}, - {"SCAN", "mykey", "COUNT"}, {2}, - {"DROP", "mykey"}, {1}, - {"SCAN", "mykey", "COUNT"}, {0}, - {"DROP", "mykey"}, {0}, - {"SCAN", "mykey", "COUNT"}, {0}, - }) + return mc.DoBatch( + Do("SET", "mykey", "myid1", "HASH", "9my5xp7").OK(), + Do("SET", "mykey", "myid2", "HASH", "9my5xp8").OK(), + Do("SCAN", "mykey", "COUNT").Str("2"), + Do("DROP").Err("wrong number of arguments for 'drop' command"), + Do("DROP", "mykey", "arg3").Err("wrong number of arguments for 'drop' command"), + Do("DROP", "mykey").Str("1"), + Do("SCAN", "mykey", "COUNT").Str("0"), + Do("DROP", "mykey").Str("0"), + Do("SCAN", "mykey", "COUNT").Str("0"), + Do("SET", "mykey", "myid1", "HASH", "9my5xp7").Str("OK"), + Do("DROP", "mykey").JSON().OK(), + Do("DROP", "mykey").JSON().OK(), + ) } func keys_RENAME_test(mc *mockServer) error { return mc.DoBatch([][]interface{}{ @@ -157,6 +162,31 @@ func keys_GET_test(mc *mockServer) error { Do("GET", "mykey", "myid").Str("value2"), Do("DEL", "mykey", "myid").Str("1"), Do("GET", "mykey", "myid").Str(""), + Do("GET", "mykey").Err("wrong number of arguments for 'get' command"), + Do("GET", "mykey", "myid", "hash").Err("wrong number of arguments for 'get' command"), + Do("GET", "mykey", "myid", "hash", "0").Err("invalid argument '0'"), + Do("GET", "mykey", "myid", "hash", "-1").Err("invalid argument '-1'"), + Do("GET", "mykey", "myid", "hash", "13").Err("invalid argument '13'"), + Do("SET", "mykey", "myid", "field", "hello", "world", "field", "hiya", 55, "point", 33, -112).OK(), + Do("GET", "mykey", "myid", "hash", "1").Str("9"), + Do("GET", "mykey", "myid", "point").Str("[33 -112]"), + Do("GET", "mykey", "myid", "bounds").Str("[[33 -112] [33 -112]]"), + Do("GET", "mykey", "myid", "object").Str(`{"type":"Point","coordinates":[-112,33]}`), + Do("GET", "mykey", "myid", "object").Str(`{"type":"Point","coordinates":[-112,33]}`), + Do("GET", "mykey", "myid", "withfields", "point").Str(`[[33 -112] [hello world hiya 55]]`), + Do("GET", "mykey", "myid", "joint").Err("wrong number of arguments for 'get' command"), + Do("GET", "mykey2", "myid").Str(""), + Do("GET", "mykey2", "myid").JSON().Err("key not found"), + Do("GET", "mykey", "myid2").Str(""), + Do("GET", "mykey", "myid2").JSON().Err("id not found"), + Do("GET", "mykey", "myid", "point").JSON().Str(`{"ok":true,"point":{"lat":33,"lon":-112}}`), + Do("GET", "mykey", "myid", "object").JSON().Str(`{"ok":true,"object":{"type":"Point","coordinates":[-112,33]}}`), + Do("GET", "mykey", "myid", "hash", "1").JSON().Str(`{"ok":true,"hash":"9"}`), + Do("GET", "mykey", "myid", "bounds").JSON().Str(`{"ok":true,"bounds":{"sw":{"lat":33,"lon":-112},"ne":{"lat":33,"lon":-112}}}`), + Do("SET", "mykey", "myid2", "point", 33, -112, 55).OK(), + Do("GET", "mykey", "myid2", "point").Str("[33 -112 55]"), + Do("GET", "mykey", "myid2", "point").JSON().Str(`{"ok":true,"point":{"lat":33,"lon":-112,"z":55}}`), + Do("GET", "mykey", "myid", "withfields").JSON().Str(`{"ok":true,"object":{"type":"Point","coordinates":[-112,33]},"fields":{"hello":"world","hiya":55}}`), ) } func keys_KEYS_test(mc *mockServer) error {