diff --git a/.gitignore b/.gitignore index cbce990..c8e24df 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ naviwatcher.exe config.yaml data/ coverage.out +navidrome_cov.out diff --git a/docs/plans/2026-05-21-musicbrainz-provider.md b/docs/plans/2026-05-21-musicbrainz-provider.md new file mode 100644 index 0000000..7b7ea01 --- /dev/null +++ b/docs/plans/2026-05-21-musicbrainz-provider.md @@ -0,0 +1,134 @@ +# 2026-05-21-musicbrainz-provider + +## Overview +Implement a MusicBrainz API provider with strict 1 request/second rate limiting, 24-hour caching of Release Group data, and filtering capabilities as per specification. The provider will fetch artist discographies from MusicBrainz, normalize the data, and store it in the external_releases table for use by the scanner engine. + +## Context (from discovery) +- Files/components involved: internal/musicbrainz/ package (new), database schema updates, main.go integration +- Related patterns found: Follows the internal/navidrome/ pattern with client.go, sync.go, and model separation +- Dependencies identified: Will add golang.org/x/time/rate for rate limiting, use net/http for API calls + +## Development Approach +- **Testing approach**: Regular (code first, then tests) +- Complete each task fully before moving to the next +- Make small, focused changes +- **CRITICAL: every task MUST include new/updated tests** for code changes in that task + - tests are not optional - they are a required part of the checklist + - write unit tests for new functions/methods + - write unit tests for modified functions/methods + - add new test cases for new code paths + - update existing test cases if behavior changes + - tests cover both success and error scenarios +- **CRITICAL: all tests must pass before starting next task** - no exceptions +- **CRITICAL: update this plan file when scope changes during implementation** +- Run tests after each change +- Maintain backward compatibility + +## Testing Strategy +- **Unit tests**: required for every task (see Development Approach above) +- **E2E tests**: if project has UI-based e2e tests (Playwright, Cypress, etc.): + - UI changes → add/update e2e tests in same task as UI code + - Backend changes supporting UI → add/update e2e tests in same task + - Treat e2e tests with same rigor as unit tests (must pass before next task) + - Store e2e tests alongside unit tests (or in designated e2e directory) + +## Progress Tracking +- Mark completed items with `[x]` immediately when done +- Add newly discovered tasks with ➕ prefix +- Document issues/blockers with ⚠️ prefix +- Update plan if implementation deviates from original scope +- Keep plan in sync with actual work done + +## What Goes Where +- **Implementation Steps** (`[ ]` checkboxes): tasks achievable within this codebase - code changes, tests, documentation updates +- **Post-Completion** (no checkboxes): items requiring external action - manual testing, changes in consuming projects, deployment configs, third-party verifications +- **Checkbox placement**: Checkboxes belong only in Task sections (`### Task N:` or `### Iteration N:`). Do not put checkboxes in Success criteria, Overview, or Context — they cause extra loop iterations. + +## Implementation Steps + +### Task 1: Create MusicBrainz client and data models +- [ ] create `internal/musicbrainz/client.go` with MusicBrainzClient struct wrapping net/http.Client +- [ ] implement constructor taking config and rate limiter +- [ ] create `internal/musicbrainz/model.go` with structs for MusicBrainz API responses (ReleaseGroup, Artist, etc.) +- [ ] implement XML parsing functions for MusicBrainz responses +- [ ] write tests for XML parsing (success + error cases) +- [ ] write tests for client constructor and basic API call structure +- [ ] run tests - must pass before next task + +### Task 2: Implement rate limiting and caching layer +- [ ] add golang.org/x/time/rate dependency to go.mod +- [ ] implement rate limiter using golang.org/x/time/rate.NewLimiter(1, 1) for 1 req/sec +- [ ] create wrapper method for rate-limited HTTP GET requests +- [ ] implement caching check: query database for existing Release Group data within TTL +- [ ] write tests for rate limiting behavior (timing tests) +- [ ] write tests for cache hit/miss logic +- [ ] run tests - must pass before next task + +### Task 3: Implement MusicBrainz API endpoints and filtering +- [ ] implement GetArtistReleaseGroups(artistMBID string) method +- [ ] apply filters: exclude Bootleg/Promotion/Pseudo-Release status +- [ ] apply type filters: include Album/Single/EP/Compilation only +- [ ] implement per-artist type filtering hooks (placeholder for Web UI integration) +- [ ] normalize artist names and titles (remove special characters, years, brackets) +- [ ] write tests for filtering logic (table-driven test cases) +- [ ] write tests for normalization functions +- [ ] run tests - must pass before next task + +### Task 4: Implement database integration and sync orchestration +- [ ] create `internal/musicbrainz/sync.go` with SyncArtistDiscography function +- [ ] implement upsert logic: INSERT OR REPLACE into external_releases table +- [ ] add cached_at column to external_releases table via migration +- [ ] implement context.Context support for cancellation +- [ ] write tests for database upsert operations +- [ ] write integration tests with in-memory SQLite +- [ ] run tests - must pass before next task + +### Task 5: Wire up provider in application entry point +- [ ] update `cmd/naviwatcher/main.go` to initialize MusicBrainz client +- [ ] add MusicBrainz client to application context/dependencies +- [ ] ensure graceful shutdown includes closing HTTP client connections +- [ ] update config validation to ensure MusicBrainz.UserAgent is set +- [ ] write tests for main.go integration (startup/shutdown) +- [ ] run tests - must pass before next task + +### Task 6: Verify acceptance criteria and run full test suite +- [ ] verify all requirements from Overview are implemented +- [ ] verify edge cases are handled (network errors, invalid responses, rate limit blocking) +- [ ] run full test suite (unit tests) +- [ ] run linter - all issues must be fixed +- [ ] verify test coverage meets project standard (80%+) + +## Technical Details +### Data Structures +- MusicBrainzClient: wraps *http.Client with rate limiter and config +- ExternalRelease: matches existing database struct with addition of CachedAt time.Time +- MusicBrainz API Response Models: ReleaseGroup, Artist, etc. based on XML schema + +### Parameters and Formats +- Rate Limiter: 1 request per second burst size of 1 (strict limit) +- Cache TTL: configurable via MusicBrainzConfig.CacheTTL (default 24h) +- API Endpoint: https://musicbrainz.org/ws/2/ with proper User-Agent header +- Response Format: XML parsing of MusicBrainz Web Service responses + +### Processing Flow +1. SyncArtistDiscography called with MusicBrainz Artist ID +2. Check cache: query external_releases for RGIDs with cached_at within TTL +3. If cache miss or expired: call MusicBrainz API with rate limiting +4. Parse XML response into ReleaseGroup models +5. Apply status/type filtering (Bootleg/Promotion/Pseudo-Release excluded) +6. Apply per-artist type filtering ( Singles/Compilations toggle via Web UI) +7. Normalize strings (remove special chars, years, brackets for fuzzy matching) +8. Upsert each Release Group to external_releases with current timestamp +9. Return list of Release Groups for scanner consumption + +## Post-Completion +*Items requiring manual intervention or external systems - no checkboxes, informational only* + +**Manual verification** (if applicable): +- Manual testing of rate limiting under load +- Verify cache expiration behavior over time +- Test with real MusicBrainz API to ensure compliance with their usage policy +- Performance testing of XML parsing and filtering logic + +**External system updates** (if applicable): +- None - this is a standalone provider implementation \ No newline at end of file