From 56e0981936838c8e24c55a6fdb3b66ced227b22b Mon Sep 17 00:00:00 2001 From: John Hensley Date: Thu, 2 Apr 2020 10:43:18 -0400 Subject: [PATCH] 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 | 4 +++- securedrop_client/storage.py | 10 +++++----- securedrop_client/utils.py | 2 +- tests/test_storage.py | 16 +++++++++++++++- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/securedrop_client/api_jobs/uploads.py b/securedrop_client/api_jobs/uploads.py index 61ba02ccc..2b9059617 100644 --- a/securedrop_client/api_jobs/uploads.py +++ b/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. diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index a7f7716c6..4880ceb35 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -329,9 +329,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. @@ -343,12 +341,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 diff --git a/securedrop_client/utils.py b/securedrop_client/utils.py index 8f627566c..d78e83e55 100644 --- a/securedrop_client/utils.py +++ b/securedrop_client/utils.py @@ -88,7 +88,7 @@ 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") class SourceCache(object): diff --git a/tests/test_storage.py b/tests/test_storage.py index 4f1f5a5fd..b7d0634d5 100644 --- a/tests/test_storage.py +++ b/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 @@ -1206,3 +1207,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