feat: MediaDB Media and Title Persistence#666
feat: MediaDB Media and Title Persistence#666wizzomafizzo merged 15 commits intoZaparooProject:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds persistent missing-media tracking and reconciliation, a new DevErr test system and launchers, selective cache rebuilds for partial indexing, many DB/API additions for title/tag/missing-flag maintenance, and refactors the mediascanner into a persistent per-system indexing driver with accompanying tests and migrations. Changes
Sequence Diagram(s)sequenceDiagram
participant Scanner
participant ScanState as ScanState
participant MediaDB
participant SQLite
rect rgba(100,150,200,0.5)
Note over Scanner,SQLite: Resume / Seed Phase
Scanner->>MediaDB: PopulatePersistentScanStateForSystem(systemID)
MediaDB->>SQLite: SELECT Media & Titles for system
SQLite-->>MediaDB: Rows (IDs, titles, tags)
MediaDB->>ScanState: Seed MediaTitleIDs, MediaTagIDs, MissingMedia
end
rect rgba(150,100,200,0.5)
Note over Scanner,MediaDB: Per-file processing
Scanner->>ScanState: AddMediaPath(path, title, tags)
alt media exists
ScanState->>ScanState: remove DBID from MissingMedia
ScanState->>MediaDB: UpdateMediaTitle / reconcile tags (delete/insert)
MediaDB->>SQLite: UPDATE/Delete/INSERT queries
else new media
Scanner->>MediaDB: INSERT Media/Title/Tags
MediaDB->>SQLite: INSERT queries
end
end
rect rgba(200,150,100,0.5)
Note over Scanner,SQLite: Finalize missing flags
Scanner->>MediaDB: ResetMissingFlags(systemDBID)
MediaDB->>SQLite: UPDATE Media SET IsMissing=0 WHERE SystemDBID=?
Scanner->>MediaDB: BulkSetMediaMissing(remainingDBIDs)
MediaDB->>SQLite: UPDATE Media SET IsMissing=1 WHERE DBID IN (...)
end
rect rgba(100,200,150,0.5)
Note over Scanner,MediaDB: Selective cache refresh
Scanner->>MediaDB: PopulateSystemTagsCacheForSystems(systems)
MediaDB->>SQLite: SELECT tags for touched systems
MediaDB->>MediaDB: Merge partial tag/slug cache into global caches
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/testing/helpers/db_mocks.go (1)
1291-1305:⚠️ Potential issue | 🟠 MajorTrack missing-flag writes in the mock, too.
BulkSetMediaMissingandResetMissingFlagsmutate DB state, but they still skiptrackDatabaseOperation(). That leavesOperationsOutsideTxnblind to the new missing-media write path, so transaction-safety regressions can slip through mock-based tests.💡 Minimal fix
func (m *MockMediaDBI) BulkSetMediaMissing(dbids map[int]struct{}) error { + m.trackDatabaseOperation() // Track if called outside transaction args := m.Called(dbids) if err := args.Error(0); err != nil { return fmt.Errorf("mock operation failed: %w", err) } return nil } func (m *MockMediaDBI) ResetMissingFlags(systemDBIDs []int) error { + m.trackDatabaseOperation() // Track if called outside transaction args := m.Called(systemDBIDs) if err := args.Error(0); err != nil { return fmt.Errorf("mock operation failed: %w", err) } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/testing/helpers/db_mocks.go` around lines 1291 - 1305, The mock methods BulkSetMediaMissing and ResetMissingFlags perform DB-mutating actions but don't call trackDatabaseOperation, so OperationsOutsideTxn can't detect these writes; update both MockMediaDBI.BulkSetMediaMissing and MockMediaDBI.ResetMissingFlags to invoke trackDatabaseOperation (or the mock's equivalent tracking helper) when the method runs (before or after the m.Called check) so the missing-media write is recorded for OperationsOutsideTxn checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/testing/helpers/db_mocks.go`:
- Around line 1291-1305: The mock methods BulkSetMediaMissing and
ResetMissingFlags perform DB-mutating actions but don't call
trackDatabaseOperation, so OperationsOutsideTxn can't detect these writes;
update both MockMediaDBI.BulkSetMediaMissing and MockMediaDBI.ResetMissingFlags
to invoke trackDatabaseOperation (or the mock's equivalent tracking helper) when
the method runs (before or after the m.Called check) so the missing-media write
is recorded for OperationsOutsideTxn checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab6153f7-419d-45e9-b18e-9fd4f1f76976
📒 Files selected for processing (6)
pkg/api/methods/methods_test.gopkg/database/mediadb/mediadb.gopkg/database/mediascanner/indexing_pipeline.gopkg/database/mediascanner/mediascanner.gopkg/service/daemon/daemon_test.gopkg/testing/helpers/db_mocks.go
✅ Files skipped from review due to trivial changes (1)
- pkg/service/daemon/daemon_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/api/methods/methods_test.go
Refactors MediaDB indexing to not truncate existing records.
To achieve this the ScanState is repopulated with Media and Titles on each loop.
Pulling this data from DB isn't free so indexing was refactored to handle all 3 launcher types per system.
Also added a DevErr system which should have no practical application other than making indexing verification easier between the 3 types.
So far tested as migration, fresh DB, and full/partial DB scans.
Re-found records self heal on next scan.
Rename some files and folders and get to scanning/browsing!
We should discuss purposeful truncation and orphan cleanup mechanisms and where they might live in the app/core.
Next stop: scrapers 🚂
Summary by CodeRabbit
New Features
Bug Fixes
Tests