From eb9ce9c7146a6a4f8b3dee6448b7172561bf4079 Mon Sep 17 00:00:00 2001
From: "Nicholas H.Tollervey" <ntoll@ntoll.org>
Date: Mon, 27 Jan 2020 11:10:03 +0000
Subject: [PATCH] Address review comments by @creviera: resume queues on sync
 success, only emit resume signal if queue is paused. Fixed tests too.

---
 securedrop_client/logic.py |  1 +
 securedrop_client/queue.py | 14 +++++++++-----
 tests/test_logic.py        |  2 ++
 tests/test_queue.py        |  8 ++++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py
index 244d9f6cf..ccc91ff22 100644
--- a/securedrop_client/logic.py
+++ b/securedrop_client/logic.py
@@ -428,6 +428,7 @@ def on_sync_success(self) -> None:
         self.download_new_messages()
         self.download_new_replies()
         self.sync_events.emit('synced')
+        self.resume_queues()
 
     def on_sync_failure(self, result: Exception) -> None:
         """
diff --git a/securedrop_client/queue.py b/securedrop_client/queue.py
index fe7f243d2..a52ce4517 100644
--- a/securedrop_client/queue.py
+++ b/securedrop_client/queue.py
@@ -171,8 +171,6 @@ def __init__(self, api_client: API, session_maker: scoped_session) -> None:
         self.download_file_queue.paused.connect(self.on_queue_paused)
         self.metadata_queue.paused.connect(self.on_queue_paused)
 
-        self.main_queue.pinged.connect(self.resume_queues)
-        self.download_file_queue.pinged.connect(self.resume_queues)
         self.metadata_queue.pinged.connect(self.resume_queues)
 
     def logout(self) -> None:
@@ -205,10 +203,16 @@ def on_queue_paused(self) -> None:
 
     def resume_queues(self) -> None:
         logger.info("Resuming queues")
+        main_paused = not self.main_thread.isRunning()
+        download_paused = not self.download_file_thread.isRunning()
+        metadata_paused = not self.metadata_thread.isRunning()
         self.start_queues()
-        self.main_queue.resume.emit()
-        self.download_file_queue.resume.emit()
-        self.metadata_queue.resume.emit()
+        if main_paused:
+            self.main_queue.resume.emit()
+        if download_paused:
+            self.download_file_queue.resume.emit()
+        if metadata_paused:
+            self.metadata_queue.resume.emit()
 
     def enqueue(self, job: ApiJob) -> None:
         # Prevent api jobs being added to the queue when not logged in.
diff --git a/tests/test_logic.py b/tests/test_logic.py
index 4ecf67c8b..ecbaf364e 100644
--- a/tests/test_logic.py
+++ b/tests/test_logic.py
@@ -475,6 +475,7 @@ def test_Controller_on_sync_success(homedir, config, mocker):
     co.download_new_messages = mocker.MagicMock()
     co.download_new_replies = mocker.MagicMock()
     co.gpg = mocker.MagicMock()
+    co.resume_queues = mocker.MagicMock()
     mock_storage = mocker.patch('securedrop_client.logic.storage')
 
     co.on_sync_success()
@@ -483,6 +484,7 @@ def test_Controller_on_sync_success(homedir, config, mocker):
     co.update_sources.assert_called_once_with()
     co.download_new_messages.assert_called_once_with()
     co.download_new_replies.assert_called_once_with()
+    co.resume_queues.assert_called_once_with()
 
 
 def test_Controller_update_sync(homedir, config, mocker, session_maker):
diff --git a/tests/test_queue.py b/tests/test_queue.py
index a7e718658..ba49a3567 100644
--- a/tests/test_queue.py
+++ b/tests/test_queue.py
@@ -312,9 +312,16 @@ def test_ApiJobQueue_pause_queues(mocker):
 
 
 def test_ApiJobQueue_resume_queues_emits_resume_signal(mocker):
+    """
+    Resume only emits if the queue is paused.
+    """
     job_queue = ApiJobQueue(mocker.MagicMock(), mocker.MagicMock())
     mocker.patch.object(job_queue.main_queue, 'resume')
     mocker.patch.object(job_queue.download_file_queue, 'resume')
+    mocker.patch.object(job_queue.metadata_queue, 'resume')
+    job_queue.main_thread.isRunning = mocker.MagicMock(return_value=False)
+    job_queue.download_file_thread.isRunning = mocker.MagicMock(return_value=False)
+    job_queue.metadata_thread.isRunning = mocker.MagicMock(return_value=False)
     job_queue.start_queues = mocker.MagicMock()
 
     job_queue.resume_queues()
@@ -322,6 +329,7 @@ def test_ApiJobQueue_resume_queues_emits_resume_signal(mocker):
     job_queue.start_queues.assert_called_once_with()
     job_queue.main_queue.resume.emit.assert_called_once_with()
     job_queue.download_file_queue.resume.emit.assert_called_once_with()
+    job_queue.metadata_queue.resume.emit.assert_called_once_with()
 
 
 def test_ApiJobQueue_enqueue_no_auth(mocker):