diff --git a/cmd/naviwatcher/main.go b/cmd/naviwatcher/main.go index d153b34..14e1f35 100644 --- a/cmd/naviwatcher/main.go +++ b/cmd/naviwatcher/main.go @@ -16,9 +16,9 @@ import ( // App holds all application dependencies for clean shutdown and testability. type App struct { - cfg *config.Config - db *database.DB - mbClient *musicbrainz.MusicBrainzClient + cfg *config.Config + db *database.DB + mbClient *musicbrainz.MusicBrainzClient } const defaultConfigPath = "config.yaml" diff --git a/internal/musicbrainz/api.go b/internal/musicbrainz/api.go index 0658ef2..7aa0327 100644 --- a/internal/musicbrainz/api.go +++ b/internal/musicbrainz/api.go @@ -60,6 +60,12 @@ func (c *MusicBrainzClient) GetArtistReleaseGroups(ctx context.Context, artistMB if offset+len(parsed.ReleaseGroups) >= parsed.Count { break } + + // Check context cancellation between pages for responsive shutdown. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("fetch release groups for artist %s: %w", artistMBID, err) + } + offset += limit } diff --git a/internal/musicbrainz/cache.go b/internal/musicbrainz/cache.go index 2cfa444..55e0b16 100644 --- a/internal/musicbrainz/cache.go +++ b/internal/musicbrainz/cache.go @@ -9,11 +9,11 @@ import ( // GetCachedReleases queries the external_releases table for entries // belonging to the given artist that were cached within the specified TTL. -// It returns the count of cached entries and any error encountered. -func GetCachedReleases(db *database.DB, artistID string, ttl time.Duration) (int, error) { +// It returns the cached releases and any error encountered. +func GetCachedReleases(db *database.DB, artistID string, ttl time.Duration) ([]database.ExternalRelease, error) { releases, err := database.GetExternalReleasesByArtistWithCache(db, artistID, ttl) if err != nil { - return 0, fmt.Errorf("get cached releases: %w", err) + return nil, fmt.Errorf("get cached releases: %w", err) } - return len(releases), nil + return releases, nil } diff --git a/internal/musicbrainz/cache_test.go b/internal/musicbrainz/cache_test.go index efd02b2..f61040b 100644 --- a/internal/musicbrainz/cache_test.go +++ b/internal/musicbrainz/cache_test.go @@ -45,13 +45,13 @@ func TestGetCachedReleases_CacheHit(t *testing.T) { } ttl := 24 * time.Hour - count, err := GetCachedReleases(db, artistID, ttl) + releases, err := GetCachedReleases(db, artistID, ttl) if err != nil { t.Fatalf("GetCachedReleases() error: %v", err) } - if count != 2 { - t.Errorf("GetCachedReleases() = %d, want 2", count) + if len(releases) != 2 { + t.Errorf("GetCachedReleases() returned %d releases, want 2", len(releases)) } } @@ -78,13 +78,13 @@ func TestGetCachedReleases_CacheMiss_Expired(t *testing.T) { } ttl := 24 * time.Hour - count, err := GetCachedReleases(db, artistID, ttl) + releases, err := GetCachedReleases(db, artistID, ttl) if err != nil { t.Fatalf("GetCachedReleases() error: %v", err) } - if count != 0 { - t.Errorf("GetCachedReleases() = %d, want 0 (expired entry should not be cached)", count) + if len(releases) != 0 { + t.Errorf("GetCachedReleases() returned %d releases, want 0 (expired entry should not be cached)", len(releases)) } } @@ -110,13 +110,13 @@ func TestGetCachedReleases_CacheMiss_NoCachedAt(t *testing.T) { } ttl := 24 * time.Hour - count, err := GetCachedReleases(db, artistID, ttl) + releases, err := GetCachedReleases(db, artistID, ttl) if err != nil { t.Fatalf("GetCachedReleases() error: %v", err) } - if count != 0 { - t.Errorf("GetCachedReleases() = %d, want 0 (NULL cached_at should not be cached)", count) + if len(releases) != 0 { + t.Errorf("GetCachedReleases() returned %d releases, want 0 (NULL cached_at should not be cached)", len(releases)) } } @@ -128,13 +128,13 @@ func TestGetCachedReleases_EmptyArtist(t *testing.T) { defer db.Close() ttl := 24 * time.Hour - count, err := GetCachedReleases(db, "nonexistent-artist", ttl) + releases, err := GetCachedReleases(db, "nonexistent-artist", ttl) if err != nil { t.Fatalf("GetCachedReleases() error: %v", err) } - if count != 0 { - t.Errorf("GetCachedReleases() = %d, want 0 for nonexistent artist", count) + if len(releases) != 0 { + t.Errorf("GetCachedReleases() returned %d releases, want 0 for nonexistent artist", len(releases)) } } @@ -170,12 +170,15 @@ func TestGetCachedReleases_MixedExpiry(t *testing.T) { } ttl := 24 * time.Hour - count, err := GetCachedReleases(db, artistID, ttl) + releases, err := GetCachedReleases(db, artistID, ttl) if err != nil { t.Fatalf("GetCachedReleases() error: %v", err) } - if count != 1 { - t.Errorf("GetCachedReleases() = %d, want 1 (only fresh entry)", count) + if len(releases) != 1 { + t.Errorf("GetCachedReleases() returned %d releases, want 1 (only fresh entry)", len(releases)) + } + if len(releases) > 0 && releases[0].RGID != "rg-fresh" { + t.Errorf("expected rg-fresh, got %s", releases[0].RGID) } } diff --git a/internal/musicbrainz/sync.go b/internal/musicbrainz/sync.go index a65921c..d9f3b85 100644 --- a/internal/musicbrainz/sync.go +++ b/internal/musicbrainz/sync.go @@ -32,17 +32,17 @@ func SyncArtistDiscography( } // Step 1: Check cache. - cachedCount, err := GetCachedReleases(db, artistMBID, ttl) + cachedReleases, 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 cachedCount > 0 { + if len(cachedReleases) > 0 { if err := ctx.Err(); err != nil { return nil, fmt.Errorf("sync artist discography: %w", err) } - return database.GetExternalReleasesByArtistWithCache(db, artistMBID, ttl) + return cachedReleases, nil } // Step 3: Cache miss — fetch from MusicBrainz API.