From 8d541d00043bb65c60ee837927dc06c163729954 Mon Sep 17 00:00:00 2001 From: georgysavva Date: Mon, 1 Jun 2020 19:20:17 +0300 Subject: [PATCH 1/2] Add Config.Copy() method that return a smart copy of the config. --- config.go | 9 +++++++++ config_test.go | 33 +++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 2 ++ 4 files changed, 45 insertions(+) diff --git a/config.go b/config.go index 299d4784..6640038b 100644 --- a/config.go +++ b/config.go @@ -17,6 +17,8 @@ import ( "strings" "time" + "github.com/mohae/deepcopy" + "github.com/jackc/chunkreader/v2" "github.com/jackc/pgpassfile" "github.com/jackc/pgproto3/v2" @@ -62,6 +64,13 @@ type Config struct { createdByParseConfig bool // Used to enforce created by ParseConfig rule. } +func (c *Config) Copy() *Config { + newConf := deepcopy.Copy(c).(*Config) + // We need to set this field manually because it's unexported and deep copy won't touch it. + newConf.createdByParseConfig = c.createdByParseConfig + return newConf +} + // FallbackConfig is additional settings to attempt a connection with when the primary Config fails to establish a // network connection. It is used for TLS fallback such as sslmode=prefer and high availability (HA) connections. type FallbackConfig struct { diff --git a/config_test.go b/config_test.go index 515ea6d3..72b775d4 100644 --- a/config_test.go +++ b/config_test.go @@ -1,6 +1,7 @@ package pgconn_test import ( + "context" "crypto/tls" "fmt" "io/ioutil" @@ -527,6 +528,38 @@ func TestParseConfig(t *testing.T) { } } +func TestConfigCopyReturnsEqualConfig(t *testing.T) { + connString := "postgres://jack:secret@localhost:5432/mydb?sslmode=disable&application_name=pgxtest&search_path=myschema&connect_timeout=5" + original, err := pgconn.ParseConfig(connString) + require.NoError(t, err) + + copied := original.Copy() + assertConfigsEqual(t, original, copied, "Test Config.Copy() returns equal config") +} + +func TestConfigCopyOriginalConfigDidNotChange(t *testing.T) { + connString := "postgres://jack:secret@localhost:5432/mydb?sslmode=disable&application_name=pgxtest&search_path=myschema&connect_timeout=5" + original, err := pgconn.ParseConfig(connString) + require.NoError(t, err) + + copied := original.Copy() + copied.Port = uint16(5433) + + assert.Equal(t, uint16(5432), original.Port) +} + +func TestConfigCopyCanBeUsedToConnect(t *testing.T) { + connString := os.Getenv("PGX_TEST_CONN_STRING") + original, err := pgconn.ParseConfig(connString) + require.NoError(t, err) + + copied := original.Copy() + assert.NotPanics(t, func() { + _, err = pgconn.ConnectConfig(context.Background(), copied) + }) + assert.NoError(t, err) +} + func assertConfigsEqual(t *testing.T, expected, actual *pgconn.Config, testName string) { if !assert.NotNil(t, expected) { return diff --git a/go.mod b/go.mod index 4dc095ca..841eccc7 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/jackc/pgpassfile v1.0.0 github.com/jackc/pgproto3/v2 v2.0.1 github.com/jackc/pgservicefile v0.0.0-20200307190119-3430c5407db8 + github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/stretchr/testify v1.5.1 golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59 golang.org/x/text v0.3.2 diff --git a/go.sum b/go.sum index 23fb8b32..1514a339 100644 --- a/go.sum +++ b/go.sum @@ -54,6 +54,8 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.7/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= From a6d9265506df51336914769fd786ef8712997c31 Mon Sep 17 00:00:00 2001 From: georgysavva Date: Mon, 1 Jun 2020 20:52:08 +0300 Subject: [PATCH 2/2] Implement deep copy manually, stop using an external deep copy library. Add comment to the Config.Copy() method. --- config.go | 30 +++++++++++++++++++++++++----- config_test.go | 10 ++++++++-- go.mod | 1 - go.sum | 2 -- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/config.go b/config.go index 6640038b..7ed99096 100644 --- a/config.go +++ b/config.go @@ -17,8 +17,6 @@ import ( "strings" "time" - "github.com/mohae/deepcopy" - "github.com/jackc/chunkreader/v2" "github.com/jackc/pgpassfile" "github.com/jackc/pgproto3/v2" @@ -64,10 +62,32 @@ type Config struct { createdByParseConfig bool // Used to enforce created by ParseConfig rule. } +// Copy returns a deep copy of the config that is safe to use and modify. +// The only exception is the TLSConfig field: +// according to the tls.Config docs it must not be modified after creation. func (c *Config) Copy() *Config { - newConf := deepcopy.Copy(c).(*Config) - // We need to set this field manually because it's unexported and deep copy won't touch it. - newConf.createdByParseConfig = c.createdByParseConfig + newConf := new(Config) + *newConf = *c + if newConf.TLSConfig != nil { + newConf.TLSConfig = c.TLSConfig.Clone() + } + if newConf.RuntimeParams != nil { + newConf.RuntimeParams = make(map[string]string, len(c.RuntimeParams)) + for k, v := range c.RuntimeParams { + newConf.RuntimeParams[k] = v + } + } + if newConf.Fallbacks != nil { + newConf.Fallbacks = make([]*FallbackConfig, len(c.Fallbacks)) + for i, fallback := range c.Fallbacks { + newFallback := new(FallbackConfig) + *newFallback = *fallback + if newFallback.TLSConfig != nil { + newFallback.TLSConfig = fallback.TLSConfig.Clone() + } + newConf.Fallbacks[i] = newFallback + } + } return newConf } diff --git a/config_test.go b/config_test.go index 72b775d4..ebe627b1 100644 --- a/config_test.go +++ b/config_test.go @@ -529,7 +529,7 @@ func TestParseConfig(t *testing.T) { } func TestConfigCopyReturnsEqualConfig(t *testing.T) { - connString := "postgres://jack:secret@localhost:5432/mydb?sslmode=disable&application_name=pgxtest&search_path=myschema&connect_timeout=5" + connString := "postgres://jack:secret@localhost:5432/mydb?application_name=pgxtest&search_path=myschema&connect_timeout=5" original, err := pgconn.ParseConfig(connString) require.NoError(t, err) @@ -538,14 +538,20 @@ func TestConfigCopyReturnsEqualConfig(t *testing.T) { } func TestConfigCopyOriginalConfigDidNotChange(t *testing.T) { - connString := "postgres://jack:secret@localhost:5432/mydb?sslmode=disable&application_name=pgxtest&search_path=myschema&connect_timeout=5" + connString := "postgres://jack:secret@localhost:5432/mydb?application_name=pgxtest&search_path=myschema&connect_timeout=5" original, err := pgconn.ParseConfig(connString) require.NoError(t, err) copied := original.Copy() + assertConfigsEqual(t, original, copied, "Test Config.Copy() returns equal config") + copied.Port = uint16(5433) + copied.RuntimeParams["foo"] = "bar" + copied.Fallbacks[0].Port = uint16(5433) assert.Equal(t, uint16(5432), original.Port) + assert.Equal(t, "", original.RuntimeParams["foo"]) + assert.Equal(t, uint16(5432), original.Fallbacks[0].Port) } func TestConfigCopyCanBeUsedToConnect(t *testing.T) { diff --git a/go.mod b/go.mod index 841eccc7..4dc095ca 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,6 @@ require ( github.com/jackc/pgpassfile v1.0.0 github.com/jackc/pgproto3/v2 v2.0.1 github.com/jackc/pgservicefile v0.0.0-20200307190119-3430c5407db8 - github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/stretchr/testify v1.5.1 golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59 golang.org/x/text v0.3.2 diff --git a/go.sum b/go.sum index 1514a339..23fb8b32 100644 --- a/go.sum +++ b/go.sum @@ -54,8 +54,6 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.7/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= -github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw= -github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=