Implementation of the API queue

[heartsucker]

Fixes #224

Adds the initial implementation of the ApiJobQueue. This diverged a bit from the original design spec, but still fundamentally accomplishes the task of strict ordering of replies. This PR only contains ApiJobs for downloading files from the API, but using the patterns here, the rest should be relatively easy to implement.

Design Choices

An ApiJob only needs to implement one method called call_api that should do everything needed to process an API including writing it to the local DB. This method might be renamed run_job for clarity.

On success/failure, respective signals are emitted. These are connected to callbacks on the Controller which then takes action directly and possible relays the signal to other QObjects. In the case of a DownloadSubmissionJob, we don't want to directly connect the FileWidgets to the signals on the job because that is likely too much connectedness. Using the Controller as a relay helps maintain separation.

Follow Ups

There's a few tickets we'll need to add to polish this up. Addressing all of these would require too much time and already this PR is fairly large and complex.

Timeout / No Auth

There is no signal for for ApiInacessibleError or RequestTimeoutError, and there needs to be as these being raised requires action from the user. As it stands now, either of these errors will either abort the thread (with no way to restart) or crash the app. I haven't tested.

Restarting Threads

If a thread aborts because it has no auth or timeout, there needs to be a way to restart it. Likely this would mean calling ApiJobQueue.start_queues() again, but as it's written now, this wouldn't restart the current threads but it would create new ones (or do some other badness).

[redshiftzero]

see my commit here: b84f34e which resolves a situation where the first queue (main_queue) would begin in the main thread, and since it continuously runs that will cause the second queue (download_queue) to never start. looking at the diff in the above commit, removing the signal/slot registration happening across threads (halt_signal) resolves, and both queues can start without issue. this is something we should be careful about (I don't think we're doing this anywhere else in the client codebase, which is why we've never hit this, worth double checking though). The relevant place in the docs for this is this nice page on thread synchronization which describes about signal/slot connections:

... a queued connection must be used because a direct connection bypasses the event system and runs the method immediately in the current thread.

[sssoleileraaa]

First Problem
Test this this morning, the application would hang for about 10 seconds and then I'd have to force quit:

Solution
fd6b8b7 (connect threads to the queue's start method)

Second Problem
The api_client is unexpectedly None: 25c63ee#diff-dfebdbc5258e6805fadd416909ed2a90R83

Solution
@heartsucker solved this somehow, which we will see in an upcoming commit, along with tests! 🎉

[sssoleileraaa]

Second Problem
The api_client is unexpectedly None: 25c63ee#diff-dfebdbc5258e6805fadd416909ed2a90R83

Solution
@heartsucker solved this somehow, which we will see in an upcoming commit, along with tests! 🎉

So recap: The problem here was that we could not make an api call from a DownloadSubmissionJob instance because the API object called api_client was None. The problem was solved here: f9d151b#diff-473509107be52a27c95435c68c00c7d1R320 by passing in the API object to the start_queues method after a user is successfully authenticated.

[sssoleileraaa]

Third Problem

SQLite objects created in a thread can only be used in that same thread.The object was created in thread id 140518486951680 and this is thread id 140518512129792

Solution
Instead of passing database objects to jobs that are executed on a separate thread, pass around the primary key to access that object and a session maker so that the object can be queried and accessed in a new session. This is a solution that heartsucker made in the most recent commits.

Fourth Problem

1. Click on 'Download' next to the FileWidget from the client GUI
2. See that the download starts from the activity bar
3. See that the file is downloaded and decrypted by seeing the is_downloaded and is_decrypted fields of the file in the files table are set. See the files on the filesystem.
4. Even though the file is downloaded/decrypted and the success signal is sent to the GUI, the GUI's database session is not picking up the latest changes, so the file looks like it hasn't been downloaded still.

The problem is still be investigated and debugged.

[sssoleileraaa]

Follow-up issues

These are issues that we can address outside of this PR, mentioned in the description

• Timeout / No Auth
• Restarting Threads
• Passing around session_maker
• handle RuntimeError when file hash does not validate
• add retries

[redshiftzero]

this looks great to me, just a couple notes inline

[sssoleileraaa]

Checkpoint

There are the following 7 more broken tests and some missing test coverage for this PR, in case someone in a different timezone wants to work on this with me.

Current test coverage (lowest percentage is 84% for message_sync.py):

securedrop_client/gui/widgets.py            775      2    99%   1322-1323
securedrop_client/logic.py                  271      8    97%   525-535
securedrop_client/message_sync.py            85     14    84%   62-65, 67-68, 122-127, 181-186
securedrop_client/queue.py                  138     19    86%   149-167, 225-229, 232-235

Please just push frequently. I'll probably be back online in a few hours, but maybe not until tomorrow morning.

[sssoleileraaa]

A couple bugs that need to be fixed:

• Messages are not loaded on a clean startup of the application, STR: start the server, start the client, sources are missing messages
• Refresh status in GUI continues to show active state after a manual refresh, STR: click refresh
• Clicking on "Open" next to file crashes, STR: click open

[sssoleileraaa]

Note

Need to document what ETag is all about, this could be addressed when working on this issue: #383

See log:

(_check_file_integrity) DEBUG: No ETag. Skipping integrity check for file at 1-censorious_dhow-doc.gz.gpg

[heartsucker]

@creviera

Messages are not loaded on a clean startup of the application, STR: start the server, start the client, sources are missing messages

This is #361 I think.

[heartsucker]

Refresh status in GUI continues to show active state after a manual refresh, STR: click refresh

This is a bug in master and should be handled in a separate ticket.

[sssoleileraaa]

@heartsucker - I see the issues happening on master now. Makes sense to address them outside of this PR.

[redshiftzero]

my comments were also addressed, lgtm!