-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 389 sync #405
Merged
Merged
Issue 389 sync #405
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b6710aa
add SyncJob
1e67efd
create MessageDownloadJob
0d847a9
only download if it's marked as not downloaded
7e746a7
remove message_sync and clean up
51434fa
show download msg only if there are new msg
cb9335e
fix existing tests
9f88d96
download_queue -> download_file_queue
adeb6dc
add new tests for MessageDownloadJob
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,19 +22,19 @@ | |
import os | ||
import sdclientapi | ||
import uuid | ||
from typing import Dict, Tuple, Union, Any, Type # noqa: F401 | ||
|
||
from gettext import gettext as _ | ||
from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer, QProcess, Qt | ||
from sdclientapi import RequestTimeoutError | ||
from sqlalchemy.orm.session import sessionmaker | ||
from typing import Dict, Tuple, Union, Any, Type # noqa: F401 | ||
|
||
from securedrop_client import storage | ||
from securedrop_client import db | ||
from securedrop_client.api_jobs.downloads import DownloadSubmissionJob | ||
from securedrop_client.api_jobs.downloads import FileDownloadJob, MessageDownloadJob | ||
from securedrop_client.api_jobs.uploads import SendReplyJob | ||
from securedrop_client.crypto import GpgHelper, CryptoError | ||
from securedrop_client.message_sync import MessageSync, ReplySync | ||
from securedrop_client.message_sync import ReplySync | ||
from securedrop_client.queue import ApiJobQueue | ||
from securedrop_client.utils import check_dir_permissions | ||
|
||
|
@@ -119,6 +119,12 @@ class Controller(QObject): | |
""" | ||
file_ready = pyqtSignal(str) | ||
|
||
""" | ||
This signal indicates that a file has been successfully downloaded by emitting the file's | ||
UUID as a string. | ||
""" | ||
message_ready = pyqtSignal([str, str]) | ||
|
||
def __init__(self, hostname: str, gui, session_maker: sessionmaker, | ||
home: str, proxy: bool = True) -> None: | ||
""" | ||
|
@@ -159,10 +165,6 @@ def __init__(self, hostname: str, gui, session_maker: sessionmaker, | |
|
||
self.gpg = GpgHelper(home, self.session_maker, proxy) | ||
|
||
# thread responsible for fetching messages | ||
self.message_thread = None | ||
self.message_sync = MessageSync(self.api, self.gpg, self.session_maker) | ||
|
||
# thread responsible for fetching replies | ||
self.reply_thread = None | ||
self.reply_sync = ReplySync(self.api, self.gpg, self.session_maker) | ||
|
@@ -271,19 +273,6 @@ def completed_api_call(self, thread_id, user_callback): | |
else: | ||
user_callback(result_data) | ||
|
||
def start_message_thread(self): | ||
""" | ||
Starts the message-fetching thread in the background. | ||
""" | ||
if not self.message_thread: | ||
self.message_sync.api = self.api | ||
self.message_thread = QThread() | ||
self.message_sync.moveToThread(self.message_thread) | ||
self.message_thread.started.connect(self.message_sync.run) | ||
self.message_thread.start() | ||
else: # Already running from last login | ||
self.message_sync.api = self.api | ||
|
||
def start_reply_thread(self): | ||
""" | ||
Starts the reply-fetching thread in the background. | ||
|
@@ -316,7 +305,6 @@ def on_authenticate_success(self, result): | |
self.sync_api() | ||
self.gui.show_main_window(self.api.username) | ||
|
||
self.start_message_thread() | ||
self.start_reply_thread() | ||
|
||
self.api_job_queue.start_queues(self.api) | ||
|
@@ -340,7 +328,6 @@ def login_offline_mode(self): | |
""" | ||
self.gui.hide_login() | ||
self.gui.show_main_window() | ||
self.start_message_thread() | ||
self.start_reply_thread() | ||
self.is_authenticated = False | ||
self.update_sources() | ||
|
@@ -389,19 +376,19 @@ def on_sync_success(self, result) -> None: | |
""" | ||
Called when syncronisation of data via the API succeeds | ||
""" | ||
# Update db with new metadata | ||
remote_sources, remote_submissions, remote_replies = result | ||
|
||
storage.update_local_storage(self.session, | ||
remote_sources, | ||
remote_submissions, | ||
remote_replies, | ||
self.data_dir) | ||
|
||
# Set last sync flag. | ||
# Set last sync flag | ||
with open(self.sync_flag, 'w') as f: | ||
f.write(arrow.now().format()) | ||
|
||
# import keys into keyring | ||
# Import keys into keyring | ||
for source in remote_sources: | ||
if source.key and source.key.get('type', None) == 'PGP': | ||
pub_key = source.key.get('public', None) | ||
|
@@ -413,15 +400,19 @@ def on_sync_success(self, result) -> None: | |
except CryptoError: | ||
logger.warning('Failed to import key for source {}'.format(source.uuid)) | ||
|
||
self.sync_events.emit('synced') | ||
# Update sources | ||
self.update_sources() | ||
|
||
# Download new messages | ||
self.download_new_messages() | ||
|
||
self.sync_events.emit('synced') | ||
|
||
def on_sync_failure(self, result: Exception) -> None: | ||
""" | ||
Called when syncronisation of data via the API fails. | ||
""" | ||
pass | ||
self.update_sources() | ||
logger.debug('Sync failed: "{}".'.format(result)) | ||
|
||
def update_sync(self): | ||
""" | ||
|
@@ -484,7 +475,6 @@ def logout(self): | |
state. | ||
""" | ||
self.api = None | ||
self.message_sync.api = None | ||
self.reply_sync.api = None | ||
self.gui.logout() | ||
self.is_authenticated = False | ||
|
@@ -496,6 +486,31 @@ def set_status(self, message, duration=5000): | |
""" | ||
self.gui.update_activity_status(message, duration) | ||
|
||
def download_new_messages(self) -> None: | ||
messages = storage.find_new_messages(self.session) | ||
|
||
if len(messages) > 0: | ||
self.set_status(_('Downloading new messages')) | ||
|
||
for message in messages: | ||
job = MessageDownloadJob(message.uuid, self.data_dir, self.gpg) | ||
job.success_signal.connect(self.on_message_download_success, type=Qt.QueuedConnection) | ||
job.failure_signal.connect(self.on_message_download_failure, type=Qt.QueuedConnection) | ||
self.api_job_queue.enqueue(job) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 |
||
|
||
def on_message_download_success(self, uuid: str) -> None: | ||
""" | ||
Called when a message has downloaded. | ||
""" | ||
message = storage.get_message(self.session, uuid) | ||
self.message_ready.emit(message.uuid, message.content) | ||
|
||
def on_message_download_failure(self, exception: Exception) -> None: | ||
""" | ||
Called when a message fails to download. | ||
""" | ||
self.set_status("The message download failed.") | ||
|
||
def on_file_open(self, file_uuid: str) -> None: | ||
""" | ||
Open the already downloaded file associated with the message (which is a `File`). | ||
|
@@ -528,7 +543,7 @@ def on_submission_download( | |
""" | ||
Download the file associated with the Submission (which may be a File or Message). | ||
""" | ||
job = DownloadSubmissionJob( | ||
job = FileDownloadJob( | ||
submission_type, | ||
submission_uuid, | ||
self.data_dir, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the plan still to inherit from a new common object to reduce code duplication with
FileDownloadJob
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! That's the plan:
MessageDownloadJob
,FileDownloadJob
, andReplyDownloadJob
will all inherit from aDownloadJob
class. I tried to keep the_decrypt_file
method generic enough so that we can move it intoDownloadJob
. There should be a follow-up PR for this.I just wanted to keep code changes small here since deleting the message_sync thread felt like a big change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filed #406 for this