From aa3cb6e39e30deac9cb13e20c0b631112d7d32c9 Mon Sep 17 00:00:00 2001 From: Valdemar Erk Date: Mon, 8 Jul 2024 17:24:43 +0200 Subject: [PATCH 1/7] fix(model): Unavailable guild must always have unavailable as true This adds a new construct that can be used to only deserialize a specific boolean value. We then use that value to ensure that only guilds which are actually unavailable get deserialized as such. --- .../gateway/payload/incoming/guild_create.rs | 8 +- .../src/gateway/payload/incoming/ready.rs | 5 +- twilight-model/src/guild/unavailable_guild.rs | 11 +- twilight-model/src/util/mod.rs | 1 + twilight-model/src/util/mustbe.rs | 152 ++++++++++++++++++ 5 files changed, 168 insertions(+), 9 deletions(-) create mode 100644 twilight-model/src/util/mustbe.rs diff --git a/twilight-model/src/gateway/payload/incoming/guild_create.rs b/twilight-model/src/gateway/payload/incoming/guild_create.rs index 348d647a6dd..23fbdc44ff3 100644 --- a/twilight-model/src/gateway/payload/incoming/guild_create.rs +++ b/twilight-model/src/gateway/payload/incoming/guild_create.rs @@ -4,11 +4,13 @@ use crate::{ }; use serde::{Deserialize, Serialize}; +// Developer note: Do not change order as we want unavailable to fail +// first. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(untagged)] pub enum GuildCreate { - Available(Guild), Unavailable(UnavailableGuild), + Available(Guild), } impl GuildCreate { @@ -25,7 +27,7 @@ impl GuildCreate { mod tests { use serde_test::Token; - use crate::{guild::UnavailableGuild, id::Id}; + use crate::{guild::UnavailableGuild, id::Id, util::mustbe::MustBeBool}; use super::GuildCreate; @@ -33,7 +35,7 @@ mod tests { fn unavailable_guild() { let expected = GuildCreate::Unavailable(UnavailableGuild { id: Id::new(1234), - unavailable: true, + unavailable: MustBeBool, }); // Note: serde(untagged) makes the enum transparent which is diff --git a/twilight-model/src/gateway/payload/incoming/ready.rs b/twilight-model/src/gateway/payload/incoming/ready.rs index 3808e9d1e71..197e925a334 100644 --- a/twilight-model/src/gateway/payload/incoming/ready.rs +++ b/twilight-model/src/gateway/payload/incoming/ready.rs @@ -25,6 +25,7 @@ mod tests { id::Id, oauth::{ApplicationFlags, PartialApplication}, user::CurrentUser, + util::mustbe::MustBeBool, }; use serde_test::Token; @@ -34,11 +35,11 @@ mod tests { let guilds = vec![ UnavailableGuild { id: Id::new(1), - unavailable: true, + unavailable: MustBeBool, }, UnavailableGuild { id: Id::new(2), - unavailable: true, + unavailable: MustBeBool, }, ]; diff --git a/twilight-model/src/guild/unavailable_guild.rs b/twilight-model/src/guild/unavailable_guild.rs index f4689e53e3c..4065e2abcee 100644 --- a/twilight-model/src/guild/unavailable_guild.rs +++ b/twilight-model/src/guild/unavailable_guild.rs @@ -1,23 +1,26 @@ -use crate::id::{marker::GuildMarker, Id}; +use crate::{ + id::{marker::GuildMarker, Id}, + util::mustbe::MustBeBool, +}; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] pub struct UnavailableGuild { pub id: Id, - pub unavailable: bool, + pub unavailable: MustBeBool, } #[cfg(test)] mod tests { use super::UnavailableGuild; - use crate::id::Id; + use crate::{id::Id, util::mustbe::MustBeBool}; use serde_test::Token; #[test] fn unavailable_guild() { let value = UnavailableGuild { id: Id::new(1), - unavailable: true, + unavailable: MustBeBool, }; serde_test::assert_tokens( diff --git a/twilight-model/src/util/mod.rs b/twilight-model/src/util/mod.rs index c30df90e120..6215efdc53c 100644 --- a/twilight-model/src/util/mod.rs +++ b/twilight-model/src/util/mod.rs @@ -3,6 +3,7 @@ pub mod datetime; pub mod hex_color; pub mod image_hash; +pub mod mustbe; pub use self::{datetime::Timestamp, hex_color::HexColor, image_hash::ImageHash}; diff --git a/twilight-model/src/util/mustbe.rs b/twilight-model/src/util/mustbe.rs new file mode 100644 index 00000000000..4380f6a2989 --- /dev/null +++ b/twilight-model/src/util/mustbe.rs @@ -0,0 +1,152 @@ +//! A struct that only deserializes from one specific boolean value. +//! +//! This module is heavily based upon +//! . + +use std::{ + fmt::{self, Debug}, + hash::Hash, +}; + +use serde::{ + de::{Error, Unexpected, Visitor}, + Deserialize, Serialize, +}; + +/// Struct that will only serialize from the bool specified as `T`. +#[derive(Clone, Copy, Default)] +pub struct MustBeBool; + +impl MustBeBool { + /// Get the expected boolean + pub const fn get(self) -> bool { + T + } +} + +impl PartialEq> for MustBeBool { + fn eq(&self, _: &MustBeBool) -> bool { + T.eq(&U) + } +} + +impl Eq for MustBeBool {} + +impl Debug for MustBeBool { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("MustBeBool").field(&T).finish() + } +} + +impl Hash for MustBeBool { + fn hash(&self, state: &mut H) { + T.hash(state) + } +} + +impl<'de, const T: bool> Deserialize<'de> for MustBeBool { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct MustBeBoolVisitor(bool); + + impl<'de> Visitor<'de> for MustBeBoolVisitor { + type Value = (); + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + write!(formatter, "boolean `{}`", self.0) + } + + fn visit_bool(self, v: bool) -> Result + where + E: Error, + { + if v == self.0 { + Ok(()) + } else { + Err(E::invalid_value(Unexpected::Bool(v), &self)) + } + } + } + + deserializer + .deserialize_any(MustBeBoolVisitor(T)) + .map(|()| MustBeBool) + } +} + +impl Serialize for MustBeBool { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_bool(T) + } +} + +#[cfg(test)] +mod tests { + use super::MustBeBool; + + use serde::{Deserialize, Serialize}; + + #[derive(Deserialize, Serialize)] + struct MTrue { + m: MustBeBool, + } + + #[derive(Deserialize, Serialize)] + struct MFalse { + m: MustBeBool, + } + + #[derive(Deserialize, Serialize)] + #[serde(untagged)] + enum TestEnum { + VariantTrue(MTrue), + VariantFalse(MFalse), + } + + #[test] + #[allow(unused)] + fn true_false_enum() { + let json_1 = r#"{ "m": false }"#; + let result_1 = serde_json::from_str::(json_1).unwrap(); + assert!(matches!(result_1, TestEnum::VariantFalse(_))); + + let json_2 = r#"{ "m": true }"#; + let result_2 = serde_json::from_str::(json_2).unwrap(); + assert!(matches!(result_2, TestEnum::VariantTrue(_))); + } + + #[test] + fn default_value() { + #[derive(Deserialize, Serialize)] + struct MFalse { + #[serde(default)] + m: MustBeBool, + } + + let json_1 = r#"{}"#; + serde_json::from_str::(json_1).unwrap(); + } + + #[test] + fn ser() { + let val = TestEnum::VariantTrue(MTrue { m: MustBeBool }); + let result = serde_json::to_string(&val).unwrap(); + assert_eq!(r#"{"m":true}"#, result); + + let val = TestEnum::VariantFalse(MFalse { m: MustBeBool }); + let result = serde_json::to_string(&val).unwrap(); + assert_eq!(r#"{"m":false}"#, result); + } + + #[test] + fn equality() { + assert_ne!(MustBeBool::, MustBeBool::); + assert_eq!(MustBeBool::, MustBeBool::); + assert_eq!(MustBeBool::, MustBeBool::); + } +} From 3f417fe160b2ab6ab9c3c345ee0fe5444e82d40a Mon Sep 17 00:00:00 2001 From: Valdemar Erk Date: Wed, 4 Sep 2024 20:32:12 +0200 Subject: [PATCH 2/7] fix test --- twilight-cache-inmemory/src/event/guild.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/twilight-cache-inmemory/src/event/guild.rs b/twilight-cache-inmemory/src/event/guild.rs index 8d8c9b15d44..e96004e9dbb 100644 --- a/twilight-cache-inmemory/src/event/guild.rs +++ b/twilight-cache-inmemory/src/event/guild.rs @@ -181,7 +181,10 @@ mod tests { VerificationLevel, }, id::Id, - util::datetime::{Timestamp, TimestampParseError}, + util::{ + datetime::{Timestamp, TimestampParseError}, + mustbe::MustBeBool, + }, }; #[allow(clippy::too_many_lines)] @@ -358,7 +361,7 @@ mod tests { cache.update(&GuildCreate::Unavailable( twilight_model::guild::UnavailableGuild { id: guild.id, - unavailable: true, + unavailable: MustBeBool, }, )); assert!(cache.unavailable_guilds.get(&guild.id).is_some()); @@ -370,7 +373,7 @@ mod tests { cache.update(&GuildCreate::Unavailable( twilight_model::guild::UnavailableGuild { id: guild.id, - unavailable: true, + unavailable: MustBeBool, }, )); assert!(cache.unavailable_guilds.get(&guild.id).is_some()); From df1008d48be4b978adda3293a7b6f14aecbebeff Mon Sep 17 00:00:00 2001 From: Valdemar Erk Date: Wed, 4 Sep 2024 20:44:56 +0200 Subject: [PATCH 3/7] make clippy happy --- twilight-model/src/util/mustbe.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/twilight-model/src/util/mustbe.rs b/twilight-model/src/util/mustbe.rs index 4380f6a2989..25f7b510abd 100644 --- a/twilight-model/src/util/mustbe.rs +++ b/twilight-model/src/util/mustbe.rs @@ -128,7 +128,7 @@ mod tests { m: MustBeBool, } - let json_1 = r#"{}"#; + let json_1 = r"{}"; serde_json::from_str::(json_1).unwrap(); } From 5192b810b67a7afb1ca1d47fcb080442de72ad32 Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Wed, 4 Sep 2024 21:50:59 +0200 Subject: [PATCH 4/7] Hide MustBeBool from public API Signed-off-by: Jens Reidel --- .../gateway/payload/incoming/guild_create.rs | 4 +-- .../src/gateway/payload/incoming/ready.rs | 5 ++-- twilight-model/src/guild/unavailable_guild.rs | 29 ++++++++++++++++--- twilight-model/src/util/mod.rs | 2 +- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/twilight-model/src/gateway/payload/incoming/guild_create.rs b/twilight-model/src/gateway/payload/incoming/guild_create.rs index 23fbdc44ff3..82391c736f7 100644 --- a/twilight-model/src/gateway/payload/incoming/guild_create.rs +++ b/twilight-model/src/gateway/payload/incoming/guild_create.rs @@ -27,7 +27,7 @@ impl GuildCreate { mod tests { use serde_test::Token; - use crate::{guild::UnavailableGuild, id::Id, util::mustbe::MustBeBool}; + use crate::{guild::UnavailableGuild, id::Id}; use super::GuildCreate; @@ -35,7 +35,7 @@ mod tests { fn unavailable_guild() { let expected = GuildCreate::Unavailable(UnavailableGuild { id: Id::new(1234), - unavailable: MustBeBool, + unavailable: true, }); // Note: serde(untagged) makes the enum transparent which is diff --git a/twilight-model/src/gateway/payload/incoming/ready.rs b/twilight-model/src/gateway/payload/incoming/ready.rs index 197e925a334..3808e9d1e71 100644 --- a/twilight-model/src/gateway/payload/incoming/ready.rs +++ b/twilight-model/src/gateway/payload/incoming/ready.rs @@ -25,7 +25,6 @@ mod tests { id::Id, oauth::{ApplicationFlags, PartialApplication}, user::CurrentUser, - util::mustbe::MustBeBool, }; use serde_test::Token; @@ -35,11 +34,11 @@ mod tests { let guilds = vec![ UnavailableGuild { id: Id::new(1), - unavailable: MustBeBool, + unavailable: true, }, UnavailableGuild { id: Id::new(2), - unavailable: MustBeBool, + unavailable: true, }, ]; diff --git a/twilight-model/src/guild/unavailable_guild.rs b/twilight-model/src/guild/unavailable_guild.rs index 4065e2abcee..1167c62af97 100644 --- a/twilight-model/src/guild/unavailable_guild.rs +++ b/twilight-model/src/guild/unavailable_guild.rs @@ -4,23 +4,44 @@ use crate::{ }; use serde::{Deserialize, Serialize}; -#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize)] pub struct UnavailableGuild { pub id: Id, - pub unavailable: MustBeBool, + pub unavailable: bool, +} + +impl<'de> Deserialize<'de> for UnavailableGuild { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + #[derive(Deserialize)] + #[serde(rename = "UnavailableGuild")] // Tests expect this struct name + struct UnavailableGuildIntermediate { + id: Id, + unavailable: MustBeBool, + } + + let intermediate = UnavailableGuildIntermediate::deserialize(deserializer)?; + + Ok(Self { + id: intermediate.id, + unavailable: intermediate.unavailable.get(), + }) + } } #[cfg(test)] mod tests { use super::UnavailableGuild; - use crate::{id::Id, util::mustbe::MustBeBool}; + use crate::id::Id; use serde_test::Token; #[test] fn unavailable_guild() { let value = UnavailableGuild { id: Id::new(1), - unavailable: MustBeBool, + unavailable: true, }; serde_test::assert_tokens( diff --git a/twilight-model/src/util/mod.rs b/twilight-model/src/util/mod.rs index 6215efdc53c..7a013cfb727 100644 --- a/twilight-model/src/util/mod.rs +++ b/twilight-model/src/util/mod.rs @@ -3,7 +3,7 @@ pub mod datetime; pub mod hex_color; pub mod image_hash; -pub mod mustbe; +pub(crate) mod mustbe; pub use self::{datetime::Timestamp, hex_color::HexColor, image_hash::ImageHash}; From 8328cc8719693b2b0d8c9d9efcdb6fcd9f7ebd2c Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Wed, 4 Sep 2024 21:54:21 +0200 Subject: [PATCH 5/7] Fix cache compilation Signed-off-by: Jens Reidel --- twilight-cache-inmemory/src/event/guild.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/twilight-cache-inmemory/src/event/guild.rs b/twilight-cache-inmemory/src/event/guild.rs index e96004e9dbb..8d8c9b15d44 100644 --- a/twilight-cache-inmemory/src/event/guild.rs +++ b/twilight-cache-inmemory/src/event/guild.rs @@ -181,10 +181,7 @@ mod tests { VerificationLevel, }, id::Id, - util::{ - datetime::{Timestamp, TimestampParseError}, - mustbe::MustBeBool, - }, + util::datetime::{Timestamp, TimestampParseError}, }; #[allow(clippy::too_many_lines)] @@ -361,7 +358,7 @@ mod tests { cache.update(&GuildCreate::Unavailable( twilight_model::guild::UnavailableGuild { id: guild.id, - unavailable: MustBeBool, + unavailable: true, }, )); assert!(cache.unavailable_guilds.get(&guild.id).is_some()); @@ -373,7 +370,7 @@ mod tests { cache.update(&GuildCreate::Unavailable( twilight_model::guild::UnavailableGuild { id: guild.id, - unavailable: MustBeBool, + unavailable: true, }, )); assert!(cache.unavailable_guilds.get(&guild.id).is_some()); From 7d3eb2af5fc2bacea723394837ce39820243bc5e Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Wed, 4 Sep 2024 22:00:46 +0200 Subject: [PATCH 6/7] Fix clippy warning Signed-off-by: Jens Reidel --- twilight-model/src/guild/unavailable_guild.rs | 3 ++- twilight-model/src/util/mustbe.rs | 7 ------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/twilight-model/src/guild/unavailable_guild.rs b/twilight-model/src/guild/unavailable_guild.rs index 1167c62af97..86a8db4f7c9 100644 --- a/twilight-model/src/guild/unavailable_guild.rs +++ b/twilight-model/src/guild/unavailable_guild.rs @@ -19,6 +19,7 @@ impl<'de> Deserialize<'de> for UnavailableGuild { #[serde(rename = "UnavailableGuild")] // Tests expect this struct name struct UnavailableGuildIntermediate { id: Id, + #[allow(unused)] // Only used in the derived impl unavailable: MustBeBool, } @@ -26,7 +27,7 @@ impl<'de> Deserialize<'de> for UnavailableGuild { Ok(Self { id: intermediate.id, - unavailable: intermediate.unavailable.get(), + unavailable: true, }) } } diff --git a/twilight-model/src/util/mustbe.rs b/twilight-model/src/util/mustbe.rs index 25f7b510abd..33ba2b4c404 100644 --- a/twilight-model/src/util/mustbe.rs +++ b/twilight-model/src/util/mustbe.rs @@ -17,13 +17,6 @@ use serde::{ #[derive(Clone, Copy, Default)] pub struct MustBeBool; -impl MustBeBool { - /// Get the expected boolean - pub const fn get(self) -> bool { - T - } -} - impl PartialEq> for MustBeBool { fn eq(&self, _: &MustBeBool) -> bool { T.eq(&U) From 1229f2b51935186d64ab98657e851cd200b9521e Mon Sep 17 00:00:00 2001 From: Jens Reidel Date: Wed, 4 Sep 2024 22:07:52 +0200 Subject: [PATCH 7/7] Remove unused impls in mustbe Signed-off-by: Jens Reidel --- twilight-model/src/util/mustbe.rs | 77 ++++--------------------------- 1 file changed, 8 insertions(+), 69 deletions(-) diff --git a/twilight-model/src/util/mustbe.rs b/twilight-model/src/util/mustbe.rs index 33ba2b4c404..f9b65632b26 100644 --- a/twilight-model/src/util/mustbe.rs +++ b/twilight-model/src/util/mustbe.rs @@ -3,40 +3,16 @@ //! This module is heavily based upon //! . -use std::{ - fmt::{self, Debug}, - hash::Hash, -}; +use std::fmt; use serde::{ de::{Error, Unexpected, Visitor}, - Deserialize, Serialize, + Deserialize, }; /// Struct that will only serialize from the bool specified as `T`. -#[derive(Clone, Copy, Default)] pub struct MustBeBool; -impl PartialEq> for MustBeBool { - fn eq(&self, _: &MustBeBool) -> bool { - T.eq(&U) - } -} - -impl Eq for MustBeBool {} - -impl Debug for MustBeBool { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("MustBeBool").field(&T).finish() - } -} - -impl Hash for MustBeBool { - fn hash(&self, state: &mut H) { - T.hash(state) - } -} - impl<'de, const T: bool> Deserialize<'de> for MustBeBool { fn deserialize(deserializer: D) -> Result where @@ -69,32 +45,25 @@ impl<'de, const T: bool> Deserialize<'de> for MustBeBool { } } -impl Serialize for MustBeBool { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - serializer.serialize_bool(T) - } -} - #[cfg(test)] mod tests { use super::MustBeBool; - use serde::{Deserialize, Serialize}; + use serde::Deserialize; - #[derive(Deserialize, Serialize)] + #[derive(Deserialize)] struct MTrue { + #[allow(unused)] m: MustBeBool, } - #[derive(Deserialize, Serialize)] + #[derive(Deserialize)] struct MFalse { + #[allow(unused)] m: MustBeBool, } - #[derive(Deserialize, Serialize)] + #[derive(Deserialize)] #[serde(untagged)] enum TestEnum { VariantTrue(MTrue), @@ -112,34 +81,4 @@ mod tests { let result_2 = serde_json::from_str::(json_2).unwrap(); assert!(matches!(result_2, TestEnum::VariantTrue(_))); } - - #[test] - fn default_value() { - #[derive(Deserialize, Serialize)] - struct MFalse { - #[serde(default)] - m: MustBeBool, - } - - let json_1 = r"{}"; - serde_json::from_str::(json_1).unwrap(); - } - - #[test] - fn ser() { - let val = TestEnum::VariantTrue(MTrue { m: MustBeBool }); - let result = serde_json::to_string(&val).unwrap(); - assert_eq!(r#"{"m":true}"#, result); - - let val = TestEnum::VariantFalse(MFalse { m: MustBeBool }); - let result = serde_json::to_string(&val).unwrap(); - assert_eq!(r#"{"m":false}"#, result); - } - - #[test] - fn equality() { - assert_ne!(MustBeBool::, MustBeBool::); - assert_eq!(MustBeBool::, MustBeBool::); - assert_eq!(MustBeBool::, MustBeBool::); - } }