Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces repository statistics functionality with a new API endpoint ( Changes
Possibly related PRs
🚥 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. 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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
7b1e719 to
3345fe2
Compare
2de6e21 to
562e7d7
Compare
3345fe2 to
eb3ecd8
Compare
584762f to
29ee163
Compare
eb3ecd8 to
49abcc5
Compare
04c299c to
80ee9c9
Compare
refactor: use a smarter cache key logic for easier invalidations
80ee9c9 to
438ea1c
Compare
|
Documentation Updates 1 document(s) were updated by changes in this PR: Repositories in Zerobyte: Types, Architecture, and OperationsView Changes@@ -239,9 +239,14 @@
**Repository Statistics:**
- [Get repository statistics](https://github.com/nicotsx/zerobyte/blob/bad944a2329540088a51f7699d6f53d9ac82137c/app/server/modules/repositories/repositories.service.ts#L197-L218): Retrieve compression and storage statistics
- - Endpoint: `GET /api/v1/repositories/{shortId}/stats`
- - Uses shared repository locks to safely read stats while allowing concurrent operations
- - Results are cached using `cacheKeys.repository.stats(repositoryId)` to improve performance
+ - Endpoint: `GET /api/v1/repositories/{shortId}/stats` (production-ready as of PR #551)
+ - Uses shared repository locks (`repoMutex.acquireShared`) to safely read stats while allowing concurrent operations
+ - Results are cached using `cacheKeys.repository.stats(repository.id)` to improve performance
+ - Protected by the same authentication and authorization as other repository endpoints
+ - Cache is invalidated when:
+ - Repository configuration is changed
+ - Snapshots are deleted or modified (via `cacheKeys.repository.all()`)
+ - Repository is deleted
- Returns `ResticStatsDto` with the following fields:
- `total_size`: Total size of stored data in bytes
- `total_uncompressed_size`: Total uncompressed size in bytes
@@ -310,11 +315,11 @@
## UI Components
### CompressionStatsChart
-The `CompressionStatsChart` component visualizes repository compression statistics in a card format:
+The `CompressionStatsChart` component visualizes repository compression statistics in a card format and is production-ready as of PR #551.
**Location:** `app/client/modules/repositories/components/compression-stats-chart.tsx`
-**Displayed on:** Repository info tab
+**Displayed on:** Repository info tab (rendered within a grid layout alongside the main repository settings card)
**Purpose:** Provides visual feedback on repository storage efficiency and compression effectiveness
@@ -325,10 +330,11 @@
- **Compression Ratio**: How much the data has been compressed (e.g., 2.5x)
- **Snapshots Count**: Total number of snapshots with compression progress percentage
-**Features:**
+**Implementation:**
+- Fetches data from the `/api/v1/repositories/{shortId}/stats` endpoint using React Query (`getRepositoryStatsOptions`)
- Handles loading states while fetching statistics
- Displays error messages if statistics cannot be retrieved
- Shows empty state when no statistics are available yet (e.g., before first backup)
-- Uses the `GET /api/v1/repositories/{shortId}/stats` endpoint
-- Formats byte sizes and percentages for readability
-
+- Formats byte sizes and percentages for readability using the `ByteSize` component
+- Uses safe number parsing to handle edge cases and prevent display of invalid values
+ |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/server/utils/restic.ts (2)
697-721:restic stats --mode raw-datashould have a timeout; it can block indefinitely on large repositories.Unlike
snapshots(metadata only),restic stats --mode raw-datareads every pack file to compute actual sizes. On large repositories or slow/remote backends (S3, R2, GCS, SFTP), this can take several minutes and will block the HTTP request thread with no escape hatch. Theexechelper already accepts atimeoutoption (seeinit).Consider adding a configurable timeout — or at minimum a hard cap — so the endpoint degrades gracefully rather than timing out at the HTTP layer.
⏱️ Suggested fix: add a timeout option
-const stats = async (config: RepositoryConfig, options: { organizationId: string }) => { +const stats = async (config: RepositoryConfig, options: { organizationId: string; timeoutMs?: number }) => { const repoUrl = buildRepoUrl(config); const env = await buildEnv(config, options.organizationId); const args = ["--repo", repoUrl, "stats", "--mode", "raw-data"]; addCommonArgs(args, env, config); - const res = await exec({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env, timeout: options.timeoutMs ?? 120_000 }); await cleanupTemporaryKeys(env);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/utils/restic.ts` around lines 697 - 721, The stats function calls exec({ command: "restic", args, env }) without a timeout; update stats to pass a timeout to exec (e.g., exec({ command: "restic", args, env, timeout: <ms> })) and make that timeout configurable (via RepositoryConfig or a service-level env/config value) with a sensible hard cap, ensure buildEnv/cleanupTemporaryKeys usage remains unchanged, and handle timeout failures the same way other exec errors are handled (log with logger.error and throw a ResticError) so long-running restic stats (--mode raw-data) cannot block indefinitely.
420-420:ResticStats = ResticStatsDtotype alias is redundant.The alias adds no additional constraint and requires consumers to choose between two names for the same type.
ResticStatsDtois already exported from~/schemas/restic-dtoand can be used directly.♻️ Proposed cleanup
-export type ResticStats = ResticStatsDto;Then import
ResticStatsDtodirectly whereverResticStatswas used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/utils/restic.ts` at line 420, Remove the redundant type alias ResticStats that simply re-exports ResticStatsDto: delete the line "export type ResticStats = ResticStatsDto;" and update any imports/usages to reference ResticStatsDto directly (search for ResticStats and replace with ResticStatsDto, keeping any existing import from ~/schemas/restic-dto or adding it where missing).app/server/modules/repositories/__tests__/repositories.controller.test.ts (1)
119-128: Consider adding a happy-path test with a mockedrestic.stats.The current tests cover auth (401) and missing repo (404), but there is no test verifying the 200 response shape, that stats fields are returned correctly, or that the cache is populated. If
resticis spied/mocked in other tests in this suite, a similar pattern could cover the successful path.Would you like me to draft a test for the 200 case that mocks
restic.stats(e.g., usingbun:test'smock.moduleor aspyOn)?As per coding guidelines, tests should be run with
bunx dotenv-cli -e .env.test -- bun test --preload ./app/test/setup.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/modules/repositories/__tests__/repositories.controller.test.ts` around lines 119 - 128, Add a happy-path test that mocks restic.stats and verifies a 200 response and response shape: in repositories.controller.test.ts use createTestSession() and getAuthHeaders(token) and spy on or mock restic.stats (via bun:test mock.module or jest.spyOn equivalent) to return a predefined stats object, call app.request("/api/v1/repositories/<repo>/stats") and expect status 200 and the body to contain the same stats fields (e.g., size, snapshots, etc. as returned by your mock); also assert that the controller's cache is populated for that repository (check whatever cache API your controller exposes or the cache key used by the stats endpoint) and that subsequent requests use the cached value (call the endpoint twice and verify restic.stats was only invoked once).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/server/utils/cache.ts`:
- Around line 102-114: delByPrefix is building a LIKE pattern from keys (used
with cacheKeys.repository.all) without escaping SQLite wildcards, so
repositoryIds containing % or _ would match incorrectly; fix by escaping '%' '_'
and backslash characters in the prefix inside delByPrefix (replace '\' with
'\\', '%' with '\%', '_' with '\_'), then build the LIKE pattern using the
escaped prefix plus '%' and pass the pattern as a parameter to the SQL query
(and include an ESCAPE '\' clause if using raw SQL) to ensure only keys with the
literal prefix are deleted.
---
Nitpick comments:
In `@app/server/modules/repositories/__tests__/repositories.controller.test.ts`:
- Around line 119-128: Add a happy-path test that mocks restic.stats and
verifies a 200 response and response shape: in repositories.controller.test.ts
use createTestSession() and getAuthHeaders(token) and spy on or mock
restic.stats (via bun:test mock.module or jest.spyOn equivalent) to return a
predefined stats object, call app.request("/api/v1/repositories/<repo>/stats")
and expect status 200 and the body to contain the same stats fields (e.g., size,
snapshots, etc. as returned by your mock); also assert that the controller's
cache is populated for that repository (check whatever cache API your controller
exposes or the cache key used by the stats endpoint) and that subsequent
requests use the cached value (call the endpoint twice and verify restic.stats
was only invoked once).
In `@app/server/utils/restic.ts`:
- Around line 697-721: The stats function calls exec({ command: "restic", args,
env }) without a timeout; update stats to pass a timeout to exec (e.g., exec({
command: "restic", args, env, timeout: <ms> })) and make that timeout
configurable (via RepositoryConfig or a service-level env/config value) with a
sensible hard cap, ensure buildEnv/cleanupTemporaryKeys usage remains unchanged,
and handle timeout failures the same way other exec errors are handled (log with
logger.error and throw a ResticError) so long-running restic stats (--mode
raw-data) cannot block indefinitely.
- Line 420: Remove the redundant type alias ResticStats that simply re-exports
ResticStatsDto: delete the line "export type ResticStats = ResticStatsDto;" and
update any imports/usages to reference ResticStatsDto directly (search for
ResticStats and replace with ResticStatsDto, keeping any existing import from
~/schemas/restic-dto or adding it where missing).
ea50047 to
c19f2f5
Compare
c19f2f5 to
43b5562
Compare

Summary by CodeRabbit
Release Notes