Archive and doc pages record a view in its own write transaction (#813)

Viewing an archive or a doc marked it viewed on the request's read transaction. SQLite cannot
upgrade a read to a writer once another connection has committed (during a download or refresh),
and it does not wait on busy_timeout for that. The archive page returned a 500; the doc page
returned 200 but its view was lost when the request's commit failed.

Build the response on the read, then record the view in `request.ctx.db.write()`, as the video
page already does.

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

modules/archive/api.py

@@ -38,9 +38,13 @@ async def statistics(request: Request):
 async def get_archive(request: Request, file_group_id: int):
     session = request.ctx.session
     skip_viewed = request.args.get('skip_viewed', '').lower() == 'true'
-    archive = lib.get_archive_by_file_group_id(session, file_group_id, skip_viewed=skip_viewed)
+    archive = lib.get_archive_by_file_group_id(session, file_group_id, skip_viewed=True)
     archive_file_group = archive.file_group.__json__()
     history = [i.file_group.__json__() for i in archive.history]
+    if not skip_viewed:
+        # Marking the Archive viewed is a write; it must not upgrade the read above (see `RequestDB.write`).
+        with request.ctx.db.write():
+            archive_file_group['viewed'] = archive.file_group.set_viewed()
     return json_response({'file_group': archive_file_group, 'history': history})
 
 

modules/archive/test/test_api.py

@@ -7,6 +7,10 @@ import pytest
 from modules.archive import lib, Archive
 from modules.archive.lib import UploadedArchive
 from wrolpi.common import get_relative_to_media_directory
+from wrolpi.conftest import production_like_sessions
+from wrolpi.db import get_db_session
+from wrolpi.files.models import FileGroup
+from wrolpi.tags import Tag
 from wrolpi.test.common import skip_ci, skip_macos
 
 
@@ -632,3 +636,33 @@ async def test_get_archive_loads_history_eagerly(test_session, archive_factory,
     assert all(i['tags'] == [tag.name] for i in long_json['history'])
     # Five history Archives cost no more queries than one.
     assert long_statements == short_statements, (short_statements, long_statements)
+
+
+@pytest.mark.asyncio
+async def test_get_archive_records_view_after_a_concurrent_write(test_session, archive_factory, async_client):
+    """Viewing an Archive still succeeds, and is recorded, when another connection commits during the request."""
+    archive = archive_factory('example.com', 'https://example.com/page')
+    archive_factory('example.com', 'https://example.com/page')
+    file_group_id = archive.file_group_id
+    test_session.commit()
+
+    __json__ = FileGroup.__json__
+    concurrent_writes = []
+
+    def json_after_a_concurrent_write(self):
+        # A download or refresh commits while the page is being read.
+        if not concurrent_writes:
+            with get_db_session(commit=True) as session:
+                session.add(Tag(name='concurrent', color='#000000'))
+            concurrent_writes.append(True)
+        return __json__(self)
+
+    with production_like_sessions(test_session), \
+            patch.object(FileGroup, '__json__', json_after_a_concurrent_write):
+        request, response = await async_client.get(f'/api/archive/{file_group_id}')
+    assert response.status_code == HTTPStatus.OK, response.json
+    assert len(response.json['history']) == 1
+    assert response.json['file_group']['viewed']
+
+    test_session.rollback()  # End the test's own read snapshot to see the request's commit.
+    assert test_session.query(FileGroup).filter_by(id=file_group_id).one().viewed is not None

modules/docs/api.py

@@ -31,8 +31,13 @@ async def statistics(request: Request):
 async def get_doc(request: Request, file_group_id: int):
     session = request.ctx.session
     skip_viewed = request.args.get('skip_viewed', '').lower() == 'true'
-    doc = _get_doc(session, file_group_id, skip_viewed=skip_viewed)
-    return json_response(_doc_response(doc))
+    doc = _get_doc(session, file_group_id, skip_viewed=True)
+    response = _doc_response(doc)
+    if not skip_viewed:
+        # Marking the doc viewed is a write; it must not upgrade the read above (see `RequestDB.write`).
+        with request.ctx.db.write():
+            response['file_group']['viewed'] = doc.file_group.set_viewed()
+    return json_response(response)
 
 
 doc_limit_limiter = api_param_limiter(100)

modules/docs/test/test_api.py

@@ -1,10 +1,15 @@
 """Tests for the docs API."""
 import json
 from http import HTTPStatus
+from unittest.mock import patch
 
 import pytest
 
 from modules.docs.models import Doc
+from wrolpi.conftest import production_like_sessions
+from wrolpi.db import get_db_session
+from wrolpi.files.models import FileGroup
+from wrolpi.tags import Tag
 
 
 @pytest.mark.asyncio
@@ -169,3 +174,30 @@ async def test_delete_doc_atomic_when_some_tagged(async_client, test_session, do
     assert all(p.is_file() for p in untagged_paths)
     assert all(p.is_file() for p in tagged_paths)
     assert test_session.query(Doc).count() == 2
+
+
+@pytest.mark.asyncio
+async def test_get_doc_records_view_after_a_concurrent_write(async_client, test_session, doc_factory):
+    """Viewing a doc still succeeds, and is recorded, when another connection commits during the request."""
+    file_group_id = doc_factory().file_group_id
+    test_session.commit()
+
+    __json__ = FileGroup.__json__
+    concurrent_writes = []
+
+    def json_after_a_concurrent_write(self):
+        # A download or refresh commits while the doc is being read.
+        if not concurrent_writes:
+            with get_db_session(commit=True) as session:
+                session.add(Tag(name='concurrent', color='#000000'))
+            concurrent_writes.append(True)
+        return __json__(self)
+
+    with production_like_sessions(test_session), \
+            patch.object(FileGroup, '__json__', json_after_a_concurrent_write):
+        request, response = await async_client.get(f'/api/docs/{file_group_id}')
+    assert response.status_code == HTTPStatus.OK, response.json
+    assert response.json['file_group']['viewed']
+
+    test_session.rollback()  # End the test's own read snapshot to see the request's commit.
+    assert test_session.query(FileGroup).filter_by(id=file_group_id).one().viewed is not None