From ffa5b5754b5ea4069ee5fadb6118bb68f6d8851f Mon Sep 17 00:00:00 2001 From: Noikoio Date: Fri, 21 Jun 2019 15:05:58 -0700 Subject: [PATCH] Several fixes after a round of testing Fixed the following compilation errors: -Moderators collection not initialized -Outdated method signatures for ban and kick in ModuleBase -Update author name in manifests -Fixed incorrect method signature in AutoScriptResponder Minor improvements: -Updated external dependencies -Remove unused variables in ConfDefinition of RegexModerator -Improve parallel execution of matches? -Send exception message on logging failure to reporting channel -Slightly change ModuleLoader logging output -Add Discord.Net unhandled exception output to logging -Updated link to Github -Changed GuildState exception handling message for conciseness Fixes: -SQL index creation in LoggingService -SQL view creation in UserCacheService -Add casts from ulong to long in SQL inserts -External modules no longer loaded twice -Non-Info messages from Discord.Net will now be reported -User data had not been recorded at proper times -Some modules had been returning null instead of Task with null -Guild state exception handling should not have handled config errors --- Kerobot/Kerobot.cs | 2 +- Kerobot/Kerobot.csproj | 14 ++++----- Kerobot/ModuleBase.cs | 30 ++++++++----------- Kerobot/ModuleLoader.cs | 7 ++--- .../Services/EntityCache/UserCacheService.cs | 22 +++++++------- .../Services/GuildState/GuildStateService.cs | 11 ++++--- Kerobot/Services/Logging/LoggingService.cs | 14 +++++---- .../AutoResponder/AutoResponder.cs | 2 +- .../Modules-PublicInstance.csproj | 5 ++-- .../RegexModerator/ConfDefinition.cs | 6 +--- .../RegexModerator/RegexModerator.cs | 11 +++++-- .../AutoScriptResponder.cs | 4 +-- Modules-SelfHosted/EntryRole/EntryRole.cs | 2 +- Modules-SelfHosted/Modules-SelfHosted.csproj | 2 +- 14 files changed, 63 insertions(+), 69 deletions(-) diff --git a/Kerobot/Kerobot.cs b/Kerobot/Kerobot.cs index 419283b..77e0ebe 100644 --- a/Kerobot/Kerobot.cs +++ b/Kerobot/Kerobot.cs @@ -46,7 +46,7 @@ namespace Kerobot // Everything's ready to go. Print the welcome message here. var ver = System.Reflection.Assembly.GetExecutingAssembly().GetName().Version; InstanceLogAsync(false, "Kerobot", - $"This is Kerobot v{ver.ToString(3)}. https://github.com/Noikoio/Kerobot").Wait(); + $"This is Kerobot v{ver.ToString(3)}. https://github.com/Noiiko/Kerobot").Wait(); // We return to Program.cs at this point. } diff --git a/Kerobot/Kerobot.csproj b/Kerobot/Kerobot.csproj index 7964904..ce84e4f 100644 --- a/Kerobot/Kerobot.csproj +++ b/Kerobot/Kerobot.csproj @@ -1,14 +1,12 @@ - + Exe netcoreapp2.0 Kerobot.Program - 0.0.1 - Noikoio + Noiiko Advanced and flexible Discord moderation bot. - 0.0.1 0.0.1 7.2 @@ -27,10 +25,10 @@ - - - - + + + + diff --git a/Kerobot/ModuleBase.cs b/Kerobot/ModuleBase.cs index 350b87f..ed9d878 100644 --- a/Kerobot/ModuleBase.cs +++ b/Kerobot/ModuleBase.cs @@ -88,23 +88,20 @@ namespace Kerobot /// The user which to perform the action to. /// Number of days of prior post history to delete on ban. Must be between 0-7. /// Reason for the action. Sent to the Audit Log and user (if specified). - /// - /// Message to DM the target user. Set null to disable. Instances of "%s" are replaced with the guild name - /// and instances of "%r" are replaced with the reason. - /// - protected Task BanAsync(SocketGuild guild, string source, ulong targetUser, int purgeDays, string reason, string dmMsg) - => Kerobot.BanOrKickAsync(RemovalType.Ban, guild, source, targetUser, purgeDays, reason, dmMsg); + /// Specify whether to send a direct message to the target user informing them of the action being taken. + protected Task BanAsync(SocketGuild guild, string source, ulong targetUser, int purgeDays, string reason, bool sendDMToTarget) + => Kerobot.BanOrKickAsync(RemovalType.Ban, guild, source, targetUser, purgeDays, reason, sendDMToTarget); /// - /// Similar to , but making use of an + /// Similar to , but making use of an /// EntityCache lookup to determine the target. /// /// The EntityCache search string. - protected async Task BanAsync(SocketGuild guild, string source, string targetSearch, int purgeDays, string reason, string dmMsg) + protected async Task BanAsync(SocketGuild guild, string source, string targetSearch, int purgeDays, string reason, bool sendDMToTarget) { var result = await Kerobot.EcQueryUser(guild.Id, targetSearch); if (result == null) return new BanKickResult(null, false, true); - return await BanAsync(guild, source, result.UserID, purgeDays, reason, dmMsg); + return await BanAsync(guild, source, result.UserID, purgeDays, reason, sendDMToTarget); } /// @@ -117,23 +114,20 @@ namespace Kerobot /// The user, if any, which requested the action to be taken. /// The user which to perform the action to. /// Reason for the action. Sent to the Audit Log and user (if specified). - /// - /// Message to DM the target user. Set null to disable. Instances of "%s" are replaced with the guild name - /// and instances of "%r" are replaced with the reason. - /// - protected Task KickAsync(SocketGuild guild, string source, ulong targetUser, string reason, string dmMsg) - => Kerobot.BanOrKickAsync(RemovalType.Ban, guild, source, targetUser, 0, reason, dmMsg); + /// Specify whether to send a direct message to the target user informing them of the action being taken. + protected Task KickAsync(SocketGuild guild, string source, ulong targetUser, string reason, bool sendDMToTarget) + => Kerobot.BanOrKickAsync(RemovalType.Ban, guild, source, targetUser, 0, reason, sendDMToTarget); /// - /// Similar to , but making use of an + /// Similar to , but making use of an /// EntityCache lookup to determine the target. /// /// The EntityCache search string. - protected async Task KickAsync(SocketGuild guild, string source, string targetSearch, string reason, string dmMsg) + protected async Task KickAsync(SocketGuild guild, string source, string targetSearch, string reason, bool sendDMToTarget) { var result = await Kerobot.EcQueryUser(guild.Id, targetSearch); if (result == null) return new BanKickResult(null, false, true); - return await KickAsync(guild, source, result.UserID, reason, dmMsg); + return await KickAsync(guild, source, result.UserID, reason, sendDMToTarget); } /// diff --git a/Kerobot/ModuleLoader.cs b/Kerobot/ModuleLoader.cs index 842bf9a..d9fd448 100644 --- a/Kerobot/ModuleLoader.cs +++ b/Kerobot/ModuleLoader.cs @@ -45,7 +45,7 @@ namespace Kerobot Console.WriteLine(ex.ToString()); Environment.Exit(2); } - modules.AddRange(LoadModulesFromAssembly(a, k)); + modules.AddRange(amods); } return modules.AsReadOnly(); } @@ -56,15 +56,14 @@ namespace Kerobot where !type.IsAssignableFrom(typeof(ModuleBase)) where type.GetCustomAttribute() != null select type; - k.InstanceLogAsync(false, LogName, - $"{asm.GetName().Name} has {eligibleTypes.Count()} usable types"); + k.InstanceLogAsync(false, LogName, $"Scanning {asm.GetName().Name}"); var newmods = new List(); foreach (var t in eligibleTypes) { var mod = Activator.CreateInstance(t, k); k.InstanceLogAsync(false, LogName, - $"---> Instance created: {t.FullName}"); + $"---> Loading module {t.FullName}"); newmods.Add((ModuleBase)mod); } return newmods; diff --git a/Kerobot/Services/EntityCache/UserCacheService.cs b/Kerobot/Services/EntityCache/UserCacheService.cs index b819737..8b3d001 100644 --- a/Kerobot/Services/EntityCache/UserCacheService.cs +++ b/Kerobot/Services/EntityCache/UserCacheService.cs @@ -66,7 +66,7 @@ namespace Kerobot.Services.EntityCache c.CommandText = $"create or replace view {UserView} as " + $"select {GlobalUserTable}.user_id, {GuildUserTable}.guild_id, {GuildUserTable}.first_seen, " + $"{GuildUserTable}.cache_update_time, " + - $"{GlobalUserTable}.username, {GlobalUserTable}.discriminator, {GlobalUserTable}.nickname, " + + $"{GlobalUserTable}.username, {GlobalUserTable}.discriminator, {GuildUserTable}.nickname, " + $"{GlobalUserTable}.avatar_url " + $"from {GlobalUserTable} join {GuildUserTable} on {GlobalUserTable}.user_id = {GuildUserTable}.user_id"; await c.ExecuteNonQueryAsync(); @@ -90,7 +90,7 @@ namespace Kerobot.Services.EntityCache "on conflict (user_id) do update " + "set cache_update_time = EXCLUDED.cache_update_time, username = EXCLUDED.username, " + "discriminator = EXCLUDED.discriminator, avatar_url = EXCLUDED.avatar_url"; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = current.Id; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)current.Id; c.Parameters.Add("@Uname", NpgsqlDbType.Text).Value = current.Username; c.Parameters.Add("@Disc", NpgsqlDbType.Text).Value = current.Discriminator; var aurl = c.Parameters.Add("@Aurl", NpgsqlDbType.Text); @@ -106,6 +106,9 @@ namespace Kerobot.Services.EntityCache private async Task DiscordClient_GuildMemberUpdated(SocketGuildUser old, SocketGuildUser current) { + // Also update user data here, in case it's unknown (avoid foreign key constraint violation) + await DiscordClient_UserUpdated(old, current); + using (var db = await _kb.GetOpenNpgsqlConnectionAsync()) { using (var c = db.CreateCommand()) @@ -113,11 +116,10 @@ namespace Kerobot.Services.EntityCache c.CommandText = $"insert into {GuildUserTable} " + "(user_id, guild_id, cache_update_time, nickname) values " + "(@Uid, @Gid, now(), @Nname) " + - "on conflict (user_id) do update " + - "set cache_update_time = EXCLUDED.cache_update_time, username = EXCLUDED.username, " + - "discriminator = EXCLUDED.discriminator, avatar_url = EXCLUDED.avatar_url"; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = current.Id; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = current.Guild.Id; + "on conflict (user_id, guild_id) do update " + + "set cache_update_time = EXCLUDED.cache_update_time, nickname = EXCLUDED.nickname"; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)current.Id; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)current.Guild.Id; var nname = c.Parameters.Add("@Nname", NpgsqlDbType.Text); if (current.Nickname != null) nname.Value = current.Nickname; else nname.Value = DBNull.Value; @@ -130,7 +132,7 @@ namespace Kerobot.Services.EntityCache #endregion #region Querying - private static Regex DiscriminatorSearch = new Regex(@"(.+)#(\d{4}(?!\d))", RegexOptions.Compiled); + private static readonly Regex DiscriminatorSearch = new Regex(@"(.+)#(\d{4}(?!\d))", RegexOptions.Compiled); /// /// See . @@ -174,12 +176,12 @@ namespace Kerobot.Services.EntityCache var c = db.CreateCommand(); c.CommandText = $"select * from {UserView} " + "where guild_id = @Gid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = guildId; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; if (sID.HasValue) { c.CommandText += " and user_id = @Uid"; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = sID.Value; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)sID.Value; } if (sName != null) diff --git a/Kerobot/Services/GuildState/GuildStateService.cs b/Kerobot/Services/GuildState/GuildStateService.cs index dbfba6f..18c42e1 100644 --- a/Kerobot/Services/GuildState/GuildStateService.cs +++ b/Kerobot/Services/GuildState/GuildStateService.cs @@ -149,15 +149,15 @@ namespace Kerobot.Services.GuildState var tn = t.Name; try { - object state; try { - state = await mod.CreateGuildStateAsync(guildId, guildConf[tn]); // can be null + var state = await mod.CreateGuildStateAsync(guildId, guildConf[tn]); // can be null + newStates.Add(t, new StateInfo(state, jstrHash)); } - catch (Exception ex) + catch (Exception ex) when (!(ex is ModuleLoadException)) { - Log("Encountered unhandled exception during guild state initialization:\n" + - $"Module: {tn}\n" + + Log("Unhandled exception while initializing guild state for module:\n" + + $"Module: {tn} | " + $"Guild: {guildId} ({Kerobot.DiscordClient.GetGuild(guildId)?.Name ?? "unknown name"})\n" + $"```\n{ex.ToString()}\n```", true).Wait(); Kerobot.GuildLogAsync(guildId, GuildLogSource, @@ -165,7 +165,6 @@ namespace Kerobot.Services.GuildState "The bot owner has been notified.").Wait(); return false; } - newStates.Add(t, new StateInfo(state, jstrHash)); } catch (ModuleLoadException ex) { diff --git a/Kerobot/Services/Logging/LoggingService.cs b/Kerobot/Services/Logging/LoggingService.cs index 7cbf842..009bb96 100644 --- a/Kerobot/Services/Logging/LoggingService.cs +++ b/Kerobot/Services/Logging/LoggingService.cs @@ -35,10 +35,10 @@ namespace Kerobot.Services.Logging /// private async Task DiscordClient_Log(LogMessage arg) { - var ts = DateTimeOffset.UtcNow; - bool important = arg.Severity > LogSeverity.Info; + bool important = arg.Severity != LogSeverity.Info; string msg = $"[{Enum.GetName(typeof(LogSeverity), arg.Severity)}] {arg.Message}"; const string logSource = "Discord.Net"; + if (arg.Exception != null) msg += "\n```\n" + arg.Exception.ToString() + "\n```"; if (important) await DoInstanceLogAsync(true, logSource, msg); else FormatToConsole(DateTimeOffset.UtcNow, logSource, msg); @@ -64,7 +64,7 @@ namespace Kerobot.Services.Logging using (var c = db.CreateCommand()) { c.CommandText = "create index if not exists " + - $"{TableLog}_guildid_idx on {TableLog} guild_id"; + $"{TableLog}_guild_id_idx on {TableLog} (guild_id)"; await c.ExecuteNonQueryAsync(); } } @@ -78,8 +78,8 @@ namespace Kerobot.Services.Logging { c.CommandText = $"insert into {TableLog} (guild_id, log_timestamp, log_source, message) values" + "(@Gid, @Ts, @Src, @Msg)"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = guildId; - c.Parameters.Add("@Ts", NpgsqlDbType.TimestampTZ).Value = timestamp; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; + c.Parameters.Add("@Ts", NpgsqlDbType.TimestampTz).Value = timestamp; c.Parameters.Add("@Src", NpgsqlDbType.Text).Value = source; c.Parameters.Add("@Msg", NpgsqlDbType.Text).Value = message; c.Prepare(); @@ -126,6 +126,7 @@ namespace Kerobot.Services.Logging } // Report to logging channel if necessary and possible + // TODO replace with webhook? var (g, c) = Kerobot.Config.InstanceLogReportTarget; if ((insertException != null || report) && g != 0 && c != 0 && Kerobot.DiscordClient.ConnectionState == ConnectionState.Connected) @@ -142,7 +143,8 @@ namespace Kerobot.Services.Logging { Footer = new EmbedFooterBuilder() { Text = Name }, Timestamp = DateTimeOffset.UtcNow, - Description = "Error during recording to instance log.\nCheck the console.", + Description = "Error during recording to instance log: `" + + insertException.Message + "`\nCheck the console.", Color = Color.DarkRed }; await ch.SendMessageAsync("", embed: e.Build()); diff --git a/Modules-PublicInstance/AutoResponder/AutoResponder.cs b/Modules-PublicInstance/AutoResponder/AutoResponder.cs index 5b06ce2..dd2f655 100644 --- a/Modules-PublicInstance/AutoResponder/AutoResponder.cs +++ b/Modules-PublicInstance/AutoResponder/AutoResponder.cs @@ -38,7 +38,7 @@ namespace Kerobot.Modules.AutoResponder public override Task CreateGuildStateAsync(ulong guild, JToken config) { // Guild state is a read-only IEnumerable - if (config == null) return null; + if (config == null) return Task.FromResult(null); var guildDefs = new List(); foreach (var defconf in config.Children()) { diff --git a/Modules-PublicInstance/Modules-PublicInstance.csproj b/Modules-PublicInstance/Modules-PublicInstance.csproj index 1022834..2d467ab 100644 --- a/Modules-PublicInstance/Modules-PublicInstance.csproj +++ b/Modules-PublicInstance/Modules-PublicInstance.csproj @@ -1,9 +1,8 @@ - + netcoreapp2.0 - Noikoio - Noikoio + Noiiko Kerobot 0.0.1 Essential functions for Kerobot which are available in the public bot instance. diff --git a/Modules-PublicInstance/RegexModerator/ConfDefinition.cs b/Modules-PublicInstance/RegexModerator/ConfDefinition.cs index 2b48a85..44c483f 100644 --- a/Modules-PublicInstance/RegexModerator/ConfDefinition.cs +++ b/Modules-PublicInstance/RegexModerator/ConfDefinition.cs @@ -18,8 +18,6 @@ namespace Kerobot.Modules.RegexModerator class ConfDefinition { public string Label { get; } - readonly RegexModerator _module; // TODO is this needed? - readonly ulong _guild; // corresponding guild, for debug purposes. (is this needed?) // Matching settings readonly IEnumerable _regex; @@ -40,10 +38,8 @@ namespace Kerobot.Modules.RegexModerator public bool RemovalSendUserNotification; // send ban/kick notification to user? public bool DeleteMessage { get; } - public ConfDefinition(RegexModerator instance, JObject def, ulong guildId) + public ConfDefinition(JObject def) { - _module = instance; - Label = def["Label"].Value(); if (string.IsNullOrWhiteSpace(Label)) throw new ModuleLoadException("A rule does not have a label defined."); diff --git a/Modules-PublicInstance/RegexModerator/RegexModerator.cs b/Modules-PublicInstance/RegexModerator/RegexModerator.cs index 07fa47b..d21b459 100644 --- a/Modules-PublicInstance/RegexModerator/RegexModerator.cs +++ b/Modules-PublicInstance/RegexModerator/RegexModerator.cs @@ -23,8 +23,9 @@ namespace Kerobot.Modules.RegexModerator if (config == null) return Task.FromResult(null); var defs = new List(); + // TODO better error reporting during this process foreach (var def in config.Children()) - defs.Add(new ConfDefinition(this, def, guildID)); + defs.Add(new ConfDefinition(def)); if (defs.Count == 0) return Task.FromResult(null); return Task.FromResult(defs.AsReadOnly()); @@ -49,19 +50,23 @@ namespace Kerobot.Modules.RegexModerator // Send further processing to thread pool. // Match checking is a CPU-intensive task, thus very little checking is done here. + + + var msgProcessingTasks = new List(); foreach (var item in defs) { // Need to check sender's moderator status here. Definition can't access mod list. var isMod = GetModerators(ch.Guild.Id).IsListMatch(msg, true); var match = item.IsMatch(msg, isMod); - await Task.Run(async () => await ProcessMessage(item, msg, isMod)); + msgProcessingTasks.Add(Task.Run(async () => await ProcessMessage(item, msg, isMod))); } + await Task.WhenAll(msgProcessingTasks); } /// /// Does further message checking and response execution. - /// Invocations of this method are meant to be on the thread pool. + /// Invocations of this method are meant to be placed onto a thread separate from the caller. /// private async Task ProcessMessage(ConfDefinition def, SocketMessage msg, bool isMod) { diff --git a/Modules-SelfHosted/AutoScriptResponder/AutoScriptResponder.cs b/Modules-SelfHosted/AutoScriptResponder/AutoScriptResponder.cs index 10ba005..801cc56 100644 --- a/Modules-SelfHosted/AutoScriptResponder/AutoScriptResponder.cs +++ b/Modules-SelfHosted/AutoScriptResponder/AutoScriptResponder.cs @@ -37,10 +37,10 @@ namespace Kerobot.Modules.AutoScriptResponder await Task.WhenAll(tasks); } - public override Task CreateGuildStateAsync(JToken config) + public override Task CreateGuildStateAsync(ulong guild, JToken config) { // Guild state is a read-only IEnumerable - if (config == null) return null; + if (config == null) return Task.FromResult(null); var guildDefs = new List(); foreach (var defconf in config.Children()) { diff --git a/Modules-SelfHosted/EntryRole/EntryRole.cs b/Modules-SelfHosted/EntryRole/EntryRole.cs index 5d16188..c2863ab 100644 --- a/Modules-SelfHosted/EntryRole/EntryRole.cs +++ b/Modules-SelfHosted/EntryRole/EntryRole.cs @@ -43,7 +43,7 @@ namespace Kerobot.Modules.EntryRole public override Task CreateGuildStateAsync(ulong guildID, JToken config) { - if (config == null) return null; + if (config == null) return Task.FromResult(null); if (config.Type != JTokenType.Object) throw new ModuleLoadException("Configuration is not properly defined."); diff --git a/Modules-SelfHosted/Modules-SelfHosted.csproj b/Modules-SelfHosted/Modules-SelfHosted.csproj index d4b4ea8..8fa2af6 100644 --- a/Modules-SelfHosted/Modules-SelfHosted.csproj +++ b/Modules-SelfHosted/Modules-SelfHosted.csproj @@ -2,7 +2,7 @@ netcoreapp2.0 - Noikoio + Noiiko Kerobot 0.0.1 Kerobot modules with more specific purposes that may not work well in a public instance, but are manageable in self-hosted bot instances.