MRG: adjust Signature::name() to return Option<String> instead of…

… `filename()` and `md5sum()` (#3434)

This PR adjusts `Signature::name()` to return `None` when no name is
set, instead of returning first `filename()` or (if empty) `md5sum()`.
It also adds `name_str()` which returns an empty string, to avoid too
many `unwrap_or` scattered throughout the codebase.

Fixes #3441

src/core/src/index/revindex/disk_revindex.rs

@@ -351,7 +351,14 @@ impl RevIndexOps for RevIndex {
                         .collection
                         .record_for_dataset(dataset_id)
                         .expect("dataset not found");
-                    Some((row.name().into(), size))
+
+                    let name = [row.name(), row.filename(), row.md5()]
+                        .into_iter()
+                        .skip_while(|v| v.is_empty())
+                        .next()
+                        .unwrap(); // guaranteed to succeed because `md5` always exists
+
+                    Some((name.into(), size))
                 } else {
                     None
                 }

src/core/src/index/revindex/mod.rs

@@ -558,7 +558,7 @@ mod test {
         )?;
 
         assert_eq!(matches.len(), 1);
-        assert_eq!(matches[0].name(), "../genome-s10.fa.gz");
+        assert_eq!(matches[0].name(), ""); // signature name is empty
         assert_eq!(matches[0].f_match(), 1.0);
 
         Ok(())

src/core/src/manifest.rs

@@ -129,7 +129,7 @@ impl Record {
                 Self {
                     internal_location: path.into(),
                     moltype: moltype.to_string(),
-                    name: sig.name(),
+                    name: sig.name_str(),
                     ksize,
                     md5,
                     md5short,

src/core/src/signature.rs

@@ -445,14 +445,13 @@ fn default_version() -> f64 {
 }
 
 impl Signature {
-    pub fn name(&self) -> String {
-        if let Some(name) = &self.name {
-            name.clone()
-        } else if let Some(filename) = &self.filename {
-            filename.clone()
-        } else {
-            self.md5sum()
-        }
+    pub fn name(&self) -> Option<String> {
+        self.name.clone()
+    }
+
+    /// return name, if not None; or "" if None.
+    pub fn name_str(&self) -> String {
+        self.name().unwrap_or("".into())
     }
 
     pub fn set_name(&mut self, name: &str) {
@@ -982,6 +981,8 @@ mod test {
         assert_eq!(sig.signatures[0].size(), 3);
         assert_eq!(sig.signatures[1].size(), 2);
         assert_eq!(sig.signatures[2].size(), 1);
+
+        assert_eq!(sig.name_str(), "");
     }
 
     #[test]

src/core/src/storage/mod.rs

@@ -466,7 +466,7 @@ impl ZipStorage {
 
 impl SigStore {
     pub fn new_with_storage(sig: Signature, storage: InnerStorage) -> Self {
-        let name = sig.name();
+        let name = sig.name_str();
         let filename = sig.filename();
 
         SigStore::builder()
@@ -555,7 +555,7 @@ impl Deref for SigStore {
 
 impl From<Signature> for SigStore {
     fn from(other: Signature) -> SigStore {
-        let name = other.name();
+        let name = other.name_str();
         let filename = other.filename();
 
         SigStore::builder()

src/core/tests/storage.rs

@@ -98,7 +98,7 @@ fn innerstorage_save_sig() -> Result<(), Box<dyn std::error::Error>> {
 
     let loaded_sig = instorage.load_sig("test")?;
 
-    assert_eq!(sig.name(), loaded_sig.name());
+    assert_eq!(sig.name_str(), loaded_sig.name());
     assert_eq!(sig.md5sum(), loaded_sig.md5sum());
 
     Ok(())