model: Always pass *sql.Tx to CRUD methods

When nesting objects like steps into other objects like recipes it is
required to pass a *sql.Tx value to the CRUD methods of the inner
object to be able to roll back the whole transaction.

The top level object used to be responsible for the creation of this
*sql.Tx inside its CRUD methods.

This is now moved to the caller of the CRUD methods (here the HTTP
handler function). The advantage is that all CRUD methods now accept a
*sql.Tx as only argument which makes those methods more consistent.

5 files changed, 99 insertions, 56 deletions

diff --git a/controller/recipe.go b/controller/recipe.go
index 09f65a7..da58d35 100644
--- a/controller/recipe.go
+++ b/controller/recipe.go
@@ -18,7 +18,20 @@ func RecipeCreate(w http.ResponseWriter, r *http.Request) {
 	recipe.LastChanged = fmt.Sprint(time.Now().Unix())
 	recipe.Created = recipe.LastChanged
 
-	err := recipe.Create()
+	tx, err := model.NewTx()
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = recipe.Create(tx)
+	if err != nil {
+		model.Rollback(tx)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = tx.Commit()
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -48,7 +61,20 @@ func RecipeUpdate(w http.ResponseWriter, r *http.Request) {
 
 	recipe.LastChanged = fmt.Sprint(time.Now().Unix())
 
-	err = recipe.Update()
+	tx, err := model.NewTx()
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = recipe.Update(tx)
+	if err != nil {
+		model.Rollback(tx)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = tx.Commit()
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -61,7 +87,20 @@ func RecipeDelete(w http.ResponseWriter, r *http.Request) {
 	recipe := model.Recipe{}
 	recipe.Id = mux.Vars(r)[`id`]
 
-	err := recipe.Delete()
+	tx, err := model.NewTx()
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = recipe.Delete(tx)
+	if err != nil {
+		model.Rollback(tx)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = tx.Commit()
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
diff --git a/model/database.go b/model/database.go
index 7021181..3d8a0c4 100644
--- a/model/database.go
+++ b/model/database.go
@@ -52,12 +52,23 @@ func InitDatabase() {
 func InjectTestRecipes() {
 	recipes := RecipeTestData()
 
+	tx, err := NewTx()
+	if err != nil {
+		log.Fatalf("Failed to inject test recipes: %v\n", err)
+	}
+
 	for _, recipe := range recipes {
-		err := recipe.Create()
+		err = recipe.Create(tx)
 		if err != nil {
+			Rollback(tx)
 			log.Fatalf("Failed to inject test recipe: %v\n", err)
 		}
 	}
+
+	err = tx.Commit()
+	if err != nil {
+		log.Fatalf("Failed to inject test recipe: %v\n", err)
+	}
 }
 
 func CloseDatabase() {
@@ -69,7 +80,11 @@ func CloseDatabase() {
 	}
 }
 
-func rollback(tx *sql.Tx) {
+func NewTx() (*sql.Tx, error) {
+	return db.Begin()
+}
+
+func Rollback(tx *sql.Tx) {
 	err := tx.Rollback()
 	if err != nil {
 		log.Printf("Failed to rollback transaction: %v\n", err)
diff --git a/model/recipe.go b/model/recipe.go
index 6750426..b1ad781 100644
--- a/model/recipe.go
+++ b/model/recipe.go
@@ -23,12 +23,7 @@ func (r Recipe) String() string {
 	return string(b)
 }
 
-func (r *Recipe) Create() error {
-	tx, err := db.Begin()
-	if err != nil {
-		return err
-	}
-
+func (r *Recipe) Create(tx *sql.Tx) error {
 	cmd := `
 INSERT INTO recipes
     (title, portions, url, notes, created, last_changed)
@@ -39,13 +34,11 @@ VALUES
 	result, err := tx.Exec(cmd, r.Title, r.Portions, r.Url, r.Notes, r.Created,
 		r.LastChanged)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
 	id, err := result.LastInsertId()
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
@@ -53,19 +46,13 @@ VALUES
 
 	err = r.CreateSteps(tx)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
-	return tx.Commit()
+	return nil
 }
 
-func (r *Recipe) Read() error {
-	tx, err := db.Begin()
-	if err != nil {
-		return err
-	}
-
+func (r *Recipe) Read(tx *sql.Tx) error {
 	cmd := `
 SELECT id, title, portions, url, notes, created, last_changed
 FROM recipes
@@ -74,13 +61,11 @@ WHERE id = ?
 
 	rows, err := tx.Query(cmd, r.Id)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 	defer rows.Close()
 
 	if !rows.Next() {
-		rollback(tx)
 		return sql.ErrNoRows
 	}
 
@@ -94,31 +79,23 @@ WHERE id = ?
 		&r.LastChanged,
 	)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
 	err = r.ReadSteps(tx)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
-	return tx.Commit()
+	return nil
 }
 
-func (r *Recipe) Update() error {
-	tx, err := db.Begin()
+func (r *Recipe) Update(tx *sql.Tx) error {
+	err := r.UpdateSteps(tx)
 	if err != nil {
 		return err
 	}
 
-	err = r.UpdateSteps(tx)
-	if err != nil {
-		rollback(tx)
-		return err
-	}
-
 	query := `UPDATE
     recipes
 SET
@@ -133,32 +110,23 @@ WHERE
 	res, err := tx.Exec(query, r.Title, r.Portions, r.Url, r.Notes,
 		r.LastChanged, r.Id)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
 	affected, err := res.RowsAffected()
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 	if affected != 1 {
-		rollback(tx)
 		return fmt.Errorf("Recipe update affected %d rows instead of 1", affected)
 	}
 
-	return tx.Commit()
+	return nil
 }
 
-func (r *Recipe) Delete() error {
-	tx, err := db.Begin()
-	if err != nil {
-		return err
-	}
-
-	err = r.DeleteSteps(tx)
+func (r *Recipe) Delete(tx *sql.Tx) error {
+	err := r.DeleteSteps(tx)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
@@ -171,17 +139,15 @@ WHERE
 
 	result, err := tx.Exec(query, r.Id)
 	if err != nil {
-		rollback(tx)
 		return err
 	}
 
 	rows, err := result.RowsAffected()
 	if rows != 1 {
-		rollback(tx)
 		return errors.New("Recipe deletion did not affect exactly one row")
 	}
 
-	return tx.Commit()
+	return nil
 }
 
 func RecipeTestData() []Recipe {
diff --git a/model/recipe_test.go b/model/recipe_test.go
index 55041a6..b9a44e1 100644
--- a/model/recipe_test.go
+++ b/model/recipe_test.go
@@ -12,19 +12,24 @@ func TestRecipeCrud(t *testing.T) {
 	InitDatabase()
 	defer CloseDatabase()
 
+	tx, err := NewTx()
+	if err != nil {
+		t.Fatalf("Failed to inject test recipes: %v\n", err)
+	}
+
 	var original, readback, update, updated, deleted Recipe
 
 	recipes := RecipeTestData()
 	original = recipes[0]
 	update = recipes[1]
 
-	err := original.Create()
+	err = original.Create(tx)
 	if err != nil {
 		t.Fatalf("Failed to create test recipe in DB: %v\n", err)
 	}
 
 	readback.Id = original.Id
-	err = readback.Read()
+	err = readback.Read(tx)
 	if err != nil {
 		t.Fatalf("Failed to create test recipe in DB: %v\n", err)
 	}
@@ -36,13 +41,13 @@ func TestRecipeCrud(t *testing.T) {
 
 	update.Id = original.Id
 
-	err = update.Update()
+	err = update.Update(tx)
 	if err != nil {
 		t.Fatalf("Failed to update recipe: %v\n", err)
 	}
 
 	updated.Id = original.Id
-	err = updated.Read()
+	err = updated.Read(tx)
 	if err != nil {
 		t.Fatalf("Failed to read back updated recipe: %v\n", err)
 	}
@@ -56,14 +61,19 @@ func TestRecipeCrud(t *testing.T) {
 		t.Fatalf("Updated and original recipe match")
 	}
 
-	err = updated.Delete()
+	err = updated.Delete(tx)
 	if err != nil {
 		t.Fatalf("Failed to delete updated recipe: %v\n", err)
 	}
 
 	deleted.Id = updated.Id
-	err = deleted.Read()
+	err = deleted.Read(tx)
 	if err == nil {
 		t.Fatalf("Was able to read back deleted recipe")
 	}
+
+	err = tx.Commit()
+	if err != nil {
+		t.Fatalf("Unable to commit test transaction")
+	}
 }
diff --git a/view/recipe.go b/view/recipe.go
index 0b545d9..52b7a7e 100644
--- a/view/recipe.go
+++ b/view/recipe.go
@@ -12,7 +12,20 @@ func RecipeRead(w http.ResponseWriter, r *http.Request) {
 	recipe := model.Recipe{}
 	recipe.Id = mux.Vars(r)[`id`]
 
-	err := recipe.Read()
+	tx, err := model.NewTx()
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = recipe.Read(tx)
+	if err != nil {
+		model.Rollback(tx)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	err = tx.Commit()
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return