domains.yaml: converge without a save; no links while entries are skipped

A save after the import is refused while an entry is skipped, so the
import must converge without one:
- a removed domain with Archives that is already reset is left as it is
  (prepare reads `import_db_rows`); before, every import reset it again;
- after prepare, the framework drops changed rows that now match the
  database (a dropped unknown Tag), so they are not written again.

Linking Downloads changes domains.yaml after its version is recorded;
while entries are skipped the save is refused, the database stays newer,
and the next startup would save over the skipped entries.  Linking now
waits for an import without skipped entries.

An entry that is not a mapping is skipped and counted, instead of failing
the whole file.

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

modules/archive/lib.py

@@ -64,8 +64,9 @@ class DomainsSection(ConfigSection):
     def file_rows(self, config: dict) -> Tuple[Dict[str, dict], Set[str]]:
         rows, invalid = dict(), set()
         for position, data in enumerate(config.get('collections') or []):
-            if not (name := data.get('name')):
-                logger.error(f'Domain collection at index {position} has no name, skipping')
+            name = data.get('name') if isinstance(data, dict) else None
+            if not name:
+                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}')
                 continue
@@ -169,8 +170,22 @@ class DomainsConfig(ConfigFile):
         return self._config.get('collections', [])
 
     def prepare(self, diffs: dict):
-        """A Tag that no longer exists (it was removed) is dropped, and the config is saved without it."""
+        """A Tag that no longer exists (it was removed) is dropped, and the config is saved without it.  A removed
+        domain with Archives which is already reset (see `DomainsSection.delete`) is left as it is; one which is not
+        is reset, and the config is saved with it."""
         diff = diffs['domains']
+        if diff.removed:
+            with get_db_session() as session:
+                with_archives = {name for name, in session.query(Collection.name).join(
+                    Archive, Archive.collection_id == Collection.id).filter(
+                    Collection.kind == 'domain', Collection.name.in_(list(diff.removed))).distinct()}
+            reset = _domain_row(None, None, None, None)
+            for name in with_archives:
+                if self.import_db_rows['domains'][name] == reset:
+                    diff.removed.discard(name)
+                else:
+                    self._config_outdated = True
+
         rows = [*diff.added.values(), *diff.changed.values()]
         unknown = unknown_tag_names({i['tag_name'] for i in rows if i['tag_name']})
         for row in rows:
@@ -184,8 +199,7 @@ class DomainsConfig(ConfigFile):
         super().import_config(file, send_events)
 
     def after_import(self, diffs: dict):
-        # A removed domain with Archives was reset, not deleted; the config lists it again.
-        if self._config_outdated or diffs['domains'].removed:
+        if self._config_outdated:
             self.background_dump.activate_switch()
 
     def dump_config(self, file: pathlib.Path = None, send_events=False, overwrite=False):
@@ -301,6 +315,11 @@ def import_domains_config():
     when there is something to link)."""
     logger.info('Importing domains config')
     domains_config.import_config()
+    if domains_config.import_skipped:
+        # Linking changes domains.yaml, which cannot be saved while it has skipped entries: the database would be
+        # newer than the file, and the next startup would save over them.
+        logger.warning('Not linking Downloads to domains while domains.yaml has skipped entries')
+        return
 
     with get_db_session() as session:
         links = domain_download_links(session)

modules/archive/test/test_domains_config_import.py

@@ -157,3 +157,77 @@ async def test_downloads_are_linked(test_session, saved_domains, await_switches)
     }
     assert len([i for i in collected if 'FROM collection' in i and 'WHERE collection.name' in i]) == 0
     assert 'https://other.org/feed.xml' in saved_domains.read_text()
+
+
+def add_invalid_entry(path):
+    edit_config(path, lambda data: data['collections'].append(dict(name='nodot', kind='domain')))
+
+
+@pytest.mark.asyncio
+async def test_reset_with_a_skipped_entry_converges(test_session, saved_domains, await_switches):
+    """A skipped entry stops the config from being saved; a domain reset by this import is not reset again by the
+    next one (which writes nothing)."""
+    edit_config(saved_domains, lambda data: data.update(
+        collections=[i for i in data['collections'] if i['name'] != 'archived.net']))
+    add_invalid_entry(saved_domains)
+
+    config = get_domains_config()
+    config.import_config()
+    await await_switches()
+    assert domains(test_session)['archived.net'].description is None
+
+    with statements() as collected:
+        config.import_config()
+    assert config.import_skipped == 1
+    assert writes(collected) == []
+
+
+@pytest.mark.asyncio
+async def test_unknown_tag_with_a_skipped_entry_converges(test_session, saved_domains, test_tags_config,
+                                                          await_switches):
+    """A dropped Tag the config cannot be saved without (an entry was skipped) is not dropped again."""
+    from wrolpi.tags import get_tags_config
+    get_tags_config().dump_config()
+    get_tags_config().import_config()
+    edit_domain(saved_domains, 'other.org', tag_name='gone')
+    add_invalid_entry(saved_domains)
+
+    config = get_domains_config()
+    config.import_config()
+    await await_switches()
+
+    with statements() as collected:
+        config.import_config()
+    assert writes(collected) == []
+
+
+@pytest.mark.asyncio
+async def test_no_links_while_entries_are_skipped(test_session, saved_domains, await_switches):
+    """Linking changes domains.yaml; while an entry is skipped the config cannot be saved, so Downloads are not
+    linked (the database would be newer than the file, and the next startup would save over the skipped entry)."""
+    test_session.add(Download(url='https://other.org/feed.xml', downloader='rss', sub_downloader='archive',
+                              frequency=86400))
+    test_session.commit()
+    add_invalid_entry(saved_domains)
+
+    import_domains_config()
+    await await_switches()
+
+    test_session.expire_all()
+    assert test_session.query(Download).one().collection_id is None
+    assert get_config_version(test_session, 'domains.yaml') == yaml.safe_load(saved_domains.read_text())['version']
+    assert 'nodot' in saved_domains.read_text()
+
+
+@pytest.mark.asyncio
+async def test_entry_that_is_not_a_mapping_is_skipped(test_session, saved_domains):
+    """An entry that is not a mapping is skipped (counted); the rest of the file is imported."""
+    edit_config(saved_domains, lambda data: data['collections'].extend(['example.net', None]))
+    edit_domain(saved_domains, 'other.org', description='Changed')
+
+    config = get_domains_config()
+    config.import_config()
+
+    assert config.successful_import is True
+    assert config.import_skipped == 2
+    assert domains(test_session)['other.org'].description == 'Changed'

wrolpi/common.py

@@ -850,8 +850,15 @@ class ConfigFile:
         # `prepare` may skip rows it cannot apply.
         self.import_skipped = sum(len(invalid) for _, invalid in file_rows.values())
         self.import_deletes = decision == IMPORT
+        self.import_db_rows = db_rows
         if any(diffs.values()):
             self.prepare(diffs)
+            # A row `prepare` brought in line with the database (e.g. it dropped a Tag the file names but no longer
+            # exists) is not written.
+            for section in self.sections:
+                changed = diffs[section.name].changed
+                for key in [i for i in changed if section.same(changed[i], db_rows[section.name][i])]:
+                    del changed[key]
 
         file_version = self.version or 0
         if any(diffs.values()) or (decision == IMPORT and db_version != file_version):
@@ -892,7 +899,7 @@ class ConfigFile:
         """Slow work the write needs (files, network), run with no transaction open, and only when there are
         differences.  `diffs` maps each section's name to its `SectionDiff`; it may drop rows it cannot apply
         (count them in `import_skipped`), or add what it found to a row for `upsert`.  `import_deletes`: whether
-        this import deletes what the file no longer lists.
+        this import deletes what the file no longer lists.  `import_db_rows`: each section's database rows.
 
         It must be safe to repeat and to abandon: when the database changes before the write, the import decides
         again (up to three passes), and the diffs it was given may never be written (the write fails, or the next