Page MenuHomePhabricator

[client-backup] implement API call to upload backup
ClosedPublic

Authored by kamil on Aug 29 2023, 2:13 AM.
Tags
None
Referenced Files
F3184503: D8993.id30672.diff
Fri, Nov 8, 10:57 AM
F3184445: D8993.id30471.diff
Fri, Nov 8, 10:50 AM
Unknown Object (File)
Sun, Oct 27, 4:09 PM
Unknown Object (File)
Wed, Oct 23, 3:37 AM
Unknown Object (File)
Mon, Oct 21, 4:37 AM
Unknown Object (File)
Sun, Oct 20, 4:12 AM
Unknown Object (File)
Thu, Oct 17, 10:51 PM
Unknown Object (File)
Thu, Oct 17, 10:32 PM

Details

Summary

Created multipart form data and uploads it with authentication.

Depends on D8992

Test Plan

Call this with mock data and check if works, call it with wrong data/auth to make sure it will be rejected.

Diff Detail

Repository
rCOMM Comm
Branch
client-backup
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
native/backup/api.js
16–17

This will slow down this a bit and bloat size, but due to react-native limitations, we have to implement this this way, because react-native will ignore any data that is not a real media file that can be read from disk src.

We can not use methods implemented in D8888 because there is no guarantee that the buffer is a valid UTF-8 string.

25

unused for now

kamil published this revision for review.Aug 29 2023, 3:15 AM
bartek added inline comments.
native/backup/api.js
25

Looks like this field is optional on server side, so we don't need it here but let it stay for clarity

This revision is now accepted and ready to land.Aug 29 2023, 5:39 AM
native/backup/api.js
16–17

This will slow down this a bit and bloat size, but due to react-native limitations, we have to implement this this way, because react-native will ignore any data that is not a real media file that can be read from disk src.

Then why can't we store those data into temporary files and pass paths to fetch API?

If this sounds like too much of a hassle then it might be worth to add a dedicated thread to CommCoreModule to perform this operation.

Mobile app already has poor performance so we shouldn't be optimistic that heavy computation we introduce to JS thread will not make it worse

native/backup/api.js
16–17

this is temporary in the initial version, in future we will have to upload data from native code anyway (see ENG-4678).

in C++ or Rust, we will be able to append bytes to multipart, this is only a JS fetch API limitation.

bartek added inline comments.
native/backup/api.js
16–17

Then why can't we store those data into temporary files and pass paths to fetch API?

The problem is fetch API in React Native doesn't understand any blobs/arraybuffers, and file paths either (besides that ph:// photo library assets). Seriously, fetch API is terrible for anything that is not a image or JSON

If we want to use temporary files, we need to use a 3rd party lib like rn-upload-blob (that @patryk is going to replace with expo-file-system)

At some point we'll probably want to think about how uploads might work in the background. If the backup is large, I could imagine that the client would need background upload in order to complete the upload. Is there a task to track this?

At some point we'll probably want to think about how uploads might work in the background. If the backup is large, I could imagine that the client would need background upload in order to complete the upload. Is there a task to track this?

I was thinking about moving this implementation to native code to have access to DB logs and performing upload on a separate thread : ENG-4678.