[web/native] Migrate keyserverStore to new tables
ClosedPublic
Actions

Authored by michal on Mar 29 2024, 1:48 PM.

Details

Reviewers

kamil
marcin

Commits

rCOMM6c3a54498b2c: [web/native] Migrate keyserverStore to new tables

Summary

Migrates the current data in one keyserver table to two. It works a bit unusually but the general gist is:

Copy the data from keyservers to keyserver_synced_data
Fetch the data in the migrations (db ops will merge the data from keyserver and keyserver_synced_data and so return the correct KeyserverInfo)
Save the data in the migration (db ops will now split the data correctly into two tables)

Also adds the non-synced keyservers table to backup blocklist.

Depends on https://phab.comm.dev/D11481

Test Plan

Make sure the migration runs correctly on both web and native. Compare the results from before the migration and after -> the keyserver infos should be the same (but urlPrefix is in a different table).

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Mar 29 2024, 1:48 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 29 2024, 1:48 PM

Harbormaster completed remote builds in B27830: Diff 38557.Mar 29 2024, 2:02 PM

michal requested review of this revision.Mar 29 2024, 2:02 PM

michal added a parent revision: D11481: [sqlite/lib] Split keyserver ops for backed and non-backed up data.Apr 2 2024, 3:21 AM

marcin accepted this revision.Apr 2 2024, 3:42 AM

This revision is now accepted and ready to land.Apr 2 2024, 3:42 AM

kamil accepted this revision.Apr 2 2024, 5:49 AM

kamil added inline comments.

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
602–604 ↗	(On Diff #38557)	make indentation consistent with the rest of the file
612 ↗	(On Diff #38557)	I bit confusing in this diff you modifying existing migration - make sure to test it properly
native/redux/persist.js
1170–1190 ↗	(On Diff #38557)	we could reduce code duplication and move this to function but here we can at least make sure that no one will modify this code in the future - with function in a different place is less maintainable

Improve indentation

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
612 ↗	(On Diff #38557)	These diffs will be landed together, it's just that the data migration required changes on JS side, which required changes to db schema. So to make the review easier and not put everything in one diff I decided to split it like this.
native/redux/persist.js
1170–1190 ↗	(On Diff #38557)	We are only repeating the code once, it's very simple, we don't expect that it will be modified in the future (as it's a migration) and it probably won't be reused in the future (as it's a no-op in other context) so I think it's fine to reuse code here.