add note/card removal to backend

proto/backend.proto

@@ -127,6 +127,7 @@ service BackendService {
     rpc GetCard (CardID) returns (Card);
     rpc UpdateCard (Card) returns (Empty);
     rpc AddCard (Card) returns (CardID);
+    rpc RemoveCards (RemoveCardsIn) returns (Empty);
 
     // notes
 
@@ -134,6 +135,7 @@ service BackendService {
     rpc AddNote (AddNoteIn) returns (NoteID);
     rpc UpdateNote (Note) returns (Empty);
     rpc GetNote (NoteID) returns (Note);
+    rpc RemoveNotes (RemoveNotesIn) returns (Empty);
     rpc AddNoteTags (AddNoteTagsIn) returns (UInt32);
     rpc UpdateNoteTags (UpdateNoteTagsIn) returns (UInt32);
     rpc ClozeNumbersInNote (Note) returns (ClozeNumbersInNoteOut);
@@ -933,3 +935,12 @@ message SyncAuth {
     string hkey = 1;
     uint32 host_number = 2;
 }
+
+message RemoveNotesIn {
+    repeated int64 note_ids = 1;
+    repeated int64 card_ids = 2;
+}
+
+message RemoveCardsIn {
+    repeated int64 card_ids = 1;
+}

pylib/anki/collection.py

@@ -10,7 +10,7 @@ import re
 import time
 import traceback
 import weakref
-from typing import TYPE_CHECKING, Any, Iterable, List, Optional, Sequence, Tuple, Union
+from typing import TYPE_CHECKING, Any, List, Optional, Sequence, Tuple, Union
 
 import anki.find
 import anki.latex  # sets up hook
@@ -343,24 +343,29 @@ class Collection:
     def add_note(self, note: Note, deck_id: int) -> None:
         note.id = self.backend.add_note(note=note.to_backend_note(), deck_id=deck_id)
 
+    def remove_notes(self, note_ids: Sequence[int]) -> None:
+        hooks.notes_will_be_deleted(self, note_ids)
+        self.backend.remove_notes(note_ids=note_ids, card_ids=[])
+
+    def remove_notes_by_card(self, card_ids: List[int]) -> None:
+        if hooks.notes_will_be_deleted.count():
+            nids = self.db.list(
+                "select nid from cards where id in " + ids2str(card_ids)
+            )
+            hooks.notes_will_be_deleted(self, nids)
+        self.backend.remove_notes(note_ids=[], card_ids=card_ids)
+
+    # legacy
+
     def addNote(self, note: Note) -> int:
         self.add_note(note, note.model()["did"])
         return len(note.cards())
 
-    def remNotes(self, ids: Iterable[int]) -> None:
+    def remNotes(self, ids: Sequence[int]) -> None:
-        """Deletes notes with the given IDs."""
+        self.remove_notes(ids)
-        self.remCards(self.db.list("select id from cards where nid in " + ids2str(ids)))
 
     def _remNotes(self, ids: List[int]) -> None:
-        """Bulk delete notes by ID. Don't call this directly."""
+        pass
-        if not ids:
-            return
-        strids = ids2str(ids)
-        # we need to log these independently of cards, as one side may have
-        # more card templates
-        hooks.notes_will_be_deleted(self, ids)
-        self._logRem(ids, REM_NOTE)
-        self.db.execute("delete from notes where id in %s" % strids)
 
     # Cards
     ##########################################################################
@@ -371,24 +376,14 @@ class Collection:
     def cardCount(self) -> Any:
         return self.db.scalar("select count() from cards")
 
+    def remove_cards_and_orphaned_notes(self, card_ids: Sequence[int]):
+        "You probably want .remove_notes_by_card() instead."
+        self.backend.remove_cards(card_ids=card_ids)
+
+    # legacy
+
     def remCards(self, ids: List[int], notes: bool = True) -> None:
-        "Bulk delete cards by ID."
+        self.remove_cards_and_orphaned_notes(ids)
-        if not ids:
-            return
-        sids = ids2str(ids)
-        nids = self.db.list("select nid from cards where id in " + sids)
-        # remove cards
-        self._logRem(ids, REM_CARD)
-        self.db.execute("delete from cards where id in " + sids)
-        # then notes
-        if not notes:
-            return
-        nids = self.db.list(
-            """
-select id from notes where id in %s and id not in (select nid from cards)"""
-            % ids2str(nids)
-        )
-        self._remNotes(nids)
 
     def emptyCids(self) -> List[int]:
         print("emptyCids() will go away")

pylib/anki/models.py

@@ -466,7 +466,7 @@ and notes.mid = ? and cards.ord = ?""",
             else:
                 deleted.append(cid)
         self.col.db.executemany("update cards set ord=?,usn=?,mod=? where id=?", d)
-        self.col.remCards(deleted)
+        self.col.remove_cards_and_orphaned_notes(deleted)
 
     # Schema hash
     ##########################################################################

pylib/tests/test_cards.py

@@ -12,7 +12,7 @@ def test_delete():
     cid = f.cards()[0].id
     deck.reset()
     deck.sched.answerCard(deck.sched.getCard(), 2)
-    deck.remCards([cid])
+    deck.remove_cards_and_orphaned_notes([cid])
     assert deck.cardCount() == 0
     assert deck.noteCount() == 0
     assert deck.db.scalar("select count() from notes") == 0
@@ -53,7 +53,7 @@ def test_genrem():
     mm.save(m, templates=True)
     rep = d.backend.get_empty_cards()
     for note in rep.notes:
-        d.remCards(note.card_ids)
+        d.remove_cards_and_orphaned_notes(note.card_ids)
     assert len(f.cards()) == 1
     # if we add to the note, a card should be automatically generated
     f.load()

pylib/tests/test_importing.py

@@ -46,7 +46,7 @@ def test_anki2_mediadupes():
     imp.run()
     assert os.listdir(empty.media.dir()) == ["foo.mp3"]
     # and importing again will not duplicate, as the file content matches
-    empty.remCards(empty.db.list("select id from cards"))
+    empty.remove_cards_and_orphaned_notes(empty.db.list("select id from cards"))
     imp = Anki2Importer(empty, tmp.path)
     imp.run()
     assert os.listdir(empty.media.dir()) == ["foo.mp3"]
@@ -54,7 +54,7 @@ def test_anki2_mediadupes():
     assert "foo.mp3" in n.fields[0]
     # if the local file content is different, and import should trigger a
     # rename
-    empty.remCards(empty.db.list("select id from cards"))
+    empty.remove_cards_and_orphaned_notes(empty.db.list("select id from cards"))
     with open(os.path.join(empty.media.dir(), "foo.mp3"), "w") as f:
         f.write("bar")
     imp = Anki2Importer(empty, tmp.path)
@@ -64,7 +64,7 @@ def test_anki2_mediadupes():
     assert "_" in n.fields[0]
     # if the localized media file already exists, we rewrite the note and
     # media
-    empty.remCards(empty.db.list("select id from cards"))
+    empty.remove_cards_and_orphaned_notes(empty.db.list("select id from cards"))
     with open(os.path.join(empty.media.dir(), "foo.mp3"), "w") as f:
         f.write("bar")
     imp = Anki2Importer(empty, tmp.path)
@@ -83,12 +83,12 @@ def test_apkg():
     imp.run()
     assert os.listdir(tmp.media.dir()) == ["foo.wav"]
     # importing again should be idempotent in terms of media
-    tmp.remCards(tmp.db.list("select id from cards"))
+    tmp.remove_cards_and_orphaned_notes(tmp.db.list("select id from cards"))
     imp = AnkiPackageImporter(tmp, apkg)
     imp.run()
     assert os.listdir(tmp.media.dir()) == ["foo.wav"]
     # but if the local file has different data, it will rename
-    tmp.remCards(tmp.db.list("select id from cards"))
+    tmp.remove_cards_and_orphaned_notes(tmp.db.list("select id from cards"))
     with open(os.path.join(tmp.media.dir(), "foo.wav"), "w") as f:
         f.write("xyz")
     imp = AnkiPackageImporter(tmp, apkg)

qt/aqt/browser.py

@@ -1611,7 +1611,7 @@ where id in %s"""
         else:
             # last selection at top; place one above topmost selection
             newRow = min(selectedRows) - 1
-        self.col.remNotes(nids)
+        self.col.remove_notes(nids)
         self.search()
         if len(self.model.cards):
             newRow = min(newRow, len(self.model.cards) - 1)

qt/aqt/emptycards.py

@@ -95,5 +95,5 @@ class EmptyCardsDialog(QDialog):
             else:
                 to_delete.extend(note.card_ids)
 
-        self.mw.col.remCards(to_delete)
+        self.mw.col.remove_cards_and_orphaned_notes(to_delete)
         return len(to_delete)

qt/aqt/main.py

@@ -1279,7 +1279,7 @@ and if the problem comes up again, please ask on the support site."""
     # Log note deletion
     ##########################################################################
 
-    def onRemNotes(self, col: Collection, nids: List[int]) -> None:
+    def onRemNotes(self, col: Collection, nids: Sequence[int]) -> None:
         path = os.path.join(self.pm.profileFolder(), "deleted.txt")
         existed = os.path.exists(path)
         with open(path, "ab") as f:

qt/aqt/reviewer.py

@@ -791,7 +791,7 @@ time = %(time)d;
             return
         self.mw.checkpoint(_("Delete"))
         cnt = len(self.card.note().cards())
-        self.mw.col.remNotes([self.card.note().id])
+        self.mw.col.remove_notes([self.card.note().id])
         self.mw.reset()
         tooltip(
             ngettext(

rslib/src/backend/mod.rs

@@ -653,6 +653,21 @@ impl BackendService for Backend {
         Ok(pb::CardId { cid: card.id.0 })
     }
 
+    fn remove_cards(&mut self, input: pb::RemoveCardsIn) -> BackendResult<Empty> {
+        self.with_col(|col| {
+            col.transact(None, |col| {
+                col.remove_cards_and_orphaned_notes(
+                    &input
+                        .card_ids
+                        .into_iter()
+                        .map(Into::into)
+                        .collect::<Vec<_>>(),
+                )?;
+                Ok(().into())
+            })
+        })
+    }
+
     // notes
     //-------------------------------------------------------------------
 
@@ -688,6 +703,31 @@ impl BackendService for Backend {
         })
     }
 
+    fn remove_notes(&mut self, input: pb::RemoveNotesIn) -> BackendResult<Empty> {
+        self.with_col(|col| {
+            if !input.note_ids.is_empty() {
+                col.remove_notes(
+                    &input
+                        .note_ids
+                        .into_iter()
+                        .map(Into::into)
+                        .collect::<Vec<_>>(),
+                )?;
+            }
+            if !input.card_ids.is_empty() {
+                let nids = col.storage.note_ids_of_cards(
+                    &input
+                        .card_ids
+                        .into_iter()
+                        .map(Into::into)
+                        .collect::<Vec<_>>(),
+                )?;
+                col.remove_notes(&nids.into_iter().collect::<Vec<_>>())?
+            }
+            Ok(().into())
+        })
+    }
+
     fn add_note_tags(&mut self, input: pb::AddNoteTagsIn) -> BackendResult<pb::UInt32> {
         self.with_col(|col| {
             col.add_tags_for_notes(&to_nids(input.nids), &input.tags)

rslib/src/card.rs

@@ -206,7 +206,7 @@ impl Collection {
 
     /// Remove cards and any resulting orphaned notes.
     /// Expects a transaction.
-    pub(crate) fn remove_cards_inner(&mut self, cids: &[CardID]) -> Result<()> {
+    pub(crate) fn remove_cards_and_orphaned_notes(&mut self, cids: &[CardID]) -> Result<()> {
         let usn = self.usn()?;
         let mut nids = HashSet::new();
         for cid in cids {
@@ -225,6 +225,14 @@ impl Collection {
 
         Ok(())
     }
+
+    pub(crate) fn remove_card_only(&mut self, card: Card, usn: Usn) -> Result<()> {
+        // fixme: undo
+        self.storage.remove_card(card.id)?;
+        self.storage.add_card_grave(card.id, usn)?;
+
+        Ok(())
+    }
 }
 
 #[cfg(test)]

rslib/src/decks/mod.rs

@@ -420,7 +420,7 @@ impl Collection {
 
     fn delete_all_cards_in_normal_deck(&mut self, did: DeckID) -> Result<()> {
         let cids = self.storage.all_cards_in_single_deck(did)?;
-        self.remove_cards_inner(&cids)
+        self.remove_cards_and_orphaned_notes(&cids)
     }
 
     fn return_all_cards_in_filtered_deck(&mut self, did: DeckID) -> Result<()> {

rslib/src/notes.rs

@@ -321,6 +321,25 @@ impl Collection {
         Ok(())
     }
 
+    /// Remove provided notes, and any cards that use them.
+    pub(crate) fn remove_notes(&mut self, nids: &[NoteID]) -> Result<()> {
+        let usn = self.usn()?;
+        self.transact(None, |col| {
+            for nid in nids {
+                let nid = *nid;
+                if let Some(_existing_note) = col.storage.get_note(nid)? {
+                    // fixme: undo
+                    for card in col.storage.all_cards_of_note(nid)? {
+                        col.remove_card_only(card, usn)?;
+                    }
+                    col.remove_note_only(nid, usn)?;
+                }
+            }
+
+            Ok(())
+        })
+    }
+
     /// Update cards and field cache after notes modified externally.
     /// If gencards is false, skip card generation.
     pub(crate) fn after_note_updates(
@@ -501,7 +520,7 @@ mod test {
             1
         );
         let cids = col.search_cards("", SortMode::NoOrder)?;
-        col.remove_cards_inner(&cids)?;
+        col.remove_cards_and_orphaned_notes(&cids)?;
 
         // if normalization turned off, note text is entered as-is
         let mut note = nt.new_note();

rslib/src/storage/card/mod.rs

@@ -14,7 +14,7 @@ use rusqlite::{
     types::{FromSql, FromSqlError, ValueRef},
     OptionalExtension, Row, NO_PARAMS,
 };
-use std::{convert::TryFrom, result};
+use std::{collections::HashSet, convert::TryFrom, result};
 
 impl FromSql for CardType {
     fn column_result(value: ValueRef<'_>) -> std::result::Result<Self, FromSqlError> {
@@ -227,6 +227,29 @@ impl super::SqliteStorage {
             .map(|o| o.is_none())
             .map_err(Into::into)
     }
+
+    pub(crate) fn all_cards_of_note(&self, nid: NoteID) -> Result<Vec<Card>> {
+        self.db
+            .prepare_cached(concat!(include_str!("get_card.sql"), " where nid = ?"))?
+            .query_and_then(&[nid], |r| row_to_card(r).map_err(Into::into))?
+            .collect()
+    }
+
+    pub(crate) fn note_ids_of_cards(&self, cids: &[CardID]) -> Result<HashSet<NoteID>> {
+        let mut stmt = self
+            .db
+            .prepare_cached("select nid from cards where id = ?")?;
+        let mut nids = HashSet::new();
+        for cid in cids {
+            if let Some(nid) = stmt
+                .query_row(&[cid], |r| r.get::<_, NoteID>(0))
+                .optional()?
+            {
+                nids.insert(nid);
+            }
+        }
+        Ok(nids)
+    }
 }
 
 #[cfg(test)]

rslib/src/sync/mod.rs

@@ -1400,7 +1400,7 @@ mod test {
         }
 
         // fixme: inconsistent usn arg
-        col1.remove_cards_inner(&[cardid])?;
+        col1.remove_cards_and_orphaned_notes(&[cardid])?;
         col1.remove_note_only(noteid, col1.usn()?)?;
         col1.remove_deck_and_child_decks(deckid)?;
 

rslib/src/types.rs

@@ -52,6 +52,18 @@ macro_rules! define_newtype {
                 ))
             }
         }
+
+        impl From<$type> for $name {
+            fn from(t: $type) -> $name {
+                $name(t)
+            }
+        }
+
+        impl From<$name> for $type {
+            fn from(n: $name) -> $type {
+                n.0
+            }
+        }
     };
 }
 

rspy/src/lib.rs

@@ -113,6 +113,8 @@ fn want_release_gil(method: u32) -> bool {
             BackendMethod::SyncStatus => true,
             BackendMethod::FullUpload => true,
             BackendMethod::FullDownload => true,
+            BackendMethod::RemoveNotes => true,
+            BackendMethod::RemoveCards => true,
         }
     } else {
         false