From c280904cb8272e68b9e35e4443528cf85463449f Mon Sep 17 00:00:00 2001 From: Noi Date: Mon, 21 Mar 2022 12:11:30 -0700 Subject: [PATCH] Update services to use EF queries --- BackgroundServices/AutoUserDownload.cs | 34 ++--- BackgroundServices/BackgroundService.cs | 5 +- BackgroundServices/BirthdayRoleUpdate.cs | 101 ++++++-------- BackgroundServices/DataRetention.cs | 128 ++++++------------ .../ExternalStatisticsReporting.cs | 8 +- Data/BotDatabaseContext.cs | 6 +- 6 files changed, 104 insertions(+), 178 deletions(-) diff --git a/BackgroundServices/AutoUserDownload.cs b/BackgroundServices/AutoUserDownload.cs index ecb90c3..3472020 100644 --- a/BackgroundServices/AutoUserDownload.cs +++ b/BackgroundServices/AutoUserDownload.cs @@ -9,29 +9,21 @@ class AutoUserDownload : BackgroundService { public AutoUserDownload(ShardInstance instance) : base(instance) { } public override async Task OnTick(int tickCount, CancellationToken token) { - foreach (var guild in ShardInstance.DiscordClient.Guilds) { - // Has the potential to disconnect while in the middle of processing. + using var db = new BotDatabaseContext(); + + // Take action if a guild's cache is incomplete... + var incompleteCaches = ShardInstance.DiscordClient.Guilds.Where(g => !g.HasAllMembers).Select(g => (long)g.Id).ToHashSet(); + // ...and if the guild contains any user data + var mustFetch = db.UserEntries.Where(e => incompleteCaches.Contains(e.GuildId)).Select(e => e.GuildId).Distinct(); + + foreach (var item in mustFetch) { + // May cause a disconnect in certain situations. Cancel all further attempts until the next pass if it happens. if (ShardInstance.DiscordClient.ConnectionState != ConnectionState.Connected) return; - // Determine if there is action to be taken... - if (!guild.HasAllMembers && await GuildUserAnyAsync(guild.Id).ConfigureAwait(false)) { - await guild.DownloadUsersAsync().ConfigureAwait(false); // This is already on a separate thread; no need to Task.Run - await Task.Delay(200, CancellationToken.None).ConfigureAwait(false); // Must delay, or else it seems to hang... - } + var guild = ShardInstance.DiscordClient.GetGuild((ulong)item); + if (guild == null) continue; // A guild disappeared...? + await guild.DownloadUsersAsync().ConfigureAwait(false); // We're already on a seperate thread, no need to use Task.Run + await Task.Delay(200, CancellationToken.None).ConfigureAwait(false); // Must delay, or else it seems to hang... } } - - /// - /// Determines if the user database contains any entries corresponding to this guild. - /// - /// True if any entries exist. - private static async Task GuildUserAnyAsync(ulong guildId) { - using var db = await Database.OpenConnectionAsync().ConfigureAwait(false); - using var c = db.CreateCommand(); - c.CommandText = $"select true from {GuildUserConfiguration.BackingTable} where guild_id = @Gid limit 1"; - c.Parameters.Add("@Gid", NpgsqlTypes.NpgsqlDbType.Bigint).Value = (long)guildId; - await c.PrepareAsync(CancellationToken.None).ConfigureAwait(false); - using var r = await c.ExecuteReaderAsync(CancellationToken.None).ConfigureAwait(false); - return r.Read(); - } } diff --git a/BackgroundServices/BackgroundService.cs b/BackgroundServices/BackgroundService.cs index 06c1c27..59ab1ce 100644 --- a/BackgroundServices/BackgroundService.cs +++ b/BackgroundServices/BackgroundService.cs @@ -1,7 +1,4 @@ -using System.Threading; -using System.Threading.Tasks; - -namespace BirthdayBot.BackgroundServices; +namespace BirthdayBot.BackgroundServices; abstract class BackgroundService { protected ShardInstance ShardInstance { get; } diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index 649f094..031e7d3 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -15,62 +15,51 @@ class BirthdayRoleUpdate : BackgroundService { /// Processes birthday updates for all available guilds synchronously. /// public override async Task OnTick(int tickCount, CancellationToken token) { - var exs = new List(); - foreach (var guild in ShardInstance.DiscordClient.Guilds) { + // For database efficiency, fetch all database information at once before proceeding + // and combine it into the guild IDs that will be processed + using var db = new BotDatabaseContext(); + var shardGuilds = ShardInstance.DiscordClient.Guilds.Select(g => (long)g.Id).ToHashSet(); + var settings = db.GuildConfigurations.Where(s => shardGuilds.Contains(s.GuildId)); + var guildChecks = shardGuilds.Join(settings, o => o, i => i.GuildId, (id, conf) => new { Key = (ulong)id, Value = conf }); + + var exceptions = new List(); + foreach (var pair in guildChecks) { + var guild = ShardInstance.DiscordClient.GetGuild(pair.Key); + if (guild == null) continue; // A guild disappeared...? + var guildConf = pair.Value; + + // Check task cancellation here. Processing during a single guild is never interrupted. + if (token.IsCancellationRequested) throw new TaskCanceledException(); + if (ShardInstance.DiscordClient.ConnectionState != Discord.ConnectionState.Connected) { Log("Client is not connected. Stopping early."); return; } - // Check task cancellation here. Processing during a single guild is never interrupted. - if (token.IsCancellationRequested) throw new TaskCanceledException(); - try { - await ProcessGuildAsync(guild).ConfigureAwait(false); + // Verify that role settings and permissions are usable + SocketRole? role = guild.GetRole((ulong)(guildConf.RoleId ?? 0)); + if (role == null || !guild.CurrentUser.GuildPermissions.ManageRoles || role.Position >= guild.CurrentUser.Hierarchy) return; + + // Load up user configs and begin processing birthdays + await db.Entry(guildConf).Collection(t => t.UserEntries).LoadAsync(CancellationToken.None); + var birthdays = GetGuildCurrentBirthdays(guildConf.UserEntries, guildConf.TimeZone); + // Note: Don't quit here if zero people are having birthdays. Roles may still need to be removed by BirthdayApply. + + // Update roles as appropriate + var announcementList = await UpdateGuildBirthdayRoles(guild, role, birthdays); + + // Birthday announcement + var channel = guild.GetTextChannel((ulong)(guildConf.ChannelAnnounceId ?? 0)); + if (announcementList.Any()) { + await AnnounceBirthdaysAsync(guildConf, channel, announcementList); + } } catch (Exception ex) { // Catch all exceptions per-guild but continue processing, throw at end. - exs.Add(ex); + exceptions.Add(ex); } } - if (exs.Count != 0) throw new AggregateException(exs); - } - - /// - /// Main method where actual guild processing occurs. - /// - private static async Task ProcessGuildAsync(SocketGuild guild) { - // Load guild information - stop if local cache is unavailable. - if (!Common.HasMostMembersDownloaded(guild)) return; - var gc = await GuildConfiguration.LoadAsync(guild.Id, true).ConfigureAwait(false); - if (gc == null) return; - - // Check if role settings are correct before continuing with further processing - SocketRole? role = guild.GetRole(gc.RoleId ?? 0); - if (role == null || !guild.CurrentUser.GuildPermissions.ManageRoles || role.Position >= guild.CurrentUser.Hierarchy) return; - - // Determine who's currently having a birthday - var users = await GuildUserConfiguration.LoadAllAsync(guild.Id).ConfigureAwait(false); - 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. - - IEnumerable announcementList; - // Update roles as appropriate - try { - var updateResult = await UpdateGuildBirthdayRoles(guild, role, birthdays).ConfigureAwait(false); - announcementList = updateResult.Item1; - } catch (Discord.Net.HttpException) { - return; - } - - // Birthday announcement - var announce = gc.AnnounceMessages; - var announceping = gc.AnnouncePing; - SocketTextChannel? channel = null; - if (gc.AnnounceChannelId.HasValue) channel = guild.GetTextChannel(gc.AnnounceChannelId.Value); - if (announcementList.Any()) { - await AnnounceBirthdaysAsync(announce, announceping, channel, announcementList).ConfigureAwait(false); - } + if (exceptions.Count != 0) throw new AggregateException(exceptions); } /// @@ -82,7 +71,8 @@ class BirthdayRoleUpdate : BackgroundService { #pragma warning restore 618 public static HashSet GetGuildCurrentBirthdays(IEnumerable guildUsers, string? defaultTzStr) { var tzdb = DateTimeZoneProviders.Tzdb; - DateTimeZone defaultTz = (defaultTzStr != null ? DateTimeZoneProviders.Tzdb.GetZoneOrNull(defaultTzStr) : null) ?? tzdb.GetZoneOrNull("UTC")!; + DateTimeZone defaultTz = (defaultTzStr != null ? DateTimeZoneProviders.Tzdb.GetZoneOrNull(defaultTzStr) : null) + ?? tzdb.GetZoneOrNull("UTC")!; var birthdayUsers = new HashSet(); foreach (var item in guildUsers) { @@ -138,11 +128,9 @@ class BirthdayRoleUpdate : BackgroundService { /// Sets the birthday role to all applicable users. Unsets it from all others who may have it. /// /// - /// First item: List of users who had the birthday role applied, used to announce. - /// Second item: Counts of users who have had roles added/removed, used for operation reporting. + /// List of users who had the birthday role applied, used to announce. /// - private static async Task<(IEnumerable, (int, int))> UpdateGuildBirthdayRoles( - SocketGuild g, SocketRole r, HashSet names) { + private static async Task> UpdateGuildBirthdayRoles(SocketGuild g, SocketRole r, HashSet names) { // Check members currently with the role. Figure out which users to remove it from. var roleRemoves = new List(); var roleKeeps = new HashSet(); @@ -165,7 +153,7 @@ class BirthdayRoleUpdate : BackgroundService { newBirthdays.Add(member); } - return (newBirthdays, (newBirthdays.Count, roleRemoves.Count)); + return newBirthdays; } public const string DefaultAnnounce = "Please wish a happy birthday to %n!"; @@ -174,21 +162,20 @@ class BirthdayRoleUpdate : BackgroundService { /// /// Attempts to send an announcement message. /// - private static async Task AnnounceBirthdaysAsync( - (string?, string?) announce, bool announcePing, SocketTextChannel? c, IEnumerable names) { + private static async Task AnnounceBirthdaysAsync(GuildConfig settings, SocketTextChannel? c, IEnumerable names) { if (c == null) return; if (!c.Guild.CurrentUser.GetPermissions(c).SendMessages) return; string announceMsg; - if (names.Count() == 1) announceMsg = announce.Item1 ?? announce.Item2 ?? DefaultAnnounce; - else announceMsg = announce.Item2 ?? announce.Item1 ?? DefaultAnnouncePl; + if (names.Count() == 1) announceMsg = settings.AnnounceMessage ?? settings.AnnounceMessagePl ?? DefaultAnnounce; + else announceMsg = settings.AnnounceMessagePl ?? settings.AnnounceMessage ?? DefaultAnnouncePl; announceMsg = announceMsg.TrimEnd(); if (!announceMsg.Contains("%n")) announceMsg += " %n"; // Build sorted name list var namestrings = new List(); foreach (var item in names) - namestrings.Add(Common.FormatName(item, announcePing)); + namestrings.Add(Common.FormatName(item, settings.AnnouncePing)); namestrings.Sort(StringComparer.OrdinalIgnoreCase); var namedisplay = new StringBuilder(); diff --git a/BackgroundServices/DataRetention.cs b/BackgroundServices/DataRetention.cs index a24f12d..6bdfe18 100644 --- a/BackgroundServices/DataRetention.cs +++ b/BackgroundServices/DataRetention.cs @@ -1,5 +1,4 @@ using BirthdayBot.Data; -using NpgsqlTypes; using System.Text; namespace BirthdayBot.BackgroundServices; @@ -20,96 +19,49 @@ class DataRetention : BackgroundService { // On each tick, run only a set group of guilds, each group still processed every ProcessInterval ticks. if ((tickCount + ShardInstance.ShardId) % ProcessInterval != 0) return; - try { - // A semaphore is used to restrict this work being done concurrently on other shards - // to avoid putting pressure on the SQL connection pool. Clearing old database information - // ultimately is a low priority among other tasks. - await _updateLock.WaitAsync(token).ConfigureAwait(false); - } catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException) { - // Caller does not expect the exception that SemaphoreSlim throws... - throw new TaskCanceledException(); + using var db = new BotDatabaseContext(); + var now = DateTimeOffset.UtcNow; + int updatedGuilds = 0, updatedUsers = 0; + + foreach (var guild in ShardInstance.DiscordClient.Guilds) { + // Update guild, fetch users from database + var dbGuild = db.GuildConfigurations.Where(s => s.GuildId == (long)guild.Id).FirstOrDefault(); + if (dbGuild == null) continue; + dbGuild.LastSeen = now; + updatedGuilds++; + + // Update users + var localIds = guild.Users.Select(u => (long)u.Id); + var dbSavedIds = db.UserEntries.Where(e => e.GuildId == (long)guild.Id).Select(e => e.UserId); + var usersToUpdate = localIds.Intersect(dbSavedIds).ToHashSet(); + foreach (var user in db.UserEntries.Where(e => e.GuildId == (long)guild.Id && usersToUpdate.Contains(e.UserId))) { + user.LastSeen = now; + updatedUsers++; + } } - try { - // Build a list of all values across all guilds to update - var updateList = new Dictionary>(); - foreach (var g in ShardInstance.DiscordClient.Guilds) { - // Get list of IDs for all users who exist in the database and currently exist in the guild - var userList = GuildUserConfiguration.LoadAllAsync(g.Id); - var guildUserIds = from gu in g.Users select gu.Id; - var savedUserIds = from cu in await userList.ConfigureAwait(false) select cu.UserId; - var existingCachedIds = savedUserIds.Intersect(guildUserIds); - updateList[g.Id] = existingCachedIds.ToList(); + + // And let go of old data + var staleGuilds = db.GuildConfigurations.Where(s => now - TimeSpan.FromDays(StaleGuildThreshold) > s.LastSeen); + var staleUsers = db.UserEntries.Where(e => now - TimeSpan.FromDays(StaleUserThreashold) > e.LastSeen); + int staleGuildCount = staleGuilds.Count(), staleUserCount = staleUsers.Count(); + db.GuildConfigurations.RemoveRange(staleGuilds); + db.UserEntries.RemoveRange(staleUsers); + + await db.SaveChangesAsync(CancellationToken.None); + + var resultText = new StringBuilder(); + resultText.Append($"Updated {updatedGuilds} guilds, {updatedUsers} users."); + if (staleGuildCount != 0 || staleUserCount != 0) { + resultText.Append(" Discarded "); + if (staleGuildCount != 0) { + resultText.Append($"{staleGuildCount} guilds"); + if (staleUserCount != 0) resultText.Append(", "); } - - using var db = await Database.OpenConnectionAsync().ConfigureAwait(false); - - // 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 - int updatedGuilds = 0; - int updatedUsers = 0; - using (var tUpdate = db.BeginTransaction()) { - foreach (var item in updateList) { - var guild = item.Key; - var userlist = item.Value; - - pUpdateG.Value = (long)guild; - updatedGuilds += await cUpdateGuild.ExecuteNonQueryAsync(CancellationToken.None).ConfigureAwait(false); - - pUpdateGU_g.Value = (long)guild; - foreach (var userid in userlist) { - pUpdateGU_u.Value = (long)userid; - updatedUsers += await cUpdateGuildUser.ExecuteNonQueryAsync(CancellationToken.None).ConfigureAwait(false); - } - } - await tUpdate.CommitAsync(CancellationToken.None).ConfigureAwait(false); + if (staleUserCount != 0) { + resultText.Append($"{staleUserCount} users"); } - var resultText = new StringBuilder(); - resultText.Append($"Updated {updatedGuilds} guilds, {updatedUsers} users."); - - // Deletes both guild and user data if it hasn't been seen for over the threshold defined at the top of this file - // Expects referencing tables to have 'on delete cascade' - int staleGuilds, staleUsers; - using (var tRemove = db.BeginTransaction()) { - using (var c = db.CreateCommand()) { - c.CommandText = $"delete from {GuildConfiguration.BackingTable}" + - $" where (now() - interval '{StaleGuildThreshold} days') > last_seen"; - staleGuilds = await c.ExecuteNonQueryAsync(CancellationToken.None).ConfigureAwait(false); - } - using (var c = db.CreateCommand()) { - c.CommandText = $"delete from {GuildUserConfiguration.BackingTable}" + - $" where (now() - interval '{StaleUserThreashold} days') > last_seen"; - staleUsers = await c.ExecuteNonQueryAsync(CancellationToken.None).ConfigureAwait(false); - } - await tRemove.CommitAsync(CancellationToken.None).ConfigureAwait(false); - } - if (staleGuilds != 0 || staleUsers != 0) { - resultText.Append(" Discarded "); - if (staleGuilds != 0) { - resultText.Append($"{staleGuilds} guilds"); - if (staleUsers != 0) resultText.Append(", "); - } - if (staleUsers != 0) { - resultText.Append($"{staleUsers} users"); - } - resultText.Append('.'); - } - Log(resultText.ToString()); - } finally { - _updateLock.Release(); + resultText.Append('.'); } + Log(resultText.ToString()); } } diff --git a/BackgroundServices/ExternalStatisticsReporting.cs b/BackgroundServices/ExternalStatisticsReporting.cs index 63b46a0..fdcd165 100644 --- a/BackgroundServices/ExternalStatisticsReporting.cs +++ b/BackgroundServices/ExternalStatisticsReporting.cs @@ -1,8 +1,4 @@ -using System; -using System.Net.Http; -using System.Text; -using System.Threading; -using System.Threading.Tasks; +using System.Text; namespace BirthdayBot.BackgroundServices; @@ -11,7 +7,7 @@ namespace BirthdayBot.BackgroundServices; /// class ExternalStatisticsReporting : BackgroundService { const int ProcessInterval = 1200 / ShardBackgroundWorker.Interval; // Process every ~20 minutes - const int ProcessOffset = 300 / ShardBackgroundWorker.Interval; // Begin processing 5 minutes after shard start + const int ProcessOffset = 300 / ShardBackgroundWorker.Interval; // Begin processing ~5 minutes after shard start private static readonly HttpClient _httpClient = new(); diff --git a/Data/BotDatabaseContext.cs b/Data/BotDatabaseContext.cs index 9b3b6b9..cc88741 100644 --- a/Data/BotDatabaseContext.cs +++ b/Data/BotDatabaseContext.cs @@ -37,7 +37,8 @@ public class BotDatabaseContext : DbContext { entity.HasOne(d => d.Guild) .WithMany(p => p.BlockedUsers) .HasForeignKey(d => d.GuildId) - .HasConstraintName("banned_users_guild_id_fkey"); + .HasConstraintName("banned_users_guild_id_fkey") + .OnDelete(DeleteBehavior.Cascade); }); modelBuilder.Entity(entity => { @@ -58,7 +59,8 @@ public class BotDatabaseContext : DbContext { entity.HasOne(d => d.Guild) .WithMany(p => p.UserEntries) .HasForeignKey(d => d.GuildId) - .HasConstraintName("user_birthdays_guild_id_fkey"); + .HasConstraintName("user_birthdays_guild_id_fkey") + .OnDelete(DeleteBehavior.Cascade); }); } }