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

Decrypt and Import full backups in chunk with progress #4005

Merged
merged 10 commits into from
Jan 19, 2024

Conversation

BillCarsonFr
Copy link
Member

@BillCarsonFr BillCarsonFr commented Jan 12, 2024

Fixes element-hq/element-web#26856
Fixes element-hq/element-web#23359 as it will chunk the importing of keys.

Current State

Currently Web will download all the keys from the server, then will decrypt them all and keep them in memory (with no feedbacks during the decryption process), then will import them all at once to the crypto store.

Changes

Now the keys will be split in chunks, then each chunk is decrypted and imported before processing to the next one.

restore_progress

This is a good improvement, but more work need to be done to have a proper download of a big backup. As an example you can't cancel an import at the moment. So if you close the dialog it will continue without feedback, and you can also start a new one in parallel.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

🐛 Bug Fixes

@BillCarsonFr BillCarsonFr force-pushed the valere/report_progress_on_fullbakup_import branch from f76e465 to 509613f Compare January 12, 2024 17:16
@BillCarsonFr BillCarsonFr marked this pull request as ready for review January 12, 2024 17:21
@BillCarsonFr BillCarsonFr requested a review from a team as a code owner January 12, 2024 17:21
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM generally. I assume this still downloads the lot in one lump, but this would need server changes so one step at a time and this is definitely a step in the right direcvtion. Might want a look from someone crypto-y too.

*
* @returns A promise that resolves when the decryption is complete.
*/
private async handleDecryptionOfAFullBackup(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a nice use of a generator function, potentially? Not a blocker though.

Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - lots of tiny comments but nothing major. Thanks!

spec/integ/crypto/megolm-backup.spec.ts Outdated Show resolved Hide resolved
spec/integ/crypto/megolm-backup.spec.ts Outdated Show resolved Hide resolved
spec/integ/crypto/megolm-backup.spec.ts Outdated Show resolved Hide resolved
spec/integ/crypto/megolm-backup.spec.ts Show resolved Hide resolved
spec/integ/crypto/megolm-backup.spec.ts Outdated Show resolved Hide resolved
src/client.ts Outdated Show resolved Hide resolved
src/client.ts Outdated Show resolved Hide resolved
src/client.ts Show resolved Hide resolved
spec/integ/crypto/megolm-backup.spec.ts Outdated Show resolved Hide resolved
src/client.ts Outdated Show resolved Hide resolved
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - one typo. Thanks!

spec/integ/crypto/megolm-backup.spec.ts Outdated Show resolved Hide resolved
@BillCarsonFr BillCarsonFr added this pull request to the merge queue Jan 19, 2024
Merged via the queue into develop with commit 4cddc73 Jan 19, 2024
23 checks passed
@BillCarsonFr BillCarsonFr deleted the valere/report_progress_on_fullbakup_import branch January 19, 2024 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants