Skip to content

Commit

Permalink
Remove PrivateChannel from caching and events (serenity-rs#2603)
Browse files Browse the repository at this point in the history
The entire `PrivateChannel` dashmap was never used, Discord hasn't supplied
events for it since API v8, and all the methods handling it were just unneeded.

This allows for removal of `Channel` from a lot of the API surface, replacing
it with the specific `GuildChannel` or specifically using `PrivateChannel`.

This was motivated by the `#[allow(clippy::large_enum_variant)]` on `Channel`
making me reconsider if it is strictly needed, which it is for a few endpoints
(http get channel can return private channels), but was able to be removed from
a lot of endpoints.
  • Loading branch information
GnomedDev authored Nov 19, 2023
1 parent ad85039 commit 02ea51b
Show file tree
Hide file tree
Showing 24 changed files with 134 additions and 430 deletions.
4 changes: 2 additions & 2 deletions examples/e05_command_framework/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use serenity::framework::standard::{
};
use serenity::gateway::ShardManager;
use serenity::http::Http;
use serenity::model::channel::{Channel, Message};
use serenity::model::channel::Message;
use serenity::model::gateway::{GatewayIntents, Ready};
use serenity::model::id::UserId;
use serenity::model::permissions::Permissions;
Expand Down Expand Up @@ -557,7 +557,7 @@ async fn slow_mode(ctx: &Context, msg: &Message, mut args: Args) -> CommandResul
} else {
format!("Successfully set slow mode rate to `{slow_mode_rate_seconds}` seconds.")
}
} else if let Some(Channel::Guild(channel)) = msg.channel_id.to_channel_cached(&ctx.cache) {
} else if let Some(channel) = msg.channel_id.to_channel_cached(&ctx.cache) {
let slow_mode_rate = channel.rate_limit_per_user.unwrap_or(0);
format!("Current slow mode rate is `{slow_mode_rate}` seconds.")
} else {
Expand Down
24 changes: 8 additions & 16 deletions src/builder/create_invite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::model::prelude::*;
/// impl EventHandler for Handler {
/// async fn message(&self, context: Context, msg: Message) {
/// if msg.content == "!createinvite" {
/// let channel_opt = context.cache.guild_channel(msg.channel_id).as_deref().cloned();
/// let channel_opt = context.cache.channel(msg.channel_id).as_deref().cloned();
/// let channel = match channel_opt {
/// Some(channel) => channel,
/// None => {
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<'a> CreateInvite<'a> {
/// # #[cfg(all(feature = "cache", feature = "client", feature = "framework", feature = "http"))]
/// # #[command]
/// # async fn example(context: &Context) -> CommandResult {
/// # let channel = context.cache.guild_channel(81384788765712384).unwrap().clone();
/// # let channel = context.cache.channel(81384788765712384).unwrap().clone();
/// let builder = CreateInvite::new().max_age(3600);
/// let invite = channel.create_invite(context, builder).await?;
/// # Ok(())
Expand Down Expand Up @@ -150,7 +150,7 @@ impl<'a> CreateInvite<'a> {
/// # #[cfg(all(feature = "cache", feature = "client", feature = "framework", feature = "http"))]
/// # #[command]
/// # async fn example(context: &Context) -> CommandResult {
/// # let channel = context.cache.guild_channel(81384788765712384).unwrap().clone();
/// # let channel = context.cache.channel(81384788765712384).unwrap().clone();
/// let builder = CreateInvite::new().max_uses(5);
/// let invite = channel.create_invite(context, builder).await?;
/// # Ok(())
Expand Down Expand Up @@ -180,7 +180,7 @@ impl<'a> CreateInvite<'a> {
/// # #[cfg(all(feature = "cache", feature = "client", feature = "framework", feature = "http"))]
/// # #[command]
/// # async fn example(context: &Context) -> CommandResult {
/// # let channel = context.cache.guild_channel(81384788765712384).unwrap().clone();
/// # let channel = context.cache.channel(81384788765712384).unwrap().clone();
/// let builder = CreateInvite::new().temporary(true);
/// let invite = channel.create_invite(context, builder).await?;
/// # Ok(())
Expand Down Expand Up @@ -210,7 +210,7 @@ impl<'a> CreateInvite<'a> {
/// # #[cfg(all(feature = "cache", feature = "client", feature = "framework", feature = "http"))]
/// # #[command]
/// # async fn example(context: &Context) -> CommandResult {
/// # let channel = context.cache.guild_channel(81384788765712384).unwrap().clone();
/// # let channel = context.cache.channel(81384788765712384).unwrap().clone();
/// let builder = CreateInvite::new().unique(true);
/// let invite = channel.create_invite(context, builder).await?;
/// # Ok(())
Expand Down Expand Up @@ -265,10 +265,7 @@ impl<'a> CreateInvite<'a> {
#[cfg(feature = "http")]
#[async_trait::async_trait]
impl<'a> Builder for CreateInvite<'a> {
#[cfg(feature = "cache")]
type Context<'ctx> = (ChannelId, Option<GuildId>);
#[cfg(not(feature = "cache"))]
type Context<'ctx> = (ChannelId,);
type Context<'ctx> = ChannelId;
type Built = RichInvite;

/// Creates an invite for the given channel.
Expand All @@ -289,15 +286,10 @@ impl<'a> Builder for CreateInvite<'a> {
#[cfg(feature = "cache")]
{
if let Some(cache) = cache_http.cache() {
crate::utils::user_has_perms_cache(
cache,
ctx.0,
ctx.1,
Permissions::CREATE_INSTANT_INVITE,
)?;
crate::utils::user_has_perms_cache(cache, ctx, Permissions::CREATE_INSTANT_INVITE)?;
}
}

cache_http.http().create_invite(ctx.0, &self, self.audit_log_reason).await
cache_http.http().create_invite(ctx, &self, self.audit_log_reason).await
}
}
4 changes: 2 additions & 2 deletions src/builder/create_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ impl Builder for CreateMessage {
req |= Permissions::ATTACH_FILES;
}
if let Some(cache) = cache_http.cache() {
crate::utils::user_has_perms_cache(cache, channel_id, guild_id, req)?;
crate::utils::user_has_perms_cache(cache, channel_id, req)?;
}
}

Expand All @@ -344,7 +344,7 @@ impl Builder for CreateMessage {
// Use either the passed in guild ID (e.g. if we were called from GuildChannel directly
// we already know our guild ID), and otherwise find the guild ID in cache
message.guild_id = guild_id
.or_else(|| Some(cache_http.cache()?.guild_channel(message.channel_id)?.guild_id));
.or_else(|| Some(cache_http.cache()?.channel(message.channel_id)?.guild_id));
}

Ok(message)
Expand Down
2 changes: 1 addition & 1 deletion src/builder/create_stage_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'a> Builder for CreateStageInstance<'a> {
#[cfg(feature = "cache")]
{
if let Some(cache) = cache_http.cache() {
if let Some(channel) = cache.guild_channel(ctx) {
if let Some(channel) = cache.channel(ctx) {
if channel.kind != ChannelType::Stage {
return Err(Error::Model(ModelError::InvalidChannelType));
}
Expand Down
2 changes: 1 addition & 1 deletion src/builder/create_webhook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<'a> Builder for CreateWebhook<'a> {
#[cfg(feature = "cache")]
{
if let Some(cache) = cache_http.cache() {
if let Some(channel) = cache.guild_channel(ctx) {
if let Some(channel) = cache.channel(ctx) {
// forum channels are not text-based, but webhooks can be created in them
// and used to send messages in their posts
if !channel.is_text_based() && channel.kind != ChannelType::Forum {
Expand Down
21 changes: 4 additions & 17 deletions src/builder/edit_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,7 @@ impl<'a> EditChannel<'a> {
#[cfg(feature = "http")]
#[async_trait::async_trait]
impl<'a> Builder for EditChannel<'a> {
#[cfg(feature = "cache")]
type Context<'ctx> = (ChannelId, Option<GuildId>);
#[cfg(not(feature = "cache"))]
type Context<'ctx> = (ChannelId,);
type Context<'ctx> = ChannelId;
type Built = GuildChannel;

/// Edits the channel's settings.
Expand All @@ -320,23 +317,13 @@ impl<'a> Builder for EditChannel<'a> {
#[cfg(feature = "cache")]
{
if let Some(cache) = cache_http.cache() {
crate::utils::user_has_perms_cache(
cache,
ctx.0,
ctx.1,
Permissions::MANAGE_CHANNELS,
)?;
crate::utils::user_has_perms_cache(cache, ctx, Permissions::MANAGE_CHANNELS)?;
if self.permission_overwrites.is_some() {
crate::utils::user_has_perms_cache(
cache,
ctx.0,
ctx.1,
Permissions::MANAGE_ROLES,
)?;
crate::utils::user_has_perms_cache(cache, ctx, Permissions::MANAGE_ROLES)?;
}
}
}

cache_http.http().edit_channel(ctx.0, &self, self.audit_log_reason).await
cache_http.http().edit_channel(ctx, &self, self.audit_log_reason).await
}
}
2 changes: 1 addition & 1 deletion src/builder/edit_stage_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl<'a> Builder for EditStageInstance<'a> {
#[cfg(feature = "cache")]
{
if let Some(cache) = cache_http.cache() {
if let Some(channel) = cache.guild_channel(ctx) {
if let Some(channel) = cache.channel(ctx) {
if channel.kind != ChannelType::Stage {
return Err(Error::Model(ModelError::InvalidChannelType));
}
Expand Down
2 changes: 1 addition & 1 deletion src/builder/edit_voice_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl Builder for EditVoiceState {
#[cfg(feature = "cache")]
{
if let Some(cache) = cache_http.cache() {
if let Some(channel) = cache.guild_channel(channel_id) {
if let Some(channel) = cache.channel(channel_id) {
if channel.kind != ChannelType::Stage {
return Err(Error::from(ModelError::InvalidChannelType));
}
Expand Down
103 changes: 21 additions & 82 deletions src/cache/event.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::HashSet;

use super::{Cache, CacheUpdate};
use crate::model::channel::{Channel, GuildChannel, Message};
use crate::model::channel::{GuildChannel, Message};
use crate::model::event::{
ChannelCreateEvent,
ChannelDeleteEvent,
Expand Down Expand Up @@ -38,113 +38,52 @@ use crate::model::user::{CurrentUser, OnlineStatus};
use crate::model::voice::VoiceState;

impl CacheUpdate for ChannelCreateEvent {
type Output = Channel;
type Output = GuildChannel;

fn update(&mut self, cache: &Cache) -> Option<Self::Output> {
match self.channel {
Channel::Guild(ref channel) => {
let (guild_id, channel_id) = (channel.guild_id, channel.id);

let old_channel = cache
.guilds
.get_mut(&guild_id)
.and_then(|mut g| g.channels.insert(channel_id, channel.clone()));

cache.channels.insert(channel_id, channel.guild_id);
old_channel.map(Channel::Guild)
},
Channel::Private(ref mut channel) => {
if let Some(channel) = cache.private_channels.get(&channel.id) {
return Some(Channel::Private(channel.clone()));
}

let id = {
let user_id = {
cache.update_user_entry(&channel.recipient);

channel.recipient.id
};

if let Some(u) = cache.users.get(&user_id) {
channel.recipient = u.clone();
}

channel.id
};
let old_channel = cache
.guilds
.get_mut(&self.channel.guild_id)
.and_then(|mut g| g.channels.insert(self.channel.id, self.channel.clone()));

cache.private_channels.insert(id, channel.clone()).map(Channel::Private)
},
}
cache.channels.insert(self.channel.id, self.channel.guild_id);
old_channel
}
}

impl CacheUpdate for ChannelDeleteEvent {
type Output = Vec<Message>;

fn update(&mut self, cache: &Cache) -> Option<Vec<Message>> {
match &self.channel {
Channel::Guild(channel) => {
let (guild_id, channel_id) = (channel.guild_id, channel.id);
let (channel_id, guild_id) = (self.channel.id, self.channel.guild_id);

cache.channels.remove(&channel_id);

cache.guilds.get_mut(&guild_id).map(|mut g| g.channels.remove(&channel_id));
},
Channel::Private(channel) => {
let id = { channel.id };

cache.private_channels.remove(&id);
},
};
cache.channels.remove(&channel_id);
cache.guilds.get_mut(&guild_id).map(|mut g| g.channels.remove(&channel_id));

// Remove the cached messages for the channel.
cache
.messages
.remove(&self.channel.id())
.map(|(_, messages)| messages.into_values().collect())
cache.messages.remove(&channel_id).map(|(_, messages)| messages.into_values().collect())
}
}

impl CacheUpdate for ChannelUpdateEvent {
type Output = Channel;

fn update(&mut self, cache: &Cache) -> Option<Channel> {
let old_channel = cache.channel(self.channel.id());
type Output = GuildChannel;

match &self.channel {
Channel::Guild(channel) => {
cache.channels.insert(channel.id, channel.guild_id);
fn update(&mut self, cache: &Cache) -> Option<GuildChannel> {
cache.channels.insert(self.channel.id, self.channel.guild_id);

cache
.guilds
.get_mut(&channel.guild_id)
.map(|mut g| g.channels.insert(channel.id, channel.clone()));
},
Channel::Private(channel) => {
if let Some(mut c) = cache.private_channels.get_mut(&channel.id) {
c.clone_from(channel);
}
},
}

old_channel
cache
.guilds
.get_mut(&self.channel.guild_id)
.and_then(|mut g| g.channels.insert(self.channel.id, self.channel.clone()))
}
}

impl CacheUpdate for ChannelPinsUpdateEvent {
type Output = ();

fn update(&mut self, cache: &Cache) -> Option<()> {
if let Some(mut channel) = cache.guild_channel_mut(self.channel_id) {
channel.last_pin_timestamp = self.last_pin_timestamp;

return None;
}

if let Some(mut channel) = cache.private_channels.get_mut(&self.channel_id) {
if let Some(mut channel) = cache.channel_mut(self.channel_id) {
channel.last_pin_timestamp = self.last_pin_timestamp;

return None;
}

None
Expand Down Expand Up @@ -626,7 +565,7 @@ impl CacheUpdate for VoiceChannelStatusUpdateEvent {
type Output = String;

fn update(&mut self, cache: &Cache) -> Option<Self::Output> {
if let Some(mut channel) = cache.guild_channel_mut(self.id) {
if let Some(mut channel) = cache.channel_mut(self.id) {
let old = channel.status.clone();
channel.status = self.status.clone();
// Discord updates topic but doesn't fire ChannelUpdate.
Expand Down
Loading

0 comments on commit 02ea51b

Please sign in to comment.