fix(): transactions were not really working

Signed-off-by: Geoffrey Bauduin <geoffrey.bauduin@corp.ovh.com>

Author: geoffreybauduin

db.go

@@ -22,15 +22,16 @@ type DB interface {
 
 type db struct {
 	zesty.DB
+	dbmap        *gorp.DbMap
 	system       DMS
 	executorHook ExecutorHook
 }
 
-func (d db) System() DMS {
+func (d *db) System() DMS {
 	return d.system
 }
 
-func (d db) ExecutorHook() ExecutorHook {
+func (d *db) ExecutorHook() ExecutorHook {
 	if d.executorHook == nil {
 		return &DefaultExecutorHook{}
 	}
@@ -56,6 +57,15 @@ type DatabaseConfiguration struct {
 	ExecutorHook ExecutorHook
 }
 
+// GetDB returns a database object from its name
+func GetDB(name string) (DB, error) {
+	db, ok := registry[name]
+	if !ok {
+		return nil, errors.NotFoundf("database %s", name)
+	}
+	return db, nil
+}
+
 // RegisterDB creates a new database with configuration
 func RegisterDB(config *DatabaseConfiguration) error {
 	dblock.Lock()
@@ -86,7 +96,7 @@ func RegisterDB(config *DatabaseConfiguration) error {
 		return err
 	}
 
-	registry[config.Name] = db{
+	registry[config.Name] = &db{
 		DB:           dbHandler,
 		system:       config.System,
 		executorHook: config.ExecutorHook,

db_test.go

@@ -79,9 +79,11 @@ func TestDb_System(t *testing.T) {
 		AutoCreateTables: true,
 	})
 	assert.Nil(t, err)
-	dbp, err := yaorm.NewDBProvider(context.TODO(), "test")
+	_, err = yaorm.NewDBProvider(context.TODO(), "test")
 	assert.Nil(t, err)
-	assert.Equal(t, yaorm.DatabaseSqlite3, dbp.DB().(yaorm.DB).System())
+	db, err := yaorm.GetDB("test")
+	assert.Nil(t, err)
+	assert.Equal(t, yaorm.DatabaseSqlite3, db.System())
 }
 
 func TestDb_ExecutorHook(t *testing.T) {
@@ -96,9 +98,11 @@ func TestDb_ExecutorHook(t *testing.T) {
 		AutoCreateTables: true,
 	})
 	assert.Nil(t, err)
-	dbp, err := yaorm.NewDBProvider(context.TODO(), "test")
+	_, err = yaorm.NewDBProvider(context.TODO(), "test")
+	assert.Nil(t, err)
+	db, err := yaorm.GetDB("test")
 	assert.Nil(t, err)
-	inst := dbp.DB().(yaorm.DB).ExecutorHook()
+	inst := db.ExecutorHook()
 	assert.NotNil(t, inst)
 	assert.IsType(t, inst, &yaorm.DefaultExecutorHook{})
 }
@@ -126,9 +130,11 @@ func TestDb_ExecutorHook_Custom(t *testing.T) {
 		ExecutorHook:     customExecutorHook{},
 	})
 	assert.Nil(t, err)
-	dbp, err := yaorm.NewDBProvider(context.TODO(), "test")
+	_, err = yaorm.NewDBProvider(context.TODO(), "test")
+	assert.Nil(t, err)
+	db, err := yaorm.GetDB("test")
 	assert.Nil(t, err)
-	inst := dbp.DB().(yaorm.DB).ExecutorHook()
+	inst := db.ExecutorHook()
 	assert.NotNil(t, inst)
 	assert.IsType(t, inst, &customExecutorHook{})
 }

dbprovider.go

@@ -46,8 +46,14 @@ func NewDBProvider(ctx context.Context, name string) (DBProvider, error) {
 
 // DB returns a SQL Executor interface
 func (dbp *dbprovider) DB() gorp.SqlExecutor {
-	db := registry[dbp.name]
-	return &SqlExecutor{DB: db, ctx: dbp.Context(), dbp: dbp}
+	dbRetrieved := dbp.getDb()
+	dbUsed := dbp.DBProvider.DB()
+	return &SqlExecutor{
+		SqlExecutor: dbUsed,
+		db:          dbRetrieved,
+		ctx:         dbp.Context(),
+		dbp:         dbp,
+	}
 }
 
 // EscapeValue escapes the value sent according to the dialect
@@ -70,10 +76,9 @@ func (dbp *dbprovider) getDb() DB {
 }
 
 func (dbp *dbprovider) getDialect() gorp.Dialect {
-	v := tools.GetNonPtrValue(dbp.DB())
-	dbField := tools.GetNonPtrValue(v.FieldByName("DB").Interface()).FieldByName("DB")
-	realValue := tools.GetNonPtrValue(dbField.Interface())
-	field := realValue.FieldByName("DbMap")
+	v := tools.GetNonPtrValue(dbp.getDb())
+	dbField := tools.GetNonPtrValue(v.FieldByName("DB").Interface())
+	field := dbField.FieldByName("DbMap")
 	s := field.Interface().(*gorp.DbMap)
 	return s.Dialect
 }

dbprovider_test.go

@@ -0,0 +1,84 @@
+package yaorm_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/geoffreybauduin/yaorm"
+	"github.com/geoffreybauduin/yaorm/testdata"
+	"github.com/geoffreybauduin/yaorm/yaormfilter"
+	"github.com/juju/errors"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDbprovider_Tx(t *testing.T) {
+	killDb, err := testdata.SetupTestDatabase("test")
+	defer killDb()
+	assert.Nil(t, err)
+	dbp, err := yaorm.NewDBProvider(context.TODO(), "test")
+	assert.Nil(t, err)
+	err = dbp.Tx()
+	assert.Nil(t, err)
+	m := &testdata.Category{Name: "category"}
+	m.SetDBP(dbp)
+	assert.Equal(t, int64(0), m.ID)
+	err = yaorm.GenericInsert(m)
+	assert.Nil(t, err)
+	dbp2, err := yaorm.NewDBProvider(context.TODO(), "test")
+	assert.Nil(t, err)
+
+	// does not exist outside of tx
+	_, err = yaorm.GenericSelectOne(dbp2, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.NotNil(t, err)
+	assert.True(t, errors.IsNotFound(err))
+
+	// exists in tx
+	_, err = yaorm.GenericSelectOne(dbp, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.Nil(t, err)
+
+	err = dbp.Commit()
+	assert.Nil(t, err)
+
+	// now it exists outside of tx
+	_, err = yaorm.GenericSelectOne(dbp2, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.Nil(t, err)
+}
+
+func TestDbprovider_Rollback(t *testing.T) {
+	killDb, err := testdata.SetupTestDatabase("test")
+	defer killDb()
+	assert.Nil(t, err)
+	dbp, err := yaorm.NewDBProvider(context.TODO(), "test")
+	assert.Nil(t, err)
+	err = dbp.Tx()
+	assert.Nil(t, err)
+	m := &testdata.Category{Name: "category"}
+	m.SetDBP(dbp)
+	assert.Equal(t, int64(0), m.ID)
+	err = yaorm.GenericInsert(m)
+	assert.Nil(t, err)
+	dbp2, err := yaorm.NewDBProvider(context.TODO(), "test")
+	assert.Nil(t, err)
+
+	// does not exist outside of tx
+	_, err = yaorm.GenericSelectOne(dbp2, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.NotNil(t, err)
+	assert.True(t, errors.IsNotFound(err))
+
+	// exists in tx
+	_, err = yaorm.GenericSelectOne(dbp, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.Nil(t, err)
+
+	err = dbp.Rollback()
+	assert.Nil(t, err)
+
+	// still does not exist outside of tx
+	_, err = yaorm.GenericSelectOne(dbp2, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.NotNil(t, err)
+	assert.True(t, errors.IsNotFound(err))
+
+	// inside old tx it does not exist either
+	_, err = yaorm.GenericSelectOne(dbp, testdata.NewCategoryFilter().ID(yaormfilter.Equals(m.ID)))
+	assert.NotNil(t, err)
+	assert.True(t, errors.IsNotFound(err))
+}

requestbuilder.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"reflect"
 
-
 	"github.com/geoffreybauduin/yaorm/_vendor/github.com/lann/squirrel"
 	"github.com/geoffreybauduin/yaorm/tools"
 )

sqlexecutor.go

@@ -1,17 +1,22 @@
 package yaorm
 
-import "context"
+import (
+	"context"
+
+	"github.com/go-gorp/gorp"
+)
 
 type SqlExecutor struct {
-	DB
+	gorp.SqlExecutor
+	db  DB
 	ctx context.Context
 	dbp DBProvider
 }
 
 func (e *SqlExecutor) SelectOne(holder interface{}, query string, args ...interface{}) error {
-	hook := e.ExecutorHook()
+	hook := e.db.ExecutorHook()
 	hook.BeforeSelectOne(e.ctx, query, args...)
-	err := e.DB.SelectOne(holder, query, args...)
+	err := e.SqlExecutor.SelectOne(holder, query, args...)
 	hook.AfterSelectOne(e.ctx, query, args...)
 	return err
 }
@@ -22,15 +27,15 @@ type queryArgs struct {
 }
 
 func (e *SqlExecutor) Select(i interface{}, query string, args ...interface{}) ([]interface{}, error) {
-	hook := e.ExecutorHook()
+	hook := e.db.ExecutorHook()
 	hook.BeforeSelect(e.ctx, query, args...)
-	v, err := e.DB.Select(i, query, args...)
+	v, err := e.SqlExecutor.Select(i, query, args...)
 	hook.AfterSelect(e.ctx, query, args...)
 	return v, err
 }
 
 func (e *SqlExecutor) Insert(list ...interface{}) error {
-	hook := e.ExecutorHook()
+	hook := e.db.ExecutorHook()
 	for _, item := range list {
 		var qArg queryArgs
 		builder, err := buildInsert(e.dbp, item.(Model))
@@ -42,7 +47,7 @@ func (e *SqlExecutor) Insert(list ...interface{}) error {
 			return err
 		}
 		hook.BeforeInsert(e.ctx, qArg.query, qArg.args...)
-		err = e.DB.Insert(item)
+		err = e.SqlExecutor.Insert(item)
 		if err != nil {
 			return err
 		}
@@ -52,7 +57,7 @@ func (e *SqlExecutor) Insert(list ...interface{}) error {
 }
 
 func (e *SqlExecutor) Update(list ...interface{}) (int64, error) {
-	hook := e.ExecutorHook()
+	hook := e.db.ExecutorHook()
 	var rv int64
 	for _, item := range list {
 		var qArg queryArgs
@@ -65,7 +70,7 @@ func (e *SqlExecutor) Update(list ...interface{}) (int64, error) {
 			return rv, err
 		}
 		hook.BeforeUpdate(e.ctx, qArg.query, qArg.args...)
-		v, err := e.DB.Update(item)
+		v, err := e.SqlExecutor.Update(item)
 		if err != nil {
 			return rv, err
 		}
@@ -76,7 +81,7 @@ func (e *SqlExecutor) Update(list ...interface{}) (int64, error) {
 }
 
 func (e *SqlExecutor) Delete(list ...interface{}) (int64, error) {
-	hook := e.ExecutorHook()
+	hook := e.db.ExecutorHook()
 	var rv int64
 	for _, item := range list {
 		var qArg queryArgs
@@ -89,7 +94,7 @@ func (e *SqlExecutor) Delete(list ...interface{}) (int64, error) {
 			return rv, err
 		}
 		hook.BeforeDelete(e.ctx, qArg.query, qArg.args...)
-		v, err := e.DB.Delete(item)
+		v, err := e.SqlExecutor.Delete(item)
 		if err != nil {
 			return rv, err
 		}