Refactor DB management #276
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis PR consolidates IndexedDB lifecycle management into a centralized database manager module (src/db/manager.ts) with reference counting and caching, introduces a WalletState type for wallet persistence, removes deprecated ContractRepository collection methods, updates repository exports, and modifies the build process to run Changes
Sequence DiagramsequenceDiagram
participant App as Application Code
participant Repo as Repository (IndexedDB)
participant DBMgr as DB Manager
participant IDB as IndexedDB
App->>Repo: getContracts()
activate Repo
Repo->>Repo: getDB()
activate Repo
Repo->>DBMgr: openDatabase(dbName, version, initDatabase)
activate DBMgr
DBMgr->>DBMgr: Check dbCache, refCounts
DBMgr->>IDB: open(dbName, version)
activate IDB
IDB->>DBMgr: onupgradeneeded
DBMgr->>DBMgr: Call initDatabase(db)
deactivate IDB
DBMgr->>Repo: Return Promise<IDBDatabase>
deactivate DBMgr
Repo->>Repo: Query contracts from store
deactivate Repo
Repo->>App: Return contracts[]
deactivate Repo
App->>Repo: closeDatabase()
activate Repo
Repo->>DBMgr: closeDatabase(dbName)
activate DBMgr
DBMgr->>DBMgr: Decrement refCount
alt refCount === 0
DBMgr->>IDB: close()
DBMgr->>DBMgr: Remove from dbCache
end
DBMgr->>Repo: Return boolean
deactivate DBMgr
deactivate Repo
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
pietro909
requested review from
Kukks,
Copilot and
louisinger
and removed request for
Copilot
|
@CodeRabbit please review |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/db/manager.ts`:
- Around line 46-76: The cache currently keys by dbName only, so reuse via
dbCache.get(dbName) can return a connection opened with a different dbVersion;
update the logic in the manager around dbCache/refCounts/dbPromise to include
version validation (either incorporate dbVersion into the cache key, e.g.,
`${dbName}@${dbVersion}`, or store the opened version alongside the cached
Promise and compare on retrieval), and if a mismatch is detected reject/throw or
open the correct version instead; ensure the same validation is applied wherever
dbCache.get/dbCache.set and refCounts are used and that initDatabase is invoked
when the requested version requires upgrade.
🧹 Nitpick comments (1)
package.json (1)
82-83: Confirm Husky hooks and publish build behavior remain intentional.Line 82 replaces
huskywithpnpm run build, so Husky hooks won’t auto‑install unless you handle it elsewhere, andnpm publishmay run the build twice (prepare + prepublishOnly). Please confirm that’s intended; if hooks are still required, consider runninghuskyalongside build or moving build toprepackwhile restoring Husky install.
| "Only Alice's data is present in IndexedDB `contracts` table, check the .sqlite file to verify this." | ||
| ); | ||
| await aliceWallet.walletRepository.saveWalletState(state); | ||
| await bobWallet.walletRepository.saveWalletState(state); |
There was a problem hiding this comment.
NOTE:
The example was changed to persist something: the point is to show that Alice wallet's data is in IndexedDB whereas Bob's isn't.
Using the ContractRepository would have made this example become the same as ../contract-manager.ts.
| export const DB_VERSION = 2; | ||
|
|
||
| export function initDatabase(db: IDBDatabase): IDBDatabase { | ||
| export function initDatabase(db: IDBDatabase): void { |
There was a problem hiding this comment.
The returned type was unused and the function mutates directly the argument: returning a IDBDatabase may induce the consumer to believe that it gets an updated version of the DB.
Returning void makes it crystal clear that this function performs side-effects.
pietro909
force-pushed
the
repository-v2-fixes-for-boltz-swap
branch
from
e44ca44 to
a9be62d
Compare
Introducing repositories v2 to boltz-swap requires some boilerplate related to IndexedDB management.
Rather than duplicate the same logic, I've made the
src/dbutils first-class citizens and exposed them.Any class following our repository pattern will then implement its own DB initialization by passing the name, version, and a migration function:
Things like reusing the connection, closing the DB once all consumers are done, and triggering migrations when
onupgradeneededevent is fired (and queuing the migrations) is handled in one place.Summary by CodeRabbit
New Features
Chores