From a402879e72dc3736c2ee7b6c57590c8ca0e639fa Mon Sep 17 00:00:00 2001 From: RumovZ Date: Sun, 10 Apr 2022 11:03:46 +0200 Subject: [PATCH] Refactor card and revlog importing --- .../package/apkg/import/cards.rs | 58 +++++++++++-------- .../import_export/package/apkg/import/mod.rs | 39 ++++--------- 2 files changed, 45 insertions(+), 52 deletions(-) diff --git a/rslib/src/import_export/package/apkg/import/cards.rs b/rslib/src/import_export/package/apkg/import/cards.rs index 04704c71b..dea7d724d 100644 --- a/rslib/src/import_export/package/apkg/import/cards.rs +++ b/rslib/src/import_export/package/apkg/import/cards.rs @@ -11,14 +11,14 @@ use crate::{ card::{CardQueue, CardType}, config::SchedulerVersion, prelude::*, + revlog::RevlogEntry, }; struct CardContext<'a> { target_col: &'a mut Collection, usn: Usn, - conflicting_notes: &'a HashSet, - remapped_notes: &'a HashMap, + imported_notes: &'a HashMap, remapped_decks: &'a HashMap, /// The number of days the source collection is ahead of the target collection @@ -37,9 +37,8 @@ impl<'c> CardContext<'c> { usn: Usn, days_elapsed: u32, target_col: &'a mut Collection, - remapped_notes: &'a HashMap, + imported_notes: &'a HashMap, remapped_decks: &'a HashMap, - conflicting_notes: &'a HashSet, ) -> Result { let targets = target_col.storage.all_cards_as_nid_and_ord()?; let collection_delta = target_col.collection_delta(days_elapsed)?; @@ -48,8 +47,7 @@ impl<'c> CardContext<'c> { Ok(Self { target_col, usn, - conflicting_notes, - remapped_notes, + imported_notes, remapped_decks, targets, collection_delta, @@ -68,34 +66,54 @@ impl Collection { } impl<'a> Context<'a> { - pub(super) fn import_cards(&mut self) -> Result> { + pub(super) fn import_cards_and_revlog(&mut self) -> Result<()> { let mut ctx = CardContext::new( self.usn, self.data.days_elapsed, self.target_col, - &self.remapped_notes, + &self.imported_notes, &self.remapped_decks, - &self.conflicting_notes, )?; ctx.import_cards(mem::take(&mut self.data.cards))?; - Ok(ctx.imported_cards) + ctx.import_revlog(mem::take(&mut self.data.revlog)) } } impl CardContext<'_> { - pub(super) fn import_cards(&mut self, mut cards: Vec) -> Result<()> { + fn import_cards(&mut self, mut cards: Vec) -> Result<()> { for card in &mut cards { - if !self.conflicting_notes.contains(&card.note_id) { - self.remap_note_id(card); - if !self.targets.contains(&(card.note_id, card.template_idx)) { - self.add_card(card)?; - } - // TODO: maybe update + if self.map_to_imported_note(card) && !self.target_already_exists(card) { + self.add_card(card)?; + } + // TODO: could update existing card + } + Ok(()) + } + + fn import_revlog(&mut self, revlog: Vec) -> Result<()> { + for mut entry in revlog { + if let Some(cid) = self.imported_cards.get(&entry.cid) { + entry.cid = *cid; + entry.usn = self.usn; + self.target_col.add_revlog_entry_if_unique_undoable(entry)?; } } Ok(()) } + fn map_to_imported_note(&self, card: &mut Card) -> bool { + if let Some(nid) = self.imported_notes.get(&card.note_id) { + card.note_id = *nid; + true + } else { + false + } + } + + fn target_already_exists(&self, card: &Card) -> bool { + self.targets.contains(&(card.note_id, card.template_idx)) + } + fn add_card(&mut self, card: &mut Card) -> Result<()> { card.usn = self.usn; self.remap_deck_id(card); @@ -110,12 +128,6 @@ impl CardContext<'_> { Ok(()) } - fn remap_note_id(&self, card: &mut Card) { - if let Some(nid) = self.remapped_notes.get(&card.note_id) { - card.note_id = *nid; - } - } - fn uniquify_card_id(&mut self, card: &mut Card) -> CardId { let original = card.id; while self.target_ids.contains(&card.id) { diff --git a/rslib/src/import_export/package/apkg/import/mod.rs b/rslib/src/import_export/package/apkg/import/mod.rs index b0254de1b..0fe0c5c54 100644 --- a/rslib/src/import_export/package/apkg/import/mod.rs +++ b/rslib/src/import_export/package/apkg/import/mod.rs @@ -40,7 +40,7 @@ struct Context<'a> { archive: ZipArchive, guid_map: HashMap, remapped_notetypes: HashMap, - remapped_notes: HashMap, + imported_notes: HashMap, existing_notes: HashSet, remapped_decks: HashMap, data: ExchangeData, @@ -50,10 +50,6 @@ struct Context<'a> { /// original, normalized file name → (refererenced on import material, /// entry with possibly remapped file name) used_media_entries: HashMap, - /// Source notes that cannot be imported, because notes with the same guid - /// exist in the target, but their notetypes don't match. - conflicting_notes: HashSet, - remapped_cards: HashMap, normalize_notes: bool, } @@ -139,12 +135,10 @@ impl<'a> Context<'a> { data, guid_map, usn, - conflicting_notes: HashSet::new(), remapped_notetypes: HashMap::new(), - remapped_notes: HashMap::new(), + imported_notes: HashMap::new(), existing_notes, remapped_decks: HashMap::new(), - remapped_cards: HashMap::new(), used_media_entries: HashMap::new(), normalize_notes, }) @@ -156,8 +150,7 @@ impl<'a> Context<'a> { self.import_notes()?; self.import_deck_configs()?; self.import_decks()?; - self.import_cards()?; - self.import_revlog()?; + self.import_cards_and_revlog()?; self.copy_media() } @@ -233,7 +226,6 @@ impl<'a> Context<'a> { for mut note in mem::take(&mut self.data.notes) { if let Some(notetype_id) = self.remapped_notetypes.get(¬e.notetype_id) { if self.guid_map.contains_key(¬e.guid) { - self.conflicting_notes.insert(note.id); // TODO: Log ignore } else { note.notetype_id = *notetype_id; @@ -255,21 +247,21 @@ impl<'a> Context<'a> { let notetype = self.get_expected_notetype(note.notetype_id)?; note.prepare_for_update(¬etype, self.normalize_notes)?; note.usn = self.usn; - self.uniquify_note_id(note); + let old_id = self.uniquify_note_id(note); self.target_col.add_note_only_with_id_undoable(note)?; self.existing_notes.insert(note.id); + self.imported_notes.insert(old_id, note.id); + Ok(()) } - fn uniquify_note_id(&mut self, note: &mut Note) { + fn uniquify_note_id(&mut self, note: &mut Note) -> NoteId { let original = note.id; while self.existing_notes.contains(¬e.id) { note.id.0 += 999; } - if original != note.id { - self.remapped_notes.insert(original, note.id); - } + original } fn get_expected_notetype(&mut self, ntid: NotetypeId) -> Result> { @@ -288,15 +280,15 @@ impl<'a> Context<'a> { fn maybe_update_note(&mut self, note: &mut Note, meta: NoteMeta) -> Result<()> { if meta.mtime < note.mtime { if meta.notetype_id == note.notetype_id { - self.remapped_notes.insert(note.id, meta.id); + self.imported_notes.insert(note.id, meta.id); note.id = meta.id; self.update_note(note)?; } else { - self.conflicting_notes.insert(note.id); // TODO: Log ignore } } else { // TODO: Log duplicate + self.imported_notes.insert(note.id, meta.id); } Ok(()) } @@ -396,17 +388,6 @@ impl<'a> Context<'a> { .get_deck_by_name(deck.name.as_native_str()) } - fn import_revlog(&mut self) -> Result<()> { - for mut entry in mem::take(&mut self.data.revlog) { - if let Some(cid) = self.remapped_cards.get(&entry.cid) { - entry.cid = *cid; - entry.usn = self.usn; - self.target_col.add_revlog_entry_if_unique_undoable(entry)?; - } - } - Ok(()) - } - fn copy_media(&mut self) -> Result<()> { for (used, entry) in self.used_media_entries.values() { if *used {