Replace the complete serialisation variant of Resource with a pair of functions to expand and compress the already existing structure

Also adds additional tests to resource.rs

src/editor/commands/editor.rs

@@ -5,20 +5,23 @@ use crate::schema::resource::Resource;
 use std::path::PathBuf;
 use tracing::{error, info};
 
-pub fn editor(database_path: PathBuf, save_settings: SaveSettings, resource: String, create: bool) {
+pub fn editor(database_path: PathBuf, save_settings: SaveSettings, resource_lookup: String, create_new_resource: bool) {
     let resources = LookupHandler::load(&database_path).unwrap();
-    let (index, resource) = match resources.lookup_with_index(resource.as_str()) {
+
+    let (index, resource) = match resources.lookup_with_index(resource_lookup.as_str()) {
         None => {
-            if create {
+            if create_new_resource {
-                (None, Resource::new(resource))
+                (None, Resource::new(resource_lookup))
             } else {
-                error!("Couldn't find a resource for the query \"{resource}\"");
+                error!("Couldn't find a resource for the query \"{resource_lookup}\"");
                 return;
             }
         }
         Some((index, resource)) => (Some(index), resource.clone()),
     };
-    let resource = open_resource_in_editor(resource).unwrap();
+
+    let resource = open_resource_in_editor(&resource).unwrap();
+
     if save_settings.save {
         let mut resources = resources.into_inner();
         if let Some(index) = index {

src/editor/open_in_editor.rs

@@ -1,5 +1,4 @@
 use crate::schema::resource::Resource;
-use crate::schema::resource_complete_serialisation::ResourceComplete;
 use std::fmt::{Debug, Display};
 use std::io::{Read, Seek, SeekFrom, Write};
 use std::process::Command;
@@ -71,21 +70,22 @@ pub fn spawn_editor(buffer: impl AsRef<str> + Display) -> Result<String, EditorS
 /// Error type returned by `open_resource_in_editor`
 #[derive(Debug, Error)]
 pub enum ResourceEditingError {
-    #[error("Failed to pretty print resource")]
+    #[error("failed to pretty print resource")]
     PrettyPrintFailed(serde_json::Error),
-    #[error("Downstream editor spawn failed: {0}")]
+    #[error("downstream editor spawn failed: {0}")]
     EditorSpawnError(#[from] EditorSpawnError),
-    #[error("Failed to parse edited JSON")]
+    #[error("failed to parse edited JSON")]
     ParseFailed(serde_json::Error),
 }
 
 /// Opens the provided `resource` in the system editor and returns the edited version or an error if something goes wrong.
-#[instrument(skip(resource))]
+#[instrument(skip_all)]
-pub fn open_resource_in_editor(resource: Resource) -> Result<Resource, ResourceEditingError> {
+pub fn open_resource_in_editor(resource: &Resource) -> Result<Resource, ResourceEditingError> {
     use ResourceEditingError::*;
-    let resource = ResourceComplete::from_resource(resource);
+
+    let resource = resource.as_completely_serializable();
     let printed = serde_json::to_string_pretty(&resource).map_err(PrettyPrintFailed)?;
     let edited = spawn_editor(printed)?;
-    let parsed: ResourceComplete = serde_json::from_str(edited.as_str()).map_err(ParseFailed)?;
+    let parsed: Resource = serde_json::from_str(edited.as_str()).map_err(ParseFailed)?;
-    Ok(parsed.into_resource())
+    Ok(parsed.compress())
 }

src/schema/mod.rs

@@ -1,4 +1,3 @@
 pub mod lookup_handler;
 pub mod resource;
-pub mod resource_complete_serialisation;
 pub mod resource_list;

src/schema/resource.rs

@@ -45,6 +45,10 @@ impl Resource {
     }
 
     pub fn add_new_primary_subject(&mut self, mut subject: String) {
+        if subject == self.subject {
+            debug!("New and old subjects match, skipping...");
+            return;
+        }
         debug!("Swapping new and old subject");
         std::mem::swap(&mut subject, &mut self.subject);
         debug!("Pushing current subject into aliases");
@@ -59,6 +63,70 @@ impl Resource {
             self.aliases = Some(vec![subject])
         }
     }
+
+    /// Returns a clone of the underlying data with all instances of `Option::None` removed
+    /// and instead replaced with `Some(Default::default())`. Useful to provide an easier to
+    /// extend version of the data for manual editing.
+    pub fn as_completely_serializable(&self) -> Self {
+        let clone_hashmap_with_option_value_as_complete = |props: &HashMap<String, Option<String>>| {
+            HashMap::from_iter(props.iter().map(|(key, value)| {
+                (key.clone(), Some(value.clone().unwrap_or_default()))
+            }))
+        };
+
+        Self {
+            subject: self.subject.clone(),
+            aliases: Some(self.aliases.clone().unwrap_or_default()),
+            properties: Some(self.properties.as_ref().map(clone_hashmap_with_option_value_as_complete).unwrap_or_default()),
+            links: Some(self.links.as_ref().map(|links| {
+                Vec::from_iter(links.iter().map(|link| {
+                    Link {
+                        rel: link.rel.clone(),
+                        media_type: Some(link.media_type.clone().unwrap_or_default()),
+                        href: Some(link.href.clone().unwrap_or_default()),
+                        titles: Some(link.titles.clone().unwrap_or_default()),
+                        properties: Some(link.properties.as_ref().map(clone_hashmap_with_option_value_as_complete).unwrap_or_default()),
+                    }
+                }))
+            }).unwrap_or_default()),
+        }
+    }
+
+    /// Replaces all instances of `Some(Default::default())` with `None`
+    ///
+    /// Useful to reduce the size of the serialized representation.
+    pub fn compress(self) -> Self {
+        Self {
+            subject: self.subject,
+            aliases: self.aliases.filter(|data| !data.is_empty()),
+            properties: self.properties.filter(|data| !data.is_empty()).map(|mut data| {
+                for value in data.values_mut() {
+                    if let Some(ref mut string) = value {
+                        if string.is_empty() {
+                            *value = None;
+                        }
+                    }
+                }
+                data
+            }),
+            links: self.links.filter(|links| !links.is_empty()).map(|mut links| {
+                links.retain(|link| {
+                    // Empty `rel` is invalid, but short-circuiting here would delete records
+                    // that are only partially edited. Better to store invalid data than to delete
+                    // users' work.
+                    let mut is_default = link.rel.is_empty();
+                    is_default &= link.media_type.as_ref().filter(|media_type| !media_type.is_empty()).is_none();
+                    is_default &= link.href.as_ref().filter(|href| !href.is_empty()).is_none();
+                    is_default &= link.titles.as_ref().filter(|titles| !titles.is_empty()).is_none();
+                    is_default &= link.properties.as_ref().filter(|titles| !titles.is_empty()).is_none();
+
+                    is_default
+                });
+
+                links
+            })
+        }
+    }
 }
 
 /// A link contained within a WebFinger resource
@@ -95,6 +163,15 @@ pub mod test_data {
             links: None,
         }
     }
+
+    pub fn user_with_matching_subject_and_alias() -> Resource {
+        Resource {
+            subject: "acct:user@domain.tld".to_string(),
+            aliases: Some(vec!["acct:user@domain.tld".to_string()]),
+            properties: None,
+            links: None,
+        }
+    }
 }
 
 #[cfg(test)]
@@ -115,4 +192,56 @@ mod tests {
             }
         )
     }
+
+    #[test]
+    fn insert_new_primary_subject_into_new_user() {
+        let mut data = Resource::new("testing_subject".to_string());
+        data.add_new_primary_subject("new_subject".to_string());
+        assert_eq!(
+            data,
+            Resource {
+                subject: "new_subject".to_string(),
+                aliases: Some(vec!["testing_subject".to_string()]),
+                properties: None,
+                links: None
+            }
+        )
+    }
+
+    #[test]
+    fn insert_new_primary_subject_into_new_user_multiple_times() {
+        let mut data = Resource::new("testing_subject".to_string());
+        for _ in 0..10 {
+            data.add_new_primary_subject("new_subject".to_string())
+        }
+        assert_eq!(
+            data,
+            Resource {
+                subject: "new_subject".to_string(),
+                aliases: Some(vec!["testing_subject".to_string()]),
+                properties: None,
+                links: None
+            }
+        )
+    }
+
+    #[test]
+    fn check_preventing_alias_deduplication() {
+        let mut data = test_data::user_with_matching_subject_and_alias();
+        data.add_new_primary_subject("new_subject".to_string());
+        let mut check = test_data::user_with_matching_subject_and_alias();
+        check.subject = "new_subject".to_string();
+        assert_eq!(data, check)
+    }
+
+    #[test]
+    fn test_compression_decompression_round_trip() {
+        for data in [
+            test_data::barebones_user(),
+            test_data::user_with_matching_subject_and_alias(),
+            test_data::user_with_single_alias()
+        ] {
+            assert_eq!(data, data.as_completely_serializable().compress());
+        }
+    }
 }