This implements the web/native migration for the integrityStore table to sqlite
Details
Details
- Reviewers
kamil tomek atul - Commits
- rCOMMd93872ce1f66: [native] migrate integrityStore to SQLite
integrity table on dev environment didn't have prior integrity store thread hashes, so I tested by migrating raw thread hash data.
console.log('running migration 64');
const threadHashes1: ThreadHashes = {
thread1: 123,
thread2: 456,
thread3: 789,
};
const threadHashes2: ThreadHashes = {
thread1: 893,
thread2: 443,
thread3: 913,
};
const threadHashes3: ThreadHashes = {
thread4: 593,
};
const replaceOp1: ReplaceIntegrityThreadHashesOperation = {
type: 'replace_integrity_thread_hashes',
payload: {
threadHashes: threadHashes1,
},
};
const clearOp: RemoveAllIntegrityThreadHashesOperation = {
type: 'remove_all_integrity_thread_hashes',
};
const replaceOp2: ReplaceIntegrityThreadHashesOperation = {
type: 'replace_integrity_thread_hashes',
payload: {
threadHashes: threadHashes2,
},
};
const replaceOp3: ReplaceIntegrityThreadHashesOperation = {
type: 'replace_integrity_thread_hashes',
payload: {
threadHashes: threadHashes3,
},
};
const dbOperations: $ReadOnlyArray<ClientDBIntegrityStoreOperation> =
integrityStoreOpsHandlers.convertOpsToClientDBOps([
clearOp,
replaceOp1,
clearOp,
replaceOp2,
replaceOp3,
]);
try {
await commCoreModule.processIntegrityStoreOperations(dbOperations);
console.log('migration 64 succeeded');
} catch (exception) {
console.log('migration 64 failed');
if (isTaskCancelledError(exception)) {
return state;
}
return handleReduxMigrationFailure(state);
}
return state;
},Successfully wrote test thread hashes to the sqlite database:
Tested web with these steps:
- Log out, clear the app data, log in
- Run the app from before the store is introduced to the DB. Close the app, serve a new version, and open the app.
- Close the app while hashes are being computed (in the old version). Close the app, serve a new version, and open the app.
Used Kamil's diff for downloading database on web to check database contents are correct and reflect latest thread changes
Depends on D11309
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/redux/persist.js | ||
|---|---|---|
| 280 | Have you checked if this store isn't empty? It seems like persistWhitelist doesn't contain integrityStore which means that state.integrityStore.threadHashes should be always empty. | |
| web/redux/persist.js | ||
|---|---|---|
| 280 | Realizing now that the user store sqlite migration only migrated native (https://phab.comm.dev/D9912). I mistakenly went off the keyserver migration diffs for this diff. I think we can remove the web migration in this case then. Or should I be adding integrityStore to the persistWhiteList? | |
| web/redux/persist.js | ||
|---|---|---|
| 280 |
We shouldn't do that because we don't want these to be persisted using redux-persist. In this case, I think that we can remove this migration, but that needs to be tested thoroughly. | |
| web/redux/persist.js | ||
|---|---|---|
| 280 | Could I get some clarification on what you mean by testing thoroughly? | |
| web/redux/persist.js | ||
|---|---|---|
| 280 | I think we should test a couple of scenarios:
In all of these, we should check if the content of the DB is correct. | |
Comment Actions
At some point we should also blacklist integrityStore on native so that it isn't persisted using redux-persist
Comment Actions
Was this ever done? It looks like this stack was landed and integrityStore is not in persistBlacklist on native...
Comment Actions
I understood this to be a later follow-up for after we verify sqlite is working for a bit after landing. I should have made a linear issue for this and will do that right now.
Currently, the blacklist includes
'loadingStatuses', 'lifecycleState', 'dimensions', 'draftStore', 'connectivity', 'deviceOrientation', 'frozen', 'threadstore', 'storeloaded',
It seems like we haven't blacklisted other migrations, but I can't find a linear issue for them. Will create a follow up parent task for now
Comment Actions
Spoke to @will about this in person and he clarified that we're purposing double-storing this data right for validation. @will will post a Linear issue in a sec that will track:
- Removing the data from redux-persist
- Adding the field to the blacklist for redux-persist on native (and making sure it's not on the whitelist on web)
- Updating the reducer on setClientDBStoreActionType so it actually puts that data into the store
- Any other follow-ups necessary
We'll deploy the current state to production for a bit, and let devs monitor to see if the Alert hits. If it doesn't, we'll move forward with the forthcoming task.
Comment Actions
Here's the linear issue tracking putting integrityStore on redux persist black list and switching to using sqlite integrityStore:
https://linear.app/comm/issue/ENG-7512/after-verifying-integritystore-sqlite-storing-properly-remove