add api sync background task

securedrop_client/sync.py

@@ -4,6 +4,7 @@
 from sqlalchemy.orm import scoped_session
 from sdclientapi import API, RequestTimeoutError
 
+from securedrop_client.api_jobs.base import ApiInaccessibleError
 from securedrop_client.api_jobs.sync import MetadataSyncJob
 from securedrop_client.crypto import GpgHelper
 
@@ -13,8 +14,9 @@
 
 class ApiSync(QObject):
     '''
-    ApiSync continuously executes a MetadataSyncJob, waiting 15 seconds between jobs.
+    ApiSync continuously syncs, waiting 15 seconds between task completion.
     '''
+
     sync_started = pyqtSignal()
     sync_success = pyqtSignal()
     sync_failure = pyqtSignal(Exception)
@@ -25,14 +27,20 @@ def __init__(
         self, api_client: API, session_maker: scoped_session, gpg: GpgHelper, data_dir: str
     ):
         super().__init__()
-
         self.api_client = api_client
-        self.session_maker = session_maker
-        self.gpg = gpg
-        self.data_dir = data_dir
 
         self.sync_thread = QThread()
-        self.sync_thread.started.connect(self._sync)
+        self.api_sync_bg_task = ApiSyncBackgroundTask(
+            api_client,
+            session_maker,
+            gpg,
+            data_dir,
+            self.sync_started,
+            self.on_sync_success,
+            self.on_sync_failure)
+        self.api_sync_bg_task.moveToThread(self.sync_thread)
+
+        self.sync_thread.started.connect(self.api_sync_bg_task.sync)
 
     def start(self, api_client: API) -> None:
         '''
@@ -42,9 +50,10 @@ def start(self, api_client: API) -> None:
 
         if not self.sync_thread.isRunning():
             logger.debug('Starting sync thread')
+            self.api_sync_bg_task.api_client = self.api_client
             self.sync_thread.start()
 
-    def stop(self):
+    def stop(self) -> None:
         '''
         Stop metadata syncs.
         '''
@@ -54,23 +63,62 @@ def stop(self):
             logger.debug('Stopping sync thread')
             self.sync_thread.quit()
 
-    def _sync(self):
+    def on_sync_success(self) -> None:
         '''
-        Create and run a new MetadataSyncJob.
+        Start another sync on success.
         '''
-        job = MetadataSyncJob(self.data_dir, self.gpg)
-        job.success_signal.connect(self._on_sync_success, type=Qt.QueuedConnection)
-        job.failure_signal.connect(self._on_sync_failure, type=Qt.QueuedConnection)
-
-        session = self.session_maker()
-        job._do_call_api(self.api_client, session)
-        self.sync_started.emit()
-
-    def _on_sync_success(self) -> None:
         self.sync_success.emit()
-        QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self._sync)
+        QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self.api_sync_bg_task.sync)
 
-    def _on_sync_failure(self, result: Exception) -> None:
+    def on_sync_failure(self, result: Exception) -> None:
+        '''
+        Only start another sync on failure if the reason is a timeout request.
+        '''
         self.sync_failure.emit(result)
         if isinstance(result, RequestTimeoutError):
-            QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self._sync)
+            QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self.api_sync_bg_task.sync)
+
+
+class ApiSyncBackgroundTask(QObject):
+    '''
+    ApiSyncBackgroundTask provides a sync method that executes a MetadataSyncJob.
+    '''
+
+    def __init__(
+        self,
+        api_client: API,
+        session_maker: scoped_session,
+        gpg: GpgHelper,
+        data_dir: str,
+        sync_started: pyqtSignal,
+        on_sync_success,
+        on_sync_failure
+    ):
+        super().__init__()
+
+        self.api_client = api_client
+        self.session_maker = session_maker
+        self.gpg = gpg
+        self.data_dir = data_dir
+        self.sync_started = sync_started
+        self.on_sync_success = on_sync_success
+        self.on_sync_failure = on_sync_failure
+
+        self.job = MetadataSyncJob(self.data_dir, self.gpg)
+        self.job.success_signal.connect(self.on_sync_success, type=Qt.QueuedConnection)
+        self.job.failure_signal.connect(self.on_sync_failure, type=Qt.QueuedConnection)
+
+    def sync(self) -> None:
+        '''
+        Create and run a new MetadataSyncJob.
+        '''
+        try:
+            self.sync_started.emit()
+            session = self.session_maker()
+            self.job._do_call_api(self.api_client, session)
+        except ApiInaccessibleError as e:
+            self.job.failure_signal.emit(e)  # the job's failure signal is not emitted in base
+        except Exception:
+            pass  # the job's failure signal is emitted for everything else in base
+        finally:
+            session.close()

tests/test_sync.py

@@ -1,5 +1,6 @@
 from sdclientapi import RequestTimeoutError
 
+from securedrop_client.api_jobs.base import ApiInaccessibleError
 from securedrop_client.sync import ApiSync
 
 
@@ -63,35 +64,79 @@ def test_ApiSync_stop_calls_quit(mocker, session_maker, homedir):
     api_sync.sync_thread.quit.assert_called_once_with()
 
 
-def test_ApiSync__sync(mocker, session_maker, homedir):
+def test_ApiSyncBackgroundTask_sync(mocker, session_maker, homedir):
     '''
     Ensure sync enqueues a MetadataSyncJob and calls it's parent's processing function
     '''
     api_client = mocker.MagicMock()
     api_sync = ApiSync(api_client, session_maker, mocker.MagicMock(), homedir)
-    sync_started = mocker.patch.object(api_sync, 'sync_started')
+    sync_started = mocker.patch.object(api_sync.api_sync_bg_task, 'sync_started')
     _do_call_api_fn = mocker.patch('securedrop_client.sync.MetadataSyncJob._do_call_api')
 
-    api_sync._sync()
+    api_sync.api_sync_bg_task.sync()
 
     sync_started.emit.assert_called_once_with()
     assert _do_call_api_fn.called
 
 
-def test_ApiSync__on_sync_success(mocker, session_maker, homedir):
+def test_ApiSyncBackgroundTask_sync_catches_ApiInaccessibleError(mocker, session_maker, homedir):
+    '''
+    Ensure sync calls the parent processing function of MetadataSyncJob, catches
+    ApiInaccessibleError exception, and emits failure signal.
+    '''
+    api_client = mocker.MagicMock()
+    api_sync = ApiSync(api_client, session_maker, mocker.MagicMock(), homedir)
+    sync_started = mocker.patch.object(api_sync.api_sync_bg_task, 'sync_started')
+    success_signal = mocker.patch('securedrop_client.sync.MetadataSyncJob.success_signal')
+    failure_signal = mocker.patch('securedrop_client.sync.MetadataSyncJob.failure_signal')
+    error = ApiInaccessibleError()
+    _do_call_api_fn = mocker.patch(
+        'securedrop_client.sync.MetadataSyncJob._do_call_api', side_effect=error)
+
+    api_sync.api_sync_bg_task.sync()
+
+    assert _do_call_api_fn.called
+    sync_started.emit.assert_called_once_with()
+    success_signal.emit.assert_not_called()
+    failure_signal.emit.assert_called_once_with(error)
+
+
+def test_ApiSyncBackgroundTask_sync_catches_all_other_exceptions(mocker, session_maker, homedir):
+    '''
+    Ensure sync calls the parent processing function of MetadataSyncJob, catches all exceptions,
+    and emits failure signal.
+    '''
+    api_client = mocker.MagicMock()
+    api_sync = ApiSync(api_client, session_maker, mocker.MagicMock(), homedir)
+    sync_started = mocker.patch.object(api_sync.api_sync_bg_task, 'sync_started')
+    success_signal = mocker.patch('securedrop_client.sync.MetadataSyncJob.success_signal')
+    failure_signal = mocker.patch('securedrop_client.sync.MetadataSyncJob.failure_signal')
+    error = Exception()
+    call_api_fn = mocker.patch(
+        'securedrop_client.sync.MetadataSyncJob.call_api', side_effect=error)
+
+    api_sync.api_sync_bg_task.sync()
+
+    assert call_api_fn.called
+    sync_started.emit.assert_called_once_with()
+    success_signal.emit.assert_not_called()
+    failure_signal.emit.assert_called_once_with(error)
+
+
+def test_ApiSync_on_sync_success(mocker, session_maker, homedir):
     '''
     Ensure success handler emits success signal that the Controller links to and fires another sync
     after a supplied amount of time.
     '''
     api_sync = ApiSync(mocker.MagicMock(), session_maker, mocker.MagicMock(), homedir)
     sync_success = mocker.patch.object(api_sync, 'sync_success')
 
-    api_sync._on_sync_success()
+    api_sync.on_sync_success()
 
     sync_success.emit.assert_called_once_with()
 
 
-def test_ApiSync__on_sync_failure(mocker, session_maker, homedir):
+def test_ApiSync_on_sync_failure(mocker, session_maker, homedir):
     '''
     Ensure failure handler emits failure signal that the Controller links to and does not fire
     another sync for errors other than RequestTimeoutError
@@ -102,13 +147,13 @@ def test_ApiSync__on_sync_failure(mocker, session_maker, homedir):
 
     error = Exception()
 
-    api_sync._on_sync_failure(error)
+    api_sync.on_sync_failure(error)
 
     sync_failure.emit.assert_called_once_with(error)
     singleShot_fn.assert_not_called()
 
 
-def test_ApiSync__on_sync_failure_because_of_timeout(mocker, session_maker, homedir):
+def test_ApiSync_on_sync_failure_because_of_timeout(mocker, session_maker, homedir):
     '''
     Ensure failure handler emits failure signal that the Controller links to and sets up timer to
     fire another sync after 15 seconds if the failure reason is a RequestTimeoutError.
@@ -118,7 +163,7 @@ def test_ApiSync__on_sync_failure_because_of_timeout(mocker, session_maker, home
     singleShot_fn = mocker.patch('securedrop_client.sync.QTimer.singleShot')
     error = RequestTimeoutError()
 
-    api_sync._on_sync_failure(error)
+    api_sync.on_sync_failure(error)
 
     sync_failure.emit.assert_called_once_with(error)
-    singleShot_fn.assert_called_once_with(15000, api_sync._sync)
+    singleShot_fn.assert_called_once_with(15000, api_sync.api_sync_bg_task.sync)