Paths

Table of Contentst

-
lib/
-
reducers/
-
dm-operations-queue-reducer.js
-
shared/dm-ops/
-
dm-ops/
-
dm-ops-queue-handler.react.js
-
types/
-
dm-ops.js
-
redux-types.js
-
utils/
-
reducers-utils.test.js
-
native/redux/
-
redux/
-
default-state.js
-
persist.js
-
web/redux/
-
redux/
-
default-state.js
-
persist-constants.js
-
persist.js

[lib] Create unshimming queue
ClosedPublic
Actions

Authored by tomek on Mon, Mar 17, 7:10 AM.

Details

Reviewers

kamil
bartek

Commits

rCOMMfe3542bad7fc: [lib] Create unshimming queue

Summary

The unshimming process happens during a migration when it isn't convenient to call e.g. processDMOperations. To make this easier, we're saving the operations in Redux. We're processing them using a queue so that we don't have issues with e.g. Redux changes being skipped. We're removing the operations from SQLite only after they are processed.

https://linear.app/comm/issue/ENG-10329/create-a-new-action-and-modify-the-reducedmoperationsqueue-reducer

Depends on D14456

Test Plan

Tested the whole stack on both native and web:

Run the app on Redux version 86 with a modification to processDMOperation making all the INBOUND messages unsupported
On one platform created a thread and sent a text message. These were visible on the sender, and invisible on the recipient
Closed the recipient and added a migration that unshimms the thread creation operations, e.g.

unshimDMOperations(
  state,
  dmOperationTypes.CREATE_THREAD,
  handleReduxMigrationFailure,
): MigrationFunction<WebNavInfo, AppState>),

Run the recipient and noticed the thread appearing
Added another migration, this time unshimming text messages
Run the recipient and noticed messages appearing

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Mon, Mar 17, 7:10 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMon, Mar 17, 7:10 AM

tomek added a child revision: D14458: [lib] Create a migration function.Mon, Mar 17, 7:13 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Mon, Mar 17, 7:47 AM

Harbormaster failed remote builds in B33733: Diff 47422!

Rebase

Harbormaster completed remote builds in B33759: Diff 47449.Tue, Mar 18, 4:06 AM

tomek requested review of this revision.Tue, Mar 18, 4:06 AM

Good idea not unshimming in a migration

lib/shared/dm-ops/dm-ops-queue-handler.react.js
284 ↗	(On Diff #47449)	This is never set false to make sure we attempt unshimming only once after app start? If so, might be good to add code coment
285–302 ↗	(On Diff #47449)	I would use `flatMap()` but this is fine too
287–301 ↗	(On Diff #47449)	My memory about queue model we have is rusty, but I assume it is safe to perform operation and separately dispatch action? (I am talking about case when e.g. operation could not be processed)

This revision is now accepted and ready to land.Tue, Mar 18, 9:05 AM

Enqueue operations directly

lib/shared/dm-ops/dm-ops-queue-handler.react.js
284 ↗	(On Diff #47449)	This is never set false to make sure we attempt unshimming only once after app start? Yes, exactly
285–302 ↗	(On Diff #47449)	I think we should replace this code a bit to follow patterns from this file.
287–301 ↗	(On Diff #47449)	Yes, this is the same pattern we're using in other places. Both operation and removing it could fail, but there isn't too much we can do in such cases.