A removed Tag leaves no Download that would apply it

The tags import removes a deleted Tag's name from every Download's
tag_names and from the tag_names and channel_tag_name its settings pass
on (a change to download_manager.yaml, which is then saved).  Applying a
Tag that does not exist fails the download, and it is retried.  Once
tags.yaml is imported, a newer download_manager.yaml drops names no Tag
has, and is saved without them.

A replaced Zim's entry tags move to the newest Zim of the same parsed
name, by the date in its name (as migrate_outdated_zim_tags picks it),
not by name prefix and path order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

wrolpi/downloader.py

@@ -2232,6 +2232,36 @@ class DownloadManagerConfig(ConfigFile):
     def downloads(self, value: List[dict]):
         self.update({'downloads': value})
 
+    def prepare(self, diffs: dict):
+        """Drop Tag names that no Tag has (the Tag was removed) from the Downloads to write: applying one fails the
+        download.  Only once tags.yaml was imported; until then its Tags may be missing."""
+        from wrolpi.tags import Tag, get_tags_config, without_tag_names
+        if not get_tags_config().successful_import:
+            return
+        rows = [*diffs['downloads'].added.values(), *diffs['downloads'].changed.values()]
+        names = set()
+        for row in rows:
+            names.update(row['tag_names'] or [])
+            for key in ('tag_names', 'channel_tag_name'):
+                names.update((row['settings'] or dict()).get(key) or [])
+        if not names:
+            return
+        with get_db_session() as session:
+            unknown = names - {i for i, in session.query(Tag.name).filter(Tag.name.in_(list(names)))}
+        if not unknown:
+            return
+        logger.warning(f'Removing unknown Tags from Downloads: {sorted(unknown)}')
+        for row in rows:
+            tag_names, row['settings'] = without_tag_names(row['tag_names'], row['settings'], unknown)
+            row['tag_names'] = tag_names or None
+        # The file is saved without them.
+        self._config_outdated = True
+
+    def after_import(self, diffs: dict):
+        if getattr(self, '_config_outdated', False):
+            self._config_outdated = False
+            self.background_dump.activate_switch()
+
     @staticmethod
     def _filter_settings_for_yaml(settings: dict, global_defaults: dict = None) -> dict:
         """Remove inheritable video settings that match current global defaults.

wrolpi/tags.py

@@ -9,7 +9,7 @@ import pytz
 from cachetools.keys import hashkey
 from sqlalchemy import Column, Integer, String, ForeignKey, BigInteger, event, UniqueConstraint, insert
 from sqlalchemy.exc import IntegrityError
-from sqlalchemy.orm import Mapped, relationship, Session
+from sqlalchemy.orm import Mapped, relationship, Session, load_only
 
 from wrolpi import dates, flags
 from wrolpi.common import loaded, ModelHelper, Base, logger, ConfigFile, get_media_directory, background_task, \
@@ -313,6 +313,32 @@ def _relative_path(path) -> str:
     return str(path.relative_to(get_media_directory()) if path.is_absolute() else path)
 
 
+def without_tag_names(download_tag_names: Optional[List[str]], settings: Optional[dict], names: Set[str]) \
+        -> Tuple[Optional[List[str]], Optional[dict]]:
+    """A Download's tag_names and settings without these Tag names, in the settings' tag_names and channel_tag_name
+    (passed on to the Downloads and Channels it creates) too.  New objects, so an ORM change to the JSON is seen."""
+    if download_tag_names:
+        download_tag_names = [i for i in download_tag_names if i not in names]
+    if settings:
+        settings = {key: [i for i in value if i not in names] if key in ('tag_names', 'channel_tag_name') and value
+                    else value for key, value in settings.items()}
+    return download_tag_names, settings
+
+
+def remove_tag_names_from_downloads(session: Session, names: Set[str]):
+    """Remove these Tag names from every Download that would apply them; applying a Tag that does not exist fails
+    the download.  (Changes download_manager.yaml.)"""
+    from wrolpi.downloader import Download
+    changed = False
+    for download in session.query(Download).options(load_only(Download.id, Download.tag_names, Download.settings)):
+        tag_names, settings = without_tag_names(download.tag_names, download.settings, names)
+        if tag_names != download.tag_names or settings != download.settings:
+            download.tag_names, download.settings = tag_names, settings
+            changed = True
+    if changed:
+        save_downloads_config.activate_switch()
+
+
 def _tag_ids(session: Session, names) -> Dict[str, int]:
     tag_ids = dict()
     for chunk in chunks(names):
@@ -351,6 +377,7 @@ class TagsSection(ConfigSection):
         """A removed Tag is deleted with its tagged files and Zim entries.  A Collection using it keeps its
         directory, and loses only the Tag (its config is then saved)."""
         from wrolpi.collections import Collection
+        remove_tag_names_from_downloads(session, set(keys))
         for tag in session.query(Tag).filter(Tag.name.in_(list(keys))):
             for collection in session.query(Collection).filter(Collection.tag_id == tag.id):
                 logger.warning(f'Removing Tag {repr(tag.name)} from {collection}')
@@ -695,8 +722,17 @@ class TagsConfig(ConfigFile):
             return
         media_directory = get_media_directory()
         with get_db_session() as session:
-            zim_paths = sorted(i for i, in session.query(Zim.path))
+            zim_paths = [i for i, in session.query(Zim.path)]
         known = {str(i.relative_to(media_directory)) for i in zim_paths}
+        # {name: (date, path)} of the newest Zim of each name, like `migrate_outdated_zim_tags`.
+        newest = dict()
+        for path in zim_paths:
+            try:
+                name, date = zim_lib.parse_name(path)
+            except ValueError:
+                continue
+            if name not in newest or date > newest[name][0]:
+                newest[name] = (date, path)
 
         for key, row in list(diff.added.items()):
             tag_name, zim_path, zim_entry = key
@@ -707,12 +743,11 @@ class TagsConfig(ConfigFile):
                 name, _ = zim_lib.parse_name(media_directory / zim_path)
             except ValueError:
                 name = None
-            newest = [i for i in zim_paths if name and i.name.startswith(name)]
-            if not newest:
+            if name not in newest:
                 logger.warning(f'Cannot find Zim for {repr(zim_path)}')
                 self.import_skipped += 1
                 continue
-            new_key = (tag_name, str(newest[-1].relative_to(media_directory)), zim_entry)
+            new_key = (tag_name, str(newest[name][1].relative_to(media_directory)), zim_entry)
             logger.warning(f'Migrating Zim entry tag {repr(tag_name)} from {zim_path} to {new_key[1]}')
             self._config_outdated = True
             diff.removed.discard(new_key)

wrolpi/test/test_config_import.py

@@ -384,3 +384,35 @@ async def test_nothing_to_write_skips_prepare(test_session, saved_downloads, mon
     get_download_manager_config().import_config()
 
     assert prepared == []
+
+
+@pytest.mark.asyncio
+async def test_unknown_tag_names_are_dropped(test_session, saved_downloads, test_tags_config, tag_factory,
+                                             await_switches):
+    """Once tags.yaml is imported, a Tag name the file gives a Download that no Tag has (the Tag was removed) is
+    dropped, and the file is saved without it: the next import writes nothing."""
+    from wrolpi.tags import get_tags_config
+    await tag_factory('one')
+    test_session.commit()
+    get_tags_config().dump_config()
+    get_tags_config().import_config()
+    assert get_tags_config().successful_import is True
+
+    def change(data):
+        for download in data['downloads']:
+            if download['url'] == 'https://example.com/recurring':
+                download['tag_names'] = ['one', 'gone']
+                download['settings'] = dict(download.get('settings') or {}, channel_tag_name=['gone'])
+
+    edit_config(saved_downloads, change)
+    config = get_download_manager_config()
+    config.import_config()
+    await await_switches()
+
+    test_session.expire_all()
+    download = Download.get_by_url(test_session, 'https://example.com/recurring')
+    assert download.tag_names == ['one']
+    assert download.settings['channel_tag_name'] == []
+    with statements() as collected:
+        config.import_config()
+    assert writes(collected) == []

wrolpi/test/test_tags_config_import.py

@@ -256,3 +256,50 @@ async def test_refresh_changing_primary_file_counts(test_session, test_directory
     test_session.expire_all()
     assert test_session.query(FileGroup).one().primary_path != test_directory / 'video.mp4'
     assert get_config_version(test_session, 'tags.yaml') > 0
+
+
+@pytest.mark.asyncio
+async def test_removed_tag_is_removed_from_downloads(test_session, saved_tags, test_download_manager_config):
+    """A removed Tag's name is removed from every Download that would apply it (its tag_names, and the tag_names and
+    channel_tag_name it passes on); download_manager.yaml is then newer than its file, so it is saved."""
+    from wrolpi.config_version import set_config_version
+    from wrolpi.downloader import Download
+    test_session.add(Download(url='https://example.com/channel', downloader='video_channel', frequency=86400,
+                              tag_names=['one', 'two'],
+                              settings=dict(tag_names=['one'], channel_tag_name=['one'], video_resolutions=['720p'])))
+    test_session.commit()
+    set_config_version(test_session, 'download_manager.yaml', 1)
+    test_session.commit()
+    edit_config(saved_tags, lambda data: data['tags'].pop('one'))
+
+    tags.get_tags_config().import_config()
+
+    test_session.expire_all()
+    download = Download.get_by_url(test_session, 'https://example.com/channel')
+    assert download.tag_names == ['two']
+    assert download.settings == dict(tag_names=[], channel_tag_name=[], video_resolutions=['720p'])
+    assert get_config_version(test_session, 'download_manager.yaml') > 1
+
+
+@pytest.mark.asyncio
+async def test_zim_entry_migrates_by_name_and_date(test_session, test_directory, test_tags_config, zim_factory,
+                                                   tag_factory, await_switches):
+    """The newest Zim of the same name is chosen by the date in its name, wherever it is, and a Zim whose name
+    merely starts with the same name is not the same Zim."""
+    (test_directory / 'a').mkdir()
+    (test_directory / 'b').mkdir()
+    old = zim_factory('wikipedia_en_all_maxi_2020-01.zim')
+    newest = zim_factory('a/wikipedia_en_all_maxi_2020-03.zim')
+    zim_factory('b/wikipedia_en_all_maxi_2020-02.zim')
+    zim_factory('wikipedia_en_all_maxi_extra_2021-01.zim')
+    await tag_factory('one')
+    old.tag_entry(test_session, 'one', 'home')
+    test_session.commit()
+    tags.get_tags_config().dump_config()
+    await await_switches()
+    old.delete()
+
+    tags.get_tags_config().import_config()
+
+    test_session.expire_all()
+    assert [(i.tag.name, i.zim_id) for i in test_session.query(TagZimEntry)] == [('one', newest.id)]