From 20bd207f00a4c58d0ffd3262d3c684a17b37ce40 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Sat, 17 Apr 2021 22:47:04 +0200 Subject: [PATCH 1/6] Give deck.name the newtype NativeDeckName The deck name must be constructed by calling associated functions of NativeDeckName, unless the name is guaranteed to be valid machine name (like "Default"). NativeDeckName exposes methods to mutate the deck name and return the human name. The storage routines take &strs, but those should be slices of NativeDeckNames to ensure machine form and normalization. --- rslib/src/backend/decks.rs | 10 +- rslib/src/dbcheck.rs | 2 +- rslib/src/decks/add.rs | 29 ++--- rslib/src/decks/filtered.rs | 2 +- rslib/src/decks/mod.rs | 37 +++--- rslib/src/decks/name.rs | 185 ++++++++++++++-------------- rslib/src/decks/remove.rs | 2 +- rslib/src/decks/reparent.rs | 5 +- rslib/src/decks/schema11.rs | 12 +- rslib/src/prelude.rs | 2 +- rslib/src/scheduler/filtered/mod.rs | 11 +- rslib/src/search/sqlwriter.rs | 5 +- rslib/src/storage/deck/mod.rs | 25 ++-- rslib/src/sync/mod.rs | 2 +- 14 files changed, 158 insertions(+), 171 deletions(-) diff --git a/rslib/src/backend/decks.rs b/rslib/src/backend/decks.rs index 578d6b558..5f88279af 100644 --- a/rslib/src/backend/decks.rs +++ b/rslib/src/backend/decks.rs @@ -6,10 +6,7 @@ use std::convert::TryFrom; use super::Backend; use crate::{ backend_proto::{self as pb}, - decks::{ - human_deck_name_to_native, native_deck_name_to_human, Deck, DeckId, DeckSchema11, - FilteredSearchOrder, - }, + decks::{DeckSchema11, FilteredSearchOrder}, prelude::*, scheduler::filtered::FilteredDeckForUpdate, }; @@ -89,7 +86,6 @@ impl DecksService for Backend { .storage .get_deck(input.into())? .ok_or(AnkiError::NotFound)? - .with_human_name() .into()) }) } @@ -241,7 +237,7 @@ impl From for pb::Deck { fn from(d: Deck) -> Self { pb::Deck { id: d.id.0, - name: native_deck_name_to_human(&d.name), + name: d.name.human_name(), mtime_secs: d.mtime_secs.0, usn: d.usn.0, common: Some(d.common), @@ -256,7 +252,7 @@ impl TryFrom for Deck { fn try_from(d: pb::Deck) -> Result { Ok(Deck { id: DeckId(d.id), - name: human_deck_name_to_native(&d.name), + name: NativeDeckName::from_human_name(&d.name), mtime_secs: TimestampSecs(d.mtime_secs), usn: Usn(d.usn), common: d.common.unwrap_or_default(), diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index 98ff36c4d..5f99075b3 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -430,7 +430,7 @@ mod test { } ); assert_eq!( - col.storage.get_deck(DeckId(123))?.unwrap().name, + col.storage.get_deck(DeckId(123))?.unwrap().name.as_str(), "recovered123" ); diff --git a/rslib/src/decks/add.rs b/rslib/src/decks/add.rs index bf30c9c35..ed4d2ce0e 100644 --- a/rslib/src/decks/add.rs +++ b/rslib/src/decks/add.rs @@ -1,17 +1,12 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use super::name::{immediate_parent_name, normalize_native_name}; +use super::name::immediate_parent_name; use crate::{error::FilteredDeckError, prelude::*}; -use std::borrow::Cow; impl Collection { - /// Normalize deck name and rename if not unique. Bumps mtime and usn if + /// Rename deck if not unique. Bumps mtime and usn if /// name was changed, but otherwise leaves it the same. pub(super) fn prepare_deck_for_update(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> { - if let Cow::Owned(name) = normalize_native_name(&deck.name) { - deck.name = name; - deck.set_modified(usn); - } self.ensure_deck_name_unique(deck, usn) } @@ -69,7 +64,7 @@ impl Collection { if name_changed { // after updating, we need to ensure all grandparents exist, which may not be the case // in the parent->child case - self.create_missing_parents(&deck.name, usn)?; + self.create_missing_parents(deck.name.as_str(), usn)?; } Ok(()) } @@ -89,7 +84,7 @@ impl Collection { pub(crate) fn recover_missing_deck(&mut self, did: DeckId, usn: Usn) -> Result<()> { let mut deck = Deck::new_normal(); deck.id = did; - deck.name = format!("recovered{}", did); + deck.name = NativeDeckName(format!("recovered{}", did)); deck.set_modified(usn); self.add_or_update_single_deck_with_existing_id(&mut deck, usn) } @@ -100,7 +95,7 @@ impl Collection { /// Caller must have done necessarily validation on name. fn add_parent_deck(&mut self, machine_name: &str, usn: Usn) -> Result<()> { let mut deck = Deck::new_normal(); - deck.name = machine_name.into(); + deck.name = NativeDeckName(machine_name.into()); deck.set_modified(usn); self.add_deck_undoable(&mut deck) } @@ -109,20 +104,20 @@ impl Collection { /// If they don't exist, create them. /// Returns an error if a DB operation fails, or if the first existing parent is a filtered deck. fn match_or_create_parents(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> { - let child_split: Vec<_> = deck.name.split('\x1f').collect(); - if let Some(parent_deck) = self.first_existing_parent(&deck.name, 0)? { + let child_split: Vec<_> = deck.name.components().collect(); + if let Some(parent_deck) = self.first_existing_parent(deck.name.as_str(), 0)? { if parent_deck.is_filtered() { return Err(FilteredDeckError::MustBeLeafNode.into()); } - let parent_count = parent_deck.name.matches('\x1f').count() + 1; + let parent_count = parent_deck.name.components().count(); let need_create = parent_count != child_split.len() - 1; - deck.name = format!( + deck.name = NativeDeckName(format!( "{}\x1f{}", parent_deck.name, &child_split[parent_count..].join("\x1f") - ); + )); if need_create { - self.create_missing_parents(&deck.name, usn)?; + self.create_missing_parents(deck.name.as_str(), usn)?; } Ok(()) } else if child_split.len() == 1 { @@ -130,7 +125,7 @@ impl Collection { Ok(()) } else { // no existing parents - self.create_missing_parents(&deck.name, usn) + self.create_missing_parents(deck.name.as_str(), usn) } } diff --git a/rslib/src/decks/filtered.rs b/rslib/src/decks/filtered.rs index 84c065dca..e1a676328 100644 --- a/rslib/src/decks/filtered.rs +++ b/rslib/src/decks/filtered.rs @@ -23,7 +23,7 @@ impl Deck { filt.reschedule = true; Deck { id: DeckId(0), - name: "".into(), + name: NativeDeckName("".into()), mtime_secs: TimestampSecs(0), usn: Usn(0), common: DeckCommon { diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 557ab0b39..faf115962 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -25,9 +25,8 @@ use crate::{ text::sanitize_html_no_images, }; pub(crate) use counts::DueCounts; -pub(crate) use name::{ - human_deck_name_to_native, immediate_parent_name, native_deck_name_to_human, -}; +pub(crate) use name::immediate_parent_name; +pub use name::NativeDeckName; pub use schema11::DeckSchema11; use std::sync::Arc; @@ -36,7 +35,7 @@ define_newtype!(DeckId, i64); #[derive(Debug, Clone, PartialEq)] pub struct Deck { pub id: DeckId, - pub name: String, + pub name: NativeDeckName, pub mtime_secs: TimestampSecs, pub usn: Usn, pub common: DeckCommon, @@ -47,7 +46,7 @@ impl Deck { pub fn new_normal() -> Deck { Deck { id: DeckId(0), - name: "".into(), + name: NativeDeckName("".into()), mtime_secs: TimestampSecs(0), usn: Usn(0), common: DeckCommon { @@ -150,12 +149,12 @@ impl Collection { } pub fn get_or_create_normal_deck(&mut self, human_name: &str) -> Result { - let native_name = human_deck_name_to_native(human_name); - if let Some(did) = self.storage.get_deck_id(&native_name)? { + let name = NativeDeckName::from_human_name(human_name); + if let Some(did) = self.storage.get_deck_id(name.as_str())? { self.storage.get_deck(did).map(|opt| opt.unwrap()) } else { let mut deck = Deck::new_normal(); - deck.name = native_name; + deck.name = name; self.add_or_update_deck(&mut deck)?; Ok(deck) } @@ -164,18 +163,14 @@ impl Collection { /// Get a deck based on its human name. If you have a machine name, /// use the method in storage instead. pub(crate) fn get_deck_id(&self, human_name: &str) -> Result> { - let machine_name = human_deck_name_to_native(&human_name); - self.storage.get_deck_id(&machine_name) + self.storage + .get_deck_id(NativeDeckName::from_human_name(human_name).as_str()) } } #[cfg(test)] mod test { - use crate::{ - collection::{open_test_collection, Collection}, - error::Result, - search::SortMode, - }; + use crate::{collection::open_test_collection, prelude::*, search::SortMode}; fn sorted_names(col: &Collection) -> Vec { col.storage @@ -212,7 +207,7 @@ mod test { let _ = col.get_or_create_normal_deck("foo::bar::baz")?; let mut top_deck = col.get_or_create_normal_deck("foo")?; - top_deck.name = "other".into(); + top_deck.name = NativeDeckName("other".into()); col.add_or_update_deck(&mut top_deck)?; assert_eq!( sorted_names(&col), @@ -221,7 +216,7 @@ mod test { // should do the right thing in the middle of the tree as well let mut middle = col.get_or_create_normal_deck("other::bar")?; - middle.name = "quux\x1ffoo".into(); + middle.name = NativeDeckName("quux\x1ffoo".into()); col.add_or_update_deck(&mut middle)?; assert_eq!( sorted_names(&col), @@ -234,7 +229,7 @@ mod test { // quux::foo -> quux::foo::baz::four // means quux::foo::baz2 should be quux::foo::baz::four::baz2 // and a new quux::foo should have been created - middle.name = "quux\x1ffoo\x1fbaz\x1ffour".into(); + middle.name = NativeDeckName("quux\x1ffoo\x1fbaz\x1ffour".into()); col.add_or_update_deck(&mut middle)?; assert_eq!( sorted_names(&col), @@ -251,9 +246,9 @@ mod test { ); // should handle name conflicts - middle.name = "other".into(); + middle.name = NativeDeckName("other".into()); col.add_or_update_deck(&mut middle)?; - assert_eq!(middle.name, "other+"); + assert_eq!(middle.name.as_str(), "other+"); // public function takes human name col.rename_deck(middle.id, "one::two")?; @@ -282,7 +277,7 @@ mod test { let mut col = open_test_collection(); let mut default = col.get_or_create_normal_deck("default")?; - default.name = "one\x1ftwo".into(); + default.name = NativeDeckName("one\x1ftwo".into()); col.add_or_update_deck(&mut default)?; // create a non-default deck confusingly named "default" diff --git a/rslib/src/decks/name.rs b/rslib/src/decks/name.rs index ff0dc7294..603bbe64c 100644 --- a/rslib/src/decks/name.rs +++ b/rslib/src/decks/name.rs @@ -1,17 +1,74 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html use crate::{prelude::*, text::normalize_to_nfc}; +use itertools::Itertools; use std::borrow::Cow; +#[derive(Debug, Clone, PartialEq)] +pub struct NativeDeckName(pub String); + +impl NativeDeckName { + pub fn from_human_name(name: &str) -> Self { + NativeDeckName( + name.split("::") + .map(normalized_deck_name_component) + .join("\x1f"), + ) + } + + pub fn human_name(&self) -> String { + self.0.replace('\x1f', "::") + } + + pub(crate) fn add_suffix(&mut self, suffix: &str) { + self.0 += suffix + } + + pub(crate) fn as_str(&self) -> &str { + &self.0 + } + + pub(crate) fn components(&self) -> std::str::Split<'_, char> { + self.0.split('\x1f') + } + + /// Determine name to rename a deck to, when `self` is dropped on `target`. + /// `target` being unset represents a drop at the top or bottom of the deck list. + /// The returned name should be used to replace `self`. + pub(crate) fn reparented_name(&self, target: Option<&NativeDeckName>) -> Option { + let dragged_base = self.0.rsplit('\x1f').next().unwrap(); + if let Some(target) = target { + if target.0.starts_with(&self.0) { + // foo onto foo::bar, or foo onto itself -> no-op + None + } else { + // foo::bar onto baz -> baz::bar + Some(NativeDeckName(format!("{}\x1f{}", target.0, dragged_base))) + } + } else { + // foo::bar onto top level -> bar + Some(NativeDeckName(dragged_base.into())) + } + } + + /// Replace the old parent's name with the new parent's name in self's name, where the old + /// parent's name is expected to be a prefix. + fn reparent(&mut self, old_parent: &NativeDeckName, new_parent: &NativeDeckName) { + self.0 = std::iter::once(new_parent.as_str()) + .chain(self.components().skip(old_parent.components().count())) + .join("\x1f") + } +} + +impl std::fmt::Display for NativeDeckName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + impl Deck { pub fn human_name(&self) -> String { - self.name.replace("\x1f", "::") - } - - // Mutate name to human representation for sharing. - pub fn with_human_name(mut self) -> Self { - self.name = self.human_name(); - self + self.name.human_name() } } @@ -32,7 +89,7 @@ impl Collection { self.transact(Op::RenameDeck, |col| { let existing_deck = col.storage.get_deck(did)?.ok_or(AnkiError::NotFound)?; let mut deck = existing_deck.clone(); - deck.name = human_deck_name_to_native(new_human_name); + deck.name = NativeDeckName::from_human_name(new_human_name); col.update_deck_inner(&mut deck, existing_deck, col.usn()?) }) } @@ -40,18 +97,13 @@ impl Collection { pub(super) fn rename_child_decks( &mut self, old: &Deck, - new_name: &str, + new_name: &NativeDeckName, usn: Usn, ) -> Result<()> { let children = self.storage.child_decks(old)?; - let old_component_count = old.name.matches('\x1f').count() + 1; - for mut child in children { let original = child.clone(); - let child_components: Vec<_> = child.name.split('\x1f').collect(); - let child_only = &child_components[old_component_count..]; - let new_name = format!("{}\x1f{}", new_name, child_only.join("\x1f")); - child.name = new_name; + child.name.reparent(&old.name, new_name); child.set_modified(usn); self.update_single_deck_undoable(&mut child, original)?; } @@ -61,14 +113,12 @@ impl Collection { pub(crate) fn ensure_deck_name_unique(&self, deck: &mut Deck, usn: Usn) -> Result<()> { loop { - match self.storage.get_deck_id(&deck.name)? { - Some(did) if did == deck.id => { - break; - } + match self.storage.get_deck_id(deck.name.as_str())? { + Some(did) if did == deck.id => break, None => break, _ => (), } - deck.name += "+"; + deck.name.add_suffix("+"); deck.set_modified(usn); } @@ -108,59 +158,10 @@ fn normalized_deck_name_component(comp: &str) -> Cow { } } -pub(super) fn normalize_native_name(name: &str) -> Cow { - if name - .split('\x1f') - .any(|comp| matches!(normalized_deck_name_component(comp), Cow::Owned(_))) - { - let comps: Vec<_> = name - .split('\x1f') - .map(normalized_deck_name_component) - .collect::>(); - comps.join("\x1f").into() - } else { - // no changes required - name.into() - } -} - -pub(crate) fn human_deck_name_to_native(name: &str) -> String { - let mut out = String::with_capacity(name.len()); - for comp in name.split("::") { - out.push_str(&normalized_deck_name_component(comp)); - out.push('\x1f'); - } - out.trim_end_matches('\x1f').into() -} - -pub(crate) fn native_deck_name_to_human(name: &str) -> String { - name.replace('\x1f', "::") -} - pub(crate) fn immediate_parent_name(machine_name: &str) -> Option<&str> { machine_name.rsplitn(2, '\x1f').nth(1) } -/// Determine name to rename a deck to, when `dragged` is dropped on `dropped`. -/// `dropped` being unset represents a drop at the top or bottom of the deck list. -/// The returned name should be used to rename `dragged`. -/// Arguments are expected in 'machine' form with an \x1f separator. -pub(crate) fn reparented_name(dragged: &str, dropped: Option<&str>) -> Option { - let dragged_base = dragged.rsplit('\x1f').next().unwrap(); - if let Some(dropped) = dropped { - if dropped.starts_with(dragged) { - // foo onto foo::bar, or foo onto itself -> no-op - None - } else { - // foo::bar onto baz -> baz::bar - Some(format!("{}\x1f{}", dropped, dragged_base)) - } - } else { - // foo::bar onto top level -> bar - Some(dragged_base.into()) - } -} - #[cfg(test)] mod test { use super::*; @@ -177,54 +178,52 @@ mod test { #[test] fn from_human() { - assert_eq!(&human_deck_name_to_native("foo"), "foo"); - assert_eq!(&human_deck_name_to_native("foo::bar"), "foo\x1fbar"); - assert_eq!(&human_deck_name_to_native("fo\x1fo::ba\nr"), "foo\x1fbar"); - assert_eq!( - &human_deck_name_to_native("foo::::baz"), - "foo\x1fblank\x1fbaz" - ); - } + fn native_name(name: &str) -> String { + NativeDeckName::from_human_name(name).0 + } - #[test] - fn normalize() { - assert_eq!(&normalize_native_name("foo\x1fbar"), "foo\x1fbar"); - assert_eq!(&normalize_native_name("fo\u{a}o\x1fbar"), "foo\x1fbar"); + assert_eq!(native_name("foo"), "foo"); + assert_eq!(native_name("foo::bar"), "foo\x1fbar"); + assert_eq!(native_name("foo::::baz"), "foo\x1fblank\x1fbaz"); + // normalize + assert_eq!(native_name("fo\x1fo::ba\nr"), "foo\x1fbar"); + assert_eq!(native_name("fo\u{a}o\x1fbar"), "foobar"); } #[test] fn drag_drop() { // use custom separator to make the tests easier to read - fn n(s: &str) -> String { - s.replace(":", "\x1f") + fn n(s: &str) -> NativeDeckName { + NativeDeckName(s.replace(":", "\x1f")) } #[allow(clippy::unnecessary_wraps)] - fn n_opt(s: &str) -> Option { + fn n_opt(s: &str) -> Option { Some(n(s)) } + fn reparented_name(drag: &str, drop: Option<&str>) -> Option { + n(drag).reparented_name(drop.map(n).as_ref()) + } + assert_eq!(reparented_name("drag", Some("drop")), n_opt("drop:drag")); assert_eq!(reparented_name("drag", None), n_opt("drag")); - assert_eq!(reparented_name(&n("drag:child"), None), n_opt("child")); + assert_eq!(reparented_name("drag:child", None), n_opt("child")); assert_eq!( - reparented_name(&n("drag:child"), Some(&n("drop:deck"))), + reparented_name("drag:child", Some("drop:deck")), n_opt("drop:deck:child") ); assert_eq!( - reparented_name(&n("drag:child"), Some("drag")), + reparented_name("drag:child", Some("drag")), n_opt("drag:child") ); assert_eq!( - reparented_name(&n("drag:child:grandchild"), Some("drag")), + reparented_name("drag:child:grandchild", Some("drag")), n_opt("drag:grandchild") ); // drops to child not supported - assert_eq!( - reparented_name(&n("drag"), Some(&n("drag:child:grandchild"))), - None - ); + assert_eq!(reparented_name("drag", Some("drag:child:grandchild")), None); // name doesn't change when deck dropped on itself - assert_eq!(reparented_name(&n("foo:bar"), Some(&n("foo:bar"))), None); + assert_eq!(reparented_name("foo:bar", Some("foo:bar")), None); } } diff --git a/rslib/src/decks/remove.rs b/rslib/src/decks/remove.rs index 997100dfa..37b4ab627 100644 --- a/rslib/src/decks/remove.rs +++ b/rslib/src/decks/remove.rs @@ -37,7 +37,7 @@ impl Collection { // if the default deck is included, just ensure it's reset to the default // name, as we've already removed its cards let mut modified_default = deck.clone(); - modified_default.name = self.tr.deck_config_default_name().into(); + modified_default.name = NativeDeckName(self.tr.deck_config_default_name().into()); self.prepare_deck_for_update(&mut modified_default, usn)?; modified_default.set_modified(usn); self.update_single_deck_undoable(&mut modified_default, deck.clone())?; diff --git a/rslib/src/decks/reparent.rs b/rslib/src/decks/reparent.rs index 97186953c..15de58cbb 100644 --- a/rslib/src/decks/reparent.rs +++ b/rslib/src/decks/reparent.rs @@ -1,6 +1,5 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use super::name::reparented_name; use crate::{error::FilteredDeckError, prelude::*}; impl Collection { @@ -28,14 +27,14 @@ impl Collection { return Err(FilteredDeckError::MustBeLeafNode.into()); } target_deck = target; - target_name = Some(target_deck.name.as_str()); + target_name = Some(&target_deck.name); } } let mut count = 0; for deck in deck_ids { if let Some(mut deck) = self.storage.get_deck(*deck)? { - if let Some(new_name) = reparented_name(&deck.name, target_name) { + if let Some(new_name) = deck.name.reparented_name(target_name) { count += 1; let orig = deck.clone(); diff --git a/rslib/src/decks/schema11.rs b/rslib/src/decks/schema11.rs index bf08625b0..23b873a27 100644 --- a/rslib/src/decks/schema11.rs +++ b/rslib/src/decks/schema11.rs @@ -1,11 +1,7 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use super::DeckId; -use super::{ - human_deck_name_to_native, native_deck_name_to_human, DeckCommon, FilteredDeck, - FilteredSearchTerm, NormalDeck, -}; +use super::{DeckCommon, FilteredDeck, FilteredSearchTerm, NormalDeck}; use crate::{ prelude::*, serde::{default_on_invalid, deserialize_bool_from_anything, deserialize_number_from_string}, @@ -239,7 +235,7 @@ impl From for Deck { match deck { DeckSchema11::Normal(d) => Deck { id: d.common.id, - name: human_deck_name_to_native(&d.common.name), + name: NativeDeckName::from_human_name(&d.common.name), mtime_secs: d.common.mtime, usn: d.common.usn, common: (&d.common).into(), @@ -247,7 +243,7 @@ impl From for Deck { }, DeckSchema11::Filtered(d) => Deck { id: d.common.id, - name: human_deck_name_to_native(&d.common.name), + name: NativeDeckName::from_human_name(&d.common.name), mtime_secs: d.common.mtime, usn: d.common.usn, common: (&d.common).into(), @@ -362,7 +358,7 @@ impl From for DeckCommonSchema11 { DeckCommonSchema11 { id: deck.id, mtime: deck.mtime_secs, - name: native_deck_name_to_human(&deck.name), + name: deck.human_name(), usn: deck.usn, today: (&deck).into(), study_collapsed: deck.common.study_collapsed, diff --git a/rslib/src/prelude.rs b/rslib/src/prelude.rs index 544c5dac3..53acf131b 100644 --- a/rslib/src/prelude.rs +++ b/rslib/src/prelude.rs @@ -7,7 +7,7 @@ pub use crate::{ collection::Collection, config::BoolKey, deckconf::{DeckConf, DeckConfId}, - decks::{Deck, DeckId, DeckKind}, + decks::{Deck, DeckId, DeckKind, NativeDeckName}, error::{AnkiError, Result}, i18n::I18n, notes::{Note, NoteId}, diff --git a/rslib/src/scheduler/filtered/mod.rs b/rslib/src/scheduler/filtered/mod.rs index 2bcf709e3..153ec7cca 100644 --- a/rslib/src/scheduler/filtered/mod.rs +++ b/rslib/src/scheduler/filtered/mod.rs @@ -7,7 +7,7 @@ use std::convert::{TryFrom, TryInto}; use crate::{ config::ConfigKey, - decks::{human_deck_name_to_native, FilteredDeck, FilteredSearchTerm}, + decks::{FilteredDeck, FilteredSearchTerm}, error::FilteredDeckError, search::writer::{deck_search, normalize_search}, }; @@ -146,8 +146,11 @@ impl Collection { Ok(position) } - fn get_next_filtered_deck_name(&self) -> String { - format!("Filtered Deck {}", TimestampSecs::now().time_string()) + fn get_next_filtered_deck_name(&self) -> NativeDeckName { + NativeDeckName(format!( + "Filtered Deck {}", + TimestampSecs::now().time_string() + )) } fn add_or_update_filtered_deck_inner( @@ -253,6 +256,6 @@ impl TryFrom for FilteredDeckForUpdate { fn apply_update_to_filtered_deck(deck: &mut Deck, update: FilteredDeckForUpdate) { deck.id = update.id; - deck.name = human_deck_name_to_native(&update.human_name); + deck.name = NativeDeckName::from_human_name(&update.human_name); deck.kind = DeckKind::Filtered(update.config); } diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index 652adc0c7..ab133409d 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -8,7 +8,6 @@ use super::{ use crate::{ card::{CardQueue, CardType}, collection::Collection, - decks::human_deck_name_to_native, error::Result, notes::field_checksum, notetype::NotetypeId, @@ -347,11 +346,11 @@ impl SqlWriter<'_> { .storage .get_deck(current_did)? .map(|d| d.name) - .unwrap_or_else(|| "Default".into()) + .unwrap_or_else(|| NativeDeckName("Default".into())) .as_str(), ) } else { - human_deck_name_to_native(&to_re(deck)) + NativeDeckName::from_human_name(&to_re(deck)).0 }; // convert to a regex that includes child decks diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 2853e903d..1bbe43447 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -10,6 +10,7 @@ use crate::{ decks::{Deck, DeckCommon, DeckId, DeckKindContainer, DeckSchema11, DueCounts}, error::{AnkiError, DbErrorKind, Result}, i18n::I18n, + prelude::*, timestamp::TimestampMillis, }; use prost::Message; @@ -23,7 +24,7 @@ fn row_to_deck(row: &Row) -> Result { let id = row.get(0)?; Ok(Deck { id, - name: row.get(1)?, + name: NativeDeckName(row.get(1)?), mtime_secs: row.get(2)?, usn: row.get(3)?, common, @@ -123,7 +124,7 @@ impl SqliteStorage { let mut kind = vec![]; kind_enum.encode(&mut kind)?; let count = stmt.execute(params![ - deck.name, + deck.name.as_str(), deck.mtime_secs, deck.usn, common, @@ -158,7 +159,7 @@ impl SqliteStorage { kind_enum.encode(&mut kind)?; stmt.execute(params![ deck.id, - deck.name, + deck.name.as_str(), deck.mtime_secs, deck.usn, common, @@ -228,9 +229,13 @@ impl SqliteStorage { /// Return the parents of `child`, with the most immediate parent coming first. pub(crate) fn parent_decks(&self, child: &Deck) -> Result> { let mut decks: Vec = vec![]; - while let Some(parent_name) = - immediate_parent_name(decks.last().map(|d| &d.name).unwrap_or_else(|| &child.name)) - { + while let Some(parent_name) = immediate_parent_name( + decks + .last() + .map(|d| &d.name) + .unwrap_or_else(|| &child.name) + .as_str(), + ) { if let Some(parent_did) = self.get_deck_id(parent_name)? { let parent = self.get_deck(parent_did)?.unwrap(); decks.push(parent); @@ -324,7 +329,7 @@ impl SqliteStorage { "create temporary table active_decks (id integer primary key not null);" ))?; - let top = ¤t.name; + let top = current.name.as_str(); let prefix_start = &format!("{}\x1f", top); let prefix_end = &format!("{}\x20", top); @@ -341,7 +346,7 @@ impl SqliteStorage { let mut deck = Deck::new_normal(); deck.id.0 = 1; // fixme: separate key - deck.name = tr.deck_config_default_name().into(); + deck.name = NativeDeckName(tr.deck_config_default_name().into()); self.add_or_update_deck_with_existing_id(&deck) } @@ -356,12 +361,12 @@ impl SqliteStorage { deck.set_modified(usn); } loop { - let name = UniCase::new(deck.name.clone()); + let name = UniCase::new(deck.name.0.clone()); if !names.contains(&name) { names.insert(name); break; } - deck.name.push('_'); + deck.name.add_suffix("_"); deck.set_modified(usn); } self.add_or_update_deck_with_existing_id(&deck)?; diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index 6634aa2bc..d0076d16e 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -1500,7 +1500,7 @@ mod test { })?; let mut deck = col2.storage.get_deck(deck.id)?.unwrap(); - deck.name = "newer".into(); + deck.name = NativeDeckName("newer".into()); col2.add_or_update_deck(&mut deck)?; let mut nt = col2.storage.get_notetype(nt.id)?.unwrap(); From f9245395745e788912dd69a56b487a4e0630ee47 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 18 Apr 2021 09:20:23 +1000 Subject: [PATCH 2/6] create_missing_parents() can take a native name directly --- rslib/src/decks/add.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rslib/src/decks/add.rs b/rslib/src/decks/add.rs index ed4d2ce0e..2fefb383c 100644 --- a/rslib/src/decks/add.rs +++ b/rslib/src/decks/add.rs @@ -64,7 +64,7 @@ impl Collection { if name_changed { // after updating, we need to ensure all grandparents exist, which may not be the case // in the parent->child case - self.create_missing_parents(deck.name.as_str(), usn)?; + self.create_missing_parents(&deck.name, usn)?; } Ok(()) } @@ -117,7 +117,7 @@ impl Collection { &child_split[parent_count..].join("\x1f") )); if need_create { - self.create_missing_parents(deck.name.as_str(), usn)?; + self.create_missing_parents(&deck.name, usn)?; } Ok(()) } else if child_split.len() == 1 { @@ -125,11 +125,12 @@ impl Collection { Ok(()) } else { // no existing parents - self.create_missing_parents(deck.name.as_str(), usn) + self.create_missing_parents(&deck.name, usn) } } - fn create_missing_parents(&mut self, mut machine_name: &str, usn: Usn) -> Result<()> { + fn create_missing_parents(&mut self, name: &NativeDeckName, usn: Usn) -> Result<()> { + let mut machine_name = name.as_str(); while let Some(parent_name) = immediate_parent_name(machine_name) { if self.storage.get_deck_id(parent_name)?.is_none() { self.add_parent_deck(parent_name, usn)?; From 1acc679e8f540f54aa2bdfca2ccce625255bf39f Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 18 Apr 2021 09:29:35 +1000 Subject: [PATCH 3/6] hide NativeName inner value, and require explicit accessors --- rslib/src/decks/add.rs | 6 +++--- rslib/src/decks/filtered.rs | 2 +- rslib/src/decks/mod.rs | 12 ++++++------ rslib/src/decks/name.rs | 6 +++++- rslib/src/decks/remove.rs | 3 ++- rslib/src/scheduler/filtered/mod.rs | 2 +- rslib/src/search/sqlwriter.rs | 6 ++++-- rslib/src/storage/deck/mod.rs | 6 +++--- rslib/src/sync/mod.rs | 2 +- 9 files changed, 26 insertions(+), 19 deletions(-) diff --git a/rslib/src/decks/add.rs b/rslib/src/decks/add.rs index 2fefb383c..6047046f1 100644 --- a/rslib/src/decks/add.rs +++ b/rslib/src/decks/add.rs @@ -84,7 +84,7 @@ impl Collection { pub(crate) fn recover_missing_deck(&mut self, did: DeckId, usn: Usn) -> Result<()> { let mut deck = Deck::new_normal(); deck.id = did; - deck.name = NativeDeckName(format!("recovered{}", did)); + deck.name = NativeDeckName::from_native_str(format!("recovered{}", did)); deck.set_modified(usn); self.add_or_update_single_deck_with_existing_id(&mut deck, usn) } @@ -95,7 +95,7 @@ impl Collection { /// Caller must have done necessarily validation on name. fn add_parent_deck(&mut self, machine_name: &str, usn: Usn) -> Result<()> { let mut deck = Deck::new_normal(); - deck.name = NativeDeckName(machine_name.into()); + deck.name = NativeDeckName::from_native_str(machine_name); deck.set_modified(usn); self.add_deck_undoable(&mut deck) } @@ -111,7 +111,7 @@ impl Collection { } let parent_count = parent_deck.name.components().count(); let need_create = parent_count != child_split.len() - 1; - deck.name = NativeDeckName(format!( + deck.name = NativeDeckName::from_native_str(format!( "{}\x1f{}", parent_deck.name, &child_split[parent_count..].join("\x1f") diff --git a/rslib/src/decks/filtered.rs b/rslib/src/decks/filtered.rs index e1a676328..ed797eb6a 100644 --- a/rslib/src/decks/filtered.rs +++ b/rslib/src/decks/filtered.rs @@ -23,7 +23,7 @@ impl Deck { filt.reschedule = true; Deck { id: DeckId(0), - name: NativeDeckName("".into()), + name: NativeDeckName::from_native_str(""), mtime_secs: TimestampSecs(0), usn: Usn(0), common: DeckCommon { diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index faf115962..f47cd3ca7 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -46,7 +46,7 @@ impl Deck { pub fn new_normal() -> Deck { Deck { id: DeckId(0), - name: NativeDeckName("".into()), + name: NativeDeckName::from_native_str(""), mtime_secs: TimestampSecs(0), usn: Usn(0), common: DeckCommon { @@ -207,7 +207,7 @@ mod test { let _ = col.get_or_create_normal_deck("foo::bar::baz")?; let mut top_deck = col.get_or_create_normal_deck("foo")?; - top_deck.name = NativeDeckName("other".into()); + top_deck.name = NativeDeckName::from_native_str("other"); col.add_or_update_deck(&mut top_deck)?; assert_eq!( sorted_names(&col), @@ -216,7 +216,7 @@ mod test { // should do the right thing in the middle of the tree as well let mut middle = col.get_or_create_normal_deck("other::bar")?; - middle.name = NativeDeckName("quux\x1ffoo".into()); + middle.name = NativeDeckName::from_native_str("quux\x1ffoo"); col.add_or_update_deck(&mut middle)?; assert_eq!( sorted_names(&col), @@ -229,7 +229,7 @@ mod test { // quux::foo -> quux::foo::baz::four // means quux::foo::baz2 should be quux::foo::baz::four::baz2 // and a new quux::foo should have been created - middle.name = NativeDeckName("quux\x1ffoo\x1fbaz\x1ffour".into()); + middle.name = NativeDeckName::from_native_str("quux\x1ffoo\x1fbaz\x1ffour"); col.add_or_update_deck(&mut middle)?; assert_eq!( sorted_names(&col), @@ -246,7 +246,7 @@ mod test { ); // should handle name conflicts - middle.name = NativeDeckName("other".into()); + middle.name = NativeDeckName::from_native_str("other"); col.add_or_update_deck(&mut middle)?; assert_eq!(middle.name.as_str(), "other+"); @@ -277,7 +277,7 @@ mod test { let mut col = open_test_collection(); let mut default = col.get_or_create_normal_deck("default")?; - default.name = NativeDeckName("one\x1ftwo".into()); + default.name = NativeDeckName::from_native_str("one\x1ftwo"); col.add_or_update_deck(&mut default)?; // create a non-default deck confusingly named "default" diff --git a/rslib/src/decks/name.rs b/rslib/src/decks/name.rs index 603bbe64c..41a05cbf0 100644 --- a/rslib/src/decks/name.rs +++ b/rslib/src/decks/name.rs @@ -5,9 +5,13 @@ use itertools::Itertools; use std::borrow::Cow; #[derive(Debug, Clone, PartialEq)] -pub struct NativeDeckName(pub String); +pub struct NativeDeckName(String); impl NativeDeckName { + pub fn from_native_str>(name: N) -> Self { + NativeDeckName(name.into()) + } + pub fn from_human_name(name: &str) -> Self { NativeDeckName( name.split("::") diff --git a/rslib/src/decks/remove.rs b/rslib/src/decks/remove.rs index 37b4ab627..befb770f8 100644 --- a/rslib/src/decks/remove.rs +++ b/rslib/src/decks/remove.rs @@ -37,7 +37,8 @@ impl Collection { // if the default deck is included, just ensure it's reset to the default // name, as we've already removed its cards let mut modified_default = deck.clone(); - modified_default.name = NativeDeckName(self.tr.deck_config_default_name().into()); + modified_default.name = + NativeDeckName::from_native_str(self.tr.deck_config_default_name()); self.prepare_deck_for_update(&mut modified_default, usn)?; modified_default.set_modified(usn); self.update_single_deck_undoable(&mut modified_default, deck.clone())?; diff --git a/rslib/src/scheduler/filtered/mod.rs b/rslib/src/scheduler/filtered/mod.rs index 153ec7cca..e39e1a25a 100644 --- a/rslib/src/scheduler/filtered/mod.rs +++ b/rslib/src/scheduler/filtered/mod.rs @@ -147,7 +147,7 @@ impl Collection { } fn get_next_filtered_deck_name(&self) -> NativeDeckName { - NativeDeckName(format!( + NativeDeckName::from_native_str(format!( "Filtered Deck {}", TimestampSecs::now().time_string() )) diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index ab133409d..10a5082fa 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -346,11 +346,13 @@ impl SqlWriter<'_> { .storage .get_deck(current_did)? .map(|d| d.name) - .unwrap_or_else(|| NativeDeckName("Default".into())) + .unwrap_or_else(|| NativeDeckName::from_native_str("Default")) .as_str(), ) } else { - NativeDeckName::from_human_name(&to_re(deck)).0 + NativeDeckName::from_human_name(&to_re(deck)) + .as_str() + .to_string() }; // convert to a regex that includes child decks diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 1bbe43447..b0f34c5ee 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -24,7 +24,7 @@ fn row_to_deck(row: &Row) -> Result { let id = row.get(0)?; Ok(Deck { id, - name: NativeDeckName(row.get(1)?), + name: NativeDeckName::from_native_str(row.get_raw(1).as_str()?), mtime_secs: row.get(2)?, usn: row.get(3)?, common, @@ -346,7 +346,7 @@ impl SqliteStorage { let mut deck = Deck::new_normal(); deck.id.0 = 1; // fixme: separate key - deck.name = NativeDeckName(tr.deck_config_default_name().into()); + deck.name = NativeDeckName::from_native_str(tr.deck_config_default_name()); self.add_or_update_deck_with_existing_id(&deck) } @@ -361,7 +361,7 @@ impl SqliteStorage { deck.set_modified(usn); } loop { - let name = UniCase::new(deck.name.0.clone()); + let name = UniCase::new(deck.name.as_str().to_string()); if !names.contains(&name) { names.insert(name); break; diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index d0076d16e..cb145ecf7 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -1500,7 +1500,7 @@ mod test { })?; let mut deck = col2.storage.get_deck(deck.id)?.unwrap(); - deck.name = NativeDeckName("newer".into()); + deck.name = NativeDeckName::from_native_str("newer"); col2.add_or_update_deck(&mut deck)?; let mut nt = col2.storage.get_notetype(nt.id)?.unwrap(); From e71f7714ad0838644511b33b158da56f0fbfaad4 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 18 Apr 2021 09:33:39 +1000 Subject: [PATCH 4/6] as_str() -> as_native_str() --- rslib/src/dbcheck.rs | 6 +++++- rslib/src/decks/add.rs | 4 ++-- rslib/src/decks/mod.rs | 6 +++--- rslib/src/decks/name.rs | 18 +++++++++++------- rslib/src/decks/tree.rs | 2 +- rslib/src/scheduler/queue/limits.rs | 2 +- rslib/src/search/sqlwriter.rs | 4 ++-- rslib/src/storage/deck/mod.rs | 10 +++++----- 8 files changed, 30 insertions(+), 22 deletions(-) diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index 5f99075b3..39be2868b 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -430,7 +430,11 @@ mod test { } ); assert_eq!( - col.storage.get_deck(DeckId(123))?.unwrap().name.as_str(), + col.storage + .get_deck(DeckId(123))? + .unwrap() + .name + .as_native_str(), "recovered123" ); diff --git a/rslib/src/decks/add.rs b/rslib/src/decks/add.rs index 6047046f1..452050efb 100644 --- a/rslib/src/decks/add.rs +++ b/rslib/src/decks/add.rs @@ -105,7 +105,7 @@ impl Collection { /// Returns an error if a DB operation fails, or if the first existing parent is a filtered deck. fn match_or_create_parents(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> { let child_split: Vec<_> = deck.name.components().collect(); - if let Some(parent_deck) = self.first_existing_parent(deck.name.as_str(), 0)? { + if let Some(parent_deck) = self.first_existing_parent(deck.name.as_native_str(), 0)? { if parent_deck.is_filtered() { return Err(FilteredDeckError::MustBeLeafNode.into()); } @@ -130,7 +130,7 @@ impl Collection { } fn create_missing_parents(&mut self, name: &NativeDeckName, usn: Usn) -> Result<()> { - let mut machine_name = name.as_str(); + let mut machine_name = name.as_native_str(); while let Some(parent_name) = immediate_parent_name(machine_name) { if self.storage.get_deck_id(parent_name)?.is_none() { self.add_parent_deck(parent_name, usn)?; diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index f47cd3ca7..384041a22 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -150,7 +150,7 @@ impl Collection { pub fn get_or_create_normal_deck(&mut self, human_name: &str) -> Result { let name = NativeDeckName::from_human_name(human_name); - if let Some(did) = self.storage.get_deck_id(name.as_str())? { + if let Some(did) = self.storage.get_deck_id(name.as_native_str())? { self.storage.get_deck(did).map(|opt| opt.unwrap()) } else { let mut deck = Deck::new_normal(); @@ -164,7 +164,7 @@ impl Collection { /// use the method in storage instead. pub(crate) fn get_deck_id(&self, human_name: &str) -> Result> { self.storage - .get_deck_id(NativeDeckName::from_human_name(human_name).as_str()) + .get_deck_id(NativeDeckName::from_human_name(human_name).as_native_str()) } } @@ -248,7 +248,7 @@ mod test { // should handle name conflicts middle.name = NativeDeckName::from_native_str("other"); col.add_or_update_deck(&mut middle)?; - assert_eq!(middle.name.as_str(), "other+"); + assert_eq!(middle.name.as_native_str(), "other+"); // public function takes human name col.rename_deck(middle.id, "one::two")?; diff --git a/rslib/src/decks/name.rs b/rslib/src/decks/name.rs index 41a05cbf0..9d6f3ac1a 100644 --- a/rslib/src/decks/name.rs +++ b/rslib/src/decks/name.rs @@ -8,10 +8,7 @@ use std::borrow::Cow; pub struct NativeDeckName(String); impl NativeDeckName { - pub fn from_native_str>(name: N) -> Self { - NativeDeckName(name.into()) - } - + /// Create from a '::'-separated string pub fn from_human_name(name: &str) -> Self { NativeDeckName( name.split("::") @@ -20,6 +17,7 @@ impl NativeDeckName { ) } + /// Return a '::'-separated string. pub fn human_name(&self) -> String { self.0.replace('\x1f', "::") } @@ -28,7 +26,13 @@ impl NativeDeckName { self.0 += suffix } - pub(crate) fn as_str(&self) -> &str { + /// Create from an '\x1f'-separated string + pub(crate) fn from_native_str>(name: N) -> Self { + NativeDeckName(name.into()) + } + + /// Return a reference to the inner '\x1f'-separated string. + pub(crate) fn as_native_str(&self) -> &str { &self.0 } @@ -58,7 +62,7 @@ impl NativeDeckName { /// Replace the old parent's name with the new parent's name in self's name, where the old /// parent's name is expected to be a prefix. fn reparent(&mut self, old_parent: &NativeDeckName, new_parent: &NativeDeckName) { - self.0 = std::iter::once(new_parent.as_str()) + self.0 = std::iter::once(new_parent.as_native_str()) .chain(self.components().skip(old_parent.components().count())) .join("\x1f") } @@ -117,7 +121,7 @@ impl Collection { pub(crate) fn ensure_deck_name_unique(&self, deck: &mut Deck, usn: Usn) -> Result<()> { loop { - match self.storage.get_deck_id(deck.name.as_str())? { + match self.storage.get_deck_id(deck.name.as_native_str())? { Some(did) if did == deck.id => break, None => break, _ => (), diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 4c580819c..0e5d8b939 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -267,7 +267,7 @@ impl Collection { if let Some(now) = now { let limit = top_deck_id.and_then(|did| { if let Some(deck) = decks_map.get(&did) { - Some(deck.name.as_str()) + Some(deck.name.as_native_str()) } else { None } diff --git a/rslib/src/scheduler/queue/limits.rs b/rslib/src/scheduler/queue/limits.rs index f5252e36c..d80322517 100644 --- a/rslib/src/scheduler/queue/limits.rs +++ b/rslib/src/scheduler/queue/limits.rs @@ -39,7 +39,7 @@ pub(super) fn remaining_limits_capped_to_parents( today: u32, ) -> Vec { let mut limits = get_remaining_limits(decks, config, today); - cap_limits_to_parents(decks.iter().map(|d| d.name.as_str()), &mut limits); + cap_limits_to_parents(decks.iter().map(|d| d.name.as_native_str()), &mut limits); limits } diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index 10a5082fa..793b504aa 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -347,11 +347,11 @@ impl SqlWriter<'_> { .get_deck(current_did)? .map(|d| d.name) .unwrap_or_else(|| NativeDeckName::from_native_str("Default")) - .as_str(), + .as_native_str(), ) } else { NativeDeckName::from_human_name(&to_re(deck)) - .as_str() + .as_native_str() .to_string() }; diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index b0f34c5ee..ee3779bc7 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -124,7 +124,7 @@ impl SqliteStorage { let mut kind = vec![]; kind_enum.encode(&mut kind)?; let count = stmt.execute(params![ - deck.name.as_str(), + deck.name.as_native_str(), deck.mtime_secs, deck.usn, common, @@ -159,7 +159,7 @@ impl SqliteStorage { kind_enum.encode(&mut kind)?; stmt.execute(params![ deck.id, - deck.name.as_str(), + deck.name.as_native_str(), deck.mtime_secs, deck.usn, common, @@ -234,7 +234,7 @@ impl SqliteStorage { .last() .map(|d| &d.name) .unwrap_or_else(|| &child.name) - .as_str(), + .as_native_str(), ) { if let Some(parent_did) = self.get_deck_id(parent_name)? { let parent = self.get_deck(parent_did)?.unwrap(); @@ -329,7 +329,7 @@ impl SqliteStorage { "create temporary table active_decks (id integer primary key not null);" ))?; - let top = current.name.as_str(); + let top = current.name.as_native_str(); let prefix_start = &format!("{}\x1f", top); let prefix_end = &format!("{}\x20", top); @@ -361,7 +361,7 @@ impl SqliteStorage { deck.set_modified(usn); } loop { - let name = UniCase::new(deck.name.as_str().to_string()); + let name = UniCase::new(deck.name.as_native_str().to_string()); if !names.contains(&name) { names.insert(name); break; From 5e3e1942896a2e68eaffe1f4de0095afa686eab4 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Sun, 18 Apr 2021 08:43:46 +0200 Subject: [PATCH 5/6] Remove redundant imports --- rslib/src/storage/deck/mod.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index ee3779bc7..dd27df435 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -3,15 +3,12 @@ use super::SqliteStorage; use crate::{ - card::CardId, card::CardQueue, config::SchedulerVersion, decks::immediate_parent_name, - decks::{Deck, DeckCommon, DeckId, DeckKindContainer, DeckSchema11, DueCounts}, - error::{AnkiError, DbErrorKind, Result}, - i18n::I18n, + decks::{DeckCommon, DeckKindContainer, DeckSchema11, DueCounts}, + error::DbErrorKind, prelude::*, - timestamp::TimestampMillis, }; use prost::Message; use rusqlite::{named_params, params, Row, NO_PARAMS}; From fece39ca749c2ba66977c32055a01d51018d1456 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Sun, 18 Apr 2021 09:16:43 +0200 Subject: [PATCH 6/6] Maybe normalize name when preparing deck update --- rslib/src/decks/add.rs | 3 +++ rslib/src/decks/name.rs | 31 ++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/rslib/src/decks/add.rs b/rslib/src/decks/add.rs index 452050efb..ca723f5b7 100644 --- a/rslib/src/decks/add.rs +++ b/rslib/src/decks/add.rs @@ -7,6 +7,9 @@ impl Collection { /// Rename deck if not unique. Bumps mtime and usn if /// name was changed, but otherwise leaves it the same. pub(super) fn prepare_deck_for_update(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> { + if deck.name.maybe_normalize() { + deck.set_modified(usn); + } self.ensure_deck_name_unique(deck, usn) } diff --git a/rslib/src/decks/name.rs b/rslib/src/decks/name.rs index 9d6f3ac1a..3b1db1b4e 100644 --- a/rslib/src/decks/name.rs +++ b/rslib/src/decks/name.rs @@ -40,6 +40,20 @@ impl NativeDeckName { self.0.split('\x1f') } + /// Normalize the name's components if necessary. True if mutation took place. + pub(crate) fn maybe_normalize(&mut self) -> bool { + let needs_normalization = self + .components() + .any(|comp| matches!(normalized_deck_name_component(comp), Cow::Owned(_))); + if needs_normalization { + self.0 = self + .components() + .map(normalized_deck_name_component) + .join("\x1f"); + } + needs_normalization + } + /// Determine name to rename a deck to, when `self` is dropped on `target`. /// `target` being unset represents a drop at the top or bottom of the deck list. /// The returned name should be used to replace `self`. @@ -193,11 +207,26 @@ mod test { assert_eq!(native_name("foo"), "foo"); assert_eq!(native_name("foo::bar"), "foo\x1fbar"); assert_eq!(native_name("foo::::baz"), "foo\x1fblank\x1fbaz"); - // normalize + // implicitly normalize assert_eq!(native_name("fo\x1fo::ba\nr"), "foo\x1fbar"); assert_eq!(native_name("fo\u{a}o\x1fbar"), "foobar"); } + #[test] + fn normalize() { + fn normalize_res(name: &str) -> (bool, String) { + let mut name = NativeDeckName::from_native_str(name); + (name.maybe_normalize(), name.0) + } + + assert_eq!(normalize_res("foo\x1fbar"), (false, "foo\x1fbar".into())); + assert_eq!( + normalize_res("fo\x1fo::ba\nr"), + (true, "fo\x1fo::bar".into()) + ); + assert_eq!(normalize_res("fo\u{a}obar"), (true, "foobar".into())); + } + #[test] fn drag_drop() { // use custom separator to make the tests easier to read