From 9303dbb91ef6101c26497e3636db15ad278074e0 Mon Sep 17 00:00:00 2001 From: spiral Date: Tue, 6 Sep 2022 09:52:37 +0000 Subject: [PATCH] refactor(bot): remove saving own user ID from ready event, rely on ID in config --- Myriad/Cache/DiscordCacheExtensions.cs | 4 +--- Myriad/Cache/IDiscordCache.cs | 3 +-- Myriad/Cache/MemoryDiscordCache.cs | 18 +++++++----------- Myriad/Cache/RedisDiscordCache.cs | 17 +++++------------ PluralKit.Bot/Bot.cs | 9 +++------ PluralKit.Bot/BotConfig.cs | 2 +- PluralKit.Bot/Commands/Checks.cs | 13 +++++-------- PluralKit.Bot/Commands/Misc.cs | 8 ++------ PluralKit.Bot/Handlers/MessageCreated.cs | 6 ++---- PluralKit.Bot/Handlers/MessageEdited.cs | 6 ++++-- PluralKit.Bot/Handlers/ReactionAdded.cs | 6 ++++-- PluralKit.Bot/Init.cs | 2 +- PluralKit.Bot/Modules.cs | 4 ++-- PluralKit.Bot/Services/LogChannelService.cs | 8 +++++--- PluralKit.Bot/Services/WebhookCacheService.cs | 9 +++++---- README.md | 2 +- pluralkit.conf.example | 3 ++- 17 files changed, 51 insertions(+), 69 deletions(-) diff --git a/Myriad/Cache/DiscordCacheExtensions.cs b/Myriad/Cache/DiscordCacheExtensions.cs index 7a47f9f0..eb1ff0c2 100644 --- a/Myriad/Cache/DiscordCacheExtensions.cs +++ b/Myriad/Cache/DiscordCacheExtensions.cs @@ -10,8 +10,6 @@ public static class DiscordCacheExtensions { switch (evt) { - case ReadyEvent ready: - return cache.SaveOwnUser(ready.User.Id); case GuildCreateEvent gc: return cache.SaveGuildCreate(gc); case GuildUpdateEvent gu: @@ -108,7 +106,7 @@ public static class DiscordCacheExtensions if (channel.GuildId != null) { - var userId = await cache.GetOwnUser(); + var userId = cache.GetOwnUser(); var member = await cache.TryGetSelfMember(channel.GuildId.Value); return await cache.PermissionsFor(channelId, userId, member); } diff --git a/Myriad/Cache/IDiscordCache.cs b/Myriad/Cache/IDiscordCache.cs index a9ecf4de..e1b4fad4 100644 --- a/Myriad/Cache/IDiscordCache.cs +++ b/Myriad/Cache/IDiscordCache.cs @@ -4,7 +4,6 @@ namespace Myriad.Cache; public interface IDiscordCache { - public ValueTask SaveOwnUser(ulong userId); public ValueTask SaveGuild(Guild guild); public ValueTask SaveChannel(Channel channel); public ValueTask SaveUser(User user); @@ -17,7 +16,7 @@ public interface IDiscordCache public ValueTask RemoveUser(ulong userId); public ValueTask RemoveRole(ulong guildId, ulong roleId); - public Task GetOwnUser(); + internal ulong GetOwnUser(); public Task TryGetGuild(ulong guildId); public Task TryGetChannel(ulong channelId); public Task TryGetUser(ulong userId); diff --git a/Myriad/Cache/MemoryDiscordCache.cs b/Myriad/Cache/MemoryDiscordCache.cs index 6a6d704b..193d4606 100644 --- a/Myriad/Cache/MemoryDiscordCache.cs +++ b/Myriad/Cache/MemoryDiscordCache.cs @@ -11,7 +11,12 @@ public class MemoryDiscordCache: IDiscordCache private readonly ConcurrentDictionary _guilds = new(); private readonly ConcurrentDictionary _roles = new(); private readonly ConcurrentDictionary _users = new(); - private ulong? _ownUserId { get; set; } + private readonly ulong _ownUserId; + + public MemoryDiscordCache(ulong ownUserId) + { + _ownUserId = ownUserId; + } public ValueTask SaveGuild(Guild guild) { @@ -48,15 +53,6 @@ public class MemoryDiscordCache: IDiscordCache await SaveUser(recipient); } - public ValueTask SaveOwnUser(ulong userId) - { - // this (hopefully) never changes at runtime, so we skip out on re-assigning it - if (_ownUserId == null) - _ownUserId = userId; - - return default; - } - public ValueTask SaveUser(User user) { _users[user.Id] = user; @@ -127,7 +123,7 @@ public class MemoryDiscordCache: IDiscordCache return default; } - public Task GetOwnUser() => Task.FromResult(_ownUserId!.Value); + public ulong GetOwnUser() => _ownUserId; public ValueTask RemoveRole(ulong guildId, ulong roleId) { diff --git a/Myriad/Cache/RedisDiscordCache.cs b/Myriad/Cache/RedisDiscordCache.cs index ffbcebdb..fff7920e 100644 --- a/Myriad/Cache/RedisDiscordCache.cs +++ b/Myriad/Cache/RedisDiscordCache.cs @@ -13,18 +13,18 @@ namespace Myriad.Cache; public class RedisDiscordCache: IDiscordCache { private readonly ILogger _logger; - public RedisDiscordCache(ILogger logger) + private readonly ulong _ownUserId; + public RedisDiscordCache(ILogger logger, ulong ownUserId) { _logger = logger; + _ownUserId = ownUserId; } private ConnectionMultiplexer _redis { get; set; } - private ulong _ownUserId { get; set; } - public async Task InitAsync(string addr, ulong ownUserId) + public async Task InitAsync(string addr) { _redis = await ConnectionMultiplexer.ConnectAsync(addr); - _ownUserId = ownUserId; } private IDatabase db => _redis.GetDatabase().WithKeyPrefix("discord:"); @@ -78,12 +78,6 @@ public class RedisDiscordCache: IDiscordCache await SaveUser(recipient); } - public ValueTask SaveOwnUser(ulong userId) - { - // we get the own user ID in InitAsync, so no need to save it here - return default; - } - public async ValueTask SaveUser(User user) { _logger.Verbose("Saving user {UserId} to redis", user.Id); @@ -161,8 +155,7 @@ public class RedisDiscordCache: IDiscordCache public async ValueTask RemoveUser(ulong userId) => await db.HashDeleteAsync("users", userId); - // todo: try getting this from redis if we don't have it yet - public Task GetOwnUser() => Task.FromResult(_ownUserId); + public ulong GetOwnUser() => _ownUserId; public async ValueTask RemoveRole(ulong guildId, ulong roleId) { diff --git a/PluralKit.Bot/Bot.cs b/PluralKit.Bot/Bot.cs index f8d21342..61769203 100644 --- a/PluralKit.Bot/Bot.cs +++ b/PluralKit.Bot/Bot.cs @@ -94,8 +94,7 @@ public class Bot // we HandleGatewayEvent **before** getting the own user, because the own user is set in HandleGatewayEvent for ReadyEvent await _cache.HandleGatewayEvent(evt); - var userId = await _cache.GetOwnUser(); - await _cache.TryUpdateSelfMember(userId, evt); + await _cache.TryUpdateSelfMember(_config.ClientId, evt); await OnEventReceivedInner(shardId, evt); } @@ -200,13 +199,11 @@ public class Bot { _metrics.Measure.Meter.Mark(BotMetrics.BotErrors, exc.GetType().FullName); - var ourUserId = await _cache.GetOwnUser(); - // Make this beforehand so we can access the event ID for logging var sentryEvent = new SentryEvent(exc); // If the event is us responding to our own error messages, don't bother logging - if (evt is MessageCreateEvent mc && mc.Author.Id == ourUserId) + if (evt is MessageCreateEvent mc && mc.Author.Id == _config.ClientId) return; var shouldReport = exc.IsOurProblem(); @@ -235,7 +232,7 @@ public class Bot return; // Once we've sent it to Sentry, report it to the user (if we have permission to) - var reportChannel = handler.ErrorChannelFor(evt, ourUserId); + var reportChannel = handler.ErrorChannelFor(evt, _config.ClientId); if (reportChannel == null) return; diff --git a/PluralKit.Bot/BotConfig.cs b/PluralKit.Bot/BotConfig.cs index 8c42ed7f..18c44b28 100644 --- a/PluralKit.Bot/BotConfig.cs +++ b/PluralKit.Bot/BotConfig.cs @@ -5,7 +5,7 @@ public class BotConfig public static readonly string[] DefaultPrefixes = { "pk;", "pk!" }; public string Token { get; set; } - public ulong? ClientId { get; set; } + public ulong ClientId { get; set; } // ASP.NET configuration merges arrays with defaults, so we leave this field nullable // and fall back to the separate default array at the use site :) diff --git a/PluralKit.Bot/Commands/Checks.cs b/PluralKit.Bot/Commands/Checks.cs index 4377233f..f4d37a27 100644 --- a/PluralKit.Bot/Commands/Checks.cs +++ b/PluralKit.Bot/Commands/Checks.cs @@ -14,8 +14,6 @@ namespace PluralKit.Bot; public class Checks { private readonly BotConfig _botConfig; - // this must ONLY be used to get the bot's user ID - private readonly IDiscordCache _cache; private readonly ProxyMatcher _matcher; private readonly ProxyService _proxy; private readonly DiscordApiClient _rest; @@ -28,10 +26,9 @@ public class Checks }; // todo: make sure everything uses the minimum amount of REST calls necessary - public Checks(DiscordApiClient rest, IDiscordCache cache, BotConfig botConfig, ProxyService proxy, ProxyMatcher matcher) + public Checks(DiscordApiClient rest, BotConfig botConfig, ProxyService proxy, ProxyMatcher matcher) { _rest = rest; - _cache = cache; _botConfig = botConfig; _proxy = proxy; _matcher = matcher; @@ -69,14 +66,14 @@ public class Checks throw Errors.GuildNotFound(guildId); } - var guildMember = await _rest.GetGuildMember(guild.Id, await _cache.GetOwnUser()); + var guildMember = await _rest.GetGuildMember(guild.Id, _botConfig.ClientId); // Loop through every channel and group them by sets of permissions missing var permissionsMissing = new Dictionary>(); var hiddenChannels = false; foreach (var channel in await _rest.GetGuildChannels(guild.Id)) { - var botPermissions = PermissionExtensions.PermissionsFor(guild, channel, await _cache.GetOwnUser(), guildMember); + var botPermissions = PermissionExtensions.PermissionsFor(guild, channel, _botConfig.ClientId, guildMember); var userPermissions = PermissionExtensions.PermissionsFor(guild, channel, ctx.Author.Id, senderGuildUser); if ((userPermissions & PermissionSet.ViewChannel) == 0) @@ -152,12 +149,12 @@ public class Checks if (guild == null) throw new PKError(error); - var guildMember = await _rest.GetGuildMember(channel.GuildId.Value, await _cache.GetOwnUser()); + var guildMember = await _rest.GetGuildMember(channel.GuildId.Value, _botConfig.ClientId); if (!await ctx.CheckPermissionsInGuildChannel(channel, PermissionSet.ViewChannel)) throw new PKError(error); - var botPermissions = PermissionExtensions.PermissionsFor(guild, channel, await _cache.GetOwnUser(), guildMember); + var botPermissions = PermissionExtensions.PermissionsFor(guild, channel, _botConfig.ClientId, guildMember); // We use a bitfield so we can set individual permission bits ulong missingPermissions = 0; diff --git a/PluralKit.Bot/Commands/Misc.cs b/PluralKit.Bot/Commands/Misc.cs index 25bbdf85..8ada0ee4 100644 --- a/PluralKit.Bot/Commands/Misc.cs +++ b/PluralKit.Bot/Commands/Misc.cs @@ -18,26 +18,22 @@ namespace PluralKit.Bot; public class Misc { private readonly BotConfig _botConfig; - private readonly IDiscordCache _cache; private readonly CpuStatService _cpu; private readonly IMetrics _metrics; private readonly ShardInfoService _shards; private readonly ModelRepository _repo; - public Misc(BotConfig botConfig, IMetrics metrics, CpuStatService cpu, ModelRepository repo, ShardInfoService shards, IDiscordCache cache) + public Misc(BotConfig botConfig, IMetrics metrics, CpuStatService cpu, ModelRepository repo, ShardInfoService shards) { _botConfig = botConfig; _metrics = metrics; _cpu = cpu; _repo = repo; _shards = shards; - _cache = cache; } public async Task Invite(Context ctx) { - var clientId = _botConfig.ClientId ?? await _cache.GetOwnUser(); - var permissions = PermissionSet.AddReactions | PermissionSet.AttachFiles | @@ -48,7 +44,7 @@ public class Misc PermissionSet.SendMessages; var invite = - $"https://discord.com/oauth2/authorize?client_id={clientId}&scope=bot%20applications.commands&permissions={(ulong)permissions}"; + $"https://discord.com/oauth2/authorize?client_id={_botConfig.ClientId}&scope=bot%20applications.commands&permissions={(ulong)permissions}"; var botName = _botConfig.IsBetaBot ? "PluralKit Beta" : "PluralKit"; await ctx.Reply($"{Emojis.Success} Use this link to add {botName} to your server:\n<{invite}>"); diff --git a/PluralKit.Bot/Handlers/MessageCreated.cs b/PluralKit.Bot/Handlers/MessageCreated.cs index 000e05ae..3625ea58 100644 --- a/PluralKit.Bot/Handlers/MessageCreated.cs +++ b/PluralKit.Bot/Handlers/MessageCreated.cs @@ -59,7 +59,7 @@ public class MessageCreated: IEventHandler public async Task Handle(int shardId, MessageCreateEvent evt) { - if (evt.Author.Id == await _cache.GetOwnUser()) return; + if (evt.Author.Id == _config.ClientId) return; if (evt.Type != Message.MessageType.Default && evt.Type != Message.MessageType.Reply) return; if (IsDuplicateMessage(evt)) return; @@ -109,10 +109,8 @@ public class MessageCreated: IEventHandler var content = evt.Content; if (content == null) return false; - var ourUserId = await _cache.GetOwnUser(); - // Check for command prefix - if (!HasCommandPrefix(content, ourUserId, out var cmdStart) || cmdStart == content.Length) + if (!HasCommandPrefix(content, _config.ClientId, out var cmdStart) || cmdStart == content.Length) return false; // Trim leading whitespace from command without actually modifying the string diff --git a/PluralKit.Bot/Handlers/MessageEdited.cs b/PluralKit.Bot/Handlers/MessageEdited.cs index 5e271d9f..a57269c1 100644 --- a/PluralKit.Bot/Handlers/MessageEdited.cs +++ b/PluralKit.Bot/Handlers/MessageEdited.cs @@ -15,6 +15,7 @@ namespace PluralKit.Bot; public class MessageEdited: IEventHandler { private readonly Bot _bot; + private readonly BotConfig _config; private readonly IDiscordCache _cache; private readonly Cluster _client; private readonly IDatabase _db; @@ -27,7 +28,7 @@ public class MessageEdited: IEventHandler public MessageEdited(LastMessageCacheService lastMessageCache, ProxyService proxy, IDatabase db, IMetrics metrics, ModelRepository repo, Cluster client, IDiscordCache cache, Bot bot, - DiscordApiClient rest, ILogger logger) + BotConfig config, DiscordApiClient rest, ILogger logger) { _lastMessageCache = lastMessageCache; _proxy = proxy; @@ -37,13 +38,14 @@ public class MessageEdited: IEventHandler _client = client; _cache = cache; _bot = bot; + _config = config; _rest = rest; _logger = logger.ForContext(); } public async Task Handle(int shardId, MessageUpdateEvent evt) { - if (evt.Author.Value?.Id == await _cache.GetOwnUser()) return; + if (evt.Author.Value?.Id == _config.ClientId) return; // Edit message events sometimes arrive with missing data; double-check it's all there if (!evt.Content.HasValue || !evt.Author.HasValue || !evt.Member.HasValue) diff --git a/PluralKit.Bot/Handlers/ReactionAdded.cs b/PluralKit.Bot/Handlers/ReactionAdded.cs index 155e2bf8..50952e08 100644 --- a/PluralKit.Bot/Handlers/ReactionAdded.cs +++ b/PluralKit.Bot/Handlers/ReactionAdded.cs @@ -18,6 +18,7 @@ namespace PluralKit.Bot; public class ReactionAdded: IEventHandler { private readonly Bot _bot; + private readonly BotConfig _config; private readonly IDiscordCache _cache; private readonly Cluster _cluster; private readonly CommandMessageService _commandMessageService; @@ -30,13 +31,14 @@ public class ReactionAdded: IEventHandler public ReactionAdded(ILogger logger, IDatabase db, ModelRepository repo, CommandMessageService commandMessageService, IDiscordCache cache, Bot bot, Cluster cluster, - DiscordApiClient rest, EmbedService embeds, PrivateChannelService dmCache) + BotConfig config, DiscordApiClient rest, EmbedService embeds, PrivateChannelService dmCache) { _db = db; _repo = repo; _commandMessageService = commandMessageService; _cache = cache; _bot = bot; + _config = config; _cluster = cluster; _rest = rest; _embeds = embeds; @@ -52,7 +54,7 @@ public class ReactionAdded: IEventHandler private async ValueTask TryHandleProxyMessageReactions(MessageReactionAddEvent evt) { // ignore any reactions added by *us* - if (evt.UserId == await _cache.GetOwnUser()) + if (evt.UserId == _config.ClientId) return; // Ignore reactions from bots (we can't DM them anyway) diff --git a/PluralKit.Bot/Init.cs b/PluralKit.Bot/Init.cs index fdbf186e..066d0d16 100644 --- a/PluralKit.Bot/Init.cs +++ b/PluralKit.Bot/Init.cs @@ -58,7 +58,7 @@ public class Init var cache = services.Resolve(); if (cache is RedisDiscordCache) - await (cache as RedisDiscordCache).InitAsync(coreConfig.RedisAddr, config.ClientId!.Value); + await (cache as RedisDiscordCache).InitAsync(coreConfig.RedisAddr); if (config.Cluster == null) { diff --git a/PluralKit.Bot/Modules.cs b/PluralKit.Bot/Modules.cs index 39448f2a..3c14a1cf 100644 --- a/PluralKit.Bot/Modules.cs +++ b/PluralKit.Bot/Modules.cs @@ -49,8 +49,8 @@ public class BotModule: Module var botConfig = c.Resolve(); if (botConfig.UseRedisCache) - return new RedisDiscordCache(c.Resolve()); - return new MemoryDiscordCache(); + return new RedisDiscordCache(c.Resolve(), botConfig.ClientId); + return new MemoryDiscordCache(botConfig.ClientId); }).AsSelf().SingleInstance(); builder.RegisterType().AsSelf().SingleInstance(); diff --git a/PluralKit.Bot/Services/LogChannelService.cs b/PluralKit.Bot/Services/LogChannelService.cs index 35eace35..5da33ea5 100644 --- a/PluralKit.Bot/Services/LogChannelService.cs +++ b/PluralKit.Bot/Services/LogChannelService.cs @@ -15,6 +15,7 @@ namespace PluralKit.Bot; public class LogChannelService { private readonly Bot _bot; + private readonly BotConfig _config; private readonly IDiscordCache _cache; private readonly IDatabase _db; private readonly EmbedService _embed; @@ -23,7 +24,7 @@ public class LogChannelService private readonly DiscordApiClient _rest; public LogChannelService(EmbedService embed, ILogger logger, IDatabase db, ModelRepository repo, - IDiscordCache cache, DiscordApiClient rest, Bot bot) + IDiscordCache cache, DiscordApiClient rest, Bot bot, BotConfig config) { _embed = embed; _db = db; @@ -31,6 +32,7 @@ public class LogChannelService _cache = cache; _rest = rest; _bot = bot; + _config = config; _logger = logger.ForContext(); } @@ -97,9 +99,9 @@ public class LogChannelService var guildMember = await _cache.TryGetSelfMember(channel.GuildId.Value); if (guildMember == null) - guildMember = await _rest.GetGuildMember(channel.GuildId.Value, await _cache.GetOwnUser()); + guildMember = await _rest.GetGuildMember(channel.GuildId.Value, _config.ClientId); - var perms = PermissionExtensions.PermissionsFor(guild, channel, await _cache.GetOwnUser(), guildMember); + var perms = PermissionExtensions.PermissionsFor(guild, channel, _config.ClientId, guildMember); return perms; } diff --git a/PluralKit.Bot/Services/WebhookCacheService.cs b/PluralKit.Bot/Services/WebhookCacheService.cs index d7e8aa98..6eb5a236 100644 --- a/PluralKit.Bot/Services/WebhookCacheService.cs +++ b/PluralKit.Bot/Services/WebhookCacheService.cs @@ -17,16 +17,18 @@ public class WebhookCacheService private readonly IDiscordCache _cache; private readonly ILogger _logger; private readonly IMetrics _metrics; + private readonly BotConfig _config; private readonly DiscordApiClient _rest; private readonly ConcurrentDictionary>> _webhooks; - public WebhookCacheService(ILogger logger, IMetrics metrics, DiscordApiClient rest, IDiscordCache cache) + public WebhookCacheService(ILogger logger, IMetrics metrics, DiscordApiClient rest, IDiscordCache cache, BotConfig config) { _metrics = metrics; _rest = rest; _cache = cache; + _config = config; _logger = logger.ForContext(); _webhooks = new ConcurrentDictionary>>(); } @@ -86,8 +88,7 @@ public class WebhookCacheService var webhooks = await FetchChannelWebhooks(channelId); // If the channel has a webhook created by PK, just return that one - var ourUserId = await _cache.GetOwnUser(); - var ourWebhook = webhooks.FirstOrDefault(hook => IsWebhookMine(ourUserId, hook)); + var ourWebhook = webhooks.FirstOrDefault(hook => IsWebhookMine(hook)); if (ourWebhook != null) return ourWebhook; @@ -122,5 +123,5 @@ public class WebhookCacheService return await _rest.CreateWebhook(channelId, new CreateWebhookRequest(WebhookName)); } - private bool IsWebhookMine(ulong userId, Webhook arg) => arg.User?.Id == userId && arg.Name == WebhookName; + private bool IsWebhookMine(Webhook arg) => arg.User?.Id == _config.ClientId && arg.Name == WebhookName; } \ No newline at end of file diff --git a/README.md b/README.md index 3c569737..2f807698 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ The bot can also take configuration from environment variables, which will overr The easiest way to get the bot running is with Docker. The repository contains a `docker-compose.yml` file ready to use. * Clone this repository: `git clone https://github.com/PluralKit/PluralKit` -* Create a `pluralkit.conf` file in the same directory as `docker-compose.yml` containing at least a `PluralKit.Bot.Token` field +* Create a `pluralkit.conf` file in the same directory as `docker-compose.yml` containing at least `PluralKit.Bot.Token` and `PluralKit.Bot.ClientId` fields * (`PluralKit.Database` is overridden in `docker-compose.yml` to point to the Postgres container) * Build the bot: `docker-compose build` * Run the bot: `docker-compose up` diff --git a/pluralkit.conf.example b/pluralkit.conf.example index 60ca0f30..06e3fc05 100644 --- a/pluralkit.conf.example +++ b/pluralkit.conf.example @@ -1,7 +1,8 @@ { "PluralKit": { "Bot": { - "Token": "BOT_TOKEN_GOES_HERE" + "Token": "BOT_TOKEN_GOES_HERE", + "ClientId": 466707357099884544, }, "Database": "Host=localhost;Port=5432;Username=myusername;Password=mypassword;Database=mydatabasename" }