Merge pull request #695 from tgstation/694-FixNullListException

Some Chat Bot API improvements

docs/API.dox

@@ -232,26 +232,26 @@ I DELETE "/Job/{JobId}" => OK
 
 @subsection api_chat Chat Bots
 
-Each chat bot is represented by a @ref Tgstation.Server.Api.Models.ChatSettings object
+Each chat bot is represented by a @ref Tgstation.Server.Api.Models.ChatBot object
 
 Chat bots can be created/updated/deleted with the following requests respectively
 
-I PUT "/Chat" @ref Tgstation.Server.Api.Models.ChatSettings => Tgstation.Server.Api.Models.ChatSettings
-I POST "/Chat" @ref Tgstation.Server.Api.Models.ChatSettings => Tgstation.Server.Api.Models.ChatSettings
-I DELETE "/Chat/{ChatSettingsId}" => OK
+I PUT "/Chat" @ref Tgstation.Server.Api.Models.ChatBot => Tgstation.Server.Api.Models.ChatBot
+I POST "/Chat" @ref Tgstation.Server.Api.Models.ChatBot => Tgstation.Server.Api.Models.ChatBot
+I DELETE "/Chat/{ChatBotId}" => OK
 
-The @ref Tgstation.Server.Api.Models.Internal.ChatSettings.ConnectionString must differ based on what kind of chat bot you wish to create
+The @ref Tgstation.Server.Api.Models.Internal.ChatBot.ConnectionString must differ based on what kind of chat bot you wish to create. Each @ref Tgstation.Server.Api.Models.ChatProvider has a @ref Tgstation.Server.Api.Models.Internal.ChatConnectionStringBuilder that dictates how to form it
 
-For IRC chat bots it should be in the following format:
-`"<Server URL or IP address>;<Server Port>;<Bot nickname>;<1 to use SSL, 0 otherwise>[;<`The @ref Tgstation.Server.Api.Models.IrcPasswordType`;<The password>]"`
+For IRC chat bots see @ref Tgstation.Server.Api.Models.IrcConnectionStringBuilder
+For Discord chat bots see @ref Tgstation.Server.Api.Models.DiscordConnectionStringBuilder
 
 For Discord chat bots it should be the <a href="https://discordapp.com/developers/docs/topics/oauth2#bots">bot's Token</a>
 
 A specific bot's settings may be retrieved with:
 
-I GET "/Chat/{ChatSettingsId}" => @ref Tgstation.Server.Api.Models.ChatSettings
+I GET "/Chat/{ChatBotId}" => @ref Tgstation.Server.Api.Models.ChatBot
 
-Also note that if the @ref Tgstation.Server.Api.Models.ChatSettings.Channels is present in a POST request, the list will fully replace any active channels
+Also note that if the @ref Tgstation.Server.Api.Models.ChatBot.Channels is present in a POST request, the list will fully replace any active channels
 
 @subsection api_byond Byond Version Management
 

src/Tgstation.Server.Api/Models/ChatBot.cs

@@ -15,15 +15,15 @@ namespace Tgstation.Server.Api.Models
 		/// <summary>
 		/// Validates <see cref="Channels"/> are correct for the <see cref="Internal.ChatBot.Provider"/>
 		/// </summary>
-		/// <returns></returns>
+		/// <returns><see langword="true"/> if the <see cref="Channels"/> are valid for the <see cref="Internal.ChatBot.Provider"/>, <see langword="false"/> otherwise</returns>
 		public bool ValidateProviderChannelTypes()
 		{
 			switch (Provider)
 			{
 				case ChatProvider.Discord:
-					return Channels.Select(x => x.DiscordChannelId.HasValue && x.IrcChannel == null).All(x => x);
+					return Channels?.Select(x => x.DiscordChannelId.HasValue && x.IrcChannel == null).All(x => x) ?? true;
 				case ChatProvider.Irc:
-					return Channels.Select(x => !x.DiscordChannelId.HasValue && x.IrcChannel != null).All(x => x);
+					return Channels?.Select(x => !x.DiscordChannelId.HasValue && x.IrcChannel != null).All(x => x) ?? true;
 				default:
 					throw new InvalidOperationException("Invalid provider type!");
 			}

src/Tgstation.Server.Api/Models/DiscordConnectionStringBuilder.cs

@@ -0,0 +1,37 @@
+using System;
+using Tgstation.Server.Api.Models.Internal;
+
+namespace Tgstation.Server.Api.Models
+{
+	/// <summary>
+	/// <see cref="ChatConnectionStringBuilder"/> for <see cref="ChatProvider.Discord"/>
+	/// </summary>
+	public sealed class DiscordConnectionStringBuilder : ChatConnectionStringBuilder
+	{
+		/// <inheritdoc />
+		public override bool Valid => !String.IsNullOrEmpty(BotToken);
+
+		/// <summary>
+		/// The Discord bot token
+		/// </summary>
+		/// <remarks>See https://discordapp.com/developers/docs/topics/oauth2#bots</remarks>
+		public string BotToken { get; set; }
+
+		/// <summary>
+		/// Construct a <see cref="DiscordConnectionStringBuilder"/>
+		/// </summary>
+		public DiscordConnectionStringBuilder() { }
+
+		/// <summary>
+		/// Construct a <see cref="DiscordConnectionStringBuilder"/> from a <paramref name="connectionString"/>
+		/// </summary>
+		/// <param name="connectionString">The connection string</param>
+		public DiscordConnectionStringBuilder(string connectionString)
+		{
+			BotToken = connectionString ?? throw new ArgumentNullException(nameof(connectionString));
+		}
+
+		/// <inheritdoc />
+		public override string ToString() => BotToken;
+	}
+}

src/Tgstation.Server.Api/Models/Internal/ChatBot.cs

@@ -1,4 +1,6 @@
-using System.ComponentModel.DataAnnotations;
+using System;
+using System.ComponentModel.DataAnnotations;
+using System.ComponentModel.DataAnnotations.Schema;
 
 namespace Tgstation.Server.Api.Models.Internal
 {
@@ -33,5 +35,33 @@ namespace Tgstation.Server.Api.Models.Internal
 		/// </summary>
 		[Required]
 		public string ConnectionString { get; set; }
+
+		/// <summary>
+		/// The <see cref="ChatConnectionStringBuilder"/> which maps to the <see cref="ConnectionString"/>
+		/// </summary>
+		[NotMapped]
+		public ChatConnectionStringBuilder ConnectionStringBuilder
+		{
+			get
+			{
+				if (ConnectionString == null)
+					return null;
+				switch (Provider)
+				{
+					case ChatProvider.Discord:
+						return new DiscordConnectionStringBuilder(ConnectionString);
+					case ChatProvider.Irc:
+						return new IrcConnectionStringBuilder(ConnectionString);
+					default:
+						throw new InvalidOperationException("Invalid Provider!");
+				}
+			}
+			set
+			{
+				if (value?.Valid == false)
+					throw new InvalidOperationException("Cannot set invalid ChatConnectionStringBuilder!");
+				ConnectionString = value?.ToString();
+			}
+		}
 	}
 }

src/Tgstation.Server.Api/Models/Internal/ChatConnectionStringBuilder.cs

@@ -0,0 +1,19 @@
+namespace Tgstation.Server.Api.Models.Internal
+{
+	/// <summary>
+	/// Helper for building <see cref="ChatBot.ConnectionString"/>s
+	/// </summary>
+	public abstract class ChatConnectionStringBuilder
+	{
+		/// <summary>
+		/// If the <see cref="ChatConnectionStringBuilder"/> evaluates to a valid <see cref="ChatBot.ConnectionString"/>
+		/// </summary>
+		public abstract bool Valid { get; }
+
+		/// <summary>
+		/// Gets the <see cref="ChatBot.ConnectionString"/> associated with the <see cref="ChatConnectionStringBuilder"/>
+		/// </summary>
+		/// <returns></returns>
+		public abstract override string ToString();
+	}
+}

src/Tgstation.Server.Api/Models/IrcConnectionStringBuilder.cs

@@ -0,0 +1,122 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+using Tgstation.Server.Api.Models.Internal;
+
+namespace Tgstation.Server.Api.Models
+{
+	/// <summary>
+	/// <see cref="ChatConnectionStringBuilder"/> for <see cref="ChatProvider.Irc"/>
+	/// </summary>
+	public sealed class IrcConnectionStringBuilder : ChatConnectionStringBuilder
+	{
+		/// <inheritdoc />
+		public override bool Valid => Address != null && Port.HasValue && Port != 0 && UseSsl.HasValue && (PasswordType.HasValue ^ Password == null);
+
+		/// <summary>
+		/// The IP address or URL of the IRC server
+		/// </summary>
+		public string Address { get; set; }
+
+		/// <summary>
+		/// The port the server runs on
+		/// </summary>
+		public ushort? Port { get; set; }
+
+		/// <summary>
+		/// The nickname for the bot to use
+		/// </summary>
+		public string Nickname { get; set; }
+
+		/// <summary>
+		/// If the connection should be made using SSL
+		/// </summary>
+		public bool? UseSsl { get; set; }
+
+		/// <summary>
+		/// The optional <see cref="IrcPasswordType"/> to use
+		/// </summary>
+		public IrcPasswordType? PasswordType { get; set; }
+
+		/// <summary>
+		/// The optional password to use
+		/// </summary>
+		public string Password { get; set; }
+
+		/// <summary>
+		/// Construct an <see cref="IrcConnectionStringBuilder"/>
+		/// </summary>
+		public IrcConnectionStringBuilder() { }
+
+		/// <summary>
+		/// Construct a <see cref="DiscordConnectionStringBuilder"/> from a <paramref name="connectionString"/>
+		/// </summary>
+		/// <param name="connectionString">The connection string</param>
+		public IrcConnectionStringBuilder(string connectionString)
+		{
+			if (connectionString == null)
+				throw new ArgumentNullException(nameof(connectionString));
+			var splits = connectionString.Split(';');
+
+			Address = splits[0];
+
+			if (splits.Length < 2)
+				return;
+
+			if (UInt16.TryParse(splits[1], out var port))
+				Port = port;
+
+			if (splits.Length < 3)
+				return;
+
+			Nickname = splits[2];
+
+			if (splits.Length < 4)
+				return;
+
+			if (Boolean.TryParse(splits[3], out var useSsl))
+				UseSsl = useSsl;
+
+			if (splits.Length < 5)
+				return;
+			if (Enum.TryParse<IrcPasswordType>(splits[4], out var passwordType))
+				switch (passwordType)
+				{
+					case IrcPasswordType.NickServ:
+					case IrcPasswordType.Sasl:
+					case IrcPasswordType.Server:
+						PasswordType = passwordType;
+						break;
+				}
+
+			if (splits.Length < 6)
+				return;
+
+			var rest = new List<string>(splits);
+			rest.RemoveRange(0, 5);
+			Password = String.Join(";", rest);
+		}
+
+		/// <inheritdoc />
+		public override string ToString()
+		{
+			var sb = new StringBuilder();
+			sb.Append(Address);
+			sb.Append(';');
+			sb.Append(Port);
+			sb.Append(';');
+			sb.Append(Nickname);
+			sb.Append(';');
+			if(UseSsl.HasValue)
+				sb.Append(Convert.ToInt32(UseSsl.Value));
+			if (PasswordType.HasValue)
+			{
+				sb.Append(';');
+				sb.Append((int)PasswordType);
+				sb.Append(';');
+				sb.Append(Password);
+			}
+			return sb.ToString();
+		}
+	}
+}

src/Tgstation.Server.Api/Models/IrcPasswordType.cs

@@ -1,9 +1,9 @@
-namespace Tgstation.Server.Host.Components.Chat.Providers
+namespace Tgstation.Server.Api.Models
 {
 	/// <summary>
-	/// Represents the type of a password passed to the constructor of <see cref="IrcProvider"/>
+	/// Represents the type of a password for a <see cref="ChatProvider.Irc"/>
 	/// </summary>
-	enum IrcPasswordType
+	public enum IrcPasswordType
 	{
 		/// <summary>
 		/// Use server authentication

src/Tgstation.Server.Api/Tgstation.Server.Api.csproj

@@ -17,7 +17,7 @@
     <FileVersion>4.0.0.0</FileVersion>
     <PackageTags>json web api tgstation-server tgstation ss13 byond</PackageTags>
     <PackageReleaseNotes>Prototype release</PackageReleaseNotes>
-    <Version>4.0.0.0-preview6004</Version>
+    <Version>4.0.0.0-preview6005</Version>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">

src/Tgstation.Server.Client/Tgstation.Server.Client.csproj

@@ -3,7 +3,7 @@
   <PropertyGroup>
     <TargetFramework>netstandard2.0</TargetFramework>
     <DebugType>Full</DebugType>
-    <Version>4.0.0.0-preview9111</Version>
+    <Version>4.0.0.0-preview9112</Version>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <Authors>Cyberboss</Authors>
     <Company>/tg/station 13</Company>

src/Tgstation.Server.Host/Components/Chat/ProviderFactory.cs

@@ -37,52 +37,17 @@ namespace Tgstation.Server.Host.Components.Chat
 		{
 			if (settings == null)
 				throw new ArgumentNullException(nameof(settings));
+			var builder = settings.ConnectionStringBuilder;
+			if (builder == null || !builder.Valid)
+				throw new InvalidOperationException("Invalid ChatConnectionStringBuilder!");
 			switch (settings.Provider)
 			{
 				case ChatProvider.Irc:
-					//Connection string semicolon delimited until the password field
-					if (settings.ConnectionString == null)
-						throw new InvalidOperationException("ConnectionString cannot be null!");
-					var splits = settings.ConnectionString.Split(';');
-					if (splits.Length < 4)
-						throw new InvalidOperationException("Invalid connection string!");
-
-					var address = splits[0];
-					if (!UInt16.TryParse(splits[1], out var port))
-						throw new InvalidOperationException("Unable to parse port!");
-					var nick = splits[2];
-					if (!Int32.TryParse(splits[3], out var intSsl))
-						throw new InvalidOperationException("Unable to parse ssl option!");
-
-					IrcPasswordType? passwordType = null;
-					string password = null;
-					if (splits.Length > 4)
-					{
-						if (splits.Length < 6)
-							throw new InvalidOperationException("Invalid connection string!");
-						if (!Int32.TryParse(splits[4], out var intPasswordType))
-							throw new InvalidOperationException("Unable to parse password type!");
-
-						passwordType = (IrcPasswordType)intPasswordType;
-						switch (passwordType)
-						{
-							case IrcPasswordType.NickServ:
-							case IrcPasswordType.Sasl:
-							case IrcPasswordType.Server:
-								break;
-							default:
-								throw new InvalidOperationException("Invalid password type!");
-						}
-
-						var rest = new List<string>(splits);
-						rest.RemoveRange(0, 5);
-						password = String.Join(";", rest);
-					}
-
-					return new IrcProvider(loggerFactory.CreateLogger<IrcProvider>(), application, address, port, nick, password, passwordType, intSsl != 0);
+					var ircBuilder = (IrcConnectionStringBuilder)builder;
+					return new IrcProvider(loggerFactory.CreateLogger<IrcProvider>(), application, ircBuilder.Address, ircBuilder.Port.Value, ircBuilder.Nickname, ircBuilder.Password, ircBuilder.PasswordType, ircBuilder.UseSsl.Value);
 				case ChatProvider.Discord:
-					//discord is just the bot token
-					return new DiscordProvider(loggerFactory.CreateLogger<DiscordProvider>(), settings.ConnectionString);
+					var discordBuilder = (DiscordConnectionStringBuilder)builder;
+					return new DiscordProvider(loggerFactory.CreateLogger<DiscordProvider>(), discordBuilder.BotToken);
 				default:
 					throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Invalid ChatProvider: {0}", settings.Provider));
 			}

src/Tgstation.Server.Host/Controllers/ChatController.cs

@@ -81,12 +81,11 @@ namespace Tgstation.Server.Host.Controllers
 					return BadRequest(new ErrorMessage { Message = "Invalid provider!" });
 			}
 
-			if (!model.Enabled.HasValue)
-				return BadRequest(new ErrorMessage { Message = "enabled cannot be null!" });
-
 			if (!model.ValidateProviderChannelTypes())
 				return BadRequest(new ErrorMessage { Message = "One or more of channels aren't formatted correctly for the given provider!" });
 
+			model.Enabled = model.Enabled ?? false;
+
 			//try to update das db first
 			var dbModel = new Models.ChatBot
 			{