diff --git a/securedrop_client/sync.py b/securedrop_client/sync.py index 186dec17b..cac434b69 100644 --- a/securedrop_client/sync.py +++ b/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() diff --git a/tests/test_sync.py b/tests/test_sync.py index 39663bb70..bd1b4f739 100644 --- a/tests/test_sync.py +++ b/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,22 +64,66 @@ 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. @@ -86,12 +131,12 @@ def test_ApiSync__on_sync_success(mocker, session_maker, homedir): 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)