134 lines
7.3 KiB
Markdown
134 lines
7.3 KiB
Markdown
# 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
|
||
- [x] create `internal/musicbrainz/client.go` with MusicBrainzClient struct wrapping net/http.Client
|
||
- [x] implement constructor taking config and rate limiter
|
||
- [x] create `internal/musicbrainz/model.go` with structs for MusicBrainz API responses (ReleaseGroup, Artist, etc.)
|
||
- [x] implement XML parsing functions for MusicBrainz responses
|
||
- [x] write tests for XML parsing (success + error cases)
|
||
- [x] write tests for client constructor and basic API call structure
|
||
- [x] run tests - must pass before next task
|
||
|
||
### Task 2: Implement rate limiting and caching layer
|
||
- [x] add golang.org/x/time/rate dependency to go.mod
|
||
- [x] implement rate limiter using golang.org/x/time/rate.NewLimiter(1, 1) for 1 req/sec
|
||
- [x] create wrapper method for rate-limited HTTP GET requests
|
||
- [x] implement caching check: query database for existing Release Group data within TTL
|
||
- [x] write tests for rate limiting behavior (timing tests)
|
||
- [x] write tests for cache hit/miss logic
|
||
- [x] run tests - must pass before next task
|
||
|
||
### Task 3: Implement MusicBrainz API endpoints and filtering
|
||
- [x] implement GetArtistReleaseGroups(artistMBID string) method
|
||
- [x] apply filters: exclude Bootleg/Promotion/Pseudo-Release status
|
||
- [x] apply type filters: include Album/Single/EP/Compilation only
|
||
- [x] implement per-artist type filtering hooks (placeholder for Web UI integration)
|
||
- [x] normalize artist names and titles (remove special characters, years, brackets)
|
||
- [x] write tests for filtering logic (table-driven test cases)
|
||
- [x] write tests for normalization functions
|
||
- [x] run tests - must pass before next task
|
||
|
||
### Task 4: Implement database integration and sync orchestration
|
||
- [x] create `internal/musicbrainz/sync.go` with SyncArtistDiscography function
|
||
- [x] implement upsert logic: INSERT OR REPLACE into external_releases table
|
||
- [x] add cached_at column to external_releases table via migration (done in Task 3 as migration 005)
|
||
- [x] implement context.Context support for cancellation
|
||
- [x] write tests for database upsert operations
|
||
- [x] write integration tests with in-memory SQLite
|
||
- [x] run tests - must pass before next task
|
||
|
||
### Task 5: Wire up provider in application entry point
|
||
- [x] update `cmd/naviwatcher/main.go` to initialize MusicBrainz client
|
||
- [x] add MusicBrainz client to application context/dependencies
|
||
- [x] ensure graceful shutdown includes closing HTTP client connections
|
||
- [x] update config validation to ensure MusicBrainz.UserAgent is set
|
||
- [x] write tests for main.go integration (startup/shutdown)
|
||
- [x] run tests - must pass before next task
|
||
|
||
### Task 6: Verify acceptance criteria and run full test suite
|
||
- [x] verify all requirements from Overview are implemented
|
||
- [x] verify edge cases are handled (network errors, invalid responses, rate limit blocking)
|
||
- [x] run full test suite (unit tests)
|
||
- [x] run linter - all issues must be fixed
|
||
- [x] 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 |