Page MenuHomePhabricator

[lib] Support encrypted media types in updateMultimediaMessageMediaAction
ClosedPublic

Authored by bartek on Mar 29 2023, 6:59 AM.
Tags
None
Referenced Files
F3516653: D7235.id24535.diff
Sun, Dec 22, 3:05 PM
F3516381: D7235.id24332.diff
Sun, Dec 22, 2:10 PM
F3515711: D7235.diff
Sun, Dec 22, 9:42 AM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:42 PM
Subscribers

Details

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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
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

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

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

Same as above

This revision is now accepted and ready to land.Mar 29 2023, 1:03 PM
lib/reducers/message-reducer.js
1237–1241

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