Redis: PR propsal: Scanning results and maps to struct

[knadh]

Hi @vmihailenco,

Here's a simple solution that scans the results of type []interface{} from NewSliceCmd and NewStringSliceCmd to a struct using reflection. The comment on the function shows example usage. Useful for scanning the results of HMGET, HGETALL etc. directly into a struct.

What do you think of merging this feature into the lib in one of the following ways? If yes, I can send a PR.

• As goredis.ScanToStruct(vals, target)
• As utils.ScanToStruct(vals, target)
• As SliceCmd.ScanToStruct(target)

Related: https://github.com/go-redis/redis/issues/672 https://github.com/go-redis/redis/issues/348 https://github.com/go-redis/redis/issues/970 https://github.com/go-redis/redis/issues/16

[vmihailenco]

Hi @knadh,

Two obvious things that are missing are tests and docs/examples. I won't insist much on docs, but tests are mandatory.

And I guess code requires more work too and I don't have much time to spend on this. I would say that this issue requires 10-20 hours of work to finish. Do you have that time? :)

[knadh]

@vmihailenco the gist is just a proof of concept. Of course, it'll need tests + examples before it can go into the lib.

And I guess code requires more work too

Could you elaborate on what exactly?

[vmihailenco]

Could you elaborate on what exactly?

Take this only as suggestions since I have not spend much time on this but I have some experience with https://github.com/vmihailenco/msgpack.

I think it makes sense to start with with decoder func. So for each type you have a func that scans a string into a value:

type decoderFunc func(v reflect.Value, s string) error

var valueDecoders = []decoderFunc{
    reflect.Bool:        decodeBoolValue,
    reflect.Int:           decodeInt64Value,
    ..
}

func decodeBoolValue(v reflect.Value, s string) error {
    b, err := strconv.ParseBool(val)
    if err != nil {
        return fmt.Errorf("expected boolean")
    }
    f.SetBool(b)
    return nil
}

Then for each struct you can construct a map of decoders once and reuse it:

type field struct {
    name     string // field name
    decoder decoderFunc
}

type structInfo struct {
    typ reflect.Type // struct type
    m   map[string]*field
}

func (s *structInfo) Decode(v reflect.Value, key, value string) error {
    return s.m[key].decoder(v, value)
}

Then your function will look like this:

func Scan(vals interface{}, dst interface{}) error {
    v := reflect.ValueOf(v)
    s := descStruct(v.Type())

    switch vals := vals.(type) {
        case []string:
            for i := 0; i < len(vals); i += 2 {
                 if err := s.Decode(v, vals[i], vals[i+1]); err != nil {
                     return err
                 }
            }
            return nil
        case []interface{}:
            // todo
    }
}

This way:

• you can easily add more decoders
• construct decoders map once for performance
• can support []interface{} and []string

Does that make sense?

[knadh]

Yup, that makes sense. I will try and find some time to prototype this.

[knadh]

@vmihailenco, here's a prototype with a working example.

• A field map of decoders are created once for a struct and is cached and reused.
• The custom decoder approach is simple like what encoding/json does with UnmarshalJSON(). You can define a Decode() on any type, not just structs. eg:

type Tag string
func (t *Tag) Decode(s string) error {
    *t = "<<" + Tag(s) + ">>"
    return nil
}

type CSV struct {
    Vals []string
}
func (c *CSV) Decode(s string) error {
    c.Vals = strings.Split(s, ",")
    return nil
}

type Record struct {
    Name   string `redis:"name"`
    Number int    `redis:"number"`
    Tag    Tag    `redis:"tag"`
    Items  CSV    `redis:"items"`
    Empty  bool   `redis:"-"`
}

var r Record
Scan(results, &r, "redis")

• Is it necessary to support both []interface{} and []string as all slice/map commands seem to produce []interface{} as the result? Won't []interface{} do?

If you think the approach makes sense, I can add tests and send a PR. Thanks.

[vmihailenco]

:+1:

@knadh looks good - please send a PR (before writing tests). I have few comments but it is easier to make them on a PR.

Is it necessary to support both []interface{} and []string as all slice/map commands seem to produce []interface{} as the result? Won't []interface{} do?

No need to support []string then.

[knadh]

Sent a PR: https://github.com/go-redis/redis/pull/1631

[knadh]

https://github.com/go-redis/redis/pull/1631 is merged! Thanks @vmihailenco.