[ARVADOS] created: 1.3.0-2752-gb39a9f6af
Git user
git at public.arvados.org
Mon Jul 6 19:22:38 UTC 2020
at b39a9f6af414f8f7628fa56ee3f35dee0ad92a7f (commit)
commit b39a9f6af414f8f7628fa56ee3f35dee0ad92a7f
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Jul 6 15:22:31 2020 -0400
16534: Move controller transaction/context code to its own package.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 4d4963413..e742bbc59 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -15,9 +15,9 @@ import (
"time"
"git.arvados.org/arvados.git/lib/controller/federation"
- "git.arvados.org/arvados.git/lib/controller/localdb"
"git.arvados.org/arvados.git/lib/controller/railsproxy"
"git.arvados.org/arvados.git/lib/controller/router"
+ "git.arvados.org/arvados.git/lib/ctrlctx"
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/ctxlog"
"git.arvados.org/arvados.git/sdk/go/health"
@@ -87,7 +87,7 @@ func (h *Handler) setup() {
Routes: health.Routes{"ping": func() error { _, err := h.db(context.TODO()); return err }},
})
- rtr := router.New(federation.New(h.Cluster), localdb.WrapCallsInTransactions(h.db))
+ rtr := router.New(federation.New(h.Cluster), ctrlctx.WrapCallsInTransactions(h.db))
mux.Handle("/arvados/v1/config", rtr)
mux.Handle("/"+arvados.EndpointUserAuthenticate.Path, rtr)
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index dc2c7c875..ee1ea5692 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -15,6 +15,7 @@ import (
"strings"
"git.arvados.org/arvados.git/lib/controller/rpc"
+ "git.arvados.org/arvados.git/lib/ctrlctx"
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/auth"
"git.arvados.org/arvados.git/sdk/go/httpserver"
@@ -117,7 +118,7 @@ func createAPIClientAuthorization(ctx context.Context, conn *rpc.Conn, rootToken
return
}
token := target.Query().Get("api_token")
- tx, err := currenttx(ctx)
+ tx, err := ctrlctx.CurrentTx(ctx)
if err != nil {
return
}
diff --git a/lib/controller/localdb/login_ldap_test.go b/lib/controller/localdb/login_ldap_test.go
index 15343ab32..0c94fa6c0 100644
--- a/lib/controller/localdb/login_ldap_test.go
+++ b/lib/controller/localdb/login_ldap_test.go
@@ -12,6 +12,7 @@ import (
"git.arvados.org/arvados.git/lib/config"
"git.arvados.org/arvados.git/lib/controller/railsproxy"
+ "git.arvados.org/arvados.git/lib/ctrlctx"
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
"git.arvados.org/arvados.git/sdk/go/auth"
@@ -31,7 +32,7 @@ type LDAPSuite struct {
// transaction context
ctx context.Context
- rollback func()
+ rollback func() error
}
func (s *LDAPSuite) TearDownSuite(c *check.C) {
@@ -91,15 +92,20 @@ func (s *LDAPSuite) SetUpSuite(c *check.C) {
Cluster: s.cluster,
RailsProxy: railsproxy.NewConn(s.cluster),
}
- s.db = testdb(c, s.cluster)
+ s.db = arvadostest.DB(c, s.cluster)
}
func (s *LDAPSuite) SetUpTest(c *check.C) {
- s.ctx, s.rollback = testctx(c, s.db)
+ tx, err := s.db.Beginx()
+ c.Assert(err, check.IsNil)
+ s.ctx = ctrlctx.NewWithTransaction(context.Background(), tx)
+ s.rollback = tx.Rollback
}
func (s *LDAPSuite) TearDownTest(c *check.C) {
- s.rollback()
+ if s.rollback != nil {
+ s.rollback()
+ }
}
func (s *LDAPSuite) TestLoginSuccess(c *check.C) {
diff --git a/lib/controller/localdb/db.go b/lib/ctrlctx/db.go
similarity index 72%
rename from lib/controller/localdb/db.go
rename to lib/ctrlctx/db.go
index cad530885..3f5cc9466 100644
--- a/lib/controller/localdb/db.go
+++ b/lib/ctrlctx/db.go
@@ -2,7 +2,7 @@
//
// SPDX-License-Identifier: AGPL-3.0
-package localdb
+package ctrlctx
import (
"context"
@@ -12,6 +12,12 @@ import (
"git.arvados.org/arvados.git/lib/controller/router"
"git.arvados.org/arvados.git/sdk/go/ctxlog"
"github.com/jmoiron/sqlx"
+ _ "github.com/lib/pq"
+)
+
+var (
+ ErrNoTransaction = errors.New("bug: there is no transaction in this context")
+ ErrContextFinished = errors.New("refusing to start a transaction after wrapped function already returned")
)
// WrapCallsInTransactions returns a call wrapper (suitable for
@@ -23,17 +29,17 @@ import (
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)
+ ctx, finishtx := New(ctx, getdb)
defer finishtx(&err)
return origFunc(ctx, opts)
}
}
}
-// ContextWithTransaction returns a child context in which the given
+// NewWithTransaction 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 *sqlx.Tx) context.Context {
+func NewWithTransaction(ctx context.Context, tx *sqlx.Tx) context.Context {
txn := &transaction{tx: tx}
txn.setup.Do(func() {})
return context.WithValue(ctx, contextKeyTransaction, txn)
@@ -50,20 +56,20 @@ type transaction struct {
setup sync.Once
}
-type transactionFinishFunc func(*error)
+type finishFunc func(*error)
-// starttx returns a new child context that can be used with
-// currenttx(). It does not open a database transaction until the
-// first call to currenttx().
+// New returns a new child context that can be used with
+// CurrentTx(). It does not open a database transaction until the
+// first call to CurrentTx().
//
// The caller must eventually call the returned finishtx() func to
// commit or rollback the transaction, if any.
//
// func example(ctx context.Context) (err error) {
-// ctx, finishtx := starttx(ctx, dber)
+// ctx, finishtx := NewContext(ctx, dber)
// defer finishtx(&err)
// // ...
-// tx, err := currenttx(ctx)
+// tx, err := CurrentTx(ctx)
// if err != nil {
// return fmt.Errorf("example: %s", err)
// }
@@ -75,17 +81,17 @@ 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) (*sqlx.DB, error)) (context.Context, transactionFinishFunc) {
+func New(ctx context.Context, getdb func(context.Context) (*sqlx.DB, error)) (context.Context, finishFunc) {
txn := &transaction{getdb: getdb}
return context.WithValue(ctx, contextKeyTransaction, txn), func(err *error) {
txn.setup.Do(func() {
// Using (*sync.Once)Do() prevents a future
- // call to currenttx() from opening a
+ // call to CurrentTx() from opening a
// transaction which would never get committed
- // or rolled back. If currenttx() hasn't been
+ // or rolled back. If CurrentTx() hasn't been
// called before now, future calls will return
// this error.
- txn.err = errors.New("refusing to start a transaction after wrapped function already returned")
+ txn.err = ErrContextFinished
})
if txn.tx == nil {
// we never [successfully] started a transaction
@@ -100,10 +106,10 @@ func starttx(ctx context.Context, getdb func(context.Context) (*sqlx.DB, error))
}
}
-func currenttx(ctx context.Context) (*sqlx.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")
+ return nil, ErrNoTransaction
}
txn.setup.Do(func() {
if db, err := txn.getdb(ctx); err != nil {
diff --git a/lib/controller/localdb/db_test.go b/lib/ctrlctx/db_test.go
similarity index 65%
rename from lib/controller/localdb/db_test.go
rename to lib/ctrlctx/db_test.go
index 741eabad9..5361f13c6 100644
--- a/lib/controller/localdb/db_test.go
+++ b/lib/ctrlctx/db_test.go
@@ -2,37 +2,24 @@
//
// SPDX-License-Identifier: AGPL-3.0
-package localdb
+package ctrlctx
import (
"context"
"sync"
"sync/atomic"
+ "testing"
"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) *sqlx.DB {
- db, err := sqlx.Open("postgres", cluster.PostgreSQL.Connection.String())
- c.Assert(err, check.IsNil)
- return 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 *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)
- }
+// Gocheck boilerplate
+func Test(t *testing.T) {
+ check.TestingT(t)
}
var _ = check.Suite(&DatabaseSuite{})
@@ -48,7 +35,9 @@ func (*DatabaseSuite) TestTransactionContext(c *check.C) {
var getterCalled int64
getter := func(context.Context) (*sqlx.DB, error) {
atomic.AddInt64(&getterCalled, 1)
- return testdb(c, cluster), nil
+ db, err := sqlx.Open("postgres", cluster.PostgreSQL.Connection.String())
+ c.Assert(err, check.IsNil)
+ return db, nil
}
wrapper := WrapCallsInTransactions(getter)
wrappedFunc := wrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
@@ -58,14 +47,14 @@ func (*DatabaseSuite) TestTransactionContext(c *check.C) {
i := i
wg.Add(1)
go func() {
- // Concurrent calls to currenttx(),
+ // Concurrent calls to CurrentTx(),
// with different children of the same
// parent context, will all return the
// same transaction.
defer wg.Done()
ctx, cancel := context.WithCancel(ctx)
defer cancel()
- tx, err := currenttx(ctx)
+ tx, err := CurrentTx(ctx)
c.Check(err, check.IsNil)
txes[i] = tx
}()
@@ -82,8 +71,8 @@ func (*DatabaseSuite) TestTransactionContext(c *check.C) {
c.Check(err, check.IsNil)
c.Check(getterCalled, check.Equals, int64(1))
- // When a wrapped func returns without calling currenttx(),
- // calling currenttx() later shouldn't start a new
+ // When a wrapped func returns without calling CurrentTx(),
+ // calling CurrentTx() later shouldn't start a new
// transaction.
var savedctx context.Context
ok, err = wrapper(func(ctx context.Context, opts interface{}) (interface{}, error) {
@@ -92,7 +81,7 @@ func (*DatabaseSuite) TestTransactionContext(c *check.C) {
})(context.Background(), "blah")
c.Check(ok, check.Equals, true)
c.Check(err, check.IsNil)
- tx, err := currenttx(savedctx)
+ tx, err := CurrentTx(savedctx)
c.Check(tx, check.IsNil)
c.Check(err, check.NotNil)
}
diff --git a/sdk/go/arvadostest/db.go b/sdk/go/arvadostest/db.go
new file mode 100644
index 000000000..41ecfacc4
--- /dev/null
+++ b/sdk/go/arvadostest/db.go
@@ -0,0 +1,33 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package arvadostest
+
+import (
+ "context"
+
+ "git.arvados.org/arvados.git/lib/ctrlctx"
+ "git.arvados.org/arvados.git/sdk/go/arvados"
+ "github.com/jmoiron/sqlx"
+ _ "github.com/lib/pq"
+ "gopkg.in/check.v1"
+)
+
+// DB returns a DB connection for the given cluster config.
+func DB(c *check.C, cluster *arvados.Cluster) *sqlx.DB {
+ db, err := sqlx.Open("postgres", cluster.PostgreSQL.Connection.String())
+ c.Assert(err, check.IsNil)
+ return db
+}
+
+// TransactionContext 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 TransactionContext(c *check.C, db *sqlx.DB) (ctx context.Context, rollback func()) {
+ tx, err := db.Beginx()
+ c.Assert(err, check.IsNil)
+ return ctrlctx.NewWithTransaction(context.Background(), tx), func() {
+ c.Check(tx.Rollback(), check.IsNil)
+ }
+}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list