Merge pull request #1884 from dmke/feat/parseurl

Allow query parameters for ParseURL

example_test.go

@@ -39,13 +39,14 @@ func ExampleNewClient() {
 }
 
 func ExampleParseURL() {
-	opt, err := redis.ParseURL("redis://:qwerty@localhost:6379/1")
+	opt, err := redis.ParseURL("redis://:qwerty@localhost:6379/1?dial_timeout=5s")
 	if err != nil {
 		panic(err)
 	}
 	fmt.Println("addr is", opt.Addr)
 	fmt.Println("db is", opt.DB)
 	fmt.Println("password is", opt.Password)
+	fmt.Println("dial timeout is", opt.DialTimeout)
 
 	// Create client as usually.
 	_ = redis.NewClient(opt)
@@ -53,6 +54,7 @@ func ExampleParseURL() {
 	// Output: addr is localhost:6379
 	// db is 1
 	// password is qwerty
+	// dial timeout is 5s
 }
 
 func ExampleNewFailoverClient() {

options.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/url"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -192,9 +193,32 @@ func (opt *Options) clone() *Options {
 // Scheme is required.
 // There are two connection types: by tcp socket and by unix socket.
 // Tcp connection:
-// 		redis://<user>:<password>@<host>:<port>/<db_number>
+//		redis://<user>:<password>@<host>:<port>/<db_number>
 // Unix connection:
 //		unix://<user>:<password>@</path/to/redis.sock>?db=<db_number>
+// Most Option fields can be set using query parameters, with the following restrictions:
+//	- field names are mapped using snake-case conversion: to set MaxRetries, use max_retries
+//	- only scalar type fields are supported (bool, int, time.Duration)
+//	- for time.Duration fields, values must be a valid input for time.ParseDuration();
+//	  additionally a plain integer as value (i.e. without unit) is intepreted as seconds
+//	- to disable a duration field, use value less than or equal to 0; to use the default
+//	  value, leave the value blank or remove the parameter
+//	- only the last value is interpreted if a parameter is given multiple times
+//	- fields "network", "addr", "username" and "password" can only be set using other
+//	  URL attributes (scheme, host, userinfo, resp.), query paremeters using these
+//	  names will be treated as unknown parameters
+//	- unknown parameter names will result in an error
+// Examples:
+//		redis://user:password@localhost:6789/3?dial_timeout=3&db=1&read_timeout=6s&max_retries=2
+//		is equivalent to:
+//		&Options{
+//			Network:     "tcp",
+//			Addr:        "localhost:6789",
+//			DB:          1,               // path "/3" was overridden by "&db=1"
+//			DialTimeout: 3 * time.Second, // no time unit = seconds
+//			ReadTimeout: 6 * time.Second,
+//			MaxRetries:  2,
+//		}
 func ParseURL(redisURL string) (*Options, error) {
 	u, err := url.Parse(redisURL)
 	if err != nil {
@@ -216,10 +240,6 @@ func setupTCPConn(u *url.URL) (*Options, error) {
 
 	o.Username, o.Password = getUserPassword(u)
 
-	if len(u.Query()) > 0 {
-		return nil, errors.New("redis: no options supported")
-	}
-
 	h, p, err := net.SplitHostPort(u.Host)
 	if err != nil {
 		h = u.Host
@@ -250,7 +270,7 @@ func setupTCPConn(u *url.URL) (*Options, error) {
 		o.TLSConfig = &tls.Config{ServerName: h}
 	}
 
-	return o, nil
+	return setupConnParams(u, o)
 }
 
 func setupUnixConn(u *url.URL) (*Options, error) {
@@ -262,19 +282,122 @@ func setupUnixConn(u *url.URL) (*Options, error) {
 		return nil, errors.New("redis: empty unix socket path")
 	}
 	o.Addr = u.Path
-
 	o.Username, o.Password = getUserPassword(u)
+	return setupConnParams(u, o)
+}
 
-	dbStr := u.Query().Get("db")
+type queryOptions struct {
-	if dbStr == "" {
+	q   url.Values
-		return o, nil // if database is not set, connect to 0 db.
+	err error
+}
+
+func (o *queryOptions) string(name string) string {
+	vs := o.q[name]
+	if len(vs) == 0 {
+		return ""
+	}
+	delete(o.q, name) // enable detection of unknown parameters
+	return vs[len(vs)-1]
+}
+
+func (o *queryOptions) int(name string) int {
+	s := o.string(name)
+	if s == "" {
+		return 0
+	}
+	i, err := strconv.Atoi(s)
+	if err == nil {
+		return i
+	}
+	if o.err == nil {
+		o.err = fmt.Errorf("redis: invalid %s number: %s", name, err)
+	}
+	return 0
+}
+
+func (o *queryOptions) duration(name string) time.Duration {
+	s := o.string(name)
+	if s == "" {
+		return 0
+	}
+	// try plain number first
+	if i, err := strconv.Atoi(s); err == nil {
+		if i <= 0 {
+			// disable timeouts
+			return -1
+		}
+		return time.Duration(i) * time.Second
+	}
+	dur, err := time.ParseDuration(s)
+	if err == nil {
+		return dur
+	}
+	if o.err == nil {
+		o.err = fmt.Errorf("redis: invalid %s duration: %w", name, err)
+	}
+	return 0
+}
+
+func (o *queryOptions) bool(name string) bool {
+	switch s := o.string(name); s {
+	case "true", "1":
+		return true
+	case "false", "0", "":
+		return false
+	default:
+		if o.err == nil {
+			o.err = fmt.Errorf("redis: invalid %s boolean: expected true/false/1/0 or an empty string, got %q", name, s)
+		}
+		return false
+	}
+}
+
+func (o *queryOptions) remaining() []string {
+	if len(o.q) == 0 {
+		return nil
+	}
+	keys := make([]string, 0, len(o.q))
+	for k := range o.q {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
+// setupConnParams converts query parameters in u to option value in o.
+func setupConnParams(u *url.URL, o *Options) (*Options, error) {
+	q := queryOptions{q: u.Query()}
+
+	// compat: a future major release may use q.int("db")
+	if tmp := q.string("db"); tmp != "" {
+		db, err := strconv.Atoi(tmp)
+		if err != nil {
+			return nil, fmt.Errorf("redis: invalid database number: %w", err)
+		}
+		o.DB = db
 	}
 
-	db, err := strconv.Atoi(dbStr)
+	o.MaxRetries = q.int("max_retries")
-	if err != nil {
+	o.MinRetryBackoff = q.duration("min_retry_backoff")
-		return nil, fmt.Errorf("redis: invalid database number: %w", err)
+	o.MaxRetryBackoff = q.duration("max_retry_backoff")
+	o.DialTimeout = q.duration("dial_timeout")
+	o.ReadTimeout = q.duration("read_timeout")
+	o.WriteTimeout = q.duration("write_timeout")
+	o.PoolFIFO = q.bool("pool_fifo")
+	o.PoolSize = q.int("pool_size")
+	o.MinIdleConns = q.int("min_idle_conns")
+	o.MaxConnAge = q.duration("max_conn_age")
+	o.PoolTimeout = q.duration("pool_timeout")
+	o.IdleTimeout = q.duration("idle_timeout")
+	o.IdleCheckFrequency = q.duration("idle_check_frequency")
+	if q.err != nil {
+		return nil, q.err
+	}
+
+	// any parameters left?
+	if r := q.remaining(); len(r) > 0 {
+		return nil, fmt.Errorf("redis: unexpected option: %s", strings.Join(r, ", "))
 	}
-	o.DB = db
 
 	return o, nil
 }

options_test.go

@@ -4,6 +4,7 @@
 package redis
 
 import (
+	"crypto/tls"
 	"errors"
 	"testing"
 	"time"
@@ -11,150 +12,185 @@ import (
 
 func TestParseURL(t *testing.T) {
 	cases := []struct {
-		u    string
+		url string
-		addr string
+		o   *Options // expected value
-		db   int
+		err error
-		tls  bool
-		err  error
-		user string
-		pass string
 	}{
 		{
-			"redis://localhost:123/1",
+			url: "redis://localhost:123/1",
-			"localhost:123",
+			o:   &Options{Addr: "localhost:123", DB: 1},
-			1, false, nil,
+		}, {
-			"", "",
+			url: "redis://localhost:123",
-		},
+			o:   &Options{Addr: "localhost:123"},
-		{
+		}, {
-			"redis://localhost:123",
+			url: "redis://localhost/1",
-			"localhost:123",
+			o:   &Options{Addr: "localhost:6379", DB: 1},
-			0, false, nil,
+		}, {
-			"", "",
+			url: "redis://12345",
-		},
+			o:   &Options{Addr: "12345:6379"},
-		{
+		}, {
-			"redis://localhost/1",
+			url: "rediss://localhost:123",
-			"localhost:6379",
+			o:   &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ /* no deep comparison */ }},
-			1, false, nil,
+		}, {
-			"", "",
+			url: "redis://:bar@localhost:123",
-		},
+			o:   &Options{Addr: "localhost:123", Password: "bar"},
-		{
+		}, {
-			"redis://12345",
+			url: "redis://foo@localhost:123",
-			"12345:6379",
+			o:   &Options{Addr: "localhost:123", Username: "foo"},
-			0, false, nil,
+		}, {
-			"", "",
+			url: "redis://foo:bar@localhost:123",
-		},
+			o:   &Options{Addr: "localhost:123", Username: "foo", Password: "bar"},
-		{
+		}, {
-			"rediss://localhost:123",
+			// multiple params
-			"localhost:123",
+			url: "redis://localhost:123/?db=2&read_timeout=2&pool_fifo=true",
-			0, true, nil,
+			o:   &Options{Addr: "localhost:123", DB: 2, ReadTimeout: 2 * time.Second, PoolFIFO: true},
-			"", "",
+		}, {
-		},
+			// special case handling for disabled timeouts
-		{
+			url: "redis://localhost:123/?db=2&idle_timeout=0",
-			"redis://:bar@localhost:123",
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: -1},
-			"localhost:123",
+		}, {
-			0, false, nil,
+			// negative values disable timeouts as well
-			"", "bar",
+			url: "redis://localhost:123/?db=2&idle_timeout=-1",
-		},
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: -1},
-		{
+		}, {
-			"redis://foo@localhost:123",
+			// absent timeout values will use defaults
-			"localhost:123",
+			url: "redis://localhost:123/?db=2&idle_timeout=",
-			0, false, nil,
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: 0},
-			"foo", "",
+		}, {
-		},
+			url: "redis://localhost:123/?db=2&idle_timeout", // missing "=" at the end
-		{
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: 0},
-			"redis://foo:bar@localhost:123",
+		}, {
-			"localhost:123",
+			url: "unix:///tmp/redis.sock",
-			0, false, nil,
+			o:   &Options{Addr: "/tmp/redis.sock"},
-			"foo", "bar",
+		}, {
-		},
+			url: "unix://foo:bar@/tmp/redis.sock",
-		{
+			o:   &Options{Addr: "/tmp/redis.sock", Username: "foo", Password: "bar"},
-			"unix:///tmp/redis.sock",
+		}, {
-			"/tmp/redis.sock",
+			url: "unix://foo:bar@/tmp/redis.sock?db=3",
-			0, false, nil,
+			o:   &Options{Addr: "/tmp/redis.sock", Username: "foo", Password: "bar", DB: 3},
-			"", "",
+		}, {
-		},
+			// invalid db format
-		{
+			url: "unix://foo:bar@/tmp/redis.sock?db=test",
-			"unix://foo:bar@/tmp/redis.sock",
+			err: errors.New(`redis: invalid database number: strconv.Atoi: parsing "test": invalid syntax`),
-			"/tmp/redis.sock",
+		}, {
-			0, false, nil,
+			// invalid int value
-			"foo", "bar",
+			url: "redis://localhost/?pool_size=five",
-		},
+			err: errors.New(`redis: invalid pool_size number: strconv.Atoi: parsing "five": invalid syntax`),
-		{
+		}, {
-			"unix://foo:bar@/tmp/redis.sock?db=3",
+			// invalid bool value
-			"/tmp/redis.sock",
+			url: "redis://localhost/?pool_fifo=yes",
-			3, false, nil,
+			err: errors.New(`redis: invalid pool_fifo boolean: expected true/false/1/0 or an empty string, got "yes"`),
-			"foo", "bar",
+		}, {
-		},
+			// it returns first error
-		{
+			url: "redis://localhost/?db=foo&pool_size=five",
-			"unix://foo:bar@/tmp/redis.sock?db=test",
+			err: errors.New(`redis: invalid database number: strconv.Atoi: parsing "foo": invalid syntax`),
-			"/tmp/redis.sock",
+		}, {
-			0, false, errors.New("redis: invalid database number: strconv.Atoi: parsing \"test\": invalid syntax"),
+			url: "redis://localhost/?abc=123",
-			"", "",
+			err: errors.New("redis: unexpected option: abc"),
-		},
+		}, {
-		{
+			url: "redis://foo@localhost/?username=bar",
-			"redis://localhost/?abc=123",
+			err: errors.New("redis: unexpected option: username"),
-			"",
+		}, {
-			0, false, errors.New("redis: no options supported"),
+			url: "redis://localhost/?wrte_timout=10s&abc=123",
-			"", "",
+			err: errors.New("redis: unexpected option: abc, wrte_timout"),
-		},
+		}, {
-		{
+			url: "http://google.com",
-			"http://google.com",
+			err: errors.New("redis: invalid URL scheme: http"),
-			"",
+		}, {
-			0, false, errors.New("redis: invalid URL scheme: http"),
+			url: "redis://localhost/1/2/3/4",
-			"", "",
+			err: errors.New("redis: invalid URL path: /1/2/3/4"),
-		},
+		}, {
-		{
+			url: "12345",
-			"redis://localhost/1/2/3/4",
+			err: errors.New("redis: invalid URL scheme: "),
-			"",
+		}, {
-			0, false, errors.New("redis: invalid URL path: /1/2/3/4"),
+			url: "redis://localhost/iamadatabase",
-			"", "",
+			err: errors.New(`redis: invalid database number: "iamadatabase"`),
-		},
-		{
-			"12345",
-			"",
-			0, false, errors.New("redis: invalid URL scheme: "),
-			"", "",
-		},
-		{
-			"redis://localhost/iamadatabase",
-			"",
-			0, false, errors.New(`redis: invalid database number: "iamadatabase"`),
-			"", "",
 		},
 	}
 
-	for _, c := range cases {
+	for i := range cases {
-		t.Run(c.u, func(t *testing.T) {
+		tc := cases[i]
-			o, err := ParseURL(c.u)
+		t.Run(tc.url, func(t *testing.T) {
-			if c.err == nil && err != nil {
+			t.Parallel()
+
+			actual, err := ParseURL(tc.url)
+			if tc.err == nil && err != nil {
 				t.Fatalf("unexpected error: %q", err)
 				return
 			}
-			if c.err != nil && err != nil {
+			if tc.err != nil && err != nil {
-				if c.err.Error() != err.Error() {
+				if tc.err.Error() != err.Error() {
-					t.Fatalf("got %q, expected %q", err, c.err)
+					t.Fatalf("got %q, expected %q", err, tc.err)
 				}
 				return
 			}
-			if o.Addr != c.addr {
+			comprareOptions(t, actual, tc.o)
-				t.Errorf("got %q, want %q", o.Addr, c.addr)
-			}
-			if o.DB != c.db {
-				t.Errorf("got %q, expected %q", o.DB, c.db)
-			}
-			if c.tls && o.TLSConfig == nil {
-				t.Errorf("got nil TLSConfig, expected a TLSConfig")
-			}
-			if o.Username != c.user {
-				t.Errorf("got %q, expected %q", o.Username, c.user)
-			}
-			if o.Password != c.pass {
-				t.Errorf("got %q, expected %q", o.Password, c.pass)
-			}
 		})
 	}
 }
 
+func comprareOptions(t *testing.T, actual, expected *Options) {
+	t.Helper()
+
+	if actual.Addr != expected.Addr {
+		t.Errorf("got %q, want %q", actual.Addr, expected.Addr)
+	}
+	if actual.DB != expected.DB {
+		t.Errorf("DB: got %q, expected %q", actual.DB, expected.DB)
+	}
+	if actual.TLSConfig == nil && expected.TLSConfig != nil {
+		t.Errorf("got nil TLSConfig, expected a TLSConfig")
+	}
+	if actual.TLSConfig != nil && expected.TLSConfig == nil {
+		t.Errorf("got TLSConfig, expected no TLSConfig")
+	}
+	if actual.Username != expected.Username {
+		t.Errorf("Username: got %q, expected %q", actual.Username, expected.Username)
+	}
+	if actual.Password != expected.Password {
+		t.Errorf("Password: got %q, expected %q", actual.Password, expected.Password)
+	}
+	if actual.MaxRetries != expected.MaxRetries {
+		t.Errorf("MaxRetries: got %v, expected %v", actual.MaxRetries, expected.MaxRetries)
+	}
+	if actual.MinRetryBackoff != expected.MinRetryBackoff {
+		t.Errorf("MinRetryBackoff: got %v, expected %v", actual.MinRetryBackoff, expected.MinRetryBackoff)
+	}
+	if actual.MaxRetryBackoff != expected.MaxRetryBackoff {
+		t.Errorf("MaxRetryBackoff: got %v, expected %v", actual.MaxRetryBackoff, expected.MaxRetryBackoff)
+	}
+	if actual.DialTimeout != expected.DialTimeout {
+		t.Errorf("DialTimeout: got %v, expected %v", actual.DialTimeout, expected.DialTimeout)
+	}
+	if actual.ReadTimeout != expected.ReadTimeout {
+		t.Errorf("ReadTimeout: got %v, expected %v", actual.ReadTimeout, expected.ReadTimeout)
+	}
+	if actual.WriteTimeout != expected.WriteTimeout {
+		t.Errorf("WriteTimeout: got %v, expected %v", actual.WriteTimeout, expected.WriteTimeout)
+	}
+	if actual.PoolFIFO != expected.PoolFIFO {
+		t.Errorf("PoolFIFO: got %v, expected %v", actual.PoolFIFO, expected.PoolFIFO)
+	}
+	if actual.PoolSize != expected.PoolSize {
+		t.Errorf("PoolSize: got %v, expected %v", actual.PoolSize, expected.PoolSize)
+	}
+	if actual.MinIdleConns != expected.MinIdleConns {
+		t.Errorf("MinIdleConns: got %v, expected %v", actual.MinIdleConns, expected.MinIdleConns)
+	}
+	if actual.MaxConnAge != expected.MaxConnAge {
+		t.Errorf("MaxConnAge: got %v, expected %v", actual.MaxConnAge, expected.MaxConnAge)
+	}
+	if actual.PoolTimeout != expected.PoolTimeout {
+		t.Errorf("PoolTimeout: got %v, expected %v", actual.PoolTimeout, expected.PoolTimeout)
+	}
+	if actual.IdleTimeout != expected.IdleTimeout {
+		t.Errorf("IdleTimeout: got %v, expected %v", actual.IdleTimeout, expected.IdleTimeout)
+	}
+	if actual.IdleCheckFrequency != expected.IdleCheckFrequency {
+		t.Errorf("IdleCheckFrequency: got %v, expected %v", actual.IdleCheckFrequency, expected.IdleCheckFrequency)
+	}
+}
+
 // Test ReadTimeout option initialization, including special values -1 and 0.
 // And also test behaviour of WriteTimeout option, when it is not explicitly set and use
 // ReadTimeout value.