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

A malformed config entry is skipped and counted instead of failing the import

Browse source 
An entry that is not a mapping (or whose key is not text, or a list that is not a list) made the whole file fail to
import in playlists.yaml, repos.yaml, download_manager.yaml and tags.yaml (tag_files/tag_zims), and a non-text name
did in domains.yaml.  Each is now skipped and counted in import_skipped, so the config is not saved over it, and the
rest of the file imports.  A Channel whose `downloads` is not a list keeps its Downloads.

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

2
modules/archive/lib.py
View file

@ -65,7 +65,7 @@ class DomainsSection(ConfigSection):
rows, invalid = dict(), set()
for position, data in enumerate(config.get('collections') or []):
name = data.get('name') if isinstance(data, dict) else None
if not name:
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}')

6
modules/repos/lib.py
View file

@ -1397,9 +1397,9 @@ class ReposSection(ConfigSection):
media_directory = get_media_directory()
rows, invalid = dict(), set()
for position, data in enumerate(config.get('repos') or []):
url = data.get('url')
if not url:
logger.error(f'Repo config entry has no url, skipping: {data}')
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}')
continue

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

@ -102,6 +102,22 @@ async def test_invalid_entry_is_kept(test_session, saved_repos):
assert (found[TWO].mode, found[ONE].collection.description) == ('snapshot', 'Changed')
@pytest.mark.asyncio
async def test_malformed_entry_is_skipped(test_session, saved_repos):
"""An entry that is not a mapping is skipped and counted; the rest of the file still imports."""
edit_repo(saved_repos, ONE, description='Changed')
edit_config(saved_repos, lambda data: data['repos'].insert(0, 'not a mapping'))
config = lib.get_repos_config()
config.import_config()
assert config.successful_import is True
assert config.import_skipped == 1
found = repos(test_session)
assert set(found) == {ONE, TWO}
assert found[ONE].collection.description == 'Changed'
@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

@ -394,7 +394,13 @@ class ChannelDownloadsSection(ConfigSection):
key = _channel_key(data)
except Exception:
continue
for position, download in enumerate(data.get('downloads') or []):
downloads = data.get('downloads') or []
if not isinstance(downloads, list):
channel_import_logger.error(f'Skipping the Downloads of Channel {key!r}, they are not a list')
# Counted as skipped; the Channel keeps its Downloads (see `ChannelsConfig.prepare`).
invalid.add((key, '\0downloads'))
continue
for position, download in enumerate(downloads):
if isinstance(download, dict) and download.get('url'):
rows[(key, download['url'])] = dict(frequency=download.get('frequency'))
else:

31
wrolpi/collections/config.py
View file

@ -23,16 +23,24 @@ def _playlist_tag_names(rows) -> Set[str]:
return {i['tag_name'] for i in rows if i['tag_name']}
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
return name if isinstance(name, str) and name else None
class PlaylistsSection(ConfigSection):
"""playlists.yaml's playlists (Collections with kind 'playlist'), keyed by name."""
name = 'playlists'
def file_rows(self, config: dict) -> Tuple[Dict[str, dict], Set[str]]:
rows, invalid = dict(), set()
for data in config.get('playlists') or []:
name = data.get('name')
for position, data in enumerate(config.get('playlists') or []):
name = _playlist_name(data)
if not name:
logger.error(f'Playlist config entry has no name, skipping: {data}')
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}')
continue
try:
rows[name] = dict(
@ -88,23 +96,32 @@ class PlaylistItemsSection(ConfigSection):
@staticmethod
def item_key(name: str, data: dict) -> Optional[tuple]:
if not isinstance(data, dict):
raise ValueError('An item must be a mapping')
if 'file' in data:
return name, 'file', media_relative(data['file']), None
elif 'zim' in data:
if not data.get('entry'):
if not data.get('entry') or not isinstance(data['entry'], str):
raise ValueError('A zim item requires an entry')
return name, 'zim', media_relative(data['zim']), data['entry']
elif 'url' in data:
if not isinstance(data['url'], str):
raise ValueError('An item\'s url must be text')
return name, 'url', data['url'], None
raise ValueError('An item requires a file, zim, or url')
def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
rows, invalid = dict(), set()
for data in config.get('playlists') or []:
if not (name := data.get('name')):
if not (name := _playlist_name(data)):
continue
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))
continue
# Positions follow the file's order; an item skipped now keeps its place for a later import.
for position, item in enumerate(data.get('items') or [], start=1):
for position, item in enumerate(items, start=1):
try:
key = self.item_key(name, item)
except Exception as e:
@ -121,7 +138,7 @@ class PlaylistItemsSection(ConfigSection):
def present(self, config: dict, file_rows: dict, invalid_keys: set) -> bool:
# A listed playlist without items has none: only an empty or missing playlists list removes nothing.
return any(i.get('name') for i in config.get('playlists') or [])
return any(map(_playlist_name, config.get('playlists') or []))
@staticmethod
def _query(session: Session, *columns):

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

@ -414,3 +414,23 @@ async def test_import_without_deletes_keeps_database_order(test_session, test_di
test_session.expire_all()
collection = test_session.query(Collection).filter_by(name='Mix', kind='playlist').one()
assert [(i.url, i.position) for i in collection.items] == [('/a', 1), ('/x', 2), ('/b', 3)]
@pytest.mark.asyncio
async def test_import_skips_malformed_entries(test_session, test_directory, playlists_config):
"""A playlist entry or item that is not a mapping is skipped and counted; the rest of the file still imports."""
_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: Survives\n items:\n - also not a mapping\n'
' - url: /map?lat=1&lon=2&z=3\n title: spot\n- name: New\n')
playlists_config.import_config()
assert playlists_config.successful_import is True
assert playlists_config.import_skipped == 2
test_session.expire_all()
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

9
wrolpi/downloader.py
View file

@ -2109,8 +2109,13 @@ class DownloadsSection(ConfigSection):
def file_rows(self, config: dict) -> Tuple[Dict[str, dict], Set[str]]:
defaults = inheritable_video_defaults()
rows, invalid = dict(), set()
for data in config.get('downloads') or []:
url = data.get('url')
for position, data in enumerate(config.get('downloads') or []):
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}')
continue
try:
settings = dict(data.get('settings') or {})
# An old config kept `destination` and `tag_names` in the settings.

20
wrolpi/tags.py
View file

@ -397,12 +397,16 @@ class TagFilesSection(ConfigSection):
def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
tag_names = set(config.get('tags') or dict())
rows, invalid = dict(), set()
for item in config.get('tag_files') or []:
for position, item in enumerate(config.get('tag_files') or []):
try:
tag_name, path, *rest = item
tag_name, path, *rest = item if isinstance(item, list) else ()
if not isinstance(tag_name, str):
raise ValueError('A Tag name must be text')
key = (tag_name, media_relative(path))
except Exception as e:
logger.error(f'Skipping invalid tagged file in config: {item}', exc_info=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}',))
continue
if tag_names and tag_name not in tag_names:
logger.warning(f'Cannot find Tag for {repr(key[1])}')
@ -496,12 +500,16 @@ class TagZimsSection(ConfigSection):
def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
tag_names = set(config.get('tags') or dict())
rows, invalid = dict(), set()
for item in config.get('tag_zims') or []:
for position, item in enumerate(config.get('tag_zims') or []):
try:
tag_name, zim_path, zim_entry, *rest = item
tag_name, zim_path, zim_entry, *rest = item if isinstance(item, list) else ()
if not isinstance(tag_name, str) or not isinstance(zim_entry, str):
raise ValueError('A Tag name and a Zim entry must be text')
key = (tag_name, media_relative(zim_path), zim_entry)
except Exception as e:
logger.error(f'Skipping invalid tagged Zim entry in config: {item}', exc_info=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}',))
continue
if tag_names and tag_name not in tag_names:
logger.warning(f'Cannot find Tag for {repr(key[1])}')

62
wrolpi/test/test_config_malformed_entries.py Normal file
View file

@ -0,0 +1,62 @@
"""A malformed entry in a config (not a mapping, or a key that is not a string) is skipped and counted, so the config
is never saved over it; the rest of the file still imports."""
import pytest
from modules.archive.lib import DomainsSection
from modules.repos.lib import ReposSection
from modules.videos.lib import ChannelDownloadsSection, ChannelsSection
from wrolpi.collections.config import PlaylistItemsSection, PlaylistsSection
from wrolpi.downloader import DownloadsSection
from wrolpi.tags import TagFilesSection, TagsSection, TagZimsSection
MALFORMED = ['a string', 5, ['a', 'list'], None]
def assert_skipped(section, config, valid_key, malformed_count):
rows, invalid = section.file_rows(config)
assert valid_key in rows
assert len(invalid) >= malformed_count, 'every malformed entry must be counted, so the config is not saved over it'
@pytest.mark.parametrize('section,config,valid_key,malformed_count', [
(PlaylistsSection(), dict(playlists=[*MALFORMED, dict(name=['a', 'list']), dict(name='good')]), 'good', 5),
(ReposSection(), dict(repos=[*MALFORMED, dict(url=['a', 'list']), dict(url='https://example.com/a/b.git')]),
'example.com/a/b', 5),
(DomainsSection(), dict(collections=[*MALFORMED, dict(name=['a', 'list']), dict(name='example.com')]),
'example.com', 5),
(ChannelsSection(), dict(channels=[*MALFORMED, dict(name='c', directory=['a', 'list']),
dict(name='good', directory='videos/good')]), 'videos/good', 5),
(DownloadsSection(), dict(downloads=[*MALFORMED, dict(url=['a', 'list'], downloader='archive'),
dict(url='https://example.com/good', downloader='archive')]),
'https://example.com/good', 5),
(TagFilesSection(), dict(tags=dict(one=dict()), tag_files=[*MALFORMED, [['a', 'list'], 'foo.txt'],
['one', 'good.txt']]), ('one', 'good.txt'), 5),
(TagZimsSection(), dict(tags=dict(one=dict()), tag_zims=[*MALFORMED, [['a', 'list'], 'a.zim', 'A'],
['one', 'good.zim', 'A']]), ('one', 'good.zim', 'A'), 5),
])
def test_malformed_entries(test_directory, section, config, valid_key, malformed_count):
assert_skipped(section, config, valid_key, malformed_count)
def test_malformed_tags(test_directory):
"""A Tag whose attributes are not a mapping is skipped and counted."""
assert_skipped(TagsSection(), dict(tags={'one': dict(color='#fff'), 'two': 'a string', 'three': 5}), 'one', 2)
def test_malformed_playlist_items(test_directory):
"""A playlist's malformed items are skipped and counted; so is an `items` that is not a list."""
config = dict(playlists=[
dict(name='good', items=[*MALFORMED, dict(url=['a', 'list']), dict(url='https://example.com/good')]),
dict(name='bad', items=5),
])
rows, invalid = PlaylistItemsSection().file_rows(config)
assert list(rows) == [('good', 'url', 'https://example.com/good', None)]
assert len(invalid) == 6
def test_malformed_channel_downloads(test_directory):
"""A Channel whose `downloads` is not a list keeps its Downloads (counted)."""
config = dict(channels=[dict(name='good', directory='videos/good', downloads=5)])
rows, invalid = ChannelDownloadsSection().file_rows(config)
assert rows == dict()
assert invalid