From 5189b69e7eb4d1dad9e525692f5998eb9f5bb852 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Sun, 29 Sep 2019 00:54:07 +0200 Subject: [PATCH] Simplify Config structure This simplifies some of the `Config` structure, in particular this means: Parameters which are meaningfully equivalent longer stored in an `Option`, an example of this is `channels`. If you don't want to join any channels you simply leave it as empty instead. In effect, `None` is behaviorally equivalent to `vec![]`. We don't allocate when accessing certain configuration options. For example, when accessing `channels` we used to allocate a vector to handle the "empty case", we simply return the slice corresponding to the list of channels instead. We skip serializing empty or optional configuration fields. From a deserialization perspective this is already something that was mostly supported through use of `Option` and `#[serde(default)]`. --- examples/build-bot.rs | 2 +- examples/multiserver.rs | 4 +- examples/repeater.rs | 6 +- examples/simple.rs | 4 +- examples/simple_ssl.rs | 4 +- examples/tooter.rs | 2 +- examples/tweeter.rs | 2 +- src/client/data/config.rs | 142 ++++++++++++++++++++------------------ src/client/mod.rs | 48 +++++++------ 9 files changed, 116 insertions(+), 98 deletions(-) diff --git a/examples/build-bot.rs b/examples/build-bot.rs index 7d85030..4972fa6 100644 --- a/examples/build-bot.rs +++ b/examples/build-bot.rs @@ -15,7 +15,7 @@ async fn main() -> irc::error::Result<()> { let config = Config { nickname: Some("irc-crate-ci".to_owned()), server: Some("irc.pdgn.co".to_owned()), - use_ssl: Some(true), + use_ssl: true, ..Default::default() }; diff --git a/examples/multiserver.rs b/examples/multiserver.rs index ecf69e2..02efb39 100644 --- a/examples/multiserver.rs +++ b/examples/multiserver.rs @@ -10,14 +10,14 @@ async fn main() -> irc::error::Result<()> { let cfg1 = Config { nickname: Some("pickles".to_owned()), server: Some("irc.mozilla.org".to_owned()), - channels: Some(vec!["#rust-spam".to_owned()]), + channels: vec!["#rust-spam".to_owned()], ..Default::default() }; let cfg2 = Config { nickname: Some("bananas".to_owned()), server: Some("irc.mozilla.org".to_owned()), - channels: Some(vec!["#rust-spam".to_owned()]), + channels: vec!["#rust-spam".to_owned()], ..Default::default() }; diff --git a/examples/repeater.rs b/examples/repeater.rs index 009616a..3342c7d 100644 --- a/examples/repeater.rs +++ b/examples/repeater.rs @@ -5,10 +5,10 @@ use irc::client::prelude::*; async fn main() -> irc::error::Result<()> { let config = Config { nickname: Some("repeater".to_owned()), - alt_nicks: Some(vec!["blaster".to_owned(), "smg".to_owned()]), + alt_nicks: vec!["blaster".to_owned(), "smg".to_owned()], server: Some("irc.mozilla.org".to_owned()), - use_ssl: Some(true), - channels: Some(vec!["#rust-spam".to_owned()]), + use_ssl: true, + channels: vec!["#rust-spam".to_owned()], burst_window_length: Some(4), max_messages_in_burst: Some(4), ..Default::default() diff --git a/examples/simple.rs b/examples/simple.rs index d8e3ae4..d0e1e9f 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -5,9 +5,9 @@ use irc::client::prelude::*; async fn main() -> irc::error::Result<()> { let config = Config { nickname: Some("pickles".to_owned()), - alt_nicks: Some(vec!["bananas".to_owned(), "apples".to_owned()]), + alt_nicks: vec!["bananas".to_owned(), "apples".to_owned()], server: Some("irc.mozilla.org".to_owned()), - channels: Some(vec!["#rust-spam".to_owned()]), + channels: vec!["#rust-spam".to_owned()], ..Default::default() }; diff --git a/examples/simple_ssl.rs b/examples/simple_ssl.rs index f6d6d2e..0b9133b 100644 --- a/examples/simple_ssl.rs +++ b/examples/simple_ssl.rs @@ -6,8 +6,8 @@ async fn main() -> irc::error::Result<()> { let config = Config { nickname: Some("pickles".to_owned()), server: Some("irc.mozilla.org".to_owned()), - channels: Some(vec!["#rust-spam".to_owned()]), - use_ssl: Some(true), + channels: vec!["#rust-spam".to_owned()], + use_ssl: true, ..Default::default() }; diff --git a/examples/tooter.rs b/examples/tooter.rs index ab0736c..921257b 100644 --- a/examples/tooter.rs +++ b/examples/tooter.rs @@ -8,7 +8,7 @@ async fn main() -> irc::error::Result<()> { let config = Config { nickname: Some("mastodon".to_owned()), server: Some("irc.mozilla.org".to_owned()), - channels: Some(vec!["#rust-spam".to_owned()]), + channels: vec!["#rust-spam".to_owned()], ..Default::default() }; diff --git a/examples/tweeter.rs b/examples/tweeter.rs index f6649d0..6c6ad4e 100644 --- a/examples/tweeter.rs +++ b/examples/tweeter.rs @@ -8,7 +8,7 @@ async fn main() -> irc::error::Result<()> { let config = Config { nickname: Some("pickles".to_owned()), server: Some("irc.mozilla.org".to_owned()), - channels: Some(vec!["#rust-spam".to_owned()]), + channels: vec!["#rust-spam".to_owned()], ..Default::default() }; diff --git a/src/client/data/config.rs b/src/client/data/config.rs index ba01b1c..f2d5753 100644 --- a/src/client/data/config.rs +++ b/src/client/data/config.rs @@ -68,77 +68,115 @@ use crate::error::{ConfigError, Result}; #[derive(Clone, Deserialize, Serialize, Default, PartialEq, Debug)] pub struct Config { /// A list of the owners of the client by nickname (for bots). - pub owners: Option>, + #[serde(skip_serializing_if = "Vec::is_empty")] + #[serde(default)] + pub owners: Vec, /// The client's nickname. + #[serde(skip_serializing_if = "Option::is_none")] pub nickname: Option, /// The client's NICKSERV password. + #[serde(skip_serializing_if = "Option::is_none")] pub nick_password: Option, /// Alternative nicknames for the client, if the default is taken. - pub alt_nicks: Option>, + #[serde(skip_serializing_if = "Vec::is_empty")] + #[serde(default)] + pub alt_nicks: Vec, /// The client's username. + #[serde(skip_serializing_if = "Option::is_none")] pub username: Option, /// The client's real name. + #[serde(skip_serializing_if = "Option::is_none")] pub realname: Option, /// The server to connect to. + #[serde(skip_serializing_if = "Option::is_none")] pub server: Option, /// The port to connect on. + #[serde(skip_serializing_if = "Option::is_none")] pub port: Option, /// The password to connect to the server. + #[serde(skip_serializing_if = "Option::is_none")] pub password: Option, /// Whether or not to use SSL. /// Clients will automatically panic if this is enabled without SSL support. - pub use_ssl: Option, + #[serde(skip_serializing_if = "is_false")] + #[serde(default)] + pub use_ssl: bool, /// The path to the SSL certificate for this server in DER format. + #[serde(skip_serializing_if = "Option::is_none")] pub cert_path: Option, /// The path to a SSL certificate to use for CertFP client authentication in DER format. + #[serde(skip_serializing_if = "Option::is_none")] pub client_cert_path: Option, /// The password for the certificate to use in CertFP authentication. + #[serde(skip_serializing_if = "Option::is_none")] pub client_cert_pass: Option, /// The encoding type used for this connection. /// This is typically UTF-8, but could be something else. + #[serde(skip_serializing_if = "Option::is_none")] pub encoding: Option, /// A list of channels to join on connection. - pub channels: Option>, + #[serde(skip_serializing_if = "Vec::is_empty")] + #[serde(default)] + pub channels: Vec, /// User modes to set on connect. Example: "+RB -x" + #[serde(skip_serializing_if = "Option::is_none")] pub umodes: Option, /// The text that'll be sent in response to CTCP USERINFO requests. + #[serde(skip_serializing_if = "Option::is_none")] pub user_info: Option, /// The text that'll be sent in response to CTCP VERSION requests. + #[serde(skip_serializing_if = "Option::is_none")] pub version: Option, /// The text that'll be sent in response to CTCP SOURCE requests. + #[serde(skip_serializing_if = "Option::is_none")] pub source: Option, /// The amount of inactivity in seconds before the client will ping the server. + #[serde(skip_serializing_if = "Option::is_none")] pub ping_time: Option, /// The amount of time in seconds for a client to reconnect due to no ping response. + #[serde(skip_serializing_if = "Option::is_none")] pub ping_timeout: Option, /// The length in seconds of a rolling window for message throttling. If more than /// `max_messages_in_burst` messages are sent within `burst_window_length` seconds, additional /// messages will be delayed automatically as appropriate. In particular, in the past /// `burst_window_length` seconds, there will never be more than `max_messages_in_burst` messages /// sent. + #[serde(skip_serializing_if = "Option::is_none")] pub burst_window_length: Option, /// The maximum number of messages that can be sent in a burst window before they'll be delayed. /// Messages are automatically delayed as appropriate. + #[serde(skip_serializing_if = "Option::is_none")] pub max_messages_in_burst: Option, /// Whether the client should use NickServ GHOST to reclaim its primary nickname if it is in /// use. This has no effect if `nick_password` is not set. - pub should_ghost: Option, + #[serde(skip_serializing_if = "is_false")] + #[serde(default)] + pub should_ghost: bool, /// The command(s) that should be sent to NickServ to recover a nickname. The nickname and /// password will be appended in that order after the command. /// E.g. `["RECOVER", "RELEASE"]` means `RECOVER nick pass` and `RELEASE nick pass` will be sent /// in that order. + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub ghost_sequence: Option>, /// Whether or not to use a fake connection for testing purposes. You probably will never want /// to enable this, but it is used in unit testing for the `irc` crate. - pub use_mock_connection: Option, + #[serde(skip_serializing_if = "is_false")] + #[serde(default)] + pub use_mock_connection: bool, /// The initial value used by the fake connection for testing. You probably will never need to /// set this, but it is used in unit testing for the `irc` crate. + #[serde(skip_serializing_if = "Option::is_none")] pub mock_initial_value: Option, /// A mapping of channel names to keys for join-on-connect. - pub channel_keys: Option>, + #[serde(skip_serializing_if = "HashMap::is_empty")] + #[serde(default)] + pub channel_keys: HashMap, /// A map of additional options to be stored in config. - pub options: Option>, + #[serde(skip_serializing_if = "HashMap::is_empty")] + #[serde(default)] + pub options: HashMap, /// The path that this configuration was loaded from. /// @@ -148,6 +186,10 @@ pub struct Config { pub path: Option, } +fn is_false(v: &bool) -> bool { + !v +} + impl Config { fn with_path>(mut self, path: P) -> Config { self.path = Some(path.as_ref().to_owned()); @@ -316,17 +358,14 @@ impl Config { /// Determines whether or not the nickname provided is the owner of the bot. pub fn is_owner(&self, nickname: &str) -> bool { - self.owners - .as_ref() - .map(|o| o.contains(&nickname.to_owned())) - .unwrap() + self.owners.iter().find(|n| *n == nickname).is_some() } /// Gets the nickname specified in the configuration. pub fn nickname(&self) -> Result<&str> { self.nickname .as_ref() - .map(|s| &s[..]) + .map(String::as_str) .ok_or_else(|| InvalidConfig { path: self.path(), cause: ConfigError::NicknameNotSpecified, @@ -336,15 +375,13 @@ impl Config { /// Gets the bot's nickserv password specified in the configuration. /// This defaults to an empty string when not specified. pub fn nick_password(&self) -> &str { - self.nick_password.as_ref().map_or("", |s| &s[..]) + self.nick_password.as_ref().map_or("", String::as_str) } /// Gets the alternate nicknames specified in the configuration. /// This defaults to an empty vector when not specified. - pub fn alternate_nicknames(&self) -> Vec<&str> { - self.alt_nicks - .as_ref() - .map_or(vec![], |v| v.iter().map(|s| &s[..]).collect()) + pub fn alternate_nicknames(&self) -> &[String] { + &self.alt_nicks } /// Gets the username specified in the configuration. @@ -367,7 +404,7 @@ impl Config { pub fn server(&self) -> Result<&str> { self.server .as_ref() - .map(|s| &s[..]) + .map(String::as_str) .ok_or_else(|| InvalidConfig { path: self.path(), cause: ConfigError::ServerNotSpecified, @@ -395,28 +432,28 @@ impl Config { /// Gets the server password specified in the configuration. /// This defaults to a blank string when not specified. pub fn password(&self) -> &str { - self.password.as_ref().map_or("", |s| &s) + self.password.as_ref().map_or("", String::as_str) } /// Gets whether or not to use SSL with this connection. /// This defaults to false when not specified. pub fn use_ssl(&self) -> bool { - self.use_ssl.as_ref().cloned().unwrap_or(false) + self.use_ssl } /// Gets the path to the SSL certificate in DER format if specified. pub fn cert_path(&self) -> Option<&str> { - self.cert_path.as_ref().map(|s| &s[..]) + self.cert_path.as_ref().map(String::as_str) } /// Gets the path to the client authentication certificate in DER format if specified. pub fn client_cert_path(&self) -> Option<&str> { - self.client_cert_path.as_ref().map(|s| &s[..]) + self.client_cert_path.as_ref().map(String::as_str) } /// Gets the password to the client authentication certificate. pub fn client_cert_pass(&self) -> &str { - self.client_cert_pass.as_ref().map_or("", |s| &s[..]) + self.client_cert_pass.as_ref().map_or("", String::as_str) } /// Gets the encoding to use for this connection. This requires the encode feature to work. @@ -427,29 +464,25 @@ impl Config { /// Gets the channels to join upon connection. /// This defaults to an empty vector if it's not specified. - pub fn channels(&self) -> Vec<&str> { - self.channels - .as_ref() - .map_or(vec![], |v| v.iter().map(|s| &s[..]).collect()) + pub fn channels(&self) -> &[String] { + &self.channels } /// Gets the key for the specified channel if it exists in the configuration. pub fn channel_key(&self, chan: &str) -> Option<&str> { - self.channel_keys - .as_ref() - .and_then(|m| m.get(&chan.to_owned()).map(|s| &s[..])) + self.channel_keys.get(chan).map(String::as_str) } /// Gets the user modes to set on connect specified in the configuration. /// This defaults to an empty string when not specified. pub fn umodes(&self) -> &str { - self.umodes.as_ref().map_or("", |s| &s[..]) + self.umodes.as_ref().map_or("", String::as_str) } /// Gets the string to be sent in response to CTCP USERINFO requests. /// This defaults to an empty string when not specified. pub fn user_info(&self) -> &str { - self.user_info.as_ref().map_or("", |s| &s[..]) + self.user_info.as_ref().map_or("", String::as_str) } /// Gets the string to be sent in response to CTCP VERSION requests. @@ -464,7 +497,7 @@ impl Config { pub fn source(&self) -> &str { self.source .as_ref() - .map_or("https://github.com/aatxe/irc", |s| &s[..]) + .map_or("https://github.com/aatxe/irc", String::as_str) } /// Gets the amount of time in seconds for the interval at which the client pings the server. @@ -500,28 +533,24 @@ impl Config { /// Gets whether or not to attempt nickname reclamation using NickServ GHOST. /// This defaults to false when not specified. pub fn should_ghost(&self) -> bool { - self.should_ghost.as_ref().cloned().unwrap_or(false) + self.should_ghost } /// Gets the NickServ command sequence to recover a nickname. /// This defaults to `["GHOST"]` when not specified. - pub fn ghost_sequence(&self) -> Vec<&str> { - self.ghost_sequence - .as_ref() - .map_or(vec!["GHOST"], |v| v.iter().map(|s| &s[..]).collect()) + pub fn ghost_sequence(&self) -> Option<&[String]> { + self.ghost_sequence.as_ref().map(Vec::as_slice) } /// Looks up the specified string in the options map. pub fn get_option(&self, option: &str) -> Option<&str> { - self.options - .as_ref() - .and_then(|o| o.get(&option.to_owned()).map(|s| &s[..])) + self.options.get(option).map(String::as_str) } /// Gets whether or not to use a mock connection for testing. /// This defaults to false when not specified. pub fn use_mock_connection(&self) -> bool { - self.use_mock_connection.as_ref().cloned().unwrap_or(false) + self.use_mock_connection } /// Gets the initial value for the mock connection. @@ -540,35 +569,16 @@ mod test { #[allow(unused)] fn test_config() -> Config { Config { - owners: Some(vec![format!("test")]), + owners: vec![format!("test")], nickname: Some(format!("test")), - nick_password: None, - alt_nicks: None, username: Some(format!("test")), realname: Some(format!("test")), password: Some(String::new()), umodes: Some(format!("+BR")), server: Some(format!("irc.test.net")), port: Some(6667), - use_ssl: Some(false), - cert_path: None, - client_cert_path: None, - client_cert_pass: None, encoding: Some(format!("UTF-8")), - channels: Some(vec![format!("#test"), format!("#test2")]), - channel_keys: None, - user_info: None, - version: None, - source: None, - ping_time: None, - ping_timeout: None, - burst_window_length: None, - max_messages_in_burst: None, - should_ghost: None, - ghost_sequence: None, - options: Some(HashMap::new()), - use_mock_connection: None, - mock_initial_value: None, + channels: vec![format!("#test"), format!("#test2")], ..Default::default() } @@ -577,7 +587,7 @@ mod test { #[test] fn is_owner() { let cfg = Config { - owners: Some(vec![format!("test"), format!("test2")]), + owners: vec![format!("test"), format!("test2")], ..Default::default() }; assert!(cfg.is_owner("test")); @@ -591,7 +601,7 @@ mod test { options: { let mut map = HashMap::new(); map.insert(format!("testing"), format!("test")); - Some(map) + map }, ..Default::default() }; diff --git a/src/client/mod.rs b/src/client/mod.rs index f945550..89d7480 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -488,6 +488,8 @@ struct ClientState { chanlists: RwLock>>, /// A thread-safe index to track the current alternative nickname being used. alt_nick_index: RwLock, + /// Default ghost sequence to send if one is required but none is configured. + default_ghost_sequence: Vec, } impl ClientState { @@ -497,6 +499,7 @@ impl ClientState { config: config, chanlists: RwLock::new(HashMap::new()), alt_nick_index: RwLock::new(0), + default_ghost_sequence: vec![String::from("GHOST")], } } @@ -520,7 +523,7 @@ impl ClientState { .config() .nickname() .expect("current_nickname should not be callable if nickname is not defined."), - i => alt_nicks[i - 1], + i => alt_nicks[i - 1].as_str(), } } @@ -576,7 +579,7 @@ impl ClientState { self.send_umodes()?; let config_chans = self.config().channels(); - for chan in &config_chans { + for chan in config_chans { match self.config().channel_key(chan) { Some(key) => self.send_join_with_keys::<&str, &str>(chan, key)?, None => self.send_join(chan)?, @@ -585,7 +588,7 @@ impl ClientState { let joined_chans = self.chanlists.read(); for chan in joined_chans .keys() - .filter(|x| !config_chans.contains(&x.as_str())) + .filter(|x| config_chans.iter().find(|c| c == x).is_none()) { self.send_join(chan)? } @@ -614,10 +617,15 @@ impl ClientState { let mut index = self.alt_nick_index.write(); if self.config().should_ghost() && *index != 0 { - for seq in &self.config().ghost_sequence() { + let seq = match self.config().ghost_sequence() { + Some(seq) => seq, + None => &*self.default_ghost_sequence, + }; + + for s in seq { self.send(NICKSERV(format!( "{} {} {}", - seq, + s, self.config().nickname()?, self.config().nick_password() )))?; @@ -1090,13 +1098,13 @@ mod test { pub fn test_config() -> Config { Config { - owners: Some(vec![format!("test")]), + owners: vec![format!("test")], nickname: Some(format!("test")), - alt_nicks: Some(vec![format!("test2")]), + alt_nicks: vec![format!("test2")], server: Some(format!("irc.test.net")), - channels: Some(vec![format!("#test"), format!("#test2")]), + channels: vec![format!("#test"), format!("#test2")], user_info: Some(format!("Testing.")), - use_mock_connection: Some(true), + use_mock_connection: true, ..Default::default() } } @@ -1157,7 +1165,7 @@ mod test { let mut client = Client::from_config(Config { mock_initial_value: Some(value.to_owned()), nick_password: Some(format!("password")), - channels: Some(vec![format!("#test"), format!("#test2")]), + channels: vec![format!("#test"), format!("#test2")], ..test_config() }) .await?; @@ -1176,11 +1184,11 @@ mod test { let mut client = Client::from_config(Config { mock_initial_value: Some(value.to_owned()), nickname: Some(format!("test")), - channels: Some(vec![format!("#test"), format!("#test2")]), + channels: vec![format!("#test"), format!("#test2")], channel_keys: { let mut map = HashMap::new(); map.insert(format!("#test2"), format!("password")); - Some(map) + map }, ..test_config() }) @@ -1200,10 +1208,10 @@ mod test { let mut client = Client::from_config(Config { mock_initial_value: Some(value.to_owned()), nickname: Some(format!("test")), - alt_nicks: Some(vec![format!("test2")]), + alt_nicks: vec![format!("test2")], nick_password: Some(format!("password")), - channels: Some(vec![format!("#test"), format!("#test2")]), - should_ghost: Some(true), + channels: vec![format!("#test"), format!("#test2")], + should_ghost: true, ..test_config() }) .await?; @@ -1223,10 +1231,10 @@ mod test { let mut client = Client::from_config(Config { mock_initial_value: Some(value.to_owned()), nickname: Some(format!("test")), - alt_nicks: Some(vec![format!("test2")]), + alt_nicks: vec![format!("test2")], nick_password: Some(format!("password")), - channels: Some(vec![format!("#test"), format!("#test2")]), - should_ghost: Some(true), + channels: vec![format!("#test"), format!("#test2")], + should_ghost: true, ghost_sequence: Some(vec![format!("RECOVER"), format!("RELEASE")]), ..test_config() }) @@ -1248,7 +1256,7 @@ mod test { mock_initial_value: Some(value.to_owned()), nickname: Some(format!("test")), umodes: Some(format!("+B")), - channels: Some(vec![format!("#test"), format!("#test2")]), + channels: vec![format!("#test"), format!("#test2")], ..test_config() }) .await?; @@ -1496,7 +1504,7 @@ mod test { let mut client = Client::from_config(Config { mock_initial_value: Some(value.to_owned()), nickname: Some(format!("test")), - channels: Some(vec![format!("#test"), format!("#test2")]), + channels: vec![format!("#test"), format!("#test2")], ..test_config() }) .await?;