diff --git a/cmd/server/noop_repo.go b/cmd/server/noop_repo.go index 86edca3..b78cf47 100644 --- a/cmd/server/noop_repo.go +++ b/cmd/server/noop_repo.go @@ -39,10 +39,18 @@ func (r *noopAlbumRepo) GetByArtistID(ctx context.Context, artistID string, limi return &domain.SearchResult[domain.Album]{}, nil } +func (r *noopAlbumRepo) GetAllByArtistID(ctx context.Context, artistID string) ([]domain.Album, error) { + return nil, nil +} + func (r *noopAlbumRepo) Save(ctx context.Context, album *domain.Album) error { return nil } +func (r *noopAlbumRepo) SaveAll(ctx context.Context, albums []domain.Album) error { + return nil +} + type noopTrackRepo struct{} func (r *noopTrackRepo) GetByID(ctx context.Context, id string) (*domain.Track, error) { diff --git a/database/migrations/001_extensions.down.sql b/database/migrations/001_extensions.down.sql deleted file mode 100644 index 87d8cd5..0000000 --- a/database/migrations/001_extensions.down.sql +++ /dev/null @@ -1 +0,0 @@ -DROP EXTENSION IF EXISTS pg_prewarm; diff --git a/database/migrations/001_extensions.up.sql b/database/migrations/001_extensions.up.sql deleted file mode 100644 index f08f11a..0000000 --- a/database/migrations/001_extensions.up.sql +++ /dev/null @@ -1 +0,0 @@ -CREATE EXTENSION IF NOT EXISTS pg_prewarm; diff --git a/database/migrations/002_initial_schema.down.sql b/database/migrations/002_initial_schema.down.sql deleted file mode 100644 index 548806d..0000000 --- a/database/migrations/002_initial_schema.down.sql +++ /dev/null @@ -1,33 +0,0 @@ -DROP INDEX IF EXISTS idx_playlist_tracks_position; -DROP INDEX IF EXISTS idx_lyrics_track_id; -DROP INDEX IF EXISTS idx_genres_name; -DROP INDEX IF EXISTS idx_albums_release_date; -DROP INDEX IF EXISTS idx_albums_source; -DROP INDEX IF EXISTS idx_albums_upc; -DROP INDEX IF EXISTS idx_tracks_source; -DROP INDEX IF EXISTS idx_tracks_isrc; -DROP INDEX IF EXISTS idx_artists_source; -DROP INDEX IF EXISTS idx_artists_name; - -DROP TABLE IF EXISTS track_external_ids; -DROP TABLE IF EXISTS album_external_ids; -DROP TABLE IF EXISTS artist_external_ids; - -DROP TABLE IF EXISTS playlist_tracks; -DROP TABLE IF EXISTS playlists; -DROP TABLE IF EXISTS lyrics; - -DROP TABLE IF EXISTS similar_artists; -DROP TABLE IF EXISTS album_genres; -DROP TABLE IF EXISTS artist_genres; -DROP TABLE IF EXISTS work_artists; -DROP TABLE IF EXISTS album_tracks; -DROP TABLE IF EXISTS album_artists; -DROP TABLE IF EXISTS track_artists; - -DROP TABLE IF EXISTS genres; -DROP TABLE IF EXISTS albums; -DROP TABLE IF EXISTS labels; -DROP TABLE IF EXISTS tracks; -DROP TABLE IF EXISTS works; -DROP TABLE IF EXISTS artists; diff --git a/database/migrations/002_initial_schema.up.sql b/database/schema.sql similarity index 98% rename from database/migrations/002_initial_schema.up.sql rename to database/schema.sql index 4b47ada..d88553b 100644 --- a/database/migrations/002_initial_schema.up.sql +++ b/database/schema.sql @@ -1,4 +1,4 @@ --- Core Entities +CREATE EXTENSION IF NOT EXISTS pg_prewarm; CREATE TABLE artists ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), @@ -56,6 +56,7 @@ CREATE TABLE albums ( label_id UUID REFERENCES labels(id), title TEXT NOT NULL, album_type TEXT, + secondary_types TEXT[] DEFAULT '{}', release_date DATE, upc TEXT, total_tracks INT, @@ -73,8 +74,6 @@ CREATE TABLE genres ( parent_id UUID REFERENCES genres(id) ); --- Relationships - CREATE TABLE track_artists ( track_id UUID REFERENCES tracks(id) ON DELETE CASCADE, artist_id UUID REFERENCES artists(id) ON DELETE CASCADE, @@ -125,8 +124,6 @@ CREATE TABLE similar_artists ( PRIMARY KEY (artist_id, similar_artist_id) ); --- Content - CREATE TABLE lyrics ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), track_id UUID REFERENCES tracks(id) ON DELETE CASCADE, @@ -156,8 +153,6 @@ CREATE TABLE playlist_tracks ( PRIMARY KEY (playlist_id, track_id) ); --- External IDs - CREATE TABLE artist_external_ids ( artist_id UUID REFERENCES artists(id) ON DELETE CASCADE, source TEXT NOT NULL, @@ -185,8 +180,6 @@ CREATE TABLE track_external_ids ( PRIMARY KEY (track_id, source, source_id) ); --- Indexes - CREATE INDEX idx_artists_name ON artists(name); CREATE INDEX idx_artists_source ON artists(source, source_id); CREATE INDEX idx_tracks_isrc ON tracks(isrc) WHERE isrc IS NOT NULL; diff --git a/internal/domain/types.go b/internal/domain/types.go index 47a23cb..faa2158 100644 --- a/internal/domain/types.go +++ b/internal/domain/types.go @@ -17,18 +17,19 @@ type Artist struct { } type Album struct { - ID string - Title string - Type string - ReleaseDate *time.Time - UPC string - TotalTracks int - TotalDiscs int - CoverURL string - Artists []ArtistCredit - Label *Label - Genres []Genre - ExternalIDs []ExternalID + ID string + Title string + Type string + SecondaryTypes []string + ReleaseDate *time.Time + UPC string + TotalTracks int + TotalDiscs int + CoverURL string + Artists []ArtistCredit + Label *Label + Genres []Genre + ExternalIDs []ExternalID } type Track struct { diff --git a/internal/provider/musicbrainz/client.go b/internal/provider/musicbrainz/client.go index 867ba18..47b3366 100644 --- a/internal/provider/musicbrainz/client.go +++ b/internal/provider/musicbrainz/client.go @@ -107,6 +107,12 @@ func (c *client) readResponse(ctx context.Context, resp *http.Response, endpoint metrics.ProviderRequests.WithLabelValues("musicbrainz", endpoint, "ok").Inc() return io.ReadAll(resp.Body) + case http.StatusBadRequest: + body, _ := io.ReadAll(resp.Body) + log.Warn().Str("endpoint", endpoint).Str("body", string(body)).Dur("duration", duration).Msg("HTTP 400 bad request") + metrics.ProviderRequests.WithLabelValues("musicbrainz", endpoint, "bad_request").Inc() + return nil, fmt.Errorf("%w: %s", ErrBadRequest, extractErrorMessage(body)) + case http.StatusNotFound: log.Debug().Str("endpoint", endpoint).Dur("duration", duration).Msg("HTTP 404") metrics.ProviderRequests.WithLabelValues("musicbrainz", endpoint, "not_found").Inc() @@ -137,17 +143,12 @@ func (c *client) lookup(ctx context.Context, entity, id string, inc []string) ([ } func (c *client) browse(ctx context.Context, entity, linkedEntity, linkedID string, limit, offset int, inc []string) ([]byte, error) { - return c.browseWithTypes(ctx, entity, linkedEntity, linkedID, limit, offset, inc, nil) -} - -func (c *client) browseWithTypes(ctx context.Context, entity, linkedEntity, linkedID string, limit, offset int, inc []string, types []string) ([]byte, error) { zerolog.Ctx(ctx).Debug(). Str("entity", entity). Str("linked_entity", linkedEntity). Str("linked_id", linkedID). Int("limit", limit). Int("offset", offset). - Strs("types", types). Msg("provider browse") params := url.Values{} @@ -159,10 +160,6 @@ func (c *client) browseWithTypes(ctx context.Context, entity, linkedEntity, link params.Set("inc", joined) } - if typeStr := strings.Join(types, "|"); typeStr != "" { - params.Set("type", typeStr) - } - return c.get(ctx, entity, params) } @@ -184,6 +181,16 @@ func joinIncludes(inc []string) string { return strings.Join(inc, "+") } +func extractErrorMessage(body []byte) string { + var errResp struct { + Error string `json:"error"` + } + if json.Unmarshal(body, &errResp) == nil && errResp.Error != "" { + return errResp.Error + } + return string(body) +} + func decode[T any](data []byte) (*T, error) { var result T if err := json.Unmarshal(data, &result); err != nil { diff --git a/internal/provider/musicbrainz/errors.go b/internal/provider/musicbrainz/errors.go index 09eadbb..3ba9fc6 100644 --- a/internal/provider/musicbrainz/errors.go +++ b/internal/provider/musicbrainz/errors.go @@ -4,5 +4,6 @@ import "errors" var ( ErrNotFound = errors.New("not found") + ErrBadRequest = errors.New("bad request") ErrRateLimited = errors.New("rate limited") ) diff --git a/internal/provider/musicbrainz/mapper.go b/internal/provider/musicbrainz/mapper.go index 29f7ef7..45af550 100644 --- a/internal/provider/musicbrainz/mapper.go +++ b/internal/provider/musicbrainz/mapper.go @@ -61,9 +61,10 @@ func mapAlbum(mb *mbReleaseGroup, release *mbRelease) *domain.Album { } album := &domain.Album{ - ID: mb.ID, - Title: mb.Title, - Type: mb.PrimaryType, + ID: mb.ID, + Title: mb.Title, + Type: mb.PrimaryType, + SecondaryTypes: mb.SecondaryTypes, ExternalIDs: []domain.ExternalID{{ Source: "musicbrainz", SourceID: mb.ID, diff --git a/internal/provider/musicbrainz/provider.go b/internal/provider/musicbrainz/provider.go index a0a86ac..490a987 100644 --- a/internal/provider/musicbrainz/provider.go +++ b/internal/provider/musicbrainz/provider.go @@ -91,7 +91,7 @@ func (p *Provider) GetAlbum(ctx context.Context, id string) (*domain.Album, erro return mapAlbum(mb, release), nil } -func (p *Provider) SearchAlbums(ctx context.Context, query string, artist string, limit, offset int, albumTypes []string) (*domain.SearchResult[domain.Album], error) { +func (p *Provider) SearchAlbums(ctx context.Context, query string, artist string, limit, offset int) (*domain.SearchResult[domain.Album], error) { if limit <= 0 || limit > 100 { limit = 25 } @@ -105,10 +105,6 @@ func (p *Provider) SearchAlbums(ctx context.Context, query string, artist string luceneQuery = fmt.Sprintf("releasegroup:%s", escapeQuery(query)) } - if typeFilter := buildTypeFilter(albumTypes); typeFilter != "" { - luceneQuery += " AND " + typeFilter - } - data, err := p.client.search(ctx, "release-group", luceneQuery, limit, offset) if err != nil { return nil, fmt.Errorf("search albums: %w", err) @@ -138,38 +134,38 @@ func (p *Provider) SearchAlbums(ctx context.Context, query string, artist string return result, nil } -func (p *Provider) GetArtistAlbums(ctx context.Context, artistID string, limit, offset int, albumTypes []string) (*domain.SearchResult[domain.Album], error) { - if limit <= 0 || limit > 100 { - limit = 25 - } +func (p *Provider) GetArtistAlbums(ctx context.Context, artistID string) ([]domain.Album, error) { + var allAlbums []domain.Album + offset := 0 + batchSize := 100 - data, err := p.client.browseWithTypes(ctx, "release-group", "artist", artistID, limit, offset, []string{"artist-credits"}, albumTypes) - if err != nil { - return nil, fmt.Errorf("browse release-groups: %w", err) - } + for { + data, err := p.client.browse(ctx, "release-group", "artist", artistID, batchSize, offset, []string{"artist-credits"}) + if err != nil { + return nil, fmt.Errorf("browse release-groups: %w", err) + } - var resp struct { - ReleaseGroupCount int `json:"release-group-count"` - ReleaseGroupOffset int `json:"release-group-offset"` - ReleaseGroups []*mbReleaseGroup `json:"release-groups"` - } - if err := decodeInto(data, &resp); err != nil { - return nil, err - } + var resp struct { + ReleaseGroupCount int `json:"release-group-count"` + ReleaseGroups []*mbReleaseGroup `json:"release-groups"` + } + if err := decodeInto(data, &resp); err != nil { + return nil, err + } - result := &domain.SearchResult[domain.Album]{ - Total: resp.ReleaseGroupCount, - Limit: limit, - Offset: resp.ReleaseGroupOffset, - } + for _, mb := range resp.ReleaseGroups { + if album := mapAlbum(mb, nil); album != nil { + allAlbums = append(allAlbums, *album) + } + } - for _, mb := range resp.ReleaseGroups { - if album := mapAlbum(mb, nil); album != nil { - result.Items = append(result.Items, *album) + offset += len(resp.ReleaseGroups) + if offset >= resp.ReleaseGroupCount || len(resp.ReleaseGroups) == 0 { + break } } - return result, nil + return allAlbums, nil } func (p *Provider) GetTrack(ctx context.Context, id string) (*domain.Track, error) { @@ -315,20 +311,6 @@ func selectCanonicalRelease(releases []*mbRelease) *mbRelease { return best } -func buildTypeFilter(types []string) string { - if len(types) == 0 { - return "" - } - if len(types) == 1 { - return fmt.Sprintf("primarytype:%s", types[0]) - } - escaped := make([]string, len(types)) - for i, t := range types { - escaped[i] = fmt.Sprintf("primarytype:%s", t) - } - return "(" + strings.Join(escaped, " OR ") + ")" -} - func escapeQuery(s string) string { special := []string{`+`, `-`, `&`, `|`, `!`, `(`, `)`, `{`, `}`, `[`, `]`, `^`, `"`, `~`, `*`, `?`, `:`, `/`, `\`} result := s diff --git a/internal/provider/musicbrainz/types.go b/internal/provider/musicbrainz/types.go index a07b5be..6d8e3a0 100644 --- a/internal/provider/musicbrainz/types.go +++ b/internal/provider/musicbrainz/types.go @@ -1,15 +1,15 @@ package musicbrainz type mbArtist struct { - ID string `json:"id"` - Name string `json:"name"` - SortName string `json:"sort-name"` - Type string `json:"type"` - Country string `json:"country"` - Disambiguation string `json:"disambiguation"` - LifeSpan mbLifeSpan `json:"life-span"` - Genres []mbGenre `json:"genres"` - Relations []mbRelation `json:"relations"` + ID string `json:"id"` + Name string `json:"name"` + SortName string `json:"sort-name"` + Type string `json:"type"` + Country string `json:"country"` + Disambiguation string `json:"disambiguation"` + LifeSpan mbLifeSpan `json:"life-span"` + Genres []mbGenre `json:"genres"` + Relations []mbRelation `json:"relations"` } type mbLifeSpan struct { @@ -22,6 +22,7 @@ type mbReleaseGroup struct { ID string `json:"id"` Title string `json:"title"` PrimaryType string `json:"primary-type"` + SecondaryTypes []string `json:"secondary-types"` FirstReleaseDate string `json:"first-release-date"` ArtistCredit []mbArtistCredit `json:"artist-credit"` Genres []mbGenre `json:"genres"` @@ -29,16 +30,16 @@ type mbReleaseGroup struct { } type mbRelease struct { - ID string `json:"id"` - Title string `json:"title"` - Status string `json:"status"` - Date string `json:"date"` - Country string `json:"country"` - Barcode string `json:"barcode"` - LabelInfo []mbLabelInfo `json:"label-info"` - Media []mbMedium `json:"media"` - ReleaseGroup *mbReleaseGroup `json:"release-group"` - ArtistCredit []mbArtistCredit `json:"artist-credit"` + ID string `json:"id"` + Title string `json:"title"` + Status string `json:"status"` + Date string `json:"date"` + Country string `json:"country"` + Barcode string `json:"barcode"` + LabelInfo []mbLabelInfo `json:"label-info"` + Media []mbMedium `json:"media"` + ReleaseGroup *mbReleaseGroup `json:"release-group"` + ArtistCredit []mbArtistCredit `json:"artist-credit"` CoverArtArchive mbCoverArtArchive `json:"cover-art-archive"` } @@ -60,10 +61,10 @@ type mbLabel struct { } type mbMedium struct { - Position int `json:"position"` - Format string `json:"format"` - TrackCount int `json:"track-count"` - Tracks []mbTrack `json:"tracks"` + Position int `json:"position"` + Format string `json:"format"` + TrackCount int `json:"track-count"` + Tracks []mbTrack `json:"tracks"` } type mbTrack struct { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index b941d63..eb46b6a 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -13,8 +13,8 @@ type Provider interface { SearchArtists(ctx context.Context, query string, limit, offset int) (*domain.SearchResult[domain.Artist], error) GetAlbum(ctx context.Context, id string) (*domain.Album, error) - SearchAlbums(ctx context.Context, query string, artist string, limit, offset int, albumTypes []string) (*domain.SearchResult[domain.Album], error) - GetArtistAlbums(ctx context.Context, artistID string, limit, offset int, albumTypes []string) (*domain.SearchResult[domain.Album], error) + SearchAlbums(ctx context.Context, query string, artist string, limit, offset int) (*domain.SearchResult[domain.Album], error) + GetArtistAlbums(ctx context.Context, artistID string) ([]domain.Album, error) GetTrack(ctx context.Context, id string) (*domain.Track, error) GetAlbumTracks(ctx context.Context, albumID string) ([]domain.Track, error) diff --git a/internal/repository/postgres/album.go b/internal/repository/postgres/album.go index dd75f80..110bed6 100644 --- a/internal/repository/postgres/album.go +++ b/internal/repository/postgres/album.go @@ -22,8 +22,8 @@ func NewAlbumRepository(pool *pgxpool.Pool) *AlbumRepository { func (r *AlbumRepository) GetByID(ctx context.Context, id string) (*domain.Album, error) { query := ` - SELECT id, title, album_type, release_date, upc, total_tracks, total_discs, - cover_url, source, source_id + SELECT id, title, album_type, secondary_types, release_date, upc, total_tracks, + total_discs, cover_url, source, source_id FROM albums WHERE id = $1` @@ -41,8 +41,8 @@ func (r *AlbumRepository) GetByID(ctx context.Context, id string) (*domain.Album func (r *AlbumRepository) GetByExternalID(ctx context.Context, source, sourceID string) (*domain.Album, error) { query := ` - SELECT a.id, a.title, a.album_type, a.release_date, a.upc, a.total_tracks, - a.total_discs, a.cover_url, a.source, a.source_id + SELECT a.id, a.title, a.album_type, a.secondary_types, a.release_date, a.upc, + a.total_tracks, a.total_discs, a.cover_url, a.source, a.source_id FROM albums a JOIN album_external_ids e ON a.id = e.album_id WHERE e.source = $1 AND e.source_id = $2` @@ -68,8 +68,8 @@ func (r *AlbumRepository) GetByArtistID(ctx context.Context, artistID string, li WHERE ae.source_id = $1` searchQuery := ` - SELECT DISTINCT a.id, a.title, a.album_type, a.release_date, a.upc, - a.total_tracks, a.total_discs, a.cover_url, a.source, a.source_id + SELECT DISTINCT a.id, a.title, a.album_type, a.secondary_types, a.release_date, + a.upc, a.total_tracks, a.total_discs, a.cover_url, a.source, a.source_id FROM albums a JOIN album_artists aa ON a.id = aa.album_id JOIN artist_external_ids ae ON aa.artist_id = ae.artist_id @@ -105,6 +105,106 @@ func (r *AlbumRepository) GetByArtistID(ctx context.Context, artistID string, li }, nil } +func (r *AlbumRepository) GetAllByArtistID(ctx context.Context, artistID string) ([]domain.Album, error) { + query := ` + SELECT DISTINCT a.id, a.title, a.album_type, a.secondary_types, a.release_date, + a.upc, a.total_tracks, a.total_discs, a.cover_url, a.source, a.source_id + FROM albums a + JOIN album_artists aa ON a.id = aa.album_id + JOIN artist_external_ids ae ON aa.artist_id = ae.artist_id + WHERE ae.source_id = $1 + ORDER BY a.release_date DESC NULLS LAST` + + rows, err := r.pool.Query(ctx, query, artistID) + if err != nil { + return nil, err + } + defer rows.Close() + + var albums []domain.Album + for rows.Next() { + album, err := r.scanAlbumFromRow(rows) + if err != nil { + return nil, err + } + if err := r.loadRelations(ctx, album); err != nil { + return nil, err + } + albums = append(albums, *album) + } + + return albums, rows.Err() +} + +func (r *AlbumRepository) SaveAll(ctx context.Context, albums []domain.Album) error { + tx, err := r.pool.Begin(ctx) + if err != nil { + return err + } + defer tx.Rollback(ctx) + + for i := range albums { + album := &albums[i] + + var source, sourceID string + if len(album.ExternalIDs) > 0 { + source = album.ExternalIDs[0].Source + sourceID = album.ExternalIDs[0].SourceID + } + + query := ` + INSERT INTO albums (id, title, album_type, secondary_types, release_date, upc, + total_tracks, total_discs, cover_url, source, source_id) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + ON CONFLICT (id) DO UPDATE SET + title = EXCLUDED.title, + album_type = EXCLUDED.album_type, + secondary_types = EXCLUDED.secondary_types, + release_date = EXCLUDED.release_date, + upc = EXCLUDED.upc, + total_tracks = EXCLUDED.total_tracks, + total_discs = EXCLUDED.total_discs, + cover_url = EXCLUDED.cover_url, + updated_at = now()` + + _, err = tx.Exec(ctx, query, + album.ID, album.Title, nullString(album.Type), album.SecondaryTypes, + album.ReleaseDate, nullString(album.UPC), album.TotalTracks, album.TotalDiscs, + nullString(album.CoverURL), source, sourceID) + if err != nil { + return err + } + + for _, ext := range album.ExternalIDs { + extQuery := ` + INSERT INTO album_external_ids (album_id, source, source_id, url) + VALUES ($1, $2, $3, $4) + ON CONFLICT (album_id, source, source_id) DO UPDATE SET + url = EXCLUDED.url, + fetched_at = now()` + + _, err = tx.Exec(ctx, extQuery, album.ID, ext.Source, ext.SourceID, nullString(ext.URL)) + if err != nil { + return err + } + } + + for _, ac := range album.Artists { + artistQuery := ` + INSERT INTO album_artists (album_id, artist_id, role, position) + VALUES ($1, $2, $3, $4) + ON CONFLICT (album_id, artist_id, role) DO NOTHING` + + _, err = tx.Exec(ctx, artistQuery, album.ID, ac.Artist.ID, ac.Role, ac.Position) + if err != nil { + return err + } + } + } + + return tx.Commit(ctx) +} + func (r *AlbumRepository) Save(ctx context.Context, album *domain.Album) error { tx, err := r.pool.Begin(ctx) if err != nil { @@ -119,12 +219,13 @@ func (r *AlbumRepository) Save(ctx context.Context, album *domain.Album) error { } query := ` - INSERT INTO albums (id, title, album_type, release_date, upc, total_tracks, - total_discs, cover_url, source, source_id) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + INSERT INTO albums (id, title, album_type, secondary_types, release_date, upc, + total_tracks, total_discs, cover_url, source, source_id) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) ON CONFLICT (id) DO UPDATE SET title = EXCLUDED.title, album_type = EXCLUDED.album_type, + secondary_types = EXCLUDED.secondary_types, release_date = EXCLUDED.release_date, upc = EXCLUDED.upc, total_tracks = EXCLUDED.total_tracks, @@ -133,8 +234,8 @@ func (r *AlbumRepository) Save(ctx context.Context, album *domain.Album) error { updated_at = now()` _, err = tx.Exec(ctx, query, - album.ID, album.Title, nullString(album.Type), album.ReleaseDate, - nullString(album.UPC), album.TotalTracks, album.TotalDiscs, + album.ID, album.Title, nullString(album.Type), album.SecondaryTypes, + album.ReleaseDate, nullString(album.UPC), album.TotalTracks, album.TotalDiscs, nullString(album.CoverURL), source, sourceID) if err != nil { return err @@ -192,7 +293,7 @@ func (r *AlbumRepository) scanAlbumRow(row pgx.Row) (*domain.Album, error) { ) err := row.Scan( - &album.ID, &album.Title, &albumType, &releaseDate, &upc, + &album.ID, &album.Title, &albumType, &album.SecondaryTypes, &releaseDate, &upc, &totalTracks, &totalDiscs, &coverURL, &source, &sourceID, ) if errors.Is(err, pgx.ErrNoRows) { diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 589f994..884f1d8 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -17,7 +17,9 @@ type AlbumRepository interface { GetByID(ctx context.Context, id string) (*domain.Album, error) GetByExternalID(ctx context.Context, source, sourceID string) (*domain.Album, error) GetByArtistID(ctx context.Context, artistID string, limit, offset int) (*domain.SearchResult[domain.Album], error) + GetAllByArtistID(ctx context.Context, artistID string) ([]domain.Album, error) Save(ctx context.Context, album *domain.Album) error + SaveAll(ctx context.Context, albums []domain.Album) error } type TrackRepository interface { diff --git a/internal/server/server.go b/internal/server/server.go index a6c1d6d..179b6f8 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,11 +3,13 @@ package server import ( "context" "errors" + "strings" "github.com/rs/zerolog" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/metadata-agregator/internal/domain" "github.com/metadata-agregator/internal/provider/musicbrainz" "github.com/metadata-agregator/internal/repository" "github.com/metadata-agregator/internal/service" @@ -114,10 +116,9 @@ func (s *MetadataServer) SearchAlbums(ctx context.Context, req *metadatav1.Searc limit = 25 } - albumTypes := defaultAlbumTypes(req.AlbumTypes) - log.Debug().Str("query", req.Query).Str("artist", req.Artist).Int("limit", limit).Int("offset", int(req.Offset)).Strs("album_types", albumTypes).Msg("searching albums") + log.Debug().Str("query", req.Query).Str("artist", req.Artist).Int("limit", limit).Int("offset", int(req.Offset)).Msg("searching albums") - result, err := svc.SearchAlbums(ctx, req.Query, req.Artist, limit, int(req.Offset), albumTypes) + result, err := svc.SearchAlbums(ctx, req.Query, req.Artist, limit, int(req.Offset)) if err != nil { return nil, toGRPCError(ctx, err) } @@ -172,28 +173,25 @@ func (s *MetadataServer) GetArtistAlbums(ctx context.Context, req *metadatav1.Ge return nil, err } - limit := int(req.Limit) - if limit <= 0 { - limit = 25 - } - albumTypes := defaultAlbumTypes(req.AlbumTypes) - log.Debug().Str("artist_id", req.ArtistId).Int("limit", limit).Int("offset", int(req.Offset)).Strs("album_types", albumTypes).Msg("getting artist albums") + log.Debug().Str("artist_id", req.ArtistId).Strs("album_types", albumTypes).Msg("getting artist albums") - result, err := svc.GetArtistAlbums(ctx, req.ArtistId, limit, int(req.Offset), albumTypes) + allAlbums, err := svc.GetArtistAlbums(ctx, req.ArtistId) if err != nil { return nil, toGRPCError(ctx, err) } + filtered := filterAlbums(allAlbums, albumTypes) + resp := &metadatav1.GetArtistAlbumsResponse{ - Total: int32(result.Total), + Total: int32(len(filtered)), } - for _, a := range result.Items { + for _, a := range filtered { resp.Albums = append(resp.Albums, toProtoAlbum(&a)) } - log.Trace().Int("total", result.Total).Int("returned", len(resp.Albums)).Msg("artist albums retrieved") + log.Trace().Int("total_fetched", len(allAlbums)).Int("filtered", len(filtered)).Strs("album_types", albumTypes).Msg("artist albums retrieved") return resp, nil } @@ -279,6 +277,25 @@ func defaultAlbumTypes(types []string) []string { return defaultTypes } +func filterAlbums(albums []domain.Album, types []string) []domain.Album { + allowed := make(map[string]bool, len(types)) + for _, t := range types { + allowed[strings.ToLower(t)] = true + } + + var result []domain.Album + for _, a := range albums { + if len(a.SecondaryTypes) > 0 { + continue + } + if !allowed[strings.ToLower(a.Type)] { + continue + } + result = append(result, a) + } + return result +} + func toGRPCError(ctx context.Context, err error) error { if err == nil { return nil @@ -286,6 +303,11 @@ func toGRPCError(ctx context.Context, err error) error { log := zerolog.Ctx(ctx) + if errors.Is(err, musicbrainz.ErrBadRequest) { + log.Debug().Err(err).Msg("bad request") + return status.Error(codes.InvalidArgument, err.Error()) + } + if errors.Is(err, repository.ErrNotFound) { log.Debug().Msg("entity not found") return status.Error(codes.NotFound, "not found") diff --git a/internal/service/metadata.go b/internal/service/metadata.go index 049fac3..b79fd5d 100644 --- a/internal/service/metadata.go +++ b/internal/service/metadata.go @@ -80,9 +80,9 @@ func (s *MetadataService) SearchArtists(ctx context.Context, query string, limit return s.provider.SearchArtists(ctx, query, limit, offset) } -func (s *MetadataService) SearchAlbums(ctx context.Context, query string, artist string, limit, offset int, albumTypes []string) (*domain.SearchResult[domain.Album], error) { - zerolog.Ctx(ctx).Debug().Str("query", query).Str("artist", artist).Strs("album_types", albumTypes).Str("provider", s.provider.Name()).Msg("searching albums via provider") - return s.provider.SearchAlbums(ctx, query, artist, limit, offset, albumTypes) +func (s *MetadataService) SearchAlbums(ctx context.Context, query string, artist string, limit, offset int) (*domain.SearchResult[domain.Album], error) { + zerolog.Ctx(ctx).Debug().Str("query", query).Str("artist", artist).Str("provider", s.provider.Name()).Msg("searching albums via provider") + return s.provider.SearchAlbums(ctx, query, artist, limit, offset) } func (s *MetadataService) GetAlbum(ctx context.Context, id string) (*domain.Album, error) { @@ -117,19 +117,57 @@ func (s *MetadataService) GetAlbum(ctx context.Context, id string) (*domain.Albu return album, nil } -func (s *MetadataService) GetArtistAlbums(ctx context.Context, artistID string, limit, offset int, albumTypes []string) (*domain.SearchResult[domain.Album], error) { +func (s *MetadataService) GetArtistAlbums(ctx context.Context, artistID string) ([]domain.Album, error) { log := zerolog.Ctx(ctx) - result, err := s.albums.GetByArtistID(ctx, artistID, limit, offset) - if err == nil && len(result.Items) > 0 { - log.Debug().Str("artist_id", artistID).Int("count", len(result.Items)).Msg("artist albums served from cache") + cached, err := s.albums.GetAllByArtistID(ctx, artistID) + if err == nil && len(cached) > 0 { + log.Debug().Str("artist_id", artistID).Int("count", len(cached)).Msg("artist albums cache hit") metrics.CacheHits.WithLabelValues("artist_albums").Inc() - return result, nil + return cached, nil } metrics.CacheMisses.WithLabelValues("artist_albums").Inc() - log.Debug().Str("artist_id", artistID).Strs("album_types", albumTypes).Str("provider", s.provider.Name()).Msg("artist albums cache miss, querying provider") - return s.provider.GetArtistAlbums(ctx, artistID, limit, offset, albumTypes) + log.Debug().Str("artist_id", artistID).Str("provider", s.provider.Name()).Msg("artist albums cache miss, fetching from provider") + + albums, err := s.provider.GetArtistAlbums(ctx, artistID) + if err != nil { + return nil, err + } + + s.ensureArtistsCached(ctx, albums) + + if saveErr := s.albums.SaveAll(ctx, albums); saveErr != nil { + log.Warn().Err(saveErr).Str("artist_id", artistID).Msg("failed to cache artist albums") + } else { + log.Debug().Str("artist_id", artistID).Int("count", len(albums)).Msg("artist albums cached") + } + + return albums, nil +} + +func (s *MetadataService) ensureArtistsCached(ctx context.Context, albums []domain.Album) { + log := zerolog.Ctx(ctx) + seen := make(map[string]bool) + + for _, album := range albums { + for _, ac := range album.Artists { + id := ac.Artist.ID + if id == "" || seen[id] { + continue + } + seen[id] = true + + _, err := s.artists.GetByID(ctx, id) + if err == nil { + continue + } + + if _, err := s.GetArtist(ctx, id); err != nil { + log.Warn().Err(err).Str("artist_id", id).Msg("failed to fetch and cache artist from album credit") + } + } + } } func (s *MetadataService) GetTrack(ctx context.Context, id string) (*domain.Track, error) { diff --git a/tests/e2e/noop_repo_test.go b/tests/e2e/noop_repo_test.go index e2e80b0..ca36423 100644 --- a/tests/e2e/noop_repo_test.go +++ b/tests/e2e/noop_repo_test.go @@ -39,10 +39,18 @@ func (r *noopAlbumRepo) GetByArtistID(ctx context.Context, artistID string, limi return &domain.SearchResult[domain.Album]{}, nil } +func (r *noopAlbumRepo) GetAllByArtistID(ctx context.Context, artistID string) ([]domain.Album, error) { + return nil, nil +} + func (r *noopAlbumRepo) Save(ctx context.Context, album *domain.Album) error { return nil } +func (r *noopAlbumRepo) SaveAll(ctx context.Context, albums []domain.Album) error { + return nil +} + type noopTrackRepo struct{} func (r *noopTrackRepo) GetByID(ctx context.Context, id string) (*domain.Track, error) {