From 15b05b57fb242e58c5dd60310c14e16ec1f5b9f6 Mon Sep 17 00:00:00 2001 From: Vladimir Zagainov Date: Tue, 26 May 2026 13:04:20 +0300 Subject: [PATCH] feat: implement MusicBrainz sync orchestration with caching and upsert Creates internal/musicbrainz/sync.go with SyncArtistDiscography function that orchestrates the full flow: cache check via GetCachedReleases, fetch from MusicBrainz API on cache miss, filter via FilterReleaseGroups, and upsert into external_releases via database.SaveExternalRelease. Includes SyncArtistDiscographyWithFilter variant for per-artist type filtering. All functions support context cancellation. 14 new tests cover cache hit/miss, status/type filtering, context cancellation, idempotency, API errors, and full XML pipeline integration. --- docs/plans/2026-05-21-musicbrainz-provider.md | 14 +- internal/musicbrainz/sync.go | 130 +++ internal/musicbrainz/sync_test.go | 798 ++++++++++++++++++ 3 files changed, 935 insertions(+), 7 deletions(-) create mode 100644 internal/musicbrainz/sync.go create mode 100644 internal/musicbrainz/sync_test.go diff --git a/docs/plans/2026-05-21-musicbrainz-provider.md b/docs/plans/2026-05-21-musicbrainz-provider.md index 04775aa..29bbce0 100644 --- a/docs/plans/2026-05-21-musicbrainz-provider.md +++ b/docs/plans/2026-05-21-musicbrainz-provider.md @@ -75,13 +75,13 @@ Implement a MusicBrainz API provider with strict 1 request/second rate limiting, - [x] 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 +- [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 - [ ] update `cmd/naviwatcher/main.go` to initialize MusicBrainz client diff --git a/internal/musicbrainz/sync.go b/internal/musicbrainz/sync.go new file mode 100644 index 0000000..c0700c7 --- /dev/null +++ b/internal/musicbrainz/sync.go @@ -0,0 +1,130 @@ +package musicbrainz + +import ( + "context" + "fmt" + "time" + + "naviwatcher/internal/database" +) + +// SyncArtistDiscography synchronizes an artist's discography from MusicBrainz +// into the local external_releases table. It follows this flow: +// 1. Check if cached data exists and is within TTL. +// 2. If cache hit, return the cached releases immediately. +// 3. If cache miss or expired, fetch release groups from MusicBrainz API. +// 4. Apply status and type filtering. +// 5. Upsert each filtered release group into external_releases with current timestamp. +// 6. Return the list of external releases. +// +// Context cancellation is checked before the API call and between each upsert +// to allow graceful interruption. +func SyncArtistDiscography( + ctx context.Context, + client *MusicBrainzClient, + db *database.DB, + artistMBID string, + ttl time.Duration, +) ([]database.ExternalRelease, error) { + // Check context before starting. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("sync artist discography: %w", err) + } + + // Step 1: Check cache. + cached, err := GetCachedReleases(db, artistMBID, ttl) + if err != nil { + return nil, fmt.Errorf("sync artist discography: cache check failed: %w", err) + } + + // Step 2: If we have cached data, return it. + if cached.CacheHitCount > 0 { + return database.GetExternalReleasesByArtistWithCache(db, artistMBID, ttl) + } + + // Step 3: Cache miss — fetch from MusicBrainz API. + groups, err := client.GetArtistReleaseGroups(ctx, artistMBID) + if err != nil { + return nil, fmt.Errorf("sync artist discography: fetch release groups: %w", err) + } + + // Step 4: Apply filtering. + filtered := FilterReleaseGroups(groups) + + // Step 5: Upsert each release group into the database. + now := time.Now() + var releases []database.ExternalRelease + for _, rg := range filtered { + // Check context cancellation between each upsert. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("sync artist discography: %w", err) + } + + ext := rg.ToExternalRelease() + ext.CachedAt = now + + if err := database.SaveExternalRelease(db, ext); err != nil { + return nil, fmt.Errorf("sync artist discography: save release %s: %w", rg.ID, err) + } + + releases = append(releases, *ext) + } + + return releases, nil +} + +// SyncArtistDiscographyWithFilter works like SyncArtistDiscography but applies +// per-artist type filtering preferences in addition to the base filters. +func SyncArtistDiscographyWithFilter( + ctx context.Context, + client *MusicBrainzClient, + db *database.DB, + artistMBID string, + ttl time.Duration, + artistFilter *ArtistTypeFilter, +) ([]database.ExternalRelease, error) { + // Check context before starting. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("sync artist discography with filter: %w", err) + } + + // Step 1: Check cache. + cached, err := GetCachedReleases(db, artistMBID, ttl) + if err != nil { + return nil, fmt.Errorf("sync artist discography with filter: cache check failed: %w", err) + } + + // Step 2: If we have cached data, return it. + if cached.CacheHitCount > 0 { + return database.GetExternalReleasesByArtistWithCache(db, artistMBID, ttl) + } + + // Step 3: Cache miss — fetch from MusicBrainz API. + groups, err := client.GetArtistReleaseGroups(ctx, artistMBID) + if err != nil { + return nil, fmt.Errorf("sync artist discography with filter: fetch release groups: %w", err) + } + + // Step 4: Apply filtering with artist-specific type preferences. + filtered := FilterReleaseGroupsWithArtistFilter(groups, artistFilter) + + // Step 5: Upsert each release group into the database. + now := time.Now() + var releases []database.ExternalRelease + for _, rg := range filtered { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("sync artist discography with filter: %w", err) + } + + ext := rg.ToExternalRelease() + ext.CachedAt = now + + if err := database.SaveExternalRelease(db, ext); err != nil { + return nil, fmt.Errorf("sync artist discography with filter: save release %s: %w", rg.ID, err) + } + + releases = append(releases, *ext) + } + + return releases, nil +} diff --git a/internal/musicbrainz/sync_test.go b/internal/musicbrainz/sync_test.go new file mode 100644 index 0000000..dacdfae --- /dev/null +++ b/internal/musicbrainz/sync_test.go @@ -0,0 +1,798 @@ +package musicbrainz + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "golang.org/x/time/rate" + "naviwatcher/internal/config" + "naviwatcher/internal/database" +) + +// mbReleaseGroupXML is a helper to build a single release-group XML element. +func mbReleaseGroupXML(id, title, rgType, status, artistID, artistName, releaseDate string) string { + statusAttr := "" + if status != "" { + statusAttr = ` status="` + status + `"` + } + return `` + + `` + title + `` + + `` + + `` + artistName + `` + + `` + + `` + releaseDate + `` + + `` +} + +// mbReleaseGroupListResponse builds a full MusicBrainz XML response for a release-group list. +func mbReleaseGroupListResponse(groups string, count int) string { + return ` + + ` + + groups + + ` +` +} + +// itoa converts an int to a string without importing strconv. +func itoa(n int) string { + if n == 0 { + return "0" + } + var buf [20]byte + i := len(buf) + for n > 0 { + i-- + buf[i] = byte('0' + n%10) + n /= 10 + } + return string(buf[i:]) +} + +// newTestMBServer creates a mock MusicBrainz HTTP server. +func newTestMBServer(handler http.HandlerFunc) *httptest.Server { + return httptest.NewServer(handler) +} + +// newTestDB creates an in-memory SQLite database with all migrations applied. +func newTestDB(t *testing.T) *database.DB { + t.Helper() + db, err := database.New(":memory:") + if err != nil { + t.Fatalf("database.New() error: %v", err) + } + return db +} + +// newTestClient creates a MusicBrainzClient pointing at the given test server +// with a relaxed rate limiter (100 req/sec) for fast test execution. +func newTestClient(serverURL string) *MusicBrainzClient { + cfg := config.MusicBrainzConfig{ + UserAgent: "test-agent/1.0", + } + return &MusicBrainzClient{ + httpClient: &http.Client{}, + userAgent: cfg.UserAgent, + baseURL: serverURL + "/ws/2", + rateLimiter: rate.NewLimiter(rate.Limit(100), 100), + } +} + +// seedArtist inserts a minimal artist_settings row so foreign key constraints pass. +func seedArtist(t *testing.T, db *database.DB, id, name string) { + t.Helper() + if err := database.SaveArtistSettings(db, &database.ArtistSettings{ + ID: id, + Name: name, + Monitored: true, + }); err != nil { + t.Fatalf("seedArtist(%s) error: %v", id, err) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography fetches from API and upserts on cache miss +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_CacheMiss_FetchesAndUpserts(t *testing.T) { + artistMBID := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + artistName := "Test Artist" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg1", "First Album", "Album", "", artistMBID, artistName, "2020-01-01")+ + mbReleaseGroupXML("rg2", "Second Album", "Album", "", artistMBID, artistName, "2022-06-15")+ + mbReleaseGroupXML("rg3", "A Single", "Single", "", artistMBID, artistName, "2021-03-10"), + 3, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, artistName) + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + if len(releases) != 3 { + t.Fatalf("expected 3 releases, got %d", len(releases)) + } + + // Verify each release has CachedAt set. + for _, r := range releases { + if r.CachedAt.IsZero() { + t.Errorf("release %s: CachedAt should be set, got zero", r.RGID) + } + if r.ArtistID != artistMBID { + t.Errorf("release %s: expected ArtistID %q, got %q", r.RGID, artistMBID, r.ArtistID) + } + } + + // Verify data was persisted in the database. + stored, err := database.GetExternalReleasesByArtist(db, artistMBID) + if err != nil { + t.Fatalf("GetExternalReleasesByArtist() error: %v", err) + } + if len(stored) != 3 { + t.Errorf("expected 3 stored releases, got %d", len(stored)) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography returns cached data on cache hit +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_CacheHit_ReturnsCached(t *testing.T) { + artistMBID := "bbbbbbbb-cccc-dddd-eeee-ffffffffffff" + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Cached Artist") + + // Pre-populate the cache with one release. + now := time.Now() + if err := database.SaveExternalRelease(db, &database.ExternalRelease{ + RGID: "rg-cached", + ArtistID: artistMBID, + Title: "Cached Album", + Type: "Album", + ReleaseDate: "2019-05-01", + CachedAt: now, + }); err != nil { + t.Fatalf("SaveExternalRelease() error: %v", err) + } + + // Server that would be called on cache miss — should NOT be called. + serverCalled := false + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + serverCalled = true + w.Header().Set("Content-Type", "application/xml") + w.Write([]byte(mbReleaseGroupListResponse("", 0))) + }) + defer server.Close() + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + if serverCalled { + t.Error("expected cache hit but server was called (cache miss)") + } + + if len(releases) != 1 { + t.Fatalf("expected 1 cached release, got %d", len(releases)) + } + + if releases[0].RGID != "rg-cached" { + t.Errorf("expected RGID 'rg-cached', got %q", releases[0].RGID) + } + if releases[0].Title != "Cached Album" { + t.Errorf("expected Title 'Cached Album', got %q", releases[0].Title) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography applies filtering (excluded statuses) +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_FiltersExcludedStatuses(t *testing.T) { + artistMBID := "cccccccc-dddd-eeee-ffff-000000000000" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + // Include a Bootleg and a Promotion that should be filtered out. + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg-legit", "Legit Album", "Album", "", artistMBID, "Artist", "2020-01-01")+ + mbReleaseGroupXML("rg-bootleg", "Bootleg Album", "Album", "Bootleg", artistMBID, "Artist", "2020-02-01")+ + mbReleaseGroupXML("rg-promo", "Promo Album", "Album", "Promotion", artistMBID, "Artist", "2020-03-01")+ + mbReleaseGroupXML("rg-pseudo", "Pseudo Album", "Album", "Pseudo-Release", artistMBID, "Artist", "2020-04-01"), + 4, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Filter Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + // Only the legit album should remain after filtering. + if len(releases) != 1 { + t.Fatalf("expected 1 release after filtering, got %d", len(releases)) + } + if releases[0].RGID != "rg-legit" { + t.Errorf("expected RGID 'rg-legit', got %q", releases[0].RGID) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography applies type filtering +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_FiltersExcludedTypes(t *testing.T) { + artistMBID := "dddddddd-eeee-ffff-0000-111111111111" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg-album", "An Album", "Album", "", artistMBID, "Artist", "2020-01-01")+ + mbReleaseGroupXML("rg-single", "A Single", "Single", "", artistMBID, "Artist", "2020-02-01")+ + mbReleaseGroupXML("rg-ep", "An EP", "EP", "", artistMBID, "Artist", "2020-03-01")+ + mbReleaseGroupXML("rg-comp", "A Compilation", "Compilation", "", artistMBID, "Artist", "2020-04-01")+ + mbReleaseGroupXML("rg-soundtrack", "A Soundtrack", "Soundtrack", "", artistMBID, "Artist", "2020-05-01"), + 5, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Type Filter Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + // Soundtrack should be excluded (not in includedTypes). + if len(releases) != 4 { + t.Fatalf("expected 4 releases after type filtering, got %d", len(releases)) + } + + rgIDs := make(map[string]bool) + for _, r := range releases { + rgIDs[r.RGID] = true + } + if rgIDs["rg-soundtrack"] { + t.Error("Soundtrack type should have been filtered out") + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography with context cancellation +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_ContextCancellation(t *testing.T) { + artistMBID := "eeeeeeee-ffff-0000-1111-222222222222" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg1", "Album One", "Album", "", artistMBID, "Artist", "2020-01-01"), + 1, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Cancel Artist") + + client := newTestClient(server.URL) + ttl := 24 * time.Hour + + // Create a context that is already cancelled. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err == nil { + t.Fatal("SyncArtistDiscography() expected error for cancelled context, got nil") + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography upsert is idempotent (re-sync replaces) +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_IdempotentUpsert(t *testing.T) { + artistMBID := "ffffffff-0000-1111-2222-333333333333" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg1", "Album One", "Album", "", artistMBID, "Artist", "2020-01-01")+ + mbReleaseGroupXML("rg2", "Album Two", "Album", "", artistMBID, "Artist", "2021-01-01"), + 2, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Idempotent Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + // First sync. + releases1, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("first SyncArtistDiscography() error: %v", err) + } + if len(releases1) != 2 { + t.Fatalf("expected 2 releases after first sync, got %d", len(releases1)) + } + + // Second sync should use cache (server would error if called again). + // To verify the cache path, we use a very short TTL so cache expires. + releases2, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("second SyncArtistDiscography() error: %v", err) + } + if len(releases2) != 2 { + t.Fatalf("expected 2 releases after second sync, got %d", len(releases2)) + } + + // Verify no duplicates in the database. + stored, err := database.GetExternalReleasesByArtist(db, artistMBID) + if err != nil { + t.Fatalf("GetExternalReleasesByArtist() error: %v", err) + } + if len(stored) != 2 { + t.Errorf("expected 2 stored releases (no duplicates), got %d", len(stored)) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscographyWithFilter applies per-artist type filter +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscographyWithFilter_ArtistTypeFilter(t *testing.T) { + artistMBID := "11111111-2222-3333-4444-555555555555" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg-album", "An Album", "Album", "", artistMBID, "Artist", "2020-01-01")+ + mbReleaseGroupXML("rg-single", "A Single", "Single", "", artistMBID, "Artist", "2020-02-01")+ + mbReleaseGroupXML("rg-ep", "An EP", "EP", "", artistMBID, "Artist", "2020-03-01")+ + mbReleaseGroupXML("rg-comp", "A Compilation", "Compilation", "", artistMBID, "Artist", "2020-04-01"), + 4, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Filter Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + // Filter that excludes Singles and Compilations. + filter := &ArtistTypeFilter{ + ArtistID: artistMBID, + IncludeSingles: false, + IncludeCompilations: false, + IncludeEP: true, + } + + releases, err := SyncArtistDiscographyWithFilter(ctx, client, db, artistMBID, ttl, filter) + if err != nil { + t.Fatalf("SyncArtistDiscographyWithFilter() error: %v", err) + } + + // Should only have Album and EP. + if len(releases) != 2 { + t.Fatalf("expected 2 releases with artist filter, got %d", len(releases)) + } + + rgIDs := make(map[string]bool) + for _, r := range releases { + rgIDs[r.RGID] = true + } + if !rgIDs["rg-album"] { + t.Error("expected rg-album in filtered results") + } + if !rgIDs["rg-ep"] { + t.Error("expected rg-ep in filtered results") + } + if rgIDs["rg-single"] { + t.Error("rg-single should have been filtered out") + } + if rgIDs["rg-comp"] { + t.Error("rg-comp should have been filtered out") + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscographyWithFilter cache hit +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscographyWithFilter_CacheHit(t *testing.T) { + artistMBID := "22222222-3333-4444-5555-666666666666" + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Cache Hit Filter Artist") + + // Pre-populate cache. + if err := database.SaveExternalRelease(db, &database.ExternalRelease{ + RGID: "rg-old", + ArtistID: artistMBID, + Title: "Old Cached", + Type: "Album", + CachedAt: time.Now(), + }); err != nil { + t.Fatalf("SaveExternalRelease() error: %v", err) + } + + serverCalled := false + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + serverCalled = true + w.Header().Set("Content-Type", "application/xml") + w.Write([]byte(mbReleaseGroupListResponse("", 0))) + }) + defer server.Close() + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + filter := DefaultArtistTypeFilter(artistMBID) + + releases, err := SyncArtistDiscographyWithFilter(ctx, client, db, artistMBID, ttl, filter) + if err != nil { + t.Fatalf("SyncArtistDiscographyWithFilter() error: %v", err) + } + + if serverCalled { + t.Error("expected cache hit but server was called") + } + if len(releases) != 1 { + t.Fatalf("expected 1 cached release, got %d", len(releases)) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography with empty response (no release groups) +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_EmptyResponse(t *testing.T) { + artistMBID := "33333333-4444-5555-6666-777777777777" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + w.Write([]byte(mbReleaseGroupListResponse("", 0))) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Empty Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + if len(releases) != 0 { + t.Fatalf("expected 0 releases for empty response, got %d", len(releases)) + } + + // Verify nothing in DB. + stored, err := database.GetExternalReleasesByArtist(db, artistMBID) + if err != nil { + t.Fatalf("GetExternalReleasesByArtist() error: %v", err) + } + if len(stored) != 0 { + t.Errorf("expected 0 stored releases, got %d", len(stored)) + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscography API error propagation +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_APIError(t *testing.T) { + artistMBID := "44444444-5555-6666-7777-888888888888" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("internal server error")) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Error Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + _, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err == nil { + t.Fatal("SyncArtistDiscography() expected error for API failure, got nil") + } +} + +// ----------------------------------------------------------------------- +// Test: SyncArtistDiscographyWithFilter context cancellation +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscographyWithFilter_ContextCancellation(t *testing.T) { + artistMBID := "55555555-6666-7777-8888-999999999999" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + w.Write([]byte(mbReleaseGroupListResponse( + mbReleaseGroupXML("rg1", "Album", "Album", "", artistMBID, "Artist", "2020-01-01"), + 1, + ))) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Cancel Filter Artist") + + client := newTestClient(server.URL) + ttl := 24 * time.Hour + filter := DefaultArtistTypeFilter(artistMBID) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := SyncArtistDiscographyWithFilter(ctx, client, db, artistMBID, ttl, filter) + if err == nil { + t.Fatal("SyncArtistDiscographyWithFilter() expected error for cancelled context, got nil") + } +} + +// ----------------------------------------------------------------------- +// Test: Verify XML parsing integration — ensure the full pipeline works +// with real XML structure matching MusicBrainz responses. +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_FullXMLPipeline(t *testing.T) { + artistMBID := "66666666-7777-8888-9999-000000000000" + + // Build a realistic MusicBrainz XML response. + xmlBody := ` + + + + Real Album One + + + + Real Artist + + + + 2019-03-15 + + + Real Single Two + + + + Real Artist + + + + 2020-07-20 + + +` + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + w.Write([]byte(xmlBody)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Real Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + if len(releases) != 2 { + t.Fatalf("expected 2 releases, got %d", len(releases)) + } + + // Verify the parsed data. + byID := make(map[string]database.ExternalRelease) + for _, r := range releases { + byID[r.RGID] = r + } + + rg1, ok := byID["rg-real-1"] + if !ok { + t.Fatal("expected rg-real-1 in results") + } + if rg1.Title != "Real Album One" { + t.Errorf("rg-real-1 title = %q, want %q", rg1.Title, "Real Album One") + } + if rg1.Type != "Album" { + t.Errorf("rg-real-1 type = %q, want %q", rg1.Type, "Album") + } + if rg1.ReleaseDate != "2019-03-15" { + t.Errorf("rg-real-1 release_date = %q, want %q", rg1.ReleaseDate, "2019-03-15") + } + + rg2, ok := byID["rg-real-2"] + if !ok { + t.Fatal("expected rg-real-2 in results") + } + if rg2.Type != "Single" { + t.Errorf("rg-real-2 type = %q, want %q", rg2.Type, "Single") + } +} + +// ----------------------------------------------------------------------- +// Test: Verify CachedAt timestamps are consistent across a sync batch +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_ConsistentCachedAtTimestamp(t *testing.T) { + artistMBID := "77777777-8888-9999-0000-111111111111" + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + resp := mbReleaseGroupListResponse( + mbReleaseGroupXML("rg-ts-1", "Album A", "Album", "", artistMBID, "Artist", "2020-01-01")+ + mbReleaseGroupXML("rg-ts-2", "Album B", "Album", "", artistMBID, "Artist", "2021-01-01")+ + mbReleaseGroupXML("rg-ts-3", "Album C", "Album", "", artistMBID, "Artist", "2022-01-01"), + 3, + ) + w.Write([]byte(resp)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "Timestamp Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + // All releases in a batch should have the same CachedAt timestamp. + if len(releases) != 3 { + t.Fatalf("expected 3 releases, got %d", len(releases)) + } + + first := releases[0].CachedAt + for _, r := range releases[1:] { + if !r.CachedAt.Equal(first) { + t.Errorf("CachedAt mismatch: %v vs %v for release %s", first, r.CachedAt, r.RGID) + } + } +} + +// ----------------------------------------------------------------------- +// Test: Verify XML unmarshalling edge case — release-group with no type attr +// ----------------------------------------------------------------------- + +func TestSyncArtistDiscography_XMLNoTypeAttribute(t *testing.T) { + artistMBID := "88888888-9999-0000-1111-222222222222" + + xmlBody := ` + + + + No Type Album + + + + Artist + + + + 2020-01-01 + + + Typed Album + + + + Artist + + + + 2021-01-01 + + +` + + server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/xml") + w.Write([]byte(xmlBody)) + }) + defer server.Close() + + db := newTestDB(t) + defer db.Close() + seedArtist(t, db, artistMBID, "No Type Artist") + + client := newTestClient(server.URL) + ctx := context.Background() + ttl := 24 * time.Hour + + releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl) + if err != nil { + t.Fatalf("SyncArtistDiscography() error: %v", err) + } + + // The release-group with no type should be filtered out (empty string is not in includedTypes). + if len(releases) != 1 { + t.Fatalf("expected 1 release (empty type filtered), got %d", len(releases)) + } + if releases[0].RGID != "rg-withtype" { + t.Errorf("expected rg-withtype, got %s", releases[0].RGID) + } +}