From 65b05a0f63dd137d680af4d98ae8d32dd2603158 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Mon, 6 Jul 2020 09:47:46 -0400 Subject: [PATCH] 16534: Supply *sqlx.Tx to controller handlers. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- go.mod | 1 + go.sum | 5 +++++ lib/controller/handler.go | 8 ++++---- lib/controller/localdb/db.go | 16 ++++++++-------- lib/controller/localdb/db_test.go | 14 +++++++------- lib/controller/localdb/login.go | 2 +- lib/controller/localdb/login_ldap_test.go | 4 ++-- 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/go.mod b/go.mod index cc5457975f..45bde2cef4 100644 --- a/go.mod +++ b/go.mod @@ -35,6 +35,7 @@ require ( github.com/imdario/mergo v0.3.8-0.20190415133143-5ef87b449ca7 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/jmcvetta/randutil v0.0.0-20150817122601-2bb1b664bcff + github.com/jmoiron/sqlx v1.2.0 github.com/julienschmidt/httprouter v1.2.0 github.com/karalabe/xgo v0.0.0-20191115072854-c5ccff8648a7 // indirect github.com/kevinburke/ssh_config v0.0.0-20171013211458-802051befeb5 // indirect diff --git a/go.sum b/go.sum index 38153ce3ea..6dda3992bc 100644 --- a/go.sum +++ b/go.sum @@ -72,6 +72,7 @@ github.com/go-ldap/ldap v3.0.3+incompatible h1:HTeSZO8hWMS1Rgb2Ziku6b8a7qRIZZMHj github.com/go-ldap/ldap v3.0.3+incompatible/go.mod h1:qfd9rJvER9Q0/D/Sqn1DfHRoBp40uXYvFoEVrNEPqRc= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= +github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/gogo/protobuf v1.1.1 h1:72R+M5VuhED/KujmZVcIquuo8mBgX4oVda//DQb3PXo= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= @@ -109,6 +110,8 @@ github.com/jmcvetta/randutil v0.0.0-20150817122601-2bb1b664bcff h1:6NvhExg4omUC9 github.com/jmcvetta/randutil v0.0.0-20150817122601-2bb1b664bcff/go.mod h1:ddfPX8Z28YMjiqoaJhNBzWHapTHXejnB5cDCUWDwriw= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5imkbgOkpRUYLnmbU7UEFbjtDA2hxJ1ichM= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= +github.com/jmoiron/sqlx v1.2.0 h1:41Ip0zITnmWNR/vHV+S4m+VoUivnWY5E4OJfLZjCJMA= +github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= @@ -121,10 +124,12 @@ github.com/kevinburke/ssh_config v0.0.0-20171013211458-802051befeb5/go.mod h1:CT github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= +github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.3.0 h1:/qkRGz8zljWiDcFvgpwUpwIAPu3r07TDvs3Rws+o/pU= github.com/lib/pq v1.3.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/marstr/guid v1.1.1-0.20170427235115-8bdf7d1a087c h1:ouxemItv3B/Zh008HJkEXDYCN3BIRyNHxtUN7ThJ5Js= github.com/marstr/guid v1.1.1-0.20170427235115-8bdf7d1a087c/go.mod h1:74gB1z2wpxxInTG6yaqA7KrtM0NZ+RbrcqDvYHefzho= +github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/mitchellh/go-homedir v0.0.0-20161203194507-b8bc1bf76747 h1:eQox4Rh4ewJF+mqYPxCkmBAirRnPaHEB26UkNuPyjlk= diff --git a/lib/controller/handler.go b/lib/controller/handler.go index cc06246420..4d4963413b 100644 --- a/lib/controller/handler.go +++ b/lib/controller/handler.go @@ -6,7 +6,6 @@ package controller import ( "context" - "database/sql" "errors" "fmt" "net/http" @@ -23,6 +22,7 @@ import ( "git.arvados.org/arvados.git/sdk/go/ctxlog" "git.arvados.org/arvados.git/sdk/go/health" "git.arvados.org/arvados.git/sdk/go/httpserver" + "github.com/jmoiron/sqlx" _ "github.com/lib/pq" ) @@ -34,7 +34,7 @@ type Handler struct { proxy *proxy secureClient *http.Client insecureClient *http.Client - pgdb *sql.DB + pgdb *sqlx.DB pgdbMtx sync.Mutex } @@ -121,14 +121,14 @@ func (h *Handler) setup() { var errDBConnection = errors.New("database connection error") -func (h *Handler) db(ctx context.Context) (*sql.DB, error) { +func (h *Handler) db(ctx context.Context) (*sqlx.DB, error) { h.pgdbMtx.Lock() defer h.pgdbMtx.Unlock() if h.pgdb != nil { return h.pgdb, nil } - db, err := sql.Open("postgres", h.Cluster.PostgreSQL.Connection.String()) + db, err := sqlx.Open("postgres", h.Cluster.PostgreSQL.Connection.String()) if err != nil { ctxlog.FromContext(ctx).WithError(err).Error("postgresql connect failed") return nil, errDBConnection diff --git a/lib/controller/localdb/db.go b/lib/controller/localdb/db.go index 4f64e63524..cad5308853 100644 --- a/lib/controller/localdb/db.go +++ b/lib/controller/localdb/db.go @@ -6,12 +6,12 @@ package localdb import ( "context" - "database/sql" "errors" "sync" "git.arvados.org/arvados.git/lib/controller/router" "git.arvados.org/arvados.git/sdk/go/ctxlog" + "github.com/jmoiron/sqlx" ) // WrapCallsInTransactions returns a call wrapper (suitable for @@ -20,7 +20,7 @@ import ( // // The wrapper calls getdb() to get a database handle before each API // call. -func WrapCallsInTransactions(getdb func(context.Context) (*sql.DB, error)) func(router.RoutableFunc) router.RoutableFunc { +func WrapCallsInTransactions(getdb func(context.Context) (*sqlx.DB, error)) func(router.RoutableFunc) router.RoutableFunc { return func(origFunc router.RoutableFunc) router.RoutableFunc { return func(ctx context.Context, opts interface{}) (_ interface{}, err error) { ctx, finishtx := starttx(ctx, getdb) @@ -33,7 +33,7 @@ func WrapCallsInTransactions(getdb func(context.Context) (*sql.DB, error)) func( // ContextWithTransaction returns a child context in which the given // transaction will be used by any localdb API call that needs one. // The caller is responsible for calling Commit or Rollback on tx. -func ContextWithTransaction(ctx context.Context, tx *sql.Tx) context.Context { +func ContextWithTransaction(ctx context.Context, tx *sqlx.Tx) context.Context { txn := &transaction{tx: tx} txn.setup.Do(func() {}) return context.WithValue(ctx, contextKeyTransaction, txn) @@ -44,9 +44,9 @@ type contextKeyT string var contextKeyTransaction = contextKeyT("transaction") type transaction struct { - tx *sql.Tx + tx *sqlx.Tx err error - getdb func(context.Context) (*sql.DB, error) + getdb func(context.Context) (*sqlx.DB, error) setup sync.Once } @@ -75,7 +75,7 @@ type transactionFinishFunc func(*error) // // If *err is non-nil, finishtx() rolls back the transaction, and // does not modify *err. -func starttx(ctx context.Context, getdb func(context.Context) (*sql.DB, error)) (context.Context, transactionFinishFunc) { +func starttx(ctx context.Context, getdb func(context.Context) (*sqlx.DB, error)) (context.Context, transactionFinishFunc) { txn := &transaction{getdb: getdb} return context.WithValue(ctx, contextKeyTransaction, txn), func(err *error) { txn.setup.Do(func() { @@ -100,7 +100,7 @@ func starttx(ctx context.Context, getdb func(context.Context) (*sql.DB, error)) } } -func currenttx(ctx context.Context) (*sql.Tx, error) { +func currenttx(ctx context.Context) (*sqlx.Tx, error) { txn, ok := ctx.Value(contextKeyTransaction).(*transaction) if !ok { return nil, errors.New("bug: there is no transaction in this context") @@ -109,7 +109,7 @@ func currenttx(ctx context.Context) (*sql.Tx, error) { if db, err := txn.getdb(ctx); err != nil { txn.err = err } else { - txn.tx, txn.err = db.Begin() + txn.tx, txn.err = db.Beginx() } }) return txn.tx, txn.err diff --git a/lib/controller/localdb/db_test.go b/lib/controller/localdb/db_test.go index 5bab86c602..741eabad9a 100644 --- a/lib/controller/localdb/db_test.go +++ b/lib/controller/localdb/db_test.go @@ -6,20 +6,20 @@ package localdb import ( "context" - "database/sql" "sync" "sync/atomic" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/ctxlog" + "github.com/jmoiron/sqlx" _ "github.com/lib/pq" check "gopkg.in/check.v1" ) // testdb returns a DB connection for the given cluster config. -func testdb(c *check.C, cluster *arvados.Cluster) *sql.DB { - db, err := sql.Open("postgres", cluster.PostgreSQL.Connection.String()) +func testdb(c *check.C, cluster *arvados.Cluster) *sqlx.DB { + db, err := sqlx.Open("postgres", cluster.PostgreSQL.Connection.String()) c.Assert(err, check.IsNil) return db } @@ -27,8 +27,8 @@ func testdb(c *check.C, cluster *arvados.Cluster) *sql.DB { // testctx returns a context suitable for running a test case in a new // transaction, and a rollback func which the caller should call after // the test. -func testctx(c *check.C, db *sql.DB) (ctx context.Context, rollback func()) { - tx, err := db.Begin() +func testctx(c *check.C, db *sqlx.DB) (ctx context.Context, rollback func()) { + tx, err := db.Beginx() c.Assert(err, check.IsNil) return ContextWithTransaction(context.Background(), tx), func() { c.Check(tx.Rollback(), check.IsNil) @@ -46,13 +46,13 @@ func (*DatabaseSuite) TestTransactionContext(c *check.C) { c.Assert(err, check.IsNil) var getterCalled int64 - getter := func(context.Context) (*sql.DB, error) { + getter := func(context.Context) (*sqlx.DB, error) { atomic.AddInt64(&getterCalled, 1) return testdb(c, cluster), nil } wrapper := WrapCallsInTransactions(getter) wrappedFunc := wrapper(func(ctx context.Context, opts interface{}) (interface{}, error) { - txes := make([]*sql.Tx, 20) + txes := make([]*sqlx.Tx, 20) var wg sync.WaitGroup for i := range txes { i := i diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go index 1cd349a10e..dc2c7c875c 100644 --- a/lib/controller/localdb/login.go +++ b/lib/controller/localdb/login.go @@ -130,7 +130,7 @@ func createAPIClientAuthorization(ctx context.Context, conn *rpc.Conn, rootToken } var exp sql.NullString var scopes []byte - err = tx.QueryRowContext(ctx, "select uuid, api_token, expires_at, scopes from api_client_authorizations where api_token=$1", tokensecret).Scan(&resp.UUID, &resp.APIToken, &exp, &scopes) + err = tx.QueryRowxContext(ctx, "select uuid, api_token, expires_at, scopes from api_client_authorizations where api_token=$1", tokensecret).Scan(&resp.UUID, &resp.APIToken, &exp, &scopes) if err != nil { return } diff --git a/lib/controller/localdb/login_ldap_test.go b/lib/controller/localdb/login_ldap_test.go index 64ae58bce2..15343ab322 100644 --- a/lib/controller/localdb/login_ldap_test.go +++ b/lib/controller/localdb/login_ldap_test.go @@ -6,7 +6,6 @@ package localdb import ( "context" - "database/sql" "encoding/json" "net" "net/http" @@ -18,6 +17,7 @@ import ( "git.arvados.org/arvados.git/sdk/go/auth" "git.arvados.org/arvados.git/sdk/go/ctxlog" "github.com/bradleypeabody/godap" + "github.com/jmoiron/sqlx" check "gopkg.in/check.v1" ) @@ -27,7 +27,7 @@ type LDAPSuite struct { cluster *arvados.Cluster ctrl *ldapLoginController ldap *godap.LDAPServer // fake ldap server that accepts auth goodusername/goodpassword - db *sql.DB + db *sqlx.DB // transaction context ctx context.Context -- 2.30.2