Database: use newtypes to represent settings

This allows much for a cleaner and idiomatic settings interface.

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<String> {
+    async fn get_setting<S: settings::Setting<'a>, 'a>(&self, user: &'_ str) -> Result<S> {
         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<String, String> = 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::<S>(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<S: settings::Setting<'a>, '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<String, String> = match File::open(&path).await {
+        let mut settings: HashMap<String, serde_json::Value> = 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<String> {
+    async fn get_setting<S: settings::Setting<'a>, 'a>(&'_ self, user: &'_ str) -> Result<S> {
         todo!()
     }
 
     #[allow(unused_variables)]
-    async fn set_setting(&self, setting: Settings, user: &'_ str, value: &'_ str) -> Result<()> {
+    async fn set_setting<S: settings::Setting<'a>, '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<Settings> 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<Settings> +*/ 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<str> 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<String>;
+    async fn get_setting<S: Setting<'a>, 'a>(&'_ self, user: &'_ str) -> Result<S>;
 
     /// Commits a setting to the setting store.
-    async fn set_setting(&self, setting: Settings, user: &'_ str, value: &'_ str) -> Result<()>;
+    async fn set_setting<S: Setting<'a>, '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: Storage>(backend: Backend) {
         backend
-            .set_setting(
-                crate::database::Settings::SiteName,
+            .set_setting::<settings::SiteName>(
                 "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::<settings::SiteName>("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<D: Storage>(
             //
             // 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::<crate::database::settings::SiteName>(&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<D: Storage>(
                     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<D: Storage>(
                 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::<crate::database::settings::SiteName>(&path)
+                    .map(Result::unwrap_or_default),
+
                     db.get_channels(&path).map(|i| i.unwrap_or_default())
                 );
 
@@ -182,8 +184,8 @@ pub async fn homepage<D: Storage>(
                         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<D: Storage>(
     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::<crate::database::settings::SiteName>(&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<D: Storage>(
                     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<D: Storage>(
         }
         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::<crate::database::settings::SiteName>(&host)
+                .map(Result::unwrap_or_default),
+
                 db.get_channels(&host).map(|i| i.unwrap_or_default())
             );
             (
@@ -256,8 +260,8 @@ pub async fn catchall<D: Storage>(
                     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<D: Storage + 'static>(
         ));
     }
 
-    db.set_setting(Settings::SiteName, user.me.as_str(), &data.blog_name)
+    db.set_setting::<settings::SiteName>(user.me.as_str(), data.blog_name.to_owned())
         .await
         .map_err(FrontendError::from)?;