From cfbed9e4d7b507d5312145b0325c3e16fb2e2ea3 Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Sun, 4 Oct 2026 18:28:08 -0600 Subject: [PATCH] db: serializers read only loaded data (#777) * db: serializers read only loaded data Serializing a model must not query. A lazy load there is a hidden query per row (search serialized 24 rows with 55 statements), and would be a `MissingGreenlet` error under an async Session. - `wrolpi.db.no_db_access(reason)` and `@serializer` (on every model `__json__`, and around `json_response`): an engine `before_cursor_execute` listener raises `UnexpectedQuery` during tests, and in production logs once per serializer and lets the query run. Tests record each trip as well, and the autouse `no_unexpected_queries` fixture fails a test even when the code caught the error (Video's serializer was swallowing its per-row loads of the deferred `ffprobe_json`). - Relationships serializers read load with their parent: `selectin` for FileGroup.tag_files and Collection.downloads; `joined` for TagFile.tag, Collection.tag, Channel.collection, Video.channel, Archive.collection, ZimSubscription.download, CollectionItem.zim, and the file_group of Video, Archive, Doc, Zim and CollectionItem. - Search and the Video lookups the video page uses load the deferred `ffprobe_json` (`undefer`). - `wrolpi/test/test_serializers.py`: every serialized model, freshly loaded, serializes without SQL. Search: 55 -> 5 statements for 24 videos (31 -> 5 for archives), and a few ms faster locally. Not in this change: guarding the `dict()` serializers and `expire_on_commit=False`, which go together (about 50 serialize-after-commit sites, and 40 tests that read values changed elsewhere after a commit). Co-Authored-By: Claude Opus 5.5 * db: serializer loads per query; the guard ignores transaction control The query guard reported the engine's own BEGIN (emitted through the same event as the first statement on a Session with no open transaction) instead of the lazy load after it, and in production that BEGIN used up the one warning per serializer. Transaction-control statements are now ignored. Mapper-level eager loads reached background code: every Video/Archive load in the modelers (one per file, inside the write transaction) joined file_group/channel/collection/tag and selectin-loaded the Collection's Download rows with their yt-dlp info_json. A second refresh of 100 videos took 0.40 s instead of 0.16 s. Serializer needs are now declared per model (`json_options()`) and added only by the queries whose results are serialized; Download.info_json is deferred there (no serializer reads it). Mapper defaults keep only narrow joins nearly every use needs: TagFile.tag, Collection.tag and Channel.collection. A second refresh is back to master's statement count and time; search still drops from 55 to 3 statements. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- modules/archive/models.py | 10 ++- modules/docs/lib.py | 7 +- modules/docs/models.py | 10 ++- modules/videos/channel/lib.py | 5 +- modules/videos/models.py | 40 +++++++--- modules/videos/test/test_video.py | 3 + modules/videos/video/lib.py | 2 +- modules/zim/lib.py | 2 +- modules/zim/models.py | 18 ++++- pytest.ini | 2 + wrolpi/api_utils.py | 5 +- wrolpi/collections/api.py | 4 + wrolpi/collections/lib.py | 10 ++- wrolpi/collections/models.py | 21 +++++- .../collections/test/test_playlist_items.py | 73 ++++++++++--------- wrolpi/conftest.py | 14 ++++ wrolpi/db.py | 58 +++++++++++++++ wrolpi/downloader.py | 3 +- wrolpi/files/lib.py | 7 +- wrolpi/files/models.py | 11 ++- wrolpi/tags.py | 6 +- wrolpi/test/test_db.py | 67 ++++++++++++++++- wrolpi/test/test_serializers.py | 63 ++++++++++++++++ wrolpi/test/test_tags.py | 2 + 24 files changed, 371 insertions(+), 72 deletions(-) create mode 100644 wrolpi/test/test_serializers.py diff --git a/modules/archive/models.py b/modules/archive/models.py index 475df06f..a6604918 100644 --- a/modules/archive/models.py +++ b/modules/archive/models.py @@ -5,13 +5,14 @@ from typing import Iterable, List, Optional import pytz from sqlalchemy import Column, Integer, ForeignKey, BigInteger, Index -from sqlalchemy.orm import relationship, Session +from sqlalchemy.orm import relationship, Session, joinedload from wrolpi import dates from wrolpi.collections import Collection from wrolpi.common import ModelHelper, Base, logger, get_title_from_html, get_wrolpi_config, get_media_directory, \ strip_surrogates from wrolpi.dates import now +from wrolpi.db import serializer from wrolpi.errors import UnknownArchive from wrolpi.files.models import FileGroup from wrolpi.tags import TagFile @@ -39,6 +40,13 @@ class Archive(Base, ModelHelper): file_group_id = Column(BigInteger, ForeignKey('file_group.id', ondelete='CASCADE'), unique=True, nullable=False) file_group: FileGroup = relationship('FileGroup') + @staticmethod + def json_options() -> tuple: + """Loader options for a query whose Archives will be serialized: `__json__` reads the FileGroup and its Tags, + and the domain (the Collection's name).""" + return joinedload(Archive.file_group).selectinload(FileGroup.tag_files), joinedload(Archive.collection) + + @serializer def __json__(self) -> dict: d = self.file_group.__json__() d['archive'] = dict( diff --git a/modules/docs/lib.py b/modules/docs/lib.py index 72d51de6..d28db9fe 100644 --- a/modules/docs/lib.py +++ b/modules/docs/lib.py @@ -340,7 +340,7 @@ def _doc_response(doc) -> dict: def _get_doc(session, file_group_id: int): from .models import Doc - doc = session.query(Doc).filter_by(file_group_id=file_group_id).one_or_none() + doc = session.query(Doc).options(*Doc.json_options()).filter_by(file_group_id=file_group_id).one_or_none() if not doc: raise ValidationError(f'Doc with file_group_id {file_group_id} not found') return doc @@ -375,7 +375,7 @@ def _search_docs(session: Session, search_str=None, author=None, subject=None, l # the much smaller a/b/c columns. match = fts.translate_websearch(search_str, None if deep else fts.ABC_COLUMNS) if search_str else None - query = session.query(FileGroup).join(Doc, Doc.file_group_id == FileGroup.id) + query = session.query(FileGroup).options(*FileGroup.json_options()).join(Doc, Doc.file_group_id == FileGroup.id) fts_sq = None if match is not None: @@ -441,7 +441,8 @@ def _search_docs(session: Session, search_str=None, author=None, subject=None, l browse_sql = _doc_browse_sql(order_by if order_by in DOC_BROWSE_ORDERS else 'id') if browse_sql: fg_ids = [i for i, in session.execute(text(browse_sql), dict(limit=limit, offset=offset)).fetchall()] - by_id = {fg.id: fg for fg in session.query(FileGroup).filter(FileGroup.id.in_(fg_ids))} if fg_ids else {} + by_id = {fg.id: fg for fg in session.query(FileGroup).options(*FileGroup.json_options()) + .filter(FileGroup.id.in_(fg_ids))} if fg_ids else {} file_groups = [by_id[i] for i in fg_ids] else: file_groups = query.offset(offset).limit(limit).all() diff --git a/modules/docs/models.py b/modules/docs/models.py index ff5d08ac..ba2e0f62 100644 --- a/modules/docs/models.py +++ b/modules/docs/models.py @@ -2,10 +2,10 @@ import pathlib from typing import Optional from sqlalchemy import Column, Integer, BigInteger, ForeignKey, String, Text, Index, or_ -from sqlalchemy.orm import relationship, Session +from sqlalchemy.orm import relationship, Session, joinedload from wrolpi.common import ModelHelper, Base -from wrolpi.db import get_db_session +from wrolpi.db import get_db_session, serializer from wrolpi.files.models import FileGroup EPUB_MIMETYPE = 'application/epub' @@ -65,6 +65,12 @@ class Doc(ModelHelper, Base): path = str(self.file_group.primary_path) if self.file_group else 'None' return f'' + @staticmethod + def json_options() -> tuple: + """Loader options for a query whose Docs will be serialized: `__json__` reads the FileGroup and its Tags.""" + return joinedload(Doc.file_group).selectinload(FileGroup.tag_files), + + @serializer def __json__(self) -> dict: d = self.file_group.__json__() if d.get('data') and d['data'].get('doc_path'): diff --git a/modules/videos/channel/lib.py b/modules/videos/channel/lib.py index 86f48db1..f32d31c1 100644 --- a/modules/videos/channel/lib.py +++ b/modules/videos/channel/lib.py @@ -208,6 +208,7 @@ async def search_channels_by_name(session: Session, name: str, limit: int = 5, name_order = (func.lower(func.replace(Collection.name, ' ', '')).asc(), Collection.name.desc()) if order_by_video_count: stmt = session.query(Channel, func.count(Video.id).label('video_count')) \ + .options(*Channel.json_options()) \ .join(Collection) \ .filter(or_( Collection.name.ilike(f'%{name}%'), @@ -220,6 +221,7 @@ async def search_channels_by_name(session: Session, name: str, limit: int = 5, channels = [i[0] for i in stmt] else: stmt = session.query(Channel) \ + .options(*Channel.json_options()) \ .join(Collection) \ .filter(or_( Collection.name.ilike(f'%{name}%'), @@ -319,5 +321,6 @@ async def tag_channel(session: Session, tag_name: str | None, directory: pathlib async def search_channels(session: Session, tag_names: List[str]) -> List[Channel]: """Search Tagged Channels.""" - channels = session.query(Channel).join(Collection).join(Tag).filter(Tag.name.in_(tag_names)).all() + channels = session.query(Channel).options(*Channel.json_options()) \ + .join(Collection).join(Tag).filter(Tag.name.in_(tag_names)).all() return channels diff --git a/modules/videos/models.py b/modules/videos/models.py index 3e3d87ee..b8591ef4 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -3,14 +3,15 @@ import pathlib from typing import Optional, Dict, List from sqlalchemy import Column, Integer, String, Boolean, Date, ForeignKey, BigInteger, Index, text, JSON -from sqlalchemy.orm import relationship, Session, deferred +from sqlalchemy.orm import relationship, Session, deferred, undefer, joinedload, selectinload from sqlalchemy.orm.collections import InstrumentedList from modules.videos.errors import UnknownVideo, UnknownChannel from wrolpi.captions import read_captions, read_captions_with_timestamps from wrolpi.common import Base, ModelHelper, logger, get_media_directory, get_relative_to_media_directory, \ background_task -from wrolpi.db import get_db_curs, get_db_session, session_curs +from wrolpi.db import get_db_curs, get_db_session, session_curs, serializer +from wrolpi.collections.models import Collection from wrolpi.downloader import Download from wrolpi.files.lib import split_path_stem_and_suffix from wrolpi.files.worker import file_worker @@ -57,6 +58,14 @@ class Video(ModelHelper, Base): return f'