From 08e182f59e1f4fc679c8535152239ad4291fceeb Mon Sep 17 00:00:00 2001 From: ProfessorXZ Date: Sat, 23 Sep 2017 22:41:41 +0200 Subject: [PATCH] All GroupManager.RenameGroup() database calls are now done in a transaction As pointed out by @hakusaro, in order to prevent any damage during the process all database calls need to be done in a transaction. Transactions allow us to rollback from a pending state in case something goes wrong. --- TShockAPI/Commands.cs | 32 +++++----- TShockAPI/DB/GroupManager.cs | 110 +++++++++++++++++++++++++---------- 2 files changed, 94 insertions(+), 48 deletions(-) diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs index 6fd3ca93..c171c1e0 100644 --- a/TShockAPI/Commands.cs +++ b/TShockAPI/Commands.cs @@ -3077,25 +3077,25 @@ namespace TShockAPI return; case "rename": #region Rename group - { - if (args.Parameters.Count != 3) { - args.Player.SendErrorMessage("Invalid syntax! Proper syntax: {0}group rename ", Specifier); - return; - } + if (args.Parameters.Count != 3) + { + args.Player.SendErrorMessage("Invalid syntax! Proper syntax: {0}group rename ", Specifier); + return; + } - string group = args.Parameters[1]; - string newName = args.Parameters[2]; - try - { - string response = TShock.Groups.RenameGroup(group, newName); - args.Player.SendSuccessMessage(response); + string group = args.Parameters[1]; + string newName = args.Parameters[2]; + try + { + string response = TShock.Groups.RenameGroup(group, newName); + args.Player.SendSuccessMessage(response); + } + catch (GroupManagerException ex) + { + args.Player.SendErrorMessage(ex.Message); + } } - catch (GroupManagerException ex) - { - args.Player.SendErrorMessage(ex.Message); - } - } #endregion return; case "del": diff --git a/TShockAPI/DB/GroupManager.cs b/TShockAPI/DB/GroupManager.cs index 0332379f..78596083 100644 --- a/TShockAPI/DB/GroupManager.cs +++ b/TShockAPI/DB/GroupManager.cs @@ -34,6 +34,10 @@ namespace TShockAPI.DB private IDbConnection database; public readonly List groups = new List(); + /// + /// Initializes a new instance of the class with the specified database connection. + /// + /// The connection. public GroupManager(IDbConnection db) { database = db; @@ -234,41 +238,83 @@ namespace TShockAPI.DB throw new GroupExistsException(newName); } - if (database.Query("UPDATE GroupList SET GroupName = @0 WHERE GroupName = @1", newName, name) == 1) + using (var db = database.CloneEx()) { - var oldGroup = GetGroupByName(name); - var newGroup = new Group(newName, oldGroup.Parent, oldGroup.ChatColor, oldGroup.Permissions) + db.Open(); + using (var transaction = db.BeginTransaction()) { - Prefix = oldGroup.Prefix, - Suffix = oldGroup.Suffix - }; + try + { + using (var command = db.CreateCommand()) + { + command.CommandText = "UPDATE GroupList SET GroupName = @0 WHERE GroupName = @1"; + command.AddParameter("@0", newName); + command.AddParameter("@1", name); + command.ExecuteNonQuery(); + } - groups.Remove(oldGroup); - groups.Add(newGroup); - // We need to check if the old group has been referenced as a parent and update those references accordingly - database.Query("UPDATE GroupList SET Parent = @0 WHERE Parent = @1", newName, name); - foreach (var group in groups.Where(g => g.Parent != null && g.Parent == oldGroup)) - { - group.Parent = newGroup; - } + var oldGroup = GetGroupByName(name); + var newGroup = new Group(newName, oldGroup.Parent, oldGroup.ChatColor, oldGroup.Permissions) + { + Prefix = oldGroup.Prefix, + Suffix = oldGroup.Suffix + }; + groups.Remove(oldGroup); + groups.Add(newGroup); - if (TShock.Config.DefaultGuestGroupName == oldGroup.Name) - { - TShock.Config.DefaultGuestGroupName = newGroup.Name; - Group.DefaultGroup = newGroup; - } - if (TShock.Config.DefaultRegistrationGroupName == oldGroup.Name) - { - TShock.Config.DefaultRegistrationGroupName = newGroup.Name; - } + // We need to check if the old group has been referenced as a parent and update those references accordingly + using (var command = db.CreateCommand()) + { + command.CommandText = "UPDATE GroupList SET Parent = @0 WHERE Parent = @1"; + command.AddParameter("@0", newName); + command.AddParameter("@1", name); + command.ExecuteNonQuery(); + } + foreach (var group in groups.Where(g => g.Parent != null && g.Parent == oldGroup)) + { + group.Parent = newGroup; + } - TShock.Config.Write(FileTools.ConfigPath); - database.Query("UPDATE Users SET Usergroup = @0 WHERE Usergroup = @1", newName, name); - foreach (var player in TShock.Players.Where(p => p?.Group == oldGroup)) - { - player.Group = newGroup; + if (TShock.Config.DefaultGuestGroupName == oldGroup.Name) + { + TShock.Config.DefaultGuestGroupName = newGroup.Name; + Group.DefaultGroup = newGroup; + } + if (TShock.Config.DefaultRegistrationGroupName == oldGroup.Name) + { + TShock.Config.DefaultRegistrationGroupName = newGroup.Name; + } + TShock.Config.Write(FileTools.ConfigPath); + + // We also need to check if any users belong to the old group and automatically apply changes + using (var command = db.CreateCommand()) + { + command.CommandText = "UPDATE Users SET Usergroup = @0 WHERE Usergroup = @1"; + command.AddParameter("@0", newName); + command.AddParameter("@1", name); + command.ExecuteNonQuery(); + } + foreach (var player in TShock.Players.Where(p => p?.Group == oldGroup)) + { + player.Group = newGroup; + } + + transaction.Commit(); + return $"Group \"{name}\" has been renamed to \"{newName}\"."; + } + catch (Exception ex) + { + TShock.Log.Error($"An exception has occured during database transaction: {ex.Message}"); + try + { + transaction.Rollback(); + } + catch (Exception rollbackEx) + { + TShock.Log.Error($"An exception has occured during database rollback: {rollbackEx.Message}"); + } + } } - return $"Group \"{name}\" has been renamed to \"{newName}\"."; } throw new GroupManagerException($"Failed to rename group \"{name}\"."); @@ -279,7 +325,7 @@ namespace TShockAPI.DB /// /// The group's name. /// Whether exceptions will be thrown in case something goes wrong. - /// + /// The response. public String DeleteGroup(String name, bool exceptions = false) { if (!GroupExists(name)) @@ -305,7 +351,7 @@ namespace TShockAPI.DB /// /// The group name. /// The permission list. - /// + /// The response. public String AddPermissions(String name, List permissions) { if (!GroupExists(name)) @@ -328,7 +374,7 @@ namespace TShockAPI.DB /// /// The group name. /// The permission list. - /// + /// The response. public String DeletePermissions(String name, List permissions) { if (!GroupExists(name))