docs/plans/2026-05-21-musicbrainz-provider.md

@@ -47,13 +47,13 @@ Implement a MusicBrainz API provider with strict 1 request/second rate limiting,
 ## 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
+- [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
 - [ ] add golang.org/x/time/rate dependency to go.mod

internal/musicbrainz/client.go

@@ -0,0 +1,223 @@
+package musicbrainz
+
+import (
+	"encoding/xml"
+	"fmt"
+	"io"
+	"net/http"
+	"time"
+
+	"naviwatcher/internal/config"
+)
+
+// MusicBrainzClient wraps net/http.Client with rate limiting and configuration
+// for the MusicBrainz Web Service API (version 2).
+type MusicBrainzClient struct {
+	httpClient  *http.Client
+	userAgent   string
+	baseURL     string
+	rateLimiter *rateLimiter
+}
+
+// rateLimiter wraps a token-bucket rate limiter for API calls.
+type rateLimiter struct {
+	// tokens is a channel-based semaphore for rate limiting.
+	// It is filled at a fixed interval by a background goroutine.
+	tokens chan struct{}
+	done   chan struct{}
+}
+
+// newRateLimiter creates a rate limiter that allows maxCalls per second.
+// It immediately fills the bucket and starts a refill goroutine.
+func newRateLimiter(callsPerSecond int) *rateLimiter {
+	rl := &rateLimiter{
+		tokens: make(chan struct{}, callsPerSecond),
+		done:   make(chan struct{}),
+	}
+	// Fill the bucket initially
+	for i := 0; i < callsPerSecond; i++ {
+		rl.tokens <- struct{}{}
+	}
+	// Refill at the specified interval
+	interval := time.Second / time.Duration(callsPerSecond)
+	go func() {
+		ticker := time.NewTicker(interval)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-ticker.C:
+				select {
+				case rl.tokens <- struct{}{}:
+				default:
+					// bucket full, skip
+				}
+			case <-rl.done:
+				return
+			}
+		}
+	}()
+	return rl
+}
+
+// wait blocks until a token is available or the rate limiter is stopped.
+func (rl *rateLimiter) wait() {
+	<-rl.tokens
+}
+
+// stop terminates the refill goroutine.
+func (rl *rateLimiter) stop() {
+	close(rl.done)
+}
+
+// NewClient creates a new MusicBrainzClient from the given configuration.
+// It initializes the HTTP client with a 30-second timeout and sets up
+// a rate limiter for 1 request per second as required by MusicBrainz policy.
+func NewClient(cfg config.MusicBrainzConfig) *MusicBrainzClient {
+	rl := newRateLimiter(1) // 1 request per second
+	return &MusicBrainzClient{
+		httpClient: &http.Client{
+			Timeout: 30 * time.Second,
+		},
+		userAgent:   cfg.UserAgent,
+		baseURL:     "https://musicbrainz.org/ws/2",
+		rateLimiter: rl,
+	}
+}
+
+// NewClientWithLimiter creates a MusicBrainzClient with a custom rate limiter.
+// This is primarily used for testing to inject a mock rate limiter.
+func NewClientWithLimiter(cfg config.MusicBrainzConfig, rl *rateLimiter) *MusicBrainzClient {
+	return &MusicBrainzClient{
+		httpClient: &http.Client{
+			Timeout: 30 * time.Second,
+		},
+		userAgent:   cfg.UserAgent,
+		baseURL:     "https://musicbrainz.org/ws/2",
+		rateLimiter: rl,
+	}
+}
+
+// Close cleans up the rate limiter goroutine.
+func (c *MusicBrainzClient) Close() {
+	c.rateLimiter.stop()
+}
+
+// doGet performs a rate-limited HTTP GET request to the MusicBrainz API.
+// It sets the proper User-Agent header and returns the response body.
+func (c *MusicBrainzClient) doGet(path string) ([]byte, error) {
+	c.rateLimiter.wait()
+
+	req, err := http.NewRequest(http.MethodGet, c.baseURL+path, nil)
+	if err != nil {
+		return nil, fmt.Errorf("create request: %w", err)
+	}
+	req.Header.Set("User-Agent", c.userAgent)
+	req.Header.Set("Accept", "application/xml")
+
+	resp, err := c.httpClient.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("execute request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
+		return nil, fmt.Errorf("musicbrainz API returned HTTP %d: %s", resp.StatusCode, string(body))
+	}
+
+	body, err := io.ReadAll(io.LimitReader(resp.Body, 10*1024*1024)) // 10MB limit
+	if err != nil {
+		return nil, fmt.Errorf("read response body: %w", err)
+	}
+
+	return body, nil
+}
+
+// mbArtistRef represents the nested artist element inside a release-group.
+type mbArtistRef struct {
+	ID   string `xml:"id,attr"`
+	Name string `xml:"name"`
+}
+
+// mbNameCredit represents the name-credit element inside a release-group.
+type mbNameCredit struct {
+	Artist mbArtistRef `xml:"artist"`
+}
+
+// mbArtistCredit represents the artist-credit element inside a release-group.
+type mbArtistCredit struct {
+	NameCredit mbNameCredit `xml:"name-credit"`
+}
+
+// mbReleaseGroup represents the XML structure of a single release-group
+// in the MusicBrainz release-group list response.
+type mbReleaseGroup struct {
+	ID            string         `xml:"id,attr"`
+	Title         string         `xml:"title"`
+	Type          string         `xml:"type,attr"`
+	Status        string         `xml:"status,attr"`
+	ArtistCredit  mbArtistCredit `xml:"artist-credit"`
+	ReleaseDate   string         `xml:"first-release-date"`
+}
+
+// mbReleaseGroupListXML wraps the release-group-list element to properly
+// capture both child elements and the count attribute.
+type mbReleaseGroupListXML struct {
+	ReleaseGroups []mbReleaseGroup `xml:"release-group"`
+	Count         int              `xml:"count,attr"`
+}
+
+// mbReleaseGroupList represents the XML structure of a release-group list response.
+type mbReleaseGroupList struct {
+	XMLName         xml.Name             `xml:"metadata"`
+	ReleaseGroupList mbReleaseGroupListXML `xml:"release-group-list"`
+}
+
+// mbArtistData represents the artist element inside metadata.
+type mbArtistData struct {
+	ID   string `xml:"id,attr"`
+	Name string `xml:"name"`
+}
+
+// mbArtist represents the XML structure of a MusicBrainz artist response.
+type mbArtist struct {
+	XMLName xml.Name     `xml:"metadata"`
+	Artist  mbArtistData `xml:"artist"`
+}
+
+// ParseReleaseGroups parses a MusicBrainz release-group list XML response
+// into a ParsedReleaseGroups struct.
+func ParseReleaseGroups(data []byte) (*ParsedReleaseGroups, error) {
+	var list mbReleaseGroupList
+	if err := xml.Unmarshal(data, &list); err != nil {
+		return nil, fmt.Errorf("parse release-group XML: %w", err)
+	}
+
+	result := &ParsedReleaseGroups{
+		Count: list.ReleaseGroupList.Count,
+	}
+	for _, rg := range list.ReleaseGroupList.ReleaseGroups {
+		result.ReleaseGroups = append(result.ReleaseGroups, ReleaseGroup{
+			ID:          rg.ID,
+			Title:       rg.Title,
+			Type:        rg.Type,
+			Status:      rg.Status,
+			ArtistID:    rg.ArtistCredit.NameCredit.Artist.ID,
+			ArtistName:  rg.ArtistCredit.NameCredit.Artist.Name,
+			ReleaseDate: rg.ReleaseDate,
+		})
+	}
+	return result, nil
+}
+
+// ParseArtist parses a MusicBrainz artist XML response into a ParsedArtist struct.
+func ParseArtist(data []byte) (*ParsedArtist, error) {
+	var artist mbArtist
+	if err := xml.Unmarshal(data, &artist); err != nil {
+		return nil, fmt.Errorf("parse artist XML: %w", err)
+	}
+	return &ParsedArtist{
+		ID:   artist.Artist.ID,
+		Name: artist.Artist.Name,
+	}, nil
+}

internal/musicbrainz/client_test.go

@@ -0,0 +1,167 @@
+package musicbrainz
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"naviwatcher/internal/config"
+)
+
+func newUnbufferedRateLimiter() *rateLimiter {
+	return newRateLimiter(1000) // high rate to avoid blocking in tests
+}
+
+func TestNewClient_ValidConfig(t *testing.T) {
+	cfg := config.MusicBrainzConfig{
+		UserAgent: "naviwatcher/0.1.0 (test@example.com)",
+	}
+
+	client := NewClient(cfg)
+	defer client.Close()
+
+	if client == nil {
+		t.Fatal("NewClient() returned nil client")
+	}
+
+	if client.httpClient == nil {
+		t.Fatal("NewClient() returned client with nil http.Client")
+	}
+
+	if client.userAgent != cfg.UserAgent {
+		t.Errorf("NewClient().userAgent = %q, want %q", client.userAgent, cfg.UserAgent)
+	}
+
+	expectedBaseURL := "https://musicbrainz.org/ws/2"
+	if client.baseURL != expectedBaseURL {
+		t.Errorf("NewClient().baseURL = %q, want %q", client.baseURL, expectedBaseURL)
+	}
+
+	if client.rateLimiter == nil {
+		t.Fatal("NewClient() returned client with nil rate limiter")
+	}
+}
+
+func TestNewClientWithLimiter(t *testing.T) {
+	cfg := config.MusicBrainzConfig{
+		UserAgent: "naviwatcher/0.1.0 (test@example.com)",
+	}
+
+	rl := newRateLimiter(1)
+	client := NewClientWithLimiter(cfg, rl)
+	defer client.Close()
+
+	if client == nil {
+		t.Fatal("NewClientWithLimiter() returned nil client")
+	}
+
+	if client.rateLimiter != rl {
+		t.Error("NewClientWithLimiter() did not use provided rate limiter")
+	}
+}
+
+func TestDoGet_Success(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("User-Agent") == "" {
+			t.Error("doGet() request missing User-Agent header")
+		}
+		if r.Header.Get("Accept") != "application/xml" {
+			t.Errorf("doGet() Accept header = %q, want %q", r.Header.Get("Accept"), "application/xml")
+		}
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(`<metadata><test>ok</test></metadata>`))
+	}))
+	defer server.Close()
+
+	cfg := config.MusicBrainzConfig{
+		UserAgent: "naviwatcher/0.1.0 (test@example.com)",
+	}
+
+	rl := newUnbufferedRateLimiter()
+	client := &MusicBrainzClient{
+		httpClient:  server.Client(),
+		userAgent:   cfg.UserAgent,
+		baseURL:     server.URL,
+		rateLimiter: rl,
+	}
+	defer client.Close()
+
+	body, err := client.doGet("/test")
+	if err != nil {
+		t.Fatalf("doGet() error = %v", err)
+	}
+
+	if string(body) != `<metadata><test>ok</test></metadata>` {
+		t.Errorf("doGet() body = %q", string(body))
+	}
+}
+
+func TestDoGet_Non200Status(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusServiceUnavailable)
+		w.Write([]byte("Rate limit exceeded"))
+	}))
+	defer server.Close()
+
+	cfg := config.MusicBrainzConfig{
+		UserAgent: "naviwatcher/0.1.0 (test@example.com)",
+	}
+
+	rl := newUnbufferedRateLimiter()
+	client := &MusicBrainzClient{
+		httpClient:  server.Client(),
+		userAgent:   cfg.UserAgent,
+		baseURL:     server.URL,
+		rateLimiter: rl,
+	}
+	defer client.Close()
+
+	_, err := client.doGet("/test")
+	if err == nil {
+		t.Fatal("doGet() expected error for non-200 status, got nil")
+	}
+}
+
+func TestDoGet_ServerUnreachable(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	server.Close()
+
+	cfg := config.MusicBrainzConfig{
+		UserAgent: "naviwatcher/0.1.0 (test@example.com)",
+	}
+
+	rl := newUnbufferedRateLimiter()
+	client := &MusicBrainzClient{
+		httpClient:  server.Client(),
+		userAgent:   cfg.UserAgent,
+		baseURL:     server.URL,
+		rateLimiter: rl,
+	}
+	defer client.Close()
+
+	_, err := client.doGet("/test")
+	if err == nil {
+		t.Fatal("doGet() expected error for unreachable server, got nil")
+	}
+}
+
+func TestRateLimiter_BasicBehavior(t *testing.T) {
+	// Test that rate limiter can produce tokens
+	rl := newRateLimiter(1)
+	defer rl.stop()
+
+	// Should be able to get a token immediately (bucket was pre-filled)
+	done := make(chan struct{})
+	go func() {
+		rl.wait()
+		close(done)
+	}()
+
+	select {
+	case <-done:
+		// success
+	case <-time.After(2 * time.Second):
+		t.Fatal("rateLimiter.wait() blocked on pre-filled bucket")
+	}
+}

internal/musicbrainz/model.go

@@ -0,0 +1,46 @@
+package musicbrainz
+
+import "time"
+
+// ReleaseGroup represents a MusicBrainz Release Group entity.
+// This is the primary data model for the provider - we work with
+// Release Groups to minimize duplicates from different releases.
+type ReleaseGroup struct {
+	ID          string
+	Title       string
+	Type        string
+	Status      string
+	ArtistID    string
+	ArtistName  string
+	ReleaseDate string
+}
+
+// Artist represents a MusicBrainz artist entity.
+type Artist struct {
+	ID   string
+	Name string
+}
+
+// ExternalRelease is the normalized form stored in the database,
+// matching the external_releases table schema.
+type ExternalRelease struct {
+	RGID        string
+	ArtistID    string
+	Title       string
+	Type        string
+	ReleaseDate string
+	CachedAt    time.Time
+}
+
+// ParsedReleaseGroups holds the result of parsing a MusicBrainz
+// release-group list XML response.
+type ParsedReleaseGroups struct {
+	ReleaseGroups []ReleaseGroup
+	Count         int
+}
+
+// ParsedArtist holds the result of parsing a MusicBrainz artist lookup.
+type ParsedArtist struct {
+	ID   string
+	Name string
+}

internal/musicbrainz/model_test.go

@@ -0,0 +1,201 @@
+package musicbrainz
+
+import (
+	"testing"
+)
+
+func TestParseReleaseGroups_Success(t *testing.T) {
+	data := []byte(`<?xml version="1.0" encoding="UTF-8"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+  <release-group-list count="2">
+    <release-group id="rg-uuid-1" type="Album">
+      <title>Dark Side of the Moon</title>
+      <first-release-date>1973-03-01</first-release-date>
+      <artist-credit>
+        <name-credit>
+          <artist id="artist-uuid-1">
+            <name>Pink Floyd</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+    </release-group>
+    <release-group id="rg-uuid-2" type="Single">
+      <title>Another Brick in the Wall</title>
+      <first-release-date>1979-11-30</first-release-date>
+      <artist-credit>
+        <name-credit>
+          <artist id="artist-uuid-1">
+            <name>Pink Floyd</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+    </release-group>
+  </release-group-list>
+</metadata>`)
+
+	result, err := ParseReleaseGroups(data)
+	if err != nil {
+		t.Fatalf("ParseReleaseGroups() error = %v", err)
+	}
+
+	if result.Count != 2 {
+		t.Errorf("ParseReleaseGroups().Count = %d, want 2", result.Count)
+	}
+
+	if len(result.ReleaseGroups) != 2 {
+		t.Fatalf("ParseReleaseGroups() returned %d groups, want 2", len(result.ReleaseGroups))
+	}
+
+	expected := []ReleaseGroup{
+		{
+			ID:          "rg-uuid-1",
+			Title:       "Dark Side of the Moon",
+			Type:        "Album",
+			ArtistID:    "artist-uuid-1",
+			ArtistName:  "Pink Floyd",
+			ReleaseDate: "1973-03-01",
+		},
+		{
+			ID:          "rg-uuid-2",
+			Title:       "Another Brick in the Wall",
+			Type:        "Single",
+			ArtistID:    "artist-uuid-1",
+			ArtistName:  "Pink Floyd",
+			ReleaseDate: "1979-11-30",
+		},
+	}
+
+	for i, rg := range result.ReleaseGroups {
+		if rg.ID != expected[i].ID {
+			t.Errorf("ReleaseGroups[%d].ID = %q, want %q", i, rg.ID, expected[i].ID)
+		}
+		if rg.Title != expected[i].Title {
+			t.Errorf("ReleaseGroups[%d].Title = %q, want %q", i, rg.Title, expected[i].Title)
+		}
+		if rg.Type != expected[i].Type {
+			t.Errorf("ReleaseGroups[%d].Type = %q, want %q", i, rg.Type, expected[i].Type)
+		}
+		if rg.ArtistID != expected[i].ArtistID {
+			t.Errorf("ReleaseGroups[%d].ArtistID = %q, want %q", i, rg.ArtistID, expected[i].ArtistID)
+		}
+		if rg.ArtistName != expected[i].ArtistName {
+			t.Errorf("ReleaseGroups[%d].ArtistName = %q, want %q", i, rg.ArtistName, expected[i].ArtistName)
+		}
+		if rg.ReleaseDate != expected[i].ReleaseDate {
+			t.Errorf("ReleaseGroups[%d].ReleaseDate = %q, want %q", i, rg.ReleaseDate, expected[i].ReleaseDate)
+		}
+	}
+}
+
+func TestParseReleaseGroups_Empty(t *testing.T) {
+	data := []byte(`<?xml version="1.0" encoding="UTF-8"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+  <release-group-list count="0">
+  </release-group-list>
+</metadata>`)
+
+	result, err := ParseReleaseGroups(data)
+	if err != nil {
+		t.Fatalf("ParseReleaseGroups() error = %v", err)
+	}
+
+	if result.Count != 0 {
+		t.Errorf("ParseReleaseGroups().Count = %d, want 0", result.Count)
+	}
+
+	if len(result.ReleaseGroups) != 0 {
+		t.Errorf("ParseReleaseGroups() returned %d groups, want 0", len(result.ReleaseGroups))
+	}
+}
+
+func TestParseReleaseGroups_MalformedXML(t *testing.T) {
+	tests := []struct {
+		name string
+		data []byte
+	}{
+		{
+			name: "truncated XML",
+			data: []byte(`<?xml version="1.0"?><metadata><release-group-list>`),
+		},
+		{
+			name: "not XML at all",
+			data: []byte(`this is not xml`),
+		},
+		{
+			name: "wrong root element",
+			data: []byte(`<?xml version="1.0"?><wrongRoot><item>test</item></wrongRoot>`),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := ParseReleaseGroups(tt.data)
+			if err == nil {
+				t.Fatal("ParseReleaseGroups() expected error for malformed XML, got nil")
+			}
+		})
+	}
+}
+
+func TestParseReleaseGroups_WithStatus(t *testing.T) {
+	data := []byte(`<?xml version="1.0" encoding="UTF-8"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+  <release-group-list count="1">
+    <release-group id="rg-bootleg" type="Album" status="Bootleg">
+      <title>Unofficial Live Recording</title>
+      <first-release-date>2020-01-01</first-release-date>
+      <artist-credit>
+        <name-credit>
+          <artist id="artist-uuid-2">
+            <name>Test Artist</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+    </release-group>
+  </release-group-list>
+</metadata>`)
+
+	result, err := ParseReleaseGroups(data)
+	if err != nil {
+		t.Fatalf("ParseReleaseGroups() error = %v", err)
+	}
+
+	if len(result.ReleaseGroups) != 1 {
+		t.Fatalf("ParseReleaseGroups() returned %d groups, want 1", len(result.ReleaseGroups))
+	}
+
+	if result.ReleaseGroups[0].Status != "Bootleg" {
+		t.Errorf("ReleaseGroups[0].Status = %q, want %q", result.ReleaseGroups[0].Status, "Bootleg")
+	}
+}
+
+func TestParseArtist_Success(t *testing.T) {
+	data := []byte(`<?xml version="1.0" encoding="UTF-8"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+  <artist id="artist-uuid-1" type="Group">
+    <name>Pink Floyd</name>
+  </artist>
+</metadata>`)
+
+	result, err := ParseArtist(data)
+	if err != nil {
+		t.Fatalf("ParseArtist() error = %v", err)
+	}
+
+	if result.ID != "artist-uuid-1" {
+		t.Errorf("ParseArtist().ID = %q, want %q", result.ID, "artist-uuid-1")
+	}
+
+	if result.Name != "Pink Floyd" {
+		t.Errorf("ParseArtist().Name = %q, want %q", result.Name, "Pink Floyd")
+	}
+}
+
+func TestParseArtist_MalformedXML(t *testing.T) {
+	data := []byte(`this is not xml`)
+
+	_, err := ParseArtist(data)
+	if err == nil {
+		t.Fatal("ParseArtist() expected error for malformed XML, got nil")
+	}
+}