From b7b52ff0794fe7a1f27c045af21adbdb92822c07 Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Sat, 24 Aug 2019 09:49:38 -0500 Subject: [PATCH] Require conn and pool configs to be created by ParseConfig This simplifies handling default values. Now there is no ambiguity between a zero value and a default value. All default values are set by ParseConfig and the user can modify them after the initial creation. fixes #567 --- conn.go | 26 +++++++++++++++++--------- conn_test.go | 10 +++++++++- go.mod | 9 ++++++--- go.sum | 24 ++++++++++++++++++++---- helper_test.go | 10 +++++----- pgxpool/pool.go | 19 +++++++++++++++---- pgxpool/pool_test.go | 8 ++++++++ 7 files changed, 80 insertions(+), 26 deletions(-) diff --git a/conn.go b/conn.go index e7c5ef94..462fc693 100644 --- a/conn.go +++ b/conn.go @@ -20,7 +20,8 @@ const ( connStatusBusy ) -// ConnConfig contains all the options used to establish a connection. +// ConnConfig contains all the options used to establish a connection. It must be created by ParseConfig and +// then it can be modified. A manually initialized ConnConfig will cause ConnectConfig to panic. type ConnConfig struct { pgconn.Config Logger Logger @@ -33,6 +34,8 @@ type ConnConfig struct { // used by default. The same functionality can be controlled on a per query basis by setting // QueryExOptions.SimpleProtocol. PreferSimpleProtocol bool + + createdByParseConfig bool // Used to enforce created by ParseConfig rule. } // Conn is a PostgreSQL connection handle. It is not safe for concurrent usage. Use a connection pool to manage access @@ -113,35 +116,40 @@ func Connect(ctx context.Context, connString string) (*Conn, error) { return connect(ctx, connConfig) } -// Connect establishes a connection with a PostgreSQL server with a configuration struct. +// Connect establishes a connection with a PostgreSQL server with a configuration struct. connConfig must have been +// created by ParseConfig. func ConnectConfig(ctx context.Context, connConfig *ConnConfig) (*Conn, error) { return connect(ctx, connConfig) } +// ParseConfig creates a ConnConfig from a connection string. See pgconn.ParseConfig for details. func ParseConfig(connString string) (*ConnConfig, error) { config, err := pgconn.ParseConfig(connString) if err != nil { return nil, err } connConfig := &ConnConfig{ - Config: *config, + Config: *config, + createdByParseConfig: true, + LogLevel: LogLevelInfo, } return connConfig, nil } func connect(ctx context.Context, config *ConnConfig) (c *Conn, err error) { + // Default values are set in ParseConfig. Enforce initial creation by ParseConfig rather than setting defaults from + // zero values. + if !config.createdByParseConfig { + panic("config must be created by ParseConfig") + } + c = new(Conn) c.config = config c.ConnInfo = pgtype.NewConnInfo() - if c.config.LogLevel != 0 { - c.logLevel = c.config.LogLevel - } else { - // Preserve pre-LogLevel behavior by defaulting to LogLevelDebug - c.logLevel = LogLevelDebug - } + c.logLevel = c.config.LogLevel c.logger = c.config.Logger if c.shouldLog(LogLevelInfo) { diff --git a/conn_test.go b/conn_test.go index 010cfa7b..2437d91c 100644 --- a/conn_test.go +++ b/conn_test.go @@ -42,7 +42,7 @@ func TestConnect(t *testing.T) { config := mustParseConfig(t, os.Getenv("PGX_TEST_DATABASE")) - conn, err := pgx.ConnectConfig(context.Background(), &config) + conn, err := pgx.ConnectConfig(context.Background(), config) if err != nil { t.Fatalf("Unable to establish connection: %v", err) } @@ -96,6 +96,14 @@ func TestConnectWithPreferSimpleProtocol(t *testing.T) { ensureConnValid(t, conn) } +func TestConnectConfigRequiresConnConfigFromParseConfig(t *testing.T) { + t.Parallel() + + config := &pgx.ConnConfig{} + + require.PanicsWithValue(t, "config must be created by ParseConfig", func() { pgx.ConnectConfig(context.Background(), config) }) +} + func TestExec(t *testing.T) { t.Parallel() diff --git a/go.mod b/go.mod index 5bcae657..0dc0c7e8 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.12 require ( github.com/cockroachdb/apd v1.1.0 github.com/go-stack/stack v1.8.0 // indirect - github.com/jackc/pgconn v0.0.0-20190806220711-f0b479097a48 + github.com/jackc/pgconn v0.0.0-20190824142844-760dd75542eb github.com/jackc/pgio v1.0.0 github.com/jackc/pgproto3/v2 v2.0.0-alpha1.0.20190609003834-432c2951c711 github.com/jackc/pgtype v0.0.0-20190421001408-4ed0de4755e0 @@ -16,9 +16,12 @@ require ( github.com/satori/go.uuid v1.2.0 github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24 github.com/sirupsen/logrus v1.4.1 - github.com/stretchr/testify v1.3.0 + github.com/stretchr/testify v1.4.0 go.uber.org/atomic v1.4.0 // indirect go.uber.org/zap v1.9.1 - golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 + golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586 // indirect + golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a // indirect + golang.org/x/text v0.3.2 // indirect + golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec ) diff --git a/go.sum b/go.sum index fbe4b5cc..bc0afd33 100644 --- a/go.sum +++ b/go.sum @@ -12,10 +12,8 @@ github.com/jackc/chunkreader/v2 v2.0.0 h1:DUwgMQuuPnS0rhMXenUtZpqZqrR/30NWY+qQvT github.com/jackc/chunkreader/v2 v2.0.0/go.mod h1:odVSm741yZoC3dpHEUXIqA9tQRhFrgOHwnPIn9lDKlk= github.com/jackc/pgconn v0.0.0-20190420214824-7e0022ef6ba3 h1:ZFYpB74Kq8xE9gmfxCmXD6QxZ27ja+j3HwGFc+YurhQ= github.com/jackc/pgconn v0.0.0-20190420214824-7e0022ef6ba3/go.mod h1:jkELnwuX+w9qN5YIfX0fl88Ehu4XC3keFuOJJk9pcnA= -github.com/jackc/pgconn v0.0.0-20190528115420-71ec1f782113 h1:EpJHD0fHY9s+K1d2gn0YrVNf2MzCZsgtGgnzKqJGnOw= -github.com/jackc/pgconn v0.0.0-20190528115420-71ec1f782113/go.mod h1:f8MMBsyH8EXpj7xNt09B6QAWl1OYflD0QeF6BBCYsdM= -github.com/jackc/pgconn v0.0.0-20190806220711-f0b479097a48 h1:M3R/SnoNYpmteW+uJE945ak1brFYxfsaBt2b5gS27V4= -github.com/jackc/pgconn v0.0.0-20190806220711-f0b479097a48/go.mod h1:lLjNuW/+OfW9/pnVKPazfWOgNfH2aPem8YQ7ilXGvJE= +github.com/jackc/pgconn v0.0.0-20190824142844-760dd75542eb h1:d6GP9szHvXVopAOAnZ7WhRnF3Xdxrylmm/9jnfmW4Ag= +github.com/jackc/pgconn v0.0.0-20190824142844-760dd75542eb/go.mod h1:lLjNuW/+OfW9/pnVKPazfWOgNfH2aPem8YQ7ilXGvJE= github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE= github.com/jackc/pgio v1.0.0/go.mod h1:oP+2QK2wFfUWgr+gxjoBH9KGBb31Eio69xUb0w5bYf8= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= @@ -64,6 +62,8 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0 h1:cxzIVoETapQEqDhQu3QfnvXAV4AlzcvUCxkVUFw3+EU= @@ -72,20 +72,36 @@ go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/zap v1.9.1 h1:XCJQEf3W6eZaVwhRBof6ImoYGJSITeKWsyeh3HFu/5o= go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a h1:Igim7XhdOpBnWPuYJ70XcNpq8q3BCACtVgNfoJxOV7g= golang.org/x/crypto v0.0.0-20190411191339-88737f569e3a/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE= +golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586 h1:7KByu05hhLed2MO29w7p1XfZvZ13m8mub3shuVftRs0= +golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33 h1:I6FyU15t786LL7oL/hn43zqTuEGr4PN7F4XJ1p4E3Y8= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e h1:nFYrTHrdrAOpShe27kaFHjsqYSEQ0KWqdWLu3xuZJts= golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a h1:aYOabOQFp6Vj6W1F80affTUvO9UxmJRx8K0gsfABByQ= +golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= +golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373 h1:PPwnA7z1Pjf7XYaBP9GL1VAMZmcIWyFz7QCMSIIa3Bg= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 h1:bhOzK9QyoD0ogCnFro1m2mz41+Ib0oOhfJnBp5MR4K4= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec h1:RlWgLqCMMIYYEVcAR5MDsuHlVkaIPDAF+5Dehzg8L5A= gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:aPpfJ7XW+gOuirDoZ8gHhLh3kZ1B08FtV2bbmy7Jv3s= +gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/helper_test.go b/helper_test.go index c67cbfd4..6b357577 100644 --- a/helper_test.go +++ b/helper_test.go @@ -17,14 +17,14 @@ func mustConnectString(t testing.TB, connString string) *pgx.Conn { return conn } -func mustParseConfig(t testing.TB, connString string) pgx.ConnConfig { - config, err := pgconn.ParseConfig(connString) +func mustParseConfig(t testing.TB, connString string) *pgx.ConnConfig { + config, err := pgx.ParseConfig(connString) require.Nil(t, err) - return pgx.ConnConfig{Config: *config} + return config } -func mustConnect(t testing.TB, config pgx.ConnConfig) *pgx.Conn { - conn, err := pgx.ConnectConfig(context.Background(), &config) +func mustConnect(t testing.TB, config *pgx.ConnConfig) *pgx.Conn { + conn, err := pgx.ConnectConfig(context.Background(), config) if err != nil { t.Fatalf("Unable to establish connection: %v", err) } diff --git a/pgxpool/pool.go b/pgxpool/pool.go index 43020fd2..b8067f4a 100644 --- a/pgxpool/pool.go +++ b/pgxpool/pool.go @@ -75,8 +75,8 @@ type Pool struct { closeChan chan struct{} } -// Config is the configuration struct for creating a pool. It is highly recommended to modify a Config returned by -// ParseConfig rather than to construct a Config from scratch. +// Config is the configuration struct for creating a pool. It must be created by ParseConfig and then it can be +// modified. A manually initialized ConnConfig will cause ConnectConfig to panic. type Config struct { ConnConfig *pgx.ConnConfig @@ -100,6 +100,8 @@ type Config struct { // HealthCheckPeriod is the duration between checks of the health of idle connections. HealthCheckPeriod time.Duration + + createdByParseConfig bool // Used to enforce created by ParseConfig rule. } // Connect creates a new Pool and immediately establishes one connection. ctx can be used to cancel this initial @@ -114,8 +116,14 @@ func Connect(ctx context.Context, connString string) (*Pool, error) { } // ConnectConfig creates a new Pool and immediately establishes one connection. ctx can be used to cancel this initial -// connection. +// connection. config must have been created by ParseConfig. func ConnectConfig(ctx context.Context, config *Config) (*Pool, error) { + // Default values are set in ParseConfig. Enforce initial creation by ParseConfig rather than setting defaults from + // zero values. + if !config.createdByParseConfig { + panic("config must be created by ParseConfig") + } + p := &Pool{ afterConnect: config.AfterConnect, beforeAcquire: config.BeforeAcquire, @@ -192,7 +200,10 @@ func ParseConfig(connString string) (*Config, error) { return nil, err } - config := &Config{ConnConfig: connConfig} + config := &Config{ + ConnConfig: connConfig, + createdByParseConfig: true, + } if s, ok := config.ConnConfig.Config.RuntimeParams["pool_max_conns"]; ok { delete(connConfig.Config.RuntimeParams, "pool_max_conns") diff --git a/pgxpool/pool_test.go b/pgxpool/pool_test.go index 14f4a27c..d375e8cf 100644 --- a/pgxpool/pool_test.go +++ b/pgxpool/pool_test.go @@ -39,6 +39,14 @@ func TestConnectCancel(t *testing.T) { assert.Equal(t, context.Canceled, err) } +func TestConnectConfigRequiresConnConfigFromParseConfig(t *testing.T) { + t.Parallel() + + config := &pgxpool.Config{} + + require.PanicsWithValue(t, "config must be created by ParseConfig", func() { pgxpool.ConnectConfig(context.Background(), config) }) +} + func TestPoolAcquireAndConnRelease(t *testing.T) { t.Parallel()