diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index e37655eb0b..6b572fe5ed 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -334,12 +334,7 @@ func (s SqlChannelStore) CreateIndexesIfNotExists() { // MigratePublicChannels initializes the PublicChannels table with data created before this version // of the Mattermost server kept it up-to-date. func (s SqlChannelStore) MigratePublicChannels() error { - transaction, err := s.GetMaster().Begin() - if err != nil { - return err - } - - if _, err := transaction.Exec(` + if _, err := s.GetMaster().Exec(` INSERT INTO PublicChannels (Id, DeleteAt, TeamId, DisplayName, Name, Header, Purpose) SELECT @@ -355,10 +350,6 @@ func (s SqlChannelStore) MigratePublicChannels() error { return err } - if err := transaction.Commit(); err != nil { - return err - } - return nil } @@ -443,16 +434,15 @@ func (s SqlChannelStore) Save(channel *model.Channel, maxChannelsPerTeam int64) result.Err = model.NewAppError("SqlChannelStore.Save", "store.sql_channel.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.saveChannelT(transaction, channel, maxChannelsPerTeam) if result.Err != nil { - transaction.Rollback() return } // Additionally propagate the write to the PublicChannels table. if err := s.upsertPublicChannelT(transaction, result.Data.(*model.Channel)); err != nil { - transaction.Rollback() result.Err = model.NewAppError("SqlChannelStore.Save", "store.sql_channel.save.upsert_public_channel.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -504,12 +494,12 @@ func (s SqlChannelStore) SaveDirectChannel(directchannel *model.Channel, member1 result.Err = model.NewAppError("SqlChannelStore.SaveDirectChannel", "store.sql_channel.save_direct_channel.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) directchannel.TeamId = "" channelResult := s.saveChannelT(transaction, directchannel, 0) if channelResult.Err != nil { - transaction.Rollback() result.Err = channelResult.Err result.Data = channelResult.Data return @@ -527,7 +517,6 @@ func (s SqlChannelStore) SaveDirectChannel(directchannel *model.Channel, member1 } if member1Result.Err != nil || member2Result.Err != nil { - transaction.Rollback() details := "" if member1Result.Err != nil { details += "Member1Err: " + member1Result.Err.Message @@ -599,16 +588,15 @@ func (s SqlChannelStore) Update(channel *model.Channel) store.StoreChannel { result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.updateChannelT(transaction, channel) if result.Err != nil { - transaction.Rollback() return } // Additionally propagate the write to the PublicChannels table. if err := s.upsertPublicChannelT(transaction, result.Data.(*model.Channel)); err != nil { - transaction.Rollback() result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.upsert_public_channel.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -786,10 +774,10 @@ func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) result.Err = model.NewAppError("SqlChannelStore.SetDeleteAt", "store.sql_channel.set_delete_at.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.setDeleteAtT(transaction, channelId, deleteAt, updateAt) if result.Err != nil { - transaction.Rollback() return } @@ -805,7 +793,6 @@ func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) "DeleteAt": deleteAt, "ChannelId": channelId, }); err != nil { - transaction.Rollback() result.Err = model.NewAppError("SqlChannelStore.SetDeleteAt", "store.sql_channel.set_delete_at.update_public_channel.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) return } @@ -837,10 +824,10 @@ func (s SqlChannelStore) PermanentDeleteByTeam(teamId string) store.StoreChannel result.Err = model.NewAppError("SqlChannelStore.PermanentDeleteByTeam", "store.sql_channel.permanent_delete_by_team.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.permanentDeleteByTeamtT(transaction, teamId) if result.Err != nil { - transaction.Rollback() return } @@ -853,7 +840,6 @@ func (s SqlChannelStore) PermanentDeleteByTeam(teamId string) store.StoreChannel `, map[string]interface{}{ "TeamId": teamId, }); err != nil { - transaction.Rollback() result.Err = model.NewAppError("SqlChannelStore.PermanentDeleteByTeamt", "store.sql_channel.permanent_delete_by_team.delete_public_channels.app_error", nil, "team_id="+teamId+", "+err.Error(), http.StatusInternalServerError) return } @@ -884,10 +870,10 @@ func (s SqlChannelStore) PermanentDelete(channelId string) store.StoreChannel { result.Err = model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.permanentDeleteT(transaction, channelId) if result.Err != nil { - transaction.Rollback() return } @@ -900,7 +886,6 @@ func (s SqlChannelStore) PermanentDelete(channelId string) store.StoreChannel { `, map[string]interface{}{ "ChannelId": channelId, }); err != nil { - transaction.Rollback() result.Err = model.NewAppError("SqlChannelStore.PermanentDelete", "store.sql_channel.permanent_delete.delete_public_channel.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) return } @@ -1316,10 +1301,10 @@ func (s SqlChannelStore) SaveMember(member *model.ChannelMember) store.StoreChan result.Err = model.NewAppError("SqlChannelStore.SaveMember", "store.sql_channel.save_member.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.saveMemberT(transaction, member, channel) if result.Err != nil { - transaction.Rollback() return } @@ -2319,6 +2304,7 @@ func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId result.Err = model.NewAppError("SqlChannelStore.MigrateChannelMembers", "store.sql_channel.migrate_channel_members.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) var channelMembers []channelMember if _, err := transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (:FromChannelId, :FromUserId) ORDER BY ChannelId, UserId LIMIT 100", map[string]interface{}{"FromChannelId": fromChannelId, "FromUserId": fromUserId}); err != nil { @@ -2352,10 +2338,6 @@ func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId member.Roles = strings.Join(newRoles, " ") if _, err := transaction.Update(&member); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlChannelStore.MigrateChannelMembers", "store.sql_channel.migrate_channel_members.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlChannelStore.MigrateChannelMembers", "store.sql_channel.migrate_channel_members.update.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -2363,10 +2345,6 @@ func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId } if err := transaction.Commit(); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlChannelStore.MigrateChannelMembers", "store.sql_channel.migrate_channel_members.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlChannelStore.MigrateChannelMembers", "store.sql_channel.migrate_channel_members.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -2385,10 +2363,10 @@ func (s SqlChannelStore) ResetAllChannelSchemes() store.StoreChannel { result.Err = model.NewAppError("SqlChannelStore.ResetAllChannelSchemes", "store.sql_channel.reset_all_channel_schemes.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.resetAllChannelSchemesT(transaction) if result.Err != nil { - transaction.Rollback() return } @@ -2427,15 +2405,13 @@ func (s SqlChannelStore) ClearAllCustomRoleAssignments() store.StoreChannel { var channelMembers []*channelMember if _, err := transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (:ChannelId, :UserId) ORDER BY ChannelId, UserId LIMIT 1000", map[string]interface{}{"ChannelId": lastChannelId, "UserId": lastUserId}); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } + finalizeTransaction(transaction) result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) return } if len(channelMembers) == 0 { + finalizeTransaction(transaction) break } @@ -2457,10 +2433,7 @@ func (s SqlChannelStore) ClearAllCustomRoleAssignments() store.StoreChannel { newRolesString := strings.Join(newRoles, " ") if newRolesString != member.Roles { if _, err := transaction.Exec("UPDATE ChannelMembers SET Roles = :Roles WHERE UserId = :UserId AND ChannelId = :ChannelId", map[string]interface{}{"Roles": newRolesString, "ChannelId": member.ChannelId, "UserId": member.UserId}); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } + finalizeTransaction(transaction) result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -2468,10 +2441,7 @@ func (s SqlChannelStore) ClearAllCustomRoleAssignments() store.StoreChannel { } if err := transaction.Commit(); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } + finalizeTransaction(transaction) result.Err = model.NewAppError("SqlChannelStore.ClearAllCustomRoleAssignments", "store.sql_channel.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -2486,10 +2456,10 @@ func (s SqlChannelStore) ResetLastPostAt() store.StoreChannel { result.Err = model.NewAppError("SqlChannelStore.ResetLastPostAt", "store.sql_channel.reset_last_post_at.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) *result = s.resetLastPostAtT(transaction) if result.Err != nil { - transaction.Rollback() return } diff --git a/store/sqlstore/oauth_store.go b/store/sqlstore/oauth_store.go index 0a9bd82666..dd8e4fc5ba 100644 --- a/store/sqlstore/oauth_store.go +++ b/store/sqlstore/oauth_store.go @@ -164,6 +164,7 @@ func (as SqlOAuthStore) DeleteApp(id string) store.StoreChannel { if err != nil { result.Err = model.NewAppError("SqlOAuthStore.DeleteApp", "store.sql_oauth.delete.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) if extrasResult := as.deleteApp(transaction, id); extrasResult.Err != nil { *result = extrasResult } @@ -173,10 +174,6 @@ func (as SqlOAuthStore) DeleteApp(id string) store.StoreChannel { // don't need to rollback here since the transaction is already closed result.Err = model.NewAppError("SqlOAuthStore.DeleteApp", "store.sql_oauth.delete.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) } - } else { - if err := transaction.Rollback(); err != nil { - result.Err = model.NewAppError("SqlOAuthStore.DeleteApp", "store.sql_oauth.delete.rollback_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - } } } }) diff --git a/store/sqlstore/preference_store.go b/store/sqlstore/preference_store.go index 5790f951ca..60fadfa281 100644 --- a/store/sqlstore/preference_store.go +++ b/store/sqlstore/preference_store.go @@ -61,6 +61,7 @@ func (s SqlPreferenceStore) Save(preferences *model.Preferences) store.StoreChan if err != nil { result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_preference.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) for _, preference := range *preferences { if upsertResult := s.save(transaction, &preference); upsertResult.Err != nil { *result = upsertResult @@ -75,10 +76,6 @@ func (s SqlPreferenceStore) Save(preferences *model.Preferences) store.StoreChan } else { result.Data = len(*preferences) } - } else { - if err := transaction.Rollback(); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_preference.save.rollback_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) - } } } }) diff --git a/store/sqlstore/role_supplier.go b/store/sqlstore/role_supplier.go index 19ef602eb2..f5ba2a35d3 100644 --- a/store/sqlstore/role_supplier.go +++ b/store/sqlstore/role_supplier.go @@ -11,7 +11,6 @@ import ( "strings" "github.com/mattermost/gorp" - "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" ) @@ -94,6 +93,7 @@ func (s *SqlSupplier) RoleSave(ctx context.Context, role *model.Role, hints ...s result.Err = model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return result } else { + defer finalizeTransaction(transaction) result = s.createRole(ctx, role, transaction, hints...) if result.Err != nil { diff --git a/store/sqlstore/scheme_supplier.go b/store/sqlstore/scheme_supplier.go index d58d64792a..b3ca7154b1 100644 --- a/store/sqlstore/scheme_supplier.go +++ b/store/sqlstore/scheme_supplier.go @@ -38,12 +38,13 @@ func (s *SqlSupplier) SchemeSave(ctx context.Context, scheme *model.Scheme, hint if transaction, err := s.GetMaster().Begin(); err != nil { result.Err = model.NewAppError("SqlSchemeStore.SaveScheme", "store.sql_scheme.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) result = s.createScheme(ctx, scheme, transaction, hints...) - if result.Err != nil { - transaction.Rollback() - } else if err := transaction.Commit(); err != nil { - result.Err = model.NewAppError("SqlSchemeStore.SchemeSave", "store.sql_scheme.save_scheme.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + if result.Err == nil { + if err := transaction.Commit(); err != nil { + result.Err = model.NewAppError("SqlSchemeStore.SchemeSave", "store.sql_scheme.save_scheme.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + } } } } else { diff --git a/store/sqlstore/supplier_reactions.go b/store/sqlstore/supplier_reactions.go index 9ec299adba..9095c7aa58 100644 --- a/store/sqlstore/supplier_reactions.go +++ b/store/sqlstore/supplier_reactions.go @@ -34,18 +34,16 @@ func (s *SqlSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction if transaction, err := s.GetMaster().Begin(); err != nil { result.Err = model.NewAppError("SqlReactionStore.Save", "store.sql_reaction.save.begin.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) err := saveReactionAndUpdatePost(transaction, reaction) if err != nil { - transaction.Rollback() - // We don't consider duplicated save calls as an error if !IsUniqueConstraintError(err, []string{"reactions_pkey", "PRIMARY"}) { result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.save.app_error", nil, err.Error(), http.StatusBadRequest) } } else { if err := transaction.Commit(); err != nil { - // don't need to rollback here since the transaction is already closed result.Err = model.NewAppError("SqlPreferenceStore.Save", "store.sql_reaction.save.commit.app_error", nil, err.Error(), http.StatusInternalServerError) } } @@ -64,14 +62,12 @@ func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reacti if transaction, err := s.GetMaster().Begin(); err != nil { result.Err = model.NewAppError("SqlReactionStore.Delete", "store.sql_reaction.delete.begin.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) err := deleteReactionAndUpdatePost(transaction, reaction) if err != nil { - transaction.Rollback() - result.Err = model.NewAppError("SqlPreferenceStore.Delete", "store.sql_reaction.delete.app_error", nil, err.Error(), http.StatusInternalServerError) } else if err := transaction.Commit(); err != nil { - // don't need to rollback here since the transaction is already closed result.Err = model.NewAppError("SqlPreferenceStore.Delete", "store.sql_reaction.delete.commit.app_error", nil, err.Error(), http.StatusInternalServerError) } else { result.Data = reaction diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 433b889f29..5a3fba27da 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -792,6 +792,7 @@ func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) s result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) var teamMembers []teamMember if _, err := transaction.Select(&teamMembers, "SELECT * from TeamMembers WHERE (TeamId, UserId) > (:FromTeamId, :FromUserId) ORDER BY TeamId, UserId LIMIT 100", map[string]interface{}{"FromTeamId": fromTeamId, "FromUserId": fromUserId}); err != nil { @@ -825,10 +826,6 @@ func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) s member.Roles = strings.Join(newRoles, " ") if _, err := transaction.Update(&member); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.update.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -836,10 +833,6 @@ func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) s } if err := transaction.Commit(); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlTeamStore.MigrateTeamMembers", "store.sql_team.migrate_team_members.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -873,13 +866,10 @@ func (s SqlTeamStore) ClearAllCustomRoleAssignments() store.StoreChannel { result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) var teamMembers []*teamMember if _, err := transaction.Select(&teamMembers, "SELECT * from TeamMembers WHERE (TeamId, UserId) > (:TeamId, :UserId) ORDER BY TeamId, UserId LIMIT 1000", map[string]interface{}{"TeamId": lastTeamId, "UserId": lastUserId}); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -906,10 +896,6 @@ func (s SqlTeamStore) ClearAllCustomRoleAssignments() store.StoreChannel { newRolesString := strings.Join(newRoles, " ") if newRolesString != member.Roles { if _, err := transaction.Exec("UPDATE TeamMembers SET Roles = :Roles WHERE UserId = :UserId AND TeamId = :TeamId", map[string]interface{}{"Roles": newRolesString, "TeamId": member.TeamId, "UserId": member.UserId}); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -917,10 +903,6 @@ func (s SqlTeamStore) ClearAllCustomRoleAssignments() store.StoreChannel { } if err := transaction.Commit(); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlTeamStore.ClearAllCustomRoleAssignments", "store.sql_team.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index 7c6a7b81cc..2356d498db 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -173,15 +173,18 @@ func UpgradeDatabaseToVersion33(sqlStore SqlStore) { if err != nil { themeMigrationFailed(err) } + defer finalizeTransaction(transaction) // increase size of Value column of Preferences table to match the size of the ThemeProps column if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { if _, err := transaction.Exec("ALTER TABLE Preferences ALTER COLUMN Value TYPE varchar(2000)"); err != nil { themeMigrationFailed(err) + return } } else if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL { if _, err := transaction.Exec("ALTER TABLE Preferences MODIFY Value text"); err != nil { themeMigrationFailed(err) + return } } @@ -196,15 +199,18 @@ func UpgradeDatabaseToVersion33(sqlStore SqlStore) { WHERE Users.ThemeProps != 'null'`, params); err != nil { themeMigrationFailed(err) + return } // delete old data if _, err := transaction.Exec("ALTER TABLE Users DROP COLUMN ThemeProps"); err != nil { themeMigrationFailed(err) + return } if err := transaction.Commit(); err != nil { themeMigrationFailed(err) + return } // rename solarized_* code themes to solarized-* to match client changes in 3.0 diff --git a/store/sqlstore/user_access_token_store.go b/store/sqlstore/user_access_token_store.go index 5df0870264..2e935f2eec 100644 --- a/store/sqlstore/user_access_token_store.go +++ b/store/sqlstore/user_access_token_store.go @@ -57,6 +57,7 @@ func (s SqlUserAccessTokenStore) Delete(tokenId string) store.StoreChannel { if err != nil { result.Err = model.NewAppError("SqlUserAccessTokenStore.Delete", "store.sql_user_access_token.delete.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) if extrasResult := s.deleteSessionsAndTokensById(transaction, tokenId); extrasResult.Err != nil { *result = extrasResult } @@ -66,10 +67,6 @@ func (s SqlUserAccessTokenStore) Delete(tokenId string) store.StoreChannel { // don't need to rollback here since the transaction is already closed result.Err = model.NewAppError("SqlUserAccessTokenStore.Delete", "store.sql_user_access_token.delete.app_error", nil, err.Error(), http.StatusInternalServerError) } - } else { - if err := transaction.Rollback(); err != nil { - result.Err = model.NewAppError("SqlUserAccessTokenStore.Delete", "store.sql_user_access_token.delete.app_error", nil, err.Error(), http.StatusInternalServerError) - } } } }) @@ -109,6 +106,7 @@ func (s SqlUserAccessTokenStore) DeleteAllForUser(userId string) store.StoreChan if err != nil { result.Err = model.NewAppError("SqlUserAccessTokenStore.DeleteAllForUser", "store.sql_user_access_token.delete.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) if extrasResult := s.deleteSessionsandTokensByUser(transaction, userId); extrasResult.Err != nil { *result = extrasResult } @@ -118,10 +116,6 @@ func (s SqlUserAccessTokenStore) DeleteAllForUser(userId string) store.StoreChan // don't need to rollback here since the transaction is already closed result.Err = model.NewAppError("SqlUserAccessTokenStore.DeleteAllForUser", "store.sql_user_access_token.delete.app_error", nil, err.Error(), http.StatusInternalServerError) } - } else { - if err := transaction.Rollback(); err != nil { - result.Err = model.NewAppError("SqlUserAccessTokenStore.DeleteAllForUser", "store.sql_user_access_token.delete.app_error", nil, err.Error(), http.StatusInternalServerError) - } } } }) @@ -247,6 +241,7 @@ func (s SqlUserAccessTokenStore) UpdateTokenDisable(tokenId string) store.StoreC if err != nil { result.Err = model.NewAppError("SqlUserAccessTokenStore.UpdateTokenDisable", "store.sql_user_access_token.update_token_disable.app_error", nil, err.Error(), http.StatusInternalServerError) } else { + defer finalizeTransaction(transaction) if extrasResult := s.deleteSessionsAndDisableToken(transaction, tokenId); extrasResult.Err != nil { *result = extrasResult } @@ -256,10 +251,6 @@ func (s SqlUserAccessTokenStore) UpdateTokenDisable(tokenId string) store.StoreC // don't need to rollback here since the transaction is already closed result.Err = model.NewAppError("SqlUserAccessTokenStore.UpdateTokenDisable", "store.sql_user_access_token.update_token_disable.app_error", nil, err.Error(), http.StatusInternalServerError) } - } else { - if err := transaction.Rollback(); err != nil { - result.Err = model.NewAppError("SqlUserAccessTokenStore.UpdateTokenDisable", "store.sql_user_access_token.update_token_disable.app_error", nil, err.Error(), http.StatusInternalServerError) - } } } }) diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 6c622f02d6..4486ae8b32 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1382,13 +1382,10 @@ func (us SqlUserStore) ClearAllCustomRoleAssignments() store.StoreChannel { result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } + defer finalizeTransaction(transaction) var users []*model.User if _, err := transaction.Select(&users, "SELECT * from Users WHERE Id > :Id ORDER BY Id LIMIT 1000", map[string]interface{}{"Id": lastUserId}); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.select.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -1414,10 +1411,6 @@ func (us SqlUserStore) ClearAllCustomRoleAssignments() store.StoreChannel { newRolesString := strings.Join(newRoles, " ") if newRolesString != user.Roles { if _, err := transaction.Exec("UPDATE Users SET Roles = :Roles WHERE Id = :Id", map[string]interface{}{"Roles": newRolesString, "Id": user.Id}); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.update.app_error", nil, err.Error(), http.StatusInternalServerError) return } @@ -1425,10 +1418,6 @@ func (us SqlUserStore) ClearAllCustomRoleAssignments() store.StoreChannel { } if err := transaction.Commit(); err != nil { - if err2 := transaction.Rollback(); err2 != nil { - result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.rollback_transaction.app_error", nil, err2.Error(), http.StatusInternalServerError) - return - } result.Err = model.NewAppError("SqlUserStore.ClearAllCustomRoleAssignments", "store.sql_user.clear_all_custom_role_assignments.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) return } diff --git a/store/sqlstore/utils.go b/store/sqlstore/utils.go index d5139da589..a5805b86d9 100644 --- a/store/sqlstore/utils.go +++ b/store/sqlstore/utils.go @@ -5,8 +5,12 @@ package sqlstore import ( "bytes" + "database/sql" "fmt" "strconv" + + "github.com/mattermost/gorp" + "github.com/mattermost/mattermost-server/mlog" ) // Converts a list of strings into a list of query parameters and a named parameter map that can @@ -26,3 +30,11 @@ func MapStringsToQueryParams(list []string, paramPrefix string) (string, map[str return fmt.Sprintf("(%v)", keys.String()), params } + +// finalizeTransaction ensures a transaction is closed after use, rolling back if not already committed. +func finalizeTransaction(transaction *gorp.Transaction) { + // Rollback returns sql.ErrTxDone if the transaction was already closed. + if err := transaction.Rollback(); err != nil && err != sql.ErrTxDone { + mlog.Error("Failed to rollback transaction", mlog.Err(err)) + } +}