Page MenuHomePhabricator

Send Darwin Notification from NSE to AppDelegate so that it consumes flat file storage immediately and sends event to JS
ClosedPublic

Authored by marcin on Apr 3 2023, 3:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, May 19, 7:05 AM
Unknown Object (File)
Apr 9 2024, 2:42 AM
Unknown Object (File)
Apr 9 2024, 2:42 AM
Unknown Object (File)
Apr 9 2024, 2:42 AM
Unknown Object (File)
Apr 3 2024, 5:53 PM
Unknown Object (File)
Mar 5 2024, 9:13 PM
Unknown Object (File)
Mar 5 2024, 9:13 PM
Unknown Object (File)
Mar 5 2024, 9:12 PM
Subscribers

Details

Summary

This differential makes NSE send Darwin Notification to AppDelegate when it receives the notif. AppDelegate reacts by consuming the flat file and sending event to JS to persist message infos.

Test Plan

Build the app. Background it. Send notification. Kill the keyserver (debug)/turn off network connection (release). Foreground the app. App is dosconnected from the keyserver but the message is visible in the app.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin requested review of this revision.Apr 3 2023, 4:15 AM
tomek requested changes to this revision.Apr 7 2023, 2:32 AM
tomek added inline comments.
native/ios/Comm/AppDelegate.mm
320–327 ↗(On Diff #24566)

Is it possible that a message will be saved twice to the db? Once in this function, and once when the JS receives it?

This revision now requires changes to proceed.Apr 7 2023, 2:32 AM
native/ios/Comm/AppDelegate.mm
320–327 ↗(On Diff #24566)

I think it's fine for this to happen... it already happens in several cases in our codebase:

  1. If you send a message, then disconnect the socket and reconnect it, the socket will download the message again
  2. If you press a notif, the message will be saved immediately, and then it will be downloaded again when the socket connects

Of course if it's easy to avoid, it doesn't hurt to avoid. In this case, we could potentially pass some sort of flag to the SAVE_MESSAGE_INFOS Redux action that tells it to update Redux without committing the results to SQLite. I'm not sure how much value that has or how complicated it might be. Defer to you and @marcin to discuss tradeoffs.

marcin added inline comments.
native/ios/Comm/AppDelegate.mm
320–327 ↗(On Diff #24566)

Adding a flag to SAVE_MESSAGE_INFOS action should be feasible but I also can't see a strong benefit of doing so. Double write is a common case in our codebase and even with this flag we wouldn't mitigate it entirely. Additionally we already landed immediate message infos persistence on Android where double write occurs as well and we didn't raise any concerns about this.
If it was up to me I would leave the double write as it is.

I'm not too familiar with this logic, but seems reasonable.

native/ios/Comm/AppDelegate.mm
320–327 ↗(On Diff #24566)

Ok, makes sense, let's keep it.

This revision is now accepted and ready to land.Apr 12 2023, 4:04 AM