[lib] Support encrypted media types in updateMultimediaMessageMediaAction
ClosedPublic
Actions

Authored by bartek on Mar 29 2023, 6:59 AM.

Details

Reviewers

ashoat
atul
marcin

Commits

rCOMMc2ef783d9fe2: [lib] Support encrypted media types in updateMultimediaMessageMediaAction

Summary

This diff modifies the reducer by adding two cases to the updateMultimediaMessageMediaAction: replacing photo with encrypted_photo and video with encrypted_video. The core part is to get rid of uri and localMediaSelection and then insert holder and encryptionKey properties.

This is needed to dynamically replace local message with the encrypted message from server once upload succeeds

Depends on D7168

Test Plan

Used redux-devtools and ensured that properties are replaced as expected/

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Mar 29 2023, 6:59 AM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptMar 29 2023, 6:59 AM

bartek added a child revision: D7236: [native] Encrypt files during media mission.Mar 29 2023, 7:04 AM

Harbormaster completed remote builds in B17781: Diff 24332.Mar 29 2023, 7:14 AM

bartek published this revision for review.Mar 29 2023, 7:21 AM

Just a question:

This is needed to dynamically replace local message with the encrypted message from server once upload succeeds

So the pattern is that we have a photo locally at first, and then it gets replaced by encrypted_photo once the upload succeeds?

What happens if the app is killed and restarted in the middle of a photo upload? I assume that SQLite will have photo, but will the app know that it should be encrypting that?

lib/reducers/message-reducer.js
1237–1241 ↗	(On Diff #24332)	Personally I prefer avoiding Lodash in favor of vanilla JS when possible. Not sure if I'm missing something, but would this be possible? const { uri, thumbnailURI, localMediaSelection, ...original } = singleMedia;

Looks good, minor note about swapping out Lodash

lib/reducers/message-reducer.js
1230 ↗	(On Diff #24332)	Can we use spread/rest pattern to pull out what we want? We do have lodash (fp-flavored) throughout `message-reducer`, but I think we want to lean towards ES6 patterns if possible.
1237–1241 ↗	(On Diff #24332)	Same as above

This revision is now accepted and ready to land.Mar 29 2023, 1:03 PM

bartek added inline comments.Mar 30 2023, 5:53 AM

lib/reducers/message-reducer.js
1237–1241 ↗	(On Diff #24332)	Yes, the only problem is that Flow complains in that case when merging tuples. However a few invariants and explicitly recreating the update part solves the issue

So the pattern is that we have a photo locally at first, and then it gets replaced by encrypted_photo once the upload succeeds?

What happens if the app is killed and restarted in the middle of a photo upload? I assume that SQLite will have photo, but will the app know that it should be encrypting that?

Sorry, missed this question earlier. Basically, this replacement doesn't occur in the middle of a photo upload, but only after a successful upload. Until either processing, encryption and upload is in progress, we display the local unencrypted media and replace it (using this action) when everything's done.
If the app is restarted during the upload, these specific cases in this diff will have nothing to do with it. Existing retry logic should take care of that and restart media processing/upload from scratch.

Thanks for explaining!

Replace lodash with spread syntax

Harbormaster completed remote builds in B17870: Diff 24480.Mar 31 2023, 3:26 AM

Closed by commit rCOMMc2ef783d9fe2: [lib] Support encrypted media types in updateMultimediaMessageMediaAction (authored by bartek). · Explain WhyApr 2 2023, 11:47 PM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMMc2ef783d9fe2: [lib] Support encrypted media types in updateMultimediaMessageMediaAction.

bartek mentioned this in D7295: [lib][redux] Ability to update encrypted media messages.Apr 3 2023, 2:40 PM

bartek mentioned this in rCOMM160c41753ee1: [lib][redux] Ability to update encrypted media messages.Apr 6 2023, 6:13 AM