From 709f8cbf3c5146a0f53a6e9b6a4aeb3abd1aff35 Mon Sep 17 00:00:00 2001 From: Vika Date: Sat, 1 Jul 2023 20:40:38 +0300 Subject: micropub: use the new, better typed updates internally --- kittybox-rs/src/database/file/mod.rs | 58 ++++++------------ kittybox-rs/src/database/memory.rs | 111 +---------------------------------- kittybox-rs/src/database/mod.rs | 11 ++-- kittybox-rs/src/micropub/mod.rs | 45 ++++++++------ 4 files changed, 53 insertions(+), 172 deletions(-) diff --git a/kittybox-rs/src/database/file/mod.rs b/kittybox-rs/src/database/file/mod.rs index 3b373d8..0f63c9d 100644 --- a/kittybox-rs/src/database/file/mod.rs +++ b/kittybox-rs/src/database/file/mod.rs @@ -1,5 +1,6 @@ //#![warn(clippy::unwrap_used)] use crate::database::{filter_post, ErrorKind, Result, settings, Storage, StorageError}; +use crate::micropub::{MicropubUpdate, MicropubPropertyDeletion}; use async_trait::async_trait; use futures::{stream, StreamExt, TryStreamExt}; use serde_json::json; @@ -122,57 +123,32 @@ fn url_to_relative_path(url: &str) -> relative_path::RelativePathBuf { path } -fn modify_post(post: &serde_json::Value, update: &serde_json::Value) -> Result { +fn modify_post(post: &serde_json::Value, update: MicropubUpdate) -> Result { + let mut post = post.clone(); + let mut add_keys: HashMap> = HashMap::new(); let mut remove_keys: Vec = vec![]; let mut remove_values: HashMap> = HashMap::new(); - let mut post = post.clone(); - if let Some(delete) = update["delete"].as_array() { - remove_keys.extend( - delete - .iter() - .filter_map(|v| v.as_str()) - .map(|v| v.to_string()), - ); - } else if let Some(delete) = update["delete"].as_object() { + if let Some(MicropubPropertyDeletion::Properties(delete)) = update.delete { + remove_keys.extend(delete.iter().cloned()); + } else if let Some(MicropubPropertyDeletion::Values(delete)) = update.delete { for (k, v) in delete { - if let Some(v) = v.as_array() { - remove_values - .entry(k.to_string()) - .or_default() - .extend(v.clone()); - } else { - return Err(StorageError::from_static( - ErrorKind::BadRequest, - "Malformed update object", - )); - } + remove_values + .entry(k.to_string()) + .or_default() + .extend(v.clone()); } } - if let Some(add) = update["add"].as_object() { + if let Some(add) = update.add { for (k, v) in add { - if let Some(v) = v.as_array() { - add_keys.insert(k.to_string(), v.clone()); - } else { - return Err(StorageError::from_static( - ErrorKind::BadRequest, - "Malformed update object", - )); - } + add_keys.insert(k.to_string(), v.clone()); } } - if let Some(replace) = update["replace"].as_object() { + if let Some(replace) = update.replace { for (k, v) in replace { remove_keys.push(k.to_string()); - if let Some(v) = v.as_array() { - add_keys.insert(k.to_string(), v.clone()); - } else { - return Err(StorageError::from_static( - ErrorKind::BadRequest, - "Malformed update object", - )); - } + add_keys.insert(k.to_string(), v.clone()); } } @@ -446,7 +422,7 @@ impl Storage for FileStorage { } #[tracing::instrument(skip(self))] - async fn update_post(&self, url: &'_ str, update: serde_json::Value) -> Result<()> { + async fn update_post(&self, url: &str, update: MicropubUpdate) -> Result<()> { let path = url_to_path(&self.root_dir, url); let tempfilename = path.with_extension("tmp"); #[allow(unused_variables)] @@ -463,7 +439,7 @@ impl Storage for FileStorage { let json: serde_json::Value = serde_json::from_str(&content)?; drop(file); // Apply the editing algorithms - let new_json = modify_post(&json, &update)?; + let new_json = modify_post(&json, update)?; temp.write_all(new_json.to_string().as_bytes()).await?; temp.flush().await?; diff --git a/kittybox-rs/src/database/memory.rs b/kittybox-rs/src/database/memory.rs index 36e924f..ce98d05 100644 --- a/kittybox-rs/src/database/memory.rs +++ b/kittybox-rs/src/database/memory.rs @@ -88,115 +88,8 @@ impl Storage for MemoryStorage { Ok(()) } - async fn update_post(&self, url: &'_ str, update: serde_json::Value) -> Result<()> { - let mut add_keys: HashMap = HashMap::new(); - let mut remove_keys: Vec = vec![]; - let mut remove_values: HashMap> = HashMap::new(); - - if let Some(delete) = update["delete"].as_array() { - remove_keys.extend( - delete - .iter() - .filter_map(|v| v.as_str()) - .map(|v| v.to_string()), - ); - } else if let Some(delete) = update["delete"].as_object() { - for (k, v) in delete { - if let Some(v) = v.as_array() { - remove_values - .entry(k.to_string()) - .or_default() - .extend(v.clone()); - } else { - return Err(StorageError::from_static( - ErrorKind::BadRequest, - "Malformed update object", - )); - } - } - } - if let Some(add) = update["add"].as_object() { - for (k, v) in add { - if v.is_array() { - add_keys.insert(k.to_string(), v.clone()); - } else { - return Err(StorageError::from_static( - ErrorKind::BadRequest, - "Malformed update object", - )); - } - } - } - if let Some(replace) = update["replace"].as_object() { - for (k, v) in replace { - remove_keys.push(k.to_string()); - add_keys.insert(k.to_string(), v.clone()); - } - } - let mut mapping = self.mapping.write().await; - if let Some(mut post) = mapping.get(url) { - if let Some(url) = post["see_other"].as_str() { - if let Some(new_post) = mapping.get(url) { - post = new_post - } else { - return Err(StorageError::from_static( - ErrorKind::NotFound, - "The post you have requested is not found in the database.", - )); - } - } - let mut post = post.clone(); - for k in remove_keys { - post["properties"].as_object_mut().unwrap().remove(&k); - } - for (k, v) in remove_values { - let k = &k; - let props = if k == "children" { - &mut post - } else { - &mut post["properties"] - }; - v.iter().for_each(|v| { - if let Some(vec) = props[k].as_array_mut() { - if let Some(index) = vec.iter().position(|w| w == v) { - vec.remove(index); - } - } - }); - } - for (k, v) in add_keys { - let props = if k == "children" { - &mut post - } else { - &mut post["properties"] - }; - let k = &k; - if let Some(prop) = props[k].as_array_mut() { - if k == "children" { - v.as_array() - .unwrap() - .iter() - .cloned() - .rev() - .for_each(|v| prop.insert(0, v)); - } else { - prop.extend(v.as_array().unwrap().iter().cloned()); - } - } else { - post["properties"][k] = v - } - } - mapping.insert( - post["properties"]["uid"][0].as_str().unwrap().to_string(), - post, - ); - } else { - return Err(StorageError::from_static( - ErrorKind::NotFound, - "The designated post wasn't found in the database.", - )); - } - Ok(()) + async fn update_post(&self, url: &'_ str, update: crate::micropub::MicropubUpdate) -> Result<()> { + todo!() } async fn get_channels(&self, user: &'_ str) -> Result> { diff --git a/kittybox-rs/src/database/mod.rs b/kittybox-rs/src/database/mod.rs index 94a93ca..baae81d 100644 --- a/kittybox-rs/src/database/mod.rs +++ b/kittybox-rs/src/database/mod.rs @@ -5,6 +5,7 @@ use async_trait::async_trait; mod file; pub use crate::database::file::FileStorage; +use crate::micropub::MicropubUpdate; #[cfg(test)] mod memory; #[cfg(test)] @@ -295,10 +296,10 @@ pub trait Storage: std::fmt::Debug + Clone + Send + Sync { /// Add post to feed. Some database implementations might have optimized ways to do this. async fn add_to_feed(&self, feed: &'_ str, post: &'_ str) -> Result<()> { - self.update_post(feed, serde_json::json!({"add": {"children": [post]}})).await + self.update_post(feed, serde_json::from_str(r#"{"add": {"children": [post]}}"#).unwrap()).await } async fn remove_from_feed(&self, feed: &'_ str, post: &'_ str) -> Result<()> { - self.update_post(feed, serde_json::json!({"delete": {"children": [post]}})).await + self.update_post(feed, serde_json::from_str(r#"{"delete": {"children": [post]}}"#).unwrap()).await } /// Modify a post using an update object as defined in the Micropub spec. @@ -308,7 +309,7 @@ pub trait Storage: std::fmt::Debug + Clone + Send + Sync { /// each other's changes or simply corrupting something. Rejecting /// is allowed in case of concurrent updates if waiting for a lock /// cannot be done. - async fn update_post(&self, url: &'_ str, update: serde_json::Value) -> Result<()>; + async fn update_post(&self, url: &str, update: MicropubUpdate) -> Result<()>; /// Get a list of channels available for the user represented by the URL `user` to write to. async fn get_channels(&self, user: &'_ str) -> Result>; @@ -449,7 +450,7 @@ mod tests { backend .update_post( &key, - json!({ + serde_json::from_value(json!({ "url": &key, "add": { "category": ["testing"], @@ -457,7 +458,7 @@ mod tests { "replace": { "content": ["Different test content"] } - }), + })).unwrap(), ) .await .unwrap(); diff --git a/kittybox-rs/src/micropub/mod.rs b/kittybox-rs/src/micropub/mod.rs index a55ea15..fe3db57 100644 --- a/kittybox-rs/src/micropub/mod.rs +++ b/kittybox-rs/src/micropub/mod.rs @@ -56,9 +56,9 @@ fn populate_reply_context( mf2: &serde_json::Value, prop: &str, ctxs: &[FetchedPostContext], -) -> Option { +) -> Option> { mf2["properties"][prop].as_array().map(|array| { - json!(array + array .iter() // TODO: This seems to be O(n^2) and I don't like it. // Switching `ctxs` to a hashmap might speed it up to O(n) @@ -68,7 +68,8 @@ fn populate_reply_context( .find(|ctx| Some(ctx.url.as_str()) == i.as_str()) .and_then(|ctx| ctx.mf2["items"].get(0)) .unwrap_or(i)) - .collect::>()) + .cloned() + .collect::>() }) } @@ -147,13 +148,16 @@ async fn background_processing( .await }; - let mut update = json!({ "replace": {} }); - for prop in &context_props { + let mut update = MicropubUpdate { + replace: Some(Default::default()), + ..Default::default() + }; + for prop in context_props { if let Some(json) = populate_reply_context(&mf2, prop, &post_contexts) { - update["replace"][prop] = json; + update.replace.as_mut().unwrap().insert(prop.to_owned(), json); } } - if !update["replace"].as_object().unwrap().is_empty() { + if !update.replace.as_ref().unwrap().is_empty() { if let Err(err) = db.update_post(uid, update).await { error!("Failed to update post with rich reply contexts: {}", err); } @@ -323,7 +327,7 @@ enum ActionType { #[serde(untagged)] pub enum MicropubPropertyDeletion { Properties(Vec), - Values(HashMap) + Values(HashMap>) } #[derive(Serialize, Deserialize)] struct MicropubFormAction { @@ -335,12 +339,20 @@ struct MicropubFormAction { pub struct MicropubAction { action: ActionType, url: String, + #[serde(flatten)] #[serde(skip_serializing_if = "Option::is_none")] - replace: Option>, + update: Option +} + +#[derive(Serialize, Deserialize, Debug, Default)] +pub struct MicropubUpdate { #[serde(skip_serializing_if = "Option::is_none")] - add: Option>, + pub replace: Option>>, #[serde(skip_serializing_if = "Option::is_none")] - delete: Option, + pub add: Option>>, + #[serde(skip_serializing_if = "Option::is_none")] + pub delete: Option, + } impl From for MicropubAction { @@ -349,9 +361,7 @@ impl From for MicropubAction { Self { action: a.action, url: a.url, - replace: None, - add: None, - delete: None, + update: None } } } @@ -407,9 +417,10 @@ async fn post_action( db.update_post( &action.url, - // Here, unwrapping is safe, because this value - // was recently deserialized from JSON already. - serde_json::to_value(&action).unwrap(), + action.update.ok_or(MicropubError { + error: ErrorType::InvalidRequest, + error_description: "Update request is not set.".to_owned(), + })? ) .await? } -- cgit 1.4.1