storage: make constructors more idiomatic

src/handlers/event/mod.rs

@@ -1,7 +1,7 @@
 use crate::{api, Data};
 
 use eyre::{Report, Result};
-use log::{debug, info};
+use log::{debug, info, trace};
 use poise::serenity_prelude::{ActivityData, Context, FullEvent, OnlineStatus};
 use poise::FrameworkContext;
 
@@ -15,7 +15,7 @@ mod support_onboard;
 pub async fn handle(
 	ctx: &Context,
 	event: &FullEvent,
-	_framework: FrameworkContext<'_, Data, Report>,
+	_: FrameworkContext<'_, Data, Report>,
 	data: &Data,
 ) -> Result<()> {
 	match event {
@@ -33,7 +33,7 @@ pub async fn handle(
 			// ignore new messages from bots
 			// NOTE: the webhook_id check allows us to still respond to PK users
 			if new_message.author.bot && new_message.webhook_id.is_none() {
-				debug!("Ignoring message {} from bot", new_message.id);
+				trace!("Ignoring message {} from bot", new_message.id);
 				return Ok(());
 			}
 

src/handlers/event/pluralkit.rs

@@ -2,7 +2,7 @@ use crate::{api, Data};
 use std::time::Duration;
 
 use eyre::Result;
-use log::debug;
+use log::{debug, trace};
 use poise::serenity_prelude::{Context, Message};
 use tokio::time::sleep;
 
@@ -20,14 +20,14 @@ pub async fn is_message_proxied(message: &Message) -> Result<bool> {
 	Ok(proxied)
 }
 
-pub async fn handle(_ctx: &Context, msg: &Message, data: &Data) -> Result<()> {
+pub async fn handle(_: &Context, msg: &Message, data: &Data) -> Result<()> {
 	if msg.webhook_id.is_some() {
 		debug!(
 			"Message {} has a webhook ID. Checking if it was sent through PluralKit",
 			msg.id
 		);
 
-		debug!(
+		trace!(
 			"Waiting on PluralKit API for {} seconds",
 			PK_DELAY_SEC.as_secs()
 		);

src/main.rs

@@ -8,6 +8,7 @@ use std::time::Duration;
 use eyre::{eyre, Context as _, Report, Result};
 use log::{info, warn};
 
+use octocrab::Octocrab;
 use poise::{
 	serenity_prelude as serenity, EditTracker, Framework, FrameworkOptions, PrefixFrameworkOptions,
 };
@@ -43,11 +44,7 @@ pub struct Data {
 }
 
 impl Data {
-	pub fn new() -> Result<Self> {
-		let config = Config::new_from_env();
-		let storage = Storage::new(&config.redis_url)?;
-		let octocrab = octocrab::instance();
-
+	pub fn new(config: Config, storage: Storage, octocrab: Arc<Octocrab>) -> Result<Self> {
 		Ok(Self {
 			config,
 			storage,
@@ -58,13 +55,16 @@ impl Data {
 
 async fn setup(
 	ctx: &serenity::Context,
-	_ready: &serenity::Ready,
+	_: &serenity::Ready,
 	framework: &Framework<Data, Report>,
 ) -> Result<Data> {
-	let data = Data::new()?;
+	let config = Config::new_from_env();
+	let storage = Storage::from_url(&config.redis_url)?;
+	let octocrab = octocrab::instance();
+	let data = Data::new(config, storage, octocrab)?;
 
 	// test redis connection
-	let mut client = data.storage.client.clone();
+	let mut client = data.storage.client().clone();
 
 	if !client.check_connection() {
 		return Err(eyre!(

src/storage/mod.rs

@@ -1,110 +1,69 @@
 use std::fmt::Debug;
 
 use eyre::Result;
-use log::{debug, info};
+use log::debug;
 use poise::serenity_prelude::UserId;
-use redis::{AsyncCommands as _, Client, FromRedisValue, ToRedisArgs};
+use redis::{AsyncCommands, Client};
 
 const PK_KEY: &str = "pluralkit-v1";
 const LAUNCHER_VERSION_KEY: &str = "launcher-version-v1";
 
 #[derive(Clone, Debug)]
 pub struct Storage {
-	pub client: Client,
+	client: Client,
 }
 
 impl Storage {
-	pub fn new(redis_url: &str) -> Result<Self> {
+	pub fn new(client: Client) -> Self {
+		Self { client }
+	}
+
+	pub fn from_url(redis_url: &str) -> Result<Self> {
 		let client = Client::open(redis_url)?;
 
-		Ok(Self { client })
+		Ok(Self::new(client))
 	}
 
-	/*
-	  these are mainly light abstractions to avoid the `let mut con`
-	  boilerplate, as well as not require the caller to format the
-	  strings for keys
-	*/
-
-	async fn get_key<T>(&self, key: &str) -> Result<T>
-	where
-		T: FromRedisValue,
-	{
-		debug!("Getting key {key}");
-
-		let mut con = self.client.get_async_connection().await?;
-		let res: T = con.get(key).await?;
-
-		Ok(res)
-	}
-
-	async fn set_key<'a>(
-		&self,
-		key: &str,
-		value: impl ToRedisArgs + Debug + Send + Sync + 'a,
-	) -> Result<()> {
-		debug!("Creating key {key}:\n{value:#?}");
-
-		let mut con = self.client.get_async_connection().await?;
-		con.set(key, value).await?;
-
-		Ok(())
-	}
-
-	async fn key_exists(&self, key: &str) -> Result<bool> {
-		debug!("Checking if key {key} exists");
-
-		let mut con = self.client.get_async_connection().await?;
-		let exists: u64 = con.exists(key).await?;
-
-		Ok(exists > 0)
-	}
-
-	/* we'll probably use this again
-	async fn delete_key(&self, key: &str) -> Result<()> {
-		debug!("Deleting key {key}");
-
-		let mut con = self.client.get_async_connection().await?;
-		con.del(key).await?;
-
-		Ok(())
-	}
-	*/
-
-	async fn expire_key(&self, key: &str, expire_seconds: i64) -> Result<()> {
-		debug!("Expiring key {key} in {expire_seconds}");
-
-		let mut con = self.client.get_async_connection().await?;
-		con.expire(key, expire_seconds).await?;
-
-		Ok(())
+	pub fn client(&self) -> &Client {
+		&self.client
 	}
 
 	pub async fn store_user_plurality(&self, sender: UserId) -> Result<()> {
-		info!("Marking {sender} as a PluralKit user");
+		debug!("Marking {sender} as a PluralKit user");
 		let key = format!("{PK_KEY}:{sender}");
 
+		let mut con = self.client.get_async_connection().await?;
 		// Just store some value. We only care about the presence of this key
-		self.set_key(&key, 0).await?;
-		self.expire_key(&key, 7 * 24 * 60 * 60).await?; // weekly
+		con.set_ex(key, 0, 7 * 24 * 60 * 60).await?; // 1 week
 
 		Ok(())
 	}
 
 	pub async fn is_user_plural(&self, user_id: UserId) -> Result<bool> {
+		debug!("Checking if user {user_id} is plural");
 		let key = format!("{PK_KEY}:{user_id}");
-		self.key_exists(&key).await
+
+		let mut con = self.client.get_async_connection().await?;
+		let exists = con.exists(key).await?;
+
+		Ok(exists)
 	}
 
 	pub async fn cache_launcher_version(&self, version: &str) -> Result<()> {
-		self.set_key(LAUNCHER_VERSION_KEY, version).await?;
-		self.expire_key(LAUNCHER_VERSION_KEY, 24 * 60 * 60).await?; // 1 day
+		debug!("Caching launcher version as {version}");
+
+		let mut con = self.client.get_async_connection().await?;
+		con.set_ex(LAUNCHER_VERSION_KEY, version, 24 * 60 * 60)
+			.await?; // 1 day
 
 		Ok(())
 	}
 
 	pub async fn get_launcher_version(&self) -> Result<String> {
-		let res = self.get_key(LAUNCHER_VERSION_KEY).await?;
+		debug!("Fetching launcher version");
+
+		let mut con = self.client.get_async_connection().await?;
+		let res = con.get(LAUNCHER_VERSION_KEY).await?;
 
 		Ok(res)
 	}

src/utils/resolve_message.rs

@@ -13,7 +13,7 @@ static MESSAGE_PATTERN: Lazy<Regex> = Lazy::new(|| {
 	Regex::new(r"/(https?:\/\/)?(?:canary\.|ptb\.)?discord(?:app)?\.com\/channels\/(?<serverId>\d+)\/(?<channelId>\d+)\/(?<messageId>\d+)/g;").unwrap()
 });
 
-pub fn find_first_image(msg: &Message) -> Option<String> {
+fn find_first_image(msg: &Message) -> Option<String> {
 	msg.attachments
 		.iter()
 		.find(|a| {