diff --git a/CHANGELOG.md b/CHANGELOG.md index 9eb9f33..7592076 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ -# v0.0.4 +# v0.0.5 +## Features +- Artist MusicBrainz IDs will now be mapped during ListenBrainz and LastFM imports, even when MusicBrainz is disabled +- Merges now support replacing images for artists and albums +- Time listened per item is now displayed on the item page, below the total play count + ## Enhancements -- Re-download images missing from cache on request +- More reliable artist MusicBrainz ID mapping when scrobbling + +## Fixes +- Token validation now correctly validates case-insensitive authorization scheme + +## Docs +- Removed the portion that mentions not being able to map MusicBrainz IDs when it is disabled, as that is no longer true \ No newline at end of file diff --git a/client/api/api.ts b/client/api/api.ts index ec79aa0..150be81 100644 --- a/client/api/api.ts +++ b/client/api/api.ts @@ -74,13 +74,13 @@ function mergeTracks(from: number, to: number): Promise { method: "POST", }) } -function mergeAlbums(from: number, to: number): Promise { - return fetch(`/apis/web/v1/merge/albums?from_id=${from}&to_id=${to}`, { +function mergeAlbums(from: number, to: number, replaceImage: boolean): Promise { + return fetch(`/apis/web/v1/merge/albums?from_id=${from}&to_id=${to}&replace_image=${replaceImage}`, { method: "POST", }) } -function mergeArtists(from: number, to: number): Promise { - return fetch(`/apis/web/v1/merge/artists?from_id=${from}&to_id=${to}`, { +function mergeArtists(from: number, to: number, replaceImage: boolean): Promise { + return fetch(`/apis/web/v1/merge/artists?from_id=${from}&to_id=${to}&replace_image=${replaceImage}`, { method: "POST", }) } @@ -200,6 +200,7 @@ type Track = { image: string album_id: number musicbrainz_id: string + time_listened: number } type Artist = { id: number @@ -208,6 +209,7 @@ type Artist = { aliases: string[] listen_count: number musicbrainz_id: string + time_listened: number } type Album = { id: number, @@ -217,6 +219,7 @@ type Album = { is_various_artists: boolean artists: SimpleArtists[] musicbrainz_id: string + time_listened: number } type Alias = { id: number diff --git a/client/app/components/TopItemList.tsx b/client/app/components/TopItemList.tsx index 22d307c..5884e63 100644 --- a/client/app/components/TopItemList.tsx +++ b/client/app/components/TopItemList.tsx @@ -82,7 +82,7 @@ function ItemCard({ item, type }: { item: Item; type: "album" | "track" | "artis Various Artists :
- +
}
{album.listen_count} plays
diff --git a/client/app/components/modals/MergeModal.tsx b/client/app/components/modals/MergeModal.tsx index ff1079b..9f3fdcc 100644 --- a/client/app/components/modals/MergeModal.tsx +++ b/client/app/components/modals/MergeModal.tsx @@ -21,6 +21,7 @@ export default function MergeModal(props: Props) { const [debouncedQuery, setDebouncedQuery] = useState(query); const [mergeTarget, setMergeTarget] = useState<{title: string, id: number}>({title: '', id: 0}) const [mergeOrderReversed, setMergeOrderReversed] = useState(false) + const [replaceImage, setReplaceImage] = useState(false) const navigate = useNavigate() @@ -53,7 +54,7 @@ export default function MergeModal(props: Props) { from = {id: props.currentId, title: props.currentTitle} to = mergeTarget } - props.mergeFunc(from.id, to.id) + props.mergeFunc(from.id, to.id, replaceImage) .then(r => { if (r.ok) { if (mergeOrderReversed) { @@ -117,6 +118,13 @@ export default function MergeModal(props: Props) { setMergeOrderReversed(!mergeOrderReversed)} /> + { + (props.type.toLowerCase() === "album" || props.type.toLowerCase() === "artist") && +
+ setReplaceImage(!replaceImage)} /> + +
+ } : ''} diff --git a/client/app/routes/MediaItems/Album.tsx b/client/app/routes/MediaItems/Album.tsx index 654fc9e..9751a87 100644 --- a/client/app/routes/MediaItems/Album.tsx +++ b/client/app/routes/MediaItems/Album.tsx @@ -6,6 +6,7 @@ import LastPlays from "~/components/LastPlays"; import PeriodSelector from "~/components/PeriodSelector"; import MediaLayout from "./MediaLayout"; import ActivityGrid from "~/components/ActivityGrid"; +import { timeListenedString } from "~/utils/utils"; export async function clientLoader({ params }: LoaderFunctionArgs) { const res = await fetch(`/apis/web/v1/album?id=${params.id}`); @@ -40,9 +41,10 @@ export default function Album() { } return r }} - subContent={<> + subContent={
{album.listen_count &&

{album.listen_count} play{ album.listen_count > 1 ? 's' : ''}

} - } + {

{timeListenedString(album.time_listened)}

} +
} >
diff --git a/client/app/routes/MediaItems/Artist.tsx b/client/app/routes/MediaItems/Artist.tsx index b742f56..272d5fb 100644 --- a/client/app/routes/MediaItems/Artist.tsx +++ b/client/app/routes/MediaItems/Artist.tsx @@ -7,6 +7,7 @@ import PeriodSelector from "~/components/PeriodSelector"; import MediaLayout from "./MediaLayout"; import ArtistAlbums from "~/components/ArtistAlbums"; import ActivityGrid from "~/components/ActivityGrid"; +import { timeListenedString } from "~/utils/utils"; export async function clientLoader({ params }: LoaderFunctionArgs) { const res = await fetch(`/apis/web/v1/artist?id=${params.id}`); @@ -46,9 +47,10 @@ export default function Artist() { } return r }} - subContent={<> + subContent={
{artist.listen_count &&

{artist.listen_count} play{ artist.listen_count > 1 ? 's' : ''}

} - } + {

{timeListenedString(artist.time_listened)}

} +
} >
diff --git a/client/app/routes/MediaItems/MediaLayout.tsx b/client/app/routes/MediaItems/MediaLayout.tsx index 18a8b78..2503d4b 100644 --- a/client/app/routes/MediaItems/MediaLayout.tsx +++ b/client/app/routes/MediaItems/MediaLayout.tsx @@ -9,7 +9,7 @@ import ImageReplaceModal from "~/components/modals/ImageReplaceModal"; import DeleteModal from "~/components/modals/DeleteModal"; import RenameModal from "~/components/modals/RenameModal"; -export type MergeFunc = (from: number, to: number) => Promise +export type MergeFunc = (from: number, to: number, replaceImage: boolean) => Promise export type MergeSearchCleanerFunc = (r: SearchResponse, id: number) => SearchResponse interface Props { diff --git a/client/app/routes/MediaItems/Track.tsx b/client/app/routes/MediaItems/Track.tsx index bd08a8f..039d951 100644 --- a/client/app/routes/MediaItems/Track.tsx +++ b/client/app/routes/MediaItems/Track.tsx @@ -5,6 +5,7 @@ import LastPlays from "~/components/LastPlays"; import PeriodSelector from "~/components/PeriodSelector"; import MediaLayout from "./MediaLayout"; import ActivityGrid from "~/components/ActivityGrid"; +import { timeListenedString } from "~/utils/utils"; export async function clientLoader({ params }: LoaderFunctionArgs) { let res = await fetch(`/apis/web/v1/track?id=${params.id}`); @@ -42,9 +43,10 @@ export default function Track() { } return r }} - subContent={
+ subContent={
appears on {album.title} {track.listen_count &&

{track.listen_count} play{ track.listen_count > 1 ? 's' : ''}

} + {

{timeListenedString(track.time_listened)}

}
} >
diff --git a/client/app/utils/utils.ts b/client/app/utils/utils.ts index 0cf0b33..fb3fc4f 100644 --- a/client/app/utils/utils.ts +++ b/client/app/utils/utils.ts @@ -86,5 +86,17 @@ const hexToHSL = (hex: string): hsl => { }; }; -export {hexToHSL} +const timeListenedString = (seconds: number) => { + if (!seconds) return "" + + if (seconds > (120 * 60) - 1) { + let hours = Math.floor(seconds / 60 / 60) + return `${hours} hours listened` + } else { + let minutes = Math.floor(seconds / 60) + return `${minutes} minutes listened` + } + } + +export {hexToHSL, timeListenedString} export type {hsl} \ No newline at end of file diff --git a/docs/src/content/docs/guides/importing.md b/docs/src/content/docs/guides/importing.md index c7c1845..cba8a4f 100644 --- a/docs/src/content/docs/guides/importing.md +++ b/docs/src/content/docs/guides/importing.md @@ -12,8 +12,7 @@ Koito currently supports the following sources to import data from: :::note ListenBrainz and LastFM imports can take a long time for large imports due to MusicBrainz requests being throttled at one per second. If you want these imports to go faster, you can [disable MusicBrainz](/reference/configuration/#koito_disable_musicbrainz) in the config while running the importer. However, this -means that artist aliases will not be automatically fetched for imported artists. This also means that artists will not be associated with their MusicBrainz IDs internally, -which can lead to some artist matching issues, especially for people who listen to lots of foreign music. You can also use +means that artist aliases will not be automatically fetched for imported artists. You can also use [your own MusicBrainz mirror](https://musicbrainz.org/doc/MusicBrainz_Server/Setup) and [disable MusicBrainz rate limiting](/reference/configuration/#koito_musicbrainz_url) in the config if you want imports to be faster. ::: diff --git a/engine/handlers/lbz_submit_listen.go b/engine/handlers/lbz_submit_listen.go index 6a0dad1..34004db 100644 --- a/engine/handlers/lbz_submit_listen.go +++ b/engine/handlers/lbz_submit_listen.go @@ -42,8 +42,19 @@ type LbzTrackMeta struct { ArtistName string `json:"artist_name"` // required TrackName string `json:"track_name"` // required ReleaseName string `json:"release_name,omitempty"` + MBIDMapping LbzMBIDMapping `json:"mbid_mapping"` AdditionalInfo LbzAdditionalInfo `json:"additional_info,omitempty"` } +type LbzArtist struct { + ArtistMBID string `json:"artist_mbid"` + ArtistName string `json:"artist_credit_name"` +} +type LbzMBIDMapping struct { + ReleaseMBID string `json:"release_mbid"` + RecordingMBID string `json:"recording_mbid"` + ArtistMBIDs []string `json:"artist_mbids"` + Artists []LbzArtist `json:"artists"` +} type LbzAdditionalInfo struct { MediaPlayer string `json:"media_player,omitempty"` @@ -128,17 +139,30 @@ func LbzSubmitListenHandler(store db.DB, mbzc mbz.MusicBrainzCaller) func(w http if err != nil { l.Debug().Err(err).Msg("LbzSubmitListenHandler: Failed to parse one or more UUIDs") } + if len(artistMbzIDs) < 1 { + l.Debug().Err(err).Msg("LbzSubmitListenHandler: Attempting to parse artist UUIDs from mbid_mapping") + utils.ParseUUIDSlice(payload.TrackMeta.MBIDMapping.ArtistMBIDs) + if err != nil { + l.Debug().Err(err).Msg("LbzSubmitListenHandler: Failed to parse one or more UUIDs") + } + } rgMbzID, err := uuid.Parse(payload.TrackMeta.AdditionalInfo.ReleaseGroupMBID) if err != nil { rgMbzID = uuid.Nil } releaseMbzID, err := uuid.Parse(payload.TrackMeta.AdditionalInfo.ReleaseMBID) if err != nil { - releaseMbzID = uuid.Nil + releaseMbzID, err = uuid.Parse(payload.TrackMeta.MBIDMapping.ReleaseMBID) + if err != nil { + releaseMbzID = uuid.Nil + } } recordingMbzID, err := uuid.Parse(payload.TrackMeta.AdditionalInfo.RecordingMBID) if err != nil { - recordingMbzID = uuid.Nil + recordingMbzID, err = uuid.Parse(payload.TrackMeta.MBIDMapping.RecordingMBID) + if err != nil { + recordingMbzID = uuid.Nil + } } var client string @@ -160,20 +184,33 @@ func LbzSubmitListenHandler(store db.DB, mbzc mbz.MusicBrainzCaller) func(w http listenedAt = time.Unix(payload.ListenedAt, 0) } + var artistMbidMap []catalog.ArtistMbidMap + for _, a := range payload.TrackMeta.MBIDMapping.Artists { + if a.ArtistMBID == "" || a.ArtistName == "" { + continue + } + mbid, err := uuid.Parse(a.ArtistMBID) + if err != nil { + l.Err(err).Msgf("LbzSubmitListenHandler: Failed to parse UUID for artist '%s'", a.ArtistName) + } + artistMbidMap = append(artistMbidMap, catalog.ArtistMbidMap{Artist: a.ArtistName, Mbid: mbid}) + } + opts := catalog.SubmitListenOpts{ - MbzCaller: mbzc, - ArtistNames: payload.TrackMeta.AdditionalInfo.ArtistNames, - Artist: payload.TrackMeta.ArtistName, - ArtistMbzIDs: artistMbzIDs, - TrackTitle: payload.TrackMeta.TrackName, - RecordingMbzID: recordingMbzID, - ReleaseTitle: payload.TrackMeta.ReleaseName, - ReleaseMbzID: releaseMbzID, - ReleaseGroupMbzID: rgMbzID, - Duration: duration, - Time: listenedAt, - UserID: u.ID, - Client: client, + MbzCaller: mbzc, + ArtistNames: payload.TrackMeta.AdditionalInfo.ArtistNames, + Artist: payload.TrackMeta.ArtistName, + ArtistMbzIDs: artistMbzIDs, + TrackTitle: payload.TrackMeta.TrackName, + RecordingMbzID: recordingMbzID, + ReleaseTitle: payload.TrackMeta.ReleaseName, + ReleaseMbzID: releaseMbzID, + ReleaseGroupMbzID: rgMbzID, + ArtistMbidMappings: artistMbidMap, + Duration: duration, + Time: listenedAt, + UserID: u.ID, + Client: client, } if req.ListenType == ListenTypePlayingNow { diff --git a/engine/handlers/merge.go b/engine/handlers/merge.go index 03e83b8..41d38cc 100644 --- a/engine/handlers/merge.go +++ b/engine/handlers/merge.go @@ -3,6 +3,7 @@ package handlers import ( "net/http" "strconv" + "strings" "github.com/gabehf/koito/internal/db" "github.com/gabehf/koito/internal/logger" @@ -67,9 +68,16 @@ func MergeReleaseGroupsHandler(store db.DB) http.HandlerFunc { return } + var replaceImage bool + replaceImgStr := r.URL.Query().Get("replace_image") + if strings.ToLower(replaceImgStr) == "true" { + l.Debug().Msg("MergeReleaseGroupsHandler: Merge will replace image") + replaceImage = true + } + l.Debug().Msgf("MergeReleaseGroupsHandler: Merging release groups from ID %d to ID %d", fromId, toId) - err = store.MergeAlbums(r.Context(), int32(fromId), int32(toId)) + err = store.MergeAlbums(r.Context(), int32(fromId), int32(toId), replaceImage) if err != nil { l.Err(err).Msg("MergeReleaseGroupsHandler: Failed to merge release groups") utils.WriteError(w, "Failed to merge release groups: "+err.Error(), http.StatusInternalServerError) @@ -103,9 +111,16 @@ func MergeArtistsHandler(store db.DB) http.HandlerFunc { return } + var replaceImage bool + replaceImgStr := r.URL.Query().Get("replace_image") + if strings.ToLower(replaceImgStr) == "true" { + l.Debug().Msg("MergeReleaseGroupsHandler: Merge will replace image") + replaceImage = true + } + l.Debug().Msgf("MergeArtistsHandler: Merging artists from ID %d to ID %d", fromId, toId) - err = store.MergeArtists(r.Context(), int32(fromId), int32(toId)) + err = store.MergeArtists(r.Context(), int32(fromId), int32(toId), replaceImage) if err != nil { l.Err(err).Msg("MergeArtistsHandler: Failed to merge artists") utils.WriteError(w, "Failed to merge artists: "+err.Error(), http.StatusInternalServerError) diff --git a/engine/import_test.go b/engine/import_test.go index b128232..4289706 100644 --- a/engine/import_test.go +++ b/engine/import_test.go @@ -119,6 +119,40 @@ func TestImportLastFM(t *testing.T) { truncateTestData(t) } +func TestImportLastFM_MbzDisabled(t *testing.T) { + + src := path.Join("..", "test_assets", "recenttracks-shoko2-1749776100.json") + destDir := filepath.Join(cfg.ConfigDir(), "import") + dest := filepath.Join(destDir, "recenttracks-shoko2-1749776100.json") + + // not going to make the dest dir because engine should make it already + + input, err := os.ReadFile(src) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(dest, input, os.ModePerm)) + + engine.RunImporter(logger.Get(), store, &mbz.MbzErrorCaller{}) + + album, err := store.GetAlbum(context.Background(), db.GetAlbumOpts{MusicBrainzID: uuid.MustParse("e9e78802-0bf8-4ca3-9655-1d943d2d2fa0")}) + require.NoError(t, err) + assert.Equal(t, "ZOO!!", album.Title) + artist, err := store.GetArtist(context.Background(), db.GetArtistOpts{MusicBrainzID: uuid.MustParse("4b00640f-3be6-43f8-9b34-ff81bd89320a")}) + require.NoError(t, err) + assert.Equal(t, "OurR", artist.Name) + artist, err = store.GetArtist(context.Background(), db.GetArtistOpts{Name: "CHUU"}) + require.NoError(t, err) + track, err := store.GetTrack(context.Background(), db.GetTrackOpts{Title: "because I'm stupid?", ArtistIDs: []int32{artist.ID}}) + require.NoError(t, err) + t.Log(track) + listens, err := store.GetListensPaginated(context.Background(), db.GetItemsOpts{TrackID: int(track.ID), Period: db.PeriodAllTime}) + require.NoError(t, err) + require.Len(t, listens.Items, 1) + assert.WithinDuration(t, time.Unix(1749776100, 0), listens.Items[0].Time, 1*time.Second) + + truncateTestData(t) +} + func TestImportListenBrainz(t *testing.T) { src := path.Join("..", "test_assets", "listenbrainz_shoko1_1749780844.zip") @@ -188,3 +222,41 @@ func TestImportListenBrainz(t *testing.T) { truncateTestData(t) } + +func TestImportListenBrainz_MbzDisabled(t *testing.T) { + + src := path.Join("..", "test_assets", "listenbrainz_shoko1_1749780844.zip") + destDir := filepath.Join(cfg.ConfigDir(), "import") + dest := filepath.Join(destDir, "listenbrainz_shoko1_1749780844.zip") + + // not going to make the dest dir because engine should make it already + + input, err := os.ReadFile(src) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(dest, input, os.ModePerm)) + + engine.RunImporter(logger.Get(), store, &mbz.MbzErrorCaller{}) + + album, err := store.GetAlbum(context.Background(), db.GetAlbumOpts{MusicBrainzID: uuid.MustParse("ce330d67-9c46-4a3b-9d62-08406370f234")}) + require.NoError(t, err) + assert.Equal(t, "酸欠少女", album.Title) + artist, err := store.GetArtist(context.Background(), db.GetArtistOpts{MusicBrainzID: uuid.MustParse("4b00640f-3be6-43f8-9b34-ff81bd89320a")}) + require.NoError(t, err) + assert.Equal(t, "OurR", artist.Name) + artist, err = store.GetArtist(context.Background(), db.GetArtistOpts{MusicBrainzID: uuid.MustParse("09887aa7-226e-4ecc-9a0c-02d2ae5777e1")}) + require.NoError(t, err) + assert.Equal(t, "Carly Rae Jepsen", artist.Name) + artist, err = store.GetArtist(context.Background(), db.GetArtistOpts{MusicBrainzID: uuid.MustParse("78e46ae5-9bfd-433b-be3f-19e993d67ecc")}) + require.NoError(t, err) + assert.Equal(t, "Rufus Wainwright", artist.Name) + track, err := store.GetTrack(context.Background(), db.GetTrackOpts{MusicBrainzID: uuid.MustParse("08e8f55b-f1a4-46b8-b2d1-fab4c592165c")}) + require.NoError(t, err) + assert.Equal(t, "Desert", track.Title) + listens, err := store.GetListensPaginated(context.Background(), db.GetItemsOpts{TrackID: int(track.ID), Period: db.PeriodAllTime}) + require.NoError(t, err) + assert.Len(t, listens.Items, 1) + assert.WithinDuration(t, time.Unix(1749780612, 0), listens.Items[0].Time, 1*time.Second) + + truncateTestData(t) +} diff --git a/engine/middleware/validate.go b/engine/middleware/validate.go index fc08a4f..b3e1369 100644 --- a/engine/middleware/validate.go +++ b/engine/middleware/validate.go @@ -87,16 +87,16 @@ func ValidateApiKey(store db.DB) func(next http.Handler) http.Handler { } authh := r.Header.Get("Authorization") - s := strings.Split(authh, "Token ") - if len(s) < 2 { - l.Debug().Msg("ValidateApiKey: Authorization header must be formatted 'Token {token}'") + var token string + if strings.HasPrefix(strings.ToLower(authh), "token ") { + token = strings.TrimSpace(authh[6:]) // strip "Token " + } else { + l.Error().Msg("ValidateApiKey: Authorization header must be formatted 'Token {token}'") utils.WriteError(w, "unauthorized", http.StatusUnauthorized) return } - key := s[1] - - u, err := store.GetUserByApiKey(ctx, key) + u, err := store.GetUserByApiKey(ctx, token) if err != nil { l.Err(err).Msg("Failed to get user from database using api key") utils.WriteError(w, "internal server error", http.StatusInternalServerError) diff --git a/go.mod b/go.mod index 874f117..b09cccb 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/gabehf/koito -go 1.23.7 +go 1.24.2 require ( github.com/go-chi/chi/v5 v5.2.1 diff --git a/internal/catalog/associate_artists.go b/internal/catalog/associate_artists.go index 0014b3e..3e0adf3 100644 --- a/internal/catalog/associate_artists.go +++ b/internal/catalog/associate_artists.go @@ -3,6 +3,7 @@ package catalog import ( "context" "errors" + "fmt" "slices" "strings" @@ -17,11 +18,12 @@ import ( ) type AssociateArtistsOpts struct { - ArtistMbzIDs []uuid.UUID - ArtistNames []string - ArtistName string - TrackTitle string - Mbzc mbz.MusicBrainzCaller + ArtistMbzIDs []uuid.UUID + ArtistNames []string + ArtistMbidMap []ArtistMbidMap + ArtistName string + TrackTitle string + Mbzc mbz.MusicBrainzCaller } func AssociateArtists(ctx context.Context, d db.DB, opts AssociateArtistsOpts) ([]*models.Artist, error) { @@ -29,9 +31,19 @@ func AssociateArtists(ctx context.Context, d db.DB, opts AssociateArtistsOpts) ( var result []*models.Artist - if len(opts.ArtistMbzIDs) > 0 { - l.Debug().Msg("Associating artists by MusicBrainz ID(s)") - mbzMatches, err := matchArtistsByMBID(ctx, d, opts) + // use mbid map first, as it is the most reliable way to get mbid for artists + if len(opts.ArtistMbidMap) > 0 { + l.Debug().Msg("Associating artists by MusicBrainz ID(s) mappings") + mbzMatches, err := matchArtistsByMBIDMappings(ctx, d, opts) + if err != nil { + return nil, err + } + result = append(result, mbzMatches...) + } + + if len(opts.ArtistMbzIDs) > len(result) { + l.Debug().Msg("Associating artists by list of MusicBrainz ID(s)") + mbzMatches, err := matchArtistsByMBID(ctx, d, opts, result) if err != nil { return nil, err } @@ -60,11 +72,82 @@ func AssociateArtists(ctx context.Context, d db.DB, opts AssociateArtistsOpts) ( return result, nil } -func matchArtistsByMBID(ctx context.Context, d db.DB, opts AssociateArtistsOpts) ([]*models.Artist, error) { +func matchArtistsByMBIDMappings(ctx context.Context, d db.DB, opts AssociateArtistsOpts) ([]*models.Artist, error) { + l := logger.FromContext(ctx) + var result []*models.Artist + + for _, a := range opts.ArtistMbidMap { + // first, try to get by mbid + artist, err := d.GetArtist(ctx, db.GetArtistOpts{ + MusicBrainzID: a.Mbid, + }) + if err == nil { + l.Debug().Msgf("Artist '%s' found by MusicBrainz ID", artist.Name) + result = append(result, artist) + continue + } + if !errors.Is(err, pgx.ErrNoRows) { + return nil, fmt.Errorf("matchArtistsBYMBIDMappings: %w", err) + } + // then, try to get by mbz name + artist, err = d.GetArtist(ctx, db.GetArtistOpts{ + Name: a.Artist, + }) + if err == nil { + l.Debug().Msgf("Artist '%s' found by Name", a.Artist) + // ...associate with mbzid if found + err = d.UpdateArtist(ctx, db.UpdateArtistOpts{ID: artist.ID, MusicBrainzID: a.Mbid}) + if err != nil { + l.Err(fmt.Errorf("matchArtistsBYMBIDMappings: %w", err)).Msgf("Failed to associate artist '%s' with MusicBrainz ID", artist.Name) + } else { + artist.MbzID = &a.Mbid + } + result = append(result, artist) + continue + } + if !errors.Is(err, pgx.ErrNoRows) { + return nil, fmt.Errorf("matchArtistsBYMBIDMappings: %w", err) + } + + // then, try to get by aliases, or create + artist, err = resolveAliasOrCreateArtist(ctx, a.Mbid, opts.ArtistNames, d, opts.Mbzc) + if err != nil { + // if mbz unreachable, just create a new artist with provided name and mbid + l.Warn().Msg("MusicBrainz unreachable, creating new artist with provided MusicBrainz ID mapping") + var imgid uuid.UUID + imgUrl, err := images.GetArtistImage(ctx, images.ArtistImageOpts{ + Aliases: []string{a.Artist}, + }) + if err == nil { + imgid = uuid.New() + err = DownloadAndCacheImage(ctx, imgid, imgUrl, ImageSourceSize()) + if err != nil { + l.Err(fmt.Errorf("matchArtistsByMBIDMappings: %w", err)).Msgf("Failed to download artist image for artist '%s'", a.Artist) + imgid = uuid.Nil + } + } else { + l.Err(fmt.Errorf("matchArtistsByMBIDMappings: %w", err)).Msgf("Failed to get artist image for artist '%s'", a.Artist) + } + artist, err = d.SaveArtist(ctx, db.SaveArtistOpts{Name: a.Artist, MusicBrainzID: a.Mbid, Image: imgid, ImageSrc: imgUrl}) + if err != nil { + l.Err(fmt.Errorf("matchArtistsByMBIDMappings: %w", err)).Msgf("Failed to create artist '%s' in database", a.Artist) + return nil, fmt.Errorf("matchArtistsByMBIDMappings: %w", err) + } + } + result = append(result, artist) + } + return result, nil +} + +func matchArtistsByMBID(ctx context.Context, d db.DB, opts AssociateArtistsOpts, existing []*models.Artist) ([]*models.Artist, error) { l := logger.FromContext(ctx) var result []*models.Artist for _, id := range opts.ArtistMbzIDs { + if artistExistsByMbzID(id, existing) || artistExistsByMbzID(id, result) { + l.Debug().Msgf("Artist with MusicBrainz ID %s already found, skipping...", id) + continue + } if id == uuid.Nil { l.Warn().Msg("Provided artist has uuid.Nil MusicBrainzID") return matchArtistsByNames(ctx, opts.ArtistNames, result, d) @@ -229,3 +312,11 @@ func artistExists(name string, artists []*models.Artist) bool { } return false } +func artistExistsByMbzID(id uuid.UUID, artists []*models.Artist) bool { + for _, a := range artists { + if a.MbzID != nil && *a.MbzID == id { + return true + } + } + return false +} diff --git a/internal/catalog/catalog.go b/internal/catalog/catalog.go index e7d3641..26b3a09 100644 --- a/internal/catalog/catalog.go +++ b/internal/catalog/catalog.go @@ -29,24 +29,30 @@ type SaveListenOpts struct { Time time.Time } +type ArtistMbidMap struct { + Artist string + Mbid uuid.UUID +} + type SubmitListenOpts struct { // When true, skips registering the listen and only associates or creates the // artist, release, release group, and track in DB SkipSaveListen bool - MbzCaller mbz.MusicBrainzCaller - ArtistNames []string - Artist string - ArtistMbzIDs []uuid.UUID - TrackTitle string - RecordingMbzID uuid.UUID - Duration int32 // in seconds - ReleaseTitle string - ReleaseMbzID uuid.UUID - ReleaseGroupMbzID uuid.UUID - Time time.Time - UserID int32 - Client string + MbzCaller mbz.MusicBrainzCaller + ArtistNames []string + Artist string + ArtistMbzIDs []uuid.UUID + ArtistMbidMappings []ArtistMbidMap + TrackTitle string + RecordingMbzID uuid.UUID + Duration int32 // in seconds + ReleaseTitle string + ReleaseMbzID uuid.UUID + ReleaseGroupMbzID uuid.UUID + Time time.Time + UserID int32 + Client string } const ( @@ -64,11 +70,12 @@ func SubmitListen(ctx context.Context, store db.DB, opts SubmitListenOpts) error ctx, store, AssociateArtistsOpts{ - ArtistMbzIDs: opts.ArtistMbzIDs, - ArtistNames: opts.ArtistNames, - ArtistName: opts.Artist, - Mbzc: opts.MbzCaller, - TrackTitle: opts.TrackTitle, + ArtistMbzIDs: opts.ArtistMbzIDs, + ArtistNames: opts.ArtistNames, + ArtistName: opts.Artist, + ArtistMbidMap: opts.ArtistMbidMappings, + Mbzc: opts.MbzCaller, + TrackTitle: opts.TrackTitle, }) if err != nil { l.Error().Err(err).Msg("Failed to associate artists to listen") diff --git a/internal/catalog/images.go b/internal/catalog/images.go index 1d6f421..ecce26c 100644 --- a/internal/catalog/images.go +++ b/internal/catalog/images.go @@ -30,6 +30,15 @@ const ( ImageCacheDir = "image_cache" ) +func ImageSourceSize() (size ImageSize) { + if cfg.FullImageCacheEnabled() { + size = ImageSizeFull + } else { + size = ImageSizeLarge + } + return +} + func ParseImageSize(size string) (ImageSize, error) { switch strings.ToLower(size) { case "small": diff --git a/internal/catalog/submit_listen_test.go b/internal/catalog/submit_listen_test.go index 5fcea61..35cb0c1 100644 --- a/internal/catalog/submit_listen_test.go +++ b/internal/catalog/submit_listen_test.go @@ -856,3 +856,64 @@ func TestSubmitListen_MusicBrainzUnreachable(t *testing.T) { require.NoError(t, err) assert.True(t, exists, "expected listen row to exist") } + +func TestSubmitListen_MusicBrainzUnreachableMBIDMappings(t *testing.T) { + truncateTestData(t) + + // correctly associate MBID when musicbrainz unreachable, but map provided + + ctx := context.Background() + mbzc := &mbz.MbzErrorCaller{} + artistMbzID := uuid.MustParse("00000000-0000-0000-0000-000000000001") + artist2MbzID := uuid.MustParse("00000000-0000-0000-0000-000000000002") + releaseGroupMbzID := uuid.MustParse("00000000-0000-0000-0000-000000000011") + releaseMbzID := uuid.MustParse("00000000-0000-0000-0000-000000000101") + trackMbzID := uuid.MustParse("00000000-0000-0000-0000-000000001001") + artistMbzIdMap := []catalog.ArtistMbidMap{{Artist: "ATARASHII GAKKO!", Mbid: artistMbzID}, {Artist: "Featured Artist", Mbid: artist2MbzID}} + opts := catalog.SubmitListenOpts{ + MbzCaller: mbzc, + ArtistNames: []string{"ATARASHII GAKKO!", "Featured Artist"}, + Artist: "ATARASHII GAKKO! feat. Featured Artist", + ArtistMbzIDs: []uuid.UUID{ + artistMbzID, + }, + TrackTitle: "Tokyo Calling", + RecordingMbzID: trackMbzID, + ReleaseTitle: "AG! Calling", + ReleaseMbzID: releaseMbzID, + ReleaseGroupMbzID: releaseGroupMbzID, + ArtistMbidMappings: artistMbzIdMap, + Time: time.Now(), + UserID: 1, + } + + err := catalog.SubmitListen(ctx, store, opts) + require.NoError(t, err) + + // Verify that the listen was saved + exists, err := store.RowExists(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM listens + WHERE track_id = $1 + )`, 1) + require.NoError(t, err) + assert.True(t, exists, "expected listen row to exist") + + // Verify that the artist has the mbid saved + exists, err = store.RowExists(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM artists + WHERE musicbrainz_id = $1 + )`, artistMbzID) + require.NoError(t, err) + assert.True(t, exists, "expected artist to have correct musicbrainz id") + + // Verify that the artist has the mbid saved + exists, err = store.RowExists(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM artists + WHERE musicbrainz_id = $1 + )`, artist2MbzID) + require.NoError(t, err) + assert.True(t, exists, "expected artist to have correct musicbrainz id") +} diff --git a/internal/db/db.go b/internal/db/db.go index 637a51f..16cecd1 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -64,6 +64,7 @@ type DB interface { CountAlbums(ctx context.Context, period Period) (int64, error) CountArtists(ctx context.Context, period Period) (int64, error) CountTimeListened(ctx context.Context, period Period) (int64, error) + CountTimeListenedToItem(ctx context.Context, opts TimeListenedOpts) (int64, error) CountUsers(ctx context.Context) (int64, error) // Search SearchArtists(ctx context.Context, q string) ([]*models.Artist, error) @@ -71,8 +72,8 @@ type DB interface { SearchTracks(ctx context.Context, q string) ([]*models.Track, error) // Merge MergeTracks(ctx context.Context, fromId, toId int32) error - MergeAlbums(ctx context.Context, fromId, toId int32) error - MergeArtists(ctx context.Context, fromId, toId int32) error + MergeAlbums(ctx context.Context, fromId, toId int32, replaceImage bool) error + MergeArtists(ctx context.Context, fromId, toId int32, replaceImage bool) error // Etc ImageHasAssociation(ctx context.Context, image uuid.UUID) (bool, error) GetImageSource(ctx context.Context, image uuid.UUID) (string, error) diff --git a/internal/db/opts.go b/internal/db/opts.go index 481ccc3..0ecd03f 100644 --- a/internal/db/opts.go +++ b/internal/db/opts.go @@ -138,3 +138,10 @@ type ListenActivityOpts struct { ArtistID int32 TrackID int32 } + +type TimeListenedOpts struct { + Period Period + AlbumID int32 + ArtistID int32 + TrackID int32 +} diff --git a/internal/db/psql/album.go b/internal/db/psql/album.go index 0444b45..94c5782 100644 --- a/internal/db/psql/album.go +++ b/internal/db/psql/album.go @@ -57,6 +57,14 @@ func (d *Psql) GetAlbum(ctx context.Context, opts db.GetAlbumOpts) (*models.Albu return nil, err } + seconds, err := d.CountTimeListenedToItem(ctx, db.TimeListenedOpts{ + Period: db.PeriodAllTime, + AlbumID: row.ID, + }) + if err != nil { + return nil, err + } + return &models.Album{ ID: row.ID, MbzID: row.MusicBrainzID, @@ -64,6 +72,7 @@ func (d *Psql) GetAlbum(ctx context.Context, opts db.GetAlbumOpts) (*models.Albu Image: row.Image, VariousArtists: row.VariousArtists, ListenCount: count, + TimeListened: seconds, }, nil } diff --git a/internal/db/psql/album_test.go b/internal/db/psql/album_test.go index 373abdb..d0848cc 100644 --- a/internal/db/psql/album_test.go +++ b/internal/db/psql/album_test.go @@ -47,21 +47,16 @@ func testDataForRelease(t *testing.T) { } func TestGetAlbum(t *testing.T) { - testDataForRelease(t) + testDataForTopItems(t) ctx := context.Background() - // Insert test data - rg, err := store.SaveAlbum(ctx, db.SaveAlbumOpts{ - Title: "Test Release Group", - ArtistIDs: []int32{1}, - }) - require.NoError(t, err) - // Test GetAlbum by ID - result, err := store.GetAlbum(ctx, db.GetAlbumOpts{ID: rg.ID}) + result, err := store.GetAlbum(ctx, db.GetAlbumOpts{ID: 1}) require.NoError(t, err) - assert.Equal(t, rg.ID, result.ID) - assert.Equal(t, "Test Release Group", result.Title) + assert.EqualValues(t, 1, result.ID) + assert.Equal(t, "Release One", result.Title) + assert.EqualValues(t, 4, result.ListenCount) + assert.EqualValues(t, 400, result.TimeListened) // Test GetAlbum with insufficient information _, err = store.GetAlbum(ctx, db.GetAlbumOpts{}) diff --git a/internal/db/psql/artist.go b/internal/db/psql/artist.go index 0368fc6..0d9b702 100644 --- a/internal/db/psql/artist.go +++ b/internal/db/psql/artist.go @@ -16,6 +16,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +// this function sucks because sqlc keeps making new types for rows that are the same func (d *Psql) GetArtist(ctx context.Context, opts db.GetArtistOpts) (*models.Artist, error) { l := logger.FromContext(ctx) if opts.ID != 0 { @@ -32,13 +33,21 @@ func (d *Psql) GetArtist(ctx context.Context, opts db.GetArtistOpts) (*models.Ar if err != nil { return nil, err } + seconds, err := d.CountTimeListenedToItem(ctx, db.TimeListenedOpts{ + Period: db.PeriodAllTime, + ArtistID: row.ID, + }) + if err != nil { + return nil, err + } return &models.Artist{ - ID: row.ID, - MbzID: row.MusicBrainzID, - Name: row.Name, - Aliases: row.Aliases, - Image: row.Image, - ListenCount: count, + ID: row.ID, + MbzID: row.MusicBrainzID, + Name: row.Name, + Aliases: row.Aliases, + Image: row.Image, + ListenCount: count, + TimeListened: seconds, }, nil } else if opts.MusicBrainzID != uuid.Nil { l.Debug().Msgf("Fetching artist from DB with MusicBrainz ID %s", opts.MusicBrainzID) @@ -54,13 +63,21 @@ func (d *Psql) GetArtist(ctx context.Context, opts db.GetArtistOpts) (*models.Ar if err != nil { return nil, err } + seconds, err := d.CountTimeListenedToItem(ctx, db.TimeListenedOpts{ + Period: db.PeriodAllTime, + ArtistID: row.ID, + }) + if err != nil { + return nil, err + } return &models.Artist{ - ID: row.ID, - MbzID: row.MusicBrainzID, - Name: row.Name, - Aliases: row.Aliases, - Image: row.Image, - ListenCount: count, + ID: row.ID, + MbzID: row.MusicBrainzID, + Name: row.Name, + Aliases: row.Aliases, + Image: row.Image, + TimeListened: seconds, + ListenCount: count, }, nil } else if opts.Name != "" { l.Debug().Msgf("Fetching artist from DB with name '%s'", opts.Name) @@ -76,13 +93,21 @@ func (d *Psql) GetArtist(ctx context.Context, opts db.GetArtistOpts) (*models.Ar if err != nil { return nil, err } + seconds, err := d.CountTimeListenedToItem(ctx, db.TimeListenedOpts{ + Period: db.PeriodAllTime, + ArtistID: row.ID, + }) + if err != nil { + return nil, err + } return &models.Artist{ - ID: row.ID, - MbzID: row.MusicBrainzID, - Name: row.Name, - Aliases: row.Aliases, - Image: row.Image, - ListenCount: count, + ID: row.ID, + MbzID: row.MusicBrainzID, + Name: row.Name, + Aliases: row.Aliases, + Image: row.Image, + ListenCount: count, + TimeListened: seconds, }, nil } else { return nil, errors.New("insufficient information to get artist") diff --git a/internal/db/psql/artist_test.go b/internal/db/psql/artist_test.go index 4928988..85ee9ed 100644 --- a/internal/db/psql/artist_test.go +++ b/internal/db/psql/artist_test.go @@ -13,30 +13,33 @@ import ( ) func TestGetArtist(t *testing.T) { + testDataForTopItems(t) ctx := context.Background() mbzId := uuid.MustParse("00000000-0000-0000-0000-000000000001") - // Insert test data - artist, err := store.SaveArtist(ctx, db.SaveArtistOpts{ - Name: "Test Artist", - MusicBrainzID: mbzId, - }) - require.NoError(t, err) // Test GetArtist by ID - result, err := store.GetArtist(ctx, db.GetArtistOpts{ID: artist.ID}) + result, err := store.GetArtist(ctx, db.GetArtistOpts{ID: 1}) require.NoError(t, err) - assert.Equal(t, artist.ID, result.ID) - assert.Equal(t, "Test Artist", result.Name) + assert.EqualValues(t, 1, result.ID) + assert.Equal(t, "Artist One", result.Name) + assert.EqualValues(t, 4, result.ListenCount) + assert.EqualValues(t, 400, result.TimeListened) // Test GetArtist by Name - result, err = store.GetArtist(ctx, db.GetArtistOpts{Name: artist.Name}) + result, err = store.GetArtist(ctx, db.GetArtistOpts{Name: "Artist One"}) require.NoError(t, err) - assert.Equal(t, artist.ID, result.ID) + assert.EqualValues(t, 1, result.ID) + assert.Equal(t, "Artist One", result.Name) + assert.EqualValues(t, 4, result.ListenCount) + assert.EqualValues(t, 400, result.TimeListened) // Test GetArtist by MusicBrainzID result, err = store.GetArtist(ctx, db.GetArtistOpts{MusicBrainzID: mbzId}) require.NoError(t, err) - assert.Equal(t, artist.ID, result.ID) + assert.EqualValues(t, 1, result.ID) + assert.Equal(t, "Artist One", result.Name) + assert.EqualValues(t, 4, result.ListenCount) + assert.EqualValues(t, 400, result.TimeListened) // Test GetArtist with insufficient information _, err = store.GetArtist(ctx, db.GetArtistOpts{}) diff --git a/internal/db/psql/counts.go b/internal/db/psql/counts.go index 5523c92..c7ab3bb 100644 --- a/internal/db/psql/counts.go +++ b/internal/db/psql/counts.go @@ -2,6 +2,7 @@ package psql import ( "context" + "errors" "time" "github.com/gabehf/koito/internal/db" @@ -68,3 +69,41 @@ func (p *Psql) CountTimeListened(ctx context.Context, period db.Period) (int64, } return count, nil } +func (p *Psql) CountTimeListenedToItem(ctx context.Context, opts db.TimeListenedOpts) (int64, error) { + t2 := time.Now() + t1 := db.StartTimeFromPeriod(opts.Period) + + if opts.ArtistID > 0 { + count, err := p.q.CountTimeListenedToArtist(ctx, repository.CountTimeListenedToArtistParams{ + ListenedAt: t1, + ListenedAt_2: t2, + ArtistID: opts.ArtistID, + }) + if err != nil { + return 0, err + } + return count, nil + } else if opts.AlbumID > 0 { + count, err := p.q.CountTimeListenedToRelease(ctx, repository.CountTimeListenedToReleaseParams{ + ListenedAt: t1, + ListenedAt_2: t2, + ReleaseID: opts.AlbumID, + }) + if err != nil { + return 0, err + } + return count, nil + + } else if opts.TrackID > 0 { + count, err := p.q.CountTimeListenedToTrack(ctx, repository.CountTimeListenedToTrackParams{ + ListenedAt: t1, + ListenedAt_2: t2, + ID: opts.TrackID, + }) + if err != nil { + return 0, err + } + return count, nil + } + return 0, errors.New("an id must be provided") +} diff --git a/internal/db/psql/counts_test.go b/internal/db/psql/counts_test.go index b6ddd18..414ebbc 100644 --- a/internal/db/psql/counts_test.go +++ b/internal/db/psql/counts_test.go @@ -74,3 +74,33 @@ func TestCountTimeListened(t *testing.T) { truncateTestData(t) } + +func TestCountTimeListenedToArtist(t *testing.T) { + ctx := context.Background() + testDataForTopItems(t) + period := db.PeriodAllTime + count, err := store.CountTimeListenedToItem(ctx, db.TimeListenedOpts{Period: period, ArtistID: 1}) + require.NoError(t, err) + assert.EqualValues(t, 400, count) + truncateTestData(t) +} + +func TestCountTimeListenedToAlbum(t *testing.T) { + ctx := context.Background() + testDataForTopItems(t) + period := db.PeriodAllTime + count, err := store.CountTimeListenedToItem(ctx, db.TimeListenedOpts{Period: period, AlbumID: 2}) + require.NoError(t, err) + assert.EqualValues(t, 300, count) + truncateTestData(t) +} + +func TestCountTimeListenedToTrack(t *testing.T) { + ctx := context.Background() + testDataForTopItems(t) + period := db.PeriodAllTime + count, err := store.CountTimeListenedToItem(ctx, db.TimeListenedOpts{Period: period, TrackID: 3}) + require.NoError(t, err) + assert.EqualValues(t, 200, count) + truncateTestData(t) +} diff --git a/internal/db/psql/merge.go b/internal/db/psql/merge.go index 91bce1a..0b4a24b 100644 --- a/internal/db/psql/merge.go +++ b/internal/db/psql/merge.go @@ -2,6 +2,7 @@ package psql import ( "context" + "fmt" "github.com/gabehf/koito/internal/logger" "github.com/gabehf/koito/internal/repository" @@ -14,7 +15,7 @@ func (d *Psql) MergeTracks(ctx context.Context, fromId, toId int32) error { tx, err := d.conn.BeginTx(ctx, pgx.TxOptions{}) if err != nil { l.Err(err).Msg("Failed to begin transaction") - return err + return fmt.Errorf("MergeTracks: %w", err) } defer tx.Rollback(ctx) qtx := d.q.WithTx(tx) @@ -23,7 +24,7 @@ func (d *Psql) MergeTracks(ctx context.Context, fromId, toId int32) error { TrackID_2: toId, }) if err != nil { - return err + return fmt.Errorf("MergeTracks: %w", err) } err = qtx.CleanOrphanedEntries(ctx) if err != nil { @@ -33,13 +34,13 @@ func (d *Psql) MergeTracks(ctx context.Context, fromId, toId int32) error { return tx.Commit(ctx) } -func (d *Psql) MergeAlbums(ctx context.Context, fromId, toId int32) error { +func (d *Psql) MergeAlbums(ctx context.Context, fromId, toId int32, replaceImage bool) error { l := logger.FromContext(ctx) l.Info().Msgf("Merging album %d into album %d", fromId, toId) tx, err := d.conn.BeginTx(ctx, pgx.TxOptions{}) if err != nil { l.Err(err).Msg("Failed to begin transaction") - return err + return fmt.Errorf("MergeAlbums: %w", err) } defer tx.Rollback(ctx) qtx := d.q.WithTx(tx) @@ -48,7 +49,21 @@ func (d *Psql) MergeAlbums(ctx context.Context, fromId, toId int32) error { ReleaseID_2: toId, }) if err != nil { - return err + return fmt.Errorf("MergeAlbums: %w", err) + } + if replaceImage { + old, err := qtx.GetRelease(ctx, fromId) + if err != nil { + return fmt.Errorf("MergeAlbums: %w", err) + } + err = qtx.UpdateReleaseImage(ctx, repository.UpdateReleaseImageParams{ + ID: toId, + Image: old.Image, + ImageSource: old.ImageSource, + }) + if err != nil { + return fmt.Errorf("MergeAlbums: %w", err) + } } err = qtx.CleanOrphanedEntries(ctx) if err != nil { @@ -58,13 +73,13 @@ func (d *Psql) MergeAlbums(ctx context.Context, fromId, toId int32) error { return tx.Commit(ctx) } -func (d *Psql) MergeArtists(ctx context.Context, fromId, toId int32) error { +func (d *Psql) MergeArtists(ctx context.Context, fromId, toId int32, replaceImage bool) error { l := logger.FromContext(ctx) l.Info().Msgf("Merging artist %d into artist %d", fromId, toId) tx, err := d.conn.BeginTx(ctx, pgx.TxOptions{}) if err != nil { l.Err(err).Msg("Failed to begin transaction") - return err + return fmt.Errorf("MergeArtists: %w", err) } defer tx.Rollback(ctx) qtx := d.q.WithTx(tx) @@ -74,7 +89,7 @@ func (d *Psql) MergeArtists(ctx context.Context, fromId, toId int32) error { }) if err != nil { l.Err(err).Msg("Failed to delete conflicting artist tracks") - return err + return fmt.Errorf("MergeArtists: %w", err) } err = qtx.DeleteConflictingArtistReleases(ctx, repository.DeleteConflictingArtistReleasesParams{ ArtistID: fromId, @@ -82,7 +97,7 @@ func (d *Psql) MergeArtists(ctx context.Context, fromId, toId int32) error { }) if err != nil { l.Err(err).Msg("Failed to delete conflicting artist releases") - return err + return fmt.Errorf("MergeArtists: %w", err) } err = qtx.UpdateArtistTracks(ctx, repository.UpdateArtistTracksParams{ ArtistID: fromId, @@ -90,7 +105,7 @@ func (d *Psql) MergeArtists(ctx context.Context, fromId, toId int32) error { }) if err != nil { l.Err(err).Msg("Failed to update artist tracks") - return err + return fmt.Errorf("MergeArtists: %w", err) } err = qtx.UpdateArtistReleases(ctx, repository.UpdateArtistReleasesParams{ ArtistID: fromId, @@ -98,12 +113,26 @@ func (d *Psql) MergeArtists(ctx context.Context, fromId, toId int32) error { }) if err != nil { l.Err(err).Msg("Failed to update artist releases") - return err + return fmt.Errorf("MergeArtists: %w", err) + } + if replaceImage { + old, err := qtx.GetArtist(ctx, fromId) + if err != nil { + return fmt.Errorf("MergeAlbums: %w", err) + } + err = qtx.UpdateArtistImage(ctx, repository.UpdateArtistImageParams{ + ID: toId, + Image: old.Image, + ImageSource: old.ImageSource, + }) + if err != nil { + return fmt.Errorf("MergeAlbums: %w", err) + } } err = qtx.CleanOrphanedEntries(ctx) if err != nil { l.Err(err).Msg("Failed to clean orphaned entries") - return err + return fmt.Errorf("MergeArtists: %w", err) } return tx.Commit(ctx) } diff --git a/internal/db/psql/merge_test.go b/internal/db/psql/merge_test.go index ceb612e..f71fccf 100644 --- a/internal/db/psql/merge_test.go +++ b/internal/db/psql/merge_test.go @@ -12,9 +12,9 @@ func setupTestDataForMerge(t *testing.T) { truncateTestData(t) // Insert artists err := store.Exec(context.Background(), - `INSERT INTO artists (musicbrainz_id) - VALUES ('00000000-0000-0000-0000-000000000001'), - ('00000000-0000-0000-0000-000000000002')`) + `INSERT INTO artists (musicbrainz_id, image, image_source) + VALUES ('00000000-0000-0000-0000-000000000001', '10000000-0000-0000-0000-000000000000', 'source.com'), + ('00000000-0000-0000-0000-000000000002', NULL, NULL)`) require.NoError(t, err) err = store.Exec(context.Background(), @@ -25,9 +25,9 @@ func setupTestDataForMerge(t *testing.T) { // Insert albums err = store.Exec(context.Background(), - `INSERT INTO releases (musicbrainz_id) - VALUES ('11111111-1111-1111-1111-111111111111'), - ('22222222-2222-2222-2222-222222222222')`) + `INSERT INTO releases (musicbrainz_id, image, image_source) + VALUES ('11111111-1111-1111-1111-111111111111', '20000000-0000-0000-0000-000000000000', 'source.com'), + ('22222222-2222-2222-2222-222222222222', NULL, NULL)`) require.NoError(t, err) err = store.Exec(context.Background(), @@ -90,11 +90,15 @@ func TestMergeAlbums(t *testing.T) { setupTestDataForMerge(t) // Merge Album 1 into Album 2 - err := store.MergeAlbums(ctx, 1, 2) + err := store.MergeAlbums(ctx, 1, 2, true) require.NoError(t, err) + // Verify image was replaced + count, err := store.Count(ctx, `SELECT COUNT(*) FROM releases WHERE image = '20000000-0000-0000-0000-000000000000' AND image_source = 'source.com'`) + require.NoError(t, err) + assert.Equal(t, 1, count, "expected merged release to contain image information") + // Verify tracks are updated - var count int count, err = store.Count(ctx, `SELECT COUNT(*) FROM tracks WHERE release_id = 2`) require.NoError(t, err) assert.Equal(t, 2, count, "expected all tracks to be merged into Album 2") @@ -107,11 +111,15 @@ func TestMergeArtists(t *testing.T) { setupTestDataForMerge(t) // Merge Artist 1 into Artist 2 - err := store.MergeArtists(ctx, 1, 2) + err := store.MergeArtists(ctx, 1, 2, true) require.NoError(t, err) + // Verify image was replaced + count, err := store.Count(ctx, `SELECT COUNT(*) FROM artists WHERE image = '10000000-0000-0000-0000-000000000000' AND image_source = 'source.com'`) + require.NoError(t, err) + assert.Equal(t, 1, count, "expected merged artist to contain image information") + // Verify artist associations are updated - var count int count, err = store.Count(ctx, `SELECT COUNT(*) FROM artist_tracks WHERE artist_id = 2`) require.NoError(t, err) assert.Equal(t, 2, count, "expected all tracks to be associated with Artist 2") diff --git a/internal/db/psql/track.go b/internal/db/psql/track.go index 0c3c2a4..5d3961d 100644 --- a/internal/db/psql/track.go +++ b/internal/db/psql/track.go @@ -72,10 +72,19 @@ func (d *Psql) GetTrack(ctx context.Context, opts db.GetTrackOpts) (*models.Trac TrackID: track.ID, }) if err != nil { - l.Err(err).Msgf("Failed to get listen count for track with id %d", track.ID) + return nil, err + } + + seconds, err := d.CountTimeListenedToItem(ctx, db.TimeListenedOpts{ + Period: db.PeriodAllTime, + TrackID: track.ID, + }) + if err != nil { + return nil, err } track.ListenCount = count + track.TimeListened = seconds return &track, nil } diff --git a/internal/db/psql/track_test.go b/internal/db/psql/track_test.go index ac79423..777b22c 100644 --- a/internal/db/psql/track_test.go +++ b/internal/db/psql/track_test.go @@ -44,9 +44,9 @@ func testDataForTracks(t *testing.T) { // Insert tracks err = store.Exec(context.Background(), - `INSERT INTO tracks (musicbrainz_id, release_id) - VALUES ('11111111-1111-1111-1111-111111111111', 1), - ('22222222-2222-2222-2222-222222222222', 2)`) + `INSERT INTO tracks (musicbrainz_id, release_id, duration) + VALUES ('11111111-1111-1111-1111-111111111111', 1, 100), + ('22222222-2222-2222-2222-222222222222', 2, 100)`) require.NoError(t, err) // Insert track aliases @@ -61,6 +61,12 @@ func testDataForTracks(t *testing.T) { `INSERT INTO artist_tracks (artist_id, track_id) VALUES (1, 1), (2, 2)`) require.NoError(t, err) + + // Associate tracks with artists + err = store.Exec(context.Background(), + `INSERT INTO listens (user_id, track_id, listened_at) + VALUES (1, 1, NOW()), (1, 2, NOW())`) + require.NoError(t, err) } func TestGetTrack(t *testing.T) { @@ -73,12 +79,14 @@ func TestGetTrack(t *testing.T) { assert.Equal(t, int32(1), track.ID) assert.Equal(t, "Track One", track.Title) assert.Equal(t, uuid.MustParse("11111111-1111-1111-1111-111111111111"), *track.MbzID) + assert.EqualValues(t, 100, track.TimeListened) // Test GetTrack by MusicBrainzID track, err = store.GetTrack(ctx, db.GetTrackOpts{MusicBrainzID: uuid.MustParse("22222222-2222-2222-2222-222222222222")}) require.NoError(t, err) assert.Equal(t, int32(2), track.ID) assert.Equal(t, "Track Two", track.Title) + assert.EqualValues(t, 100, track.TimeListened) // Test GetTrack by Title and ArtistIDs track, err = store.GetTrack(ctx, db.GetTrackOpts{ @@ -88,6 +96,7 @@ func TestGetTrack(t *testing.T) { require.NoError(t, err) assert.Equal(t, int32(1), track.ID) assert.Equal(t, "Track One", track.Title) + assert.EqualValues(t, 100, track.TimeListened) // Test GetTrack with insufficient information _, err = store.GetTrack(ctx, db.GetTrackOpts{}) diff --git a/internal/importer/lastfm.go b/internal/importer/lastfm.go index f01e4b1..d3e0028 100644 --- a/internal/importer/lastfm.go +++ b/internal/importer/lastfm.go @@ -97,17 +97,25 @@ func ImportLastFMFile(ctx context.Context, store db.DB, mbzc mbz.MusicBrainzCall l.Debug().Msgf("Skipping import due to import time rules") continue } + + var artistMbidMap []catalog.ArtistMbidMap + if artistMbzID != uuid.Nil { + artistMbidMap = append(artistMbidMap, catalog.ArtistMbidMap{Artist: track.Artist.Text, Mbid: artistMbzID}) + } + opts := catalog.SubmitListenOpts{ - MbzCaller: mbzc, - Artist: track.Artist.Text, - ArtistMbzIDs: []uuid.UUID{artistMbzID}, - TrackTitle: track.Name, - RecordingMbzID: trackMbzID, - ReleaseTitle: album, - ReleaseMbzID: albumMbzID, - Client: "lastfm", - Time: ts, - UserID: 1, + MbzCaller: mbzc, + Artist: track.Artist.Text, + ArtistNames: []string{track.Artist.Text}, + ArtistMbzIDs: []uuid.UUID{artistMbzID}, + TrackTitle: track.Name, + RecordingMbzID: trackMbzID, + ReleaseTitle: album, + ReleaseMbzID: albumMbzID, + ArtistMbidMappings: artistMbidMap, + Client: "lastfm", + Time: ts, + UserID: 1, } err = catalog.SubmitListen(ctx, store, opts) if err != nil { diff --git a/internal/importer/listenbrainz.go b/internal/importer/listenbrainz.go index c9b7355..f8a8218 100644 --- a/internal/importer/listenbrainz.go +++ b/internal/importer/listenbrainz.go @@ -113,20 +113,34 @@ func ImportListenBrainzFile(ctx context.Context, store db.DB, mbzc mbz.MusicBrai } else if payload.TrackMeta.AdditionalInfo.DurationMs != 0 { duration = payload.TrackMeta.AdditionalInfo.DurationMs / 1000 } + + var artistMbidMap []catalog.ArtistMbidMap + for _, a := range payload.TrackMeta.MBIDMapping.Artists { + if a.ArtistMBID == "" || a.ArtistName == "" { + continue + } + mbid, err := uuid.Parse(a.ArtistMBID) + if err != nil { + l.Err(err).Msgf("LbzSubmitListenHandler: Failed to parse UUID for artist '%s'", a.ArtistName) + } + artistMbidMap = append(artistMbidMap, catalog.ArtistMbidMap{Artist: a.ArtistName, Mbid: mbid}) + } + opts := catalog.SubmitListenOpts{ - MbzCaller: mbzc, - ArtistNames: payload.TrackMeta.AdditionalInfo.ArtistNames, - Artist: payload.TrackMeta.ArtistName, - ArtistMbzIDs: artistMbzIDs, - TrackTitle: payload.TrackMeta.TrackName, - RecordingMbzID: recordingMbzID, - ReleaseTitle: payload.TrackMeta.ReleaseName, - ReleaseMbzID: releaseMbzID, - ReleaseGroupMbzID: rgMbzID, - Duration: duration, - Time: ts, - UserID: 1, - Client: client, + MbzCaller: mbzc, + ArtistNames: payload.TrackMeta.AdditionalInfo.ArtistNames, + Artist: payload.TrackMeta.ArtistName, + ArtistMbzIDs: artistMbzIDs, + TrackTitle: payload.TrackMeta.TrackName, + RecordingMbzID: recordingMbzID, + ReleaseTitle: payload.TrackMeta.ReleaseName, + ReleaseMbzID: releaseMbzID, + ReleaseGroupMbzID: rgMbzID, + ArtistMbidMappings: artistMbidMap, + Duration: duration, + Time: ts, + UserID: 1, + Client: client, } err = catalog.SubmitListen(ctx, store, opts) if err != nil { diff --git a/internal/models/album.go b/internal/models/album.go index 90b8cdd..a92a3aa 100644 --- a/internal/models/album.go +++ b/internal/models/album.go @@ -10,6 +10,7 @@ type Album struct { Artists []SimpleArtist `json:"artists"` VariousArtists bool `json:"is_various_artists"` ListenCount int64 `json:"listen_count"` + TimeListened int64 `json:"time_listened"` } // type SimpleAlbum struct { diff --git a/internal/models/artist.go b/internal/models/artist.go index b240370..b515414 100644 --- a/internal/models/artist.go +++ b/internal/models/artist.go @@ -3,12 +3,13 @@ package models import "github.com/google/uuid" type Artist struct { - ID int32 `json:"id"` - MbzID *uuid.UUID `json:"musicbrainz_id"` - Name string `json:"name"` - Aliases []string `json:"aliases"` - Image *uuid.UUID `json:"image"` - ListenCount int64 `json:"listen_count"` + ID int32 `json:"id"` + MbzID *uuid.UUID `json:"musicbrainz_id"` + Name string `json:"name"` + Aliases []string `json:"aliases"` + Image *uuid.UUID `json:"image"` + ListenCount int64 `json:"listen_count"` + TimeListened int64 `json:"time_listened"` } type SimpleArtist struct { diff --git a/internal/models/track.go b/internal/models/track.go index 386a2fc..086813f 100644 --- a/internal/models/track.go +++ b/internal/models/track.go @@ -3,12 +3,13 @@ package models import "github.com/google/uuid" type Track struct { - ID int32 `json:"id"` - Title string `json:"title"` - Artists []SimpleArtist `json:"artists"` - MbzID *uuid.UUID `json:"musicbrainz_id"` - ListenCount int64 `json:"listen_count"` - Duration int32 `json:"duration"` - Image *uuid.UUID `json:"image"` - AlbumID int32 `json:"album_id"` + ID int32 `json:"id"` + Title string `json:"title"` + Artists []SimpleArtist `json:"artists"` + MbzID *uuid.UUID `json:"musicbrainz_id"` + ListenCount int64 `json:"listen_count"` + Duration int32 `json:"duration"` + Image *uuid.UUID `json:"image"` + AlbumID int32 `json:"album_id"` + TimeListened int64 `json:"time_listened"` }