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

@@ -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 <group> <new name>", Specifier);
+						if (args.Parameters.Count != 3)
-						return;
+						{
-					}
+							args.Player.SendErrorMessage("Invalid syntax! Proper syntax: {0}group rename <group> <new name>", Specifier);
+							return;
+						}
 
-					string group = args.Parameters[1];
+						string group = args.Parameters[1];
-					string newName = args.Parameters[2];
+						string newName = args.Parameters[2];
-					try
+						try
-					{
+						{
-						string response = TShock.Groups.RenameGroup(group, newName);
+							string response = TShock.Groups.RenameGroup(group, newName);
-						args.Player.SendSuccessMessage(response);
+							args.Player.SendSuccessMessage(response);
+						}
+						catch (GroupManagerException ex)
+						{
+							args.Player.SendErrorMessage(ex.Message);
+						}
 					}
-					catch (GroupManagerException ex)
-					{
-						args.Player.SendErrorMessage(ex.Message);
-					}
-				}
 					#endregion
 					return;
 				case "del":

TShockAPI/DB/GroupManager.cs

@@ -34,6 +34,10 @@ namespace TShockAPI.DB
 		private IDbConnection database;
 		public readonly List<Group> groups = new List<Group>();
 
+		/// <summary>
+		/// Initializes a new instance of the <see cref="GroupManager"/> class with the specified database connection.
+		/// </summary>
+		/// <param name="db">The connection.</param>
 		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);
+				db.Open();
-				var newGroup = new Group(newName, oldGroup.Parent, oldGroup.ChatColor, oldGroup.Permissions)
+				using (var transaction = db.BeginTransaction())
 				{
-					Prefix = oldGroup.Prefix,
+					try
-					Suffix = oldGroup.Suffix
+					{
-				};
+						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);
+						var oldGroup = GetGroupByName(name);
-				groups.Add(newGroup);
+						var newGroup = new Group(newName, oldGroup.Parent, oldGroup.ChatColor, oldGroup.Permissions)
-				// 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);
+							Prefix = oldGroup.Prefix,
-				foreach (var group in groups.Where(g => g.Parent != null && g.Parent == oldGroup))
+							Suffix = oldGroup.Suffix
-				{
+						};
-					group.Parent = newGroup;
+						groups.Remove(oldGroup);
-				}
+						groups.Add(newGroup);
 
-				if (TShock.Config.DefaultGuestGroupName == oldGroup.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())
-					TShock.Config.DefaultGuestGroupName = newGroup.Name;
+						{
-					Group.DefaultGroup = newGroup;
+							command.CommandText = "UPDATE GroupList SET Parent = @0 WHERE Parent = @1";
-				}
+							command.AddParameter("@0", newName);
-				if (TShock.Config.DefaultRegistrationGroupName == oldGroup.Name)
+							command.AddParameter("@1", name);
-				{
+							command.ExecuteNonQuery();
-					TShock.Config.DefaultRegistrationGroupName = newGroup.Name;
+						}
-				}
+						foreach (var group in groups.Where(g => g.Parent != null && g.Parent == oldGroup))
+						{
+							group.Parent = newGroup;
+						}
 
-				TShock.Config.Write(FileTools.ConfigPath);
+						if (TShock.Config.DefaultGuestGroupName == oldGroup.Name)
-				database.Query("UPDATE Users SET Usergroup = @0 WHERE Usergroup = @1", newName, name);
+						{
-				foreach (var player in TShock.Players.Where(p => p?.Group == oldGroup))
+							TShock.Config.DefaultGuestGroupName = newGroup.Name;
-				{
+							Group.DefaultGroup = newGroup;
-					player.Group = 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
 		/// </summary>
 		/// <param name="name">The group's name.</param>
 		/// <param name="exceptions">Whether exceptions will be thrown in case something goes wrong.</param>
-		/// <returns></returns>
+		/// <returns>The response.</returns>
 		public String DeleteGroup(String name, bool exceptions = false)
 		{
 			if (!GroupExists(name))
@@ -305,7 +351,7 @@ namespace TShockAPI.DB
 		/// </summary>
 		/// <param name="name">The group name.</param>
 		/// <param name="permissions">The permission list.</param>
-		/// <returns></returns>
+		/// <returns>The response.</returns>
 		public String AddPermissions(String name, List<String> permissions)
 		{
 			if (!GroupExists(name))
@@ -328,7 +374,7 @@ namespace TShockAPI.DB
 		/// </summary>
 		/// <param name="name">The group name.</param>
 		/// <param name="permissions">The permission list.</param>
-		/// <returns></returns>
+		/// <returns>The response.</returns>
 		public String DeletePermissions(String name, List<String> permissions)
 		{
 			if (!GroupExists(name))