Skip to content
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

Replace MessageSync with DownloadMessageJob #389

Closed
redshiftzero opened this issue May 29, 2019 · 4 comments · Fixed by #405
Closed

Replace MessageSync with DownloadMessageJob #389

redshiftzero opened this issue May 29, 2019 · 4 comments · Fixed by #405
Assignees
Labels
enhancement New feature or request

Comments

@redshiftzero
Copy link
Contributor

We should replace MessageSync with submitting message download jobs (DownloadMessageJob) to the general/"main" queue. This is so that we're not maintaining duplicate logic for managing the sending and retrieving data from the server.

DownloadMessageJob could be a subclass of the existing job DownloadSubmissionJob. Note that we're using the type of ApiJobQueue.enqueue to determine whether to submit to the main queue or the file download queue: we want submissions of type files to go to the download file queue, and submissions of type messages to go to the regular queue.

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Jun 5, 2019

Thoughts

My assumption is that we want to update the conversations in the GUI after BOTH MessageSync and ReplySync are finished.

Observations

Please correct me if any of my observations or incorrect in some way.

A. MessageSync has the same logic as ReplySync:

  • both run every 5s to look for new metadata that's introduced by the 5-minute metadata download (or by a manual refresh)
  • both download and decrypt new data across sources

B. DownloadSubmissionJob has similar logic:

  • it downloads and decrypts a single specified submission (either a file or message)

C. A DownloadMessageJob is really like a DownloadMessagesJob (plural) if we're looking to have it replace the MessageSync thread

D. We currently route DownloadSubmissionJobs to the download_queue (our long-job queue)

Proposal

  1. Create the following jobs:
  • DownloadJob that has download and decrypt methods
  • DownloadFileJob that subclasses DownloadJob
  • DownloadAllNewMessagesAndRepliesJob that subclasses DownloadJob
  • MetadataUpdateJob that gets routed to a metadata_queue so that it always happens every 5 minutes or when a user manually refreshes (a successful run should also trigger a DownloadAllNewMessagesAndRepliesJob so that the sync is complete)

Note: The reason it is important to have a single DownloadAllNewMessagesAndRepliesJob instead of a DownloadMessageJob, DownloadReplyJob, and SyncJob that creates a bunch of those two download jobs is so we know when to update the GUI with the new conversation items and inform the user that they're looking at up-to-date messages.

  1. Route DownloadFileJobs (not DownloadSubmissionJobs) to the download_queue (rename to file_download_queue)

  2. Route DownloadAllNewMessagesAndRepliesJob to the main queue

For this issue

Just do the following:

  • DownloadAllNewMessagesAndRepliesJob that subclasses DownloadJob

@redshiftzero what do you think?

@sssoleileraaa
Copy link
Contributor

We might want to rename DownloadJob to DownloadAndDecryptJob to be more explicit.

@sssoleileraaa
Copy link
Contributor

Now that I'm thinking about this more, DownloadAllNewMessagesAndRepliesJob also doesn't need to happen every 5 seconds. It can just happen whenever we successfully download new metadata.

@redshiftzero
Copy link
Contributor Author

redshiftzero commented Jun 5, 2019

(summarizing synchronous chat @creviera and I had about this ticket)

One of the upsides of the current MessageSync approach is that after we first get all metadata for messages, then we download/decrypt them one at a time, and update the relevant message bubble as they are decrypted and ready to be displayed. This way if there are many thousands of messages, they will start to render as they are processed, so 1. the user can start reading, and 2. they know that things are happening. This is one of the reasons why having a separate job per message or reply download/decrypt I think makes more sense (as we can preserve this individual item processing behavior). I think we'd do something like:

  1. Check for messages that need to be downloaded and/or decrypted via storage.find_new_messages(session) here in logic.py once the metadata sync completes

  2. We add a for loop directly after that which enqueues a download job for each message that was found in step 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants