simplify unused tags and DB check

- backend routines should contain minimal logic, and should call
into a routine on the collection
- instead of copying the giant-string approach the Python code was taking,
we use a HashSet to keep track of seen tags as we loop through the
notes, which should be more efficient

rslib/src/backend/mod.rs

@@ -44,7 +44,6 @@ use crate::{
         LocalServer, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput,
         SyncStage,
     },
-    tags::join_tags,
     template::RenderedNode,
     text::{escape_anki_wildcards, extract_av_tags, strip_av_tags, AVTag},
     timestamp::TimestampSecs,
@@ -1361,15 +1360,7 @@ impl BackendService for Backend {
     }
 
     fn clear_unused_tags(&self, _input: pb::Empty) -> BackendResult<pb::Empty> {
-        self.with_col(|col| {
-            col.transact(None, |col| {
-                let old_tags = col.storage.all_tags()?;
-                let note_tags = join_tags(&col.storage.get_note_tags()?);
-                col.register_tags(&note_tags, col.usn()?, true)?;
-                col.update_tags_collapse(old_tags)?;
-                Ok(().into())
-            })
-        })
+        self.with_col(|col| col.transact(None, |col| col.clear_unused_tags().map(Into::into)))
     }
 
     fn clear_tag(&self, tag: pb::String) -> BackendResult<pb::Empty> {

rslib/src/dbcheck.rs

@@ -242,8 +242,7 @@ impl Collection {
         let usn = self.usn()?;
         let stamp = TimestampMillis::now();
 
-        // will rebuild tag list below
-        let old_tags = self.storage.all_tags()?;
+        let collapsed_tags = self.storage.collapsed_tags()?;
         self.storage.clear_tags()?;
 
         let total_notes = self.storage.total_notes()?;
@@ -295,8 +294,9 @@ impl Collection {
             }
         }
 
-        // restore tags collapse state and re-register old tags that are parents of used ones
-        self.update_tags_collapse(old_tags)?;
+        // the note rebuilding process took care of adding tags back, so we just need
+        // to ensure to restore the collapse state
+        self.storage.restore_collapsed_tags(&collapsed_tags)?;
 
         // if the collection is empty and the user has deleted all note types, ensure at least
         // one note type exists
@@ -636,4 +636,23 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    fn tags() -> Result<()> {
+        let mut col = open_test_collection();
+        let nt = col.get_notetype_by_name("Basic")?.unwrap();
+        let mut note = nt.new_note();
+        note.tags.push("one".into());
+        note.tags.push("two".into());
+        col.add_note(&mut note, DeckID(1))?;
+
+        col.set_tag_collapsed("two", true)?;
+
+        col.check_database(progress_fn)?;
+
+        assert_eq!(col.storage.get_tag("one")?.unwrap().collapsed, false);
+        assert_eq!(col.storage.get_tag("two")?.unwrap().collapsed, true);
+
+        Ok(())
+    }
 }

rslib/src/storage/note/mod.rs

@@ -1,6 +1,8 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
+use std::collections::HashSet;
+
 use crate::{
     err::Result,
     notes::{Note, NoteID},
@@ -157,11 +159,19 @@ impl super::SqliteStorage {
             .map_err(Into::into)
     }
 
-    // get distinct note tags
-    pub(crate) fn get_note_tags(&self) -> Result<Vec<String>> {
-        self.db
-            .prepare_cached("select distinct tags from notes")?
-            .query_and_then(NO_PARAMS, |r| Ok(r.get_raw(0).as_str()?.to_owned()))?
-            .collect()
+    pub(crate) fn all_tags_in_notes(&self) -> Result<HashSet<String>> {
+        let mut stmt = self
+            .db
+            .prepare_cached("select tags from notes where tags != ''")?;
+        let mut query = stmt.query(NO_PARAMS)?;
+        let mut seen: HashSet<String> = HashSet::new();
+        while let Some(rows) = query.next()? {
+            for tag in split_tags(rows.get_raw(0).as_str()?) {
+                if !seen.contains(tag) {
+                    seen.insert(tag.to_string());
+                }
+            }
+        }
+        Ok(seen)
     }
 }

rslib/src/storage/tag/mod.rs

@@ -24,7 +24,23 @@ impl SqliteStorage {
             .collect()
     }
 
-    /// Get tag by human name
+    pub(crate) fn collapsed_tags(&self) -> Result<Vec<String>> {
+        self.db
+            .prepare_cached("select tag from tags where collapsed = true")?
+            .query_and_then(NO_PARAMS, |r| r.get::<_, String>(0).map_err(Into::into))?
+            .collect::<Result<Vec<_>>>()
+    }
+
+    pub(crate) fn restore_collapsed_tags(&self, tags: &[String]) -> Result<()> {
+        let mut stmt = self
+            .db
+            .prepare_cached("update tags set collapsed = true where tag = ?")?;
+        for tag in tags {
+            stmt.execute(&[tag])?;
+        }
+        Ok(())
+    }
+
     pub(crate) fn get_tag(&self, name: &str) -> Result<Option<Tag>> {
         self.db
             .prepare_cached("select tag, usn, collapsed from tags where tag = ?")?

rslib/src/tags.rs

@@ -234,16 +234,19 @@ impl Collection {
         }
     }
 
-    pub(crate) fn register_tags(&self, tags: &str, usn: Usn, clear_first: bool) -> Result<bool> {
-        let mut changed = false;
-        if clear_first {
-            self.storage.clear_tags()?;
+    pub fn clear_unused_tags(&self) -> Result<()> {
+        let collapsed: HashSet<_> = self.storage.collapsed_tags()?.into_iter().collect();
+        self.storage.clear_tags()?;
+        let usn = self.usn()?;
+        for name in self.storage.all_tags_in_notes()? {
+            self.register_tag(Tag {
+                collapsed: collapsed.contains(&name),
+                name,
+                usn,
+            })?;
         }
-        for tag in split_tags(tags) {
-            let t = self.register_tag(Tag::new(tag.to_string(), usn))?;
-            changed |= t.1;
-        }
-        Ok(changed)
+
+        Ok(())
     }
 
     pub(crate) fn set_tag_collapsed(&self, name: &str, collapsed: bool) -> Result<()> {
@@ -254,22 +257,6 @@ impl Collection {
         self.storage.set_tag_collapsed(name, collapsed)
     }
 
-    /// Update collapse state of existing tags and register tags in old_tags that are parents of those tags
-    pub(crate) fn update_tags_collapse(&self, old_tags: Vec<Tag>) -> Result<()> {
-        let new_tags = self.storage.all_tags()?;
-        for old in old_tags.into_iter() {
-            for new in new_tags.iter() {
-                if new.name == old.name {
-                    self.storage.set_tag_collapsed(&new.name, old.collapsed)?;
-                    break;
-                } else if new.name.starts_with(&old.name) {
-                    self.set_tag_collapsed(&old.name, old.collapsed)?;
-                }
-            }
-        }
-        Ok(())
-    }
-
     fn replace_tags_for_notes_inner<R: Replacer>(
         &mut self,
         nids: &[NoteID],
@@ -447,7 +434,10 @@ mod test {
         assert_eq!(&note.tags, &["foo::bar", "foo::bar::bar", "foo::bar::foo",]);
 
         // tag children are also cleared when clearing their parent
-        col.register_tags("a a::b a::b::c", Usn(-1), true)?;
+        col.storage.clear_tags()?;
+        for name in vec!["a", "a::b", "a::b::c"] {
+            col.register_tag(Tag::new(name.to_string(), Usn(0)))?;
+        }
         col.storage.clear_tag("a")?;
         assert_eq!(col.storage.all_tags()?, vec![]);
 
@@ -545,4 +535,21 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    fn clearing() -> Result<()> {
+        let mut col = open_test_collection();
+        let nt = col.get_notetype_by_name("Basic")?.unwrap();
+        let mut note = nt.new_note();
+        note.tags.push("one".into());
+        note.tags.push("two".into());
+        col.add_note(&mut note, DeckID(1))?;
+
+        col.set_tag_collapsed("two", true)?;
+        col.clear_unused_tags()?;
+        assert_eq!(col.storage.get_tag("one")?.unwrap().collapsed, false);
+        assert_eq!(col.storage.get_tag("two")?.unwrap().collapsed, true);
+
+        Ok(())
+    }
 }