Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Strings with FastStr #211

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Version numbers follow [Semantic Versioning](https://semver.org/).

## Unreleased

- All `String`s replaced with FastStr, thanks to `fast_str` crate author, @xxXyh1908.

- Breaking: Fixed a erroneous implementation of the IRCv3 tags: This crate now no longer differentiates
between empty and missing IRCv3 tag values (e.g. `@key` is equivalent to `@key=`). The type of the
`IRCTags` struct has changed to hold a `HashMap<String, String>` instead of a `HashMap<String, Option<String>>`.
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ bytes = { version = "1", optional = true }
chrono = { version = "0.4", default-features = false }
either = "1"
enum_dispatch = "0.3"
fast-str = { version = "1.0.0", features = ["stack", "serde"] }
futures-util = { version = "0.3", default-features = false, features = ["async-await", "sink", "std"] }
prometheus = { version = "0.13", default-features = false, optional = true }
reqwest = { version = "0.11", default-features = false, features = ["json"], optional = true }
Expand Down
20 changes: 11 additions & 9 deletions src/client/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::message::{IRCMessage, JoinMessage, PartMessage};
#[cfg(feature = "metrics-collection")]
use crate::metrics::MetricsBundle;
use crate::transport::Transport;
use fast_str::FastStr;
use std::collections::{HashSet, VecDeque};
use std::sync::{Arc, Weak};
use tokio::sync::{mpsc, oneshot};
Expand All @@ -27,17 +28,17 @@ pub(crate) enum ClientLoopCommand<T: Transport, L: LoginCredentials> {
return_sender: oneshot::Sender<Result<(), Error<T, L>>>,
},
Join {
channel_login: String,
channel_login: FastStr,
},
GetChannelStatus {
channel_login: String,
channel_login: FastStr,
return_sender: oneshot::Sender<(bool, bool)>,
},
Part {
channel_login: String,
channel_login: FastStr,
},
SetWantedChannels {
channels: HashSet<String>,
channels: HashSet<FastStr>,
},
Ping {
return_sender: oneshot::Sender<Result<(), Error<T, L>>>,
Expand Down Expand Up @@ -248,7 +249,7 @@ impl<T: Transport, L: LoginCredentials> ClientLoopWorker<T, L> {
///
/// The client will make best attempts to stay joined to this channel. I/O errors will be
/// compensated by retrying the join process. For this reason, this method returns no error.
fn join(&mut self, channel_login: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these kinds of changes, where the type in a struct or the argument type of a function changes, are breaking changes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, looks to be missed. I've planned to rewrite it into impl Into<FastStr>, to save backwards compatibility.

fn join(&mut self, channel_login: FastStr) {
let channel_already_confirmed_joined = self.connections.iter().any(|c| {
c.wanted_channels.contains(&channel_login) && c.server_channels.contains(&channel_login)
});
Expand Down Expand Up @@ -295,7 +296,7 @@ impl<T: Transport, L: LoginCredentials> ClientLoopWorker<T, L> {
self.update_metrics();
}

fn set_wanted_channels(&mut self, channels: HashSet<String>) {
fn set_wanted_channels(&mut self, channels: HashSet<FastStr>) {
// part channels as needed
self.connections
.iter()
Expand All @@ -312,7 +313,7 @@ impl<T: Transport, L: LoginCredentials> ClientLoopWorker<T, L> {
}
}

fn get_channel_status(&mut self, channel_login: String) -> (bool, bool) {
fn get_channel_status(&mut self, channel_login: FastStr) -> (bool, bool) {
let wanted = self
.connections
.iter()
Expand All @@ -324,7 +325,7 @@ impl<T: Transport, L: LoginCredentials> ClientLoopWorker<T, L> {
(wanted, joined_on_server)
}

fn part(&mut self, channel_login: String) {
fn part(&mut self, channel_login: FastStr) {
// skip the PART altogether if the last message we sent regarding that channel was a PART
// (or nothing at all, for that matter).
if self
Expand Down Expand Up @@ -419,7 +420,8 @@ impl<T: Transport, L: LoginCredentials> ClientLoopWorker<T, L> {
.iter_mut()
.find(|c| c.id == source_connection_id)
.unwrap();
c.server_channels.remove(channel_login);
let channel_login = FastStr::from_ref(channel_login);
c.server_channels.remove::<FastStr>(&channel_login);

// update metrics about channel numbers
self.update_metrics();
Expand Down
53 changes: 33 additions & 20 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use crate::transport::Transport;
use crate::validate::validate_login;
use crate::{irc, validate};
use fast_str::FastStr;
use std::collections::HashSet;
use std::sync::Arc;
use tokio::sync::{mpsc, oneshot};
Expand Down Expand Up @@ -121,7 +122,11 @@
///
/// If you want to just send a normal chat message, `say()` should be preferred since it
/// prevents commands like `/ban` from accidentally being executed.
pub async fn privmsg(&self, channel_login: String, message: String) -> Result<(), Error<T, L>> {
pub async fn privmsg(
&self,
channel_login: FastStr,
message: FastStr,
) -> Result<(), Error<T, L>> {
self.send_message(irc!["PRIVMSG", format!("#{}", channel_login), message])
.await
}
Expand All @@ -135,8 +140,12 @@
/// No particular filtering is performed on the message. If the message is too long for chat,
/// it will not be cut short or split into multiple messages (what happens is determined
/// by the behaviour of the Twitch IRC server).
pub async fn say(&self, channel_login: String, message: String) -> Result<(), Error<T, L>> {
self.privmsg(channel_login, format!(". {}", message)).await
pub async fn say(&self, channel_login: FastStr, message: FastStr) -> Result<(), Error<T, L>> {
self.privmsg(
channel_login,
FastStr::from_string(format!(". {}", message)),
)
.await
}

/// Say a `/me` chat message in the given Twitch channel. These messages are usually
Expand All @@ -148,9 +157,9 @@
/// # use twitch_irc::login::StaticLoginCredentials;
/// # let client: TwitchIRCClient<SecureTCPTransport, StaticLoginCredentials> = todo!();
/// client.say("sodapoppin".to_owned(), "Hey guys!".to_owned());
/// // Displayed as: A_Cool_New_Bot: Hey guys!

Check failure on line 160 in src/client/mod.rs

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest)

arguments to this method are incorrect
/// client.me("sodapoppin".to_owned(), "is now leaving to grab a drink.".to_owned());
/// // Displayed as: *A_Cool_New_Bot is now leaving to grab a drink.*

Check failure on line 162 in src/client/mod.rs

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest)

arguments to this method are incorrect
/// ```
///
/// This method automatically prevents commands from being executed. For example
Expand All @@ -160,9 +169,12 @@
/// No particular filtering is performed on the message. If the message is too long for chat,
/// it will not be cut short or split into multiple messages (what happens is determined
/// by the behaviour of the Twitch IRC server).
pub async fn me(&self, channel_login: String, message: String) -> Result<(), Error<T, L>> {
self.privmsg(channel_login, format!("/me {}", message))
.await
pub async fn me(&self, channel_login: FastStr, message: FastStr) -> Result<(), Error<T, L>> {
self.privmsg(
channel_login,
FastStr::from_string(format!("/me {}", message)),
)
.await
}

/// Reply to a given message. The sent message is tagged to be in reply of the
Expand All @@ -181,7 +193,7 @@
/// be one of the following:
///
/// * a [`&PrivmsgMessage`](crate::message::PrivmsgMessage)
/// * a tuple `(&str, &str)` or `(String, String)`, where the first member is the login name
/// * a tuple `(&str, &str)` or `(FastStr, FastStr)`, where the first member is the login name
/// of the channel the message was sent to, and the second member is the ID of the message
/// to reply to.
///
Expand All @@ -192,7 +204,7 @@
pub async fn say_in_reply_to(
&self,
reply_to: &impl ReplyToMessage,
message: String,
message: FastStr,
) -> Result<(), Error<T, L>> {
self.say_or_me_in_reply_to(reply_to, message, false).await
}
Expand All @@ -216,7 +228,7 @@
/// be one of the following:
///
/// * a [`&PrivmsgMessage`](crate::message::PrivmsgMessage)
/// * a tuple `(&str, &str)` or `(String, String)`, where the first member is the login name
/// * a tuple `(&str, &str)` or `(FastStr, FastStr)`, where the first member is the login name
/// of the channel the message was sent to, and the second member is the ID of the message
/// to reply to.
///
Expand All @@ -227,15 +239,15 @@
pub async fn me_in_reply_to(
&self,
reply_to: &impl ReplyToMessage,
message: String,
message: FastStr,
) -> Result<(), Error<T, L>> {
self.say_or_me_in_reply_to(reply_to, message, true).await
}

async fn say_or_me_in_reply_to(
&self,
reply_to: &impl ReplyToMessage,
message: String,
message: FastStr,
me: bool,
) -> Result<(), Error<T, L>> {
let mut tags = IRCTags::new();
Expand All @@ -247,10 +259,10 @@
let irc_message = IRCMessage::new(
tags,
None,
"PRIVMSG".to_owned(),
FastStr::from_static("PRIVMSG"),
vec![
format!("#{}", reply_to.channel_login()),
format!("{} {}", if me { "/me" } else { "." }, message),
FastStr::from_string(format!("#{}", reply_to.channel_login())),
FastStr::from_string(format!("{} {}", if me { "/me" } else { "." }, message)),
], // The prefixed "." prevents commands from being executed if not in /me-mode
);
self.send_message(irc_message).await
Expand Down Expand Up @@ -291,7 +303,8 @@
///
/// Returns a [validate::Error] if the passed `channel_login` is of
/// [invalid format](crate::validate::validate_login). Returns `Ok(())` otherwise.
pub fn join(&self, channel_login: String) -> Result<(), validate::Error> {
pub fn join(&self, channel_login: impl Into<FastStr>) -> Result<(), validate::Error> {
let channel_login = channel_login.into();
validate_login(&channel_login)?;

self.client_loop_tx
Expand All @@ -309,7 +322,7 @@
///
/// Returns a [validate::Error] if the passed `channel_login` is of
/// [invalid format](crate::validate::validate_login). Returns `Ok(())` otherwise.
pub fn set_wanted_channels(&self, channels: HashSet<String>) -> Result<(), validate::Error> {
pub fn set_wanted_channels(&self, channels: HashSet<FastStr>) -> Result<(), validate::Error> {
for channel_login in channels.iter() {
validate_login(channel_login)?;
}
Expand Down Expand Up @@ -344,8 +357,8 @@
///
/// `(false, false)` is returned for a channel that has not been joined previously at all
/// or where a previous `PART` command has completed.
pub async fn get_channel_status(&self, channel_login: String) -> (bool, bool) {
// channel_login format sanity check not really needed here, the code will deal with arbitrary strings just fine
pub async fn get_channel_status(&self, channel_login: FastStr) -> (bool, bool) {
// channel_login format sanity check not really needed here, the code will deal with arbitrary FastStrs just fine

let (return_tx, return_rx) = oneshot::channel();
self.client_loop_tx
Expand All @@ -362,8 +375,8 @@
///
/// This has the same semantics as `join()`. Similarly, a `part()` call will have no effect
/// if the channel is not currently joined.
pub fn part(&self, channel_login: String) {
// channel_login format sanity check not really needed here, the code will deal with arbitrary strings just fine
pub fn part(&self, channel_login: FastStr) {
// channel_login format sanity check not really needed here, the code will deal with arbitrary FastStrs just fine

self.client_loop_tx
.send(ClientLoopCommand::Part { channel_login })
Expand Down
5 changes: 3 additions & 2 deletions src/client/pool_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::config::ClientConfig;
use crate::connection::Connection;
use crate::login::LoginCredentials;
use crate::transport::Transport;
use fast_str::FastStr;
use std::collections::{HashSet, VecDeque};
use std::sync::Arc;
use std::time::Instant;
Expand Down Expand Up @@ -30,9 +31,9 @@ pub(crate) struct PoolConnection<T: Transport, L: LoginCredentials> {
/// The connection handle that this is wrapping
pub connection: Arc<Connection<T, L>>,
/// see the documentation on `TwitchIRCClient` for what `wanted_channels` and `server_channels` mean
pub wanted_channels: HashSet<String>,
pub wanted_channels: HashSet<FastStr>,
/// see the documentation on `TwitchIRCClient` for what `wanted_channels` and `server_channels` mean
pub server_channels: HashSet<String>,
pub server_channels: HashSet<FastStr>,
/// this has a list of times when messages were sent out on this pool connection,
/// at the front there will be the oldest, and at the back the newest entries
pub message_send_times: VecDeque<Instant>,
Expand Down
1 change: 0 additions & 1 deletion src/connection/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use either::Either;
use enum_dispatch::enum_dispatch;
use futures_util::{SinkExt, StreamExt};
use std::collections::VecDeque;
use std::convert::TryFrom;
use std::sync::{Arc, Weak};
use tokio::sync::{mpsc, oneshot};
use tokio::time::{interval_at, Duration, Instant};
Expand Down
Loading
Loading