Refactor building connection string from options

pkg/cli/cli.go

@@ -38,7 +38,7 @@ func initClientUsingBookmark(bookmarkPath, bookmarkName string) (*client.Client,
 	if opt.Url != "" { // if the bookmark has url set, use it
 		connStr = opt.Url
 	} else {
-		connStr, err = connection.BuildString(opt)
+		connStr, err = connection.BuildStringFromOptions(opt)
 		if err != nil {
 			return nil, fmt.Errorf("error building connection string: %v", err)
 		}

pkg/client/client.go

@@ -55,7 +55,7 @@ func getSchemaAndTable(str string) (string, string) {
 }
 
 func New() (*Client, error) {
-	str, err := connection.BuildString(command.Opts)
+	str, err := connection.BuildStringFromOptions(command.Opts)
 
 	if command.Opts.Debug && str != "" {
 		fmt.Println("Creating a new client for:", str)

pkg/connection/connection_string.go

@@ -86,7 +86,9 @@ func IsBlank(opts command.Options) bool {
 	return opts.Host == "" && opts.User == "" && opts.DbName == "" && opts.Url == ""
 }
 
-func BuildString(opts command.Options) (string, error) {
+// Build a new database connection string for the CLI options
+func BuildStringFromOptions(opts command.Options) (string, error) {
+	// If connection string is provided we just use that
 	if opts.Url != "" {
 		return FormatUrl(opts)
 	}
@@ -100,31 +102,22 @@ func BuildString(opts command.Options) (string, error) {
 	}
 
 	// Disable ssl for localhost connections, most users have it disabled
-	if opts.Host == "localhost" || opts.Host == "127.0.0.1" {
+	if opts.Ssl == "" && (opts.Host == "localhost" || opts.Host == "127.0.0.1") {
-		if opts.Ssl == "" {
 		opts.Ssl = "disable"
 	}
-	}
-
-	url := "postgres://"
-
-	if opts.User != "" {
-		url += opts.User
-	}
-
-	if opts.Pass != "" {
-		url += fmt.Sprintf(":%s", neturl.QueryEscape(opts.Pass))
-	}
-
-	url += fmt.Sprintf("@%s:%d", opts.Host, opts.Port)
-
-	if opts.DbName != "" {
-		url += fmt.Sprintf("/%s", opts.DbName)
-	}
 
+	query := neturl.Values{}
 	if opts.Ssl != "" {
-		url += fmt.Sprintf("?sslmode=%s", opts.Ssl)
+		query.Add("sslmode", opts.Ssl)
 	}
 
-	return url, nil
+	url := neturl.URL{
+		Scheme:   "postgres",
+		Host:     fmt.Sprintf("%v:%v", opts.Host, opts.Port),
+		User:     neturl.UserPassword(opts.User, opts.Pass),
+		Path:     fmt.Sprintf("/%s", opts.DbName),
+		RawQuery: query.Encode(),
+	}
+
+	return url.String(), nil
 }

pkg/connection/connection_string_test.go

@@ -19,7 +19,7 @@ func Test_Invalid_Url(t *testing.T) {
 
 	for _, val := range examples {
 		opts.Url = val
-		str, err := BuildString(opts)
+		str, err := BuildStringFromOptions(opts)
 
 		assert.Equal(t, "", str)
 		assert.Error(t, err)
@@ -29,14 +29,14 @@ func Test_Invalid_Url(t *testing.T) {
 
 func Test_Valid_Url(t *testing.T) {
 	url := "postgres://myhost/database"
-	str, err := BuildString(command.Options{Url: url})
+	str, err := BuildStringFromOptions(command.Options{Url: url})
 
 	assert.Equal(t, nil, err)
 	assert.Equal(t, url, str)
 }
 
 func Test_Url_And_Ssl_Flag(t *testing.T) {
-	str, err := BuildString(command.Options{
+	str, err := BuildStringFromOptions(command.Options{
 		Url: "postgres://myhost/database",
 		Ssl: "disable",
 	})
@@ -46,13 +46,13 @@ func Test_Url_And_Ssl_Flag(t *testing.T) {
 }
 
 func Test_Localhost_Url_And_No_Ssl_Flag(t *testing.T) {
-	str, err := BuildString(command.Options{
+	str, err := BuildStringFromOptions(command.Options{
 		Url: "postgres://localhost/database",
 	})
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "postgres://localhost/database?sslmode=disable", str)
 
-	str, err = BuildString(command.Options{
+	str, err = BuildStringFromOptions(command.Options{
 		Url: "postgres://127.0.0.1/database",
 	})
 	assert.Equal(t, nil, err)
@@ -60,14 +60,14 @@ func Test_Localhost_Url_And_No_Ssl_Flag(t *testing.T) {
 }
 
 func Test_Localhost_Url_And_Ssl_Flag(t *testing.T) {
-	str, err := BuildString(command.Options{
+	str, err := BuildStringFromOptions(command.Options{
 		Url: "postgres://localhost/database",
 		Ssl: "require",
 	})
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "postgres://localhost/database?sslmode=require", str)
 
-	str, err = BuildString(command.Options{
+	str, err = BuildStringFromOptions(command.Options{
 		Url: "postgres://127.0.0.1/database",
 		Ssl: "require",
 	})
@@ -76,13 +76,13 @@ func Test_Localhost_Url_And_Ssl_Flag(t *testing.T) {
 }
 
 func Test_Localhost_Url_And_Ssl_Arg(t *testing.T) {
-	str, err := BuildString(command.Options{
+	str, err := BuildStringFromOptions(command.Options{
 		Url: "postgres://localhost/database?sslmode=require",
 	})
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "postgres://localhost/database?sslmode=require", str)
 
-	str, err = BuildString(command.Options{
+	str, err = BuildStringFromOptions(command.Options{
 		Url: "postgres://127.0.0.1/database?sslmode=require",
 	})
 	assert.Equal(t, nil, err)
@@ -90,7 +90,7 @@ func Test_Localhost_Url_And_Ssl_Arg(t *testing.T) {
 }
 
 func Test_Flag_Args(t *testing.T) {
-	str, err := BuildString(command.Options{
+	str, err := BuildStringFromOptions(command.Options{
 		Host:   "host",
 		Port:   5432,
 		User:   "user",
@@ -111,12 +111,12 @@ func Test_Localhost(t *testing.T) {
 		DbName: "db",
 	}
 
-	str, err := BuildString(opts)
+	str, err := BuildStringFromOptions(opts)
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "postgres://user:password@localhost:5432/db?sslmode=disable", str)
 
 	opts.Host = "127.0.0.1"
-	str, err = BuildString(opts)
+	str, err = BuildStringFromOptions(opts)
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "postgres://user:password@127.0.0.1:5432/db?sslmode=disable", str)
 }
@@ -131,7 +131,7 @@ func Test_Localhost_And_Ssl(t *testing.T) {
 		Ssl:    "require",
 	}
 
-	str, err := BuildString(opts)
+	str, err := BuildStringFromOptions(opts)
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "postgres://user:password@localhost:5432/db?sslmode=require", str)
 }
@@ -139,18 +139,18 @@ func Test_Localhost_And_Ssl(t *testing.T) {
 func Test_No_User(t *testing.T) {
 	opts := command.Options{Host: "host", Port: 5432, DbName: "db"}
 	u, _ := user.Current()
-	str, err := BuildString(opts)
+	str, err := BuildStringFromOptions(opts)
 
 	assert.Equal(t, nil, err)
-	assert.Equal(t, fmt.Sprintf("postgres://%s@host:5432/db", u.Username), str)
+	assert.Equal(t, fmt.Sprintf("postgres://%s:@host:5432/db", u.Username), str)
 }
 
 func Test_Port(t *testing.T) {
 	opts := command.Options{Host: "host", User: "user", Port: 5000, DbName: "db"}
-	str, err := BuildString(opts)
+	str, err := BuildStringFromOptions(opts)
 
 	assert.Equal(t, nil, err)
-	assert.Equal(t, "postgres://user@host:5000/db", str)
+	assert.Equal(t, "postgres://user:@host:5000/db", str)
 }
 
 func Test_Blank(t *testing.T) {