Collections: changes wait for their config's import; domains with archives survive the import (#784)

* collections: refuse playlist changes until playlists.yaml is imported

While WROLPi starts, the API accepts requests before every config is imported.  A playlist created then could not be
saved, and the import which followed deleted it as "no longer in config" (and rebuilt every playlist's items from
the config, undoing any change).  Playlist changes now wait for the import (503, "still starting").

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

* collections: refuse domain changes until domains.yaml is imported

A domain created, changed, tagged or deleted through the API before domains.yaml was imported (e.g. while WROLPi
starts) was undone by the import.  These now wait for the import (503, "still starting").

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

* archive: importing domains.yaml keeps domains which have archives

Archives create their domain (e.g. one uploaded, or found by a refresh, while WROLPi starts, before domains.yaml is
imported).  The import deleted every domain missing from the config, and the database deleted that domain's Archives
with it.  A domain with Archives is now kept (and saved to the config with the next change).

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

* videos: refuse Channel changes until channels.yaml is imported

A Channel created through the API before channels.yaml was imported (e.g. while WROLPi starts) could not be saved,
and the import which followed deleted it, and its Downloads, as "not in the config".  Creating, changing, tagging and
deleting a Channel (through the Channel or Collection API) now wait for the import (503, "still starting").

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

modules/archive/lib.py

@@ -153,12 +153,19 @@ class DomainsConfig(ConfigFile):
                     collections = Collection.batch_from_config(session, valid_data_list)
                     imported_domains = {c.name for c in collections}
 
-                # Delete domain collections that are no longer in config
+                # Delete domain collections that are no longer in config.  A domain with Archives is kept: Archives
+                # create their domain (e.g. one downloaded before this import), and deleting it would delete them.
+                with_archives = {i for i, in session.query(Archive.collection_id).distinct()}
                 all_domain_collections = session.query(Collection).filter_by(kind='domain').all()
                 for collection in all_domain_collections:
-                    if collection.name not in imported_domains:
-                        logger.info(f'Deleting domain collection {repr(collection.name)} (no longer in config)')
-                        session.delete(collection)
+                    if collection.name in imported_domains:
+                        continue
+                    if collection.id in with_archives:
+                        logger.warning(f'Keeping domain collection {repr(collection.name)} which is not in config,'
+                                       f' because it has archives')
+                        continue
+                    logger.info(f'Deleting domain collection {repr(collection.name)} (no longer in config)')
+                    session.delete(collection)
 
             logger.info(f'Successfully imported {len(imported_domains)} domain collections from {file_str}')
             self.successful_import = True

modules/videos/channel/lib.py

@@ -17,12 +17,19 @@ from wrolpi.tags import Tag
 from .. import schema
 from ..common import check_for_channel_conflicts
 from ..errors import UnknownChannel
-from ..lib import save_channels_config
+from ..lib import save_channels_config, get_channels_config
 from ..models import Channel, Video
 
 logger = logger.getChild(__name__)
 
 
+def check_channels_config_imported():
+    """Raise ConfigNotImported until channels.yaml is imported; the import would undo a change made before it."""
+    # A test may have no channels config.
+    if config := get_channels_config():
+        config.check_imported()
+
+
 async def get_minimal_channels(session: Session) -> List[dict]:
     """
     Get the minimum amount of information necessary about all channels.
@@ -112,6 +119,7 @@ def get_channel(session: Session, *, channel_id: int = None, source_id: str = No
 
 async def update_channel(session: Session, *, data: schema.ChannelPutRequest, channel_id: int) -> Channel:
     """Update a Channel's DB record"""
+    check_channels_config_imported()
     channel = Channel.find_by_id(session, channel_id)
 
     # Verify that the URL/Name/directory aren't taken
@@ -149,6 +157,7 @@ def create_channel(session: Session, data: schema.ChannelPostRequest, return_dic
     """
     Create a new Channel.  Check for conflicts with existing Channels.
     """
+    check_channels_config_imported()
     try:
         # Verify that the URL/Name/directory aren't taken
         check_for_channel_conflicts(
@@ -189,6 +198,7 @@ def create_channel(session: Session, data: schema.ChannelPostRequest, return_dic
 
 
 def delete_channel(session: Session, *, channel_id: int):
+    check_channels_config_imported()
     channel = Channel.find_by_id(session, channel_id)
 
     channel_dict = channel.dict()
@@ -292,6 +302,7 @@ async def tag_channel(session: Session, tag_name: str | None, directory: pathlib
     """Add a Tag to a Channel, or remove a Tag from a Channel if no `tag_name` is provided.
 
     Move the Channel to the new directory, if provided."""
+    check_channels_config_imported()
 
     if directory and flags.file_worker_busy.is_set():
         raise FileWorkerConflict('Refusing to move channel while FileWorker is busy')

modules/videos/test/test_channels_config.py

@@ -6,6 +6,9 @@ Tests cover:
 2. Upgrade - config file exists with data
 3. Edge cases - empty config, missing config with DB data
 """
+import json
+from http import HTTPStatus
+
 import pytest
 import yaml
 
@@ -322,3 +325,44 @@ class TestChannelsConfigEdgeCases:
         # Verify file_format restored
         test_session.refresh(channel)
         assert channel.collection.file_format == '%(upload_date)s_%(title)s.%(ext)s'
+
+
+@pytest.mark.asyncio
+async def test_channel_changes_wait_for_config_import(test_session, test_directory, async_client, channel_factory,
+                                                      test_channels_config):
+    """Until channels.yaml is imported (e.g. while WROLPi starts), Channels cannot change; the import would undo the
+    change (a new Channel would be deleted, with its Downloads)."""
+    channel = channel_factory(name='Kept')
+    test_session.commit()
+    config = get_channels_config()
+    config.dump_config()
+    assert config.get_file().is_file()
+    config.successful_import = False
+
+    cid, collection_id = channel.id, channel.collection_id
+    requests = [
+        ('post', '/api/videos/channels', dict(name='New', directory='videos/new', url='https://example.com/new')),
+        ('put', f'/api/videos/channels/{cid}', dict(name='Changed', directory=str(channel.directory))),
+        ('post', f'/api/videos/channels/{cid}/tag', dict(tag_name='Tagged')),
+        ('delete', f'/api/videos/channels/{cid}', None),
+        ('put', f'/api/collections/{collection_id}', dict(description='changed')),
+        ('post', f'/api/collections/{collection_id}/tag', dict(tag_name='Tagged')),
+        ('delete', f'/api/collections/{collection_id}', None),
+    ]
+    for method, url, body in requests:
+        kwargs = dict(content=json.dumps(body)) if body else {}
+        request, response = await getattr(async_client, method)(url, **kwargs)
+        assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE, (method, url, response.json)
+        assert response.json['code'] == 'CONFIG_NOT_IMPORTED'
+
+    test_session.expire_all()
+    channel = test_session.query(Channel).one()
+    assert channel.name == 'Kept'
+    assert channel.tag_name is None
+    assert channel.collection.description is None
+
+    # Once imported, Channels can change again.
+    config.import_config()
+    request, response = await async_client.put(f'/api/collections/{collection_id}',
+                                               content=json.dumps(dict(description='x')))
+    assert response.status_code == HTTPStatus.OK, response.json

wrolpi/collections/api.py

@@ -10,7 +10,7 @@ from sanic_ext.extensions.openapi import openapi
 
 from wrolpi.api_utils import json_response
 from wrolpi.common import wrol_mode_check
-from wrolpi.errors import ValidationError
+from wrolpi.errors import ConfigNotImported, ValidationError
 from wrolpi.schema import JSONErrorResponse
 from . import lib, schema
 from .errors import UnknownCollection
@@ -211,6 +211,8 @@ async def put_collection_endpoint(request: Request, collection_id: int, body: sc
 
     except UnknownCollection as e:
         return json_response({'error': str(e)}, status=HTTPStatus.NOT_FOUND)
+    except ConfigNotImported:
+        raise
     except Exception as e:
         return json_response({'error': str(e)}, status=HTTPStatus.BAD_REQUEST)
 
@@ -236,6 +238,8 @@ async def refresh_collection_endpoint(_: Request, collection_id: int):
 
     except UnknownCollection as e:
         return json_response({'error': str(e)}, status=HTTPStatus.NOT_FOUND)
+    except ConfigNotImported:
+        raise
     except Exception as e:
         return json_response({'error': str(e)}, status=HTTPStatus.BAD_REQUEST)
 
@@ -273,6 +277,8 @@ async def tag_collection_endpoint(request: Request, collection_id: int, body: sc
 
     except UnknownCollection as e:
         return json_response({'error': str(e)}, status=HTTPStatus.NOT_FOUND)
+    except ConfigNotImported:
+        raise
     except Exception as e:
         return json_response({'error': str(e)}, status=HTTPStatus.BAD_REQUEST)
 

wrolpi/collections/lib.py

@@ -39,6 +39,22 @@ __all__ = [
 ]
 
 
+def check_config_imported(kind: str):
+    """Raise ConfigNotImported unless the config which holds this kind of Collection was imported.  A change made
+    before the import (e.g. while WROLPi starts) would be undone by it."""
+    if kind == 'playlist':
+        from .config import get_playlists_config
+        get_playlists_config().check_imported()
+    elif kind == 'domain':
+        # Local import to avoid circular import: collections -> archive -> collections
+        from modules.archive.lib import get_domains_config
+        get_domains_config().check_imported()
+    elif kind == 'channel':
+        # Local import to avoid circular import: collections -> videos -> collections
+        from modules.videos.channel.lib import check_channels_config_imported
+        check_channels_config_imported()
+
+
 def _activate_config_save(kind: str):
     """Activate the config-save switch for the config that owns this kind of Collection.
 
@@ -272,6 +288,7 @@ def update_collection(
         ValidationError: If validation fails
     """
     collection = Collection.find_by_id(session, collection_id)
+    check_config_imported(collection.kind)
 
     # The managed location before any rename/retag.  The API reports this location for a playlist
     # whose stored directory is None, so the UI round-trips it back; recognize it below and keep
@@ -372,6 +389,7 @@ def create_collection(session: Session, name: str, description: Optional[str] =
         raise ValidationError('Collection name is required')
     if not collection_type_registry.is_registered(kind):
         raise ValidationError(f'Unknown collection kind: {kind!r}')
+    check_config_imported(kind)
     if not collection_type_registry.validate(kind, name):
         description_msg = collection_type_registry.get_description(kind) or 'Invalid name format'
         raise ValidationError(f'Invalid {kind} name {name!r}. {description_msg}')
@@ -414,6 +432,7 @@ def add_collection_item(session: Session, collection_id: int, item_kind: str,
     """
     from .models import CollectionItem
     collection = Collection.find_by_id(session, collection_id)
+    check_config_imported(collection.kind)
 
     if item_kind == 'file':
         from wrolpi.files.models import FileGroup
@@ -458,6 +477,7 @@ def add_collection_item(session: Session, collection_id: int, item_kind: str,
 def remove_collection_item(session: Session, collection_id: int, item_id: int) -> bool:
     """Remove an item from a collection by CollectionItem id.  Returns True if removed."""
     collection = Collection.find_by_id(session, collection_id)
+    check_config_imported(collection.kind)
     removed = collection.remove_item(session, item_id)
     if removed:
         from .config import save_playlists_config
@@ -470,6 +490,7 @@ def remove_collection_item(session: Session, collection_id: int, item_id: int) -
 def reorder_collection_items(session: Session, collection_id: int, item_ids: List[int]):
     """Reorder a collection's items from a full list of CollectionItem ids."""
     collection = Collection.find_by_id(session, collection_id)
+    check_config_imported(collection.kind)
     collection.reorder_items(session, item_ids)
     from .config import save_playlists_config
     save_playlists_config.activate_switch()
@@ -593,6 +614,7 @@ async def tag_collection(
         FileWorkerConflict: If a file operation is in progress and directory change is requested
     """
     collection = Collection.find_by_id(session, collection_id)
+    check_config_imported(collection.kind)
 
     # Track old directory before any changes for potential file moving
     old_directory = collection.directory
@@ -758,6 +780,7 @@ def delete_collection(
         UnknownCollection: If collection not found
     """
     collection = Collection.find_by_id(session, collection_id)
+    check_config_imported(collection.kind)
 
     collection_dict = {
         'id': collection.id,

wrolpi/collections/test/test_domains_config.py

@@ -1,6 +1,9 @@
 """Tests for DomainsConfig functionality."""
+import json
 import pathlib
+from http import HTTPStatus
 
+import pytest
 import yaml
 from sqlalchemy.orm import Session
 
@@ -256,3 +259,75 @@ class TestDomainsConfig:
         assert not config['directory'].startswith('/'), \
             f"Expected relative path but got absolute: {config['directory']}"
         assert config['directory'] == 'archive/example.com'
+
+
+@pytest.mark.asyncio
+async def test_domain_changes_wait_for_config_import(test_session: Session, test_directory: pathlib.Path,
+                                                     async_client):
+    """Until domains.yaml is imported (e.g. while WROLPi starts), domains cannot be changed through the API; the
+    import would undo the change (or delete a new domain)."""
+    domain = Collection.from_config(test_session, {'name': 'kept.com', 'kind': 'domain'})
+    test_session.commit()
+    domains_config.dump_config()
+    assert domains_config.get_file().is_file()
+    domains_config.successful_import = False
+
+    did = domain.id
+    requests = [
+        ('post', '/api/collections', dict(name='new.com', kind='domain')),
+        ('put', f'/api/collections/{did}', dict(description='changed')),
+        ('post', f'/api/collections/{did}/tag', dict(tag_name='Tagged', directory='archive/Tagged/kept.com')),
+        ('delete', f'/api/collections/{did}', None),
+    ]
+    for method, url, body in requests:
+        kwargs = dict(content=json.dumps(body)) if body else {}
+        request, response = await getattr(async_client, method)(url, **kwargs)
+        assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE, (method, url, response.json)
+        assert response.json['code'] == 'CONFIG_NOT_IMPORTED'
+
+    test_session.expire_all()
+    domain = test_session.query(Collection).one()
+    assert domain.name == 'kept.com'
+    assert domain.description is None
+    assert domain.tag is None
+    assert domain.directory is None
+
+    # Once imported, domains can change again.
+    domains_config.import_config()
+    request, response = await async_client.put(f'/api/collections/{did}', content=json.dumps(dict(description='x')))
+    assert response.status_code == HTTPStatus.OK, response.json
+
+
+@pytest.mark.asyncio
+async def test_import_keeps_domains_with_archives(test_session: Session, test_directory: pathlib.Path, async_client,
+                                                  archive_factory):
+    """A domain missing from domains.yaml is deleted by the import, unless it has Archives.  Archives create their
+    domain (e.g. an upload or refresh while WROLPi starts, before domains.yaml is imported), and deleting it would
+    delete its Archives."""
+    from modules.archive.models import Archive
+    archive_factory(domain='kept.com', url='https://kept.com/1')
+    Collection.from_config(test_session, {'name': 'empty.com', 'kind': 'domain'})
+    test_session.commit()
+    domains_config.dump_config()
+    domains_config.successful_import = False
+
+    # Created before the import.
+    archive_factory(domain='new.com', url='https://new.com/1')
+    Collection.from_config(test_session, {'name': 'unused.com', 'kind': 'domain'})
+    test_session.commit()
+    assert test_session.query(Archive).count() == 2
+
+    domains_config.import_config()
+    test_session.expire_all()
+    assert {i.name for i in test_session.query(Collection)} == {'kept.com', 'empty.com', 'new.com'}
+    assert {i.collection.name for i in test_session.query(Archive)} == {'kept.com', 'new.com'}
+    assert domains_config.successful_import
+
+    # A domain with Archives which was removed from the config (by hand) is kept too.
+    config = yaml.safe_load(domains_config.get_file().read_text())
+    config['collections'] = [i for i in config['collections'] if i['name'] != 'kept.com']
+    domains_config.get_file().write_text(yaml.dump(config))
+    domains_config.import_config()
+    test_session.expire_all()
+    assert {i.name for i in test_session.query(Collection)} == {'kept.com', 'empty.com', 'new.com'}
+    assert test_session.query(Archive).count() == 2

wrolpi/collections/test/test_playlists_config.py

@@ -326,3 +326,45 @@ async def test_move_file_updates_playlists_config(
     await await_switches()
 
     await _assert_playlist_survives_reimport(test_session, playlists_config, 'docs/guide.pdf')
+
+
+@pytest.mark.asyncio
+async def test_playlist_changes_wait_for_config_import(test_session, test_directory, async_client, playlists_config):
+    """Until playlists.yaml is imported (e.g. while WROLPi starts), playlists cannot change; the import would undo
+    the change (a new playlist would be deleted, items rebuilt from the config)."""
+    import json
+    collection = _make_playlist(test_session, 'Kept')
+    collection.add_url(test_session, '/map?lat=3&lon=4&z=5', title='other')
+    test_session.commit()
+    item_ids = [i.id for i in collection.items]
+    playlists_config.dump_config()
+    assert playlists_config.get_file().is_file()
+    playlists_config.successful_import = False
+
+    cid = collection.id
+    requests = [
+        ('post', '/api/collections', dict(name='New')),
+        ('post', f'/api/collections/{cid}/items', dict(item_kind='url', url='/map?lat=5&lon=6&z=7')),
+        ('delete', f'/api/collections/{cid}/items/{item_ids[0]}', None),
+        ('put', f'/api/collections/{cid}/items/order', dict(item_ids=list(reversed(item_ids)))),
+        ('put', f'/api/collections/{cid}', dict(description='changed')),
+        ('post', f'/api/collections/{cid}/tag', dict(tag_name='Tagged')),
+        ('delete', f'/api/collections/{cid}', None),
+    ]
+    for method, url, body in requests:
+        kwargs = dict(content=json.dumps(body)) if body else {}
+        request, response = await getattr(async_client, method)(url, **kwargs)
+        assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE, (method, url, response.json)
+        assert response.json['code'] == 'CONFIG_NOT_IMPORTED'
+
+    test_session.expire_all()
+    assert [i.name for i in test_session.query(Collection)] == ['Kept']
+    collection = test_session.query(Collection).one()
+    assert [i.id for i in collection.items] == item_ids
+    assert collection.description is None
+    assert collection.tag is None
+
+    # Once imported, playlists can change again.
+    playlists_config.import_config()
+    request, response = await async_client.post('/api/collections', content=json.dumps(dict(name='New')))
+    assert response.status_code == HTTPStatus.CREATED, response.json

wrolpi/common.py

@@ -40,7 +40,7 @@ from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import Session
 
 from wrolpi.dates import now, from_timestamp
-from wrolpi.errors import WROLModeEnabled, NativeOnly, LogLevelError, InvalidConfig, \
+from wrolpi.errors import WROLModeEnabled, NativeOnly, LogLevelError, InvalidConfig, ConfigNotImported, \
     ValidationError
 from wrolpi.log_levels import TRACE_LEVEL
 from wrolpi.vars import PYTEST, DOCKERIZED, CONFIG_DIR, MEDIA_DIRECTORY, DEFAULT_HTTP_HEADERS
@@ -608,6 +608,14 @@ class ConfigFile:
         todays_backup.parent.mkdir(parents=True, exist_ok=True)
         shutil.copy2(config_file, todays_backup)
 
+    def check_imported(self):
+        """Raise ConfigNotImported unless what this config holds may be changed: the config was imported (or does not
+        exist yet).  A change before the import (e.g. while WROLPi starts) could not be saved, and the import would
+        undo it."""
+        if self.get_file().exists() and not self.successful_import:
+            raise ConfigNotImported(f'WROLPi is still starting ({self.file_name} has not been loaded), try again in'
+                                    f' a moment')
+
     def import_backup(self, backup_date: str, mode: str, send_events: bool = False):
         """Import a backup file. Subclasses override this."""
         raise NotImplementedError(f'{self.__class__.__name__} does not support backup import')

wrolpi/errors.py

@@ -159,6 +159,12 @@ class InvalidConfig(APIError):
     status_code = HTTPStatus.BAD_REQUEST
 
 
+class ConfigNotImported(APIError):
+    code = 'CONFIG_NOT_IMPORTED'
+    summary = 'WROLPi is still starting, try again in a moment'
+    status_code = HTTPStatus.SERVICE_UNAVAILABLE
+
+
 class NoPrimaryFile(APIError):
     code = 'NO_PRIMARY_FILE'
     summary = 'Could not find a primary file in the provided group'