Skip to content

[_]: refactor/database service#38

Merged
xabg2 merged 4 commits intomasterfrom
refactor/database-service
Apr 14, 2026
Merged

[_]: refactor/database service#38
xabg2 merged 4 commits intomasterfrom
refactor/database-service

Conversation

@xabg2
Copy link
Copy Markdown
Contributor

@xabg2 xabg2 commented Apr 14, 2026

Decoupling the Email Store from the Database Service.

@xabg2 xabg2 requested a review from larryrider April 14, 2026 10:39
@xabg2 xabg2 self-assigned this Apr 14, 2026
@xabg2 xabg2 added the enhancement New feature or request label Apr 14, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Warning

Rate limit exceeded

@xabg2 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 24 minutes and 58 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 24 minutes and 58 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9341c660-cfff-44b5-bff6-a02109723d2e

📥 Commits

Reviewing files that changed from the base of the PR and between efd5141 and cc5666e.

📒 Files selected for processing (3)
  • src/services/database/config/index.ts
  • src/services/database/database.service.test.ts
  • src/services/database/types/index.ts
📝 Walkthrough

Walkthrough

The database service is refactored from email-specific to generic, store-parameterized operations. Configuration moves to a new structure, custom error classes are added, and the DatabaseService implements a new Database interface with generalized CRUD methods accepting explicit store and index names instead of fixed configurations.

Changes

Cohort / File(s) Summary
Error Classes
src/errors/database/index.ts
Added three custom error subclasses: MnemonicNotFoundError, ProviderNotInitializedError, and EmailNotFoundError (with emailId parameter). Each sets proper prototype chains via Object.setPrototypeOf.
Configuration Structure
src/services/database/config/index.ts, src/services/database/config/utils.ts
Introduced new configuration module with MAIL_DB_CONFIG (multiple stores and indexes) and utility function generateEmailParamPath<K>() for typed index key path generation.
Configuration Migration
src/services/database/config.ts (deleted)
Removed legacy email-specific configuration file containing INDEXED_DB_VERSION, STORED_EMAILS_DB_LABEL, and EMAIL_DB_CONFIG.
Type Definitions
src/services/database/types/index.ts
Removed Email, StoredEmail, EmailFilters types. Added StoreConfig, Database interface, and EMAIL_DB_INDEXES_KEYS constant. Changed EmailParams.folderId from optional to required. Restructured DatabaseConfig to use stores: StoreConfig[] instead of single store.
Service Implementation
src/services/database/index.ts
Refactored DatabaseService to implement Database interface with generic, store-parameterized methods replacing email-specific operations. Constructor now accepts dbName and multi-store config. Methods like get, put, getByIndex, getByRange, deleteOldest now accept explicit store and indexName parameters.
Test Suite
src/services/database/database.service.test.ts
Updated tests to use new generic API with store-scoped method signatures. Replaced email-specific test helpers with TestRecord structure. Added "Get by index" test suite. Changed lifecycle from destroy() to close() and added store-unique database instantiation per test.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • larryrider
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and generic, using non-descriptive terms like 'refactor/database service' without conveying what specifically changed or why. Revise the title to be more specific, e.g., 'Decouple email store from generic database service' or 'Make DatabaseService generic and store-parameterized'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description 'Decoupling the Email Store from the Database Service' is directly related to the changeset and conveys the main intent.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/database-service

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.

Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.

👉 Steps to fix this

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/services/database/index.ts (1)

91-115: ⚠️ Potential issue | 🟠 Major

getBatch() cannot page safely on generic indexes.

Line 101 uses a truthiness check that fails for valid cursor positions like 0 and ''; use an explicit startCursor !== undefined check instead. More critically, storing only cursor.key as the pagination token (line 109) is unreliable when an index has duplicate values—the next call cannot distinguish which record to resume from. Use cursor.continuePrimaryKey(key, primaryKey) with both the index key and primary key, or document this API as unique-index-only.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/database/index.ts` around lines 91 - 115, The getBatch
implementation uses a truthy check for startCursor and stores only cursor.key
which breaks for falsy keys (0, '') and duplicate index values; change the
startCursor check in getBatch to explicit startCursor !== undefined, and switch
pagination to a composite cursor: when iterating capture both cursor.key and
cursor.primaryKey (use those as the pagination token nextCursor) and on
subsequent calls resume using index.openCursor(...) with index.openCursor(null,
'prev') only when no startCursor, otherwise call
cursor.continuePrimaryKey(startKey, startPrimaryKey) (or accept a composite
startCursor type) so pagination is robust for non-unique indexes; update
getBatch's return/param types to reflect the composite nextCursor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/services/database/config/index.ts`:
- Around line 8-11: The indexes defined under EMAIL_DB_INDEXES_KEYS use
generateEmailParamPath('isRead'/'from'/'to') which resolve to non-IDBValidKey
types (boolean and User[]), so they won't populate; update the email record
shape to persist derived fields (e.g., add a numeric isReadFlag: 0|1 and string
arrays fromEmails/toEmails containing only email addresses) and change the index
keyPaths to generateEmailParamPath('isReadFlag'),
generateEmailParamPath('fromEmails'), and generateEmailParamPath('toEmails');
ensure the code that writes emails (the functions that set params.isRead,
params.from, params.to) maps those into the new derived fields before storing so
the indexes can use IDBValidKey types.

In `@src/services/database/database.service.test.ts`:
- Around line 13-21: The createRecord function merges default params but then
spreads overrides after, so passing overrides.params replaces the whole params
(dropping receivedAt); fix by extracting params from overrides first (e.g.,
const { params: paramsOverrides, ...rest } = overrides) and then build the
record with params: { receivedAt: ..., isRead: false, ...paramsOverrides } and
finally spread rest so non-params overrides apply without clobbering params;
update the createRecord helper (TestRecord factory) accordingly.

In `@src/services/database/types/index.ts`:
- Around line 54-70: The Database interface is missing the get method and
incorrectly types getBatch.startCursor as string|string[]; add a get signature
to match DatabaseService (e.g., get: <T>(storeName: string, key: IDBValidKey) =>
Promise<T | undefined>) and change the getBatch startCursor type from string |
string[] to IDBValidKey (or IDBValidKey | undefined) so the public contract uses
IDBValidKey like the concrete implementation; update the Database interface
entries for get and getBatch accordingly, referencing the existing getBatch and
other methods in src/services/database/types/index.ts.

---

Outside diff comments:
In `@src/services/database/index.ts`:
- Around line 91-115: The getBatch implementation uses a truthy check for
startCursor and stores only cursor.key which breaks for falsy keys (0, '') and
duplicate index values; change the startCursor check in getBatch to explicit
startCursor !== undefined, and switch pagination to a composite cursor: when
iterating capture both cursor.key and cursor.primaryKey (use those as the
pagination token nextCursor) and on subsequent calls resume using
index.openCursor(...) with index.openCursor(null, 'prev') only when no
startCursor, otherwise call cursor.continuePrimaryKey(startKey, startPrimaryKey)
(or accept a composite startCursor type) so pagination is robust for non-unique
indexes; update getBatch's return/param types to reflect the composite
nextCursor.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7c79500a-115a-4d78-b7fd-3c4a34d8952c

📥 Commits

Reviewing files that changed from the base of the PR and between 2a2e50a and efd5141.

📒 Files selected for processing (7)
  • src/errors/database/index.ts
  • src/services/database/config.ts
  • src/services/database/config/index.ts
  • src/services/database/config/utils.ts
  • src/services/database/database.service.test.ts
  • src/services/database/index.ts
  • src/services/database/types/index.ts
💤 Files with no reviewable changes (1)
  • src/services/database/config.ts

Comment thread src/services/database/config/index.ts Outdated
Comment thread src/services/database/database.service.test.ts
Comment thread src/services/database/types/index.ts
@sonarqubecloud
Copy link
Copy Markdown

@xabg2 xabg2 merged commit 8976e37 into master Apr 14, 2026
5 checks passed
@xabg2 xabg2 deleted the refactor/database-service branch April 14, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants