perf: Speed up media_names_for_ankihub_deck (#1380)

* Use .iterator() in media_names_for_ankihub_deck() to avoid holding rows
in memory

* Extract media names from AnkiHub DB instead of collection

* Fix tests

* Avoid scanning the notes table twice and use .tuples()

* Revert "Extract media names from AnkiHub DB instead of collection"

This reverts commit bf3e891.

* Clarify why a collection scan is needed.

Author: abdnh

ankihub/db/db.py

@@ -448,28 +448,28 @@ def downloadable_media_for_ankihub_deck(self, ah_did: uuid.UUID) -> List[DeckMed
 
     def media_names_for_ankihub_deck(self, ah_did: uuid.UUID) -> Set[str]:
         """Returns the names of all media files which are referenced on notes in the given deck."""
-        notes = AnkiHubNote.select(AnkiHubNote.anki_note_type_id, AnkiHubNote.fields).filter(
-            NOTE_NOT_DELETED_CONDITION,
-            ankihub_deck_id=ah_did,
+        result: Set[str] = set()
+        note_types: Dict[int, Optional[NotetypeDict]] = {}
+        rows: Iterable[Tuple[int, Optional[Dict[str, str]]]] = (
+            AnkiHubNote.select(AnkiHubNote.anki_note_type_id, AnkiHubNote.fields)
+            .filter(NOTE_NOT_DELETED_CONDITION, ankihub_deck_id=ah_did)
+            .tuples()
+            .iterator()
         )
-        note_type_ids = set(note.anki_note_type_id for note in notes)
-        note_types: Dict[int, NotetypeDict] = {
-            nt.anki_note_type_id: nt.note_type_dict
-            for nt in AnkiHubNoteType.select(AnkiHubNoteType.anki_note_type_id, AnkiHubNoteType.note_type_dict).filter(
-                anki_note_type_id__in=note_type_ids
-            )
-        }
-        note_type_refs = {
-            name for note_type in note_types.values() for name in get_media_names_from_note_type(note_type)
-        }
-        note_refs = {
-            media_name
-            for note in notes
-            for field_value in (note.fields.values() if note.fields else [])
-            for media_name in get_media_names_from_note_field(field_value, note_types[note.anki_note_type_id])
-        }
+        for mid, fields in rows:
+            if mid not in note_types:
+                note_type = note_types[mid] = self.note_type_dict(NotetypeId(mid))
+                if note_type is not None:
+                    result.update(get_media_names_from_note_type(note_type))
+
+            note_type = note_types[mid]
+            if note_type is None or not fields:
+                continue
 
-        return {*note_type_refs, *note_refs}
+            for field_value in fields.values():
+                result.update(get_media_names_from_note_field(field_value, note_type))
+
+        return result
 
     def media_names_exist_for_ankihub_deck(self, ah_did: uuid.UUID, media_names: Set[str]) -> Dict[str, bool]:
         """Returns a dictionary where each key is a media name and the corresponding value is a boolean

ankihub/gui/media_sync.py

@@ -336,7 +336,11 @@ def _update_deck_media(self, ankihub_did: uuid.UUID) -> None:
             LOGGER.info("No new media updates for deck.", ah_did=ankihub_did)
 
     def _media_referenced_by_notes(self, ah_did: uuid.UUID) -> Set[str]:
-        """Scan all notes in the AnkiHub deck and return the set of referenced media filenames."""
+        """
+        Scan all notes in the AnkiHub deck and return the set of referenced media filenames.
+        This requires scanning the Anki collection rather than the AnkiHub DB
+        to filter out media from emptied protected fields (INTP-338)
+        """
         anki_nids: List[NoteId] = ankihub_db.anki_nids_for_ankihub_deck(ah_did)
 
         media_names: Set[str] = set()

tests/addon/test_integration.py

@@ -8479,6 +8479,7 @@ def test_download_media(
         self,
         anki_session_with_addon_data: AnkiSession,
         install_sample_ah_deck: InstallSampleAHDeck,
+        import_ah_note: ImportAHNote,
         mocker: MockerFixture,
         qtbot: QtBot,
     ):
@@ -8504,8 +8505,13 @@ def test_download_media(
                 ],
             )
 
-            # Mock the _media_referenced_by_notes method to include the test media
-            mocker.patch.object(media_sync, "_media_referenced_by_notes", return_value={"image.png"})
+            # Add a note to the deck which references the test media
+            import_ah_note(
+                ah_did=ah_did,
+                note_data=NoteInfoFactory.create(
+                    fields=[Field(name="Front", value='<img src="image.png">'), Field(name="Back", value="")],
+                ),
+            )
 
             # Mock the client method for downloading media
             download_media_mock = mocker.patch.object(AnkiHubClient, "download_media")
@@ -8567,6 +8573,7 @@ def test_download_media_with_filtering_by_referenced_media(
         self,
         anki_session_with_addon_data: AnkiSession,
         install_sample_ah_deck: InstallSampleAHDeck,
+        import_ah_note: ImportAHNote,
         mocker: MockerFixture,
         qtbot: QtBot,
     ):
@@ -8603,8 +8610,16 @@ def test_download_media_with_filtering_by_referenced_media(
                 ],
             )
 
-            # Mock the _media_referenced_by_notes method to return only one media file
-            mocker.patch.object(media_sync, "_media_referenced_by_notes", return_value={"referenced_image.png"})
+            # Add a note to the deck which references only one of the media files
+            import_ah_note(
+                ah_did=ah_did,
+                note_data=NoteInfoFactory.create(
+                    fields=[
+                        Field(name="Front", value='<img src="referenced_image.png">'),
+                        Field(name="Back", value=""),
+                    ],
+                ),
+            )
 
             download_media_mock = mocker.patch.object(AnkiHubClient, "download_media")