rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-25 18:40:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

An unreadable entry keeps no row and does not make a section present

Browse source 
The key of an entry too malformed to name a row is now `Unreadable(position)`.  It is still counted in
import_skipped, but `present` ignores it, so a section whose entries are all unreadable removes nothing (as an empty
section), instead of deleting every row.  A playlist whose `items` is not a list keeps its items, and a Channel
Download whose url is not text is counted (its Channel keeps its Downloads) instead of failing the import.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-25 16:02:08 -06:00
parent a787c02908
commit 96d5aa3a99
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
10 changed files with 134 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

6
modules/archive/lib.py
View file

@ -28,7 +28,7 @@ from wrolpi.common import get_media_directory, get_relative_to_media_directory,
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.collections.models import validate_collection_directory
from wrolpi.config_import import ConfigSection, media_relative
from wrolpi.config_import import ConfigSection, Unreadable, media_relative
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
@ -67,8 +67,8 @@ class DomainsSection(ConfigSection):
name = data.get('name') if isinstance(data, dict) else None
if not name or not isinstance(name, str):
logger.error(f'Domain collection at index {position} has no name, skipping: {data!r}')
# Counted as skipped; never a domain's name.
invalid.add(f'\0{position}')
# Counted as skipped (see `Unreadable`).
invalid.add(Unreadable(position))
continue
try:
if not Collection.is_valid_domain_name(name):

6
modules/repos/lib.py
View file

@ -25,7 +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_import import ConfigSection, media_relative
from wrolpi.config_import import ConfigSection, Unreadable, media_relative
from wrolpi.config_version import register_ownership
from wrolpi.db import get_db_session
from wrolpi.downloader import Download, DownloadFrequency, download_manager, save_downloads_config
@ -1400,8 +1400,8 @@ class ReposSection(ConfigSection):
url = data.get('url') if isinstance(data, dict) else None
if not url or not isinstance(url, str):
logger.error(f'Repo config entry has no url, skipping: {data!r}')
# Counted as skipped; never a Repo's key.
invalid.add(f'\0{position}')
# Counted as skipped (see `Unreadable`).
invalid.add(Unreadable(position))
continue
try:
parsed = parse_repo_url(url)

12
modules/repos/test/test_config_import.py
View file

@ -118,6 +118,18 @@ async def test_malformed_entry_is_skipped(test_session, saved_repos):
assert found[ONE].collection.description == 'Changed'
@pytest.mark.asyncio
async def test_only_malformed_entries_delete_nothing(test_session, saved_repos):
"""A file whose entries are all unreadable deletes no Repo (like an empty list)."""
edit_config(saved_repos, lambda data: data.update(repos=['not a mapping', dict(url=['a', 'list'])]))
config = lib.get_repos_config()
config.import_config()
assert config.import_skipped == 2
assert set(repos(test_session)) == {ONE, TWO}
@pytest.mark.asyncio
async def test_failed_import_creates_nothing(test_session, saved_repos, monkeypatch):
"""A new Repo is created in the import's one write: a failure later in the write leaves nothing (each new Repo

8
modules/videos/lib.py
View file

@ -20,7 +20,7 @@ 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.collections import Collection
from wrolpi.collections.models import validate_collection_directory
from wrolpi.config_import import ConfigSection, chunks, media_relative
from wrolpi.config_import import ConfigSection, Unreadable, chunks, media_relative
from wrolpi.config_version import register_ownership
from wrolpi.dates import Seconds, from_timestamp
from wrolpi.db import get_db_curs, get_db_session, session_curs
@ -295,8 +295,8 @@ class ChannelsSection(ConfigSection):
key = _channel_key(data)
except Exception as e:
channel_import_logger.error(f'Skipping invalid Channel in config: {data!r}', exc_info=e)
# Counted as skipped; never a directory. (The entry's name keeps its Channel, see `prepare`.)
invalid.add(f'\0{position}')
# Counted as skipped (see `Unreadable`). The entry's name keeps its Channel (see `prepare`).
invalid.add(Unreadable(position))
continue
if key in rows or key in invalid:
raise RuntimeError('Refusing to import channels config because it contains duplicate directories!')
@ -401,7 +401,7 @@ class ChannelDownloadsSection(ConfigSection):
invalid.add((key, '\0downloads'))
continue
for position, download in enumerate(downloads):
if isinstance(download, dict) and download.get('url'):
if isinstance(download, dict) and download.get('url') and isinstance(download['url'], str):
rows[(key, download['url'])] = dict(frequency=download.get('frequency'))
else:
channel_import_logger.error(f'Skipping invalid Download of Channel {key!r}: {download!r}')

19
wrolpi/collections/config.py
View file

@ -7,7 +7,7 @@ from sqlalchemy import insert
from sqlalchemy.orm import Session
from wrolpi.common import ConfigFile, get_media_directory, get_relative_to_media_directory, logger
from wrolpi.config_import import ConfigSection, SectionDiff, chunks, media_relative
from wrolpi.config_import import ConfigSection, SectionDiff, Unreadable, chunks, media_relative
from wrolpi.config_version import count_config_change
from wrolpi.db import get_db_session
from wrolpi.switches import ActivateSwitchMethod, register_switch_handler
@ -23,6 +23,10 @@ def _playlist_tag_names(rows) -> Set[str]:
return {i['tag_name'] for i in rows if i['tag_name']}
# The position in an invalid item key that stands for all of a playlist's items (its `items` is not a list).
ITEMS_UNREADABLE = 0
def _playlist_name(data) -> Optional[str]:
"""The name of a playlist entry, or None when the entry is malformed."""
name = data.get('name') if isinstance(data, dict) else None
@ -39,8 +43,8 @@ class PlaylistsSection(ConfigSection):
name = _playlist_name(data)
if not name:
logger.error(f'Playlist config entry has no name, skipping: {data!r}')
# Counted as skipped; never a playlist's name.
invalid.add(f'\0{position}')
# Counted as skipped (see `Unreadable`).
invalid.add(Unreadable(position))
continue
try:
rows[name] = dict(
@ -118,7 +122,7 @@ class PlaylistItemsSection(ConfigSection):
items = data.get('items') or []
if not isinstance(items, list):
logger.error(f'Skipping the items of playlist {name!r}, they are not a list: {items!r}')
invalid.add((name, None, 0, None))
invalid.add((name, None, ITEMS_UNREADABLE, None))
continue
# Positions follow the file's order; an item skipped now keeps its place for a later import.
for position, item in enumerate(items, start=1):
@ -267,6 +271,7 @@ class PlaylistsConfig(ConfigFile):
def prepare(self, diffs: dict):
self._drop_unknown_tags(diffs['playlists'])
self._drop_missing_items(diffs['items'])
self._keep_unreadable_items(diffs['items'])
if not self.import_deletes:
self._merge_items(diffs['items'])
@ -285,6 +290,12 @@ class PlaylistsConfig(ConfigFile):
row['tag_name'] = None
self._config_outdated = True
def _keep_unreadable_items(self, diff: SectionDiff):
"""A playlist whose items cannot be read keeps the items it has."""
_, invalid = PlaylistItemsSection().file_rows(self._config)
if names := {i[0] for i in invalid if i[2] == ITEMS_UNREADABLE}:
diff.removed = {i for i in diff.removed if i[0] not in names}
def _drop_missing_items(self, diff: SectionDiff):
"""An added item whose file or Zim is not in the database (not indexed yet, after a rebuild) is skipped
before the write; a later import (after the refresh) adds it."""

37
wrolpi/collections/test/test_playlists_config.py
View file

@ -426,6 +426,7 @@ async def test_import_skips_malformed_entries(test_session, test_directory, play
file.write_text('version: 1\nplaylists:\n- not a mapping\n- name: Survives\n items:\n - also not a mapping\n'
' - url: /map?lat=1&lon=2&z=3\n title: spot\n- name: New\n')
config_was_imported(test_session, 'playlists.yaml')
playlists_config.import_config()
assert playlists_config.successful_import is True
@ -434,3 +435,39 @@ async def test_import_skips_malformed_entries(test_session, test_directory, play
playlists = {i.name: i for i in test_session.query(Collection).filter_by(kind='playlist')}
assert set(playlists) == {'Survives', 'New'}
assert len(playlists['Survives'].items) == 1
@pytest.mark.asyncio
async def test_import_only_malformed_entries_preserves_db(test_session, test_directory, playlists_config):
"""A file whose playlist entries are all unreadable deletes no playlist (like an empty list)."""
_make_playlist(test_session)
file = playlists_config.get_file()
file.parent.mkdir(parents=True, exist_ok=True)
file.write_text('version: 1\nplaylists:\n- not a mapping\n- name: [a, list]\n')
config_was_imported(test_session, 'playlists.yaml')
playlists_config.import_config()
assert playlists_config.import_skipped == 2
test_session.expire_all()
collection = test_session.query(Collection).filter_by(kind='playlist').one()
assert (collection.name, len(collection.items)) == ('Survives', 1)
@pytest.mark.asyncio
async def test_import_unreadable_items_keeps_items(test_session, test_directory, playlists_config):
"""A playlist whose items are not a list keeps the items it has."""
_make_playlist(test_session)
file = playlists_config.get_file()
file.parent.mkdir(parents=True, exist_ok=True)
file.write_text('version: 1\nplaylists:\n- name: Survives\n items: 5\n')
config_was_imported(test_session, 'playlists.yaml')
playlists_config.import_config()
assert playlists_config.import_skipped == 1
test_session.expire_all()
collection = test_session.query(Collection).filter_by(kind='playlist').one()
assert len(collection.items) == 1

16
wrolpi/config_import.py
View file

@ -27,6 +27,18 @@ class SectionDiff:
return bool(self.added or self.changed or self.removed)
@dataclass(frozen=True)
class Unreadable:
"""The invalid key of an entry too malformed to name a row (not a mapping, no name). It is counted as skipped, so
the file is not saved over it, but it keeps no row, and on its own it does not make a section present."""
position: int
def listed(file_rows: dict, invalid_keys: set) -> bool:
"""Whether a section lists anything: a row, or an invalid row it names."""
return bool(file_rows) or any(not isinstance(i, Unreadable) for i in invalid_keys)
class ConfigSection:
"""One kind of row a database config saves.
@ -52,7 +64,7 @@ class ConfigSection:
def present(self, config: dict, file_rows: dict, invalid_keys: set) -> bool:
"""Whether the file lists this section at all; an empty section cannot be told apart from a lost or blanked
file, so it removes nothing."""
return bool(file_rows or invalid_keys)
return listed(file_rows, invalid_keys)
def same(self, file_row: dict, db_row: dict) -> bool:
"""Whether a file row is already in the database."""
@ -77,7 +89,7 @@ def diff_section(file_rows: Dict[Hashable, dict], invalid_keys: Set[Hashable], d
elif not same(row, db_rows[key]):
diff.changed[key] = row
if present is None:
present = bool(file_rows or invalid_keys)
present = listed(file_rows, invalid_keys)
if deletes and present:
diff.removed = db_rows.keys() - file_rows.keys() - invalid_keys
return diff

6
wrolpi/downloader.py
View file

@ -34,7 +34,7 @@ from wrolpi.cmd import which, run_command, CommandResult
from wrolpi.common import loaded, Base, ModelHelper, logger, wrol_mode_check, zig_zag, ConfigFile, \
wrol_mode_enabled, background_task, get_absolute_media_path, timer, aiohttp_get, \
get_download_info, trim_file_name, get_wrolpi_config, TRACE_LEVEL, normalize_domain
from wrolpi.config_import import ConfigSection, chunks
from wrolpi.config_import import ConfigSection, Unreadable, chunks
from wrolpi.config_version import collection_kinds, count_config_change, register_ownership
from wrolpi.dates import TZDateTime, now, Seconds
from wrolpi.db import get_db_session, session_curs, serializer
@ -2113,8 +2113,8 @@ class DownloadsSection(ConfigSection):
url = data.get('url') if isinstance(data, dict) else None
if not url or not isinstance(url, str):
logger.error(f'Download config entry has no url, skipping: {data!r}')
# Counted as skipped; never a Download's URL.
invalid.add(f'\0{position}')
# Counted as skipped (see `Unreadable`).
invalid.add(Unreadable(position))
continue
try:
settings = dict(data.get('settings') or {})

10
wrolpi/tags.py
View file

@ -14,7 +14,7 @@ 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, \
get_relative_to_media_directory, is_valid_hex_color, walk, INVALID_FILE_CHARS, get_wrolpi_config
from wrolpi.config_import import ConfigSection, SectionDiff, chunks, media_relative
from wrolpi.config_import import ConfigSection, SectionDiff, Unreadable, chunks, media_relative
from wrolpi.config_version import count_config_change, register_ownership
from wrolpi.dates import TZDateTime
from wrolpi.db import get_db_session, named_placeholders, session_curs, serializer
@ -405,8 +405,8 @@ class TagFilesSection(ConfigSection):
key = (tag_name, media_relative(path))
except Exception as e:
logger.error(f'Skipping invalid tagged file in config: {item!r}', exc_info=e)
# Counted as skipped; never a tagged file's key.
invalid.add((f'\0{position}',))
# Counted as skipped (see `Unreadable`).
invalid.add(Unreadable(position))
continue
if tag_names and tag_name not in tag_names:
logger.warning(f'Cannot find Tag for {repr(key[1])}')
@ -508,8 +508,8 @@ class TagZimsSection(ConfigSection):
key = (tag_name, media_relative(zim_path), zim_entry)
except Exception as e:
logger.error(f'Skipping invalid tagged Zim entry in config: {item!r}', exc_info=e)
# Counted as skipped; never a tagged Zim entry's key.
invalid.add((f'\0{position}',))
# Counted as skipped (see `Unreadable`).
invalid.add(Unreadable(position))
continue
if tag_names and tag_name not in tag_names:
logger.warning(f'Cannot find Tag for {repr(key[1])}')

38
wrolpi/test/test_config_malformed_entries.py
View file

@ -2,6 +2,8 @@
is never saved over it; the rest of the file still imports."""
import pytest
from wrolpi.config_import import Unreadable, diff_section
from modules.archive.lib import DomainsSection
from modules.repos.lib import ReposSection
from modules.videos.lib import ChannelDownloadsSection, ChannelsSection
@ -60,3 +62,39 @@ def test_malformed_channel_downloads(test_directory):
rows, invalid = ChannelDownloadsSection().file_rows(config)
assert rows == dict()
assert invalid
@pytest.mark.parametrize('section,config', [
(PlaylistsSection(), dict(playlists=MALFORMED)),
(PlaylistItemsSection(), dict(playlists=MALFORMED)),
(ReposSection(), dict(repos=MALFORMED)),
(DomainsSection(), dict(collections=[*MALFORMED, dict(name=['a', 'list'])])),
(ChannelsSection(), dict(channels=MALFORMED)),
(DownloadsSection(), dict(downloads=MALFORMED)),
(TagsSection(), dict(tags=dict())),
# A list of two texts is a tagged file's line (of an unknown Tag), not an unreadable one.
(TagFilesSection(), dict(tags=dict(one=dict()), tag_files=['a string', 5, None, [['a'], 'b.txt']])),
(TagZimsSection(), dict(tags=dict(one=dict()), tag_zims=MALFORMED)),
])
def test_only_malformed_entries_remove_nothing(test_directory, section, config):
"""A section whose entries are all unreadable is not present: like an empty section, it removes nothing (it
cannot be told apart from a damaged file)."""
rows, invalid = section.file_rows(config)
assert not rows
assert not section.present(config, rows, invalid)
def test_diff_unreadable():
"""An unreadable entry keeps no row and does not make the section present; a named invalid row does both."""
db_rows = dict(one=dict(), two=dict())
assert diff_section(dict(), {Unreadable(0)}, db_rows, deletes=True).removed == set()
assert diff_section(dict(), {Unreadable(0), 'one'}, db_rows, deletes=True).removed == {'two'}
def test_malformed_channel_download_url(test_directory):
"""A Channel's Download whose url is not text is counted, so the Channel keeps its Downloads."""
config = dict(channels=[dict(name='good', directory='videos/good', downloads=[dict(url=['a', 'list']), dict(url=5)])])
rows, invalid = ChannelDownloadsSection().file_rows(config)
assert rows == dict()
assert {i[0] for i in invalid} == {'videos/good'}
assert len(invalid) == 2