diff --git a/db/migrations/000005_rm_orphan_artist_releases.sql b/db/migrations/000005_rm_orphan_artist_releases.sql new file mode 100644 index 0000000..bfb361f --- /dev/null +++ b/db/migrations/000005_rm_orphan_artist_releases.sql @@ -0,0 +1,9 @@ +-- +goose Up +DELETE FROM artist_releases ar +WHERE NOT EXISTS ( + SELECT 1 + FROM artist_tracks at + JOIN tracks t ON at.track_id = t.id + WHERE at.artist_id = ar.artist_id + AND t.release_id = ar.release_id +); diff --git a/db/queries/etc.sql b/db/queries/etc.sql index 44139b8..38465f2 100644 --- a/db/queries/etc.sql +++ b/db/queries/etc.sql @@ -3,7 +3,13 @@ DO $$ BEGIN DELETE FROM tracks WHERE id NOT IN (SELECT l.track_id FROM listens l); DELETE FROM releases WHERE id NOT IN (SELECT t.release_id FROM tracks t); --- DELETE FROM releases WHERE release_group_id NOT IN (SELECT t.release_group_id FROM tracks t); --- DELETE FROM releases WHERE release_group_id NOT IN (SELECT rg.id FROM release_groups rg); DELETE FROM artists WHERE id NOT IN (SELECT at.artist_id FROM artist_tracks at); + DELETE FROM artist_releases ar + WHERE NOT EXISTS ( + SELECT 1 + FROM artist_tracks at + JOIN tracks t ON at.track_id = t.id + WHERE at.artist_id = ar.artist_id + AND t.release_id = ar.release_id + ); END $$; diff --git a/engine/import_test.go b/engine/import_test.go index 2a802aa..bb5c18e 100644 --- a/engine/import_test.go +++ b/engine/import_test.go @@ -276,6 +276,7 @@ func TestImportKoito(t *testing.T) { giriReleaseMBID := uuid.MustParse("ac1f8da0-21d7-426e-83b0-befff06f0871") suzukiMBID := uuid.MustParse("30f851bb-dba3-4e9b-811c-5f27f595c86a") nijinoTrackMBID := uuid.MustParse("a4f26836-3894-46c1-acac-227808308687") + lp3MBID := uuid.MustParse("d0ec30bd-7cdc-417c-979d-5a0631b8a161") input, err := os.ReadFile(src) require.NoError(t, err) @@ -312,6 +313,12 @@ func TestImportKoito(t *testing.T) { aliases, err := store.GetAllAlbumAliases(ctx, album.ID) require.NoError(t, err) assert.Contains(t, utils.FlattenAliases(aliases), "Nijinoiroyo Azayakadeare (NELKE ver.)") + // ensure album associations are saved + album, err = store.GetAlbum(ctx, db.GetAlbumOpts{MusicBrainzID: lp3MBID}) + require.NoError(t, err) + assert.Contains(t, utils.FlattenSimpleArtistNames(album.Artists), "Elizabeth Powell") + assert.Contains(t, utils.FlattenSimpleArtistNames(album.Artists), "Rachel Goswell") + assert.Contains(t, utils.FlattenSimpleArtistNames(album.Artists), "American Football") // ensure all tracks are saved track, err := store.GetTrack(ctx, db.GetTrackOpts{MusicBrainzID: nijinoTrackMBID}) diff --git a/internal/db/psql/merge.go b/internal/db/psql/merge.go index d9e24b6..dd375c5 100644 --- a/internal/db/psql/merge.go +++ b/internal/db/psql/merge.go @@ -52,7 +52,7 @@ func (d *Psql) MergeTracks(ctx context.Context, fromId, toId int32) error { } err = qtx.CleanOrphanedEntries(ctx) if err != nil { - l.Err(err).Msg("Failed to clean orphaned entries") + l.Err(err).Msg("MergeTracks: Failed to clean orphaned entries") return err } return tx.Commit(ctx) diff --git a/internal/db/psql/merge_test.go b/internal/db/psql/merge_test.go index 08169fb..38e843a 100644 --- a/internal/db/psql/merge_test.go +++ b/internal/db/psql/merge_test.go @@ -12,27 +12,27 @@ func setupTestDataForMerge(t *testing.T) { truncateTestData(t) // Insert artists err := store.Exec(context.Background(), - `INSERT INTO artists (musicbrainz_id, image, image_source) + `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(), - `INSERT INTO artist_aliases (artist_id, alias, source, is_primary) + `INSERT INTO artist_aliases (artist_id, alias, source, is_primary) VALUES (1, 'Artist One', 'Testing', true), (2, 'Artist Two', 'Testing', true)`) require.NoError(t, err) // Insert albums err = store.Exec(context.Background(), - `INSERT INTO releases (musicbrainz_id, image, image_source) + `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), (NULL, NULL, NULL)`) require.NoError(t, err) err = store.Exec(context.Background(), - `INSERT INTO release_aliases (release_id, alias, source, is_primary) + `INSERT INTO release_aliases (release_id, alias, source, is_primary) VALUES (1, 'Album One', 'Testing', true), (2, 'Album Two', 'Testing', true), (3, 'Album Three', 'Testing', true)`) @@ -40,7 +40,7 @@ func setupTestDataForMerge(t *testing.T) { // Insert tracks err = store.Exec(context.Background(), - `INSERT INTO tracks (musicbrainz_id, release_id) + `INSERT INTO tracks (musicbrainz_id, release_id) VALUES ('33333333-3333-3333-3333-333333333333', 1), ('44444444-4444-4444-4444-444444444444', 2), ('55555555-5555-5555-5555-555555555555', 1), @@ -48,7 +48,7 @@ func setupTestDataForMerge(t *testing.T) { require.NoError(t, err) err = store.Exec(context.Background(), - `INSERT INTO track_aliases (track_id, alias, source, is_primary) + `INSERT INTO track_aliases (track_id, alias, source, is_primary) VALUES (1, 'Track One', 'Testing', true), (2, 'Track Two', 'Testing', true), (3, 'Track Three', 'Testing', true), @@ -57,18 +57,18 @@ func setupTestDataForMerge(t *testing.T) { // Associate artists with albums and tracks err = store.Exec(context.Background(), - `INSERT INTO artist_releases (artist_id, release_id) + `INSERT INTO artist_releases (artist_id, release_id) VALUES (1, 1), (2, 2), (1, 3)`) require.NoError(t, err) err = store.Exec(context.Background(), - `INSERT INTO artist_tracks (artist_id, track_id) + `INSERT INTO artist_tracks (artist_id, track_id) VALUES (1, 1), (2, 2), (1, 3), (1, 4)`) require.NoError(t, err) // Insert listens err = store.Exec(context.Background(), - `INSERT INTO listens (user_id, track_id, listened_at) + `INSERT INTO listens (user_id, track_id, listened_at) VALUES (1, 1, NOW() - INTERVAL '1 day'), (1, 2, NOW() - INTERVAL '2 days'), (1, 3, NOW() - INTERVAL '3 days'), @@ -90,14 +90,14 @@ func TestMergeTracks(t *testing.T) { require.NoError(t, err) assert.Equal(t, 2, count, "expected all listens to be merged into Track 2") - // Verify artist is associated with album + // Verify old artist is not associated with album exists, err := store.RowExists(ctx, ` SELECT EXISTS ( SELECT 1 FROM artist_releases WHERE release_id = $1 AND artist_id = $2 )`, 2, 1) require.NoError(t, err) - assert.True(t, exists, "expected old artist to be associated with album") + assert.False(t, exists) truncateTestData(t) } diff --git a/internal/db/psql/track.go b/internal/db/psql/track.go index 743a20e..d4cc616 100644 --- a/internal/db/psql/track.go +++ b/internal/db/psql/track.go @@ -137,6 +137,13 @@ func (d *Psql) SaveTrack(ctx context.Context, opts db.SaveTrackOpts) (*models.Tr if err != nil { return nil, fmt.Errorf("SaveTrack: AssociateArtistToTrack: %w", err) } + err = qtx.AssociateArtistToRelease(ctx, repository.AssociateArtistToReleaseParams{ + ArtistID: aid, + ReleaseID: trackRow.ReleaseID, + }) + if err != nil { + return nil, fmt.Errorf("SaveTrack: AssociateArtistToTrack: %w", err) + } } // insert primary alias err = qtx.InsertTrackAlias(ctx, repository.InsertTrackAliasParams{ @@ -233,7 +240,28 @@ func (d *Psql) SaveTrackAliases(ctx context.Context, id int32, aliases []string, } func (d *Psql) DeleteTrack(ctx context.Context, id int32) error { - return d.q.DeleteTrack(ctx, id) + l := logger.FromContext(ctx) + tx, err := d.conn.BeginTx(ctx, pgx.TxOptions{}) + if err != nil { + l.Err(err).Msg("Failed to begin transaction") + return fmt.Errorf("DeleteTrack: %w", err) + } + defer tx.Rollback(ctx) + qtx := d.q.WithTx(tx) + + err = qtx.DeleteTrack(ctx, id) + if err != nil { + return fmt.Errorf("DeleteTrack: DeleteTrack: %w", err) + } + + // also clean orphaned entries to ensure artists are disassociated with releases where + // they no longer have any tracks on the release + err = qtx.CleanOrphanedEntries(ctx) + if err != nil { + return fmt.Errorf("DeleteTrack: CleanOrphanedEntries: %w", err) + } + + return tx.Commit(ctx) } func (d *Psql) DeleteTrackAlias(ctx context.Context, id int32, alias string) error { diff --git a/internal/db/psql/track_test.go b/internal/db/psql/track_test.go index 7fa58d4..f0ecd09 100644 --- a/internal/db/psql/track_test.go +++ b/internal/db/psql/track_test.go @@ -62,7 +62,7 @@ func testDataForTracks(t *testing.T) { VALUES (1, 1), (2, 2)`) require.NoError(t, err) - // Associate tracks with artists + // Insert listens err = store.Exec(context.Background(), `INSERT INTO listens (user_id, track_id, listened_at) VALUES (1, 1, NOW()), (1, 2, NOW())`) @@ -228,3 +228,27 @@ func TestDeleteTrack(t *testing.T) { _, err = store.Count(ctx, `SELECT * FROM tracks WHERE id = 2`) require.ErrorIs(t, err, pgx.ErrNoRows) // no rows error } + +func TestReleaseAssociations(t *testing.T) { + testDataForTracks(t) + ctx := context.Background() + + track, err := store.SaveTrack(ctx, db.SaveTrackOpts{ + Title: "Track Three", + AlbumID: 2, + ArtistIDs: []int32{2, 1}, // Artist Two feat. Artist One + Duration: 100, + }) + require.NoError(t, err) + count, err := store.Count(ctx, `SELECT COUNT(*) FROM artist_releases WHERE release_id = 2`) + require.NoError(t, err) + require.Equal(t, 2, count, "expected release to be associated with artist from inserted track") + + err = store.DeleteTrack(ctx, track.ID) + require.NoError(t, err) + + count, err = store.Count(ctx, `SELECT COUNT(*) FROM artist_releases WHERE release_id = 2`) + require.NoError(t, err) + require.Equal(t, 1, count, "expected artist no longer on release to be disassociated from release") + +} diff --git a/internal/repository/etc.sql.go b/internal/repository/etc.sql.go index ed902ea..484f5c4 100644 --- a/internal/repository/etc.sql.go +++ b/internal/repository/etc.sql.go @@ -15,11 +15,17 @@ BEGIN DELETE FROM tracks WHERE id NOT IN (SELECT l.track_id FROM listens l); DELETE FROM releases WHERE id NOT IN (SELECT t.release_id FROM tracks t); DELETE FROM artists WHERE id NOT IN (SELECT at.artist_id FROM artist_tracks at); + DELETE FROM artist_releases ar + WHERE NOT EXISTS ( + SELECT 1 + FROM artist_tracks at + JOIN tracks t ON at.track_id = t.id + WHERE at.artist_id = ar.artist_id + AND t.release_id = ar.release_id + ); END $$ ` -// DELETE FROM releases WHERE release_group_id NOT IN (SELECT t.release_group_id FROM tracks t); -// DELETE FROM releases WHERE release_group_id NOT IN (SELECT rg.id FROM release_groups rg); func (q *Queries) CleanOrphanedEntries(ctx context.Context) error { _, err := q.db.Exec(ctx, cleanOrphanedEntries) return err diff --git a/test_assets/koito_export_test.json b/test_assets/koito_export_test.json index b7ce463..e2cd8ea 100644 --- a/test_assets/koito_export_test.json +++ b/test_assets/koito_export_test.json @@ -18,7 +18,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -70,7 +70,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -122,7 +122,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -174,7 +174,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -226,7 +226,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -278,7 +278,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -330,7 +330,7 @@ }, "album": { "image_url": "https://cdn-images.dzcdn.net/images/cover/1f54d600d0ce5c88a6b2fd75659ec796/1000x1000-000000-80-0-0.jpg", - "mbid": null, + "mbid": "d0ec30bd-7cdc-417c-979d-5a0631b8a161", "aliases": [ { "alias": "American Football (LP3)", @@ -703,4 +703,4 @@ ] } ] -} \ No newline at end of file +}