Skip to content

Commit

Permalink
log and auto-retry bg syncs
Browse files Browse the repository at this point in the history
  • Loading branch information
Allie Crevier committed Dec 6, 2019
1 parent 81c6f31 commit 04289ff
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 10 deletions.
2 changes: 1 addition & 1 deletion securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def setup(self, controller):
self.controller.sync_events.connect(self._on_refresh_complete)

def _on_clicked(self):
self.controller.sync_api()
self.controller.sync_api(manual_refresh=True)
# This is a temporary solution for showing the icon as active for the entire duration of a
# refresh, rather than for just the duration of a click. The icon image will be replaced
# when the controller tells us the refresh has finished. A cleaner solution would be to
Expand Down
29 changes: 22 additions & 7 deletions securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,12 @@ def completed_api_call(self, thread_id, user_callback):

def login(self, username, password, totp):
"""
Given a username, password and time based one-time-passcode (TOTP),
create a new instance representing the SecureDrop api and authenticate.
Given a username, password and time based one-time-passcode (TOTP), create a new instance
representing the SecureDrop api and authenticate.
Default to 60 seconds until we implement a better request timeout strategy.
Default to 60 seconds until we implement a better request timeout strategy. We lower the
default_request_timeout for Queue API requests in ApiJobQueue in order to display errors
faster.
"""
storage.mark_all_pending_drafts_as_failed(self.session)
self.api = sdclientapi.API(
Expand Down Expand Up @@ -366,7 +368,7 @@ def authenticated(self):
"""
return bool(self.api and self.api.token is not None)

def sync_api(self):
def sync_api(self, manual_refresh: bool = False):
"""
Grab data from the remote SecureDrop API in a non-blocking manner.
"""
Expand All @@ -377,9 +379,13 @@ def sync_api(self):
logger.debug("You are authenticated, going to make your call")

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)
# Handle failure depending on whether or not the sync originated from a manual refresh
if manual_refresh:
job.failure_signal.connect(self.on_refresh_failure, type=Qt.QueuedConnection)
else:
job.failure_signal.connect(self.on_sync_failure, type=Qt.QueuedConnection)

self.api_job_queue.enqueue(job)

logger.debug("In sync_api, after call to submit job to queue, on "
Expand Down Expand Up @@ -415,8 +421,17 @@ def on_sync_success(self) -> None:

def on_sync_failure(self, result: Exception) -> None:
"""
Called when syncronisation of data via the API queue fails.
Called when syncronisation of data via the API fails. Just log and try again since this is
a background task.
"""
logger.debug('The SecureDrop server cannot be reached due to Error: {}'.format(result))
self.sync_api()

def on_refresh_failure(self, result: Exception) -> None:
"""
Called when syncronisation of data via the API fails after a user manual clicks refresh.
"""
logger.debug('The SecureDrop server cannot be reached due to Error: {}'.format(result))
self.gui.update_error_status(
_('The SecureDrop server cannot be reached.'),
duration=0,
Expand Down
10 changes: 10 additions & 0 deletions securedrop_client/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ def logout(self) -> None:

def login(self, api_client: API) -> None:
logger.debug('Passing API token to queues')

# Setting realistic (shorter) timeout for general requests so that user feedback is faster.
# General requests includes:
# FileDownloadJob
# MessageDownloadJob
# ReplyDownloadJo
# SendReplyJob
# UpdateStarJob
api_client.default_request_timeout = 5

self.main_queue.api_client = api_client
self.download_file_queue.api_client = api_client
self.start_queues()
Expand Down
4 changes: 2 additions & 2 deletions tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,14 @@ def test_RefreshButton_setup(mocker):

def test_RefreshButton_on_clicked(mocker):
"""
When refresh button is clicked, sync_api should be called.
When refresh button is clicked, sync_api should be called with manual_refresh set to True.
"""
rb = RefreshButton()
rb.controller = mocker.MagicMock()

rb._on_clicked()

rb.controller.sync_api.assert_called_once_with()
rb.controller.sync_api.assert_called_once_with(manual_refresh=True)


def test_RefreshButton_on_refresh_complete(mocker):
Expand Down
35 changes: 35 additions & 0 deletions tests/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,23 @@ def test_Controller_sync_api(homedir, config, mocker, session_maker):
co.api_job_queue.enqueue.call_count == 1


def test_Controller_sync_api_manual_refresh(homedir, config, mocker, session_maker):
"""
Syncing from a manual refresh also enqueues a job.
"""
mock_gui = mocker.MagicMock()

co = Controller('http://localhost', mock_gui, session_maker, homedir)

co.authenticated = mocker.MagicMock(return_value=True)
co.api_job_queue = mocker.MagicMock()
co.api_job_queue.enqueue = mocker.MagicMock()

co.sync_api(manual_refresh=True)

co.api_job_queue.enqueue.call_count == 1


def test_Controller_last_sync_with_file(homedir, config, mocker, session_maker):
"""
The flag indicating the time of the last sync with the API is stored in a
Expand Down Expand Up @@ -417,11 +434,29 @@ def test_Controller_on_sync_failure(homedir, config, mocker, session_maker):
"""
gui = mocker.MagicMock()
co = Controller('http://localhost', gui, session_maker, homedir)
co.sync_api = mocker.MagicMock()
exception = Exception('mock') # Not the expected tuple.
mock_storage = mocker.patch('securedrop_client.logic.storage')

co.on_sync_failure(exception)

assert mock_storage.update_local_storage.call_count == 0
co.sync_api.assert_called_once_with()


def test_Controller_on_refresh_failure(homedir, config, mocker, session_maker):
"""
If there's no result to syncing, then don't attempt to update local storage
and perhaps implement some as-yet-undefined UI update.
Using the `config` fixture to ensure the config is written to disk.
"""
gui = mocker.MagicMock()
co = Controller('http://localhost', gui, session_maker, homedir)
exception = Exception('mock') # Not the expected tuple.
mock_storage = mocker.patch('securedrop_client.logic.storage')

co.on_refresh_failure(exception)

assert mock_storage.update_local_storage.call_count == 0
gui.update_error_status.assert_called_once_with(
'The SecureDrop server cannot be reached.', duration=0, retry=True)
Expand Down

0 comments on commit 04289ff

Please sign in to comment.