From aedc0f2ba716dc6db5909d39565462d548d12305 Mon Sep 17 00:00:00 2001 From: Noi Date: Thu, 16 Jul 2020 11:50:12 -0700 Subject: [PATCH 01/19] Rename files --- Data/{GuildStateInformation.cs => GuildConfiguration.cs} | 0 Data/{GuildUserSettings.cs => GuildUserConfiguration.cs} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename Data/{GuildStateInformation.cs => GuildConfiguration.cs} (100%) rename Data/{GuildUserSettings.cs => GuildUserConfiguration.cs} (100%) diff --git a/Data/GuildStateInformation.cs b/Data/GuildConfiguration.cs similarity index 100% rename from Data/GuildStateInformation.cs rename to Data/GuildConfiguration.cs diff --git a/Data/GuildUserSettings.cs b/Data/GuildUserConfiguration.cs similarity index 100% rename from Data/GuildUserSettings.cs rename to Data/GuildUserConfiguration.cs From 0f05e3f0a8e1c2c2166fb8f2ecbe0975aca1cc0a Mon Sep 17 00:00:00 2001 From: Noi Date: Thu, 16 Jul 2020 11:50:55 -0700 Subject: [PATCH 02/19] Rewrite GuildConfiguration No longer assumes it's permanently cached. When used, now requires that any updates have its update method explicitly called. --- Data/GuildConfiguration.cs | 432 +++++++++++-------------------------- 1 file changed, 126 insertions(+), 306 deletions(-) diff --git a/Data/GuildConfiguration.cs b/Data/GuildConfiguration.cs index 49da0ef..1f35645 100644 --- a/Data/GuildConfiguration.cs +++ b/Data/GuildConfiguration.cs @@ -1,306 +1,138 @@ -using Discord.WebSocket; -using Npgsql; +using Npgsql; using NpgsqlTypes; using System; -using System.Collections.Generic; using System.Data.Common; -using System.Linq; using System.Threading.Tasks; namespace BirthdayBot.Data { /// - /// Holds various pieces of state information for a guild the bot is operating in. - /// Includes, among other things, a copy of the guild's settings and a list of all known users with birthdays. + /// Represents guild-specific configuration as exists in the database. + /// Updating any property requires a call to for changes to take effect. /// - class GuildStateInformation + class GuildConfiguration { - private readonly Database _db; - private ulong? _bdayRole; - private ulong? _announceCh; - private ulong? _modRole; - private string _tz; - private bool _moderated; - private string _announceMsg; - private string _announceMsgPl; - private bool _announcePing; - private readonly Dictionary _userCache; - + /// + /// Gets this configuration's corresponding guild ID. + /// public ulong GuildId { get; } /// - /// Gets a list of cached registered user information. + /// Gets or sets the guild's designated usable role ID. + /// Updating this value requires a call to . /// - public IEnumerable Users { - get { - var items = new List(); - lock (this) - { - foreach (var item in _userCache.Values) items.Add(item); - } - return items; - } - } + public ulong? RoleId { get; set; } /// - /// Gets the guild's designated Role ID. + /// Gets or sets the announcement channel ID. + /// Updating this value requires a call to . /// - public ulong? RoleId { get { lock (this) { return _bdayRole; } } } + public ulong? AnnounceChannelId { get; set; } /// - /// Gets the designated announcement Channel ID. + /// Gets or sets the guild's default time zone ztring. + /// Updating this value requires a call to . /// - public ulong? AnnounceChannelId { get { lock (this) { return _announceCh; } } } + public string TimeZone { get; set; } /// - /// Gets the guild's default time zone. + /// Gets or sets the guild's moderated mode setting. + /// Updating this value requires a call to . /// - public string TimeZone { get { lock (this) { return _tz; } } } + public bool IsModerated { get; set; } /// - /// Gets whether the guild is in moderated mode. + /// Gets or sets the guild's corresponding bot moderator role ID. + /// Updating this value requires a call to . /// - public bool IsModerated { get { lock (this) { return _moderated; } } } + public ulong? ModeratorRole { get; set; } /// - /// Gets the designated moderator role ID. + /// Gets or sets the guild-specific birthday announcement message. + /// Updating this value requires a call to . /// - public ulong? ModeratorRole { get { lock (this) { return _modRole; } } } + public (string, string) AnnounceMessages { get; set; } /// - /// Gets the guild-specific birthday announcement message. + /// Gets or sets the announcement ping setting. + /// Updating this value requires a call to . /// - public (string, string) AnnounceMessages { get { lock (this) { return (_announceMsg, _announceMsgPl); } } } + public bool AnnouncePing { get; set; } - /// - /// Gets whether to ping users in the announcement message instead of displaying their names. - /// - public bool AnnouncePing { get { lock (this) { return _announcePing; } } } - - // Called by LoadSettingsAsync. Double-check ordinals when changes are made. - private GuildStateInformation(DbDataReader reader, Database dbconfig) + // Called by Load. Double-check ordinals when changes are made. + private GuildConfiguration(DbDataReader reader) { - _db = dbconfig; - GuildId = (ulong)reader.GetInt64(0); - if (!reader.IsDBNull(1)) - { - _bdayRole = (ulong)reader.GetInt64(1); - } - if (!reader.IsDBNull(2)) _announceCh = (ulong)reader.GetInt64(2); - _tz = reader.IsDBNull(3) ? null : reader.GetString(3); - _moderated = reader.GetBoolean(4); - if (!reader.IsDBNull(5)) _modRole = (ulong)reader.GetInt64(5); - _announceMsg = reader.IsDBNull(6) ? null : reader.GetString(6); - _announceMsgPl = reader.IsDBNull(7) ? null : reader.GetString(7); - _announcePing = reader.GetBoolean(8); - - // Get user information loaded up. - var userresult = GuildUserSettings.GetGuildUsersAsync(dbconfig, GuildId); - _userCache = new Dictionary(); - foreach (var item in userresult) - { - _userCache.Add(item.UserId, item); - } + if (!reader.IsDBNull(1)) RoleId = (ulong)reader.GetInt64(1); + if (!reader.IsDBNull(2)) AnnounceChannelId = (ulong)reader.GetInt64(2); + TimeZone = reader.IsDBNull(3) ? null : reader.GetString(3); + IsModerated = reader.GetBoolean(4); + if (!reader.IsDBNull(5)) ModeratorRole = (ulong)reader.GetInt64(5); + string announceMsg = reader.IsDBNull(6) ? null : reader.GetString(6); + string announceMsgPl = reader.IsDBNull(7) ? null : reader.GetString(7); + AnnounceMessages = (announceMsg, announceMsgPl); + AnnouncePing = reader.GetBoolean(8); } /// - /// Gets user information from th is guild. If the user doesn't exist in the backing database, - /// a new instance is created which is capable of adding the user to the database. - /// - /// - /// For users with the Known property set to false, be sure to call - /// if the resulting object is otherwise unused. - /// - public GuildUserSettings GetUser(ulong userId) - { - lock (this) - { - if (_userCache.ContainsKey(userId)) - { - return _userCache[userId]; - } - - // No result. Create a blank entry and add it to the list, - // in case it gets updated and then referenced later. - var blank = new GuildUserSettings(GuildId, userId); - _userCache.Add(userId, blank); - return blank; - } - } - - /// - /// Deletes the user from the backing database. Drops the locally cached entry. - /// - public async Task DeleteUserAsync(ulong userId) - { - GuildUserSettings user = null; - lock (this) - { - if (!_userCache.TryGetValue(userId, out user)) - { - return; - } - _userCache.Remove(userId); - } - await user.DeleteAsync(_db); - } - - /// - /// Checks if the given user is blocked from issuing commands. + /// Checks if the given user exists in the block list. /// If the server is in moderated mode, this always returns true. - /// Does not check if the user is a manager. /// public async Task IsUserBlockedAsync(ulong userId) { if (IsModerated) return true; - // Block list is not cached, thus doing a database lookup - // TODO cache block list? - using (var db = await _db.OpenConnectionAsync()) - { - using (var c = db.CreateCommand()) - { - c.CommandText = $"select * from {BackingTableBans} " - + "where guild_id = @Gid and user_id = @Uid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; - c.Prepare(); - using (var r = await c.ExecuteReaderAsync()) - { - if (await r.ReadAsync()) return true; - return false; - } - } - } - } - - /// - /// Checks if the given user is a moderator either by having the Manage Server permission or - /// being in the designated moderator role. - /// - public bool IsUserModerator(SocketGuildUser user) - { - if (user.GuildPermissions.ManageGuild) return true; - lock (this) - { - if (ModeratorRole.HasValue) - { - if (user.Roles.Where(r => r.Id == ModeratorRole.Value).Count() > 0) return true; - } - } - + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"select * from {BackingTableBans} " + + "where guild_id = @Gid and user_id = @Uid"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; + c.Prepare(); + using var r = await c.ExecuteReaderAsync(); + if (await r.ReadAsync()) return true; return false; } /// - /// Adds the specified user to the block list, preventing them from issuing commands. + /// Adds the specified user to the block list corresponding to this guild. /// public async Task BlockUserAsync(ulong userId) { - // TODO cache block list? - using (var db = await _db.OpenConnectionAsync()) - { - using (var c = db.CreateCommand()) - { - c.CommandText = $"insert into {BackingTableBans} (guild_id, user_id) " - + "values (@Gid, @Uid) " - + "on conflict (guild_id, user_id) do nothing"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; - c.Prepare(); - await c.ExecuteNonQueryAsync(); - } - } + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"insert into {BackingTableBans} (guild_id, user_id) " + + "values (@Gid, @Uid) " + + "on conflict (guild_id, user_id) do nothing"; + // There is no validation on whether the requested user is even in the guild. will this be a problem? + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; + c.Prepare(); + await c.ExecuteNonQueryAsync(); } - public async Task UnbanUserAsync(ulong userId) + /// + /// Removes the specified user from the block list corresponding to this guild. + /// + /// True if a user has been removed, false if the requested user was not in this list. + public async Task UnblockUserAsync(ulong userId) { - // TODO cache block list? - using (var db = await _db.OpenConnectionAsync()) - { - using (var c = db.CreateCommand()) - { - c.CommandText = $"delete from {BackingTableBans} where " - + "guild_id = @Gid and user_id = @Uid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; - c.Prepare(); - await c.ExecuteNonQueryAsync(); - } - } - } - - public void UpdateRole(ulong roleId) - { - lock (this) - { - _bdayRole = roleId; - UpdateDatabase(); - } - } - - public void UpdateAnnounceChannel(ulong? channelId) - { - lock (this) - { - _announceCh = channelId; - UpdateDatabase(); - } - } - - public void UpdateTimeZone(string tzString) - { - lock (this) - { - _tz = tzString; - UpdateDatabase(); - } - } - - public void UpdateModeratedMode(bool isModerated) - { - lock (this) - { - _moderated = isModerated; - UpdateDatabase(); - } - } - - public void UpdateModeratorRole(ulong? roleId) - { - lock (this) - { - _modRole = roleId; - UpdateDatabase(); - } - } - - public void UpdateAnnounceMessage(string message, bool plural) - { - lock (this) - { - if (plural) _announceMsgPl = message; - else _announceMsg = message; - - UpdateDatabase(); - } - } - - public void UpdateAnnouncePing(bool value) - { - lock (this) - { - _announcePing = value; - UpdateDatabase(); - } + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"delete from {BackingTableBans} where " + + "guild_id = @Gid and user_id = @Uid"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; + c.Prepare(); + var result = await c.ExecuteNonQueryAsync(); + return result != 0; } #region Database public const string BackingTable = "settings"; public const string BackingTableBans = "banned_users"; - internal static void SetUpDatabaseTable(NpgsqlConnection db) + internal static async Task DatabaseSetupAsync(NpgsqlConnection db) { using (var c = db.CreateCommand()) { @@ -316,7 +148,7 @@ namespace BirthdayBot.Data + "announce_ping boolean not null default FALSE, " + "last_seen timestamptz not null default NOW()" + ")"; - c.ExecuteNonQuery(); + await c.ExecuteNonQueryAsync(); } using (var c = db.CreateCommand()) { @@ -325,104 +157,92 @@ namespace BirthdayBot.Data + "user_id bigint not null, " + "PRIMARY KEY (guild_id, user_id)" + ")"; - c.ExecuteNonQuery(); + await c.ExecuteNonQueryAsync(); } } /// - /// Retrieves an object instance representative of guild settings for the specified guild. - /// If settings for the given guild do not yet exist, a new value is created. + /// Fetches guild settings from the database. If no corresponding entry exists, it will be created. /// - internal async static Task LoadSettingsAsync(Database dbsettings, ulong guild) + public static async Task LoadAsync(ulong guildId) { - using (var db = await dbsettings.OpenConnectionAsync()) + using (var db = await Database.OpenConnectionAsync()) { using (var c = db.CreateCommand()) { - // Take note of ordinals for use in the constructor + // Take note of ordinals for the constructor c.CommandText = "select guild_id, role_id, channel_announce_id, time_zone, " + " moderated, moderator_role, announce_message, announce_message_pl, announce_ping " + $"from {BackingTable} where guild_id = @Gid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guild; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; c.Prepare(); - using (var r = await c.ExecuteReaderAsync()) - { - if (await r.ReadAsync()) - { - return new GuildStateInformation(r, dbsettings); - } - } + using var r = await c.ExecuteReaderAsync(); + if (await r.ReadAsync()) return new GuildConfiguration(r); } - // If we got here, no row exists. Create it. + // If we got here, no row exists. Create it with default values. using (var c = db.CreateCommand()) { c.CommandText = $"insert into {BackingTable} (guild_id) values (@Gid)"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guild; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; c.Prepare(); await c.ExecuteNonQueryAsync(); } } - - // New row created. Try this again. - return await LoadSettingsAsync(dbsettings, guild); + // With a new row created, try this again + return await LoadAsync(guildId); } /// - /// Updates the backing database with values from this instance - /// This is a non-asynchronous operation. That may be bad. + /// Updates values on the backing database with values from this object instance. /// - private void UpdateDatabase() + public async Task UpdateAsync() { - using (var db = _db.OpenConnectionAsync().GetAwaiter().GetResult()) - { - using (var c = db.CreateCommand()) - { - c.CommandText = $"update {BackingTable} set " - + "role_id = @RoleId, " - + "channel_announce_id = @ChannelId, " - + "time_zone = @TimeZone, " - + "moderated = @Moderated, " - + "moderator_role = @ModRole, " - + "announce_message = @AnnounceMsg, " - + "announce_message_pl = @AnnounceMsgPl, " - + "announce_ping = @AnnouncePing " - + "where guild_id = @Gid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; - NpgsqlParameter p; + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"update {BackingTable} set " + + "role_id = @RoleId, " + + "channel_announce_id = @ChannelId, " + + "time_zone = @TimeZone, " + + "moderated = @Moderated, " + + "moderator_role = @ModRole, " + + "announce_message = @AnnounceMsg, " + + "announce_message_pl = @AnnounceMsgPl, " + + "announce_ping = @AnnouncePing " + + "where guild_id = @Gid"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; + NpgsqlParameter p; - p = c.Parameters.Add("@RoleId", NpgsqlDbType.Bigint); - if (RoleId.HasValue) p.Value = (long)RoleId.Value; - else p.Value = DBNull.Value; + p = c.Parameters.Add("@RoleId", NpgsqlDbType.Bigint); + if (RoleId.HasValue) p.Value = (long)RoleId.Value; + else p.Value = DBNull.Value; - p = c.Parameters.Add("@ChannelId", NpgsqlDbType.Bigint); - if (_announceCh.HasValue) p.Value = (long)_announceCh.Value; - else p.Value = DBNull.Value; + p = c.Parameters.Add("@ChannelId", NpgsqlDbType.Bigint); + if (AnnounceChannelId.HasValue) p.Value = (long)AnnounceChannelId.Value; + else p.Value = DBNull.Value; - p = c.Parameters.Add("@TimeZone", NpgsqlDbType.Text); - if (_tz != null) p.Value = _tz; - else p.Value = DBNull.Value; + p = c.Parameters.Add("@TimeZone", NpgsqlDbType.Text); + if (TimeZone != null) p.Value = TimeZone; + else p.Value = DBNull.Value; - c.Parameters.Add("@Moderated", NpgsqlDbType.Boolean).Value = _moderated; + c.Parameters.Add("@Moderated", NpgsqlDbType.Boolean).Value = IsModerated; - p = c.Parameters.Add("@ModRole", NpgsqlDbType.Bigint); - if (ModeratorRole.HasValue) p.Value = (long)ModeratorRole.Value; - else p.Value = DBNull.Value; + p = c.Parameters.Add("@ModRole", NpgsqlDbType.Bigint); + if (ModeratorRole.HasValue) p.Value = (long)ModeratorRole.Value; + else p.Value = DBNull.Value; - p = c.Parameters.Add("@AnnounceMsg", NpgsqlDbType.Text); - if (_announceMsg != null) p.Value = _announceMsg; - else p.Value = DBNull.Value; + p = c.Parameters.Add("@AnnounceMsg", NpgsqlDbType.Text); + if (AnnounceMessages.Item1 != null) p.Value = AnnounceMessages.Item1; + else p.Value = DBNull.Value; - p = c.Parameters.Add("@AnnounceMsgPl", NpgsqlDbType.Text); - if (_announceMsgPl != null) p.Value = _announceMsgPl; - else p.Value = DBNull.Value; + p = c.Parameters.Add("@AnnounceMsgPl", NpgsqlDbType.Text); + if (AnnounceMessages.Item2 != null) p.Value = AnnounceMessages.Item2; + else p.Value = DBNull.Value; - c.Parameters.Add("@AnnouncePing", NpgsqlDbType.Boolean).Value = _announcePing; + c.Parameters.Add("@AnnouncePing", NpgsqlDbType.Boolean).Value = AnnouncePing; - c.Prepare(); - c.ExecuteNonQuery(); - } - } + c.Prepare(); + c.ExecuteNonQuery(); } #endregion } From f71552c46d9ab77683b0c4cd485e64c714181f36 Mon Sep 17 00:00:00 2001 From: Noi Date: Thu, 16 Jul 2020 11:55:26 -0700 Subject: [PATCH 03/19] Change Database to be a static class --- Configuration.cs | 2 +- Data/Database.cs | 35 ++++++++++++----------------------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/Configuration.cs b/Configuration.cs index efcc2cc..04f01ed 100644 --- a/Configuration.cs +++ b/Configuration.cs @@ -52,7 +52,7 @@ namespace BirthdayBot var sqlcs = jc["SqlConnectionString"]?.Value(); if (string.IsNullOrWhiteSpace(sqlcs)) throw new Exception("'SqlConnectionString' must be specified."); - DatabaseSettings = new Database(sqlcs); + Database.DBConnectionString = sqlcs; int? sc = jc["ShardCount"]?.Value(); if (!sc.HasValue) ShardCount = 1; diff --git a/Data/Database.cs b/Data/Database.cs index 90a82af..2581669 100644 --- a/Data/Database.cs +++ b/Data/Database.cs @@ -1,42 +1,31 @@ using Npgsql; +using System; using System.Threading.Tasks; namespace BirthdayBot.Data { /// - /// Some database abstractions. + /// Database access and some abstractions. /// - class Database + internal static class Database { - /* - * Database storage in this project, explained: - * Each guild gets a row in the settings table. This table is referred to when doing most things. - * Within each guild, each known user gets a row in the users table with specific information specified. - * Users can override certain settings in global, such as time zone. - */ + public static string DBConnectionString { get; set; } - private string DBConnectionString { get; } - - public Database(string connString) - { - DBConnectionString = connString; - - // Database initialization happens here as well. - SetupTables(); - } - - public async Task OpenConnectionAsync() + public static async Task OpenConnectionAsync() { + if (DBConnectionString == null) throw new Exception("Database connection string not set"); var db = new NpgsqlConnection(DBConnectionString); await db.OpenAsync(); return db; } - private void SetupTables() + public static async Task DoInitialDatabaseSetupAsync() { - using var db = OpenConnectionAsync().GetAwaiter().GetResult(); - GuildStateInformation.SetUpDatabaseTable(db); // Note: Call this first. (Foreign reference constraints.) - GuildUserSettings.SetUpDatabaseTable(db); + using var db = await OpenConnectionAsync(); + + // Refer to the methods being called for information on how the database is set up. + await GuildConfiguration.DatabaseSetupAsync(db); // Note: Call this first. (Foreign reference constraints.) + await GuildUserConfiguration.DatabaseSetupAsync(db); } } } From 7ac15e21a103d3afe4ee10749f1061ba5488b122 Mon Sep 17 00:00:00 2001 From: Noi Date: Thu, 16 Jul 2020 12:23:40 -0700 Subject: [PATCH 04/19] Remove guild cache from BirthdayBot Guild information no longer loaded on join, but instead on command invocation. This information is to be passed on directly to the command handler. --- BirthdayBot.cs | 58 +++++++++++++-------------------- UserInterface/CommandsCommon.cs | 5 +-- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/BirthdayBot.cs b/BirthdayBot.cs index 6b58c60..bb8e7c2 100644 --- a/BirthdayBot.cs +++ b/BirthdayBot.cs @@ -5,8 +5,8 @@ using Discord.Net; using Discord.Webhook; using Discord.WebSocket; using System; -using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using static BirthdayBot.UserInterface.CommandsCommon; @@ -20,20 +20,20 @@ namespace BirthdayBot private readonly HelpInfoCommands _cmdsHelp; private readonly ManagerCommands _cmdsMods; - private BackgroundServiceRunner _worker; + private readonly BackgroundServiceRunner _worker; internal Configuration Config { get; } internal DiscordShardedClient DiscordClient { get; } - // TODO consider removal of the guild cache - internal ConcurrentDictionary GuildCache { get; } internal DiscordWebhookClient LogWebhook { get; } + /// + /// Prepares the bot connection and all its event handlers + /// public BirthdayBot(Configuration conf, DiscordShardedClient dc) { Config = conf; DiscordClient = dc; LogWebhook = new DiscordWebhookClient(conf.LogWebhook); - GuildCache = new ConcurrentDictionary(); _worker = new BackgroundServiceRunner(this); @@ -49,15 +49,17 @@ namespace BirthdayBot foreach (var item in _cmdsMods.Commands) _dispatchCommands.Add(item.Item1, item.Item2); // Register event handlers - DiscordClient.JoinedGuild += LoadGuild; - DiscordClient.GuildAvailable += LoadGuild; - DiscordClient.LeftGuild += DiscardGuild; DiscordClient.ShardConnected += SetStatus; DiscordClient.MessageReceived += Dispatch; } + /// + /// Does some more basic initialization and then connects to Discord + /// public async Task Start() { + await Database.DoInitialDatabaseSetupAsync(); + await DiscordClient.LoginAsync(TokenType.Bot, Config.BotToken); await DiscordClient.StartAsync(); @@ -76,21 +78,6 @@ namespace BirthdayBot DiscordClient.Dispose(); } - private async Task LoadGuild(SocketGuild g) - { - if (!GuildCache.ContainsKey(g.Id)) - { - var gi = await GuildStateInformation.LoadSettingsAsync(Config.DatabaseSettings, g.Id); - GuildCache.TryAdd(g.Id, gi); - } - } - - private Task DiscardGuild(SocketGuild g) - { - GuildCache.TryRemove(g.Id, out _); - return Task.CompletedTask; - } - private async Task SetStatus(DiscordSocketClient shard) => await shard.SetGameAsync(CommandPrefix + "help"); public async Task PushErrorLog(string source, string message) @@ -114,9 +101,10 @@ namespace BirthdayBot private async Task Dispatch(SocketMessage msg) { - if (msg.Channel is IDMChannel) return; - if (msg.Author.IsBot) return; - // TODO determine message type (pin, join, etc) + if (!(msg.Channel is SocketTextChannel channel)) return; + if (msg.Author.IsBot || msg.Author.IsWebhook) return; + if (((IMessage)msg).Type != MessageType.Default) return; + var author = (SocketGuildUser)msg.Author; // Limit 3: // For all cases: base command, 2 parameters. @@ -124,27 +112,25 @@ namespace BirthdayBot var csplit = msg.Content.Split(" ", 3, StringSplitOptions.RemoveEmptyEntries); if (csplit.Length > 0 && csplit[0].StartsWith(CommandPrefix, StringComparison.OrdinalIgnoreCase)) { - var channel = (SocketTextChannel)msg.Channel; - var author = (SocketGuildUser)msg.Author; - // Determine if it's something we're listening for. // Doing this first before the block check because a block check triggers a database query. - CommandHandler command = null; - if (!_dispatchCommands.TryGetValue(csplit[0].Substring(CommandPrefix.Length), out command)) return; + if (!_dispatchCommands.TryGetValue(csplit[0].Substring(CommandPrefix.Length), out CommandHandler command)) return; + + // Load guild information here + var gconf = await GuildConfiguration.LoadAsync(channel.Guild.Id); // Ban check - var gi = GuildCache[channel.Guild.Id]; - // Skip ban check if user is a manager - if (!gi.IsUserModerator(author)) + bool isMod = gconf.ModeratorRole.HasValue && author.Roles.Any(r => r.Id == gconf.ModeratorRole.Value); + if (!isMod) // skip check if user is a moderator { - if (gi.IsUserBlockedAsync(author.Id).GetAwaiter().GetResult()) return; + if (await gconf.IsUserBlockedAsync(author.Id)) return; // silently ignore } // Execute the command try { Program.Log("Command", $"{channel.Guild.Name}/{author.Username}#{author.Discriminator}: {msg.Content}"); - await command(csplit, channel, author); + await command(csplit, gconf, channel, author); } catch (Exception ex) { diff --git a/UserInterface/CommandsCommon.cs b/UserInterface/CommandsCommon.cs index 4336c0f..4e92e80 100644 --- a/UserInterface/CommandsCommon.cs +++ b/UserInterface/CommandsCommon.cs @@ -1,4 +1,5 @@ -using Discord.WebSocket; +using BirthdayBot.Data; +using Discord.WebSocket; using NodaTime; using System; using System.Collections.Generic; @@ -22,7 +23,7 @@ namespace BirthdayBot.UserInterface public const string NoParameterError = ":x: This command does not accept any parameters."; public const string InternalError = ":x: An internal bot error occurred. The bot maintainer has been notified of the issue."; - public delegate Task CommandHandler(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser); + public delegate Task CommandHandler(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser); protected static Dictionary TzNameMap { get { From 488685bc79d9c1aff67fccc7c4a22df6bafd67f8 Mon Sep 17 00:00:00 2001 From: Noi Date: Thu, 16 Jul 2020 13:21:33 -0700 Subject: [PATCH 05/19] Rewrite GuildUserConfiguration Corresponding to GuildConfiguration, no longer assumes it is cached. --- Data/GuildUserConfiguration.cs | 188 +++++++++++++++------------------ 1 file changed, 85 insertions(+), 103 deletions(-) diff --git a/Data/GuildUserConfiguration.cs b/Data/GuildUserConfiguration.cs index f923fb8..b227a46 100644 --- a/Data/GuildUserConfiguration.cs +++ b/Data/GuildUserConfiguration.cs @@ -8,161 +8,143 @@ using System.Threading.Tasks; namespace BirthdayBot.Data { /// - /// Representation of a user's birthday settings within a guild. - /// Instances are held and managed by . + /// Represents configuration for a guild user. /// - class GuildUserSettings + class GuildUserConfiguration { - private int _month; - private int _day; - private string _tz; - public ulong GuildId { get; } public ulong UserId { get; } /// /// Month of birth as a numeric value. Range 1-12. /// - public int BirthMonth { get { return _month; } } + public int BirthMonth { get; private set; } /// /// Day of birth as a numeric value. Ranges between 1-31 or lower based on month value. /// - public int BirthDay { get { return _day; } } + public int BirthDay { get; private set; } - public string TimeZone { get { return _tz; } } - public bool IsKnown { get { return _month != 0 && _day != 0; } } + public string TimeZone { get; private set; } + public bool IsKnown { get { return BirthMonth != 0 && BirthDay != 0; } } /// - /// Creates a data-less instance without any useful information. + /// Creates a new, data-less instance without a corresponding database entry. /// Calling will create a real database enty /// - public GuildUserSettings(ulong guildId, ulong userId) + private GuildUserConfiguration(ulong guildId, ulong userId) { GuildId = guildId; UserId = userId; } // Called by GetGuildUsersAsync. Double-check ordinals when changes are made. - private GuildUserSettings(DbDataReader reader) + private GuildUserConfiguration(DbDataReader reader) { GuildId = (ulong)reader.GetInt64(0); UserId = (ulong)reader.GetInt64(1); - _month = reader.GetInt32(2); - _day = reader.GetInt32(3); - if (!reader.IsDBNull(4)) _tz = reader.GetString(4); + BirthMonth = reader.GetInt32(2); + BirthDay = reader.GetInt32(3); + if (!reader.IsDBNull(4)) TimeZone = reader.GetString(4); } /// /// Updates user with given information. - /// NOTE: If there exists a tz value and the update contains none, the old tz value is retained. /// - public async Task UpdateAsync(int month, int day, string newtz, Database dbconfig) + public async Task UpdateAsync(int month, int day, string newtz) { - // TODO note from rewrite: huh? why are we doing this here? - var inserttz = newtz ?? TimeZone; - - using (var db = await dbconfig.OpenConnectionAsync()) + using (var db = await Database.OpenConnectionAsync()) { - // Will do a delete/insert instead of insert...on conflict update. Because lazy. - using (var t = db.BeginTransaction()) - { - await DoDeleteAsync(db); - using (var c = db.CreateCommand()) - { - c.CommandText = $"insert into {BackingTable} " - + "(guild_id, user_id, birth_month, birth_day, time_zone) values " - + "(@Gid, @Uid, @Month, @Day, @Tz)"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)UserId; - c.Parameters.Add("@Month", NpgsqlDbType.Numeric).Value = month; - c.Parameters.Add("@Day", NpgsqlDbType.Numeric).Value = day; - var p = c.Parameters.Add("@Tz", NpgsqlDbType.Text); - if (inserttz != null) p.Value = inserttz; - else p.Value = DBNull.Value; - c.Prepare(); - await c.ExecuteNonQueryAsync(); - } - await t.CommitAsync(); - } + using var c = db.CreateCommand(); + c.CommandText = $"insert into {BackingTable} " + + "(guild_id, user_id, birth_month, birth_day, time_zone) values " + + "(@Gid, @Uid, @Month, @Day, @Tz) " + + "on conflict (guild_id, user_id) do update " + + "set birth_month = EXCLUDED.birth_month, birth_day = EXCLUDED.birth_day, time_zone = EXCLUDED.time_zone"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)UserId; + c.Parameters.Add("@Month", NpgsqlDbType.Numeric).Value = month; + c.Parameters.Add("@Day", NpgsqlDbType.Numeric).Value = day; + var tzp = c.Parameters.Add("@Tz", NpgsqlDbType.Text); + if (newtz != null) tzp.Value = newtz; + else tzp.Value = DBNull.Value; + c.Prepare(); + await c.ExecuteNonQueryAsync(); } - // We didn't crash! Get the new values stored locally. - _month = month; - _day = day; - _tz = inserttz; + // Database update succeeded; update instance values + BirthMonth = month; + BirthDay = day; + TimeZone = newtz; } /// /// Deletes information of this user from the backing database. /// The corresponding object reference should ideally be discarded after calling this. /// - public async Task DeleteAsync(Database dbconfig) + public async Task DeleteAsync() { - using (var db = await dbconfig.OpenConnectionAsync()) - { - await DoDeleteAsync(db); - } - } - - // Shared between UpdateAsync and DeleteAsync - private async Task DoDeleteAsync(NpgsqlConnection dbconn) - { - using (var c = dbconn.CreateCommand()) - { - c.CommandText = $"delete from {BackingTable} " - + "where guild_id = @Gid and user_id = @Uid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; - c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)UserId; - c.Prepare(); - await c.ExecuteNonQueryAsync(); - } + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"delete from {BackingTable} " + + "where guild_id = @Gid and user_id = @Uid"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)GuildId; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)UserId; + c.Prepare(); + await c.ExecuteNonQueryAsync(); } #region Database public const string BackingTable = "user_birthdays"; + // Take note of ordinals for use in the constructor + private const string SelectFields = "guild_id, user_id, birth_month, birth_day, time_zone"; - internal static void SetUpDatabaseTable(NpgsqlConnection db) + internal static async Task DatabaseSetupAsync(NpgsqlConnection db) { - using (var c = db.CreateCommand()) - { - c.CommandText = $"create table if not exists {BackingTable} (" - + $"guild_id bigint not null references {GuildStateInformation.BackingTable} ON DELETE CASCADE, " - + "user_id bigint not null, " - + "birth_month integer not null, " - + "birth_day integer not null, " - + "time_zone text null, " - + "last_seen timestamptz not null default NOW(), " - + "PRIMARY KEY (guild_id, user_id)" - + ")"; - c.ExecuteNonQuery(); - } + using var c = db.CreateCommand(); + c.CommandText = $"create table if not exists {BackingTable} (" + + $"guild_id bigint not null references {GuildConfiguration.BackingTable} ON DELETE CASCADE, " + + "user_id bigint not null, " + + "birth_month integer not null, " + + "birth_day integer not null, " + + "time_zone text null, " + + "last_seen timestamptz not null default NOW(), " + + "PRIMARY KEY (guild_id, user_id)" // index automatically created with this + + ")"; + await c.ExecuteNonQueryAsync(); } /// - /// Gets all known birthday records from the specified guild. No further filtering is done here. + /// Attempts to retrieve a user's configuration. Returns a new, updateable instance if none is found. /// - internal static IEnumerable GetGuildUsersAsync(Database dbsettings, ulong guildId) + public static async Task LoadAsync(ulong guildId, ulong userId) { - using (var db = dbsettings.OpenConnectionAsync().GetAwaiter().GetResult()) - { - using (var c = db.CreateCommand()) - { - // Take note of ordinals for use in the constructor - c.CommandText = "select guild_id, user_id, birth_month, birth_day, time_zone " - + $"from {BackingTable} where guild_id = @Gid"; - c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; - c.Prepare(); - using (var r = c.ExecuteReader()) - { - var result = new List(); - while (r.Read()) - { - result.Add(new GuildUserSettings(r)); - } - return result; - } - } - } + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"select {SelectFields} from {BackingTable} where guild_id = @Gid and user_id = @Uid"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; + c.Parameters.Add("@Uid", NpgsqlDbType.Bigint).Value = (long)userId; + c.Prepare(); + + using var r = c.ExecuteReader(); + if (await r.ReadAsync()) return new GuildUserConfiguration(r); + else return new GuildUserConfiguration(guildId, userId); + } + + /// + /// Gets all known user configuration records associated with the specified guild. + /// + public static async Task> LoadAllAsync(ulong guildId) + { + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = $"select {SelectFields} from {BackingTable} where guild_id = @Gid"; + c.Parameters.Add("@Gid", NpgsqlDbType.Bigint).Value = (long)guildId; + c.Prepare(); + + using var r = await c.ExecuteReaderAsync(); + var result = new List(); + while (await r.ReadAsync()) result.Add(new GuildUserConfiguration(r)); + return result; } #endregion } From 608ab37aecbf6dd4dd59cda9ab18102af8ca7c1c Mon Sep 17 00:00:00 2001 From: Noi Date: Thu, 16 Jul 2020 14:48:09 -0700 Subject: [PATCH 06/19] Update background services to remove guild cache references Possibly optimized StaleDataCleaner as well by making certain database updates asynchronous --- BackgroundServices/BirthdayRoleUpdate.cs | 24 ++--- BackgroundServices/GuildStatistics.cs | 3 +- BackgroundServices/StaleDataCleaner.cs | 109 +++++++++++------------ 3 files changed, 62 insertions(+), 74 deletions(-) diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index ae9f7e7..4393f3e 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -66,23 +66,17 @@ namespace BirthdayBot.BackgroundServices { 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)) - { - diag.FetchCachedGuild = "Server information not yet loaded by the bot. Try again later."; - return diag; - } - diag.FetchCachedGuild = null; + var gc = await GuildConfiguration.LoadAsync(guild.Id); // Check if role settings are correct before continuing with further processing SocketRole role = null; - if (gs.RoleId.HasValue) role = guild.GetRole(gs.RoleId.Value); + if (gc.RoleId.HasValue) role = guild.GetRole(gc.RoleId.Value); 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 users = await GuildUserConfiguration.LoadAllAsync(guild.Id); + var tz = gc.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(); @@ -103,10 +97,10 @@ namespace BirthdayBot.BackgroundServices diag.RoleApply = null; // Birthday announcement - var announce = gs.AnnounceMessages; - var announceping = gs.AnnouncePing; + var announce = gc.AnnounceMessages; + var announceping = gc.AnnouncePing; SocketTextChannel channel = null; - if (gs.AnnounceChannelId.HasValue) channel = guild.GetTextChannel(gs.AnnounceChannelId.Value); + if (gc.AnnounceChannelId.HasValue) channel = guild.GetTextChannel(gc.AnnounceChannelId.Value); if (announcementList.Count() != 0) { var announceResult = await AnnounceBirthdaysAsync(announce, announceping, channel, announcementList); @@ -145,7 +139,7 @@ namespace BirthdayBot.BackgroundServices /// Gets all known users from the given guild and returns a list including only those who are /// currently experiencing a birthday in the respective time zone. /// - private HashSet GetGuildCurrentBirthdays(IEnumerable guildUsers, string defaultTzStr) + private HashSet GetGuildCurrentBirthdays(IEnumerable guildUsers, string defaultTzStr) { var birthdayUsers = new HashSet(); @@ -271,7 +265,6 @@ namespace BirthdayBot.BackgroundServices { const string DefaultValue = "--"; - public string FetchCachedGuild = DefaultValue; public string RoleCheck = DefaultValue; public string CurrentBirthdays = DefaultValue; public string RoleApply = DefaultValue; @@ -282,7 +275,6 @@ namespace BirthdayBot.BackgroundServices { 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:")); diff --git a/BackgroundServices/GuildStatistics.cs b/BackgroundServices/GuildStatistics.cs index 3c93676..2b472fc 100644 --- a/BackgroundServices/GuildStatistics.cs +++ b/BackgroundServices/GuildStatistics.cs @@ -14,8 +14,7 @@ namespace BirthdayBot.BackgroundServices public async override Task OnTick() { var count = BotInstance.DiscordClient.Guilds.Count; - var cacheCount = BotInstance.GuildCache.Count; - Log($"Currently in {count} guilds. Cached guild settings: {cacheCount}."); + Log($"Currently in {count} guilds."); await SendExternalStatistics(count); } diff --git a/BackgroundServices/StaleDataCleaner.cs b/BackgroundServices/StaleDataCleaner.cs index 6a8a5dc..05d42e5 100644 --- a/BackgroundServices/StaleDataCleaner.cs +++ b/BackgroundServices/StaleDataCleaner.cs @@ -17,73 +17,70 @@ namespace BirthdayBot.BackgroundServices { // Build a list of all values to update var updateList = new Dictionary>(); - foreach (var gi in BotInstance.GuildCache) + foreach (var g in BotInstance.DiscordClient.Guilds) { var existingUsers = new List(); - updateList[gi.Key] = existingUsers; + updateList[g.Id] = existingUsers; - var guild = BotInstance.DiscordClient.GetGuild(gi.Key); - if (guild == null) continue; // Have cache without being in guild. Unlikely, but... - - // Get IDs of cached users which are currently in the guild - var cachedUserIds = from cu in gi.Value.Users select cu.UserId; - var guildUserIds = from gu in guild.Users select gu.Id; - var existingCachedIds = cachedUserIds.Intersect(guildUserIds); + // Get list of IDs for all users who exist in the database and currently exist in the guild + var savedUserIds = from cu in await GuildUserConfiguration.LoadAllAsync(g.Id) select cu.UserId; + var guildUserIds = from gu in g.Users select gu.Id; + var existingCachedIds = savedUserIds.Intersect(guildUserIds); } - using (var db = await BotInstance.Config.DatabaseSettings.OpenConnectionAsync()) + using var db = await Database.OpenConnectionAsync(); + + // Statement for updating last_seen in guilds + var cUpdateGuild = db.CreateCommand(); + cUpdateGuild.CommandText = $"update {GuildConfiguration.BackingTable} set last_seen = now() " + + "where guild_id = @Gid"; + var pUpdateG = cUpdateGuild.Parameters.Add("@Gid", NpgsqlDbType.Bigint); + cUpdateGuild.Prepare(); + + // Statement for updating last_seen in guild users + var cUpdateGuildUser = db.CreateCommand(); + cUpdateGuildUser.CommandText = $"update {GuildUserConfiguration.BackingTable} set last_seen = now() " + + "where guild_id = @Gid and user_id = @Uid"; + var pUpdateGU_g = cUpdateGuildUser.Parameters.Add("@Gid", NpgsqlDbType.Bigint); + var pUpdateGU_u = cUpdateGuildUser.Parameters.Add("@Uid", NpgsqlDbType.Bigint); + cUpdateGuildUser.Prepare(); + + // Do actual updates + var updates = new List(); + foreach (var item in updateList) { - // Prepare to update a lot of last-seen values - var cUpdateGuild = db.CreateCommand(); - cUpdateGuild.CommandText = $"update {GuildStateInformation.BackingTable} set last_seen = now() " - + "where guild_id = @Gid"; - var pUpdateG = cUpdateGuild.Parameters.Add("@Gid", NpgsqlDbType.Bigint); - cUpdateGuild.Prepare(); + var guild = item.Key; + var userlist = item.Value; - var cUpdateGuildUser = db.CreateCommand(); - cUpdateGuildUser.CommandText = $"update {GuildUserSettings.BackingTable} set last_seen = now() " - + "where guild_id = @Gid and user_id = @Uid"; - var pUpdateGU_g = cUpdateGuildUser.Parameters.Add("@Gid", NpgsqlDbType.Bigint); - var pUpdateGU_u = cUpdateGuildUser.Parameters.Add("@Uid", NpgsqlDbType.Bigint); - cUpdateGuildUser.Prepare(); + pUpdateG.Value = (long)guild; + updates.Add(cUpdateGuild.ExecuteNonQueryAsync()); - // Do actual updates - foreach (var item in updateList) + pUpdateGU_g.Value = (long)guild; + foreach (var userid in userlist) { - var guild = item.Key; - var userlist = item.Value; - - pUpdateG.Value = (long)guild; - cUpdateGuild.ExecuteNonQuery(); - - pUpdateGU_g.Value = (long)guild; - foreach (var userid in userlist) - { - pUpdateGU_u.Value = (long)userid; - cUpdateGuildUser.ExecuteNonQuery(); - } - } - - // Delete all old values - expects referencing tables to have 'on delete cascade' - using (var t = db.BeginTransaction()) - { - int staleGuilds, staleUsers; - using (var c = db.CreateCommand()) - { - // Delete data for guilds not seen in 4 weeks - c.CommandText = $"delete from {GuildStateInformation.BackingTable} where (now() - interval '28 days') > last_seen"; - staleGuilds = c.ExecuteNonQuery(); - } - using (var c = db.CreateCommand()) - { - // Delete data for users not seen in 8 weeks - c.CommandText = $"delete from {GuildUserSettings.BackingTable} where (now() - interval '56 days') > last_seen"; - staleUsers = c.ExecuteNonQuery(); - } - Log($"Will remove {staleGuilds} guilds, {staleUsers} users."); - t.Commit(); + pUpdateGU_u.Value = (long)userid; + updates.Add(cUpdateGuildUser.ExecuteNonQueryAsync()); } } + await Task.WhenAll(updates); + + // Delete all old values - expects referencing tables to have 'on delete cascade' + using var t = db.BeginTransaction(); + int staleGuilds, staleUsers; + using (var c = db.CreateCommand()) + { + // Delete data for guilds not seen in 4 weeks + c.CommandText = $"delete from {GuildConfiguration.BackingTable} where (now() - interval '28 days') > last_seen"; + staleGuilds = c.ExecuteNonQuery(); + } + using (var c = db.CreateCommand()) + { + // Delete data for users not seen in 8 weeks + c.CommandText = $"delete from {GuildUserConfiguration.BackingTable} where (now() - interval '56 days') > last_seen"; + staleUsers = c.ExecuteNonQuery(); + } + Log($"Will remove {staleGuilds} guilds, {staleUsers} users."); + t.Commit(); } } } From 581df48855888bcdcf5db5e01749b217f6ffcbab Mon Sep 17 00:00:00 2001 From: Noi Date: Sat, 18 Jul 2020 00:31:01 -0700 Subject: [PATCH 07/19] Add IsBotModerator method Very similar to a previously existing method that was mistakenly removed. --- BirthdayBot.cs | 3 +-- Data/GuildConfiguration.cs | 11 ++++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/BirthdayBot.cs b/BirthdayBot.cs index bb8e7c2..51e957a 100644 --- a/BirthdayBot.cs +++ b/BirthdayBot.cs @@ -120,8 +120,7 @@ namespace BirthdayBot var gconf = await GuildConfiguration.LoadAsync(channel.Guild.Id); // Ban check - bool isMod = gconf.ModeratorRole.HasValue && author.Roles.Any(r => r.Id == gconf.ModeratorRole.Value); - if (!isMod) // skip check if user is a moderator + if (!gconf.IsBotModerator(author)) // skip check if user is a moderator { if (await gconf.IsUserBlockedAsync(author.Id)) return; // silently ignore } diff --git a/Data/GuildConfiguration.cs b/Data/GuildConfiguration.cs index 1f35645..b84487a 100644 --- a/Data/GuildConfiguration.cs +++ b/Data/GuildConfiguration.cs @@ -1,7 +1,9 @@ -using Npgsql; +using Discord.WebSocket; +using Npgsql; using NpgsqlTypes; using System; using System.Data.Common; +using System.Linq; using System.Threading.Tasks; namespace BirthdayBot.Data @@ -128,6 +130,13 @@ namespace BirthdayBot.Data return result != 0; } + /// + /// Checks if the given user can be considered a bot moderator. + /// Checks for either the Manage Guild permission or if the user is within a predetermined role. + /// + public bool IsBotModerator(SocketGuildUser user) + => user.GuildPermissions.ManageGuild || (ModeratorRole.HasValue && user.Roles.Any(r => r.Id == ModeratorRole.Value)); + #region Database public const string BackingTable = "settings"; public const string BackingTableBans = "banned_users"; From 6d7db85310a26b537cbcc2043acbe22a6fa72015 Mon Sep 17 00:00:00 2001 From: Noi Date: Sun, 19 Jul 2020 19:01:34 -0700 Subject: [PATCH 08/19] Remove unneeded property --- Configuration.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Configuration.cs b/Configuration.cs index 04f01ed..1606920 100644 --- a/Configuration.cs +++ b/Configuration.cs @@ -14,7 +14,6 @@ namespace BirthdayBot public string BotToken { get; } public string LogWebhook { get; } public string DBotsToken { get; } - public Database DatabaseSettings { get; } public int ShardCount { get; } public Configuration() From 23dd15d741867ebf85bbf5fcd253eae780436f22 Mon Sep 17 00:00:00 2001 From: Noi Date: Sun, 19 Jul 2020 19:07:23 -0700 Subject: [PATCH 09/19] Update to use GuildConfiguration; small other tweaks --- UserInterface/ListingCommands.cs | 80 +++++++++------------ UserInterface/ManagerCommands.cs | 119 +++++++++++++++---------------- 2 files changed, 93 insertions(+), 106 deletions(-) diff --git a/UserInterface/ListingCommands.cs b/UserInterface/ListingCommands.cs index 7e2be68..ff88f7d 100644 --- a/UserInterface/ListingCommands.cs +++ b/UserInterface/ListingCommands.cs @@ -35,7 +35,7 @@ namespace BirthdayBot.UserInterface new CommandDocumentation(new string[] { "when" }, "Displays the given user's birthday information.", null); #endregion - private async Task CmdWhen(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdWhen(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // Requires a parameter if (param.Length == 1) @@ -53,8 +53,7 @@ namespace BirthdayBot.UserInterface SocketGuildUser searchTarget = null; - ulong searchId = 0; - if (!TryGetUserId(search, out searchId)) // ID lookup + if (!TryGetUserId(search, out ulong searchId)) // ID lookup { // name lookup without discriminator foreach (var searchuser in reqChannel.Guild.Users) @@ -76,9 +75,8 @@ namespace BirthdayBot.UserInterface return; } - var users = Instance.GuildCache[reqChannel.Guild.Id].Users; - var searchTargetData = users.FirstOrDefault(u => u.UserId == searchTarget.Id); - if (searchTargetData == null) + var searchTargetData = await GuildUserConfiguration.LoadAsync(reqChannel.Guild.Id, searchId); + if (!searchTargetData.IsKnown) { await reqChannel.SendMessageAsync("I do not have birthday information for that user."); return; @@ -93,10 +91,10 @@ namespace BirthdayBot.UserInterface } // Creates a file with all birthdays. - private async Task CmdList(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdList(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // For now, we're restricting this command to moderators only. This may turn into an option later. - if (!Instance.GuildCache[reqChannel.Guild.Id].IsUserModerator(reqUser)) + if (!gconf.IsBotModerator(reqUser)) { // Do not add detailed usage information to this error message. await reqChannel.SendMessageAsync(":x: Only bot moderators may use this command."); @@ -120,7 +118,7 @@ namespace BirthdayBot.UserInterface return; } - var bdlist = await LoadList(reqChannel.Guild, false); + var bdlist = await GetSortedUsersAsync(reqChannel.Guild); var filepath = Path.GetTempPath() + "birthdaybot-" + reqChannel.Guild.Id; string fileoutput; @@ -158,13 +156,13 @@ namespace BirthdayBot.UserInterface // "Recent and upcoming birthdays" // The 'recent' bit removes time zone ambiguity and spares us from extra time zone processing here - private async Task CmdUpcoming(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdUpcoming(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { var now = DateTimeOffset.UtcNow; var search = DateIndex(now.Month, now.Day) - 8; // begin search 8 days prior to current date UTC if (search <= 0) search = 366 - Math.Abs(search); - var query = await LoadList(reqChannel.Guild, true); + var query = await GetSortedUsersAsync(reqChannel.Guild); var output = new StringBuilder(); var resultCount = 0; @@ -219,43 +217,35 @@ namespace BirthdayBot.UserInterface /// Fetches all guild birthdays and places them into an easily usable structure. /// Users currently not in the guild are not included in the result. /// - private async Task> LoadList(SocketGuild guild, bool escapeFormat) + private async Task> GetSortedUsersAsync(SocketGuild guild) { - var ping = Instance.GuildCache[guild.Id].AnnouncePing; - - using (var db = await BotConfig.DatabaseSettings.OpenConnectionAsync()) + using var db = await Database.OpenConnectionAsync(); + using var c = db.CreateCommand(); + c.CommandText = "select user_id, birth_month, birth_day from " + GuildUserConfiguration.BackingTable + + " where guild_id = @Gid order by birth_month, birth_day"; + c.Parameters.Add("@Gid", NpgsqlTypes.NpgsqlDbType.Bigint).Value = (long)guild.Id; + c.Prepare(); + using var r = await c.ExecuteReaderAsync(); + var result = new List(); + while (await r.ReadAsync()) { - using (var c = db.CreateCommand()) + var id = (ulong)r.GetInt64(0); + var month = r.GetInt32(1); + var day = r.GetInt32(2); + + var guildUser = guild.GetUser(id); + if (guildUser == null) continue; // Skip user not in guild + + result.Add(new ListItem() { - c.CommandText = "select user_id, birth_month, birth_day from " + GuildUserSettings.BackingTable - + " where guild_id = @Gid order by birth_month, birth_day"; - c.Parameters.Add("@Gid", NpgsqlTypes.NpgsqlDbType.Bigint).Value = (long)guild.Id; - c.Prepare(); - using (var r = await c.ExecuteReaderAsync()) - { - var result = new List(); - while (await r.ReadAsync()) - { - var id = (ulong)r.GetInt64(0); - var month = r.GetInt32(1); - var day = r.GetInt32(2); - - var guildUser = guild.GetUser(id); - if (guildUser == null) continue; // Skip users not in guild - - result.Add(new ListItem() - { - BirthMonth = month, - BirthDay = day, - DateIndex = DateIndex(month, day), - UserId = guildUser.Id, - DisplayName = Common.FormatName(guildUser, false) - }); - } - return result; - } - } + BirthMonth = month, + BirthDay = day, + DateIndex = DateIndex(month, day), + UserId = guildUser.Id, + DisplayName = Common.FormatName(guildUser, false) + }); } + return result; } private string ListExportNormal(SocketGuildChannel channel, IEnumerable list) @@ -295,7 +285,7 @@ namespace BirthdayBot.UserInterface result.Append(','); if (user.Nickname != null) result.Append(user.Nickname); result.Append(','); - result.Append($"{Common.MonthNames[item.BirthMonth]}-{item.BirthDay.ToString("00")}"); + result.Append($"{Common.MonthNames[item.BirthMonth]}-{item.BirthDay:00}"); result.Append(','); result.Append(item.BirthMonth); result.Append(','); diff --git a/UserInterface/ManagerCommands.cs b/UserInterface/ManagerCommands.cs index 70a0c8c..8870ca9 100644 --- a/UserInterface/ManagerCommands.cs +++ b/UserInterface/ManagerCommands.cs @@ -1,4 +1,5 @@ -using Discord.WebSocket; +using BirthdayBot.Data; +using Discord.WebSocket; using System; using System.Collections.Generic; using System.Linq; @@ -11,7 +12,7 @@ namespace BirthdayBot.UserInterface { private static readonly string ConfErrorPostfix = $" Refer to the `{CommandPrefix}help-config` command for information on this command's usage."; - private delegate Task ConfigSubcommand(string[] param, SocketTextChannel reqChannel); + private delegate Task ConfigSubcommand(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel); private readonly Dictionary _subcommands; private readonly Dictionary _usercommands; @@ -57,11 +58,10 @@ namespace BirthdayBot.UserInterface "Perform certain commands on behalf of another user.", null); #endregion - private async Task CmdConfigDispatch(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdConfigDispatch(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // Ignore those without the proper permissions. - // Requires either the manage guild permission or to be in the moderators role - if (!Instance.GuildCache[reqUser.Guild.Id].IsUserModerator(reqUser)) + if (!gconf.IsBotModerator(reqUser)) { await reqChannel.SendMessageAsync(":x: This command may only be used by bot moderators."); return; @@ -73,7 +73,7 @@ namespace BirthdayBot.UserInterface return; } - // Special case: Restrict 'modrole' to only guild managers + // Special case: Restrict 'modrole' to only guild managers, not mods if (string.Equals(param[1], "modrole", StringComparison.OrdinalIgnoreCase) && !reqUser.GuildPermissions.ManageGuild) { await reqChannel.SendMessageAsync(":x: This command may only be used by those with the `Manage Server` permission."); @@ -86,13 +86,13 @@ namespace BirthdayBot.UserInterface if (_subcommands.TryGetValue(confparam[0], out ConfigSubcommand h)) { - await h(confparam, reqChannel); + await h(confparam, gconf, reqChannel); } } #region Configuration sub-commands // Birthday role set - private async Task ScmdRole(string[] param, SocketTextChannel reqChannel) + private async Task ScmdRole(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { if (param.Length != 2) { @@ -112,13 +112,14 @@ namespace BirthdayBot.UserInterface } else { - Instance.GuildCache[guild.Id].UpdateRole(role.Id); + gconf.RoleId = role.Id; + await gconf.UpdateAsync(); await reqChannel.SendMessageAsync($":white_check_mark: The birthday role has been set as **{role.Name}**."); } } // Ping setting - private async Task ScmdPing(string[] param, SocketTextChannel reqChannel) + private async Task ScmdPing(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { const string InputErr = ":x: You must specify either `off` or `on` in this setting."; if (param.Length != 2) @@ -146,26 +147,25 @@ namespace BirthdayBot.UserInterface return; } - Instance.GuildCache[reqChannel.Guild.Id].UpdateAnnouncePing(setting); + gconf.AnnouncePing = setting; + await gconf.UpdateAsync(); await reqChannel.SendMessageAsync(result); } // Announcement channel set - private async Task ScmdChannel(string[] param, SocketTextChannel reqChannel) + private async Task ScmdChannel(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { - if (param.Length == 1) + if (param.Length == 1) // No extra parameter. Unset announcement channel. { - // No extra parameter. Unset announcement channel. - var gi = Instance.GuildCache[reqChannel.Guild.Id]; - // Extra detail: Show a unique message if a channel hadn't been set prior. - if (!gi.AnnounceChannelId.HasValue) + if (!gconf.AnnounceChannelId.HasValue) { await reqChannel.SendMessageAsync(":x: There is no announcement channel set. Nothing to unset."); return; } - gi.UpdateAnnounceChannel(null); + gconf.AnnounceChannelId = null; + await gconf.UpdateAsync(); await reqChannel.SendMessageAsync(":white_check_mark: The announcement channel has been unset."); } else @@ -204,7 +204,8 @@ namespace BirthdayBot.UserInterface } // Update the value - Instance.GuildCache[reqChannel.Guild.Id].UpdateAnnounceChannel(chId); + gconf.AnnounceChannelId = chId; + await gconf.UpdateAsync(); // Report the success await reqChannel.SendMessageAsync($":white_check_mark: The announcement channel is now set to <#{chId}>."); @@ -212,7 +213,7 @@ namespace BirthdayBot.UserInterface } // Moderator role set - private async Task ScmdModRole(string[] param, SocketTextChannel reqChannel) + private async Task ScmdModRole(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { if (param.Length != 2) { @@ -228,27 +229,26 @@ namespace BirthdayBot.UserInterface } else { - Instance.GuildCache[guild.Id].UpdateModeratorRole(role.Id); + gconf.ModeratorRole = role.Id; + await gconf.UpdateAsync(); await reqChannel.SendMessageAsync($":white_check_mark: The moderator role is now **{role.Name}**."); } } // Guild default time zone set/unset - private async Task ScmdZone(string[] param, SocketTextChannel reqChannel) + private async Task ScmdZone(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { - if (param.Length == 1) + if (param.Length == 1) // No extra parameter. Unset guild default time zone. { - // No extra parameter. Unset guild default time zone. - var gi = Instance.GuildCache[reqChannel.Guild.Id]; - // Extra detail: Show a unique message if there is no set zone. - if (!gi.AnnounceChannelId.HasValue) + if (!gconf.AnnounceChannelId.HasValue) { await reqChannel.SendMessageAsync(":x: A default zone is not set. Nothing to unset."); return; } - gi.UpdateTimeZone(null); + gconf.TimeZone = null; + await gconf.UpdateAsync(); await reqChannel.SendMessageAsync(":white_check_mark: The default time zone preference has been removed."); } else @@ -266,7 +266,8 @@ namespace BirthdayBot.UserInterface } // Update value - Instance.GuildCache[reqChannel.Guild.Id].UpdateTimeZone(zone); + gconf.TimeZone = zone; + await gconf.UpdateAsync(); // Report the success await reqChannel.SendMessageAsync($":white_check_mark: The server's time zone has been set to **{zone}**."); @@ -274,7 +275,7 @@ namespace BirthdayBot.UserInterface } // Block/unblock individual non-manager users from using commands. - private async Task ScmdBlock(string[] param, SocketTextChannel reqChannel) + private async Task ScmdBlock(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { if (param.Length != 2) { @@ -284,43 +285,41 @@ namespace BirthdayBot.UserInterface bool doBan = param[0].ToLower() == "block"; // true = block, false = unblock - ulong inputId; - if (!TryGetUserId(param[1], out inputId)) + if (!TryGetUserId(param[1], out ulong inputId)) { await reqChannel.SendMessageAsync(BadUserError); return; } - var gi = Instance.GuildCache[reqChannel.Guild.Id]; - var isBanned = await gi.IsUserBlockedAsync(inputId); + var isBanned = await gconf.IsUserBlockedAsync(inputId); if (doBan) { if (!isBanned) { - await gi.BlockUserAsync(inputId); + await gconf.BlockUserAsync(inputId); await reqChannel.SendMessageAsync(":white_check_mark: User has been blocked."); } else { + // TODO bug: this is incorrectly always displayed when in moderated mode await reqChannel.SendMessageAsync(":white_check_mark: User is already blocked."); } } else { - if (isBanned) + if (await gconf.UnblockUserAsync(inputId)) { - await gi.UnbanUserAsync(inputId); await reqChannel.SendMessageAsync(":white_check_mark: User is now unblocked."); } else { - await reqChannel.SendMessageAsync(":white_check_mark: The specified user has not been blocked."); + await reqChannel.SendMessageAsync(":white_check_mark: The specified user is not blocked."); } } } // "moderated on/off" - Sets/unsets moderated mode. - private async Task ScmdModerated(string[] param, SocketTextChannel reqChannel) + private async Task ScmdModerated(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { if (param.Length != 2) { @@ -334,26 +333,24 @@ namespace BirthdayBot.UserInterface else if (parameter == "off") modSet = false; else { - await reqChannel.SendMessageAsync(":x: Expected `on` or `off` as a parameter." + ConfErrorPostfix); + await reqChannel.SendMessageAsync(":x: Expecting `on` or `off` as a parameter." + ConfErrorPostfix); return; } - var gi = Instance.GuildCache[reqChannel.Guild.Id]; - var currentSet = gi.IsModerated; - gi.UpdateModeratedMode(modSet); - - if (currentSet == modSet) + if (gconf.IsModerated == modSet) { await reqChannel.SendMessageAsync($":white_check_mark: Moderated mode is already {parameter}."); } else { + gconf.IsModerated = modSet; + await gconf.UpdateAsync(); await reqChannel.SendMessageAsync($":white_check_mark: Moderated mode has been turned {parameter}."); } } // Sets/unsets custom announcement message. - private async Task ScmdAnnounceMsg(string[] param, SocketTextChannel reqChannel) + private async Task ScmdAnnounceMsg(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel) { var plural = param[0].ToLower().EndsWith("pl"); @@ -370,17 +367,21 @@ namespace BirthdayBot.UserInterface clear = true; } - Instance.GuildCache[reqChannel.Guild.Id].UpdateAnnounceMessage(newmsg, plural); - const string report = ":white_check_mark: The {0} birthday announcement message has been {1}."; - await reqChannel.SendMessageAsync(string.Format(report, plural ? "plural" : "singular", clear ? "reset" : "updated")); + (string, string) update; + if (!plural) update = (newmsg, gconf.AnnounceMessages.Item2); + else update = (gconf.AnnounceMessages.Item1, newmsg); + gconf.AnnounceMessages = update; + await gconf.UpdateAsync(); + await reqChannel.SendMessageAsync(string.Format(":white_check_mark: The {0} birthday announcement message has been {1}.", + plural ? "plural" : "singular", clear ? "reset" : "updated")); } #endregion // Execute command as another user - private async Task CmdOverride(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdOverride(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // Moderators only. As with config, silently drop if this check fails. - if (!Instance.GuildCache[reqUser.Guild.Id].IsUserModerator(reqUser)) return; + if (!gconf.IsBotModerator(reqUser)) return; if (param.Length != 3) { @@ -389,8 +390,7 @@ namespace BirthdayBot.UserInterface } // Second parameter: determine the user to act as - ulong user = 0; - if (!TryGetUserId(param[1], out user)) + if (!TryGetUserId(param[1], out ulong user)) { await reqChannel.SendMessageAsync(BadUserError, embed: DocOverride.UsageEmbed); return; @@ -416,8 +416,7 @@ namespace BirthdayBot.UserInterface // Add command prefix to input, just in case. overparam[0] = CommandPrefix + overparam[0].ToLower(); } - CommandHandler action = null; - if (!_usercommands.TryGetValue(cmdsearch, out action)) + if (!_usercommands.TryGetValue(cmdsearch, out CommandHandler action)) { await reqChannel.SendMessageAsync($":x: `{cmdsearch}` is not an overridable command.", embed: DocOverride.UsageEmbed); return; @@ -425,15 +424,14 @@ namespace BirthdayBot.UserInterface // Preparations complete. Run the command. await reqChannel.SendMessageAsync($"Executing `{cmdsearch.ToLower()}` on behalf of {overuser.Nickname ?? overuser.Username}:"); - await action.Invoke(overparam, reqChannel, overuser); + await action.Invoke(overparam, gconf, reqChannel, overuser); } // Publicly available command that immediately processes the current guild, - private async Task CmdTest(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdTest(string[] param, GuildConfiguration gconf, 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 + if (!gconf.IsBotModerator(reqUser)) return; if (param.Length != 1) { @@ -472,8 +470,7 @@ namespace BirthdayBot.UserInterface if (rmatch.Success) input = rmatch.Groups["snowflake"].Value; // Attempt to get role by ID, or null - ulong rid; - if (ulong.TryParse(input, out rid)) + if (ulong.TryParse(input, out ulong rid)) { return guild.GetRole(rid); } From 3d3db6d5e1e81a5d52ae04d95757dccf11597efb Mon Sep 17 00:00:00 2001 From: Noi Date: Wed, 22 Jul 2020 16:58:38 -0700 Subject: [PATCH 10/19] Update remaining handlers to not use guild cache --- UserInterface/HelpInfoCommands.cs | 13 +++++++------ UserInterface/UserCommands.cs | 25 ++++++++++++------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/UserInterface/HelpInfoCommands.cs b/UserInterface/HelpInfoCommands.cs index 72d655e..9499048 100644 --- a/UserInterface/HelpInfoCommands.cs +++ b/UserInterface/HelpInfoCommands.cs @@ -1,4 +1,5 @@ -using Discord; +using BirthdayBot.Data; +using Discord; using Discord.WebSocket; using System.Collections.Generic; using System.Text; @@ -89,13 +90,13 @@ namespace BirthdayBot.UserInterface return (helpRegular.Build(), helpConfig.Build()); } - private async Task CmdHelp(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdHelp(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) => await reqChannel.SendMessageAsync(embed: _helpEmbed); - private async Task CmdHelpConfig(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdHelpConfig(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) => await reqChannel.SendMessageAsync(embed: _helpConfigEmbed); - private async Task CmdHelpTzdata(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdHelpTzdata(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { const string tzhelp = "You may specify a time zone in order to have your birthday recognized with respect to your local time. " + "This bot only accepts zone names from the IANA Time Zone Database (a.k.a. Olson Database).\n\n" @@ -110,7 +111,7 @@ namespace BirthdayBot.UserInterface await reqChannel.SendMessageAsync(embed: embed.Build()); } - private async Task CmdHelpMessage(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdHelpMessage(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { const string msghelp = "The `message` and `messagepl` subcommands allow for editing the message sent into the announcement " + "channel (defined with `{0}config channel`). This feature is separated across two commands:\n" @@ -136,7 +137,7 @@ namespace BirthdayBot.UserInterface await reqChannel.SendMessageAsync(embed: embed.Build()); } - private async Task CmdInfo(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdInfo(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { var strStats = new StringBuilder(); var asmnm = System.Reflection.Assembly.GetExecutingAssembly().GetName(); diff --git a/UserInterface/UserCommands.cs b/UserInterface/UserCommands.cs index 0753774..3bc9885 100644 --- a/UserInterface/UserCommands.cs +++ b/UserInterface/UserCommands.cs @@ -1,7 +1,7 @@ -using Discord.WebSocket; +using BirthdayBot.Data; +using Discord.WebSocket; using System; using System.Collections.Generic; -using System.Linq; using System.Text.RegularExpressions; using System.Threading.Tasks; @@ -111,7 +111,7 @@ namespace BirthdayBot.UserInterface new CommandDocumentation(new string[] { "remove" }, "Removes your birthday information from this bot.", null); #endregion - private async Task CmdSet(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdSet(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // Requires one parameter. Optionally two. if (param.Length < 2 || param.Length > 3) @@ -140,9 +140,9 @@ namespace BirthdayBot.UserInterface bool known; // Extra detail: Bot's response changes if the user was previously unknown. try { - var user = Instance.GuildCache[reqChannel.Guild.Id].GetUser(reqUser.Id); + var user = await GuildUserConfiguration.LoadAsync(gconf.GuildId, reqUser.Id); known = user.IsKnown; - await user.UpdateAsync(bmonth, bday, btz, BotConfig.DatabaseSettings); + await user.UpdateAsync(bmonth, bday, btz); } catch (Exception ex) { @@ -161,7 +161,7 @@ namespace BirthdayBot.UserInterface } } - private async Task CmdZone(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdZone(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { if (param.Length != 2) { @@ -169,7 +169,7 @@ namespace BirthdayBot.UserInterface return; } - var user = Instance.GuildCache[reqChannel.Guild.Id].GetUser(reqUser.Id); + var user = await GuildUserConfiguration.LoadAsync(gconf.GuildId, reqUser.Id); if (!user.IsKnown) { await reqChannel.SendMessageAsync(":x: You may only update your time zone when you have a birthday registered." @@ -187,12 +187,12 @@ namespace BirthdayBot.UserInterface reqChannel.SendMessageAsync(ex.Message, embed: DocZone.UsageEmbed).Wait(); return; } - await user.UpdateAsync(user.BirthMonth, user.BirthDay, btz, BotConfig.DatabaseSettings); + await user.UpdateAsync(user.BirthMonth, user.BirthDay, btz); await reqChannel.SendMessageAsync($":white_check_mark: Your time zone has been updated to **{btz}**."); } - private async Task CmdRemove(string[] param, SocketTextChannel reqChannel, SocketGuildUser reqUser) + private async Task CmdRemove(string[] param, GuildConfiguration gconf, SocketTextChannel reqChannel, SocketGuildUser reqUser) { // Parameter count check if (param.Length != 1) @@ -203,10 +203,9 @@ namespace BirthdayBot.UserInterface // Extra detail: Send a notification if the user isn't actually known by the bot. bool known; - var g = Instance.GuildCache[reqChannel.Guild.Id]; - known = g.GetUser(reqUser.Id).IsKnown; - // Delete database and cache entry - await g.DeleteUserAsync(reqUser.Id); + var u = await GuildUserConfiguration.LoadAsync(gconf.GuildId, reqUser.Id); + known = u.IsKnown; + await u.DeleteAsync(); if (!known) { await reqChannel.SendMessageAsync(":white_check_mark: This bot already does not contain your information."); From ffbffdd9b4a4aad0e00835ba18b5dcf6bb2bdbdc Mon Sep 17 00:00:00 2001 From: Noi Date: Wed, 22 Jul 2020 17:06:35 -0700 Subject: [PATCH 11/19] Remove async multiple updates --- BackgroundServices/StaleDataCleaner.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/BackgroundServices/StaleDataCleaner.cs b/BackgroundServices/StaleDataCleaner.cs index 05d42e5..1d068d5 100644 --- a/BackgroundServices/StaleDataCleaner.cs +++ b/BackgroundServices/StaleDataCleaner.cs @@ -46,23 +46,21 @@ namespace BirthdayBot.BackgroundServices cUpdateGuildUser.Prepare(); // Do actual updates - var updates = new List(); foreach (var item in updateList) { var guild = item.Key; var userlist = item.Value; pUpdateG.Value = (long)guild; - updates.Add(cUpdateGuild.ExecuteNonQueryAsync()); + await cUpdateGuild.ExecuteNonQueryAsync(); pUpdateGU_g.Value = (long)guild; foreach (var userid in userlist) { pUpdateGU_u.Value = (long)userid; - updates.Add(cUpdateGuildUser.ExecuteNonQueryAsync()); + await cUpdateGuildUser.ExecuteNonQueryAsync(); } } - await Task.WhenAll(updates); // Delete all old values - expects referencing tables to have 'on delete cascade' using var t = db.BeginTransaction(); From e8ae5bb9dab571e24eef17cedc34baa64aa5c74d Mon Sep 17 00:00:00 2001 From: Noi Date: Wed, 22 Jul 2020 17:07:30 -0700 Subject: [PATCH 12/19] Increase initial processing delay --- BackgroundServiceRunner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BackgroundServiceRunner.cs b/BackgroundServiceRunner.cs index 841f30c..32d348c 100644 --- a/BackgroundServiceRunner.cs +++ b/BackgroundServiceRunner.cs @@ -16,7 +16,7 @@ namespace BirthdayBot const int Interval = 8 * 60; // Amount of time between start and first round of processing, in seconds. - const int StartDelay = 60; + const int StartDelay = 3 * 60; // 3 minutes #else const int Interval = 10; const int StartDelay = 15; From 937538c0e24bf11f9c5359aadfedfac44c36d4a7 Mon Sep 17 00:00:00 2001 From: Noi Date: Wed, 22 Jul 2020 19:41:36 -0700 Subject: [PATCH 13/19] Update package --- BirthdayBot.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BirthdayBot.csproj b/BirthdayBot.csproj index e55f7a7..810af3b 100644 --- a/BirthdayBot.csproj +++ b/BirthdayBot.csproj @@ -22,7 +22,7 @@ - + From 73fae41ac99e32c7de3c199e7ca7d8af3fb2ba91 Mon Sep 17 00:00:00 2001 From: Noi Date: Wed, 22 Jul 2020 19:42:11 -0700 Subject: [PATCH 14/19] Automatically add pool config to database --- Data/Database.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Data/Database.cs b/Data/Database.cs index 2581669..6f395cd 100644 --- a/Data/Database.cs +++ b/Data/Database.cs @@ -9,7 +9,12 @@ namespace BirthdayBot.Data /// internal static class Database { - public static string DBConnectionString { get; set; } + private static string _connString; + public static string DBConnectionString + { + get => _connString; + set => _connString = "Minimum Pool Size=5;Maximum Pool Size=50;Connection Idle Lifetime=30;" + value; + } public static async Task OpenConnectionAsync() { From 8f84b09b218dbf31c0eda9d4f6bbc4d675898e6c Mon Sep 17 00:00:00 2001 From: Noi Date: Wed, 22 Jul 2020 19:43:45 -0700 Subject: [PATCH 15/19] Reduce amount of automatic processing These changes make the bot skip over needlessly collecting information for guilds in which it is unnecessary, in most cases guilds that have invited the bot but have not yet invoked any bot command. --- BackgroundServices/BirthdayRoleUpdate.cs | 6 +++++- BirthdayBot.cs | 2 +- Data/GuildConfiguration.cs | 9 +++++++-- Program.cs | 3 +-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index 4393f3e..94a22ae 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -40,6 +40,7 @@ namespace BirthdayBot.BackgroundServices { Log($"{exs.InnerExceptions.Count} exception(s) during bulk processing!"); // TODO needs major improvements. output to file? + foreach (var iex in exs.InnerExceptions) Log(iex.Message); } else { @@ -66,7 +67,9 @@ namespace BirthdayBot.BackgroundServices { var diag = new PGDiagnostic(); - var gc = await GuildConfiguration.LoadAsync(guild.Id); + // Load guild information - stop if there is none (bot never previously used in guild) + var gc = await GuildConfiguration.LoadAsync(guild.Id, true); + if (gc == null) return diag; // Check if role settings are correct before continuing with further processing SocketRole role = null; @@ -75,6 +78,7 @@ namespace BirthdayBot.BackgroundServices if (diag.RoleCheck != null) return diag; // Determine who's currently having a birthday + await guild.DownloadUsersAsync(); var users = await GuildUserConfiguration.LoadAllAsync(guild.Id); var tz = gc.TimeZone; var birthdays = GetGuildCurrentBirthdays(users, tz); diff --git a/BirthdayBot.cs b/BirthdayBot.cs index 51e957a..23fd4de 100644 --- a/BirthdayBot.cs +++ b/BirthdayBot.cs @@ -117,7 +117,7 @@ namespace BirthdayBot if (!_dispatchCommands.TryGetValue(csplit[0].Substring(CommandPrefix.Length), out CommandHandler command)) return; // Load guild information here - var gconf = await GuildConfiguration.LoadAsync(channel.Guild.Id); + var gconf = await GuildConfiguration.LoadAsync(channel.Guild.Id, false); // Ban check if (!gconf.IsBotModerator(author)) // skip check if user is a moderator diff --git a/Data/GuildConfiguration.cs b/Data/GuildConfiguration.cs index b84487a..1c612e2 100644 --- a/Data/GuildConfiguration.cs +++ b/Data/GuildConfiguration.cs @@ -173,7 +173,11 @@ namespace BirthdayBot.Data /// /// Fetches guild settings from the database. If no corresponding entry exists, it will be created. /// - public static async Task LoadAsync(ulong guildId) + /// + /// If true, this method shall not create a new entry and will return null if the guild does + /// not exist in the database. + /// + public static async Task LoadAsync(ulong guildId, bool nullIfUnknown) { using (var db = await Database.OpenConnectionAsync()) { @@ -188,6 +192,7 @@ namespace BirthdayBot.Data using var r = await c.ExecuteReaderAsync(); if (await r.ReadAsync()) return new GuildConfiguration(r); } + if (nullIfUnknown) return null; // If we got here, no row exists. Create it with default values. using (var c = db.CreateCommand()) @@ -199,7 +204,7 @@ namespace BirthdayBot.Data } } // With a new row created, try this again - return await LoadAsync(guildId); + return await LoadAsync(guildId, nullIfUnknown); } /// diff --git a/Program.cs b/Program.cs index eddf738..0281b71 100644 --- a/Program.cs +++ b/Program.cs @@ -21,8 +21,7 @@ namespace BirthdayBot var dc = new DiscordSocketConfig() { - AlwaysDownloadUsers = true, - DefaultRetryMode = Discord.RetryMode.RetryRatelimit, + DefaultRetryMode = RetryMode.RetryRatelimit, MessageCacheSize = 0, TotalShards = cfg.ShardCount, ExclusiveBulkDelete = true From a87426a5f6462464bb95f24a6826387290163c34 Mon Sep 17 00:00:00 2001 From: Noi Date: Mon, 27 Jul 2020 22:29:35 -0700 Subject: [PATCH 16/19] Remove unnecessary lines --- BirthdayBot.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/BirthdayBot.cs b/BirthdayBot.cs index 23fd4de..13c4b98 100644 --- a/BirthdayBot.cs +++ b/BirthdayBot.cs @@ -6,7 +6,6 @@ using Discord.Webhook; using Discord.WebSocket; using System; using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using static BirthdayBot.UserInterface.CommandsCommon; @@ -113,7 +112,6 @@ namespace BirthdayBot if (csplit.Length > 0 && csplit[0].StartsWith(CommandPrefix, StringComparison.OrdinalIgnoreCase)) { // Determine if it's something we're listening for. - // Doing this first before the block check because a block check triggers a database query. if (!_dispatchCommands.TryGetValue(csplit[0].Substring(CommandPrefix.Length), out CommandHandler command)) return; // Load guild information here From 657181ec3ed40bce93ea44ad5e3e73be11115500 Mon Sep 17 00:00:00 2001 From: Noi Date: Mon, 27 Jul 2020 22:30:15 -0700 Subject: [PATCH 17/19] Less async birthday updates; download users on connect From testing, it appears DownloadUsersAsync hangs forever and has other consequences if its corresponding server is not fully connected, or is struggling to connect. This is despite all attempts to mitigate it and detect any disconnections shortly before attempting it. Additionally, birthday updates are now run asynchronously per shard, but synchronously per guild in an attempt to keep rare instances of rate limiting under control. --- BackgroundServices/BirthdayRoleUpdate.cs | 41 ++++++++++++++++++++++-- Program.cs | 1 + 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index 94a22ae..0675cd2 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -23,9 +23,11 @@ namespace BirthdayBot.BackgroundServices public override async Task OnTick() { var tasks = new List(); - foreach (var guild in BotInstance.DiscordClient.Guilds) + + // Work on each shard concurrently; guilds within each shard synchronously + foreach (var shard in BotInstance.DiscordClient.Shards) { - tasks.Add(ProcessGuildAsync(guild)); + tasks.Add(ProcessShardAsync(shard)); } var alltasks = Task.WhenAll(tasks); @@ -60,6 +62,39 @@ namespace BirthdayBot.BackgroundServices /// Diagnostic data in string form. public async Task SingleProcessGuildAsync(SocketGuild guild) => (await ProcessGuildAsync(guild)).Export(); + /// + /// Called by , processes all guilds within a shard synchronously. + /// + private async Task ProcessShardAsync(DiscordSocketClient shard) + { + if (shard.ConnectionState != Discord.ConnectionState.Connected) + { + Log($"Shard {shard.ShardId} (with {shard.Guilds.Count} guilds) processing stopped - shard disconnected."); + return; + } + var exs = new List(); + foreach (var guild in shard.Guilds) + { + try + { + // Check if shard remains available + if (shard.ConnectionState != Discord.ConnectionState.Connected) + { + Log($"Shard {shard.ShardId} (with {shard.Guilds.Count} guilds) processing interrupted."); + return; + } + await ProcessGuildAsync(guild); + } + catch (Exception ex) + { + // Catch all exceptions per-guild but continue processing, throw at end + exs.Add(ex); + } + } + Log($"Shard {shard.ShardId} (with {shard.Guilds.Count} guilds) processing completed."); + if (exs.Count != 0) throw new AggregateException(exs); + } + /// /// Main method where actual guild processing occurs. /// @@ -78,7 +113,7 @@ namespace BirthdayBot.BackgroundServices if (diag.RoleCheck != null) return diag; // Determine who's currently having a birthday - await guild.DownloadUsersAsync(); + //await guild.DownloadUsersAsync(); var users = await GuildUserConfiguration.LoadAllAsync(guild.Id); var tz = gc.TimeZone; var birthdays = GetGuildCurrentBirthdays(users, tz); diff --git a/Program.cs b/Program.cs index 0281b71..085c49c 100644 --- a/Program.cs +++ b/Program.cs @@ -21,6 +21,7 @@ namespace BirthdayBot var dc = new DiscordSocketConfig() { + AlwaysDownloadUsers = true, DefaultRetryMode = RetryMode.RetryRatelimit, MessageCacheSize = 0, TotalShards = cfg.ShardCount, From a9e87d8021db5a18dfd526848f5db7ba06107ee1 Mon Sep 17 00:00:00 2001 From: Noi Date: Mon, 27 Jul 2020 22:39:01 -0700 Subject: [PATCH 18/19] Update intervals, increase version number --- BackgroundServiceRunner.cs | 13 +++++++------ BirthdayBot.csproj | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/BackgroundServiceRunner.cs b/BackgroundServiceRunner.cs index 32d348c..3a760a0 100644 --- a/BackgroundServiceRunner.cs +++ b/BackgroundServiceRunner.cs @@ -12,14 +12,15 @@ namespace BirthdayBot class BackgroundServiceRunner { #if !DEBUG - // Amount of idle time between each round of task execution, in seconds. - const int Interval = 8 * 60; - // Amount of time between start and first round of processing, in seconds. - const int StartDelay = 3 * 60; // 3 minutes + const int StartDelay = 3 * 60; // 3 minutes + + // Amount of idle time between each round of task execution, in seconds. + const int Interval = 5 * 60; // 5 minutes #else - const int Interval = 10; - const int StartDelay = 15; + // Short intervals for testing + const int StartDelay = 20; + const int Interval = 20; #endif const string LogName = nameof(BackgroundServiceRunner); diff --git a/BirthdayBot.csproj b/BirthdayBot.csproj index 810af3b..58cf958 100644 --- a/BirthdayBot.csproj +++ b/BirthdayBot.csproj @@ -3,7 +3,7 @@ Exe netcoreapp3.1 - 2.2.0 + 2.3.0 BirthdayBot NoiTheCat BirthdayBot From f1e7bd8c6738d315afb1c1e34bd450e69cfc54ab Mon Sep 17 00:00:00 2001 From: Noi Date: Mon, 27 Jul 2020 22:40:09 -0700 Subject: [PATCH 19/19] Removed disconnected server count Made redundant by per-shard status on every birthday update --- BackgroundServices/Heartbeat.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/BackgroundServices/Heartbeat.cs b/BackgroundServices/Heartbeat.cs index 1851d55..6f10a92 100644 --- a/BackgroundServices/Heartbeat.cs +++ b/BackgroundServices/Heartbeat.cs @@ -14,18 +14,6 @@ namespace BirthdayBot.BackgroundServices { var uptime = DateTimeOffset.UtcNow - Program.BotStartTime; Log($"Bot uptime: {Common.BotUptime}"); - - // Disconnection warn - foreach (var shard in BotInstance.DiscordClient.Shards) - { - if (shard.ConnectionState == Discord.ConnectionState.Disconnected) - { - Log($"Shard {shard.ShardId} is disconnected! Restart the app if this persists."); - // The library alone cannot be restarted as it is in an unknown state. It was not designed to be restarted. - // TODO This is the part where we'd signal something to restart us if we were fancy. - } - } - return Task.CompletedTask; } }