micropub: use the new, better typed updates internally

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<serde_json::Value> {
+fn modify_post(post: &serde_json::Value, update: MicropubUpdate) -> Result<serde_json::Value> {
+    let mut post = post.clone();
+
     let mut add_keys: HashMap<String, Vec<serde_json::Value>> = HashMap::new();
     let mut remove_keys: Vec<String> = vec![];
     let mut remove_values: HashMap<String, Vec<serde_json::Value>> = 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<String, serde_json::Value> = HashMap::new();
-        let mut remove_keys: Vec<String> = vec![];
-        let mut remove_values: HashMap<String, Vec<serde_json::Value>> = 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<Vec<MicropubChannel>> {
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<Vec<MicropubChannel>>;
@@ -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<serde_json::Value> {
+) -> Option<Vec<serde_json::Value>> {
     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::<Vec<&serde_json::Value>>())
+            .cloned()
+            .collect::<Vec<serde_json::Value>>()
     })
 }
 
@@ -147,13 +148,16 @@ async fn background_processing<D: 'static + Storage>(
             .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<String>),
-    Values(HashMap<String, serde_json::Value>)
+    Values(HashMap<String, Vec<serde_json::Value>>)
 }
 #[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<HashMap<String, serde_json::Value>>,
+    update: Option<MicropubUpdate>
+}
+
+#[derive(Serialize, Deserialize, Debug, Default)]
+pub struct MicropubUpdate {
     #[serde(skip_serializing_if = "Option::is_none")]
-    add: Option<HashMap<String, serde_json::Value>>,
+    pub replace: Option<HashMap<String, Vec<serde_json::Value>>>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    delete: Option<MicropubPropertyDeletion>,
+    pub add: Option<HashMap<String, Vec<serde_json::Value>>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub delete: Option<MicropubPropertyDeletion>,
+
 }
 
 impl From<MicropubFormAction> for MicropubAction {
@@ -349,9 +361,7 @@ impl From<MicropubFormAction> 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<D: Storage, A: AuthBackend>(
 
             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?
         }