Browse Source

Improve Scan and more

- Handle more destination types in Scan function. Scan can now be used
  to process the reply from EXEC.
- Add ScanStruct for scanning alternating name and value muti-bulk
  replies to struct fields.
- Delete old Values function. Add Values function as replacement for
  MultiBulk. MultiBulk remains for compatibility with old code.
- Misc improvements to documentation.
Gary Burd 13 năm trước cách đây
mục cha
commit
6a0ae8124d
9 tập tin đã thay đổi với 392 bổ sung96 xóa
  1. 23 7
      redis/conn.go
  2. 26 4
      redis/conn_test.go
  3. 8 9
      redis/doc.go
  4. 8 5
      redis/pool.go
  5. 5 5
      redis/pubsub.go
  6. 10 7
      redis/reply.go
  7. 256 51
      redis/scan.go
  8. 55 7
      redis/scan_test.go
  9. 1 1
      redis/script_test.go

+ 23 - 7
redis/conn.go

@@ -271,13 +271,16 @@ func (c *conn) Receive() (reply interface{}, err error) {
 	return
 	return
 }
 }
 
 
-func (c *conn) Do(cmd string, args ...interface{}) (reply interface{}, err error) {
-	// Send
+func (c *conn) Do(cmd string, args ...interface{}) (interface{}, error) {
 	if c.writeTimeout != 0 {
 	if c.writeTimeout != 0 {
 		c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
 		c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
 	}
 	}
-	c.writeCommand(cmd, args)
-	if err = c.bw.Flush(); err != nil {
+
+	if cmd != "" {
+		c.writeCommand(cmd, args)
+	}
+
+	if err := c.bw.Flush(); err != nil {
 		return nil, c.fatal(err)
 		return nil, c.fatal(err)
 	}
 	}
 
 
@@ -290,8 +293,21 @@ func (c *conn) Do(cmd string, args ...interface{}) (reply interface{}, err error
 		c.conn.SetReadDeadline(time.Now().Add(c.readTimeout))
 		c.conn.SetReadDeadline(time.Now().Add(c.readTimeout))
 	}
 	}
 
 
-	// Receive
-	for ; pending >= 0; pending-- {
+	if cmd == "" {
+		reply := make([]interface{}, pending)
+		for i := range reply {
+			if r, e := c.readReply(); e != nil {
+				return nil, c.fatal(e)
+			} else {
+				reply[i] = r
+			}
+		}
+		return reply, nil
+	}
+
+	var err error
+	var reply interface{}
+	for i := 0; i <= pending; i++ {
 		var e error
 		var e error
 		if reply, e = c.readReply(); e != nil {
 		if reply, e = c.readReply(); e != nil {
 			return nil, c.fatal(e)
 			return nil, c.fatal(e)
@@ -300,5 +316,5 @@ func (c *conn) Do(cmd string, args ...interface{}) (reply interface{}, err error
 			err = e
 			err = e
 		}
 		}
 	}
 	}
-	return
+	return reply, err
 }
 }

+ 26 - 4
redis/conn_test.go

@@ -268,8 +268,7 @@ func TestPipelineCommands(t *testing.T) {
 
 
 	for _, cmd := range testCommands {
 	for _, cmd := range testCommands {
 		if err := c.Send(cmd.args[0].(string), cmd.args[1:]...); err != nil {
 		if err := c.Send(cmd.args[0].(string), cmd.args[1:]...); err != nil {
-			t.Errorf("Send(%v) returned error %v", cmd.args, err)
-			continue
+			t.Fatalf("Send(%v) returned error %v", cmd.args, err)
 		}
 		}
 	}
 	}
 	if err := c.Flush(); err != nil {
 	if err := c.Flush(); err != nil {
@@ -278,9 +277,32 @@ func TestPipelineCommands(t *testing.T) {
 	for _, cmd := range testCommands {
 	for _, cmd := range testCommands {
 		actual, err := c.Receive()
 		actual, err := c.Receive()
 		if err != nil {
 		if err != nil {
-			t.Errorf("Receive(%v) returned error %v", cmd.args, err)
-			continue
+			t.Fatalf("Receive(%v) returned error %v", cmd.args, err)
+		}
+		if !reflect.DeepEqual(actual, cmd.expected) {
+			t.Errorf("Receive(%v) = %v, want %v", cmd.args, actual, cmd.expected)
+		}
+	}
+}
+
+func TestBlankCommmand(t *testing.T) {
+	c := dialt(t)
+	defer c.Close()
+
+	for _, cmd := range testCommands {
+		if err := c.Send(cmd.args[0].(string), cmd.args[1:]...); err != nil {
+			t.Fatalf("Send(%v) returned error %v", cmd.args, err)
 		}
 		}
+	}
+	reply, err := redis.Values(c.Do(""))
+	if err != nil {
+		t.Fatalf("Do() returned error %v", err)
+	}
+	if len(reply) != len(testCommands) {
+		t.Fatalf("len(reply)=%d, want %d", len(reply), len(testCommands))
+	}
+	for i, cmd := range testCommands {
+		actual := reply[i]
 		if !reflect.DeepEqual(actual, cmd.expected) {
 		if !reflect.DeepEqual(actual, cmd.expected) {
 			t.Errorf("Receive(%v) = %v, want %v", cmd.args, actual, cmd.expected)
 			t.Errorf("Receive(%v) = %v, want %v", cmd.args, actual, cmd.expected)
 		}
 		}

+ 8 - 9
redis/doc.go

@@ -22,7 +22,7 @@
 // The Conn interface is the primary interface for working with Redis.
 // The Conn interface is the primary interface for working with Redis.
 // Applications create connections by calling the Dial, DialWithTimeout or
 // Applications create connections by calling the Dial, DialWithTimeout or
 // NewConn functions. In the future, functions will be added for creating
 // NewConn functions. In the future, functions will be added for creating
-// shareded and other types of connections.
+// sharded and other types of connections.
 //
 //
 // The application must call the connection Close method when the application
 // The application must call the connection Close method when the application
 // is done with the connection.
 // is done with the connection.
@@ -124,12 +124,11 @@
 //
 //
 // Reply Helpers
 // Reply Helpers
 //
 //
-// The Bool, Int, Bytes, String and MultiBulk functions convert a reply to a
-// value of a specific type. To allow convenient wrapping of calls to the
-// connection Do and Receive methods, the functions take a second argument of
-// type error. If the error is non-nil, then the helper function returns the
-// error. If the error is nil, the function converts the reply to the specified
-// type:
+// The Bool, Int, Bytes, String and Values functions convert a reply to a value
+// of a specific type. To allow convenient wrapping of calls to the connection
+// Do and Receive methods, the functions take a second argument of type error.
+// If the error is non-nil, then the helper function returns the error. If the
+// error is nil, the function converts the reply to the specified type:
 //
 //
 //  exists, err := redis.Bool(c.Do("EXISTS", "foo"))
 //  exists, err := redis.Bool(c.Do("EXISTS", "foo"))
 //  if err != nil {
 //  if err != nil {
@@ -140,11 +139,11 @@
 //
 //
 //  var value1 int
 //  var value1 int
 //  var value2 string
 //  var value2 string
-//  mb, err := redis.MultiBulk(c.Do("MGET", "key1", "key2"))
+//  reply, err := redis.Values(c.Do("MGET", "key1", "key2"))
 //  if err != nil {
 //  if err != nil {
 //      // handle error
 //      // handle error
 //  }
 //  }
-//   if _, err := redis.Scan(mb, &value1, &value2); err != nil {
+//   if _, err := redis.Scan(reply, &value1, &value2); err != nil {
 //      // handle error
 //      // handle error
 //  }
 //  }
 package redis
 package redis

+ 8 - 5
redis/pool.go

@@ -39,19 +39,22 @@ var errPoolClosed = errors.New("redigo: connection pool closed")
 //
 //
 //      pool = &redis.Pool{
 //      pool = &redis.Pool{
 //              MaxIdle: 3,
 //              MaxIdle: 3,
-//              Dial: func () (c redis.Conn err error) {
-//                  c, err = redis.Dial("tcp", server)
+//              IdleTimeout: 240 * time.Second,
+//              Dial: func () (redis.Conn error) {
+//                  c, err := redis.Dial("tcp", server)
 //                  if err != nil {
 //                  if err != nil {
 //                      return nil, err
 //                      return nil, err
 //                  }
 //                  }
-//                  if err = c.Do("AUTH", password); err != nil {
+//                  if err := c.Do("AUTH", password); err != nil {
+//                      c.Close()
 //                      return nil, err    
 //                      return nil, err    
 //                  }
 //                  }
+//                  return c, err
 //              },
 //              },
 //          }
 //          }
 //
 //
 // This pool has a maximum of three connections to the server specified by the
 // This pool has a maximum of three connections to the server specified by the
-// variable "server". Each connection is authenticated using a password.
+// variable "server". Each connection is authenticated using a password. 
 //
 //
 // A request handler gets a connection from the pool and closes the connection
 // A request handler gets a connection from the pool and closes the connection
 // when the handler is done:
 // when the handler is done:
@@ -121,7 +124,7 @@ func (p *Pool) get() (c Conn, err error) {
 			p.idle.Remove(e)
 			p.idle.Remove(e)
 
 
 			// Release the pool lock during the potentially long call to the
 			// Release the pool lock during the potentially long call to the
-			// connecton's Close method.
+			// connection's Close method.
 			p.mu.Unlock()
 			p.mu.Unlock()
 			cStale.Close()
 			cStale.Close()
 			p.mu.Lock()
 			p.mu.Lock()

+ 5 - 5
redis/pubsub.go

@@ -94,13 +94,13 @@ func (c PubSubConn) PUnsubscribe(channel ...interface{}) error {
 // error. The return value is intended to be used directly in a type switch as
 // error. The return value is intended to be used directly in a type switch as
 // illustrated in the PubSubConn example.
 // illustrated in the PubSubConn example.
 func (c PubSubConn) Receive() interface{} {
 func (c PubSubConn) Receive() interface{} {
-	multiBulk, err := MultiBulk(c.Conn.Receive())
+	reply, err := Values(c.Conn.Receive())
 	if err != nil {
 	if err != nil {
 		return err
 		return err
 	}
 	}
 
 
 	var kind string
 	var kind string
-	multiBulk, err = Scan(multiBulk, &kind)
+	reply, err = Scan(reply, &kind)
 	if err != nil {
 	if err != nil {
 		return err
 		return err
 	}
 	}
@@ -108,19 +108,19 @@ func (c PubSubConn) Receive() interface{} {
 	switch kind {
 	switch kind {
 	case "message":
 	case "message":
 		var m Message
 		var m Message
-		if _, err := Scan(multiBulk, &m.Channel, &m.Data); err != nil {
+		if _, err := Scan(reply, &m.Channel, &m.Data); err != nil {
 			return err
 			return err
 		}
 		}
 		return m
 		return m
 	case "pmessage":
 	case "pmessage":
 		var pm PMessage
 		var pm PMessage
-		if _, err := Scan(multiBulk, &pm.Pattern, &pm.Channel, &pm.Data); err != nil {
+		if _, err := Scan(reply, &pm.Pattern, &pm.Channel, &pm.Data); err != nil {
 			return err
 			return err
 		}
 		}
 		return pm
 		return pm
 	case "subscribe", "psubscribe", "unsubscribe", "punsubscribe":
 	case "subscribe", "psubscribe", "unsubscribe", "punsubscribe":
 		s := Subscription{Kind: kind}
 		s := Subscription{Kind: kind}
-		if _, err := Scan(multiBulk, &s.Channel, &s.Count); err != nil {
+		if _, err := Scan(reply, &s.Channel, &s.Count); err != nil {
 			return err
 			return err
 		}
 		}
 		return s
 		return s

+ 10 - 7
redis/reply.go

@@ -22,8 +22,8 @@ import (
 
 
 var ErrNil = errors.New("redigo: nil returned")
 var ErrNil = errors.New("redigo: nil returned")
 
 
-// Values is deprecated. Use Scan instead.
-func Values(multiBulk []interface{}, values ...interface{}) ([]interface{}, error) {
+// xValues is deprecated. Use Scan instead.
+func xValues(multiBulk []interface{}, values ...interface{}) ([]interface{}, error) {
 	if len(multiBulk) < len(values) {
 	if len(multiBulk) < len(values) {
 		return nil, errors.New("redigo Values: short multibulk")
 		return nil, errors.New("redigo Values: short multibulk")
 	}
 	}
@@ -160,15 +160,18 @@ func Bool(reply interface{}, err error) (bool, error) {
 	return false, fmt.Errorf("redigo: unexpected type for Bool, got type %T", reply)
 	return false, fmt.Errorf("redigo: unexpected type for Bool, got type %T", reply)
 }
 }
 
 
-// MultiBulk is a helper that converts a command reply to a []interface{}. If
-// err is not equal to nil, then MultiBulk returns nil, err. Otherwise,
-// MultiBulk converts the reply as follows:
+// MultiBulk is deprecated. Use Values.
+func MultiBulk(reply interface{}, err error) ([]interface{}, error) { return Values(reply, err) }
+
+// Values is a helper that converts a multi-bulk command reply to a
+// []interface{}. If err is not equal to nil, then Values returns nil, err.
+// Otherwise, Multi converts the reply as follows:
 //
 //
 //  Reply type      Result
 //  Reply type      Result
 //  multi-bulk      reply, nil
 //  multi-bulk      reply, nil
 //  nil             nil, ErrNil
 //  nil             nil, ErrNil
 //  other           nil, error
 //  other           nil, error
-func MultiBulk(reply interface{}, err error) ([]interface{}, error) {
+func Values(reply interface{}, err error) ([]interface{}, error) {
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
@@ -180,5 +183,5 @@ func MultiBulk(reply interface{}, err error) ([]interface{}, error) {
 	case Error:
 	case Error:
 		return nil, reply
 		return nil, reply
 	}
 	}
-	return nil, fmt.Errorf("redigo: unexpected type for MultiBulk, got type %T", reply)
+	return nil, fmt.Errorf("redigo: unexpected type for Multi, got type %T", reply)
 }
 }

+ 256 - 51
redis/scan.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"fmt"
 	"reflect"
 	"reflect"
 	"strconv"
 	"strconv"
+	"strings"
+	"sync"
 )
 )
 
 
 func cannotConvert(d reflect.Value, s interface{}) error {
 func cannotConvert(d reflect.Value, s interface{}) error {
@@ -26,7 +28,7 @@ func cannotConvert(d reflect.Value, s interface{}) error {
 		reflect.TypeOf(s), d.Type())
 		reflect.TypeOf(s), d.Type())
 }
 }
 
 
-func convertAssignBulk(d reflect.Value, s []byte) (err error) {
+func convertAssignBytes(d reflect.Value, s []byte) (err error) {
 	switch d.Type().Kind() {
 	switch d.Type().Kind() {
 	case reflect.Float32, reflect.Float64:
 	case reflect.Float32, reflect.Float64:
 		var x float64
 		var x float64
@@ -85,72 +87,275 @@ func convertAssignInt(d reflect.Value, s int64) (err error) {
 	return
 	return
 }
 }
 
 
-// Scan copies from a multi-bulk command reply to the values pointed at by
-// dest. 
+func convertAssignValues(d reflect.Value, s []interface{}) (err error) {
+	if d.Type().Kind() != reflect.Slice {
+		return cannotConvert(d, s)
+	}
+	if len(s) > d.Cap() {
+		d.Set(reflect.MakeSlice(d.Type(), len(s), len(s)))
+	} else {
+		d.SetLen(len(s))
+	}
+	for i := 0; i < len(s); i++ {
+		switch s := s[i].(type) {
+		case []byte:
+			err = convertAssignBytes(d.Index(i), s)
+		case int64:
+			err = convertAssignInt(d.Index(i), s)
+		default:
+			err = cannotConvert(d, s)
+		}
+		if err != nil {
+			break
+		}
+	}
+	return
+}
+
+func convertAssign(d interface{}, s interface{}) (err error) {
+	// Handle the most common destination types using type switches and
+	// fall back to reflection for all other types.
+	switch s := s.(type) {
+	case nil:
+		// ingore
+	case []byte:
+		switch d := d.(type) {
+		case *string:
+			*d = string(s)
+		case *int:
+			*d, err = strconv.Atoi(string(s))
+		case *bool:
+			*d, err = strconv.ParseBool(string(s))
+		case *[]byte:
+			*d = s
+		case *interface{}:
+			*d = s
+		case nil:
+			// skip value
+		default:
+			if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
+				err = cannotConvert(d, s)
+			} else {
+				err = convertAssignBytes(d.Elem(), s)
+			}
+		}
+	case int64:
+		switch d := d.(type) {
+		case *int:
+			x := int(s)
+			if int64(x) != s {
+				err = strconv.ErrRange
+				x = 0
+			}
+			*d = x
+		case *bool:
+			*d = s != 0
+		case *interface{}:
+			*d = s
+		case nil:
+			// skip value
+		default:
+			if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
+				err = cannotConvert(d, s)
+			} else {
+				err = convertAssignInt(d.Elem(), s)
+			}
+		}
+	case []interface{}:
+		switch d := d.(type) {
+		case *[]interface{}:
+			*d = s
+		case *interface{}:
+			*d = s
+		case nil:
+			// skip value
+		default:
+			if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
+				err = cannotConvert(d, s)
+			} else {
+				err = convertAssignValues(d.Elem(), s)
+			}
+		}
+	case Error:
+		err = s
+	default:
+		err = cannotConvert(reflect.ValueOf(d), s)
+	}
+	return
+}
+
+// Scan copies from the multi-bulk src to the values pointed at by dest. 
 // 
 // 
-// The values pointed at by test must be a numeric type, boolean, string or a
-// byte slice. Scan uses the standard strconv package to convert bulk values to
-// numeric and boolean types. 
+// The values pointed at by test must be a numeric type, boolean, string,
+// []byte, interface{} or a slice of these types. Scan uses the standard
+// strconv package to convert bulk values to numeric and boolean types. 
+//
+// If a dest value is nil, then the corresponding src value is skipped.
 //
 //
 // If the multi-bulk value is nil, then the corresponding dest value is not
 // If the multi-bulk value is nil, then the corresponding dest value is not
 // modified. 
 // modified. 
 //
 //
-// To enable easy use of Scan in a loop, Scan returns the slice following the
-// copied values.
-func Scan(multiBulk []interface{}, dest ...interface{}) ([]interface{}, error) {
-
-	// We handle the most common dest types using type switches and fallback to
-	// reflection for all other types.
-
-	if len(multiBulk) < len(dest) {
+// To enable easy use of Scan in a loop, Scan returns the slice of src
+// following the copied values.
+func Scan(src []interface{}, dest ...interface{}) ([]interface{}, error) {
+	if len(src) < len(dest) {
 		return nil, errors.New("redigo: Scan multibulk short")
 		return nil, errors.New("redigo: Scan multibulk short")
 	}
 	}
 	var err error
 	var err error
 	for i, d := range dest {
 	for i, d := range dest {
-		switch s := multiBulk[i].(type) {
-		case nil:
-			// ingore
-		case []byte:
-			switch d := d.(type) {
-			case *string:
-				*d = string(s)
-			case *int:
-				*d, err = strconv.Atoi(string(s))
-			case *bool:
-				*d, err = strconv.ParseBool(string(s))
-			case *[]byte:
-				*d = s
-			default:
-				if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
-					err = cannotConvert(d, s)
-				} else {
-					err = convertAssignBulk(d.Elem(), s)
-				}
+		err = convertAssign(d, src[i])
+		if err != nil {
+			break
+		}
+	}
+	return src[len(dest):], err
+}
+
+type fieldSpec struct {
+	name  string
+	index []int
+	//omitEmpty bool
+}
+
+type structSpec struct {
+	m map[string]*fieldSpec
+	l []*fieldSpec
+}
+
+func (ss *structSpec) fieldSpec(name []byte) *fieldSpec {
+	return ss.m[string(name)]
+}
+
+func compileStructSpec(t reflect.Type, depth map[string]int, index []int, ss *structSpec) {
+	for i := 0; i < t.NumField(); i++ {
+		f := t.Field(i)
+		switch {
+		case f.PkgPath != "":
+			// Ignore unexported fields.
+		case f.Anonymous:
+			// TODO: Handle pointers. Requires change to decoder and 
+			// protection against infinite recursion.
+			if f.Type.Kind() == reflect.Struct {
+				compileStructSpec(f.Type, depth, append(index, i), ss)
 			}
 			}
-		case int64:
-			switch d := d.(type) {
-			case *int:
-				x := int(s)
-				if int64(x) != s {
-					err = strconv.ErrRange
-					x = 0
+		default:
+			fs := &fieldSpec{name: f.Name}
+			tag := f.Tag.Get("redis")
+			p := strings.Split(tag, ",")
+			if len(p) > 0 && p[0] != "-" {
+				if len(p[0]) > 0 {
+					fs.name = p[0]
 				}
 				}
-				*d = x
-			case *bool:
-				*d = s != 0
-			default:
-				if d := reflect.ValueOf(d); d.Type().Kind() != reflect.Ptr {
-					err = cannotConvert(d, s)
-				} else {
-					err = convertAssignInt(d.Elem(), s)
+				for _, s := range p[1:] {
+					switch s {
+					//case "omitempty":
+					//  fs.omitempty = true
+					default:
+						panic(errors.New("redigo: unknown field flag " + s + " for type " + t.Name()))
+					}
 				}
 				}
 			}
 			}
+			d, found := depth[fs.name]
+			if !found {
+				d = 1 << 30
+			}
+			switch {
+			case len(index) == d:
+				// At same depth, remove from result.
+				delete(ss.m, fs.name)
+				j := 0
+				for i := 0; i < len(ss.l); i++ {
+					if fs.name != ss.l[i].name {
+						ss.l[j] = ss.l[i]
+						j += 1
+					}
+				}
+				ss.l = ss.l[:j]
+			case len(index) < d:
+				fs.index = make([]int, len(index)+1)
+				copy(fs.index, index)
+				fs.index[len(index)] = i
+				depth[fs.name] = len(index)
+				ss.m[fs.name] = fs
+				ss.l = append(ss.l, fs)
+			}
+		}
+	}
+}
+
+var (
+	structSpecMutex  sync.RWMutex
+	structSpecCache  = make(map[reflect.Type]*structSpec)
+	defaultFieldSpec = &fieldSpec{}
+)
+
+func structSpecForType(t reflect.Type) *structSpec {
+
+	structSpecMutex.RLock()
+	ss, found := structSpecCache[t]
+	structSpecMutex.RUnlock()
+	if found {
+		return ss
+	}
+
+	structSpecMutex.Lock()
+	defer structSpecMutex.Unlock()
+	ss, found = structSpecCache[t]
+	if found {
+		return ss
+	}
+
+	ss = &structSpec{m: make(map[string]*fieldSpec)}
+	compileStructSpec(t, make(map[string]int), nil, ss)
+	structSpecCache[t] = ss
+	return ss
+}
+
+// ScanStruct scans a multi-bulk src containing alternating names and values to
+// a struct. The HGETALL and CONFIG GET commands return replies in this format.
+//
+// ScanStruct uses the struct field name to match values in the response. Use
+// 'redis' field tag to override the name:
+//
+//      Field int `redis:"myName"`
+//
+// Fields with the tag redis:"-" are ignored.
+func ScanStruct(src []interface{}, dest interface{}) error {
+	d := reflect.ValueOf(dest)
+	if d.Kind() != reflect.Ptr || d.IsNil() {
+		return errors.New("redigo: ScanStruct value must be non-nil pointer")
+	}
+	d = d.Elem()
+	ss := structSpecForType(d.Type())
+
+	if len(src)%2 != 0 {
+		return errors.New("redigo: ScanStruct expects even number of values in values")
+	}
+
+	for i := 0; i < len(src); i += 2 {
+		name, ok := src[i].([]byte)
+		if !ok {
+			return errors.New("redigo: ScanStruct key not a bulk value")
+		}
+		fs := ss.fieldSpec(name)
+		if fs == nil {
+			continue
+		}
+		f := d.FieldByIndex(fs.index)
+		var err error
+		switch s := src[i+1].(type) {
+		case nil:
+			// ignore
+		case []byte:
+			err = convertAssignBytes(f, s)
+		case int64:
+			err = convertAssignInt(f, s)
 		default:
 		default:
-			err = cannotConvert(reflect.ValueOf(d), s)
+			err = cannotConvert(f, s)
 		}
 		}
 		if err != nil {
 		if err != nil {
-			break
+			return err
 		}
 		}
 	}
 	}
-	return multiBulk[len(dest):], err
+	return nil
 }
 }

+ 55 - 7
redis/scan_test.go

@@ -48,6 +48,12 @@ var scanConversionTests = []struct {
 	{[]byte("t"), true},
 	{[]byte("t"), true},
 	{[]byte("hello"), "hello"},
 	{[]byte("hello"), "hello"},
 	{[]byte("world"), []byte("world")},
 	{[]byte("world"), []byte("world")},
+	{[]interface{}{[]byte("foo")}, []string{"foo"}},
+	{[]interface{}{[]byte("bar")}, [][]byte{[]byte("bar")}},
+	{[]interface{}{[]byte("1")}, []int{1}},
+	{[]interface{}{[]byte("1"), []byte("2")}, []int{1, 2}},
+	{[]interface{}{[]byte("1")}, []byte{1}},
+	{[]interface{}{[]byte("1")}, []bool{true}},
 }
 }
 
 
 var scanConversionErrorTests = []struct {
 var scanConversionErrorTests = []struct {
@@ -59,13 +65,14 @@ var scanConversionErrorTests = []struct {
 	{[]byte("-1"), byte(0)},
 	{[]byte("-1"), byte(0)},
 	{int64(-1), byte(0)},
 	{int64(-1), byte(0)},
 	{[]byte("junk"), false},
 	{[]byte("junk"), false},
+	{redis.Error("blah"), false},
 }
 }
 
 
 func TestScanConversion(t *testing.T) {
 func TestScanConversion(t *testing.T) {
 	for _, tt := range scanConversionTests {
 	for _, tt := range scanConversionTests {
-		multiBulk := []interface{}{tt.src}
+		values := []interface{}{tt.src}
 		dest := reflect.New(reflect.TypeOf(tt.dest))
 		dest := reflect.New(reflect.TypeOf(tt.dest))
-		multiBulk, err := redis.Scan(multiBulk, dest.Interface())
+		values, err := redis.Scan(values, dest.Interface())
 		if err != nil {
 		if err != nil {
 			t.Errorf("Scan(%v) returned error %v", tt, err)
 			t.Errorf("Scan(%v) returned error %v", tt, err)
 			continue
 			continue
@@ -78,9 +85,9 @@ func TestScanConversion(t *testing.T) {
 
 
 func TestScanConversionError(t *testing.T) {
 func TestScanConversionError(t *testing.T) {
 	for _, tt := range scanConversionErrorTests {
 	for _, tt := range scanConversionErrorTests {
-		multiBulk := []interface{}{tt.src}
+		values := []interface{}{tt.src}
 		dest := reflect.New(reflect.TypeOf(tt.dest))
 		dest := reflect.New(reflect.TypeOf(tt.dest))
-		multiBulk, err := redis.Scan(multiBulk, dest.Interface())
+		values, err := redis.Scan(values, dest.Interface())
 		if err == nil {
 		if err == nil {
 			t.Errorf("Scan(%v) did not return error", tt)
 			t.Errorf("Scan(%v) did not return error", tt)
 		}
 		}
@@ -100,7 +107,7 @@ func ExampleScan() {
 	c.Send("LPUSH", "albums", "1")
 	c.Send("LPUSH", "albums", "1")
 	c.Send("LPUSH", "albums", "2")
 	c.Send("LPUSH", "albums", "2")
 	c.Send("LPUSH", "albums", "3")
 	c.Send("LPUSH", "albums", "3")
-	multiBulk, err := redis.MultiBulk(c.Do("SORT", "albums",
+	values, err := redis.Values(c.Do("SORT", "albums",
 		"BY", "album:*->rating",
 		"BY", "album:*->rating",
 		"GET", "album:*->title",
 		"GET", "album:*->title",
 		"GET", "album:*->rating"))
 		"GET", "album:*->rating"))
@@ -108,10 +115,10 @@ func ExampleScan() {
 		panic(err)
 		panic(err)
 	}
 	}
 
 
-	for len(multiBulk) > 0 {
+	for len(values) > 0 {
 		var title string
 		var title string
 		rating := -1 // initialize to illegal value to detect nil.
 		rating := -1 // initialize to illegal value to detect nil.
-		multiBulk, err = redis.Scan(multiBulk, &title, &rating)
+		values, err = redis.Scan(values, &title, &rating)
 		if err != nil {
 		if err != nil {
 			panic(err)
 			panic(err)
 		}
 		}
@@ -126,3 +133,44 @@ func ExampleScan() {
 	// Earthbound 1
 	// Earthbound 1
 	// Red 5
 	// Red 5
 }
 }
+
+var scanStructTests = []struct {
+	title string
+	reply []string
+	value interface{}
+}{
+	{"basic",
+		[]string{"i", "-1234", "u", "5678", "s", "hello", "p", "world", "b", "f", "Bt", "1", "Bf", "0"},
+		&struct {
+			I  int    `redis:"i"`
+			U  uint   `redis:"u"`
+			S  string `redis:"s"`
+			P  []byte `redis:"p"`
+			B  bool   `redis:"b"`
+			Bt bool
+			Bf bool
+		}{
+			-1234, 5678, "hello", []byte("world"), false, true, false,
+		},
+	},
+}
+
+func TestScanStruct(t *testing.T) {
+	for _, tt := range scanStructTests {
+
+		var reply []interface{}
+		for _, v := range tt.reply {
+			reply = append(reply, []byte(v))
+		}
+
+		value := reflect.New(reflect.ValueOf(tt.value).Type().Elem())
+
+		if err := redis.ScanStruct(reply, value.Interface()); err != nil {
+			t.Fatalf("ScanStruct(%s) returned error %v", tt.title, err)
+		}
+
+		if !reflect.DeepEqual(value.Interface(), tt.value) {
+			t.Fatalf("ScanStruct(%s) returned %v, want %v", tt.title, value.Interface(), tt.value)
+		}
+	}
+}

+ 1 - 1
redis/script_test.go

@@ -36,7 +36,7 @@ func TestScript(t *testing.T) {
 	c := dialt(t)
 	c := dialt(t)
 	defer c.Close()
 	defer c.Close()
 
 
-	// To test fallback in Do, we make script unique by adding comment with current time.
+	// To test fall back in Do, we make script unique by adding comment with current time.
 	script := fmt.Sprintf("--%d\nreturn {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}", time.Now().UnixNano())
 	script := fmt.Sprintf("--%d\nreturn {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}", time.Now().UnixNano())
 	s := redis.NewScript(2, script)
 	s := redis.NewScript(2, script)
 	reply := []interface{}{[]byte("key1"), []byte("key2"), []byte("arg1"), []byte("arg2")}
 	reply := []interface{}{[]byte("key1"), []byte("key2"), []byte("arg1"), []byte("arg2")}