From 098363a9cc2a3bb5d13fffcdc70c4b4ce5768e2f Mon Sep 17 00:00:00 2001 From: stevenh Date: Thu, 16 Feb 2012 11:23:39 +0000 Subject: [PATCH] Fixed internal object and DB inconsitencies for modified default groups Fixed console errors for failed parent dependencies of default groups that already exist caused by fb11729547 consistency fixes Made GroupManager.groups readonly to discourage overwriting it. Could still do with better encapsulation to prevent consistency problems if modified externally. Added UpdateGroup method to GroupManager Refactored Group handling of permissions and chatcolor to prevent inconsistent data Notably:- * AddPermission now does the right thing when passed the string "!permission" * Converted ChatColor from method to getter / setter Added RemovePermission to Group Refactored GroupManager Permissions handling to ensure consistent data between DB and internal objects Removed redundent AddGroup method in preference to a default value on the main method Correct some tabs vs spaces Added ParentName helper to Group Fixed inverted parameters to error "Invalid parent {0} for group {1}" --- TShockAPI/DB/GroupManager.cs | 177 ++++++++++++++++++----------------- TShockAPI/Group.cs | 83 +++++++++++++--- TShockAPI/TShock.cs | 1 - 3 files changed, 162 insertions(+), 99 deletions(-) diff --git a/TShockAPI/DB/GroupManager.cs b/TShockAPI/DB/GroupManager.cs index 27fee59a..125a7bad 100644 --- a/TShockAPI/DB/GroupManager.cs +++ b/TShockAPI/DB/GroupManager.cs @@ -29,7 +29,7 @@ namespace TShockAPI.DB { private IDbConnection database; - public List groups = new List(); + public readonly List groups = new List(); public GroupManager(IDbConnection db) { @@ -49,14 +49,23 @@ namespace TShockAPI.DB : new MysqlQueryCreator()); creator.EnsureExists(table); - //Add default groups - AddGroup("guest", "canbuild,canregister,canlogin,canpartychat,cantalkinthird"); - AddGroup("default", "guest", "warp,canchangepassword"); - AddGroup("newadmin", "default", "kick,editspawn,reservedslot"); - AddGroup("admin", "newadmin", + // Load Permissions from the DB + LoadPermisions(); + + // Add default groups if they don't exist + AddDefaultGroup("guest", "", "canbuild,canregister,canlogin,canpartychat,cantalkinthird"); + AddDefaultGroup("default", "guest", "warp,canchangepassword"); + AddDefaultGroup("newadmin", "default", "kick,editspawn,reservedslot"); + AddDefaultGroup("admin", "newadmin", "ban,unban,whitelist,causeevents,spawnboss,spawnmob,managewarp,time,tp,pvpfun,kill,logs,immunetokick,tphere"); - AddGroup("trustedadmin", "admin", "maintenance,cfg,butcher,item,heal,immunetoban,usebanneditem,manageusers"); - AddGroup("vip", "default", "reservedslot"); + AddDefaultGroup("trustedadmin", "admin", "maintenance,cfg,butcher,item,heal,immunetoban,usebanneditem,manageusers"); + AddDefaultGroup("vip", "default", "reservedslot"); + } + + private void AddDefaultGroup(string name, string parent, string permissions) + { + if (!GroupExists(name)) + AddGroup(name, parent, permissions); } @@ -92,9 +101,8 @@ namespace TShockAPI.DB /// permissions /// chatcolor /// exceptions true indicates use exceptions for errors false otherwise - public String AddGroup(String name, string parentname, String permissions, String chatcolor, bool exceptions = false) + public String AddGroup(String name, string parentname, String permissions, String chatcolor = "255,255,255", bool exceptions = false) { - String message = ""; if (GroupExists(name)) { if (exceptions) @@ -103,13 +111,13 @@ namespace TShockAPI.DB } var group = new Group(name, null, chatcolor); - group.permissions.Add(permissions); + group.Permissions = permissions; if (!string.IsNullOrWhiteSpace(parentname)) { var parent = groups.FirstOrDefault(gp => gp.Name == parentname); if (parent == null) { - var error = "Invalid parent {0} for group {1}".SFormat(group.Name, parentname); + var error = "Invalid parent {0} for group {1}".SFormat(parentname, group.Name); if (exceptions) throw new GroupManagerException(error); Log.ConsoleError(error); @@ -123,13 +131,13 @@ namespace TShockAPI.DB : "INSERT IGNORE INTO GroupList SET GroupName=@0, Parent=@1, Commands=@2, ChatColor=@3"; if (database.Query(query, name, parentname, permissions, chatcolor) == 1) { - message = "Group " + name + " has been created successfully."; groups.Add(group); + return "Group " + name + " has been created successfully."; } else if (exceptions) throw new GroupManagerException("Failed to add group '" + name + "'"); - return message; + return ""; } public String AddGroup(String name, String permissions) @@ -137,14 +145,38 @@ namespace TShockAPI.DB return AddGroup(name, "", permissions, "255,255,255"); } - public String AddGroup(String name, string parent, String permissions) + /// + /// Updates a group including permissions + /// + /// name of the group to update + /// parent of group + /// permissions + /// chatcolor + public void UpdateGroup(string name, string parentname, string permissions, string chatcolor) { - return AddGroup(name, parent, permissions, "255,255,255"); + if (!GroupExists(name)) + throw new GroupNotExistException(name); + + Group parent = null; + if (!string.IsNullOrWhiteSpace(parentname)) + { + parent = groups.FirstOrDefault(gp => gp.Name == parentname); + if (null == parent) + throw new GroupManagerException("Invalid parent {0} for group {1}".SFormat(parentname, name)); + } + + // NOTE: we use newgroup.XYZ to ensure any validation is also persisted to the DB + var newgroup = new Group(name, parent, chatcolor, permissions); + string query = "UPDATE GroupList SET Parent=@0, Commands=@1, ChatColor=@2 WHERE GroupName=@3"; + if (database.Query(query, parentname, newgroup.Permissions, newgroup.ChatColor, name) != 1) + throw new GroupManagerException("Failed to update group '" + name + "'"); + + groups.Remove(TShock.Utils.GetGroup(name)); + groups.Add(newgroup); } public String DeleteGroup(String name, bool exceptions = false) { - String message = ""; if (!GroupExists(name)) { if (exceptions) @@ -154,60 +186,60 @@ namespace TShockAPI.DB if (database.Query("DELETE FROM GroupList WHERE GroupName=@0", name) == 1) { - message = "Group " + name + " has been deleted successfully."; groups.Remove(TShock.Utils.GetGroup(name)); + return "Group " + name + " has been deleted successfully."; } else if (exceptions) throw new GroupManagerException("Failed to delete group '" + name + "'"); - return message; + return ""; } public String AddPermissions(String name, List permissions) { - String message = ""; if (!GroupExists(name)) return "Error: Group doesn't exists."; var group = TShock.Utils.GetGroup(name); - //Add existing permissions (without duplicating) - permissions.AddRange(group.permissions.Where(s => !permissions.Contains(s))); + var oldperms = group.Permissions; // Store old permissions in case of error + permissions.ForEach(p => group.AddPermission(p)); + + if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", group.Permissions, name) == 1) + return "Group " + name + " has been modified successfully."; - if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", String.Join(",", permissions), name) != 0) - { - message = "Group " + name + " has been modified successfully."; - group.SetPermission(permissions); - } - return message; + // Restore old permissions so DB and internal object are in a consistent state + group.Permissions = oldperms; + return ""; } public String DeletePermissions(String name, List permissions) { - String message = ""; if (!GroupExists(name)) return "Error: Group doesn't exists."; var group = TShock.Utils.GetGroup(name); + var oldperms = group.Permissions; // Store old permissions in case of error + permissions.ForEach(p => group.RemovePermission(p)); - //Only get permissions that exist in the group. - var newperms = group.permissions.Except(permissions); - - if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", String.Join(",", newperms), name) != 0) - { - message = "Group " + name + " has been modified successfully."; - group.SetPermission(newperms.ToList()); - } - return message; + if (database.Query("UPDATE GroupList SET Commands=@0 WHERE GroupName=@1", group.Permissions, name) == 1) + return "Group " + name + " has been modified successfully."; + + // Restore old permissions so DB and internal object are in a consistent state + group.Permissions = oldperms; + return ""; } public void LoadPermisions() { - //Create a temporary list so if there is an error it doesn't override the currently loaded groups with broken groups. + // Create a temporary list so if there is an error it doesn't override the currently loaded groups with broken groups. var tempgroups = new List(); tempgroups.Add(new SuperAdminGroup()); if (groups == null || groups.Count < 2) - groups = tempgroups; + { + groups.Clear(); + groups.AddRange(tempgroups); + } try { @@ -216,34 +248,9 @@ namespace TShockAPI.DB { while (reader.Read()) { - string groupname = reader.Get("GroupName"); - var group = new Group(groupname); - + var group = new Group(reader.Get("GroupName"), null, reader.Get("ChatColor"), reader.Get("Commands")); group.Prefix = reader.Get("Prefix"); group.Suffix = reader.Get("Suffix"); - - //Inherit Given commands - String[] commands = reader.Get("Commands").Split(','); - foreach (var t in commands) - { - var str = t.Trim(); - if (str.StartsWith("!")) - { - group.NegatePermission(str.Substring(1)); - } - else - { - group.AddPermission(str); - } - } - String[] chatcolour = (reader.Get("ChatColor") ?? "").Split(','); - if (chatcolour.Length == 3) - { - byte.TryParse(chatcolour[0], out group.R); - byte.TryParse(chatcolour[1], out group.G); - byte.TryParse(chatcolour[2], out group.B); - } - groupsparents.Add(Tuple.Create(group, reader.Get("Parent"))); } } @@ -257,7 +264,7 @@ namespace TShockAPI.DB var parent = groupsparents.FirstOrDefault(gp => gp.Item1.Name == parentname); if (parent == null) { - Log.ConsoleError("Invalid parent {0} for group {1}".SFormat(group.Name, parentname)); + Log.ConsoleError("Invalid parent {0} for group {1}".SFormat(parentname, group.Name)); return; } group.Parent = parent.Item1; @@ -265,8 +272,8 @@ namespace TShockAPI.DB tempgroups.Add(group); } - - groups = tempgroups; + groups.Clear(); + groups.AddRange(tempgroups); } catch (Exception ex) { @@ -289,21 +296,21 @@ namespace TShockAPI.DB } } - [Serializable] - public class GroupExistsException : GroupManagerException - { - public GroupExistsException(string name) - : base("Group '" + name + "' already exists") - { - } - } + [Serializable] + public class GroupExistsException : GroupManagerException + { + public GroupExistsException(string name) + : base("Group '" + name + "' already exists") + { + } + } - [Serializable] - public class GroupNotExistException : GroupManagerException - { - public GroupNotExistException(string name) - : base("Group '" + name + "' does not exist") - { - } - } + [Serializable] + public class GroupNotExistException : GroupManagerException + { + public GroupNotExistException(string name) + : base("Group '" + name + "' does not exist") + { + } + } } diff --git a/TShockAPI/Group.cs b/TShockAPI/Group.cs index 16448677..b1a44a5e 100644 --- a/TShockAPI/Group.cs +++ b/TShockAPI/Group.cs @@ -30,23 +30,57 @@ namespace TShockAPI public int Order { get; set; } public string Prefix { get; set; } public string Suffix { get; set; } + public string ParentName { get { return (null == Parent) ? "" : Parent.Name; } } + public string ChatColor + { + get { return string.Format("{0}{1}{2}", R.ToString("X2"), G.ToString("X2"), B.ToString("X2")); } + set + { + if (null != value) + { + string[] parts = value.Split(','); + if (3 == parts.Length) + { + byte r, g, b; + if (byte.TryParse(parts[0], out r) && byte.TryParse(parts[1], out g) && byte.TryParse(parts[2], out b)) + { + R = r; + G = g; + B = b; + return; + } + } + } + } + } + + public string Permissions + { + get + { + List all = new List(permissions); + permissions.ForEach(p => all.Add("!" + p)); + return string.Join(",", all); + } + set + { + permissions.Clear(); + negatedpermissions.Clear(); + if (null != value) + value.Split(',').ForEach(p => AddPermission(p.Trim())); + } + } public byte R = 255; public byte G = 255; public byte B = 255; - public Group(string groupname, Group parentgroup = null, string chatcolor = "255,255,255") + public Group(string groupname, Group parentgroup = null, string chatcolor = "255,255,255", string permissions = null) { Name = groupname; Parent = parentgroup; - byte.TryParse(chatcolor.Split(',')[0], out R); - byte.TryParse(chatcolor.Split(',')[1], out G); - byte.TryParse(chatcolor.Split(',')[2], out B); - } - - public string ChatColor() - { - return string.Format("{0}{1}{2}", R.ToString("X2"), G.ToString("X2"), B.ToString("X2")); + ChatColor = chatcolor; + Permissions = permissions; } public virtual bool HasPermission(string permission) @@ -73,21 +107,44 @@ namespace TShockAPI public void NegatePermission(string permission) { - negatedpermissions.Add(permission); + // Avoid duplicates + if (!negatedpermissions.Contains(permission)) + { + negatedpermissions.Add(permission); + permissions.Remove(permission); // Ensure we don't have conflicting definitions for a permissions + } } public void AddPermission(string permission) { - permissions.Add(permission); + if (permission.StartsWith("!")) + { + NegatePermission(permission.Substring(1)); + return; + } + // Avoid duplicates + if (!permissions.Contains(permission)) + { + permissions.Add(permission); + negatedpermissions.Remove(permission); // Ensure we don't have conflicting definitions for a permissions + } } public void SetPermission(List permission) { permissions.Clear(); - foreach (string s in permission) + negatedpermissions.Clear(); + permission.ForEach(p => AddPermission(p)); + } + + public void RemovePermission(string permission) + { + if (permission.StartsWith("!")) { - permissions.Add(s); + negatedpermissions.Remove(permission.Substring(1)); + return; } + permissions.Remove(permission); } } diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs index c46c3ab5..2a7824ed 100755 --- a/TShockAPI/TShock.cs +++ b/TShockAPI/TShock.cs @@ -172,7 +172,6 @@ namespace TShockAPI Warps = new WarpManager(DB); Users = new UserManager(DB); Groups = new GroupManager(DB); - Groups.LoadPermisions(); Regions = new RegionManager(DB); Itembans = new ItemManager(DB); RememberedPos = new RemeberedPosManager(DB);