Skip to content

Commit aee0cc6

Browse files
stephaniehingtgenstephaniehingtgen
committed
feat: add helper func, cleanup code
1 parent eb8d0fc commit aee0cc6

File tree

5 files changed

+55
-89
lines changed

5 files changed

+55
-89
lines changed

‎cluster.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,7 @@ func ParseClusterURL(redisURL string) (*ClusterOptions, error) {
173173

174174
// add base URL to the array of addresses
175175
// more addresses may be added through the URL params
176-
h, p, err := net.SplitHostPort(u.Host)
177-
if err != nil {
178-
h = u.Host
179-
}
180-
if h == "" {
181-
h = "localhost"
182-
}
183-
if p == "" {
184-
p = "6379"
185-
}
186-
176+
h, p := getHostPortWithDefaults(u)
187177
o.Addrs = append(o.Addrs, net.JoinHostPort(h, p))
188178

189179
// setup username, password, and other configurations

‎cluster_test.go

Lines changed: 23 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"errors"
77
"fmt"
88
"net"
9-
"reflect"
109
"strconv"
1110
"strings"
1211
"sync"
@@ -15,6 +14,7 @@ import (
1514

1615
. "github.com/onsi/ginkgo"
1716
. "github.com/onsi/gomega"
17+
"github.com/stretchr/testify/assert"
1818

1919
"github.com/go-redis/redis/v8"
2020
"github.com/go-redis/redis/v8/internal/hashtag"
@@ -1300,23 +1300,23 @@ func TestParseClusterURL(t *testing.T) {
13001300
}, {
13011301
test: "ParseRedissURL",
13021302
url: "rediss://localhost:123",
1303-
o: &redis.ClusterOptions{Addrs: []string{"localhost:123"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
1303+
o: &redis.ClusterOptions{Addrs: []string{"localhost:123"}, TLSConfig: &tls.Config{ServerName: "localhost"}},
13041304
}, {
13051305
test: "MissingRedisPort",
13061306
url: "redis://localhost",
13071307
o: &redis.ClusterOptions{Addrs: []string{"localhost:6379"}},
13081308
}, {
13091309
test: "MissingRedissPort",
13101310
url: "rediss://localhost",
1311-
o: &redis.ClusterOptions{Addrs: []string{"localhost:6379"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
1311+
o: &redis.ClusterOptions{Addrs: []string{"localhost:6379"}, TLSConfig: &tls.Config{ServerName: "localhost"}},
13121312
}, {
13131313
test: "MultipleRedisURLs",
13141314
url: "redis://localhost:123?addr=localhost:1234&addr=localhost:12345",
1315-
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:12345", "localhost:1234"}},
1315+
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234", "localhost:12345"}},
13161316
}, {
13171317
test: "MultipleRedissURLs",
13181318
url: "rediss://localhost:123?addr=localhost:1234&addr=localhost:12345",
1319-
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:12345", "localhost:1234"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
1319+
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234", "localhost:12345"}, TLSConfig: &tls.Config{ServerName: "localhost"}},
13201320
}, {
13211321
test: "OnlyPassword",
13221322
url: "redis://:bar@localhost:123",
@@ -1332,7 +1332,7 @@ func TestParseClusterURL(t *testing.T) {
13321332
}, {
13331333
test: "RedissUsernamePassword",
13341334
url: "rediss://foo:bar@localhost:123?addr=localhost:1234",
1335-
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, Username: "foo", Password: "bar", TLSConfig: &tls.Config{ /* no deep comparison */ }},
1335+
o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, Username: "foo", Password: "bar", TLSConfig: &tls.Config{ServerName: "localhost"}},
13361336
}, {
13371337
test: "QueryParameters",
13381338
url: "redis://localhost:123?read_timeout=2&pool_fifo=true&addr=localhost:1234",
@@ -1403,59 +1403,21 @@ func TestParseClusterURL(t *testing.T) {
14031403

14041404
func comprareOptions(t *testing.T, actual, expected *redis.ClusterOptions) {
14051405
t.Helper()
1406-
1407-
if !reflect.DeepEqual(actual.Addrs, expected.Addrs) {
1408-
t.Errorf("got %q, want %q", actual.Addrs, expected.Addrs)
1409-
}
1410-
if actual.TLSConfig == nil && expected.TLSConfig != nil {
1411-
t.Errorf("got nil TLSConfig, expected a TLSConfig")
1412-
}
1413-
if actual.TLSConfig != nil && expected.TLSConfig == nil {
1414-
t.Errorf("got TLSConfig, expected no TLSConfig")
1415-
}
1416-
if actual.Username != expected.Username {
1417-
t.Errorf("Username: got %q, expected %q", actual.Username, expected.Username)
1418-
}
1419-
if actual.Password != expected.Password {
1420-
t.Errorf("Password: got %q, expected %q", actual.Password, expected.Password)
1421-
}
1422-
if actual.MaxRetries != expected.MaxRetries {
1423-
t.Errorf("MaxRetries: got %v, expected %v", actual.MaxRetries, expected.MaxRetries)
1424-
}
1425-
if actual.MinRetryBackoff != expected.MinRetryBackoff {
1426-
t.Errorf("MinRetryBackoff: got %v, expected %v", actual.MinRetryBackoff, expected.MinRetryBackoff)
1427-
}
1428-
if actual.MaxRetryBackoff != expected.MaxRetryBackoff {
1429-
t.Errorf("MaxRetryBackoff: got %v, expected %v", actual.MaxRetryBackoff, expected.MaxRetryBackoff)
1430-
}
1431-
if actual.DialTimeout != expected.DialTimeout {
1432-
t.Errorf("DialTimeout: got %v, expected %v", actual.DialTimeout, expected.DialTimeout)
1433-
}
1434-
if actual.ReadTimeout != expected.ReadTimeout {
1435-
t.Errorf("ReadTimeout: got %v, expected %v", actual.ReadTimeout, expected.ReadTimeout)
1436-
}
1437-
if actual.WriteTimeout != expected.WriteTimeout {
1438-
t.Errorf("WriteTimeout: got %v, expected %v", actual.WriteTimeout, expected.WriteTimeout)
1439-
}
1440-
if actual.PoolFIFO != expected.PoolFIFO {
1441-
t.Errorf("PoolFIFO: got %v, expected %v", actual.PoolFIFO, expected.PoolFIFO)
1442-
}
1443-
if actual.PoolSize != expected.PoolSize {
1444-
t.Errorf("PoolSize: got %v, expected %v", actual.PoolSize, expected.PoolSize)
1445-
}
1446-
if actual.MinIdleConns != expected.MinIdleConns {
1447-
t.Errorf("MinIdleConns: got %v, expected %v", actual.MinIdleConns, expected.MinIdleConns)
1448-
}
1449-
if actual.MaxConnAge != expected.MaxConnAge {
1450-
t.Errorf("MaxConnAge: got %v, expected %v", actual.MaxConnAge, expected.MaxConnAge)
1451-
}
1452-
if actual.PoolTimeout != expected.PoolTimeout {
1453-
t.Errorf("PoolTimeout: got %v, expected %v", actual.PoolTimeout, expected.PoolTimeout)
1454-
}
1455-
if actual.IdleTimeout != expected.IdleTimeout {
1456-
t.Errorf("IdleTimeout: got %v, expected %v", actual.IdleTimeout, expected.IdleTimeout)
1457-
}
1458-
if actual.IdleCheckFrequency != expected.IdleCheckFrequency {
1459-
t.Errorf("IdleCheckFrequency: got %v, expected %v", actual.IdleCheckFrequency, expected.IdleCheckFrequency)
1460-
}
1406+
assert.Equal(t, expected.Addrs, actual.Addrs)
1407+
assert.Equal(t, expected.TLSConfig, actual.TLSConfig)
1408+
assert.Equal(t, expected.Username, actual.Username)
1409+
assert.Equal(t, expected.Password, actual.Password)
1410+
assert.Equal(t, expected.MaxRetries, actual.MaxRetries)
1411+
assert.Equal(t, expected.MinRetryBackoff, actual.MinRetryBackoff)
1412+
assert.Equal(t, expected.MaxRetryBackoff, actual.MaxRetryBackoff)
1413+
assert.Equal(t, expected.DialTimeout, actual.DialTimeout)
1414+
assert.Equal(t, expected.ReadTimeout, actual.ReadTimeout)
1415+
assert.Equal(t, expected.WriteTimeout, actual.WriteTimeout)
1416+
assert.Equal(t, expected.PoolFIFO, actual.PoolFIFO)
1417+
assert.Equal(t, expected.PoolSize, actual.PoolSize)
1418+
assert.Equal(t, expected.MinIdleConns, actual.MinIdleConns)
1419+
assert.Equal(t, expected.MaxConnAge, actual.MaxConnAge)
1420+
assert.Equal(t, expected.PoolTimeout, actual.PoolTimeout)
1421+
assert.Equal(t, expected.IdleTimeout, actual.IdleTimeout)
1422+
assert.Equal(t, expected.IdleCheckFrequency, actual.IdleCheckFrequency)
14611423
}

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ require (
88
github.com/google/go-cmp v0.5.6 // indirect
99
github.com/onsi/ginkgo v1.16.4
1010
github.com/onsi/gomega v1.16.0
11+
github.com/stretchr/testify v1.5.1
1112
)

‎go.sum

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
github.com/cespare/xxhash/v2 v2.1.2 h1:YRXhKfTDauu4ajMg1TPgFO5jnlC2HCbmLXMcTG5cbYE=
22
github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
33
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
4+
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
45
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
56
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
67
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
@@ -36,8 +37,10 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
3637
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
3738
github.com/onsi/gomega v1.16.0 h1:6gjqkI8iiRHMvdccRJM8rVKjCWk6ZIm6FTm3ddIe4/c=
3839
github.com/onsi/gomega v1.16.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY=
40+
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
3941
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
4042
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
43+
github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
4144
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
4245
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
4346
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=

‎options.go

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -240,16 +240,7 @@ func setupTCPConn(u *url.URL) (*Options, error) {
240240

241241
o.Username, o.Password = getUserPassword(u)
242242

243-
h, p, err := net.SplitHostPort(u.Host)
244-
if err != nil {
245-
h = u.Host
246-
}
247-
if h == "" {
248-
h = "localhost"
249-
}
250-
if p == "" {
251-
p = "6379"
252-
}
243+
h, p := getHostPortWithDefaults(u)
253244
o.Addr = net.JoinHostPort(h, p)
254245

255246
f := strings.FieldsFunc(u.Path, func(r rune) bool {
@@ -259,6 +250,7 @@ func setupTCPConn(u *url.URL) (*Options, error) {
259250
case 0:
260251
o.DB = 0
261252
case 1:
253+
var err error
262254
if o.DB, err = strconv.Atoi(f[0]); err != nil {
263255
return nil, fmt.Errorf("redis: invalid database number: %q", f[0])
264256
}
@@ -273,6 +265,23 @@ func setupTCPConn(u *url.URL) (*Options, error) {
273265
return setupConnParams(u, o)
274266
}
275267

268+
// getHostPortWithDefaults is a helper function that splits the url into
269+
// a host and a port. If the host is missing, it defaults to localhost
270+
// and if the port is missing, it defaults to 6379
271+
func getHostPortWithDefaults(u *url.URL) (string, string) {
272+
host, port, err := net.SplitHostPort(u.Host)
273+
if err != nil {
274+
host = u.Host
275+
}
276+
if host == "" {
277+
host = "localhost"
278+
}
279+
if port == "" {
280+
port = "6379"
281+
}
282+
return host, port
283+
}
284+
276285
func setupUnixConn(u *url.URL) (*Options, error) {
277286
o := &Options{
278287
Network: "unix",
@@ -292,19 +301,20 @@ type queryOptions struct {
292301
}
293302

294303
func (o *queryOptions) string(name string) string {
295-
vs := o.q[name]
296-
if len(vs) == 0 {
304+
if len(o.q[name]) == 0 {
297305
return ""
298306
}
307+
// get the first item from the array to return
308+
// and remove it so it isn't processed again
309+
param := o.q[name][0]
310+
o.q[name] = o.q[name][1:]
299311

300-
// enable detection of unknown parameters
301-
if len(vs) > 1 {
302-
o.q[name] = o.q[name][:len(vs)-1]
303-
} else {
312+
// remove the key to enable detection of unknown params
313+
if len(o.q[name]) == 0 {
304314
delete(o.q, name)
305315
}
306316

307-
return vs[len(vs)-1]
317+
return param
308318
}
309319

310320
func (o *queryOptions) int(name string) int {

0 commit comments

Comments
 (0)