Database configs carry a version the database keeps too (#866)

* Benchmark every database config import (#865)

An opt-in test (WROLPI_CONFIG_BENCH=1) that builds a large library in bulk,
writes each config with its own dump_config, then times every import in
startup order: a steady pass (the database already matches) and a rebuild
(config-owned rows deleted, everything restored from the configs).  It
prints seconds, SQL statements and writes per import, and asserts the
rebuild restored every row so no timed import silently skipped its work.
WROLPI_CONFIG_BENCH_SCALE multiplies the row counts.

The baseline for the config import rework.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* Database configs carry a version the database keeps too

Step 1 of the config import rework.  A database config (tags, downloads,
channels, domains, playlists, repos, recently viewed) and the database now
each carry an integer version, so an import can tell which side is newer
without a clock (a Raspberry Pi's resets at boot):

- config_version (new table) holds the database's version of each config.
- An after_flush hook adds one to it for every config whose rows a flush
  changed, in the same transaction.  Each config registers what it owns
  (register_ownership): models, optionally only some columns, optionally
  only some rows (a Collection's kind; a Download's Collection's kind).
  Raw SQL writes to owned tables count explicitly.
- dump_config writes the database's version, read with the rows it saves;
  save() no longer adds one for these configs.
- An import compares versions first: file == database or newer imports
  with deletes; file older (the database changed and was not saved before a
  restart) saves the database instead of undoing that change; no database
  row (a config this code never imported) imports without deletes.  The
  import records the file's version last.
- The Import button and backup imports apply the file whatever its version
  (import_forced).

Importers only gate their existing deletes on the decision here; they are
otherwise unchanged.

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

* Config versions: keep unsaved rows after a no-delete import, count every owned write

Review fixes:

- An import without deletes (no database version yet) no longer records the
  file's version, which let the next startup delete the rows it had kept.
  The version stays unknown, and the database is saved into the file instead;
  a successful save records the version.  A save after a partial import is
  refused, so the version stays unknown until an import applies the whole file.
- An import removes the database's version while it runs, so its own writes
  do not count: one that fails partway no longer looks newer than the file
  and gets saved over it on the next startup.
- Playlists imported without deletes keep their database items in order; the
  file's other items are added after their neighbor from the file.
- A Download leaving a Channel or Repo counts for that Collection's config
  (collection_id loads its previous value).
- Bulk writes count: untagging, retrying Downloads, deleting a duplicate's
  Download, removing many items from a playlist.  Deleting Downloads by id
  counts channels.yaml/repos.yaml only for their Downloads.
- Only a playlist's items count for playlists.yaml (author and subject
  Collections have items too).  Collection kinds are looked up once per flush.

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

* Config versions count only the columns each config saves

A Repo's fetch (head_sha, last_fetch), a Channel's refresh (video_count,
info_json), a Collection's counts, and a Download attempt (attempts, error)
counted as config changes, so the next import of that config saved instead of
importing: a hand edit to repos.yaml was overwritten after any fetch.  Each
model now registers only the columns its config saves.

domains.yaml saves its domains' Downloads, which now count for it.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

modules/archive/lib.py

@@ -27,6 +27,7 @@ from wrolpi.collections import Collection
 from wrolpi.common import get_media_directory, get_relative_to_media_directory, logger, extract_domain, \
     escape_file_name, aiohttp_post, format_html_string, split_lines_by_length, get_html_soup, get_title_from_html, \
     get_wrolpi_config, html_screenshot, html_file_screenshot, ConfigFile, trim_file_name, chunks
+from wrolpi.config_version import IMPORT, SAVE
 from wrolpi.dates import now, Seconds
 from wrolpi.db import get_db_session, get_db_curs, session_curs
 from wrolpi.errors import UnknownArchive, InvalidOrderBy, InvalidDatetime
@@ -104,6 +105,11 @@ class DomainsConfig(ConfigFile):
 
         ConfigFile.import_config(self, file, send_events)
 
+        decision = self.import_decision()
+        if decision == SAVE:
+            self.save_database_instead()
+            return
+
         collections_data = self._config.get('collections', [])
 
         # Empty collections list = never delete DB records
@@ -158,7 +164,7 @@ class DomainsConfig(ConfigFile):
                 # create their domain (e.g. one downloaded before this import), and deleting it would delete them.
                 with_archives = {i for i, in session.query(Archive.collection_id).distinct()}
                 all_domain_collections = session.query(Collection).filter_by(kind='domain').all()
-                for collection in all_domain_collections:
+                for collection in all_domain_collections if decision == IMPORT else []:
                     if collection.name in imported_domains:
                         continue
                     if collection.id in with_archives:
@@ -169,6 +175,7 @@ class DomainsConfig(ConfigFile):
                     session.delete(collection)
 
             logger.info(f'Successfully imported {len(imported_domains)} domain collections from {file_str}')
+            self.record_import_version()
             self.successful_import = True
 
         except Exception as e:
@@ -184,6 +191,7 @@ class DomainsConfig(ConfigFile):
         logger.info('Dumping domain collections to config')
 
         with get_db_session() as session:
+            version = self.db_version_for_dump(session)
             # Get only domain collections, ordered by name
             collections = session.query(Collection).filter_by(kind='domain').order_by(Collection.name).all()
 
@@ -195,6 +203,7 @@ class DomainsConfig(ConfigFile):
             return
 
         self._config['collections'] = collections_data
+        self._config['version'] = version
 
         logger.info(f'Dumping {len(collections_data)} domain collections to config')
         self.save(file, send_events, overwrite)
@@ -261,7 +270,8 @@ class DomainsConfig(ConfigFile):
             config_file.parent.mkdir(parents=True, exist_ok=True)
             self.write_config_data(merged_data, config_file)
 
-        self.import_config(send_events=send_events)
+        # Importing a backup is the user's choice: apply it whatever the versions.
+        self.import_forced(send_events=send_events)
 
 
 # Global instance

modules/archive/test/test_domains_config.py

@@ -11,6 +11,7 @@ import yaml
 
 from modules.archive.lib import get_domains_config
 from wrolpi.collections import Collection
+from wrolpi.conftest import config_was_imported
 
 
 @pytest.mark.asyncio
@@ -205,6 +206,7 @@ class TestDomainsConfigEdgeCases:
 
         # Reinitialize and import
         config.initialize()
+        config_was_imported(test_session, 'domains.yaml')
         config.import_config()
         assert config.successful_import is True
 

modules/repos/lib.py

@@ -25,6 +25,7 @@ from wrolpi.cmd import GIT_BIN, run_command
 from wrolpi.collections.models import Collection
 from wrolpi.common import ConfigFile, get_media_directory, get_relative_to_media_directory, logger, \
     escape_file_name, background_task, get_wrolpi_config
+from wrolpi.config_version import IMPORT, SAVE, register_ownership
 from wrolpi.db import get_db_session
 from wrolpi.downloader import Download, DownloadFrequency, download_manager, save_downloads_config
 from wrolpi.errors import UnknownDirectory, ValidationError
@@ -1408,6 +1409,12 @@ class ReposConfig(ConfigFile):
             return
 
         super().import_config(file, send_events)
+
+        decision = self.import_decision()
+        if decision == SAVE:
+            self.save_database_instead()
+            return
+
         repos_data = self._config.get('repos', [])
 
         # An empty repos list never deletes DB records.  Deleting all repos is done through the API.
@@ -1428,12 +1435,13 @@ class ReposConfig(ConfigFile):
                         continue
                     imported_ids.add(repo.id)
 
-                for repo in session.query(Repository).all():
+                for repo in session.query(Repository).all() if decision == IMPORT else []:
                     if repo.id not in imported_ids:
                         logger.info(f'Deleting repo {repo.name!r} (no longer in config)')
                         _delete_repository(session, repo)
 
             logger.info(f'Imported {len(imported_ids)} repos from {file_str}')
+            self.record_import_version()
             self.successful_import = True
         except Exception as e:
             self.successful_import = False
@@ -1486,6 +1494,7 @@ class ReposConfig(ConfigFile):
     def dump_config(self, file: pathlib.Path = None, send_events=False, overwrite=False):
         """Dump all Repos to the config file."""
         with get_db_session() as session:
+            version = self.db_version_for_dump(session)
             repos = session.query(Repository).options(*Repository.json_options()) \
                 .join(Collection, Collection.id == Repository.collection_id) \
                 .order_by(func.lower(Collection.name)).all()
@@ -1508,6 +1517,7 @@ class ReposConfig(ConfigFile):
                 data.append(entry)
 
         self._config['repos'] = data
+        self._config['version'] = version
         logger.info(f'Dumping {len(data)} repos to config')
         self.save(file, send_events, overwrite)
 
@@ -1546,11 +1556,15 @@ class ReposConfig(ConfigFile):
                           if i.get('url') and repo_url_key(i['url']) not in current_keys)
             self.write_config_data(dict(repos=merged, version=current_data.get('version', 0) + 1), config_file)
 
-        self.import_config(send_events=send_events)
+        # Importing a backup is the user's choice: apply it whatever the versions.
+        self.import_forced(send_events=send_events)
         save_downloads_config.activate_switch()
 
 
 repos_config = ReposConfig()
+# Only the columns repos.yaml saves (not a fetch's commit, size, ...); its Collection saves the rest.
+register_ownership(ReposConfig.file_name, Repository,
+                   columns={'url', 'mode', 'branch', 'submodules', 'clone_token', 'collection_id'})
 
 
 def get_repos_config() -> ReposConfig:

modules/repos/test/test_config.py

@@ -3,6 +3,7 @@ import pytest
 from modules.repos import lib
 from modules.repos.models import Repository
 from wrolpi.collections.models import Collection
+from wrolpi.conftest import config_versions_lost
 from wrolpi.downloader import Download, DownloadFrequency
 
 
@@ -31,9 +32,12 @@ async def test_dump_and_import(test_session, test_directory, async_client, test_
     for repo in test_session.query(Repository).all():
         lib._delete_repository(test_session, repo)
     test_session.commit()
+    config_versions_lost(test_session)
     assert test_session.query(Collection).count() == 0
 
     config.import_config()
+    # Imported without deletes; the database is saved, which records its version.
+    await await_switches()
     test_session.expire_all()
     repos = {i.name: i for i in test_session.query(Repository)}
     assert repos['kiwix-tools'].directory == test_directory / 'repos/software/kiwix-tools'

modules/repos/test/test_never.py

@@ -9,6 +9,7 @@ import yaml
 
 from modules.repos import lib
 from modules.repos.models import Repository
+from wrolpi.conftest import config_versions_lost
 from wrolpi.downloader import Download, DownloadFrequency, DownloadStatus
 
 
@@ -146,6 +147,7 @@ async def test_never_config(test_session, test_directory, async_client, test_wro
     for download in test_session.query(Download).all():
         test_session.delete(download)
     test_session.commit()
+    config_versions_lost(test_session)
     subprocess.run(['rm', '-rf', str(test_directory / 'repos/missing')], check=True)
 
     config.import_config()

modules/repos/test/test_ownership.py

@@ -5,6 +5,7 @@ import pytest
 
 from modules.repos import lib
 from modules.repos.conftest import TEST_GIT_ENV
+from wrolpi.conftest import config_versions_lost
 from wrolpi.downloader import Download, DownloadStatus
 
 
@@ -395,6 +396,7 @@ async def test_clone_token_survives_the_config(test_session, async_client, git_r
     # Lose the database, restore it from the config.
     lib._delete_repository(test_session, repo)
     test_session.commit()
+    config_versions_lost(test_session)
     config.import_config()
     test_session.expire_all()
     restored = test_session.query(lib.Repository).one()

modules/videos/lib.py

@@ -18,6 +18,7 @@ from wrolpi import dates, flags
 from wrolpi.captions import extract_captions
 from wrolpi.common import ConfigFile, extract_domain, logger, \
     escape_file_name, get_media_directory, background_task, Base, get_wrolpi_config, trim_file_name
+from wrolpi.config_version import IMPORT, SAVE, register_ownership
 from wrolpi.dates import Seconds, from_timestamp
 from wrolpi.db import get_db_curs, get_db_session, session_curs
 from wrolpi.downloader import Download, download_manager
@@ -293,7 +294,13 @@ class ChannelsConfig(ConfigFile):
             self.successful_import = True
             return
 
-        super().import_config()
+        super().import_config(file)
+
+        decision = self.import_decision()
+        if decision == SAVE:
+            self.save_database_instead()
+            return
+
         try:
             channels = self.channels
             # Empty channels list = never delete DB records
@@ -395,7 +402,7 @@ class ChannelsConfig(ConfigFile):
             with get_db_session(commit=True) as session:
                 # Delete any Channels that were deleted from the config, along with their Collections.
                 orphaned_collection_ids = []
-                for channel in session.query(Channel):
+                for channel in session.query(Channel) if decision == IMPORT else []:
                     if channel.id not in updated_channel_ids:
                         logger.warning(f'Deleting {channel} because it is not in the config.')
                         orphaned_collection_ids.append(channel.collection_id)
@@ -435,6 +442,7 @@ class ChannelsConfig(ConfigFile):
             with get_db_session() as session:
                 link_channel_and_downloads(session)
 
+            self.record_import_version()
             self.successful_import = True
         except Exception as e:
             self.successful_import = False
@@ -450,6 +458,7 @@ class ChannelsConfig(ConfigFile):
         channel_import_logger.info('Dumping channels to config')
 
         with get_db_session() as session:
+            version = self.db_version_for_dump(session)
             # Get all channels, ordered by collection name
             channels = session.query(Channel).join(Collection).order_by(Collection.name).all()
 
@@ -457,6 +466,7 @@ class ChannelsConfig(ConfigFile):
             channels_data = [channel.config_view() for channel in channels]
 
             self._config['channels'] = channels_data
+            self._config['version'] = version
 
         channel_import_logger.info(f'Dumping {len(channels_data)} channels to config')
         self.save(file, send_events, overwrite)
@@ -515,10 +525,15 @@ class ChannelsConfig(ConfigFile):
             config_file.parent.mkdir(parents=True, exist_ok=True)
             self.write_config_data(merged_data, config_file)
 
-        self.import_config(send_events=send_events)
+        # Importing a backup is the user's choice: apply it whatever the versions.
+        self.import_forced(send_events=send_events)
 
 
 CHANNELS_CONFIG: ChannelsConfig = ChannelsConfig()
+# Only the columns channels.yaml saves (not a refresh's counts, info_json, ...); its Collection saves the rest.
+register_ownership(ChannelsConfig.file_name, Channel,
+                   columns={'url', 'source_id', 'generate_posters', 'calculate_duration', 'download_missing_data',
+                            'collection_id'})
 TEST_CHANNELS_CONFIG: ChannelsConfig = None
 
 

modules/videos/test/test_channels_config.py

@@ -14,6 +14,7 @@ import yaml
 
 from modules.videos.lib import get_channels_config
 from modules.videos.models import Channel
+from wrolpi.conftest import config_was_imported, config_versions_lost
 
 
 @pytest.mark.asyncio
@@ -187,6 +188,7 @@ class TestChannelsConfigEdgeCases:
 
         # Reinitialize and import
         config.initialize()
+        config_was_imported(test_session, 'channels.yaml')
         config.import_config()
         assert config.successful_import is True
 
@@ -317,6 +319,7 @@ class TestChannelsConfigEdgeCases:
         # Clear the file_format to simulate factory reset
         channel.collection.file_format = None
         test_session.commit()
+        config_versions_lost(test_session)
 
         # Re-import
         config.initialize()

modules/videos/test/test_common.py

@@ -9,7 +9,7 @@ from PIL import Image
 
 from modules.videos.models import Channel, Video
 from wrolpi.collections import Collection
-from wrolpi.conftest import probe_write_lock_is_held, production_like_sessions
+from wrolpi.conftest import config_was_imported, probe_write_lock_is_held, production_like_sessions
 from wrolpi.common import get_absolute_media_path, get_wrolpi_config, get_relative_to_media_directory
 from wrolpi.downloader import Download, DownloadFrequency
 from wrolpi.vars import PROJECT_DIR
@@ -254,7 +254,8 @@ async def test_import_channel_downloads(await_switches, test_session, channel_fa
     [i.delete(add_to_skip_list=False) for i in test_session.query(Download).all()]
     # Write config with Channel Downloads now that the delete() has removed them from the config.
     get_channels_config().channels = channels_backup
-    import_channels_config()
+    # Deleting the Downloads made the database newer than the file, so restoring from the file is the user's choice.
+    get_channels_config().import_forced()
     channel1, channel2 = test_session.query(Channel).order_by(Channel.url).all()
     assert test_session.query(Download).count() == 3
     assert channel1.url == 'https://example.com/channel1'
@@ -353,6 +354,7 @@ async def test_import_channel_deletes_orphaned_collections(await_switches, test_
     await await_switches()
 
     # Import config - this should delete channel2 AND its Collection
+    config_was_imported(test_session, 'channels.yaml')
     import_channels_config()
 
     # Verify only one Channel remains

modules/zim/models.py

@@ -13,6 +13,7 @@ from sqlalchemy.orm.exc import NoResultFound  # noqa
 from modules.zim.errors import UnknownZimEntry, UnknownZimTagEntry, UnknownZim
 from wrolpi import dates, tags
 from wrolpi.common import loaded, Base, logger, get_relative_to_media_directory, ModelHelper
+from wrolpi.config_version import register_ownership
 from wrolpi.dates import TZDateTime
 from wrolpi.db import session_curs, serializer
 from wrolpi.downloader import Download, download_manager
@@ -380,6 +381,9 @@ class TagZimEntry(Base):
             .one_or_none()
 
 
+register_ownership('tags.yaml', TagZimEntry)
+
+
 class ZimSubscription(Base):
     __tablename__ = 'zim_subscription'