Skip to content

Commit

Permalink
Use nonmax::NonMax* types in Options wherever possible (serenity-…
Browse files Browse the repository at this point in the history
…rs#2681)

This swaps fields that store `Option<Int>` for `Option<NonMaxInt>` where the
maximum value would be ludicrous. Since `nonmax` uses `NonZero` internally,
this gives us niche optimisations, so model sizes can drop some more.

I have had to include a workaround for [serenity-rs#17] in `optional_string` by making my
own `TryFrom<u64>`, so that should be removable once that issue is fixed.

[serenity-rs#17]: LPGhatguy/nonmax#17
  • Loading branch information
GnomedDev authored and arqunis committed Jan 16, 2024
1 parent b9f1b68 commit 759d78a
Show file tree
Hide file tree
Showing 15 changed files with 123 additions and 62 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ secrecy = { version = "0.8.0", features = ["serde"] }
arrayvec = { version = "0.7.4", features = ["serde"] }
small-fixed-array = { git = "https://github.com/GnomedDev/small-fixed-array", features = ["serde", "log_using_tracing"] }
bool_to_bitflags = { git = "https://github.com/GnomedDev/bool-to-bitflags", version = "0.1.0" }
nonmax = { version = "0.5.5", features = ["serde"] }
# Optional dependencies
fxhash = { version = "0.2.1", optional = true }
simd-json = { version = "0.13.4", optional = true }
Expand All @@ -50,7 +51,7 @@ mime_guess = { version = "2.0.4", optional = true }
dashmap = { version = "5.5.3", features = ["serde"], optional = true }
parking_lot = { version = "0.12.1", optional = true }
ed25519-dalek = { version = "2.0.0", optional = true }
typesize = { version = "0.1.4", optional = true, features = ["url", "time", "serde_json", "secrecy", "dashmap", "parking_lot", "details"] }
typesize = { version = "0.1.5", optional = true, features = ["url", "time", "serde_json", "secrecy", "dashmap", "parking_lot", "nonmax", "details"] }
# serde feature only allows for serialisation,
# Serenity workspace crates
command_attr = { version = "0.5.1", path = "./command_attr", optional = true }
Expand Down
7 changes: 5 additions & 2 deletions examples/e05_command_framework/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,11 @@ async fn slow_mode(ctx: &Context, msg: &Message, mut args: Args) -> CommandResul
format!("Successfully set slow mode rate to `{slow_mode_rate_seconds}` seconds.")
}
} 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.")
if let Some(slow_mode_rate) = channel.rate_limit_per_user {
format!("Current slow mode rate is `{slow_mode_rate}` seconds.")
} else {
"There is no current slow mode rate for this channel.".to_string()
}
} else {
"Failed to find channel in cache.".to_string()
};
Expand Down
9 changes: 5 additions & 4 deletions src/model/channel/attachment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use nonmax::NonMaxU32;
#[cfg(feature = "model")]
use reqwest::Client as ReqwestClient;

Expand Down Expand Up @@ -39,15 +40,15 @@ pub struct Attachment {
/// Sescription for the file (max 1024 characters).
pub description: Option<FixedString<u16>>,
/// If the attachment is an image, then the height of the image is provided.
pub height: Option<u32>,
pub height: Option<NonMaxU32>,
/// If the attachment is an image, then the width of the image is provided.
pub width: Option<NonMaxU32>,
/// The proxy URL.
pub proxy_url: FixedString,
/// The size of the file in bytes.
pub size: u32,
/// The URL of the uploaded attachment.
pub url: FixedString,
/// If the attachment is an image, then the width of the image is provided.
pub width: Option<u32>,
/// The attachment's [media type].
///
/// [media type]: https://en.wikipedia.org/wiki/Media_type
Expand Down Expand Up @@ -79,7 +80,7 @@ pub struct Attachment {
impl Attachment {
/// If this attachment is an image, then a tuple of the width and height in pixels is returned.
#[must_use]
pub fn dimensions(&self) -> Option<(u32, u32)> {
pub fn dimensions(&self) -> Option<(NonMaxU32, NonMaxU32)> {
self.width.and_then(|width| self.height.map(|height| (width, height)))
}

Expand Down
14 changes: 8 additions & 6 deletions src/model/channel/embed.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use nonmax::NonMaxU32;

use crate::internal::prelude::*;
use crate::model::{Colour, Timestamp};

Expand Down Expand Up @@ -171,9 +173,9 @@ pub struct EmbedImage {
/// A proxied URL of the image.
pub proxy_url: Option<FixedString>,
/// The height of the image.
pub height: Option<u32>,
pub height: Option<NonMaxU32>,
/// The width of the image.
pub width: Option<u32>,
pub width: Option<NonMaxU32>,
}

/// The provider of an embed.
Expand Down Expand Up @@ -203,9 +205,9 @@ pub struct EmbedThumbnail {
/// A proxied URL of the thumbnail.
pub proxy_url: Option<FixedString>,
/// The height of the thumbnail in pixels.
pub height: Option<u32>,
pub height: Option<NonMaxU32>,
/// The width of the thumbnail in pixels.
pub width: Option<u32>,
pub width: Option<NonMaxU32>,
}

/// Video information for an embed.
Expand All @@ -220,7 +222,7 @@ pub struct EmbedVideo {
/// A proxied URL of the thumbnail.
pub proxy_url: Option<FixedString>,
/// The height of the video in pixels.
pub height: Option<u32>,
pub height: Option<NonMaxU32>,
/// The width of the video in pixels.
pub width: Option<u32>,
pub width: Option<NonMaxU32>,
}
18 changes: 9 additions & 9 deletions src/model/channel/guild_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use std::fmt;
#[cfg(feature = "model")]
use std::sync::Arc;

use nonmax::{NonMaxU16, NonMaxU32, NonMaxU8};

#[cfg(feature = "model")]
use crate::builder::{
Builder,
Expand Down Expand Up @@ -45,7 +47,7 @@ pub struct GuildChannel {
/// The bitrate of the channel.
///
/// **Note**: This is only available for voice and stage channels.
pub bitrate: Option<u32>,
pub bitrate: Option<NonMaxU32>,
/// The Id of the parent category for a channel, or of the parent text channel for a thread.
///
/// **Note**: This is only available for channels in a category and thread channels.
Expand Down Expand Up @@ -89,7 +91,7 @@ pub struct GuildChannel {
/// The maximum number of members allowed in the channel.
///
/// **Note**: This is only available for voice channels.
pub user_limit: Option<u32>,
pub user_limit: Option<NonMaxU8>,
/// Used to tell if the channel is not safe for work. Note however, it's recommended to use
/// [`Self::is_nsfw`] as it's gonna be more accurate.
// This field can or can not be present sometimes, but if it isn't default to `false`.
Expand All @@ -100,7 +102,7 @@ pub struct GuildChannel {
/// **Note**: This is only available for text channels excluding news channels.
#[doc(alias = "slowmode")]
#[serde(default)]
pub rate_limit_per_user: Option<u16>,
pub rate_limit_per_user: Option<NonMaxU16>,
/// The region override.
///
/// **Note**: This is only available for voice and stage channels. [`None`] for voice and stage
Expand All @@ -110,14 +112,12 @@ pub struct GuildChannel {
pub video_quality_mode: Option<VideoQualityMode>,
/// An approximate count of messages in the thread.
///
/// This is currently saturated at 255 to prevent breaking.
///
/// **Note**: This is only available on thread channels.
pub message_count: Option<u32>,
pub message_count: Option<NonMaxU32>,
/// An approximate count of users in a thread, stops counting at 50.
///
/// **Note**: This is only available on thread channels.
pub member_count: Option<u8>,
pub member_count: Option<NonMaxU8>,
/// The thread metadata.
///
/// **Note**: This is only available on thread channels.
Expand All @@ -139,7 +139,7 @@ pub struct GuildChannel {
pub flags: ChannelFlags,
/// The number of messages ever sent in a thread, it's similar to `message_count` on message
/// creation, but will not decrement the number when a message is deleted.
pub total_message_sent: Option<u64>,
pub total_message_sent: Option<NonMaxU32>,
/// The set of available tags.
///
/// **Note**: This is only available in forum channels.
Expand All @@ -158,7 +158,7 @@ pub struct GuildChannel {
/// is copied to the thread at creation time and does not live update.
///
/// **Note**: This is only available in a forum or text channel.
pub default_thread_rate_limit_per_user: Option<u16>,
pub default_thread_rate_limit_per_user: Option<NonMaxU16>,
/// The status of a voice channel.
///
/// **Note**: This is only available in voice channels.
Expand Down
4 changes: 3 additions & 1 deletion src/model/channel/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::fmt::Display;
#[cfg(all(feature = "cache", feature = "model"))]
use std::fmt::Write;

use nonmax::NonMaxU64;

#[cfg(all(feature = "model", feature = "utils"))]
use crate::builder::{Builder, CreateAllowedMentions, CreateMessage, EditMessage};
#[cfg(all(feature = "cache", feature = "model"))]
Expand Down Expand Up @@ -123,7 +125,7 @@ pub struct Message {
/// A generally increasing integer (there may be gaps or duplicates) that represents the
/// approximate position of the message in a thread, it can be used to estimate the relative
/// position of the message in a thread in company with total_message_sent on parent thread.
pub position: Option<u64>,
pub position: Option<NonMaxU64>,
/// Data of the role subscription purchase or renewal that prompted this
/// [`MessageType::RoleSubscriptionPurchase`] message.
pub role_subscription_data: Option<RoleSubscriptionData>,
Expand Down
3 changes: 2 additions & 1 deletion src/model/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use std::collections::HashMap;

use nonmax::NonMaxU64;
use serde::de::Error as DeError;
use serde::Serialize;

Expand Down Expand Up @@ -539,7 +540,7 @@ pub struct MessageUpdateEvent {
pub thread: Option<Option<Box<GuildChannel>>>,
pub components: Option<FixedArray<ActionRow>>,
pub sticker_items: Option<FixedArray<StickerItem>>,
pub position: Option<Option<u64>>,
pub position: Option<Option<NonMaxU64>>,
pub role_subscription_data: Option<Option<RoleSubscriptionData>>,
pub guild_id: Option<GuildId>,
pub member: Option<Option<Box<PartialMember>>>,
Expand Down
6 changes: 3 additions & 3 deletions src/model/gateway.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Models pertaining to the gateway.

use std::num::NonZeroU16;
use std::num::{NonZeroU16, NonZeroU64};

use serde::ser::SerializeSeq;
use url::Url;
Expand Down Expand Up @@ -402,8 +402,8 @@ impl serde::Serialize for ShardInfo {
#[derive(Clone, Debug, Deserialize, Serialize)]
#[non_exhaustive]
pub struct ActivityTimestamps {
pub end: Option<u64>,
pub start: Option<u64>,
pub end: Option<NonZeroU64>,
pub start: Option<NonZeroU64>,
}

bitflags! {
Expand Down
7 changes: 4 additions & 3 deletions src/model/guild/audit_log/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::collections::HashMap;
use std::mem::transmute;

use nonmax::{NonMaxU32, NonMaxU64};
use serde::de::Deserializer;
use serde::ser::{Serialize, Serializer};

Expand Down Expand Up @@ -370,16 +371,16 @@ pub struct Options {
pub application_id: Option<ApplicationId>,
/// Number of days after which inactive members were kicked.
#[serde(default, with = "optional_string")]
pub delete_member_days: Option<u64>,
pub delete_member_days: Option<NonMaxU32>,
/// Number of members removed by the prune
#[serde(default, with = "optional_string")]
pub members_removed: Option<u64>,
pub members_removed: Option<NonMaxU64>,
/// Channel in which the messages were deleted
#[serde(default)]
pub channel_id: Option<ChannelId>,
/// Number of deleted messages.
#[serde(default, with = "optional_string")]
pub count: Option<u64>,
pub count: Option<NonMaxU64>,
/// Id of the overwritten entity
#[serde(default)]
pub id: Option<GenericId>,
Expand Down
66 changes: 54 additions & 12 deletions src/model/guild/audit_log/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,69 @@ pub mod webhooks {
/// Used with `#[serde(with = "optional_string")]`.
pub mod optional_string {
use std::fmt;
use std::marker::PhantomData;
use std::str::FromStr;

use serde::de::{Deserializer, Error, Visitor};
use serde::ser::Serializer;

pub fn deserialize<'de, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Option<u64>, D::Error> {
deserializer.deserialize_option(OptionalStringVisitor)
// Workaround for https://github.com/LPGhatguy/nonmax/issues/17
pub(crate) trait TryFromU64
where
Self: Sized,
{
type Err: fmt::Display;
fn try_from_u64(value: u64) -> Result<Self, Self::Err>;
}

impl TryFromU64 for u64 {
type Err = std::convert::Infallible;
fn try_from_u64(value: u64) -> Result<Self, Self::Err> {
Ok(value)
}
}

impl TryFromU64 for nonmax::NonMaxU64 {
type Err = nonmax::TryFromIntError;
fn try_from_u64(value: u64) -> Result<Self, Self::Err> {
Self::try_from(value)
}
}

impl TryFromU64 for nonmax::NonMaxU32 {
type Err = nonmax::TryFromIntError;
fn try_from_u64(value: u64) -> Result<Self, Self::Err> {
Self::try_from(u32::try_from(value)?)
}
}

pub fn deserialize<'de, D, T>(deserializer: D) -> Result<Option<T>, D::Error>
where
D: Deserializer<'de>,
T: FromStr + TryFromU64,
<T as FromStr>::Err: fmt::Display,
{
deserializer.deserialize_option(OptionalStringVisitor::<T>(PhantomData))
}

pub fn serialize<S: Serializer>(value: &Option<u64>, serializer: S) -> Result<S::Ok, S::Error> {
pub fn serialize<S: Serializer>(
value: &Option<impl ToString>,
serializer: S,
) -> Result<S::Ok, S::Error> {
match value {
Some(value) => serializer.serialize_some(&value.to_string()),
None => serializer.serialize_none(),
}
}

struct OptionalStringVisitor;
struct OptionalStringVisitor<T>(PhantomData<T>);

impl<'de> Visitor<'de> for OptionalStringVisitor {
type Value = Option<u64>;
impl<'de, T> Visitor<'de> for OptionalStringVisitor<T>
where
T: FromStr + TryFromU64,
<T as FromStr>::Err: fmt::Display,
{
type Value = Option<T>;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("an optional integer or a string with a valid number inside")
Expand All @@ -71,7 +113,7 @@ pub mod optional_string {
self,
deserializer: D,
) -> Result<Self::Value, D::Error> {
deserializer.deserialize_any(OptionalStringVisitor)
deserializer.deserialize_any(OptionalStringVisitor(PhantomData))
}

fn visit_none<E: Error>(self) -> Result<Self::Value, E> {
Expand All @@ -83,11 +125,11 @@ pub mod optional_string {
Ok(None)
}

fn visit_u64<E: Error>(self, val: u64) -> Result<Option<u64>, E> {
Ok(Some(val))
fn visit_u64<E: Error>(self, val: u64) -> Result<Self::Value, E> {
T::try_from_u64(val).map(Some).map_err(Error::custom)
}

fn visit_str<E: Error>(self, string: &str) -> Result<Option<u64>, E> {
fn visit_str<E: Error>(self, string: &str) -> Result<Self::Value, E> {
string.parse().map(Some).map_err(Error::custom)
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/model/guild/integration.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use nonmax::{NonMaxU32, NonMaxU64};

use super::*;
use crate::model::Timestamp;

Expand All @@ -21,11 +23,11 @@ pub struct Integration {
pub enable_emoticons: Option<bool>,
#[serde(rename = "expire_behavior")]
pub expire_behaviour: Option<IntegrationExpireBehaviour>,
pub expire_grace_period: Option<u64>,
pub expire_grace_period: Option<NonMaxU32>,
pub user: Option<User>,
pub account: IntegrationAccount,
pub synced_at: Option<Timestamp>,
pub subscriber_count: Option<u64>,
pub subscriber_count: Option<NonMaxU64>,
pub revoked: Option<bool>,
pub application: Option<IntegrationApplication>,
pub scopes: Option<Vec<Scope>>,
Expand Down
Loading

0 comments on commit 759d78a

Please sign in to comment.