From da91b91b4bf32efdfd1946c419578a767b9b2de8 Mon Sep 17 00:00:00 2001
From: bergquist <carl.bergquist@gmail.com>
Date: Fri, 15 Jun 2018 20:49:14 +0200
Subject: [PATCH] transactions: start sessions and transactions at the same
 place

this make it possible for handler to use `withSession` when
transactions is not nedded and `inTransactionCtx` if its needed
without knowing who owns the session/transaction
---
 pkg/services/sqlstore/session.go           | 19 ++++++++++++++-----
 pkg/services/sqlstore/transactions.go      | 21 +++++++++------------
 pkg/services/sqlstore/transactions_test.go |  6 +-----
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go
index fdee2c76b0c..c85346231e4 100644
--- a/pkg/services/sqlstore/session.go
+++ b/pkg/services/sqlstore/session.go
@@ -22,21 +22,30 @@ func newSession() *DBSession {
 	return &DBSession{Session: x.NewSession()}
 }
 
-func startSession(ctx context.Context) *DBSession {
+func startSession(ctx context.Context, engine *xorm.Engine, beginTran bool) (*DBSession, error) {
 	value := ctx.Value(ContextSessionName)
 	var sess *DBSession
 	sess, ok := value.(*DBSession)
 
 	if !ok {
-		newSess := newSession()
-		return newSess
+		newSess := &DBSession{Session: engine.NewSession()}
+		if beginTran {
+			err := newSess.Begin()
+			if err != nil {
+				return nil, err
+			}
+		}
+		return newSess, nil
 	}
 
-	return sess
+	return sess, nil
 }
 
 func withDbSession(ctx context.Context, callback dbTransactionFunc) error {
-	sess := startSession(ctx)
+	sess, err := startSession(ctx, x, false)
+	if err != nil {
+		return err
+	}
 
 	return callback(sess)
 }
diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go
index f72b0bb8500..3e7634dc196 100644
--- a/pkg/services/sqlstore/transactions.go
+++ b/pkg/services/sqlstore/transactions.go
@@ -14,16 +14,16 @@ func (ss *SqlStore) InTransaction(ctx context.Context, fn func(ctx context.Conte
 }
 
 func (ss *SqlStore) inTransactionWithRetry(ctx context.Context, fn func(ctx context.Context) error, retry int) error {
-	sess := startSession(ctx)
-	defer sess.Close()
-
-	if err := sess.Begin(); err != nil {
+	sess, err := startSession(ctx, ss.engine, true)
+	if err != nil {
 		return err
 	}
 
+	defer sess.Close()
+
 	withValue := context.WithValue(ctx, ContextSessionName, sess)
 
-	err := fn(withValue)
+	err = fn(withValue)
 
 	// special handling of database locked errors for sqlite, then we can retry 3 times
 	if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 {
@@ -60,16 +60,13 @@ func inTransactionWithRetry(callback dbTransactionFunc, retry int) error {
 }
 
 func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, retry int) error {
-	var err error
-
-	sess := startSession(ctx)
-
-	defer sess.Close()
-
-	if err = sess.Begin(); err != nil {
+	sess, err := startSession(ctx, x, true)
+	if err != nil {
 		return err
 	}
 
+	defer sess.Close()
+
 	err = callback(sess)
 
 	// special handling of database locked errors for sqlite, then we can retry 3 times
diff --git a/pkg/services/sqlstore/transactions_test.go b/pkg/services/sqlstore/transactions_test.go
index 2575229aad5..937649921ba 100644
--- a/pkg/services/sqlstore/transactions_test.go
+++ b/pkg/services/sqlstore/transactions_test.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/models"
 
 	. "github.com/smartystreets/goconvey/convey"
@@ -18,11 +17,9 @@ type testQuery struct {
 var ProvokedError = errors.New("testing error.")
 
 func TestTransaction(t *testing.T) {
-	InitTestDB(t)
+	ss := InitTestDB(t)
 
 	Convey("InTransaction asdf asdf", t, func() {
-		ss := SqlStore{log: log.New("test-logger")}
-
 		cmd := &models.AddApiKeyCommand{Key: "secret-key", Name: "key", OrgId: 1}
 
 		err := AddApiKey(cmd)
@@ -50,7 +47,6 @@ func TestTransaction(t *testing.T) {
 				}
 
 				return ProvokedError
-
 			})
 
 			So(err, ShouldEqual, ProvokedError)