Make some commits optional in storage.py

Add boolean "commit" argument to find_or_create_user and
update_draft_replies.

Also make utils.chronometer log at INFO, so you don't have to run at
DEBUG to time things. Debug logging is significantly slower in
intensive loops like storage.update_replies, which can take up to a
couple of seconds longer with debug enabled, given 1000 sources with
two replies each.

securedrop_client/api_jobs/uploads.py

@@ -75,7 +75,9 @@ def call_api(self, api_client: API, session: Session) -> str:
             draft_file_counter = draft_reply_db_object.file_counter
             draft_timestamp = draft_reply_db_object.timestamp
             update_draft_replies(
-                session, source.id, draft_timestamp, draft_file_counter, new_file_counter)
+                session, source.id, draft_timestamp, draft_file_counter, new_file_counter,
+                commit=False
+            )
 
             # Add reply to replies table and increase the source interaction count by 1 and delete
             # the draft reply.

securedrop_client/storage.py

@@ -287,7 +287,7 @@ def update_replies(remote_replies: List[SDKReply], local_replies: List[Reply],
             if not user:
                 logger.debug(f"user {reply.journalist_uuid} not cached")
                 user = find_or_create_user(
-                    reply.journalist_uuid, reply.journalist_username, session
+                    reply.journalist_uuid, reply.journalist_username, session, commit=False
                 )
                 users[reply.journalist_uuid] = user
 
@@ -335,9 +335,7 @@ def update_replies(remote_replies: List[SDKReply], local_replies: List[Reply],
     session.commit()
 
 
-def find_or_create_user(uuid: str,
-                        username: str,
-                        session: Session) -> User:
+def find_or_create_user(uuid: str, username: str, session: Session, commit: bool = True) -> User:
     """
     Returns a user object representing the referenced journalist UUID.
     If the user does not already exist in the data, a new instance is created.
@@ -349,12 +347,14 @@ def find_or_create_user(uuid: str,
         new_user = User(username=username)
         new_user.uuid = uuid
         session.add(new_user)
-        session.commit()
+        if commit:
+            session.commit()
         return new_user
 
     if user.username != username:
         user.username = username
-        session.commit()
+        if commit:
+            session.commit()
 
     return user
 

securedrop_client/utils.py

@@ -84,4 +84,4 @@ def chronometer(logger: logging.Logger, description: str) -> Generator:
         yield
     finally:
         elapsed = time.perf_counter() - start
-        logger.debug(f"{description} duration: {elapsed:.4f}s")
+        logger.info(f"{description} duration: {elapsed:.4f}s")

tests/test_storage.py

@@ -20,7 +20,8 @@
     delete_single_submission_or_reply_on_disk, get_local_files, find_new_files, \
     source_exists, set_message_or_reply_content, mark_as_downloaded, mark_as_decrypted, get_file, \
     get_message, get_reply, update_and_get_user, update_missing_files, mark_as_not_downloaded, \
-    mark_all_pending_drafts_as_failed, delete_local_source_by_uuid, update_file_size
+    mark_all_pending_drafts_as_failed, delete_local_source_by_uuid, update_file_size, \
+    update_draft_replies
 
 from securedrop_client import db
 from tests import factory
@@ -1180,3 +1181,16 @@ def test_update_file_size(homedir, session):
     update_file_size(f.uuid, data_dir, session)
 
     assert f.size == real_size
+
+
+def test_update_draft_replies_commit(mocker, session):
+    """
+    Tests the commit argument of storage.update_draft_replies.
+    """
+    session.commit = mocker.MagicMock()
+
+    update_draft_replies(session, "notreal", datetime.datetime.now(), 1, 2, commit=False)
+    assert session.commit.call_count == 0
+
+    update_draft_replies(session, "notreal", datetime.datetime.now(), 1, 2)
+    assert session.commit.call_count == 1