fix: release associations and add cleanup migration (#168)

* fix: release associations and add cleanup migration

* fix: incorrect test

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
+);

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 $$;

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})

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)

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)
 }

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 {

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")
+
+}

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

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 @@
       ]
     }
   ]
-}
+}