[native] Reintroduce `createUpdateDBOpsForThreadStoreThreadInfos` for new `RawThreadInfo` encoding
ClosedPublic
Actions

Authored by atul on Feb 15 2024, 12:21 PM.

Details

Reviewers

ginsu
tomek
kamil
michal

Commits

rCOMM1cfcd7f83568: [native] Reintroduce `createUpdateDBOpsForThreadStoreThreadInfos` for new…

Summary

Discussed in D11057, but we want to "freeze" the previous implementation of createUpdateDBOpsForThreadStoreThreadInfos in place to support old migrations while re-introducing a new version that only handles MinimallyEncoded -> MinimallyEncoded migrations. We will only have MinimallyEncoded -> MinimallyEncoded migrations in the future, so IMO it doesn't make sense to clutter up createUpdateDBOpsForThreadStoreThreadInfos to be able to handle

LegacyRawThreadInfos => LegacyRawThreadInfos
LegacyRawThreadInfos => MinimallyEncodedRawThreadInfos
MinimallyEncodedRawThreadInfos => MinimallyEncodedRawThreadInfos

It also lets us avoid having to go back and test some of the old migrations when we tweak createUpdateDBOpsForThreadStoreThreadInfos.

Depends on D11057

Test Plan

This gets consumed for the first time in the upcoming specialRoles migration where it will be tested implicitly.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Feb 15 2024, 12:21 PM

Herald added a subscriber: ashoat. · View Herald TranscriptFeb 15 2024, 12:21 PM

atul edited the summary of this revision. (Show Details)Feb 15 2024, 12:22 PM

atul published this revision for review.Feb 15 2024, 12:26 PM

atul added inline comments.

native/redux/client-db-utils.js
34–36 ↗	(On Diff #37273)	Similar to `deprecated*` version of the function: except we call `convertClientDBThreadInfoToRawThreadInfo` instead of `deprecatedConvertClientDBThreadInfoToRawThreadInfo` which means we will get back "MinimallyEncoded" `RawThreadInfos`... which are what we want to run the migration on and repopulate SQLite with.
38–39 ↗	(On Diff #37273)	I know we generally prefer the ES6 map/filter/reduce/etc functions, but in this case using `lodash/keyBy` was far more concise than what we had before:
41–50 ↗	(On Diff #37273)	Nothing has changed with these except some naming.
52–59 ↗	(On Diff #37273)	All that's changed here is that I pulled out construction of `replaceThreadOperations` vs what we had before:

atul attached a referenced file: F1163839: f25f0b.png. (Show Details)Feb 15 2024, 12:26 PM

atul attached a referenced file: F1163834: 708e5c.png. (Show Details)

atul attached a referenced file: F1163828: e3633a.png. (Show Details)

Harbormaster completed remote builds in B26932: Diff 37273.Feb 15 2024, 12:37 PM

atul added a child revision: D11097: [native] Reintroduce `updateClientDBThreadStoreThreadInfos`.Feb 15 2024, 1:00 PM

atul mentioned this in D11099: [native] Redux migration to patch in `specialRole` field.Feb 15 2024, 1:50 PM

atul added a reviewer: kamil.Feb 15 2024, 1:50 PM

atul added a reviewer: michal.

tomek accepted this revision.Feb 16 2024, 2:24 AM

tomek added inline comments.

native/redux/client-db-utils.js
38–39 ↗	(On Diff #37273)	Not sure if other devs agree, but for me it is more convenient to have a code block in a comment instead of a screenshot. The screenshot isn't readable unless you open it, which breaks the review flow and makes it a lot harder to compare the versions.