[web-db] start persisting SQLite content
ClosedPublic
Actions

Authored by kamil on Mar 27 2023, 4:55 AM.

Details

Reviewers

michal
tomek

Commits

rCOMMc38e70c6d6ee: [web-db] start persisting SQLite content

Summary

Saving database file in indexed db

Depends on D7185

Test Plan

Check in devtools if indexedDB key exists
Reopen app and save something into db - database should be read from the indexed db, not freshly created

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Mar 27 2023, 4:55 AM

kamil held this revision as a draft.

Herald added subscribers: atul, ashoat. · View Herald TranscriptMar 27 2023, 4:55 AM

kamil edited the summary of this revision. (Show Details)Mar 27 2023, 4:57 AM

kamil edited the test plan for this revision. (Show Details)

kamil added a parent revision: D7185: [web-db] process redux-persist operations in SQLite.

kamil added inline comments.Mar 27 2023, 5:10 AM

web/database/worker/db-worker.js
218 ↗	(On Diff #24167)	this is the line I am not sure how to handle: we can only start persisting and resolve the promise (promise with app request) to let the app proceed and trust that worker will persist data (current one) await this here to make sure it's persisted but this might slow down the main thread in the app

kamil published this revision for review.Mar 27 2023, 5:10 AM

Harbormaster completed remote builds in B17660: Diff 24167.Mar 27 2023, 5:12 AM

kamil added a child revision: D7190: [web-db] implement custom `redux-persist` storage.Mar 27 2023, 6:27 AM

kamil edited parent revisions, added: D7248: [web-db] clean-up processing app requests on worker; removed: D7185: [web-db] process redux-persist operations in SQLite.Mar 30 2023, 2:24 AM

tomek accepted this revision.Mar 30 2023, 4:48 AM

tomek added inline comments.

web/database/worker/db-worker.js
141–144 ↗	(On Diff #24167)	We don't have to `export` the data when the key is missing
218 ↗	(On Diff #24167)	What is the downside of using the same approach for all the operations?

This revision is now accepted and ready to land.Mar 30 2023, 4:48 AM

kamil added inline comments.Mar 30 2023, 6:21 AM

web/database/worker/db-worker.js
218 ↗	(On Diff #24167)	I meant a general difference between using `persist();` vs `await persist()` and my concerns are applying to all operations

add debounce

Adding debounce to persisting,
I set up the database for drafts and persist was not called on each character changed but was debounced as expected.

web/database/worker/db-worker.js
141–144 ↗	(On Diff #24167)	in that order flow is complaining: `Cannot call encryptDatabaseFile with encryptionKey bound to key because null or undefined [1] is incompatible with CryptoKey`

tomek added inline comments.Mar 31 2023, 2:47 AM

web/database/utils/constants.js
8 ↗	(On Diff #24468)	We can probably name it a little different so that it is obvious that it is a duration of debounce.

Harbormaster completed remote builds in B17864: Diff 24468.Mar 31 2023, 2:52 AM

use _throttle instead of _debounce

kamil removed a parent revision: D7185: [web-db] process redux-persist operations in SQLite.Apr 3 2023, 2:12 AM

Harbormaster completed remote builds in B17919: Diff 24549.Apr 3 2023, 2:24 AM

Throttling is a great improvement compared to just calling this function but it still might break when db save takes more than 300ms. We should probably handle that case differently.

web/database/worker/db-worker.js
203–224 ↗	(On Diff #24549)	Maybe we can improve this code a bit by extracting common part after `if`? It seems like every branch results in `persist` and `return`.