From 039ebfeed6a924e453e2f0179892d567ea5e4e07 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Sun, 19 Mar 2023 01:58:35 +0100 Subject: [PATCH] Fix invalid ids on db check (#2445) * Move open_test_collection into Collection test impl * Fix invalid ids when checking database * Report fixed invalid ids * Improve message when trying to export invalid ids Also move ImportError due to namespace conflicts with snafu macro. * Take a human name in DeckAdder::new * Mention timestamps in the db check message (dae) Will help to correlate the fix with the message shown when importing/ exporting. --- ftl/core/database-check.ftl | 5 + rslib/src/backend/ankidroid/db.rs | 3 +- rslib/src/collection/mod.rs | 5 - rslib/src/config/mod.rs | 7 +- rslib/src/config/undo.rs | 3 +- rslib/src/dbcheck.rs | 31 +++-- rslib/src/deckconfig/update.rs | 3 +- rslib/src/decks/mod.rs | 7 +- rslib/src/decks/name.rs | 5 +- rslib/src/decks/tree.rs | 9 +- rslib/src/error/mod.rs | 25 +--- rslib/src/findreplace.rs | 3 +- rslib/src/import_export/gather.rs | 5 +- rslib/src/import_export/mod.rs | 26 ++++ .../package/apkg/import/decks.rs | 11 +- .../import_export/package/apkg/import/mod.rs | 7 +- .../package/apkg/import/notes.rs | 15 ++- .../import_export/package/colpkg/import.rs | 2 +- rslib/src/import_export/package/media.rs | 2 +- rslib/src/import_export/package/meta.rs | 2 +- rslib/src/import_export/text/csv/metadata.rs | 27 ++-- rslib/src/import_export/text/import.rs | 17 ++- rslib/src/notes/mod.rs | 7 +- rslib/src/notetype/notetypechange.rs | 9 +- rslib/src/notetype/schemachange.rs | 12 +- rslib/src/scheduler/answering/mod.rs | 5 +- rslib/src/scheduler/answering/preview.rs | 3 +- rslib/src/scheduler/bury_and_suspend.rs | 3 +- rslib/src/scheduler/congrats.rs | 4 +- rslib/src/scheduler/filtered/custom_study.rs | 3 +- rslib/src/scheduler/queue/builder/mod.rs | 9 +- rslib/src/scheduler/queue/undo.rs | 7 +- rslib/src/search/sqlwriter.rs | 2 +- rslib/src/stats/card.rs | 3 +- .../src/storage/dbcheck/invalid_ids_count.sql | 13 ++ .../storage/dbcheck/invalid_ids_create.sql | 27 ++++ .../src/storage/dbcheck/invalid_ids_drop.sql | 1 + .../storage/dbcheck/invalid_ids_update.sql | 11 ++ rslib/src/storage/dbcheck/mod.rs | 115 ++++++++++++++++++ rslib/src/storage/mod.rs | 1 + rslib/src/tags/findreplace.rs | 3 +- rslib/src/tags/register.rs | 3 +- rslib/src/tags/remove.rs | 3 +- rslib/src/tags/reparent.rs | 3 +- rslib/src/tags/tree.rs | 3 +- rslib/src/tests.rs | 15 ++- rslib/src/undo/mod.rs | 7 +- 47 files changed, 330 insertions(+), 162 deletions(-) create mode 100644 rslib/src/storage/dbcheck/invalid_ids_count.sql create mode 100644 rslib/src/storage/dbcheck/invalid_ids_create.sql create mode 100644 rslib/src/storage/dbcheck/invalid_ids_drop.sql create mode 100644 rslib/src/storage/dbcheck/invalid_ids_update.sql create mode 100644 rslib/src/storage/dbcheck/mod.rs diff --git a/ftl/core/database-check.ftl b/ftl/core/database-check.ftl index 2334f0642..67d6812e7 100644 --- a/ftl/core/database-check.ftl +++ b/ftl/core/database-check.ftl @@ -45,6 +45,11 @@ database-check-notes-with-invalid-utf8 = [one] Fixed { $count } note with invalid utf8 characters. *[other] Fixed { $count } notes with invalid utf8 characters. } +database-check-fixed-invalid-ids = + { $count -> + [one] Fixed { $count } object with timestamps in the future. + *[other] Fixed { $count } objects with timestamps in the future. + } # "db-check" is always in English database-check-notetypes-recovered = One or more notetypes were missing. The notes that used them have been given new notetypes starting with "db-check", but field names and card design have been lost, so you may be better off restoring from an automatic backup. diff --git a/rslib/src/backend/ankidroid/db.rs b/rslib/src/backend/ankidroid/db.rs index 70e901cbc..ca73d674b 100644 --- a/rslib/src/backend/ankidroid/db.rs +++ b/rslib/src/backend/ankidroid/db.rs @@ -282,7 +282,6 @@ mod tests { use super::*; use crate::backend::ankidroid::db::select_slice_of_size; use crate::backend::ankidroid::db::Sizable; - use crate::collection::open_test_collection; use crate::pb::ankidroid::sql_value; use crate::pb::ankidroid::Row; use crate::pb::ankidroid::SqlValue; @@ -382,7 +381,7 @@ mod tests { #[test] fn integration_test() { - let col = open_test_collection(); + let col = Collection::new(); let row = Row { fields: gen_data() }; diff --git a/rslib/src/collection/mod.rs b/rslib/src/collection/mod.rs index c4d2ae807..4967dd419 100644 --- a/rslib/src/collection/mod.rs +++ b/rslib/src/collection/mod.rs @@ -110,11 +110,6 @@ impl CollectionBuilder { } } -#[cfg(test)] -pub fn open_test_collection() -> Collection { - CollectionBuilder::default().build().unwrap() -} - #[derive(Debug, Default)] pub struct CollectionState { pub(crate) undo: UndoManager, diff --git a/rslib/src/config/mod.rs b/rslib/src/config/mod.rs index 4a79f5550..7a5b6e6fa 100644 --- a/rslib/src/config/mod.rs +++ b/rslib/src/config/mod.rs @@ -310,18 +310,17 @@ pub(crate) enum Weekday { #[cfg(test)] mod test { - use crate::collection::open_test_collection; - use crate::decks::DeckId; + use super::*; #[test] fn defaults() { - let col = open_test_collection(); + let col = Collection::new(); assert_eq!(col.get_current_deck_id(), DeckId(1)); } #[test] fn get_set() { - let mut col = open_test_collection(); + let mut col = Collection::new(); // missing key assert_eq!(col.get_config_optional::, _>("test"), None); diff --git a/rslib/src/config/undo.rs b/rslib/src/config/undo.rs index 25dbb352d..8de01fc8e 100644 --- a/rslib/src/config/undo.rs +++ b/rslib/src/config/undo.rs @@ -71,11 +71,10 @@ impl Collection { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; #[test] fn undo() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); // the op kind doesn't matter, we just need undo enabled let op = Op::Bury; // test key diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index 747802c40..d7c683e58 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -36,6 +36,7 @@ pub struct CheckDatabaseOutput { field_count_mismatch: usize, notetypes_recovered: usize, invalid_utf8: usize, + invalid_ids: usize, } #[derive(Debug, Clone, Copy)] @@ -82,6 +83,9 @@ impl CheckDatabaseOutput { if self.invalid_utf8 > 0 { probs.push(tr.database_check_notes_with_invalid_utf8(self.invalid_utf8)); } + if self.invalid_ids > 0 { + probs.push(tr.database_check_fixed_invalid_ids(self.invalid_ids)); + } probs.into_iter().map(Into::into).collect() } @@ -139,6 +143,9 @@ impl Collection { self.update_next_new_position()?; + debug!("invalid ids"); + self.maybe_fix_invalid_ids(&mut out)?; + debug!("db check finished: {:#?}", out); Ok(out) @@ -408,12 +415,22 @@ impl Collection { let pos = self.storage.max_new_card_position().unwrap_or(0); self.set_next_card_position(pos) } + + fn maybe_fix_invalid_ids(&mut self, out: &mut CheckDatabaseOutput) -> Result<()> { + let now = TimestampMillis::now(); + let tomorrow = now.adding_secs(24 * 60 * 60).0; + out.invalid_ids = self.storage.invalid_ids(tomorrow)?; + if out.invalid_ids > 0 { + self.storage.fix_invalid_ids(tomorrow, now.0)?; + self.set_schema_modified()?; + } + Ok(()) + } } #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::decks::DeckId; use crate::search::SortMode; @@ -421,7 +438,7 @@ mod test { #[test] fn cards() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); col.add_note(&mut note, DeckId(1))?; @@ -483,7 +500,7 @@ mod test { #[test] fn revlog() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); col.storage.db.execute_batch( " @@ -508,7 +525,7 @@ mod test { #[test] fn note_card_link() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); col.add_note(&mut note, DeckId(1))?; @@ -557,7 +574,7 @@ mod test { #[test] fn note_fields() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); col.add_note(&mut note, DeckId(1))?; @@ -597,7 +614,7 @@ mod test { #[test] fn deck_names() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let deck = col.get_or_create_normal_deck("foo::bar::baz")?; // includes default @@ -631,7 +648,7 @@ mod test { #[test] fn tags() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); note.tags.push("one".into()); diff --git a/rslib/src/deckconfig/update.rs b/rslib/src/deckconfig/update.rs index 47654e2ff..ef79d7450 100644 --- a/rslib/src/deckconfig/update.rs +++ b/rslib/src/deckconfig/update.rs @@ -275,14 +275,13 @@ fn update_day_limit(day_limit: &mut Option, new_limit: Option, to #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::deckconfig::NewCardInsertOrder; use crate::tests::open_test_collection_with_learning_card; use crate::tests::open_test_collection_with_relearning_card; #[test] fn updating() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note1 = nt.new_note(); col.add_note(&mut note1, DeckId(1))?; diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 893c12569..c2184973e 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -185,7 +185,6 @@ impl Collection { #[cfg(test)] mod test { - use crate::collection::open_test_collection; use crate::prelude::*; use crate::search::SortMode; @@ -200,7 +199,7 @@ mod test { #[test] fn adding_updating() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let deck1 = col.get_or_create_normal_deck("foo")?; let deck2 = col.get_or_create_normal_deck("FOO")?; @@ -220,7 +219,7 @@ mod test { #[test] fn renaming() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let _ = col.get_or_create_normal_deck("foo::bar::baz")?; let mut top_deck = col.get_or_create_normal_deck("foo")?; @@ -291,7 +290,7 @@ mod test { fn default() -> Result<()> { // deleting the default deck will remove cards, but bring the deck back // as a top level deck - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut default = col.get_or_create_normal_deck("default")?; default.name = NativeDeckName::from_native_str("one\x1ftwo"); diff --git a/rslib/src/decks/name.rs b/rslib/src/decks/name.rs index 54028a72c..0a7459055 100644 --- a/rslib/src/decks/name.rs +++ b/rslib/src/decks/name.rs @@ -12,9 +12,10 @@ pub struct NativeDeckName(String); impl NativeDeckName { /// Create from a '::'-separated string - pub fn from_human_name(name: &str) -> Self { + pub fn from_human_name(name: impl AsRef) -> Self { NativeDeckName( - name.split("::") + name.as_ref() + .split("::") .map(normalized_deck_name_component) .join("\x1f"), ) diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 7ad5d8ab3..d73f18df7 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -406,13 +406,12 @@ impl Collection { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::deckconfig::DeckConfigId; use crate::error::Result; #[test] fn wellformed() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); col.get_or_create_normal_deck("1")?; col.get_or_create_normal_deck("2")?; @@ -436,7 +435,7 @@ mod test { #[test] fn malformed() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); col.get_or_create_normal_deck("1")?; col.get_or_create_normal_deck("2::3::4")?; @@ -453,7 +452,7 @@ mod test { #[test] fn counts() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut parent_deck = col.get_or_create_normal_deck("Default")?; let mut child_deck = col.get_or_create_normal_deck("Default::one")?; @@ -503,7 +502,7 @@ mod test { deck } - let mut col = open_test_collection(); + let mut col = Collection::new(); col.set_config_bool(BoolKey::Sched2021, true, false)?; let parent_deck = create_deck_with_new_limit(&mut col, "Default", 8); diff --git a/rslib/src/error/mod.rs b/rslib/src/error/mod.rs index 743b4164b..519fa4241 100644 --- a/rslib/src/error/mod.rs +++ b/rslib/src/error/mod.rs @@ -31,6 +31,7 @@ pub use self::invalid_input::OrInvalid; pub use self::not_found::NotFoundError; pub use self::not_found::OrNotFound; use crate::i18n::I18n; +use crate::import_export::ImportError; use crate::links::HelpPage; pub type Result = std::result::Result; @@ -150,7 +151,7 @@ impl AnkiError { AnkiError::CustomStudyError { source } => source.message(tr), AnkiError::ImportError { source } => source.message(tr), AnkiError::Deleted => tr.browsing_row_deleted().into(), - AnkiError::InvalidId => tr.errors_invalid_ids().into(), + AnkiError::InvalidId => tr.errors_please_check_database().into(), AnkiError::JsonError { .. } | AnkiError::ProtoError { .. } | AnkiError::Interrupted @@ -299,25 +300,3 @@ pub enum CardTypeErrorDetails { MissingCloze, ExtraneousCloze, } - -#[derive(Debug, PartialEq, Eq, Clone, Snafu)] -pub enum ImportError { - Corrupt, - TooNew, - MediaImportFailed { info: String }, - NoFieldColumn, -} - -impl ImportError { - fn message(&self, tr: &I18n) -> String { - match self { - ImportError::Corrupt => tr.importing_the_provided_file_is_not_a(), - ImportError::TooNew => tr.errors_collection_too_new(), - ImportError::MediaImportFailed { info } => { - tr.importing_failed_to_import_media_file(info) - } - ImportError::NoFieldColumn => tr.importing_file_must_contain_field_column(), - } - .into() - } -} diff --git a/rslib/src/findreplace.rs b/rslib/src/findreplace.rs index 62dc7a224..510848184 100644 --- a/rslib/src/findreplace.rs +++ b/rslib/src/findreplace.rs @@ -114,12 +114,11 @@ impl Collection { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::decks::DeckId; #[test] fn findreplace() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); diff --git a/rslib/src/import_export/gather.rs b/rslib/src/import_export/gather.rs index 82d50b29b..917b304e0 100644 --- a/rslib/src/import_export/gather.rs +++ b/rslib/src/import_export/gather.rs @@ -270,13 +270,12 @@ impl Collection { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::search::SearchNode; #[test] fn should_gather_valid_notes() { let mut data = ExchangeData::default(); - let mut col = open_test_collection(); + let mut col = Collection::new(); let note = NoteAdder::basic(&mut col).add(&mut col); data.gather_data(&mut col, SearchNode::WholeCollection, true) @@ -288,7 +287,7 @@ mod test { #[test] fn should_err_if_note_has_invalid_id() { let mut data = ExchangeData::default(); - let mut col = open_test_collection(); + let mut col = Collection::new(); let now_micros = TimestampMillis::now().0 * 1000; let mut note = NoteAdder::basic(&mut col).add(&mut col); diff --git a/rslib/src/import_export/mod.rs b/rslib/src/import_export/mod.rs index 5697e88d9..8fbc2bcd7 100644 --- a/rslib/src/import_export/mod.rs +++ b/rslib/src/import_export/mod.rs @@ -8,6 +8,8 @@ pub mod text; use std::marker::PhantomData; +use snafu::Snafu; + pub use crate::pb::import_export::import_response::Log as NoteLog; pub use crate::pb::import_export::import_response::Note as LogNote; use crate::prelude::*; @@ -128,3 +130,27 @@ impl Note { } } } + +#[derive(Debug, PartialEq, Eq, Clone, Snafu)] +pub enum ImportError { + Corrupt, + TooNew, + MediaImportFailed { info: String }, + NoFieldColumn, + InvalidId, +} + +impl ImportError { + pub(crate) fn message(&self, tr: &I18n) -> String { + match self { + ImportError::Corrupt => tr.importing_the_provided_file_is_not_a(), + ImportError::TooNew => tr.errors_collection_too_new(), + ImportError::MediaImportFailed { info } => { + tr.importing_failed_to_import_media_file(info) + } + ImportError::NoFieldColumn => tr.importing_file_must_contain_field_column(), + ImportError::InvalidId => tr.errors_invalid_ids(), + } + .into() + } +} diff --git a/rslib/src/import_export/package/apkg/import/decks.rs b/rslib/src/import_export/package/apkg/import/decks.rs index 2fee2d488..19b129954 100644 --- a/rslib/src/import_export/package/apkg/import/decks.rs +++ b/rslib/src/import_export/package/apkg/import/decks.rs @@ -209,11 +209,10 @@ mod test { use std::collections::HashSet; use super::*; - use crate::collection::open_test_collection; #[test] fn parents() { - let mut col = open_test_collection(); + let mut col = Collection::new(); DeckAdder::new("filtered").filtered(true).add(&mut col); DeckAdder::new("PARENT").add(&mut col); @@ -222,10 +221,10 @@ mod test { ctx.unique_suffix = "★".to_string(); let imports = vec![ - DeckAdder::new("unknown parent\x1fchild").deck(), - DeckAdder::new("filtered\x1fchild").deck(), - DeckAdder::new("parent\x1fchild").deck(), - DeckAdder::new("NEW PARENT\x1fchild").deck(), + DeckAdder::new("unknown parent::child").deck(), + DeckAdder::new("filtered::child").deck(), + DeckAdder::new("parent::child").deck(), + DeckAdder::new("NEW PARENT::child").deck(), DeckAdder::new("new parent").deck(), ]; ctx.import_decks(imports, false, false).unwrap(); diff --git a/rslib/src/import_export/package/apkg/import/mod.rs b/rslib/src/import_export/package/apkg/import/mod.rs index d1133eeec..3430bf653 100644 --- a/rslib/src/import_export/package/apkg/import/mod.rs +++ b/rslib/src/import_export/package/apkg/import/mod.rs @@ -20,6 +20,7 @@ use crate::error::FileIoSnafu; use crate::error::FileOp; use crate::import_export::gather::ExchangeData; use crate::import_export::package::Meta; +use crate::import_export::ImportError; use crate::import_export::ImportProgress; use crate::import_export::IncrementableProgress; use crate::import_export::NoteLog; @@ -110,7 +111,11 @@ impl ExchangeData { progress.call(ImportProgress::Gathering)?; let mut data = ExchangeData::default(); - data.gather_data(&mut col, search, with_scheduling)?; + data.gather_data(&mut col, search, with_scheduling) + .map_err(|error| match error { + AnkiError::InvalidId => ImportError::InvalidId.into(), + error => error, + })?; Ok(data) } diff --git a/rslib/src/import_export/package/apkg/import/notes.rs b/rslib/src/import_export/package/apkg/import/notes.rs index 8af7ce8a2..d7f2e313a 100644 --- a/rslib/src/import_export/package/apkg/import/notes.rs +++ b/rslib/src/import_export/package/apkg/import/notes.rs @@ -292,7 +292,6 @@ impl Notetype { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::import_export::package::media::SafeMediaEntry; /// Import [Note] into [Collection], optionally taking a [MediaUseMap], @@ -341,7 +340,7 @@ mod test { #[test] fn should_add_note_with_new_id_if_guid_is_unique_and_id_is_not() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut note = NoteAdder::basic(&mut col).add(&mut col); note.guid = "other".to_string(); let original_id = note.id; @@ -353,7 +352,7 @@ mod test { #[test] fn should_skip_note_if_guid_already_exists_with_newer_mtime() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut note = NoteAdder::basic(&mut col).add(&mut col); note.mtime.0 -= 1; note.fields_mut()[0] = "outdated".to_string(); @@ -365,7 +364,7 @@ mod test { #[test] fn should_update_note_if_guid_already_exists_with_different_id() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut note = NoteAdder::basic(&mut col).add(&mut col); note.id.0 = 42; note.mtime.0 += 1; @@ -378,7 +377,7 @@ mod test { #[test] fn should_ignore_note_if_guid_already_exists_with_different_notetype() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut note = NoteAdder::basic(&mut col).add(&mut col); note.notetype_id.0 = 42; note.mtime.0 += 1; @@ -391,7 +390,7 @@ mod test { #[test] fn should_add_note_with_remapped_notetype_if_in_notetype_map() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let basic_ntid = col.get_notetype_by_name("basic").unwrap().unwrap().id; let mut note = NoteAdder::basic(&mut col).note(); note.notetype_id.0 = 123; @@ -403,7 +402,7 @@ mod test { #[test] fn should_ignore_note_if_guid_already_exists_and_notetype_is_remapped() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let basic_ntid = col.get_notetype_by_name("basic").unwrap().unwrap().id; let mut note = NoteAdder::basic(&mut col).add(&mut col); note.notetype_id.0 = 123; @@ -417,7 +416,7 @@ mod test { #[test] fn should_add_note_with_remapped_media_reference_in_field_if_in_media_map() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut note = NoteAdder::basic(&mut col).note(); note.fields_mut()[0] = "".to_string(); diff --git a/rslib/src/import_export/package/colpkg/import.rs b/rslib/src/import_export/package/colpkg/import.rs index cd2224a11..75b25c224 100644 --- a/rslib/src/import_export/package/colpkg/import.rs +++ b/rslib/src/import_export/package/colpkg/import.rs @@ -14,10 +14,10 @@ use zstd::stream::copy_decode; use crate::collection::CollectionBuilder; use crate::error::FileIoSnafu; use crate::error::FileOp; -use crate::error::ImportError; use crate::import_export::package::media::extract_media_entries; use crate::import_export::package::media::SafeMediaEntry; use crate::import_export::package::Meta; +use crate::import_export::ImportError; use crate::import_export::ImportProgress; use crate::import_export::IncrementableProgress; use crate::io::atomic_rename; diff --git a/rslib/src/import_export/package/media.rs b/rslib/src/import_export/package/media.rs index 52e6ef02d..c67ea46b0 100644 --- a/rslib/src/import_export/package/media.rs +++ b/rslib/src/import_export/package/media.rs @@ -25,9 +25,9 @@ use super::MediaEntry; use super::Meta; use crate::error::FileIoError; use crate::error::FileOp; -use crate::error::ImportError; use crate::error::InvalidInputError; use crate::import_export::package::colpkg::export::MaybeEncodedWriter; +use crate::import_export::ImportError; use crate::io::atomic_rename; use crate::io::filename_is_safe; use crate::io::new_tempfile_in; diff --git a/rslib/src/import_export/package/meta.rs b/rslib/src/import_export/package/meta.rs index 81f8dea78..7d0ba36bd 100644 --- a/rslib/src/import_export/package/meta.rs +++ b/rslib/src/import_export/package/meta.rs @@ -9,7 +9,7 @@ use prost::Message; use zip::ZipArchive; use zstd::stream::copy_decode; -use crate::error::ImportError; +use crate::import_export::ImportError; pub(super) use crate::pb::import_export::package_metadata::Version; pub(super) use crate::pb::import_export::PackageMetadata as Meta; use crate::prelude::*; diff --git a/rslib/src/import_export/text/csv/metadata.rs b/rslib/src/import_export/text/csv/metadata.rs index dece5a6b5..762bc03e0 100644 --- a/rslib/src/import_export/text/csv/metadata.rs +++ b/rslib/src/import_export/text/csv/metadata.rs @@ -14,8 +14,8 @@ use strum::IntoEnumIterator; use super::import::build_csv_reader; use crate::config::I32ConfigKey; -use crate::error::ImportError; use crate::import_export::text::NameOrId; +use crate::import_export::ImportError; use crate::io::open_file; use crate::notetype::NoteField; use crate::pb::generic::StringList; @@ -574,7 +574,6 @@ mod test { use std::io::Cursor; use super::*; - use crate::collection::open_test_collection; macro_rules! metadata { ($col:expr,$csv:expr) => { @@ -604,7 +603,7 @@ mod test { #[test] fn should_detect_deck_by_name_or_id() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let deck_id = col.get_or_create_normal_deck("my deck").unwrap().id.0; assert_eq!(metadata!(col, "#deck:my deck\n").unwrap_deck_id(), deck_id); assert_eq!( @@ -618,7 +617,7 @@ mod test { #[test] fn should_detect_notetype_by_name_or_id() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let basic_id = col.get_notetype_by_name("Basic").unwrap().unwrap().id.0; assert_eq!( metadata!(col, "#notetype:Basic\n").unwrap_notetype_id(), @@ -632,7 +631,7 @@ mod test { #[test] fn should_detect_valid_delimiters() { - let mut col = open_test_collection(); + let mut col = Collection::new(); assert_eq!( metadata!(col, "#separator:comma\n").delimiter(), Delimiter::Comma @@ -661,7 +660,7 @@ mod test { #[test] fn should_enforce_valid_html_flag() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let meta = metadata!(col, "#html:true\n"); assert!(meta.is_html); @@ -676,7 +675,7 @@ mod test { #[test] fn should_set_missing_html_flag_by_first_line() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let meta = metadata!(col, "
\n"); assert!(meta.is_html); @@ -690,7 +689,7 @@ mod test { #[test] fn should_detect_old_and_new_style_tags() { - let mut col = open_test_collection(); + let mut col = Collection::new(); assert_eq!(metadata!(col, "tags:foo bar\n").global_tags, ["foo", "bar"]); assert_eq!( metadata!(col, "#tags:foo bar\n").global_tags, @@ -708,7 +707,7 @@ mod test { #[test] fn should_detect_column_number_and_names() { - let mut col = open_test_collection(); + let mut col = Collection::new(); // detect from line assert_eq!(metadata!(col, "foo;bar\n").column_labels.len(), 2); // detect encoded @@ -745,7 +744,7 @@ mod test { #[test] fn should_detect_column_number_despite_escaped_line_breaks() { - let mut col = open_test_collection(); + let mut col = Collection::new(); assert_eq!( metadata!(col, "\"foo|\nbar\"\tfoo\tbar\n") .column_labels @@ -765,21 +764,21 @@ mod test { #[test] fn should_map_default_notetype_fields_by_index_if_no_column_names() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let meta = metadata!(col, "#deck column:1\nfoo,bar,baz\n"); assert_eq!(meta.unwrap_notetype_map(), &[2, 3]); } #[test] fn should_map_default_notetype_fields_by_given_column_names() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let meta = metadata!(col, "#columns:Back\tFront\nfoo,bar,baz\n"); assert_eq!(meta.unwrap_notetype_map(), &[2, 1]); } #[test] fn should_gather_first_lines_into_preview() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let meta = metadata!(col, "#separator: \nfoo bar\nbaz
\n"); assert_eq!(meta.preview[0].vals, ["foo", "bar"]); // html is stripped @@ -788,7 +787,7 @@ mod test { #[test] fn should_parse_first_first_line_despite_bom() { - let mut col = open_test_collection(); + let mut col = Collection::new(); assert_eq!( metadata!(col, "\u{feff}#separator:tab\n").delimiter(), Delimiter::Tab diff --git a/rslib/src/import_export/text/import.rs b/rslib/src/import_export/text/import.rs index 7e58736b6..78e081266 100644 --- a/rslib/src/import_export/text/import.rs +++ b/rslib/src/import_export/text/import.rs @@ -630,7 +630,6 @@ impl ForeignTemplate { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::tests::DeckAdder; use crate::tests::NoteAdder; @@ -653,7 +652,7 @@ mod test { #[test] fn should_always_add_note_if_dupe_mode_is_add() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut data = ForeignData::with_defaults(); data.add_note(&["same", "old"]); data.dupe_resolution = DupeResolution::Duplicate; @@ -665,7 +664,7 @@ mod test { #[test] fn should_add_or_ignore_note_if_dupe_mode_is_ignore() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut data = ForeignData::with_defaults(); data.add_note(&["same", "old"]); data.dupe_resolution = DupeResolution::Preserve; @@ -682,7 +681,7 @@ mod test { #[test] fn should_update_or_add_note_if_dupe_mode_is_update() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut data = ForeignData::with_defaults(); data.add_note(&["same", "old"]); data.dupe_resolution = DupeResolution::Update; @@ -697,7 +696,7 @@ mod test { #[test] fn should_keep_old_field_content_if_no_new_one_is_supplied() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut data = ForeignData::with_defaults(); data.add_note(&["same", "unchanged"]); data.add_note(&["same", "unchanged"]); @@ -716,7 +715,7 @@ mod test { #[test] fn should_recognize_normalized_duplicate_only_if_normalization_is_enabled() { - let mut col = open_test_collection(); + let mut col = Collection::new(); NoteAdder::basic(&mut col) .fields(&["神", "old"]) .add(&mut col); @@ -737,7 +736,7 @@ mod test { #[test] fn should_add_global_tags() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut data = ForeignData::with_defaults(); data.add_note(&["foo"]); data.notes[0].tags.replace(vec![String::from("bar")]); @@ -749,7 +748,7 @@ mod test { #[test] fn should_match_note_with_same_guid() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut data = ForeignData::with_defaults(); data.add_note(&["foo"]); data.notes[0].tags.replace(vec![String::from("bar")]); @@ -761,7 +760,7 @@ mod test { #[test] fn should_only_update_duplicates_in_same_deck_if_limit_is_enabled() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let other_deck_id = DeckAdder::new("other").add(&mut col).id; NoteAdder::basic(&mut col) .fields(&["foo", "old"]) diff --git a/rslib/src/notes/mod.rs b/rslib/src/notes/mod.rs index bddb01c23..cea076c61 100644 --- a/rslib/src/notes/mod.rs +++ b/rslib/src/notes/mod.rs @@ -631,7 +631,6 @@ fn note_differs_from_db(existing_note: &mut Note, note: &mut Note) -> bool { mod test { use super::anki_base91; use super::field_checksum; - use crate::collection::open_test_collection; use crate::config::BoolKey; use crate::decks::DeckId; use crate::error::Result; @@ -655,7 +654,7 @@ mod test { #[test] fn adding_cards() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col .get_notetype_by_name("basic (and reversed card)")? .unwrap(); @@ -703,7 +702,7 @@ mod test { #[test] fn normalization() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); @@ -735,7 +734,7 @@ mod test { #[test] fn undo() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col .get_notetype_by_name("basic (and reversed card)")? .unwrap(); diff --git a/rslib/src/notetype/notetypechange.rs b/rslib/src/notetype/notetypechange.rs index fb85a5bfe..9460ab682 100644 --- a/rslib/src/notetype/notetypechange.rs +++ b/rslib/src/notetype/notetypechange.rs @@ -374,12 +374,11 @@ fn remap_fields(fields: &mut Vec, new_fields: &[Option]) { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::error::Result; #[test] fn field_map() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut basic = col .storage .get_notetype(col.get_current_notetype_id().unwrap())? @@ -446,7 +445,7 @@ mod test { #[test] fn basic() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let basic = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = basic.new_note(); note.set_field(0, "1")?; @@ -482,7 +481,7 @@ mod test { #[test] fn field_count_change() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let basic = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = basic.new_note(); note.set_field(0, "1")?; @@ -503,7 +502,7 @@ mod test { #[test] fn cloze() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let basic = col .get_notetype_by_name("Basic (and reversed card)")? .unwrap(); diff --git a/rslib/src/notetype/schemachange.rs b/rslib/src/notetype/schemachange.rs index a202f8580..3013e48e2 100644 --- a/rslib/src/notetype/schemachange.rs +++ b/rslib/src/notetype/schemachange.rs @@ -194,11 +194,7 @@ impl Notetype { #[cfg(test)] mod test { - use super::ords_changed; - use super::TemplateOrdChanges; - use crate::collection::open_test_collection; - use crate::decks::DeckId; - use crate::error::Result; + use super::*; use crate::search::SortMode; #[test] @@ -258,7 +254,7 @@ mod test { #[test] fn fields() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut nt = col .storage .get_notetype(col.get_current_notetype_id().unwrap())? @@ -286,7 +282,7 @@ mod test { #[test] fn field_renaming_and_deleting() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut nt = col .storage .get_notetype(col.get_current_notetype_id().unwrap())? @@ -307,7 +303,7 @@ mod test { #[test] fn cards() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut nt = col .storage .get_notetype(col.get_current_notetype_id().unwrap())? diff --git a/rslib/src/scheduler/answering/mod.rs b/rslib/src/scheduler/answering/mod.rs index 93e501d4c..4fa9ad268 100644 --- a/rslib/src/scheduler/answering/mod.rs +++ b/rslib/src/scheduler/answering/mod.rs @@ -447,7 +447,6 @@ fn get_fuzz_factor(seed: Option) -> Option { mod test { use super::*; use crate::card::CardType; - use crate::collection::open_test_collection; use crate::deckconfig::ReviewMix; use crate::search::SortMode; @@ -459,7 +458,7 @@ mod test { // state we applied to it #[test] fn state_application() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); if col.timing_today()?.near_cutoff() { return Ok(()); } @@ -574,7 +573,7 @@ mod test { } fn v3_test_collection(cards: usize) -> Result<(Collection, Vec)> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); for _ in 0..cards { let mut note = Note::new(&nt); diff --git a/rslib/src/scheduler/answering/preview.rs b/rslib/src/scheduler/answering/preview.rs index 74caa0da5..f8e795600 100644 --- a/rslib/src/scheduler/answering/preview.rs +++ b/rslib/src/scheduler/answering/preview.rs @@ -42,7 +42,6 @@ impl CardStateUpdater { mod test { use super::*; use crate::card::CardType; - use crate::collection::open_test_collection; use crate::prelude::*; use crate::scheduler::answering::CardAnswer; use crate::scheduler::answering::Rating; @@ -52,7 +51,7 @@ mod test { #[test] fn preview() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut c = Card { deck_id: DeckId(1), ctype: CardType::Learn, diff --git a/rslib/src/scheduler/bury_and_suspend.rs b/rslib/src/scheduler/bury_and_suspend.rs index 97b00ce7d..5a5825262 100644 --- a/rslib/src/scheduler/bury_and_suspend.rs +++ b/rslib/src/scheduler/bury_and_suspend.rs @@ -175,14 +175,13 @@ impl CardQueue { mod test { use crate::card::Card; use crate::card::CardQueue; - use crate::collection::open_test_collection; use crate::collection::Collection; use crate::search::SortMode; use crate::search::StateKind; #[test] fn unbury() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut card = Card { queue: CardQueue::UserBuried, ..Default::default() diff --git a/rslib/src/scheduler/congrats.rs b/rslib/src/scheduler/congrats.rs index 22f11c333..02e6651a7 100644 --- a/rslib/src/scheduler/congrats.rs +++ b/rslib/src/scheduler/congrats.rs @@ -44,11 +44,11 @@ impl Collection { #[cfg(test)] mod test { - use crate::collection::open_test_collection; + use super::*; #[test] fn empty() { - let mut col = open_test_collection(); + let mut col = Collection::new(); let info = col.congrats_info().unwrap(); assert_eq!( info, diff --git a/rslib/src/scheduler/filtered/custom_study.rs b/rslib/src/scheduler/filtered/custom_study.rs index 8617e7c35..afae1980d 100644 --- a/rslib/src/scheduler/filtered/custom_study.rs +++ b/rslib/src/scheduler/filtered/custom_study.rs @@ -299,7 +299,6 @@ fn tags_to_nodes(tags_to_include: &[String], tags_to_exclude: &[String]) -> Sear #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::pb::scheduler::custom_study_request::cram::CramKind; use crate::pb::scheduler::custom_study_request::Cram; use crate::pb::scheduler::custom_study_request::Value; @@ -307,7 +306,7 @@ mod test { #[test] fn tag_remembering() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); diff --git a/rslib/src/scheduler/queue/builder/mod.rs b/rslib/src/scheduler/queue/builder/mod.rs index e35d87612..68a3a9e50 100644 --- a/rslib/src/scheduler/queue/builder/mod.rs +++ b/rslib/src/scheduler/queue/builder/mod.rs @@ -270,7 +270,6 @@ mod test { use super::*; use crate::card::CardQueue; use crate::card::CardType; - use crate::collection::open_test_collection; use crate::pb::deckconfig::deck_config::config::NewCardGatherPriority; use crate::pb::deckconfig::deck_config::config::NewCardSortOrder; @@ -346,9 +345,9 @@ mod test { // ┣━━child━━grandchild // ┗━━child_2 let mut parent = DeckAdder::new("parent").add(&mut col); - let mut child = DeckAdder::new("parent\x1fchild").add(&mut col); - let child_2 = DeckAdder::new("parent\x1fchild_2").add(&mut col); - let grandchild = DeckAdder::new("parent\x1fchild\x1fgrandchild").add(&mut col); + let mut child = DeckAdder::new("parent::child").add(&mut col); + let child_2 = DeckAdder::new("parent::child_2").add(&mut col); + let grandchild = DeckAdder::new("parent::child::grandchild").add(&mut col); // add 2 new cards to each deck for deck in [&parent, &child, &child_2, &grandchild] { @@ -402,7 +401,7 @@ mod test { #[test] fn review_queue_building() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); col.set_config_bool(BoolKey::Sched2021, true, false)?; let mut deck = col.get_or_create_normal_deck("Default").unwrap(); diff --git a/rslib/src/scheduler/queue/undo.rs b/rslib/src/scheduler/queue/undo.rs index 2583a6e4e..ef7cbb68c 100644 --- a/rslib/src/scheduler/queue/undo.rs +++ b/rslib/src/scheduler/queue/undo.rs @@ -66,7 +66,6 @@ impl Collection { mod test { use crate::card::CardQueue; use crate::card::CardType; - use crate::collection::open_test_collection; use crate::deckconfig::LeechAction; use crate::prelude::*; @@ -86,7 +85,7 @@ mod test { #[test] fn undo() -> Result<()> { // add a note - let mut col = open_test_collection(); + let mut col = Collection::new(); let nid = add_note(&mut col, true)?; // turn burying and leech suspension on @@ -193,7 +192,7 @@ mod test { #[test] fn undo_counts() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); if col.timing_today()?.near_cutoff() { return Ok(()); } @@ -247,7 +246,7 @@ mod test { #[test] fn redo_after_queue_invalidation_bug() -> Result<()> { // add a note to the default deck - let mut col = open_test_collection(); + let mut col = Collection::new(); let _nid = add_note(&mut col, true)?; // add a deck and select it diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index 2b6014277..dcb6d942d 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -375,7 +375,7 @@ impl SqlWriter<'_> { .as_native_str(), ) } else { - NativeDeckName::from_human_name(&to_re(deck)) + NativeDeckName::from_human_name(to_re(deck)) .as_native_str() .to_string() }; diff --git a/rslib/src/stats/card.rs b/rslib/src/stats/card.rs index c91989ae7..4a2f385c0 100644 --- a/rslib/src/stats/card.rs +++ b/rslib/src/stats/card.rs @@ -106,12 +106,11 @@ fn stats_revlog_entry(entry: &RevlogEntry) -> pb::stats::card_stats_response::St #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::search::SortMode; #[test] fn stats() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); diff --git a/rslib/src/storage/dbcheck/invalid_ids_count.sql b/rslib/src/storage/dbcheck/invalid_ids_count.sql new file mode 100644 index 000000000..a028adc14 --- /dev/null +++ b/rslib/src/storage/dbcheck/invalid_ids_count.sql @@ -0,0 +1,13 @@ +SELECT ( + SELECT COUNT(*) + FROM notes + WHERE id > :cutoff + ) + ( + SELECT COUNT(*) + FROM cards + WHERE id > :cutoff + ) + ( + SELECT COUNT(*) + FROM revlog + WHERE id > :cutoff + ); \ No newline at end of file diff --git a/rslib/src/storage/dbcheck/invalid_ids_create.sql b/rslib/src/storage/dbcheck/invalid_ids_create.sql new file mode 100644 index 000000000..b14d2e9c5 --- /dev/null +++ b/rslib/src/storage/dbcheck/invalid_ids_create.sql @@ -0,0 +1,27 @@ +DROP TABLE IF EXISTS invalid_ids; +CREATE TEMPORARY TABLE invalid_ids AS WITH max_existing_valid_id AS ( + SELECT coalesce(max(id), 0) AS max_id + FROM "{source_table}" + WHERE id <= "{max_valid_id}" +), +first_new_id AS ( + SELECT CASE + WHEN "{new_id}" > ( + SELECT max_id + FROM max_existing_valid_id + ) THEN "{new_id}" + ELSE ( + SELECT max_id + FROM max_existing_valid_id + ) + 1 + END AS id +) +SELECT id, + ( + SELECT id + FROM first_new_id + ) + row_number() OVER ( + ORDER BY id + ) - 1 AS new_id +FROM "{source_table}" +WHERE id > "{max_valid_id}"; \ No newline at end of file diff --git a/rslib/src/storage/dbcheck/invalid_ids_drop.sql b/rslib/src/storage/dbcheck/invalid_ids_drop.sql new file mode 100644 index 000000000..4327ef1d2 --- /dev/null +++ b/rslib/src/storage/dbcheck/invalid_ids_drop.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS invalid_ids; \ No newline at end of file diff --git a/rslib/src/storage/dbcheck/invalid_ids_update.sql b/rslib/src/storage/dbcheck/invalid_ids_update.sql new file mode 100644 index 000000000..9f5bc3706 --- /dev/null +++ b/rslib/src/storage/dbcheck/invalid_ids_update.sql @@ -0,0 +1,11 @@ +UPDATE "{target_table}" +SET "{id_column}" = ( + SELECT invalid_ids.new_id + FROM invalid_ids + WHERE invalid_ids.id = "{target_table}"."{id_column}" + LIMIT 1 + ) +WHERE "{target_table}"."{id_column}" IN ( + SELECT invalid_ids.id + FROM invalid_ids + ); \ No newline at end of file diff --git a/rslib/src/storage/dbcheck/mod.rs b/rslib/src/storage/dbcheck/mod.rs new file mode 100644 index 000000000..e9ffcf91a --- /dev/null +++ b/rslib/src/storage/dbcheck/mod.rs @@ -0,0 +1,115 @@ +// Copyright: Ankitects Pty Ltd and contributors +// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +use crate::prelude::*; + +impl super::SqliteStorage { + /// True if any ids used as timestamps are larger than `cutoff`. + pub(crate) fn invalid_ids(&self, cutoff: i64) -> Result { + Ok(self + .db + .query_row_and_then(include_str!("invalid_ids_count.sql"), [cutoff], |r| { + r.get(0) + })?) + } + + /// Ensures all ids used as timestamps are `max_valid_id` or lower. + /// If not, new ids will be assigned starting at whichever is larger, + /// `new_id` or the next free valid id. + /// `new_id` must be a valid id, i.e. lower or equal to `max_valid_id`. + pub(crate) fn fix_invalid_ids(&self, max_valid_id: i64, new_id: i64) -> Result<()> { + require!(new_id <= max_valid_id, "new_id is invalid"); + for (source_table, foreign_table) in [ + ("notes", Some(("cards", "nid"))), + ("cards", Some(("revlog", "cid"))), + ("revlog", None), + ] { + self.setup_invalid_ids_table(source_table, max_valid_id, new_id)?; + self.update_invalid_ids_from_table(source_table, "id")?; + if let Some((target_table, id_column)) = foreign_table { + self.update_invalid_ids_from_table(target_table, id_column)?; + } + } + self.db.execute(include_str!("invalid_ids_drop.sql"), [])?; + Ok(()) + } + + fn setup_invalid_ids_table( + &self, + source_table: &str, + max_valid_id: i64, + new_id: i64, + ) -> Result<()> { + self.db.execute_batch(&format!( + include_str!("invalid_ids_create.sql"), + source_table = source_table, + max_valid_id = max_valid_id, + new_id = new_id, + ))?; + Ok(()) + } + + /// Fix the invalid ids in `id_column` of `target_table` using the map from + /// the invalid ids temporary table. + fn update_invalid_ids_from_table(&self, target_table: &str, id_column: &str) -> Result<()> { + self.db.execute_batch(&format!( + include_str!("invalid_ids_update.sql"), + target_table = target_table, + id_column = id_column, + ))?; + Ok(()) + } +} + +#[cfg(test)] +mod test { + use crate::prelude::*; + + #[test] + fn any_invalid_ids() { + let mut col = Collection::new(); + assert_eq!(col.storage.invalid_ids(0).unwrap(), 0); + NoteAdder::basic(&mut col).add(&mut col); + // 1 card and 1 note + assert_eq!(col.storage.invalid_ids(0).unwrap(), 2); + assert_eq!( + col.storage.invalid_ids(TimestampMillis::now().0).unwrap(), + 0 + ); + } + + #[test] + fn fix_invalid_note_ids_only_and_update_cards() { + let mut col = Collection::new(); + let valid = NoteAdder::basic(&mut col).add(&mut col); + NoteAdder::basic(&mut col).add(&mut col); + col.storage.fix_invalid_ids(valid.id.0, 42).unwrap(); + assert_eq!(col.storage.all_cards_of_note(valid.id).unwrap().len(), 1); + assert_eq!(col.storage.all_cards_of_note(NoteId(42)).unwrap().len(), 1); + } + + #[test] + fn fix_invalid_card_ids_only() { + let mut col = Collection::new(); + let mut cards = CardAdder::new().siblings(3).add(&mut col); + col.storage.fix_invalid_ids(cards[0].id.0, 42).unwrap(); + cards.sort_by(|c1, c2| c1.id.cmp(&c2.id)); + cards[1].id.0 = 42; + cards[2].id.0 = 43; + let old_first_card = cards.remove(0); + cards.push(old_first_card); + let mut new_cards = col.storage.get_all_cards(); + new_cards.sort_by(|c1, c2| c1.id.cmp(&c2.id)); + assert_eq!(new_cards, cards); + } + + #[test] + fn update_revlog_when_fixing_card_ids() { + let mut col = Collection::new(); + CardAdder::new().due_dates(["7"]).add(&mut col); + col.storage.fix_invalid_ids(42, 42).unwrap(); + // revlog id was also reset to 42 + let revlog_entry = col.storage.get_revlog_entry(RevlogId(42)).unwrap().unwrap(); + assert_eq!(revlog_entry.cid.0, 42); + } +} diff --git a/rslib/src/storage/mod.rs b/rslib/src/storage/mod.rs index 15d553aa8..6a0403bb3 100644 --- a/rslib/src/storage/mod.rs +++ b/rslib/src/storage/mod.rs @@ -4,6 +4,7 @@ pub(crate) mod card; mod collection_timestamps; mod config; +mod dbcheck; mod deck; mod deckconfig; mod graves; diff --git a/rslib/src/tags/findreplace.rs b/rslib/src/tags/findreplace.rs index 1f5d7e2ee..5db6e3ed2 100644 --- a/rslib/src/tags/findreplace.rs +++ b/rslib/src/tags/findreplace.rs @@ -101,12 +101,11 @@ where #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::decks::DeckId; #[test] fn find_replace() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); note.tags.push("test".into()); diff --git a/rslib/src/tags/register.rs b/rslib/src/tags/register.rs index bfb1fbc80..ad287506f 100644 --- a/rslib/src/tags/register.rs +++ b/rslib/src/tags/register.rs @@ -195,12 +195,11 @@ pub(super) fn normalize_tag_name(name: &str) -> Result> { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::decks::DeckId; #[test] fn tags() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); col.add_note(&mut note, DeckId(1))?; diff --git a/rslib/src/tags/remove.rs b/rslib/src/tags/remove.rs index edfd6ea8c..15606db1d 100644 --- a/rslib/src/tags/remove.rs +++ b/rslib/src/tags/remove.rs @@ -97,12 +97,11 @@ impl Collection { #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; use crate::tags::Tag; #[test] fn clearing() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); note.tags.push("one".into()); diff --git a/rslib/src/tags/reparent.rs b/rslib/src/tags/reparent.rs index 960fbd0ab..5a1b73567 100644 --- a/rslib/src/tags/reparent.rs +++ b/rslib/src/tags/reparent.rs @@ -114,7 +114,6 @@ fn reparented_name(existing_name: &str, new_parent: Option<&str>) -> Option Vec { col.storage @@ -127,7 +126,7 @@ mod test { #[test] fn dragdrop() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); for tag in &[ "a", diff --git a/rslib/src/tags/tree.rs b/rslib/src/tags/tree.rs index 7f68b9671..0efeed63b 100644 --- a/rslib/src/tags/tree.rs +++ b/rslib/src/tags/tree.rs @@ -124,7 +124,6 @@ fn add_tag_and_missing_parents<'a, 'b>( #[cfg(test)] mod test { use super::*; - use crate::collection::open_test_collection; fn node(name: &str, level: u32, children: Vec) -> TagTreeNode { TagTreeNode { @@ -141,7 +140,7 @@ mod test { #[test] fn tree() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); note.tags.push("foo::bar::a".into()); diff --git a/rslib/src/tests.rs b/rslib/src/tests.rs index 4ea7f63d2..744d76cc4 100644 --- a/rslib/src/tests.rs +++ b/rslib/src/tests.rs @@ -8,7 +8,6 @@ use itertools::Itertools; use tempfile::tempdir; use tempfile::TempDir; -use crate::collection::open_test_collection; use crate::collection::CollectionBuilder; use crate::deckconfig::DeckConfigInner; use crate::io::create_dir; @@ -28,14 +27,14 @@ pub(crate) fn open_fs_test_collection(name: &str) -> (Collection, TempDir) { } pub(crate) fn open_test_collection_with_learning_card() -> Collection { - let mut col = open_test_collection(); + let mut col = Collection::new(); NoteAdder::basic(&mut col).add(&mut col); col.answer_again(); col } pub(crate) fn open_test_collection_with_relearning_card() -> Collection { - let mut col = open_test_collection(); + let mut col = Collection::new(); NoteAdder::basic(&mut col).add(&mut col); col.answer_easy(); col.storage @@ -48,8 +47,12 @@ pub(crate) fn open_test_collection_with_relearning_card() -> Collection { } impl Collection { + pub(crate) fn new() -> Collection { + CollectionBuilder::default().build().unwrap() + } + pub(crate) fn new_v3() -> Collection { - let mut col = open_test_collection(); + let mut col = Collection::new(); col.set_config_bool(BoolKey::Sched2021, true, false) .unwrap(); col @@ -109,9 +112,9 @@ pub(crate) struct DeckAdder { } impl DeckAdder { - pub(crate) fn new(machine_name: impl Into) -> Self { + pub(crate) fn new(human_name: impl AsRef) -> Self { Self { - name: NativeDeckName::from_native_str(machine_name), + name: NativeDeckName::from_human_name(human_name), ..Default::default() } } diff --git a/rslib/src/undo/mod.rs b/rslib/src/undo/mod.rs index a64b3017c..eddbf7887 100644 --- a/rslib/src/undo/mod.rs +++ b/rslib/src/undo/mod.rs @@ -332,12 +332,11 @@ impl From<&[UndoableChange]> for StateChanges { mod test { use super::UndoableChange; use crate::card::Card; - use crate::collection::open_test_collection; use crate::prelude::*; #[test] fn undo() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); let mut card = Card { interval: 1, @@ -445,7 +444,7 @@ mod test { #[test] fn custom() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); // perform some actions in separate steps let nt = col.get_notetype_by_name("Basic")?.unwrap(); @@ -509,7 +508,7 @@ mod test { #[test] fn undo_mtime_bump() -> Result<()> { - let mut col = open_test_collection(); + let mut col = Collection::new(); col.storage.db.execute_batch("update col set mod = 0")?; // a no-op change should not bump mtime