Do not check for missing tag parents at registration time

pylib/anki/tags.py

@@ -80,6 +80,11 @@ class TagManager:
         res = self.col.db.list(query)
         return list(set(self.split(" ".join(res))))
 
+    def toggle_browser_collapse(self, name: str):
+        tag = self.col.backend.get_tag(name)
+        tag.config.browser_collapsed = not tag.config.browser_collapsed
+        self.col.backend.update_tag(tag)
+
     # Bulk addition/removal from notes
     #############################################################
 

qt/aqt/browser.py

@@ -21,7 +21,13 @@ from anki.consts import *
 from anki.lang import without_unicode_isolation
 from anki.models import NoteType
 from anki.notes import Note
-from anki.rsbackend import ConcatSeparator, DeckTreeNode, InvalidInput, TagTreeNode
+from anki.rsbackend import (
+    ConcatSeparator,
+    DeckTreeNode,
+    InvalidInput,
+    NotFoundError,
+    TagTreeNode,
+)
 from anki.stats import CardStats
 from anki.utils import htmlToTextLine, ids2str, isMac, isWin
 from aqt import AnkiQt, gui_hooks
@@ -451,8 +457,12 @@ class SidebarItem:
         expanded: bool = False,
         item_type: SidebarItemType = SidebarItemType.CUSTOM,
         id: int = 0,
+        full_name: str = None,
     ) -> None:
         self.name = name
+        if not full_name:
+            full_name = name
+        self.full_name = full_name
         self.icon = icon
         self.item_type = item_type
         self.id = id
@@ -1142,12 +1152,16 @@ QTableView {{ gridline-color: {grid} }}
                     return lambda: self.setFilter("tag", full_name)
 
                 def toggle_expand():
-                    tid = node.tag_id  # pylint: disable=cell-var-from-loop
+
+                    full_name = head + node.name  # pylint: disable=cell-var-from-loop
 
                     def toggle(_):
-                        tag = self.mw.col.backend.get_tag(tid)
+                        try:
-                        tag.config.browser_collapsed = not tag.config.browser_collapsed
+                            self.mw.col.tags.toggle_browser_collapse(full_name)
-                        self.mw.col.backend.update_tag(tag)
+                        except NotFoundError:
+                            # tag is missing, register it first
+                            self.mw.col.tags.register([full_name])
+                            self.mw.col.tags.toggle_browser_collapse(full_name)
 
                     return toggle
 
@@ -1159,6 +1173,7 @@ QTableView {{ gridline-color: {grid} }}
                     not node.collapsed,
                     item_type=SidebarItemType.TAG,
                     id=node.tag_id,
+                    full_name=head + node.name,
                 )
                 root.addChild(item)
                 newhead = head + node.name + "::"

qt/aqt/sidebar.py

@@ -118,7 +118,7 @@ class NewSidebarTreeView(SidebarTreeViewBase):
         self.browser.editor.saveNow(lambda: self._remove_tag(item))
 
     def _remove_tag(self, item: "aqt.browser.SidebarItem") -> None:
-        old_name = self.mw.col.backend.get_tag(item.id).name
+        old_name = item.full_name
 
         def do_remove():
             self.mw.col.backend.clear_tag(old_name)
@@ -138,7 +138,7 @@ class NewSidebarTreeView(SidebarTreeViewBase):
         self.browser.editor.saveNow(lambda: self._rename_tag(item))
 
     def _rename_tag(self, item: "aqt.browser.SidebarItem") -> None:
-        old_name = self.mw.col.backend.get_tag(item.id).name
+        old_name = item.full_name
         new_name = getOnlyText(tr(TR.ACTIONS_NEW_NAME), default=old_name)
         if new_name == old_name or not new_name:
             return

rslib/backend.proto

@@ -212,7 +212,7 @@ service BackendService {
 
     rpc RegisterTags (RegisterTagsIn) returns (Bool);
     rpc AllTags (Empty) returns (AllTagsOut);
-    rpc GetTag (TagID) returns (Tag);
+    rpc GetTag (String) returns (Tag);
     rpc UpdateTag (Tag) returns (Bool);
     rpc ClearTag (String) returns (Bool);
     rpc TagTree (Empty) returns (TagTreeNode);

rslib/src/backend/mod.rs

@@ -44,7 +44,6 @@ use crate::{
         get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress,
         SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage,
     },
-    tags::TagID,
     template::RenderedNode,
     text::{extract_av_tags, strip_av_tags, AVTag},
     timestamp::TimestampSecs,
@@ -1300,9 +1299,9 @@ impl BackendService for Backend {
         Ok(pb::AllTagsOut { tags })
     }
 
-    fn get_tag(&self, input: pb::TagId) -> BackendResult<pb::Tag> {
+    fn get_tag(&self, name: pb::String) -> BackendResult<pb::Tag> {
         self.with_col(|col| {
-            if let Some(tag) = col.storage.get_tag(TagID(input.tid))? {
+            if let Some(tag) = col.storage.get_tag(name.val.as_str())? {
                 Ok(tag.into())
             } else {
                 Err(AnkiError::NotFound)

rslib/src/storage/tag/mod.rs

@@ -30,22 +30,11 @@ impl SqliteStorage {
             .collect()
     }
 
-    /// Get all tags in human form, sorted by name
+    /// Get tag by human name
-    pub(crate) fn all_tags_sorted(&self) -> Result<Vec<Tag>> {
+    pub(crate) fn get_tag(&self, name: &str) -> Result<Option<Tag>> {
         self.db
-            .prepare_cached("select id, name, usn, config from tags order by name")?
+            .prepare_cached("select id, name, usn, config from tags where name = ?")?
-            .query_and_then(NO_PARAMS, |row| {
+            .query_and_then(&[human_tag_name_to_native(name)], |row| {
-                let mut tag = row_to_tag(row)?;
-                tag.name = native_tag_name_to_human(&tag.name);
-                Ok(tag)
-            })?
-            .collect()
-    }
-
-    pub(crate) fn get_tag(&self, id: TagID) -> Result<Option<Tag>> {
-        self.db
-            .prepare_cached("select id, name, usn, config from tags where id = ?")?
-            .query_and_then(&[id], |row| {
                 let mut tag = row_to_tag(row)?;
                 tag.name = native_tag_name_to_human(&tag.name);
                 Ok(tag)

rslib/src/tags.rs

@@ -13,12 +13,17 @@ use crate::{
 };
 
 use regex::{NoExpand, Regex, Replacer};
-use std::{borrow::Cow, collections::HashSet, iter::Peekable};
+use std::cmp::Ordering;
+use std::{
+    borrow::Cow,
+    collections::{HashMap, HashSet},
+    iter::Peekable,
+};
 use unicase::UniCase;
 
 define_newtype!(TagID, i64);
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone)]
 pub struct Tag {
     pub id: TagID,
     pub name: String,
@@ -26,6 +31,26 @@ pub struct Tag {
     pub config: TagConfig,
 }
 
+impl Ord for Tag {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.name.cmp(&other.name)
+    }
+}
+
+impl PartialOrd for Tag {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl PartialEq for Tag {
+    fn eq(&self, other: &Self) -> bool {
+        self.name == other.name
+    }
+}
+
+impl Eq for Tag {}
+
 impl Default for Tag {
     fn default() -> Self {
         Tag {
@@ -107,11 +132,33 @@ pub(crate) fn native_tag_name_to_human(name: &str) -> String {
     name.replace('\x1f', "::")
 }
 
-fn immediate_parent_name(native_name: &str) -> Option<&str> {
+fn fill_missing_tags(tags: Vec<Tag>) -> Vec<Tag> {
-    native_name.rsplitn(2, '\x1f').nth(1)
+    let mut filled_tags: HashMap<String, Tag> = HashMap::new();
+    for tag in tags.into_iter() {
+        let name = tag.name.to_owned();
+        let split: Vec<&str> = (&tag.name).split("::").collect();
+        for i in 0..split.len() - 1 {
+            let comp = split[0..i + 1].join("::");
+            let t = Tag {
+                name: comp.to_owned(),
+                ..Default::default()
+            };
+            if filled_tags.get(&comp).is_none() {
+                filled_tags.insert(comp, t);
+            }
+        }
+        if filled_tags.get(&name).is_none() {
+            filled_tags.insert(name, tag);
+        }
+    }
+    let mut tags: Vec<Tag> = filled_tags.values().map(|t| (*t).clone()).collect();
+    tags.sort_unstable();
+
+    tags
 }
 
 fn tags_to_tree(tags: Vec<Tag>) -> TagTreeNode {
+    let tags = fill_missing_tags(tags);
     let mut top = TagTreeNode::default();
     let mut it = tags.into_iter().peekable();
     add_child_nodes(&mut it, &mut top);
@@ -166,7 +213,7 @@ impl Collection {
     }
 
     pub fn tag_tree(&mut self) -> Result<TagTreeNode> {
-        let tags = self.storage.all_tags_sorted()?;
+        let tags = self.all_tags()?;
         let tree = tags_to_tree(tags);
 
         Ok(tree)
@@ -208,37 +255,17 @@ impl Collection {
         Ok((tags, added))
     }
 
-    fn create_missing_tag_parents(&self, mut native_name: &str, usn: Usn) -> Result<bool> {
-        let mut added = false;
-        while let Some(parent_name) = immediate_parent_name(native_name) {
-            if self.storage.preferred_tag_case(&parent_name)?.is_none() {
-                let mut t = Tag {
-                    name: parent_name.to_string(),
-                    usn,
-                    ..Default::default()
-                };
-                self.storage.register_tag(&mut t)?;
-                added = true;
-            }
-            native_name = parent_name;
-        }
-        Ok(added)
-    }
-
     pub(crate) fn register_tag<'a>(&self, tag: Tag) -> Result<(Cow<'a, str>, bool)> {
         let native_name = human_tag_name_to_native(&tag.name);
         if native_name.is_empty() {
             return Ok(("".into(), false));
         }
-        let added_parents = self.create_missing_tag_parents(&native_name, tag.usn)?;
         if let Some(preferred) = self.storage.preferred_tag_case(&native_name)? {
-            Ok((native_tag_name_to_human(&preferred).into(), added_parents))
+            Ok((native_tag_name_to_human(&preferred).into(), false))
         } else {
             let mut t = Tag {
                 name: native_name.clone(),
-                usn: tag.usn,
+                ..tag
-                config: tag.config,
-                ..Default::default()
             };
             self.storage.register_tag(&mut t)?;
             Ok((native_tag_name_to_human(&native_name).into(), true))
@@ -457,14 +484,6 @@ mod test {
         let note = col.storage.get_note(note.id)?.unwrap();
         assert_eq!(&note.tags, &["foo::bar", "foo::bar::bar", "foo::bar::foo",]);
 
-        // missing tag parents are registered too when registering their children
-        col.storage.clear_tags()?;
-        let mut note = col.storage.get_note(note.id)?.unwrap();
-        note.tags = vec!["animal::mammal::cat".into()];
-        col.update_note(&mut note)?;
-        let tags: Vec<String> = col.all_tags()?.into_iter().map(|t| t.name).collect();
-        assert_eq!(&tags, &["animal::mammal", "animal", "animal::mammal::cat"]);
-
         Ok(())
     }
 }