diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 0d0a19e7d..4b7a1a52c 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -825,7 +825,7 @@ impl BackendService for Backend { if input.preserve_usn_and_mtime { col.transact(None, |col| { let usn = col.usn()?; - col.add_or_update_single_deck(&mut deck, usn) + col.add_or_update_single_deck_with_existing_id(&mut deck, usn) })?; } else { col.add_or_update_deck(&mut deck)?; diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 9ade97912..8192ec8d8 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -5,7 +5,9 @@ pub use crate::backend_proto::{ deck_kind::Kind as DeckKind, filtered_search_term::FilteredSearchOrder, Deck as DeckProto, DeckCommon, DeckKind as DeckKindProto, FilteredDeck, FilteredSearchTerm, NormalDeck, }; -use crate::{backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images}; +use crate::{ + backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images, undo::Undo, +}; use crate::{ collection::Collection, deckconf::DeckConfID, @@ -251,7 +253,7 @@ impl Collection { } /// Normalize deck name and rename if not unique. Bumps mtime and usn if - /// deck was modified. + /// name was changed, but otherwise leaves it the same. 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; @@ -268,18 +270,28 @@ impl Collection { self.transact(None, |col| { let usn = col.usn()?; - deck.set_modified(usn); + col.prepare_deck_for_update(deck, usn)?; if deck.id.0 == 0 { - col.prepare_deck_for_update(deck, usn)?; + // TODO: undo support col.match_or_create_parents(deck, usn)?; + deck.set_modified(usn); col.storage.add_deck(deck) } else if let Some(existing_deck) = col.storage.get_deck(deck.id)? { - if existing_deck.name != deck.name { - col.update_renamed_deck(existing_deck, deck, usn) - } else { - col.add_or_update_single_deck(deck, usn) + let name_changed = existing_deck.name != deck.name; + if name_changed { + // match closest parent name + col.match_or_create_parents(deck, usn)?; + // rename children + col.rename_child_decks(&existing_deck, &deck.name, usn)?; } + col.update_single_deck_no_check(deck, &existing_deck, usn)?; + if name_changed { + // after updating, we need to ensure all grandparents exist, which may not be the case + // in the parent->child case + col.create_missing_parents(&deck.name, usn)?; + } + Ok(()) } else { Err(AnkiError::invalid_input("updating non-existent deck")) } @@ -289,9 +301,28 @@ impl Collection { /// Add/update a single deck when syncing/importing. Ensures name is unique /// & normalized, but does not check parents/children or update mtime /// (unless the name was changed). Caller must set up transaction. - pub(crate) fn add_or_update_single_deck(&mut self, deck: &mut Deck, usn: Usn) -> Result<()> { + /// TODO: undo support + pub(crate) fn add_or_update_single_deck_with_existing_id( + &mut self, + deck: &mut Deck, + usn: Usn, + ) -> Result<()> { self.state.deck_cache.clear(); self.prepare_deck_for_update(deck, usn)?; + self.storage.add_or_update_deck_with_existing_id(deck) + } + + /// Update an individual, existing deck. Caller is responsible for ensuring deck + /// is normalized, matches parents, and is not a duplicate name. Bumps mtime. + pub(crate) fn update_single_deck_no_check( + &mut self, + deck: &mut Deck, + original: &Deck, + usn: Usn, + ) -> Result<()> { + self.state.deck_cache.clear(); + deck.set_modified(usn); + self.save_undo(Box::new(DeckUpdated(original.clone()))); self.storage.update_deck(deck) } @@ -316,7 +347,7 @@ impl Collection { deck.id = did; deck.name = format!("recovered{}", did); deck.set_modified(usn); - self.add_or_update_single_deck(&mut deck, usn) + self.add_or_update_single_deck_with_existing_id(&mut deck, usn) } pub fn get_or_create_normal_deck(&mut self, human_name: &str) -> Result { @@ -331,37 +362,17 @@ impl Collection { } } - fn update_renamed_deck(&mut self, existing: Deck, updated: &mut Deck, usn: Usn) -> Result<()> { - self.state.deck_cache.clear(); - // ensure name normalized - if let Cow::Owned(name) = normalize_native_name(&updated.name) { - updated.name = name; - } - // match closest parent name - self.match_or_create_parents(updated, usn)?; - // ensure new name is unique - self.ensure_deck_name_unique(updated, usn)?; - // rename children - self.rename_child_decks(&existing, &updated.name, usn)?; - // save deck - updated.set_modified(usn); - self.storage.update_deck(updated)?; - // after updating, we need to ensure all grandparents exist, which may not be the case - // in the parent->child case - self.create_missing_parents(&updated.name, usn) - } - fn rename_child_decks(&mut self, old: &Deck, new_name: &str, 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.set_modified(usn); - self.storage.update_deck(&child)?; + self.update_single_deck_no_check(&mut child, &original, usn)?; } Ok(()) @@ -475,7 +486,7 @@ impl Collection { // fixme: separate key deck.name = self.i18n.tr(TR::DeckConfigDefaultName).into(); deck.set_modified(usn); - self.add_or_update_single_deck(&mut deck, usn)?; + self.add_or_update_single_deck_with_existing_id(&mut deck, usn)?; } else { self.storage.remove_deck(deck.id)?; self.storage.add_deck_grave(deck.id, usn)?; @@ -587,10 +598,10 @@ impl Collection { where F: FnOnce(&mut DeckCommon), { + let original = deck.clone(); deck.reset_stats_if_day_changed(today); mutator(&mut deck.common); - deck.set_modified(usn); - self.add_or_update_single_deck(deck, usn) + self.update_single_deck_no_check(deck, &original, usn) } pub fn drag_drop_decks( @@ -636,6 +647,19 @@ impl Collection { } } +#[derive(Debug)] +pub(crate) struct DeckUpdated(Deck); + +impl Undo for DeckUpdated { + fn undo(mut self: Box, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> { + let current = col + .storage + .get_deck(self.0.id)? + .ok_or_else(|| AnkiError::invalid_input("deck disappeared"))?; + col.update_single_deck_no_check(&mut self.0, ¤t, usn) + } +} + #[cfg(test)] mod test { use super::{human_deck_name_to_native, immediate_parent_name, normalize_native_name}; diff --git a/rslib/src/scheduler/answering/mod.rs b/rslib/src/scheduler/answering/mod.rs index 307ab2899..c616db7bf 100644 --- a/rslib/src/scheduler/answering/mod.rs +++ b/rslib/src/scheduler/answering/mod.rs @@ -262,15 +262,12 @@ impl Collection { current_state, answer.current_state, ))); } - if let Some(revlog_partial) = updater.apply_study_state(current_state, answer.new_state)? { self.add_partial_revlog(revlog_partial, usn, &answer)?; } self.update_deck_stats_from_answer(usn, &answer, &updater)?; - let timing = updater.timing; - self.maybe_bury_siblings(&original, &updater.config)?; - + let timing = updater.timing; let mut card = updater.into_card(); self.update_card(&mut card, &original, usn)?; if answer.new_state.leeched() { diff --git a/rslib/src/scheduler/answering/undo.rs b/rslib/src/scheduler/answering/undo.rs index 28fd77eb8..dd5f42257 100644 --- a/rslib/src/scheduler/answering/undo.rs +++ b/rslib/src/scheduler/answering/undo.rs @@ -92,6 +92,9 @@ mod test { assert_eq!(note.tags, vec!["leech".to_string()]); assert_eq!(col.storage.all_tags()?.is_empty(), false); + let deck = col.get_deck(DeckID(1))?.unwrap(); + assert_eq!(deck.common.review_studied, 1); + Ok(()) }; @@ -113,6 +116,9 @@ mod test { assert_eq!(note.tags.is_empty(), true); assert_eq!(col.storage.all_tags()?.is_empty(), true); + let deck = col.get_deck(DeckID(1))?.unwrap(); + assert_eq!(deck.common.review_studied, 0); + Ok(()) }; diff --git a/rslib/src/storage/deck/add_or_update_deck.sql b/rslib/src/storage/deck/add_or_update_deck.sql new file mode 100644 index 000000000..7e35c1902 --- /dev/null +++ b/rslib/src/storage/deck/add_or_update_deck.sql @@ -0,0 +1,3 @@ +INSERT + OR REPLACE INTO decks (id, name, mtime_secs, usn, common, kind) +VALUES (?, ?, ?, ?, ?, ?) \ No newline at end of file diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index aaf19ad69..14e50597c 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -100,23 +100,53 @@ impl SqliteStorage { .db .prepare(include_str!("alloc_id.sql"))? .query_row(&[TimestampMillis::now()], |r| r.get(0))?; - self.update_deck(deck).map_err(|err| { - // restore id of 0 - deck.id.0 = 0; - err - }) + self.add_or_update_deck_with_existing_id(deck) + .map_err(|err| { + // restore id of 0 + deck.id.0 = 0; + err + }) } pub(crate) fn update_deck(&self, deck: &Deck) -> Result<()> { if deck.id.0 == 0 { return Err(AnkiError::invalid_input("deck with id 0")); } - self.add_or_update_deck(deck) + let mut stmt = self.db.prepare_cached(include_str!("update_deck.sql"))?; + let mut common = vec![]; + deck.common.encode(&mut common)?; + let kind_enum = DeckKindProto { + kind: Some(deck.kind.clone()), + }; + let mut kind = vec![]; + kind_enum.encode(&mut kind)?; + let count = stmt.execute(params![ + deck.name, + deck.mtime_secs, + deck.usn, + common, + kind, + deck.id + ])?; + + if count == 0 { + Err(AnkiError::invalid_input( + "update_deck() called with non-existent deck", + )) + } else { + Ok(()) + } } - /// Used for syncing; will keep existing ID. - pub(crate) fn add_or_update_deck(&self, deck: &Deck) -> Result<()> { - let mut stmt = self.db.prepare_cached(include_str!("update_deck.sql"))?; + /// Used for syncing; will keep existing ID. Shouldn't be used to add new decks locally, + /// since it does not allocate an id. + pub(crate) fn add_or_update_deck_with_existing_id(&self, deck: &Deck) -> Result<()> { + if deck.id.0 == 0 { + return Err(AnkiError::invalid_input("deck with id 0")); + } + let mut stmt = self + .db + .prepare_cached(include_str!("add_or_update_deck.sql"))?; let mut common = vec![]; deck.common.encode(&mut common)?; let kind_enum = DeckKindProto { @@ -310,7 +340,7 @@ impl SqliteStorage { deck.id.0 = 1; // fixme: separate key deck.name = i18n.tr(TR::DeckConfigDefaultName).into(); - self.update_deck(&deck) + self.add_or_update_deck_with_existing_id(&deck) } pub(crate) fn upgrade_decks_to_schema15(&self, server: bool) -> Result<()> { @@ -332,7 +362,7 @@ impl SqliteStorage { deck.name.push('_'); deck.set_modified(usn); } - self.update_deck(&deck)?; + self.add_or_update_deck_with_existing_id(&deck)?; } self.db.execute("update col set decks = ''", NO_PARAMS)?; Ok(()) diff --git a/rslib/src/storage/deck/update_deck.sql b/rslib/src/storage/deck/update_deck.sql index 7e35c1902..b250edb79 100644 --- a/rslib/src/storage/deck/update_deck.sql +++ b/rslib/src/storage/deck/update_deck.sql @@ -1,3 +1,7 @@ -INSERT - OR REPLACE INTO decks (id, name, mtime_secs, usn, common, kind) -VALUES (?, ?, ?, ?, ?, ?) \ No newline at end of file +UPDATE decks +SET name = ?, + mtime_secs = ?, + usn = ?, + common = ?, + kind = ? +WHERE id = ? \ No newline at end of file diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index 457687afb..d5c34a612 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -882,7 +882,7 @@ impl Collection { if proceed { let mut deck = deck.into(); self.ensure_deck_name_unique(&mut deck, latest_usn)?; - self.storage.add_or_update_deck(&deck)?; + self.storage.add_or_update_deck_with_existing_id(&deck)?; self.state.deck_cache.remove(&deck.id); } }