[add] extended testing for pool creation/closing. adds NewClientFromP…

…ool and Close methods. includes example

.circleci/config.yml

@@ -9,7 +9,6 @@ jobs:
       - image: circleci/golang:1.12
 
       - image: redislabs/rebloom:edge
-        command: ['--requirepass', 'SUPERSECRET', '--loadmodule', '/usr/lib/redis/modules/redisbloom.so']
         port: 6379:6379
 
     working_directory: /go/src/github.com/RedisBloom/redisbloom-go
@@ -18,8 +17,8 @@ jobs:
       - run: go get -v -t -d ./...
 
       #run tests with coverage
-      - run: go test -v -race -coverprofile=coverage.txt -covermode=atomic
-      - run: go tool cover -func=coverage.txt
+      - run: make get
+      - run: make coverage
       - run: bash <(curl -s https://codecov.io/bash) -t ${CODECOV_TOKEN}
 
 workflows:

.gitignore

@@ -5,13 +5,24 @@
 *.so
 *.dylib
 
-# Test binary, built with `go test -c`
+# Test binary, build with `go test -c`
 *.test
 
 # Output of the go coverage tool, specifically when used with LiteIDE
 *.out
+coverage.txt
 
-# Dependency directories (remove the comment below to include it)
-# vendor/
-
+.idea
 .project
+
+vendor/
+
+# OS generated files #
+######################
+.DS_Store
+.DS_Store?
+._*
+.Spotlight-V100
+.Trashes
+ehthumbs.db
+Thumbs.db

Makefile

@@ -0,0 +1,21 @@
+# Go parameters
+GOCMD=go
+GOBUILD=$(GOCMD) build
+GOINSTALL=$(GOCMD) install
+GOCLEAN=$(GOCMD) clean
+GOTEST=$(GOCMD) test
+GOGET=$(GOCMD) get
+GOMOD=$(GOCMD) mod
+
+.PHONY: all test coverage
+all: test coverage
+
+get:
+	$(GOGET) -t -v ./...
+
+test: get
+	$(GOTEST) -race -covermode=atomic ./...
+
+coverage: get test
+	$(GOTEST) -race -coverprofile=coverage.txt -covermode=atomic .
+

README.md

@@ -39,13 +39,13 @@ func main() {
 		// Connect to localhost with no password
     var client = redisbloom.NewClient("localhost:6379", "nohelp", nil)
 
-    // TS.ADD mytest item 
+    // BF.ADD mytest item 
     _, err := client.Add("mytest", "myItem")
     if err != nil {
         fmt.Println("Error:", err)
     }
 
-    exists, err = client.Exists("mytest", "myItem")
+    exists, err := client.Exists("mytest", "myItem")
     if err != nil {
         fmt.Println("Error:", err)
     }

client.go

@@ -35,6 +35,16 @@ func NewClient(addr, name string, authPass *string) *Client {
 	return ret
 }
 
+
+// NewClientFromPool creates a new Client with the given pool and client name
+func NewClientFromPool(pool *redis.Pool, name string) *Client {
+	ret := &Client{
+		Pool: pool,
+		Name: name,
+	}
+	return ret
+}
+
 // Reserve - Creates an empty Bloom Filter with a given desired error ratio and initial capacity. 
 // args:
 // key - the name of the filter

client_test.go

@@ -2,32 +2,51 @@ package redis_bloom_go
 
 import (
 	"os"
-//	"reflect"
 	"testing"
 	"time"
-
-//	"github.com/gomodule/redigo/redis"
+	"github.com/gomodule/redigo/redis"
 	"github.com/stretchr/testify/assert"
 )
 
-func createClient() *Client {
-	valueh, exists := os.LookupEnv("REDISBLOOM_TEST_HOST")
+
+func getTestConnectionDetails() (string, string) {
+	value, exists := os.LookupEnv("REDISBLOOM_TEST_HOST")
 	host := "localhost:6379"
-	if exists && valueh != "" {
-		host = valueh
+	password := ""
+	valuePassword, existsPassword := os.LookupEnv("REDISBLOOM_TEST_PASSWORD")
+	if exists && value != "" {
+		host = value
 	}
-	valuep, exists := os.LookupEnv("REDISBLOOM_TEST_PASSWORD")
-	password := "SUPERSECRET"
-	var ptr *string = nil
-	if exists {
-		password = valuep
+	if existsPassword && valuePassword != "" {
+		password = valuePassword
 	}
+	return host, password
+}
+
+func createClient() *Client {
+	host, password := getTestConnectionDetails()
+	var ptr *string = nil
 	if len(password) > 0 {
 		ptr = &password
 	}
 	return NewClient(host, "test_client", ptr)
 }
 
+
+func TestNewClientFromPool(t *testing.T) {
+	host, password := getTestConnectionDetails()
+	pool := &redis.Pool{Dial: func() (redis.Conn, error) {
+		return redis.Dial("tcp", host, redis.DialPassword(password))
+	}, MaxIdle: maxConns}
+	client1 := NewClientFromPool(pool, "bloom-client-1")
+	client2 := NewClientFromPool(pool, "bloom-client-2")
+	assert.Equal(t, client1.Pool, client2.Pool)
+	err1 := client1.Pool.Close()
+	err2 := client2.Pool.Close()
+	assert.Nil(t, err1)
+	assert.Nil(t, err2)
+}
+
 var client = createClient()
 var _ = client.FlushAll()
 

example_client_test.go

@@ -0,0 +1,26 @@
+package redis_bloom_go_test
+
+import (
+	"fmt"
+	redisbloom "github.com/RedisBloom/redisbloom-go"
+)
+
+// exemplifies the NewClient function
+func ExampleNewClient() {
+	host := "localhost:6379"
+	var client = redisbloom.NewClient(host, "nohelp", nil)
+
+	// BF.ADD mytest item
+	_, err := client.Add("mytest", "myItem")
+	if err != nil {
+		fmt.Println("Error:", err)
+	}
+
+	exists, err := client.Exists("mytest", "myItem")
+	if err != nil {
+		fmt.Println("Error:", err)
+	}
+	fmt.Println("myItem exists in mytest: ", exists)
+	// myItem exists in mytest:  true
+
+}

pool.go

@@ -1,6 +1,7 @@
 package redis_bloom_go
 
 import (
+	"fmt"
 	"math/rand"
 	"sync"
 	"time"
@@ -10,11 +11,16 @@ import (
 
 type ConnPool interface {
 	Get() redis.Conn
+	Close() error
 }
 
 type SingleHostPool struct {
 	*redis.Pool
 }
+//
+//func (s SingleHostPool) Close() {
+//	s.Pool.Close()
+//}
 
 func NewSingleHostPool(host string, authPass *string) *SingleHostPool {
 	ret := &redis.Pool{
@@ -33,6 +39,25 @@ type MultiHostPool struct {
 	authPass *string
 }
 
+
+func (p *MultiHostPool) Close() (err error) {
+	p.Lock()
+	defer p.Unlock()
+	for host, pool := range p.pools {
+		poolErr := pool.Close()
+		//preserve pool error if not nil but continue
+		if poolErr != nil {
+			if err == nil {
+				err = fmt.Errorf("Error closing pool for host %s. Got %v.", host, poolErr)
+			} else {
+				err = fmt.Errorf("%v Error closing pool for host %s. Got %v.", err, host, poolErr)
+			}
+		}
+	}
+	return
+}
+
+
 func NewMultiHostPool(hosts []string, authPass *string) *MultiHostPool {
 	return &MultiHostPool{
 		pools:    make(map[string]*redis.Pool, len(hosts)),

pool_test.go

@@ -1,6 +1,8 @@
 package redis_bloom_go
 
 import (
+	"github.com/gomodule/redigo/redis"
+	"github.com/stretchr/testify/assert"
 	"testing"
 )
 
@@ -30,3 +32,65 @@ func TestNewMultiHostPool(t *testing.T) {
 		})
 	}
 }
+
+func TestMultiHostPool_Close(t *testing.T) {
+	host, password := getTestConnectionDetails()
+	// Test a simple flow
+	if password == "" {
+		oneMulti := NewMultiHostPool([]string{host}, nil)
+		conn := oneMulti.Get()
+		assert.NotNil(t, conn)
+		err := oneMulti.Close()
+		assert.Nil(t, err)
+		err = oneMulti.Close()
+		assert.NotNil(t, conn)
+		severalMulti := NewMultiHostPool([]string{host, host}, nil)
+		connMulti := severalMulti.Get()
+		assert.NotNil(t, connMulti)
+		err = severalMulti.Close()
+		assert.Nil(t, err)
+	}
+	// Exhaustive test
+	dial := func() (redis.Conn, error) {
+		return redis.Dial("tcp", host, redis.DialPassword(password))
+	}
+	pool1 := &redis.Pool{Dial: dial, MaxIdle: maxConns}
+	pool2 := &redis.Pool{Dial: dial, MaxIdle: maxConns}
+	pool3 := &redis.Pool{Dial: dial, MaxIdle: maxConns}
+	//Close pull3 prior to enforce error
+	pool3.Close()
+	pool4 := &redis.Pool{Dial: dial, MaxIdle: maxConns}
+
+	type fields struct {
+		pools map[string]*redis.Pool
+		hosts []string
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr bool
+	}{
+		{"empty", fields{map[string]*redis.Pool{}, []string{}}, false},
+		{"normal", fields{map[string]*redis.Pool{"hostpool1": pool1}, []string{"hostpool1"}}, false},
+		{"pool3-already-close", fields{map[string]*redis.Pool{"hostpool2": pool2, "hostpool3": pool3, "hostpool4": pool4}, []string{"hostpool2", "hostpool3", "hostpool3"}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &MultiHostPool{
+				pools: tt.fields.pools,
+				hosts: tt.fields.hosts,
+			}
+			if err := p.Close(); (err != nil) != tt.wantErr {
+				t.Errorf("Close() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			// ensure all connections are really closed
+			if !tt.wantErr {
+				for _, pool := range p.pools {
+					if _, err := pool.Get().Do("PING"); err == nil {
+						t.Errorf("expected error after connection closed. Got %v", err)
+					}
+				}
+			}
+		})
+	}
+}