Keep tags in human form and update the tags table structure

See https://github.com/ankitects/anki/pull/900#issuecomment-758284016

- Leave tag names alone and add the collapsed and config columns to the tags table.
- Update The DB check code to preserve the collapse state of used tags.
- Add a simple test for clearing tags and their children
This commit is contained in:
abdo 2021-01-12 23:12:35 +03:00
parent 0b5bb711a1
commit 9a68d84483
11 changed files with 106 additions and 230 deletions

View File

@ -1158,7 +1158,6 @@ QTableView {{ gridline-color: {grid} }}
toggle_expand(),
not node.collapsed,
item_type=SidebarItemType.TAG,
id=node.tag_id,
full_name=head + node.name,
)
root.addChild(item)

View File

@ -797,15 +797,10 @@ message SetTagCollapsedIn {
bool collapsed = 2;
}
message TagConfig {
bool browser_collapsed = 1;
}
message Tag {
int64 id = 1;
string name = 2;
sint32 usn = 3;
TagConfig config = 4;
string name = 1;
sint32 usn = 2;
bool collapsed = 3;
}
message GetChangedTagsOut {
@ -813,10 +808,9 @@ message GetChangedTagsOut {
}
message TagTreeNode {
int64 tag_id = 1;
string name = 2;
repeated TagTreeNode children = 3;
uint32 level = 5;
string name = 1;
repeated TagTreeNode children = 2;
uint32 level = 3;
bool collapsed = 4;
}

View File

@ -44,7 +44,6 @@ use crate::{
get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress,
SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage,
},
tags::Tag,
template::RenderedNode,
text::{extract_av_tags, strip_av_tags, AVTag},
timestamp::TimestampSecs,
@ -1295,27 +1294,23 @@ impl BackendService for Backend {
//-------------------------------------------------------------------
fn all_tags(&self, _input: Empty) -> BackendResult<pb::AllTagsOut> {
let tags: Vec<pb::Tag> =
self.with_col(|col| Ok(col.all_tags()?.into_iter().map(|t| t.into()).collect()))?;
let tags: Vec<pb::Tag> = self.with_col(|col| {
Ok(col
.storage
.all_tags()?
.into_iter()
.map(|t| t.into())
.collect())
})?;
Ok(pb::AllTagsOut { tags })
}
fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult<pb::Bool> {
self.with_col(|col| {
let name = &input.name;
let mut tag = if let Some(tag) = col.storage.get_tag(name)? {
tag
} else {
// tag is missing, register it
let t = Tag {
name: name.to_owned(),
..Default::default()
};
col.register_tag(t)?.0
};
tag.config.browser_collapsed = input.collapsed;
col.update_tag(&tag)?;
Ok(pb::Bool { val: true })
col.transact(None, |col| {
col.set_tag_collapsed(&input.name, input.collapsed)?;
Ok(pb::Bool { val: true })
})
})
}
@ -1336,7 +1331,7 @@ impl BackendService for Backend {
fn clear_tag(&self, tag: pb::String) -> BackendResult<pb::Bool> {
self.with_col(|col| {
col.transact(None, |col| {
col.clear_tag(tag.val.as_str())?;
col.storage.clear_tag(tag.val.as_str())?;
Ok(pb::Bool { val: true })
})
})

View File

@ -243,6 +243,7 @@ impl Collection {
let stamp = TimestampMillis::now();
// will rebuild tag list below
let old_tags = self.storage.all_tags_sorted()?;
self.storage.clear_tags()?;
let total_notes = self.storage.total_notes()?;
@ -294,6 +295,18 @@ impl Collection {
}
}
let new_tags = self.storage.all_tags_sorted()?;
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, new.collapsed)?;
break;
} else if new.name.starts_with(&old.name) {
self.set_tag_collapsed(&old.name, old.collapsed)?;
}
}
}
// if the collection is empty and the user has deleted all note types, ensure at least
// one note type exists
if self.storage.get_all_notetype_names()?.is_empty() {

View File

@ -10,7 +10,6 @@ use crate::{
notes::field_checksum,
notetype::NoteTypeID,
storage::ids_to_string,
tags::human_tag_name_to_native,
text::{
is_glob, matches_glob, normalize_to_nfc, strip_html_preserving_media_filenames,
to_custom_re, to_re, to_sql, to_text, without_combining,
@ -204,8 +203,7 @@ impl SqlWriter<'_> {
text => {
write!(self.sql, "n.tags regexp ?").unwrap();
let re = &to_custom_re(text, r"\S");
let native_name = human_tag_name_to_native(re);
self.args.push(format!("(?i).* {}(\x1f| ).*", native_name));
self.args.push(format!("(?i).* {}(::| ).*", re));
}
}
}
@ -681,14 +679,14 @@ mod test {
s(ctx, r"tag:one"),
(
"(n.tags regexp ?)".into(),
vec!["(?i).* one(\x1f| ).*".into()]
vec!["(?i).* one(::| ).*".into()]
)
);
assert_eq!(
s(ctx, r"tag:foo::bar"),
(
"(n.tags regexp ?)".into(),
vec!["(?i).* foo\x1fbar(\x1f| ).*".into()]
vec!["(?i).* foo::bar(::| ).*".into()]
)
);
@ -696,7 +694,7 @@ mod test {
s(ctx, r"tag:o*n\*et%w%oth_re\_e"),
(
"(n.tags regexp ?)".into(),
vec!["(?i).* o\\S*n\\*et%w%oth\\Sre_e(\u{1f}| ).*".into()]
vec![r"(?i).* o\S*n\*et%w%oth\Sre_e(::| ).*".into()]
)
);
assert_eq!(s(ctx, "tag:none"), ("(n.tags = '')".into(), vec![]));

View File

@ -5,7 +5,7 @@ use crate::{
err::Result,
notes::{Note, NoteID},
notetype::NoteTypeID,
tags::{human_tag_name_to_native, join_tags, native_tag_name_to_human, split_tags},
tags::{join_tags, split_tags},
timestamp::TimestampMillis,
};
use rusqlite::{params, Row, NO_PARAMS};
@ -18,11 +18,6 @@ pub(crate) fn join_fields(fields: &[String]) -> String {
fields.join("\x1f")
}
fn native_tags_str(tags: &[String]) -> String {
let s: Vec<_> = tags.iter().map(|t| human_tag_name_to_native(t)).collect();
join_tags(&s)
}
fn row_to_note(row: &Row) -> Result<Note> {
Ok(Note {
id: row.get(0)?,
@ -31,7 +26,7 @@ fn row_to_note(row: &Row) -> Result<Note> {
mtime: row.get(3)?,
usn: row.get(4)?,
tags: split_tags(row.get_raw(5).as_str()?)
.map(|t| native_tag_name_to_human(t))
.map(Into::into)
.collect(),
fields: split_fields(row.get_raw(6).as_str()?),
sort_field: None,
@ -57,7 +52,7 @@ impl super::SqliteStorage {
note.notetype_id,
note.mtime,
note.usn,
native_tags_str(&note.tags),
join_tags(&note.tags),
join_fields(&note.fields()),
note.sort_field.as_ref().unwrap(),
note.checksum.unwrap(),
@ -75,7 +70,7 @@ impl super::SqliteStorage {
note.notetype_id,
note.mtime,
note.usn,
native_tags_str(&note.tags),
join_tags(&note.tags),
join_fields(&note.fields()),
note.sort_field.as_ref().unwrap(),
note.checksum.unwrap(),
@ -93,7 +88,7 @@ impl super::SqliteStorage {
note.notetype_id,
note.mtime,
note.usn,
native_tags_str(&note.tags),
join_tags(&note.tags),
join_fields(&note.fields()),
note.sort_field.as_ref().unwrap(),
note.checksum.unwrap(),
@ -162,69 +157,18 @@ impl super::SqliteStorage {
.map_err(Into::into)
}
// get distinct note tags in human form
// get distinct note tags
pub(crate) fn get_note_tags(&self, nids: Vec<NoteID>) -> Result<Vec<String>> {
if nids.is_empty() {
self.db
.prepare_cached("select distinct tags from notes")?
.query_and_then(NO_PARAMS, |r| {
let t = r.get_raw(0).as_str()?;
Ok(native_tag_name_to_human(t))
})?
.query_and_then(NO_PARAMS, |r| Ok(r.get_raw(0).as_str()?.to_owned()))?
.collect()
} else {
self.db
.prepare_cached("select distinct tags from notes where id in ?")?
.query_and_then(nids, |r| {
let t = r.get_raw(0).as_str()?;
Ok(native_tag_name_to_human(t))
})?
.query_and_then(nids, |r| Ok(r.get_raw(0).as_str()?.to_owned()))?
.collect()
}
}
pub(super) fn upgrade_notes_to_schema17(&self) -> Result<()> {
let notes: Result<Vec<(NoteID, String)>> = self
.db
.prepare_cached("select id, tags from notes")?
.query_and_then(NO_PARAMS, |row| -> Result<(NoteID, String)> {
let id = NoteID(row.get_raw(0).as_i64()?);
let tags: Vec<String> = split_tags(row.get_raw(1).as_str()?)
.map(|t| human_tag_name_to_native(t))
.collect();
let tags = join_tags(&tags);
Ok((id, tags))
})?
.collect();
notes?.into_iter().try_for_each(|(id, tags)| -> Result<_> {
self.db
.prepare_cached("update notes set tags = ? where id = ?")?
.execute(params![tags, id])?;
Ok(())
})?;
Ok(())
}
pub(super) fn downgrade_notes_from_schema17(&self) -> Result<()> {
let notes: Result<Vec<(NoteID, String)>> = self
.db
.prepare_cached("select id, tags from notes")?
.query_and_then(NO_PARAMS, |row| -> Result<(NoteID, String)> {
let id = NoteID(row.get_raw(0).as_i64()?);
let tags: Vec<String> = split_tags(row.get_raw(1).as_str()?)
.map(|t| native_tag_name_to_human(t))
.collect();
let tags = join_tags(&tags);
Ok((id, tags))
})?
.collect();
notes?.into_iter().try_for_each(|(id, tags)| -> Result<_> {
self.db
.prepare_cached("update notes set tags = ? where id = ?")?
.execute(params![tags, id])?;
Ok(())
})?;
Ok(())
}
}

View File

@ -1,3 +1,3 @@
INSERT
OR REPLACE INTO tags (id, name, usn, config)
VALUES (?, ?, ?, ?)
OR REPLACE INTO tags (tag, usn, collapsed)
VALUES (?, ?, ?)

View File

@ -2,30 +2,23 @@
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
use super::SqliteStorage;
use crate::{
err::Result,
tags::{human_tag_name_to_native, native_tag_name_to_human, Tag, TagConfig, TagID},
timestamp::TimestampMillis,
types::Usn,
};
use prost::Message;
use crate::{err::Result, tags::Tag, types::Usn};
use rusqlite::{params, Row, NO_PARAMS};
use std::collections::HashMap;
fn row_to_tag(row: &Row) -> Result<Tag> {
let config = TagConfig::decode(row.get_raw(3).as_blob()?)?;
Ok(Tag {
id: row.get(0)?,
name: row.get(1)?,
usn: row.get(2)?,
config,
name: row.get(0)?,
usn: row.get(1)?,
collapsed: row.get(2)?,
})
}
impl SqliteStorage {
pub(crate) fn all_tags(&self) -> Result<Vec<Tag>> {
self.db
.prepare_cached("select id, name, usn, config from tags")?
.prepare_cached("select tag, usn, collapsed from tags")?
.query_and_then(NO_PARAMS, row_to_tag)?
.collect()
}
@ -33,54 +26,30 @@ impl SqliteStorage {
/// Get all tags in human form, sorted by name
pub(crate) fn all_tags_sorted(&self) -> Result<Vec<Tag>> {
self.db
.prepare_cached("select id, name, usn, config from tags order by name")?
.query_and_then(NO_PARAMS, |row| {
let mut tag = row_to_tag(row)?;
tag.name = native_tag_name_to_human(&tag.name);
Ok(tag)
})?
.prepare_cached("select tag, usn, collapsed from tags order by tag")?
.query_and_then(NO_PARAMS, row_to_tag)?
.collect()
}
/// Get tag by human name
pub(crate) fn get_tag(&self, name: &str) -> Result<Option<Tag>> {
self.db
.prepare_cached("select id, name, usn, config from tags where name = ?")?
.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)
})?
.prepare_cached("select tag, usn, collapsed from tags where tag = ?")?
.query_and_then(&[name], row_to_tag)?
.next()
.transpose()
}
fn alloc_id(&self) -> rusqlite::Result<TagID> {
self.db
.prepare_cached(include_str!("alloc_id.sql"))?
.query_row(&[TimestampMillis::now()], |r| r.get(0))
}
pub(crate) fn register_tag(&self, tag: &mut Tag) -> Result<()> {
let mut config = vec![];
tag.config.encode(&mut config)?;
tag.id = self.alloc_id()?;
self.update_tag(tag)?;
Ok(())
}
pub(crate) fn update_tag(&self, tag: &Tag) -> Result<()> {
let mut config = vec![];
tag.config.encode(&mut config)?;
pub(crate) fn register_tag(&self, tag: &Tag) -> Result<()> {
self.db
.prepare_cached(include_str!("add.sql"))?
.execute(params![tag.id, tag.name, tag.usn, config])?;
.execute(params![tag.name, tag.usn, tag.collapsed])?;
Ok(())
}
pub(crate) fn preferred_tag_case(&self, tag: &str) -> Result<Option<String>> {
self.db
.prepare_cached("select name from tags where name = ?")?
.prepare_cached("select tag from tags where tag = ?")?
.query_and_then(params![tag], |row| row.get(0))?
.next()
.transpose()
@ -89,8 +58,16 @@ impl SqliteStorage {
pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> {
self.db
.prepare_cached("delete from tags where name regexp ?")?
.execute(&[format!("^{}($|\x1f)", regex::escape(tag))])?;
.prepare_cached("delete from tags where tag regexp ?")?
.execute(&[format!("^{}($|::)", regex::escape(tag))])?;
Ok(())
}
pub(crate) fn set_tag_collapsed(&self, tag: &str, collapsed: bool) -> Result<()> {
self.db
.prepare_cached("update tags set collapsed = ? where tag = ?")?
.execute(params![collapsed, tag])?;
Ok(())
}
@ -111,7 +88,7 @@ impl SqliteStorage {
pub(crate) fn tags_pending_sync(&self, usn: Usn) -> Result<Vec<String>> {
self.db
.prepare_cached(&format!(
"select name from tags where {}",
"select tag from tags where {}",
usn.pending_object_clause()
))?
.query_and_then(&[usn], |r| r.get(0).map_err(Into::into))?
@ -121,7 +98,7 @@ impl SqliteStorage {
pub(crate) fn update_tag_usns(&self, tags: &[String], new_usn: Usn) -> Result<()> {
let mut stmt = self
.db
.prepare_cached("update tags set usn=? where name=?")?;
.prepare_cached("update tags set usn=? where tag=?")?;
for tag in tags {
stmt.execute(params![new_usn, tag])?;
}
@ -173,18 +150,7 @@ impl SqliteStorage {
.collect::<Result<Vec<Tag>>>()?;
self.db
.execute_batch(include_str!["../upgrades/schema17_upgrade.sql"])?;
tags.into_iter().try_for_each(|mut tag| -> Result<()> {
tag.name = human_tag_name_to_native(&tag.name);
self.register_tag(&mut tag)
})
}
pub(super) fn downgrade_tags_from_schema17(&self) -> Result<()> {
let tags = self.all_tags()?;
self.clear_tags()?;
tags.into_iter().try_for_each(|mut tag| -> Result<()> {
tag.name = native_tag_name_to_human(&tag.name);
self.register_tag(&mut tag)
})
tags.into_iter()
.try_for_each(|tag| -> Result<()> { self.register_tag(&tag) })
}
}

View File

@ -33,7 +33,6 @@ impl SqliteStorage {
}
if ver < 17 {
self.upgrade_tags_to_schema17()?;
self.upgrade_notes_to_schema17()?;
self.db.execute_batch("update col set ver = 17")?;
}
@ -47,9 +46,7 @@ impl SqliteStorage {
self.downgrade_decks_from_schema15()?;
self.downgrade_notetypes_from_schema15()?;
self.downgrade_config_from_schema14()?;
self.downgrade_tags_from_schema17()?;
self.downgrade_tags_from_schema14()?;
self.downgrade_notes_from_schema17()?;
self.db
.execute_batch(include_str!("schema11_downgrade.sql"))?;

View File

@ -1,7 +1,7 @@
DROP TABLE tags;
CREATE TABLE tags (
id integer PRIMARY KEY NOT NULL,
name text NOT NULL COLLATE unicase,
tag text NOT NULL PRIMARY KEY COLLATE unicase,
usn integer NOT NULL,
config blob NOT NULL
);
collapsed boolean NOT NULL,
config blob NULL
) without rowid;

View File

@ -1,11 +1,9 @@
// Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
pub use crate::backend_proto::TagConfig;
use crate::{
backend_proto::{Tag as TagProto, TagTreeNode},
collection::Collection,
define_newtype,
err::{AnkiError, Result},
notes::{NoteID, TransformNoteOutput},
text::{normalize_to_nfc, to_re},
@ -21,14 +19,11 @@ use std::{
};
use unicase::UniCase;
define_newtype!(TagID, i64);
#[derive(Debug, Clone)]
pub struct Tag {
pub id: TagID,
pub name: String,
pub usn: Usn,
pub config: TagConfig,
pub collapsed: bool,
}
impl Ord for Tag {
@ -54,10 +49,9 @@ impl Eq for Tag {}
impl Default for Tag {
fn default() -> Self {
Tag {
id: TagID(0),
name: "".to_string(),
usn: Usn(-1),
config: Default::default(),
collapsed: false,
}
}
}
@ -65,10 +59,9 @@ impl Default for Tag {
impl From<Tag> for TagProto {
fn from(t: Tag) -> Self {
TagProto {
id: t.id.0,
name: t.name,
usn: t.usn.0,
config: Some(t.config),
collapsed: t.collapsed,
}
}
}
@ -76,10 +69,9 @@ impl From<Tag> for TagProto {
impl From<TagProto> for Tag {
fn from(t: TagProto) -> Self {
Tag {
id: TagID(t.id),
name: t.name,
usn: Usn(t.usn),
config: t.config.unwrap(),
collapsed: t.collapsed,
}
}
}
@ -119,17 +111,13 @@ fn normalized_tag_name_component(comp: &str) -> Cow<str> {
}
}
pub(crate) fn human_tag_name_to_native(name: &str) -> String {
fn normalize_tag_name(name: &str) -> String {
let mut out = String::with_capacity(name.len());
for comp in name.split("::") {
out.push_str(&normalized_tag_name_component(comp));
out.push('\x1f');
out.push_str("::");
}
out.trim_end_matches('\x1f').into()
}
pub(crate) fn native_tag_name_to_human(name: &str) -> String {
name.replace('\x1f', "::")
out.trim_end_matches("::").into()
}
fn fill_missing_tags(tags: Vec<Tag>) -> Vec<Tag> {
@ -177,11 +165,10 @@ fn add_child_nodes(tags: &mut Peekable<impl Iterator<Item = Tag>>, parent: &mut
l if l == parent.level + 1 => {
// next item is an immediate descendent of parent
parent.children.push(TagTreeNode {
tag_id: tag.id.0,
name: (*split_name.last().unwrap()).into(),
children: vec![],
level: parent.level + 1,
collapsed: tag.config.browser_collapsed,
collapsed: tag.collapsed,
});
tags.next();
}
@ -199,19 +186,6 @@ fn add_child_nodes(tags: &mut Peekable<impl Iterator<Item = Tag>>, parent: &mut
}
impl Collection {
pub fn all_tags(&self) -> Result<Vec<Tag>> {
self.storage
.all_tags()?
.into_iter()
.map(|t| {
Ok(Tag {
name: native_tag_name_to_human(&t.name),
..t
})
})
.collect()
}
pub fn tag_tree(&mut self) -> Result<TagTreeNode> {
let tags = self.storage.all_tags_sorted()?;
let tree = tags_to_tree(tags);
@ -253,20 +227,19 @@ impl Collection {
}
pub(crate) fn register_tag(&self, tag: Tag) -> Result<(Tag, bool)> {
let native_name = human_tag_name_to_native(&tag.name);
let normalized_name = normalize_tag_name(&tag.name);
let mut t = Tag {
name: native_name.clone(),
name: normalized_name.clone(),
..tag
};
if native_name.is_empty() {
if normalized_name.is_empty() {
return Ok((t, false));
}
if let Some(preferred) = self.storage.preferred_tag_case(&native_name)? {
t.name = native_tag_name_to_human(&preferred);
if let Some(preferred) = self.storage.preferred_tag_case(&normalized_name)? {
t.name = preferred;
Ok((t, false))
} else {
self.storage.register_tag(&mut t)?;
t.name = native_tag_name_to_human(&t.name);
self.storage.register_tag(&t)?;
Ok((t, true))
}
}
@ -287,19 +260,16 @@ impl Collection {
Ok(changed)
}
pub(crate) fn update_tag(&self, tag: &Tag) -> Result<()> {
let native_name = human_tag_name_to_native(&tag.name);
self.storage.update_tag(&Tag {
id: tag.id,
name: native_name,
usn: tag.usn,
config: tag.config.clone(),
})
}
pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> {
let native_name = human_tag_name_to_native(tag);
self.storage.clear_tag(&native_name)
pub(crate) fn set_tag_collapsed(&self, name: &str, collapsed: bool) -> Result<()> {
if self.storage.get_tag(name)?.is_none() {
// tag is missing, register it
let t = Tag {
name: name.to_owned(),
..Default::default()
};
self.register_tag(t)?;
}
self.storage.set_tag_collapsed(name, collapsed)
}
fn replace_tags_for_notes_inner<R: Replacer>(
@ -425,11 +395,6 @@ mod test {
col.update_note(&mut note)?;
assert_eq!(&note.tags, &["one", "two"]);
// note.tags is in human form
note.tags = vec!["foo::bar".into()];
col.update_note(&mut note)?;
assert_eq!(&note.tags, &["foo::bar"]);
Ok(())
}
@ -483,6 +448,11 @@ mod test {
let note = col.storage.get_note(note.id)?.unwrap();
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_tag("a")?;
assert_eq!(col.storage.all_tags()?, vec![]);
Ok(())
}
}