Better GET/DROP tests

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
+	// >> Args
-	var key, id, typ, sprecision string
+
-	if vs, key, ok = tokenval(vs); !ok || key == "" {
+	args := msg.Args
-		return NOMessage, errInvalidNumberOfArguments
+
-	}
+	if len(args) < 3 {
-	if vs, id, ok = tokenval(vs); !ok || id == "" {
+		return retrerr(errInvalidNumberOfArguments)
-		return NOMessage, errInvalidNumberOfArguments
 	}
+	key, id := args[1], args[2]
 
 	withfields := false
-	if _, peek, ok := tokenval(vs); ok && strings.ToLower(peek) == "withfields" {
+	kind := "object"
-		withfields = true
+	var precision int64
-		vs = vs[1:]
+	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 o == nil {
-	if !ok {
 		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)
+	switch kind {
-	typ = strings.ToLower(typ)
-	if !ok {
-		typ = "object"
-	}
-	switch typ {
-	default:
-		return NOMessage, errInvalidArgument(typ)
 	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 {
+	if msg.OutputType == JSON {
-	case 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
+	// >> Args
-	if vs, d.key, ok = tokenval(vs); !ok || d.key == "" {
+
-		err = errInvalidNumberOfArguments
+	args := msg.Args
-		return
+	if len(args) != 2 {
+		return retwerr(errInvalidNumberOfArguments)
 	}
-	if len(vs) != 0 {
+	key := args[1]
-		err = errInvalidNumberOfArguments
+
-		return
+	// >> Operation
-	}
+
-	col, _ := s.cols.Get(d.key)
+	col, _ := s.cols.Get(key)
 	if col != nil {
-		s.cols.Delete(d.key)
+		s.cols.Delete(key)
-		d.updated = true
-	} else {
-		d.key = "" // ignore the details
-		d.updated = false
 	}
-	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) {

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":

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":

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{}{
+	return mc.DoBatch(
-		{"SET", "mykey", "myid1", "HASH", "9my5xp7"}, {"OK"},
+		Do("SET", "mykey", "myid1", "HASH", "9my5xp7").OK(),
-		{"SET", "mykey", "myid2", "HASH", "9my5xp8"}, {"OK"},
+		Do("SET", "mykey", "myid2", "HASH", "9my5xp8").OK(),
-		{"SCAN", "mykey", "COUNT"}, {2},
+		Do("SCAN", "mykey", "COUNT").Str("2"),
-		{"DROP", "mykey"}, {1},
+		Do("DROP").Err("wrong number of arguments for 'drop' command"),
-		{"SCAN", "mykey", "COUNT"}, {0},
+		Do("DROP", "mykey", "arg3").Err("wrong number of arguments for 'drop' command"),
-		{"DROP", "mykey"}, {0},
+		Do("DROP", "mykey").Str("1"),
-		{"SCAN", "mykey", "COUNT"}, {0},
+		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("<nil>"),
+		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("<nil>"),
+		Do("GET", "mykey2", "myid").JSON().Err("key not found"),
+		Do("GET", "mykey", "myid2").Str("<nil>"),
+		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 {