Refactor card and revlog importing

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<NoteId>,
+    imported_notes: &'a HashMap<NoteId, NoteId>,
-    remapped_notes: &'a HashMap<NoteId, NoteId>,
     remapped_decks: &'a HashMap<DeckId, DeckId>,
 
     /// 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<NoteId, NoteId>,
+        imported_notes: &'a HashMap<NoteId, NoteId>,
         remapped_decks: &'a HashMap<DeckId, DeckId>,
-        conflicting_notes: &'a HashSet<NoteId>,
     ) -> Result<Self> {
         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,
+            imported_notes,
-            remapped_notes,
             remapped_decks,
             targets,
             collection_delta,
@@ -68,34 +66,54 @@ impl Collection {
 }
 
 impl<'a> Context<'a> {
-    pub(super) fn import_cards(&mut self) -> Result<HashMap<CardId, CardId>> {
+    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<Card>) -> Result<()> {
+    fn import_cards(&mut self, mut cards: Vec<Card>) -> Result<()> {
         for card in &mut cards {
-            if !self.conflicting_notes.contains(&card.note_id) {
+            if self.map_to_imported_note(card) && !self.target_already_exists(card) {
-                self.remap_note_id(card);
+                self.add_card(card)?;
-                if !self.targets.contains(&(card.note_id, card.template_idx)) {
+            }
-                    self.add_card(card)?;
+            // TODO: could update existing card
-                }
+        }
-                // TODO: maybe update
+        Ok(())
+    }
+
+    fn import_revlog(&mut self, revlog: Vec<RevlogEntry>) -> 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) {

rslib/src/import_export/package/apkg/import/mod.rs

@@ -40,7 +40,7 @@ struct Context<'a> {
     archive: ZipArchive<File>,
     guid_map: HashMap<String, NoteMeta>,
     remapped_notetypes: HashMap<NotetypeId, NotetypeId>,
-    remapped_notes: HashMap<NoteId, NoteId>,
+    imported_notes: HashMap<NoteId, NoteId>,
     existing_notes: HashSet<NoteId>,
     remapped_decks: HashMap<DeckId, DeckId>,
     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<String, (bool, SafeMediaEntry)>,
-    /// 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<NoteId>,
-    remapped_cards: HashMap<CardId, CardId>,
     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_cards_and_revlog()?;
-        self.import_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(&note.notetype_id) {
                 if self.guid_map.contains_key(&note.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(&notetype, 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(&note.id) {
             note.id.0 += 999;
         }
-        if original != note.id {
+        original
-            self.remapped_notes.insert(original, note.id);
-        }
     }
 
     fn get_expected_notetype(&mut self, ntid: NotetypeId) -> Result<Arc<Notetype>> {
@@ -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 {