From eca7687439c2b6f804603de75501b6737a82e5a2 Mon Sep 17 00:00:00 2001 From: Vika Date: Thu, 15 Jun 2023 17:02:39 +0300 Subject: Database: use newtypes to represent settings This allows much for a cleaner and idiomatic settings interface. --- kittybox-rs/src/database/file/mod.rs | 31 ++++++------ kittybox-rs/src/database/memory.rs | 7 +-- kittybox-rs/src/database/mod.rs | 92 +++++++++++++++++++++++++++------- kittybox-rs/src/frontend/mod.rs | 36 +++++++------ kittybox-rs/src/frontend/onboarding.rs | 4 +- 5 files changed, 113 insertions(+), 57 deletions(-) diff --git a/kittybox-rs/src/database/file/mod.rs b/kittybox-rs/src/database/file/mod.rs index bafb7bf..9319b67 100644 --- a/kittybox-rs/src/database/file/mod.rs +++ b/kittybox-rs/src/database/file/mod.rs @@ -1,5 +1,5 @@ //#![warn(clippy::unwrap_used)] -use crate::database::{filter_post, ErrorKind, Result, Settings, Storage, StorageError}; +use crate::database::{filter_post, ErrorKind, Result, settings, Storage, StorageError}; use async_trait::async_trait; use futures::{stream, StreamExt, TryStreamExt}; use serde_json::json; @@ -582,7 +582,7 @@ impl Storage for FileStorage { } #[tracing::instrument(skip(self))] - async fn get_setting(&self, setting: Settings, user: &'_ str) -> Result { + async fn get_setting, 'a>(&self, user: &'_ str) -> Result { debug!("User for getting settings: {}", user); let url = axum::http::Uri::try_from(user).expect("Couldn't parse a URL"); let mut path = relative_path::RelativePathBuf::new(); @@ -591,22 +591,20 @@ impl Storage for FileStorage { let path = path.to_path(&self.root_dir); debug!("Getting settings from {:?}", &path); - let setting = setting.to_string(); + let mut file = File::open(path).await?; let mut content = String::new(); file.read_to_string(&mut content).await?; - let settings: HashMap = serde_json::from_str(&content)?; - // XXX consider returning string slices instead of cloning a string every time - // it might come with a performance hit and/or memory usage inflation - settings - .get(&setting) - .cloned() - .ok_or_else(|| StorageError::new(ErrorKind::Backend, "Setting not set")) + let settings: HashMap<&str, serde_json::Value> = serde_json::from_str(&content)?; + match settings.get(S::ID) { + Some(value) => Ok(serde_json::from_value::(value.clone())?), + None => Err(StorageError::new(ErrorKind::Backend, "Setting not set")) + } } #[tracing::instrument(skip(self))] - async fn set_setting(&self, setting: Settings, user: &'_ str, value: &'_ str) -> Result<()> { + async fn set_setting, 'a>(&self, user: &'_ str, value: S::Data) -> Result<()> { let url = axum::http::Uri::try_from(user).expect("Couldn't parse a URL"); let mut path = relative_path::RelativePathBuf::new(); path.push(url.authority().unwrap().to_string()); @@ -620,33 +618,32 @@ impl Storage for FileStorage { tokio::fs::create_dir_all(path.parent().unwrap()).await?; } - let (setting, value) = (setting.to_string(), value.to_string()); - let mut tempfile = OpenOptions::new() .write(true) .create_new(true) .open(&temppath) .await?; - let mut settings: HashMap = match File::open(&path).await { + let mut settings: HashMap = match File::open(&path).await { Ok(mut f) => { let mut content = String::new(); f.read_to_string(&mut content).await?; if content.is_empty() { - HashMap::default() + Default::default() } else { serde_json::from_str(&content)? } } Err(err) => { if err.kind() == IOErrorKind::NotFound { - HashMap::default() + Default::default() } else { return Err(err.into()); } } }; - settings.insert(setting, value); + settings.insert(S::ID.to_owned(), serde_json::to_value(S::new(value))?); + tempfile .write_all(serde_json::to_string(&settings)?.as_bytes()) .await?; diff --git a/kittybox-rs/src/database/memory.rs b/kittybox-rs/src/database/memory.rs index c8cc125..2fb55e5 100644 --- a/kittybox-rs/src/database/memory.rs +++ b/kittybox-rs/src/database/memory.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use std::sync::Arc; use tokio::sync::RwLock; -use crate::database::{ErrorKind, MicropubChannel, Result, Settings, Storage, StorageError}; +use crate::database::{ErrorKind, MicropubChannel, Result, settings, Storage, StorageError}; #[derive(Clone, Debug)] pub struct MemoryStorage { @@ -244,14 +244,15 @@ impl Storage for MemoryStorage { } #[allow(unused_variables)] - async fn get_setting(&self, setting: Settings, user: &'_ str) -> Result { + async fn get_setting, 'a>(&'_ self, user: &'_ str) -> Result { todo!() } #[allow(unused_variables)] - async fn set_setting(&self, setting: Settings, user: &'_ str, value: &'_ str) -> Result<()> { + async fn set_setting, 'a>(&self, user: &'_ str, value: S::Data) -> Result<()> { todo!() } + } impl Default for MemoryStorage { diff --git a/kittybox-rs/src/database/mod.rs b/kittybox-rs/src/database/mod.rs index 24cff38..2039ac0 100644 --- a/kittybox-rs/src/database/mod.rs +++ b/kittybox-rs/src/database/mod.rs @@ -1,6 +1,5 @@ #![warn(missing_docs)] use async_trait::async_trait; -use serde::{Deserialize, Serialize}; mod file; pub use crate::database::file::FileStorage; @@ -11,6 +10,8 @@ pub use crate::database::memory::MemoryStorage; pub use kittybox_util::MicropubChannel; +use self::settings::Setting; + /// Enum representing different errors that might occur during the database query. #[derive(Debug, Clone, Copy)] pub enum ErrorKind { @@ -33,17 +34,68 @@ pub enum ErrorKind { Other, } -/// Enum representing settings that might be stored in the site's database. -#[derive(Deserialize, Serialize, Debug, Clone, Copy)] -#[serde(rename_all = "snake_case")] -pub enum Settings { - /// The name of the website -- displayed in the header and the browser titlebar. - SiteName, -} +/// Settings that can be stored in the database. +pub mod settings { + mod private { + pub trait Sealed {} + } + /*#[derive(serde::Deserialize, serde::Serialize, Default)] + pub struct Settings { + pub site_name: SiteName, + pub webring: Webring + } + impl From for SiteName { + fn from(settings: Settings) -> Self { + settings.site_name + } + }*/ + + /// A trait for various settings that should be contained here. + /// + /// **Note**: this trait is sealed to prevent external + /// implementations, as it wouldn't make sense to add new settings + /// that aren't used by Kittybox itself. + pub trait Setting<'de>: private::Sealed + std::fmt::Debug + Default + Clone + serde::Serialize + serde::de::DeserializeOwned + /*From +*/ Send + Sync { + type Data: std::fmt::Debug + Send + Sync; + const ID: &'static str; + + /// Unwrap the setting type, returning owned data contained within. + fn into_inner(self) -> Self::Data; + /// Create a new instance of this type containing certain data. + fn new(data: Self::Data) -> Self; + } + + /// A website's title, shown in the header. + #[derive(Debug, serde::Deserialize, serde::Serialize, Clone, PartialEq, Eq)] + pub struct SiteName(String); + impl Default for SiteName { + fn default() -> Self { + Self("Kittybox".to_string()) + } + } + impl AsRef for SiteName { + fn as_ref(&self) -> &str { + self.0.as_str() + } + } + impl private::Sealed for SiteName {} + impl Setting<'_> for SiteName { + type Data = String; + const ID: &'static str = "site_name"; + + fn into_inner(self) -> String { + self.0 + } + fn new(data: Self::Data) -> Self { + Self(data) + } + } + impl SiteName { + fn from_str(data: &str) -> Self { + Self(data.to_owned()) + } + } -impl std::string::ToString for Settings { - fn to_string(&self) -> String { - serde_variant::to_variant_name(self).unwrap().to_string() } } @@ -248,14 +300,16 @@ pub trait Storage: std::fmt::Debug + Clone + Send + Sync { async fn delete_post(&self, url: &'_ str) -> Result<()>; /// Gets a setting from the setting store and passes the result. - async fn get_setting(&self, setting: Settings, user: &'_ str) -> Result; + async fn get_setting, 'a>(&'_ self, user: &'_ str) -> Result; /// Commits a setting to the setting store. - async fn set_setting(&self, setting: Settings, user: &'_ str, value: &'_ str) -> Result<()>; + async fn set_setting, 'a>(&self, user: &'_ str, value: S::Data) -> Result<()>; } #[cfg(test)] mod tests { + use super::settings; + use super::{MicropubChannel, Storage}; use serde_json::json; @@ -413,18 +467,18 @@ mod tests { async fn test_settings(backend: Backend) { backend - .set_setting( - crate::database::Settings::SiteName, + .set_setting::( "https://fireburn.ru/", - "Vika's Hideout", + "Vika's Hideout".to_owned() ) .await .unwrap(); assert_eq!( backend - .get_setting(crate::database::Settings::SiteName, "https://fireburn.ru/") - .await - .unwrap(), + .get_setting::("https://fireburn.ru/") + .await + .unwrap() + .as_ref(), "Vika's Hideout" ); } diff --git a/kittybox-rs/src/frontend/mod.rs b/kittybox-rs/src/frontend/mod.rs index f0f4e5a..9d5c164 100644 --- a/kittybox-rs/src/frontend/mod.rs +++ b/kittybox-rs/src/frontend/mod.rs @@ -132,8 +132,9 @@ pub async fn homepage( // // btw is it more efficient to fetch these in parallel? let (blogname, channels) = tokio::join!( - db.get_setting(crate::database::Settings::SiteName, &path) - .map(|i| i.unwrap_or_else(|_| "Kittybox".to_owned())), + db.get_setting::(&path) + .map(Result::unwrap_or_default), + db.get_channels(&path).map(|i| i.unwrap_or_default()) ); // Render the homepage @@ -144,8 +145,8 @@ pub async fn homepage( r#"text/html; charset="utf-8""#, )], Template { - title: &blogname, - blog_name: &blogname, + title: blogname.as_ref(), + blog_name: blogname.as_ref(), feeds: channels, user, content: MainPage { @@ -170,8 +171,9 @@ pub async fn homepage( error!("Error while fetching h-card and/or h-feed: {}", err); // Return the error let (blogname, channels) = tokio::join!( - db.get_setting(crate::database::Settings::SiteName, &path) - .map(|i| i.unwrap_or_else(|_| "Kittybox".to_owned())), + db.get_setting::(&path) + .map(Result::unwrap_or_default), + db.get_channels(&path).map(|i| i.unwrap_or_default()) ); @@ -182,8 +184,8 @@ pub async fn homepage( r#"text/html; charset="utf-8""#, )], Template { - title: &blogname, - blog_name: &blogname, + title: blogname.as_ref(), + blog_name: blogname.as_ref(), feeds: channels, user, content: ErrorPage { @@ -215,8 +217,9 @@ pub async fn catchall( match get_post_from_database(&db, path.as_str(), query.after, &user).await { Ok(post) => { let (blogname, channels) = tokio::join!( - db.get_setting(crate::database::Settings::SiteName, &host) - .map(|i| i.unwrap_or_else(|_| "Kittybox".to_owned())), + db.get_setting::(&host) + .map(Result::unwrap_or_default), + db.get_channels(&host).map(|i| i.unwrap_or_default()) ); // Render the homepage @@ -227,8 +230,8 @@ pub async fn catchall( r#"text/html; charset="utf-8""#, )], Template { - title: &blogname, - blog_name: &blogname, + title: blogname.as_ref(), + blog_name: blogname.as_ref(), feeds: channels, user, content: match post.pointer("/type/0").and_then(|i| i.as_str()) { @@ -245,8 +248,9 @@ pub async fn catchall( } Err(err) => { let (blogname, channels) = tokio::join!( - db.get_setting(crate::database::Settings::SiteName, &host) - .map(|i| i.unwrap_or_else(|_| "Kittybox".to_owned())), + db.get_setting::(&host) + .map(Result::unwrap_or_default), + db.get_channels(&host).map(|i| i.unwrap_or_default()) ); ( @@ -256,8 +260,8 @@ pub async fn catchall( r#"text/html; charset="utf-8""#, )], Template { - title: &blogname, - blog_name: &blogname, + title: blogname.as_ref(), + blog_name: blogname.as_ref(), feeds: channels, user, content: ErrorPage { diff --git a/kittybox-rs/src/frontend/onboarding.rs b/kittybox-rs/src/frontend/onboarding.rs index b4bae8e..88b533b 100644 --- a/kittybox-rs/src/frontend/onboarding.rs +++ b/kittybox-rs/src/frontend/onboarding.rs @@ -1,4 +1,4 @@ -use crate::database::{Settings, Storage}; +use crate::database::{settings, Storage}; use axum::{ extract::{Extension, Host}, http::StatusCode, @@ -68,7 +68,7 @@ async fn onboard( )); } - db.set_setting(Settings::SiteName, user.me.as_str(), &data.blog_name) + db.set_setting::(user.me.as_str(), data.blog_name.to_owned()) .await .map_err(FrontendError::from)?; -- cgit 1.4.1