Page MenuHomePhabricator

[native] Migrate `messageStore.messages` persistence from `redux-persist` to SQLite
ClosedPublic

Authored by atul on Jun 2 2022, 8:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 18, 8:41 AM
Unknown Object (File)
Mon, Dec 2, 2:51 PM
Unknown Object (File)
Nov 9 2024, 6:39 PM
Unknown Object (File)
Nov 9 2024, 6:19 PM
Unknown Object (File)
Nov 9 2024, 5:43 PM
Unknown Object (File)
Nov 9 2024, 3:34 PM
Unknown Object (File)
Nov 9 2024, 3:34 PM
Unknown Object (File)
Nov 9 2024, 2:55 PM

Details

Summary

This is the diff that "flips the switch": https://linear.app/comm/issue/ENG-1189/flip-from-persisting-messagestore-in-redux-to-sqlite

Bascially we're enabling the "transform" that excludes messageStore.messages from being included in what's persisted by redux-persist... and we're "rehydrating" messageStore.messages in SQLiteContextProvider as we do with the threadStore.

We also bump the persistConfig.version to 31 to ensure that the messages that are currently in messageStore.messages get persisted in the SQLite store.

Test Plan

I tried to test each "part" of this independently... and then also "went through" the migration a few times with my physical device and prod store to make sure that things worked as expected.

To test the "transform," I included the transform in the persistConfig and set breakpoints within the inbound tranformation function to ensure that the inputs and outputs were as expected.

To test the migration, I set breakpoints in the native C++ processMessageStoreOperationsSync(...) function and observed that the messages that were previously in messageStore.messages were persisted as expected.

Finally, I tested this change "as a whole" multiple times on the iOS Simulator and my physical iPhone.

  1. Load build 134 on the device by checking out the codeVersion bump commit (or via TestFlight on my physical device)
  2. Log into the app and ensure that everything loads and looks as expected
  3. Create a Release build with this change included
  4. See that the new version of the app installs without issue, I continue to be logged in, and that the redux migration completed as expected.
  5. Use the app for a bit and make sure that things continue to work as expected.6. Close the app, re-launch the app, etc a few times to ensure that things are working as expected.

Nonetheless, there are a lot of moving parts involved in this change so it is "on the riskier side" IMO... I'll make a staff release so we can all "test" the migration and see if anything comes up.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Jun 2 2022, 8:39 AM
tomek added inline comments.
native/data/sqlite-context-provider.js
48 ↗(On Diff #13296)

Why do we await a sync version? We should either await an async version getAllMessages() or not await here and keep getAllMessagesSync.

This revision is now accepted and ready to land.Jun 3 2022, 2:12 AM
native/data/sqlite-context-provider.js
48 ↗(On Diff #13296)

Yikes, thanks a ton for catching that.. should definitely be getAllMessages()

Going to go through all the other diffs I have out to give them another look…

address feedback before landing

native/redux/persist.js
378

This eslint-disable could've been deleted