Merge pull request #2271 from DeathCradle/h/fix-typo

Fix ban system

CHANGELOG.md

@@ -16,7 +16,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin
 * This could be you!
 
 ## TShock 4.5.0.1
-* Fixed typo in conversion from old to new ban system for MySQL hosted ban databases. (@DeathCradle)
+* Fixed conversion from old to new ban system for MySQL hosted ban databases. (@DeathCradle)
 
 ## TShock 4.5.0
 * Updated OTAPI and TSAPI to Terraria 1.4.2.1. (@Stealownz, @DeathCradle)

TShockAPI/DB/BanManager.cs

@@ -37,18 +37,7 @@ namespace TShockAPI.DB
 		/// <summary>
 		/// Readonly dictionary of Bans, keyed on ban ticket number.
 		/// </summary>
-		public ReadOnlyDictionary<int, Ban> Bans
+		public ReadOnlyDictionary<int, Ban> Bans => new ReadOnlyDictionary<int, Ban>(_bans);
-		{
-			get
-			{
-				if (_bans == null)
-				{
-					_bans = RetrieveAllBans().ToDictionary(b => b.TicketNumber);
-				}
-
-				return new ReadOnlyDictionary<int, Ban>(_bans);
-			}
-		}
 
 		/// <summary>
 		/// Event invoked when a ban is checked for validity
@@ -93,12 +82,24 @@ namespace TShockAPI.DB
 				throw new Exception("Could not find a database library (probably Sqlite3.dll)");
 			}
 
+			EnsureBansCollection();
 			TryConvertBans();
 
 			OnBanValidate += BanValidateCheck;
 			OnBanPreAdd += BanAddedCheck;
 		}
 
+		/// <summary>
+		/// Ensures the <see cref="_bans"/> collection is ready to use.
+		/// </summary>
+		private void EnsureBansCollection()
+		{
+			if (_bans == null)
+			{
+				_bans = RetrieveAllBans().ToDictionary(b => b.TicketNumber);
+			}
+		}
+
 		/// <summary>
 		/// Converts bans from the old ban system to the new.
 		/// </summary>
@@ -116,6 +117,7 @@ namespace TShockAPI.DB
 
 			if (res != 0)
 			{
+				var bans = new List<BanPreAddEventArgs>();
 				using (var reader = database.QueryReader("SELECT * FROM Bans"))
 				{
 					while (reader.Read())
@@ -140,22 +142,46 @@ namespace TShockAPI.DB
 
 						if (!string.IsNullOrWhiteSpace(ip))
 						{
-							InsertBan($"{Identifier.IP}{ip}", reason, banningUser, start, end);
+							bans.Add(new BanPreAddEventArgs
+							{
+								Identifier = $"{Identifier.IP}{ip}",
+								Reason = reason,
+								BanningUser = banningUser,
+								BanDateTime = start,
+								ExpirationDateTime = end
+							});
 						}
 
 						if (!string.IsNullOrWhiteSpace(account))
 						{
-							InsertBan($"{Identifier.Account}{account}", reason, banningUser, start, end);
+							bans.Add(new BanPreAddEventArgs
+							{
+								Identifier = $"{Identifier.Account}{account}",
+								Reason = reason,
+								BanningUser = banningUser,
+								BanDateTime = start,
+								ExpirationDateTime = end
+							});
 						}
 
 						if (!string.IsNullOrWhiteSpace(uuid))
 						{
-							InsertBan($"{Identifier.UUID}{uuid}", reason, banningUser, start, end);
+							bans.Add(new BanPreAddEventArgs
+							{
+								Identifier = $"{Identifier.UUID}{uuid}",
+								Reason = reason,
+								BanningUser = banningUser,
+								BanDateTime = start,
+								ExpirationDateTime = end
+							});
 						}
 					}
 				}
 
-				database.Query("DROP TABLE 'Bans'");
+				foreach (var ban in bans)
+					InsertBan(ban);
+
+				database.Query("DROP TABLE Bans");
 			}
 		}
 
@@ -232,7 +258,7 @@ namespace TShockAPI.DB
 				args.Message = args.Valid ? null : "a current ban for this identifier already exists.";
 			}
 		}
-		
+
 		/// <summary>
 		/// Adds a new ban for the given identifier. Returns a Ban object if the ban was added, else null
 		/// </summary>
@@ -252,7 +278,16 @@ namespace TShockAPI.DB
 				BanDateTime = fromDate,
 				ExpirationDateTime = toDate
 			};
+			return InsertBan(args);
+		}
 
+		/// <summary>
+		/// Adds a new ban for the given data. Returns a Ban object if the ban was added, else null
+		/// </summary>
+		/// <param name="args">A predefined instance of <see cref="BanPreAddEventArgs"/></param>
+		/// <returns></returns>
+		public AddBanResult InsertBan(BanPreAddEventArgs args)
+		{
 			OnBanPreAdd?.Invoke(this, args);
 
 			if (!args.Valid)
@@ -265,21 +300,21 @@ namespace TShockAPI.DB
 
 			if (database.GetSqlType() == SqlType.Mysql)
 			{
-				query += "SELECT CAST(LAST_INSERT_ID() as INT);";
+				query += "SELECT LAST_INSERT_ID();";
 			}
 			else
 			{
 				query += "SELECT CAST(last_insert_rowid() as INT);";
 			}
 
-			int ticketId = database.QueryScalar<int>(query, identifier, reason, banningUser, fromDate.Ticks, toDate.Ticks);
+			int ticketId = database.QueryScalar<int>(query, args.Identifier, args.Reason, args.BanningUser, args.BanDateTime.Ticks, args.ExpirationDateTime.Ticks);
 
 			if (ticketId == 0)
 			{
 				return new AddBanResult { Message = "Database insert failed." };
 			}
 
-			Ban b = new Ban(ticketId, identifier, reason, banningUser, fromDate, toDate);
+			Ban b = new Ban(ticketId, args.Identifier, args.Reason, args.BanningUser, args.BanDateTime, args.ExpirationDateTime);
 			_bans.Add(ticketId, b);
 
 			OnBanPostAdd?.Invoke(this, new BanEventArgs { Ban = b });

TShockAPI/TSPlayer.cs

@@ -1637,7 +1637,7 @@ namespace TShockAPI
 			if (force)
 			{
 				TShock.Bans.InsertBan($"{Identifier.IP}{IP}", reason, adminUserName, DateTime.UtcNow, DateTime.MaxValue);
-				TShock.Bans.InsertBan($"{Identifier.IP}{UUID}", reason, adminUserName, DateTime.UtcNow, DateTime.MaxValue);
+				TShock.Bans.InsertBan($"{Identifier.UUID}{UUID}", reason, adminUserName, DateTime.UtcNow, DateTime.MaxValue);
 				if (Account != null)
 				{
 					TShock.Bans.InsertBan($"{Identifier.Account}{Account.Name}", reason, adminUserName, DateTime.UtcNow, DateTime.MaxValue);

TShockAPI/TShockAPI.csproj

@@ -70,7 +70,7 @@
     <Reference Include="Newtonsoft.Json, Version=10.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
       <HintPath>..\packages\Newtonsoft.Json.10.0.3\lib\net45\Newtonsoft.Json.dll</HintPath>
     </Reference>
-    <Reference Include="OTAPI=">
+    <Reference Include="OTAPI">
       <SpecificVersion>False</SpecificVersion>
       <HintPath Condition="Exists('..\TerrariaServerAPI\TerrariaServerAPI\bin\$(ConfigurationName)\OTAPI.dll')">..\TerrariaServerAPI\TerrariaServerAPI\bin\$(ConfigurationName)\OTAPI.dll</HintPath>
     </Reference>