Tagging, untagging and deleting files wait for a concurrent writer

POST /api/files/tag, /untag and /delete_groups read, then wrote, in the
request's deferred transaction.  SQLite fails that lock upgrade instantly
("database is locked") when another connection wrote in between.  Tag and
untag returned a 500; delete_groups returned 204 after FileGroup.delete had
unlinked the files, then its commit failed in the response middleware, so
the FileGroup row outlived its files.  They now write inside
request.ctx.db.write(), which begins as a writer and lets busy_timeout
absorb the contention.

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

wrolpi/files/api.py

@@ -120,17 +120,17 @@ async def delete_file(_: Request, body: schema.DeleteRequest):
 async def delete_file_groups(request: Request, body: schema.DeleteFileGroupsRequest):
     if not body.file_group_ids:
         raise InvalidFile('file_group_ids cannot be empty')
-    session = request.ctx.session
+    # Read-then-write, so the transaction must begin as a writer (see `RequestDB.write`).
+    with request.ctx.db.write() as session:
+        if not body.force:
+            tagged = lib.get_tagged_file_groups_by_ids(session, body.file_group_ids)
+            if tagged:
+                return json_response(
+                    {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged},
+                    HTTPStatus.CONFLICT,
+                )
 
-    if not body.force:
-        tagged = lib.get_tagged_file_groups_by_ids(session, body.file_group_ids)
-        if tagged:
-            return json_response(
-                {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged},
-                HTTPStatus.CONFLICT,
-            )
-
-    affected_models = lib.delete_file_groups(session, body.file_group_ids, force=body.force)
+        affected_models = lib.delete_file_groups(session, body.file_group_ids, force=body.force)
 
     if 'video' in affected_models:
         # A Channel's tracked downloads may have changed; rewrite channels.yaml.
@@ -391,16 +391,18 @@ async def post_tag_file_group(request: Request, body: schema.TagFileGroupPost):
             HTTPStatus.BAD_REQUEST,
         )
 
-    await lib.add_file_group_tag(request.ctx.session, body.file_group_id, body.file_group_primary_path, body.tag_name,
-                                 body.tag_id)
+    with request.ctx.db.write() as session:
+        await lib.add_file_group_tag(session, body.file_group_id, body.file_group_primary_path, body.tag_name,
+                                     body.tag_id)
     return response.empty(HTTPStatus.CREATED)
 
 
 @files_bp.post('/untag')
 @validate(schema.TagFileGroupPost)
 async def post_untag_file_group(request: Request, body: schema.TagFileGroupPost):
-    await lib.remove_file_group_tag(request.ctx.session, body.file_group_id, body.file_group_primary_path,
-                                    body.tag_name, body.tag_id)
+    with request.ctx.db.write() as session:
+        await lib.remove_file_group_tag(session, body.file_group_id, body.file_group_primary_path,
+                                        body.tag_name, body.tag_id)
     return response.empty(HTTPStatus.NO_CONTENT)
 
 

wrolpi/files/test/test_api.py

@@ -9,6 +9,7 @@ import pytest
 
 from modules.videos import Video
 from wrolpi.common import get_wrolpi_config
+from wrolpi.conftest import production_like_sessions, write_lock_held_briefly
 from wrolpi.files.lib import get_mimetype
 from wrolpi.files.models import FileGroup, Directory
 from wrolpi.tags import TagFile
@@ -1636,3 +1637,35 @@ async def test_get_file_ignored_directory_not_tracked(test_session, async_client
     # Verify the ignored file is not in the database (because it was never refreshed)
     ignored_fgs = test_session.query(FileGroup).filter(FileGroup.primary_path.like('%config%')).all()
     assert len(ignored_fgs) == 0, "Files in ignored directories should not be in the database"
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize('step', ['tag', 'untag', 'delete_groups'])
+async def test_file_group_writes_wait_for_a_concurrent_writer(test_session, async_client, test_directory,
+                                                              example_singlefile, tag_factory, step):
+    """Tagging, untagging and deleting FileGroups read before they write, so each must begin as a writer:
+    SQLite fails a deferred transaction's lock upgrade instantly ("database is locked") if another connection
+    wrote meanwhile."""
+    file_group = FileGroup.from_paths(test_session, example_singlefile)
+    tag = await tag_factory()
+    if step == 'untag':
+        file_group.add_tag(test_session, tag.id)
+    test_session.commit()
+    db_file = test_session.get_bind().url.database
+
+    with production_like_sessions(test_session) as maker:
+        # Held past the slow work these requests do before they write (e.g. a config save).
+        with write_lock_held_briefly(db_file, seconds=3):
+            if step == 'delete_groups':
+                body = dict(file_group_ids=[file_group.id])
+                _, response = await async_client.post('/api/files/delete_groups', content=json.dumps(body))
+            else:
+                body = dict(file_group_id=file_group.id, tag_id=tag.id)
+                _, response = await async_client.post(f'/api/files/{step}', content=json.dumps(body))
+        assert response.status_code in (HTTPStatus.CREATED, HTTPStatus.NO_CONTENT), response.body
+
+        with maker() as session:
+            if step == 'delete_groups':
+                assert session.query(FileGroup).filter_by(id=file_group.id).one_or_none() is None
+            else:
+                assert session.query(TagFile).count() == (1 if step == 'tag' else 0)