diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 5112efe9..2c271228 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -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 @@ -65,10 +65,10 @@ 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}') + # Counted as skipped (see `Unreadable`). + invalid.add(Unreadable(position)) continue try: if not Collection.is_valid_domain_name(name): diff --git a/modules/repos/lib.py b/modules/repos/lib.py index 1dddd1b2..2b2f8126 100644 --- a/modules/repos/lib.py +++ b/modules/repos/lib.py @@ -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 @@ -1397,11 +1397,11 @@ 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}') - # Counted as skipped; never a Repo's key. - invalid.add(f'\0{position}') + 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 (see `Unreadable`). + invalid.add(Unreadable(position)) continue try: parsed = parse_repo_url(url) diff --git a/modules/repos/test/test_config_import.py b/modules/repos/test/test_config_import.py index caf400d3..7ddf3a34 100644 --- a/modules/repos/test/test_config_import.py +++ b/modules/repos/test/test_config_import.py @@ -102,6 +102,34 @@ 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_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 diff --git a/modules/videos/lib.py b/modules/videos/lib.py index 550ead92..bcc36bd6 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -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!') @@ -394,8 +394,14 @@ class ChannelDownloadsSection(ConfigSection): key = _channel_key(data) except Exception: continue - for position, download in enumerate(data.get('downloads') or []): - if isinstance(download, dict) and download.get('url'): + 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') 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}') diff --git a/wrolpi/collections/config.py b/wrolpi/collections/config.py index 80368d33..6478928a 100644 --- a/wrolpi/collections/config.py +++ b/wrolpi/collections/config.py @@ -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,16 +23,28 @@ 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 + 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 (see `Unreadable`). + invalid.add(Unreadable(position)) continue try: rows[name] = dict( @@ -88,23 +100,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, 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(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 +142,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): @@ -250,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']) @@ -268,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.""" diff --git a/wrolpi/collections/test/test_playlists_config.py b/wrolpi/collections/test/test_playlists_config.py index f7f55ef3..6d119cf0 100644 --- a/wrolpi/collections/test/test_playlists_config.py +++ b/wrolpi/collections/test/test_playlists_config.py @@ -414,3 +414,60 @@ 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') + + config_was_imported(test_session, 'playlists.yaml') + 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 + + +@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 diff --git a/wrolpi/config_import.py b/wrolpi/config_import.py index fb29ab09..1a10a5ac 100644 --- a/wrolpi/config_import.py +++ b/wrolpi/config_import.py @@ -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 diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 4323b259..bf0ef65a 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -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 @@ -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 (see `Unreadable`). + invalid.add(Unreadable(position)) + continue try: settings = dict(data.get('settings') or {}) # An old config kept `destination` and `tag_names` in the settings. diff --git a/wrolpi/tags.py b/wrolpi/tags.py index 0c51c2e2..462b3adf 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -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 @@ -400,12 +400,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 (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])}') @@ -499,12 +503,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 (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])}') diff --git a/wrolpi/test/test_config_malformed_entries.py b/wrolpi/test/test_config_malformed_entries.py new file mode 100644 index 00000000..b4929b7d --- /dev/null +++ b/wrolpi/test/test_config_malformed_entries.py @@ -0,0 +1,100 @@ +"""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 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 +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 + + +@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