Support go-redis as the redis client

[nitishm]

The goal of this feature is to add support for more redis clients like go-rejson, gosexy/redis,etc., rather than have it tightly coupled with redigo, which is how it is currently written to work.

The go-rejson library relies on the redigo.Conn object, which is passed in as a dependencies to each of the exported methods, which in turn rely on the Do() method of the redigo.Conn object.

See JSONSet - https://github.com/nitishm/go-rejson/blob/master/rejson.go#L273-L279

The feature/enhancement should keep the library backwards compatible (if possible) by abstracting away the conn.Do() method, in a way that it works with all supported client libraries.

[nitishm]

@Shivam010 I am not sure what you interest level is, but this might be a good issue to collaborate on, if you are interested.

Let me know !

[nitishm]

More supported Go clients - https://redis.io/clients
Let's strive to support atleast go-redis, since it is as popular, if not more, as redigo.

[Shivam010]

@nitishm Thanks for inviting me.

This feature would make the library more generic. I am in... 👍
Do you have any plan ready?

[Shivam010]

I think we don't have to worry much... The Do() method will work fine for us, in case of go-redis/redis client.

package main

import (
	"encoding/json"
	"fmt"
	"github.com/go-redis/redis"
)

func main() {
	red := redis.NewClient(&redis.Options{Addr: "localhost:6379"})
	defer red.Close()

	type Object struct {
		Name      string `json:"name"`
		LastSeen  int64  `json:"lastSeen"`
		LoggedOut bool   `json:"loggedOut"`
	}
	obj := Object{"Leonard Cohen", 1478476800, true}

	o, err := json.Marshal(obj)
	if err != nil {
		panic("json.Marshal error: " + err.Error())
	}

	val, err := red.Do("JSON.SET", "obj", ".", o).Result()
	if err != nil {
		panic("JSON.SET error: " + err.Error())
	}
	fmt.Println("JSON.SET output:", val)

	val, err = red.Do("JSON.GET", "obj").Result()
	if err != nil {
		panic("JSON.GET error: " + err.Error())
	}
	fmt.Println("JSON.GET output:", val)
}

[nitishm]

Agreed, go-redis should be fairly simple to integrate. But what I am looking for is instead a couple of options to maintain backward compatibility for the users of the library.

Option 1 - Handle client type in a switch statement
Instead of passing in the redigo.Conn as a parameter to the exported methods (eg. JSONSet), we pass the connection object as an interface, and then switch on the type.

Something like, (I haven't tested this, so the syntax might be wrong)

func JSONSet(conn interface{}, key string, path string, obj interface{}, NX bool, XX bool) (res interface{}, err error) {
	name, args, err := CommandBuilder("JSON.SET", key, path, obj, NX, XX)
	if err != nil {
		return nil, err
	}

    // Handle the client type here, like redigo / go-redis / etc.
    switch conn.(type) {
        case c := redigo.Conn: 
            return c.Do(name, args...)
        case c := go-redis.Client:
            return c.Do(name, args...)
        default:
            return nil, fmt.Errorf("Redis client not supported!")
    }
}

It keeps it backwards compatible but then, somehow feels hacky (I might be wrong and this might be the right way to do things)

Option 2 - Haven't thought of any yet. Ideas welcome.

[Shivam010]

The idea is nice but yes, it feels hacky. I have a similar approach, it comes in my mind when I encountered the similarity in different clients because of the Do() method. And it might solve the current problem, (I guess).

We can wrap our command builder logic under client identifing layer. And only export a ReJSON interface implemented for different redis clients
Something like this:

type ReJSON interface {
	JSONSet(key, path string, obj interface{}, NX bool, XX bool) (interface{}, error)
}

type ReJSONHandler struct {
	clientName     string
	implementation ReJSON
}

func NewRejsonHandler() *ReJSONHandler {
	return &ReJSONHandler{clientName: "inactive"}
}

func (r *ReJSONHandler) SetRedigoClient(conn redigo.Conn) {
	r.clientName = "redigo"
	r.implementation = _redigo{conn}
}

func (r *ReJSONHandler) JSONSet(key, path string, obj interface{}, NX bool, XX bool) (interface{}, error) {
	if r.clientName == "inactive" {
		return nil, fmt.Errorf("no redis client is set")
	}
	return r.implementation.JSONSet(key, path, obj, NX, XX)
}

type _redigo struct {
	redigo.Conn // import redigo "github.com/gomodule/redigo/redis"
}

func (r _redigo) JSONSet(key, path string, obj interface{}, NX bool, XX bool) (interface{}, error) {
	// corresponding client specific implementation
	name, args, err := rejson.CommandBuilder("JSON.SET", key, path, obj, NX, XX)
	if err != nil {
		return nil, err
	}
	return r.Do(name, args...)
}

Similarly, we can use other clients by creating their set method and implementing their logic...
Considering these three clients for now:

type _redigo struct {
	redigo.Conn // https://github.com/gomodule/redigo
}
type _goRedis struct {
	goRedis.Client // https://github.com/go-redis/redis
}
type _radix struct {
	radix.Client // https://github.com/mediocregopher/radix
}

What do you think about it?

[nitishm]

What do you think about it?

That actually sounds really good.

I think at his point in time I will need to create a v1.0.0 release with the existing code and then the proposed work can go into v2.0.0 since it will no longer be backwards compatible. But with all that said I totally on-board with this idea.

[nitishm]

@Shivam010 Let me know if you want to get started with a PR and own this piece.

[nitishm]

Created a release for the existing library - https://github.com/nitishm/go-rejson/releases/tag/v1.0.0

[Shivam010]

@Shivam010 Let me know if you want to get started with a PR and own this piece.

Sure, it would be great fun, working on it.

[nitishm]

Fixed in #29