From 0bd9b79e5008348274861c0a67f53543ad5f9b8f Mon Sep 17 00:00:00 2001 From: Noi Date: Tue, 14 Jul 2020 11:59:14 -0700 Subject: [PATCH] Replace OperationStatus with test command --- BackgroundServices/BirthdayRoleUpdate.cs | 112 ++++++++++++----------- BirthdayBot.cs | 26 ++++-- BirthdayBot.csproj | 2 +- Data/GuildStateInformation.cs | 3 - Data/OperationStatus.cs | 64 ------------- UserInterface/ManagerCommands.cs | 59 ++++++------ 6 files changed, 109 insertions(+), 157 deletions(-) delete mode 100644 Data/OperationStatus.cs diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index 6a55854..ae9f7e7 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -53,69 +53,54 @@ namespace BirthdayBot.BackgroundServices GC.Collect(); } - public async Task SingleUpdateFor(SocketGuild guild) - { - try - { - await ProcessGuildAsync(guild); - } - catch (Exception ex) - { - Log("Encountered an error during guild processing:"); - Log(ex.ToString()); - } - - // TODO metrics for role sets, unsets, announcements - and I mentioned this above too - } + /// + /// Access to for the testing command. + /// + /// Diagnostic data in string form. + public async Task SingleProcessGuildAsync(SocketGuild guild) => (await ProcessGuildAsync(guild)).Export(); /// /// Main method where actual guild processing occurs. /// - private async Task ProcessGuildAsync(SocketGuild guild) + private async Task ProcessGuildAsync(SocketGuild guild) { + var diag = new PGDiagnostic(); + // Skip processing of guild if local info has not yet been loaded - if (!BotInstance.GuildCache.TryGetValue(guild.Id, out var gs)) return; + if (!BotInstance.GuildCache.TryGetValue(guild.Id, out var gs)) + { + diag.FetchCachedGuild = "Server information not yet loaded by the bot. Try again later."; + return diag; + } + diag.FetchCachedGuild = null; // Check if role settings are correct before continuing with further processing SocketRole role = null; if (gs.RoleId.HasValue) role = guild.GetRole(gs.RoleId.Value); - var roleCheck = CheckCorrectRoleSettings(guild, role); - if (!roleCheck.Item1) - { - lock (gs) - gs.OperationLog = new OperationStatus((OperationStatus.OperationType.UpdateBirthdayRoleMembership, roleCheck.Item2)); - return; - } + diag.RoleCheck = CheckCorrectRoleSettings(guild, role); + if (diag.RoleCheck != null) return diag; // Determine who's currently having a birthday var users = gs.Users; var tz = gs.TimeZone; var birthdays = GetGuildCurrentBirthdays(users, tz); // Note: Don't quit here if zero people are having birthdays. Roles may still need to be removed by BirthdayApply. + diag.CurrentBirthdays = birthdays.Count.ToString(); IEnumerable announcementList; - (int, int) roleResult; // role additions, removals // Update roles as appropriate try { var updateResult = await UpdateGuildBirthdayRoles(guild, role, birthdays); announcementList = updateResult.Item1; - roleResult = updateResult.Item2; + diag.RoleApplyResult = updateResult.Item2; // statistics } catch (Discord.Net.HttpException ex) { - lock (gs) - gs.OperationLog = new OperationStatus((OperationStatus.OperationType.UpdateBirthdayRoleMembership, ex.Message)); - if (ex.HttpCode != System.Net.HttpStatusCode.Forbidden) - { - // Send unusual exceptions to caller - throw; - } - return; + diag.RoleApply = ex.Message; + return diag; } - (OperationStatus.OperationType, string) opResult1, opResult2; - opResult1 = (OperationStatus.OperationType.UpdateBirthdayRoleMembership, - $"Success: Added {roleResult.Item1} member(s), Removed {roleResult.Item2} member(s) from target role."); + diag.RoleApply = null; // Birthday announcement var announce = gs.AnnounceMessages; @@ -124,41 +109,36 @@ namespace BirthdayBot.BackgroundServices if (gs.AnnounceChannelId.HasValue) channel = guild.GetTextChannel(gs.AnnounceChannelId.Value); if (announcementList.Count() != 0) { - var announceOpResult = await AnnounceBirthdaysAsync(announce, announceping, channel, announcementList); - opResult2 = (OperationStatus.OperationType.SendBirthdayAnnouncementMessage, announceOpResult); + var announceResult = await AnnounceBirthdaysAsync(announce, announceping, channel, announcementList); + diag.Announcement = announceResult; } else { - opResult2 = (OperationStatus.OperationType.SendBirthdayAnnouncementMessage, "Announcement not considered."); + diag.Announcement = "No new role additions. Announcement not needed."; } - // Update status - lock (gs) gs.OperationLog = new OperationStatus(opResult1, opResult2); + return diag; } /// /// Checks if the bot may be allowed to alter roles. /// - /// - /// First item: Boolean value determining if the role setup is correct. - /// Second item: String to append to operation status in case of failure. - /// - private (bool, string) CheckCorrectRoleSettings(SocketGuild guild, SocketRole role) + private string CheckCorrectRoleSettings(SocketGuild guild, SocketRole role) { - if (role == null) return (false, "Failed: Designated role not found or defined."); + if (role == null) return "Designated role is not set, or target role cannot be found."; if (!guild.CurrentUser.GuildPermissions.ManageRoles) { - return (false, "Failed: Bot does not contain Manage Roles permission."); + return "Bot does not have the 'Manage Roles' permission."; } // Check potential role order conflict if (role.Position >= guild.CurrentUser.Hierarchy) { - return (false, "Failed: Bot is beneath the designated role in the role hierarchy."); + return "Bot is unable to access the designated role due to permission hierarchy."; } - return (true, null); + return null; } /// @@ -253,7 +233,7 @@ namespace BirthdayBot.BackgroundServices private async Task AnnounceBirthdaysAsync( (string, string) announce, bool announcePing, SocketTextChannel c, IEnumerable names) { - if (c == null) return "Announcement channel is undefined."; + if (c == null) return "Announcement channel is not set, or previous announcement channel has been deleted."; string announceMsg; if (names.Count() == 1) announceMsg = announce.Item1 ?? announce.Item2 ?? DefaultAnnounce; @@ -278,7 +258,7 @@ namespace BirthdayBot.BackgroundServices try { await c.SendMessageAsync(announceMsg.Replace("%n", namedisplay.ToString())); - return $"Successfully announced {names.Count()} name(s)"; + return null; } catch (Discord.Net.HttpException ex) { @@ -286,5 +266,33 @@ namespace BirthdayBot.BackgroundServices return ex.Message; } } + + private class PGDiagnostic + { + const string DefaultValue = "--"; + + public string FetchCachedGuild = DefaultValue; + public string RoleCheck = DefaultValue; + public string CurrentBirthdays = DefaultValue; + public string RoleApply = DefaultValue; + public (int, int)? RoleApplyResult; + public string Announcement = DefaultValue; + + public string Export() + { + var result = new StringBuilder(); + result.AppendLine("Test result:"); + result.AppendLine("Fetch guild information: " + (FetchCachedGuild ?? ":white_check_mark:")); + result.AppendLine("Check role permissions: " + (RoleCheck ?? ":white_check_mark:")); + result.AppendLine("Number of known users currently with a birthday: " + CurrentBirthdays); + result.AppendLine("Role application process: " + (RoleApply ?? ":white_check_mark:")); + result.Append("Role application metrics: "); + if (RoleApplyResult.HasValue) result.AppendLine($"{RoleApplyResult.Value.Item1} additions, {RoleApplyResult.Value.Item2} removals."); + else result.AppendLine(DefaultValue); + result.AppendLine("Announcement: " + (Announcement ?? ":white_check_mark:")); + + return result.ToString(); + } + } } } diff --git a/BirthdayBot.cs b/BirthdayBot.cs index 01880d2..6b58c60 100644 --- a/BirthdayBot.cs +++ b/BirthdayBot.cs @@ -45,7 +45,7 @@ namespace BirthdayBot foreach (var item in _cmdsListing.Commands) _dispatchCommands.Add(item.Item1, item.Item2); _cmdsHelp = new HelpInfoCommands(this, conf); foreach (var item in _cmdsHelp.Commands) _dispatchCommands.Add(item.Item1, item.Item2); - _cmdsMods = new ManagerCommands(this, conf, _cmdsUser.Commands); + _cmdsMods = new ManagerCommands(this, conf, _cmdsUser.Commands, _worker.BirthdayUpdater.SingleProcessGuildAsync); foreach (var item in _cmdsMods.Commands) _dispatchCommands.Add(item.Item1, item.Item2); // Register event handlers @@ -91,9 +91,25 @@ namespace BirthdayBot return Task.CompletedTask; } - private async Task SetStatus(DiscordSocketClient shard) + private async Task SetStatus(DiscordSocketClient shard) => await shard.SetGameAsync(CommandPrefix + "help"); + + public async Task PushErrorLog(string source, string message) { - await shard.SetGameAsync(CommandPrefix + "help"); + // Attempt to report instance logging failure to the reporting channel + try + { + EmbedBuilder e = new EmbedBuilder() + { + Footer = new EmbedFooterBuilder() { Text = source }, + Timestamp = DateTimeOffset.UtcNow, + Description = message + }; + await LogWebhook.SendMessageAsync(embeds: new Embed[] { e.Build() }); + } + catch + { + return; // Give up + } } private async Task Dispatch(SocketMessage msg) @@ -144,10 +160,6 @@ namespace BirthdayBot // Fail silently. } } - - // Immediately check for role updates in the invoking guild - // TODO be smarter about when to call this - await _worker.BirthdayUpdater.SingleUpdateFor(channel.Guild); } } } diff --git a/BirthdayBot.csproj b/BirthdayBot.csproj index eb741df..e55f7a7 100644 --- a/BirthdayBot.csproj +++ b/BirthdayBot.csproj @@ -3,7 +3,7 @@ Exe netcoreapp3.1 - 2.1.0 + 2.2.0 BirthdayBot NoiTheCat BirthdayBot diff --git a/Data/GuildStateInformation.cs b/Data/GuildStateInformation.cs index e3e380b..49da0ef 100644 --- a/Data/GuildStateInformation.cs +++ b/Data/GuildStateInformation.cs @@ -27,7 +27,6 @@ namespace BirthdayBot.Data private readonly Dictionary _userCache; public ulong GuildId { get; } - public OperationStatus OperationLog { get; set; } /// /// Gets a list of cached registered user information. @@ -83,8 +82,6 @@ namespace BirthdayBot.Data { _db = dbconfig; - OperationLog = new OperationStatus(); - GuildId = (ulong)reader.GetInt64(0); if (!reader.IsDBNull(1)) { diff --git a/Data/OperationStatus.cs b/Data/OperationStatus.cs deleted file mode 100644 index 84c9b89..0000000 --- a/Data/OperationStatus.cs +++ /dev/null @@ -1,64 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Text; - -namespace BirthdayBot.Data -{ - /// - /// Holds information regarding the previous updating information done on a guild including success/error information. - /// - class OperationStatus - { - private readonly Dictionary _log = new Dictionary(); - - public DateTimeOffset Timestamp { get; } - - public OperationStatus (params (OperationType, string)[] statuses) - { - Timestamp = DateTimeOffset.UtcNow; - foreach (var status in statuses) - { - _log[status.Item1] = status.Item2; - } - } - - /// - /// Prepares known information in a displayable format. - /// - public string GetDiagStrings() - { - var report = new StringBuilder(); - foreach (OperationType otype in Enum.GetValues(typeof(OperationType))) - { - var prefix = $"`{Enum.GetName(typeof(OperationType), otype)}`: "; - - string info = null; - - if (!_log.TryGetValue(otype, out info)) - { - report.AppendLine(prefix + "No data"); - continue; - } - - if (info == null) - { - report.AppendLine(prefix + "Success"); - } - else - { - report.AppendLine(prefix + info); - } - } - return report.ToString(); - } - - /// - /// Specifies the type of operation logged. These enum values are publicly displayed in the specified order. - /// - public enum OperationType - { - UpdateBirthdayRoleMembership, - SendBirthdayAnnouncementMessage - } - } -} diff --git a/UserInterface/ManagerCommands.cs b/UserInterface/ManagerCommands.cs index b252da6..70a0c8c 100644 --- a/UserInterface/ManagerCommands.cs +++ b/UserInterface/ManagerCommands.cs @@ -1,7 +1,4 @@ -using BirthdayBot.BackgroundServices; -using Discord; -using Discord.WebSocket; -using NodaTime; +using Discord.WebSocket; using System; using System.Collections.Generic; using System.Linq; @@ -18,8 +15,10 @@ namespace BirthdayBot.UserInterface private readonly Dictionary _subcommands; private readonly Dictionary _usercommands; + private readonly Func> _bRoleUpdAccess; - public ManagerCommands(BirthdayBot inst, Configuration db, IEnumerable<(string, CommandHandler)> userCommands) + public ManagerCommands(BirthdayBot inst, Configuration db, + IEnumerable<(string, CommandHandler)> userCommands, Func> brsingleupdate) : base(inst, db) { _subcommands = new Dictionary(StringComparer.OrdinalIgnoreCase) @@ -39,6 +38,9 @@ namespace BirthdayBot.UserInterface // Set up local copy of all user commands accessible by the override command _usercommands = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var item in userCommands) _usercommands.Add(item.Item1, item.Item2); + + // and access to the otherwise automated guild update function + _bRoleUpdAccess = brsingleupdate; } public override IEnumerable<(string, CommandHandler)> Commands @@ -46,7 +48,7 @@ namespace BirthdayBot.UserInterface { ("config", CmdConfigDispatch), ("override", CmdOverride), - ("status", CmdStatus) + ("test", CmdTest) }; #region Documentation @@ -426,39 +428,36 @@ namespace BirthdayBot.UserInterface await action.Invoke(overparam, reqChannel, overuser); } - // Prints a status report useful for troubleshooting operational issues within a guild - private async Task CmdStatus(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + // Publicly available command that immediately processes the current guild, + private async Task CmdTest(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // Moderators only. As with config, silently drop if this check fails. if (!Instance.GuildCache[reqUser.Guild.Id].IsUserModerator(reqUser)) return; + // TODO fix this or incorporate into final output - checking existence in guild cache is a step in the process - DateTimeOffset optime; - string optext; - string zone; - var gi = Instance.GuildCache[reqChannel.Guild.Id]; - lock (gi) + if (param.Length != 1) { - var opstat = gi.OperationLog; - optext = opstat.GetDiagStrings(); // !!! Bulk of output handled by this method - optime = opstat.Timestamp; - zone = gi.TimeZone ?? "UTC"; + // Too many parameters + // Note: Non-standard error display + await reqChannel.SendMessageAsync(NoParameterError); + return; } - var shard = Instance.DiscordClient.GetShardIdFor(reqChannel.Guild); - // Calculate timestamp in current zone - var zonedTimeInstant = SystemClock.Instance.GetCurrentInstant().InZone(DateTimeZoneProviders.Tzdb.GetZoneOrNull(zone)); - var timeAgoEstimate = DateTimeOffset.UtcNow - optime; + // had an option to clear roles here, but application testing revealed that running the + // test at this point would make the updater assume that roles had not yet been cleared + // may revisit this later... - var result = new EmbedBuilder + try { - Title = "Background operation status", - Description = $"Shard: {shard}\n" - + $"Operation time: {Math.Round(timeAgoEstimate.TotalSeconds)} second(s) ago at {zonedTimeInstant}\n" - + "Report:\n" - + optext.TrimEnd() - }; - - await reqChannel.SendMessageAsync(embed: result.Build()); + var result = await _bRoleUpdAccess(reqChannel.Guild); + await reqChannel.SendMessageAsync(result); + } + catch (Exception ex) + { + Program.Log("Test command", ex.ToString()); + reqChannel.SendMessageAsync(InternalError).Wait(); + // TODO webhook report + } } #region Common/helper methods