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

@@ -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:
         """

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.

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):

tests/test_queue.py

@@ -312,16 +312,24 @@ 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()
 
     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):