This is an automated email from the ASF dual-hosted git repository. dangogh pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git
commit 4aa524ae519cfd1e262e0bd101fb7f3686e97262 Author: Dylan Volz <dylan_v...@comcast.com> AuthorDate: Fri Feb 9 10:55:30 2018 -0700 properly handle errors commiting db transactions --- traffic_ops/traffic_ops_golang/cdn/cdns.go | 42 +++++++++++++++------- .../traffic_ops_golang/dbhelpers/db_helpers.go | 1 + .../dbhelpers/db_helpers_test.go | 6 ++-- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/cdn/cdns.go b/traffic_ops/traffic_ops_golang/cdn/cdns.go index 7060bea..5513a23 100644 --- a/traffic_ops/traffic_ops_golang/cdn/cdns.go +++ b/traffic_ops/traffic_ops_golang/cdn/cdns.go @@ -128,16 +128,16 @@ FROM cdn c` //if so, it will return an errorType of DataConflict and the type should be appended to the //generic error message returned func (cdn *TOCDN) Update(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) { + rollbackTransaction := true tx, err := db.Beginx() defer func() { - if tx == nil { + if tx == nil || !rollbackTransaction { return } + err := tx.Rollback() if err != nil { - tx.Rollback() - return + log.Errorln(errors.New("rolling back transaction: " + err.Error())) } - tx.Commit() }() if err != nil { @@ -178,6 +178,12 @@ func (cdn *TOCDN) Update(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiError return fmt.Errorf("this update affected too many rows: %d", rowsAffected), tc.SystemError } } + err = tx.Commit() + if err != nil { + log.Errorln("Could not commit transaction: ", err) + return tc.DBError, tc.SystemError + } + rollbackTransaction = false return nil, tc.NoError } @@ -189,16 +195,16 @@ func (cdn *TOCDN) Update(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiError //The insert sql returns the id and lastUpdated values of the newly inserted cdn and have //to be added to the struct func (cdn *TOCDN) Insert(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) { + rollbackTransaction := true tx, err := db.Beginx() defer func() { - if tx == nil { + if tx == nil || !rollbackTransaction { return } + err := tx.Rollback() if err != nil { - tx.Rollback() - return + log.Errorln(errors.New("rolling back transaction: " + err.Error())) } - tx.Commit() }() if err != nil { @@ -241,22 +247,28 @@ func (cdn *TOCDN) Insert(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiError } cdn.SetID(id) cdn.LastUpdated = lastUpdated + err = tx.Commit() + if err != nil { + log.Errorln("Could not commit transaction: ", err) + return tc.DBError, tc.SystemError + } + rollbackTransaction = false return nil, tc.NoError } //The CDN implementation of the Deleter interface //all implementations of Deleter should use transactions and return the proper errorType func (cdn *TOCDN) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) { + rollbackTransaction := true tx, err := db.Beginx() defer func() { - if tx == nil { + if tx == nil || !rollbackTransaction { return } + err := tx.Rollback() if err != nil { - tx.Rollback() - return + log.Errorln(errors.New("rolling back transaction: " + err.Error())) } - tx.Commit() }() if err != nil { @@ -280,6 +292,12 @@ func (cdn *TOCDN) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiError return fmt.Errorf("this create affected too many rows: %d", rowsAffected), tc.SystemError } } + err = tx.Commit() + if err != nil { + log.Errorln("Could not commit transaction: ", err) + return tc.DBError, tc.SystemError + } + rollbackTransaction = false return nil, tc.NoError } diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index 24f0f7c..0c8774b 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -25,6 +25,7 @@ import ( "github.com/apache/incubator-trafficcontrol/lib/go-log" "github.com/apache/incubator-trafficcontrol/lib/go-tc" + "github.com/lib/pq" ) diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go index 4cc5729..5607dfc 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go @@ -35,7 +35,7 @@ func stripAllWhitespace(s string) string { } func TestBuildQuery(t *testing.T) { - v := map[string]string{"param1": "queryParamv1","param2": "queryParamv2"} + v := map[string]string{"param1": "queryParamv1", "param2": "queryParamv2"} selectStmt := `SELECT t.col1, @@ -45,8 +45,8 @@ FROM table t // Query Parameters to Database Query column mappings // see the fields mapped in the SQL query queryParamsToSQLCols := map[string]WhereColumnInfo{ - "param1": WhereColumnInfo{"t.col1",nil}, - "param2": WhereColumnInfo{"t.col2",nil}, + "param1": WhereColumnInfo{"t.col1", nil}, + "param2": WhereColumnInfo{"t.col2", nil}, } where, orderBy, queryValues, _ := BuildWhereAndOrderBy(v, queryParamsToSQLCols) query := selectStmt + where + orderBy -- To stop receiving notification emails like this one, please contact dang...@apache.org.