mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
library: track whether an artist has credits
Add artists.credited, updated at the end of a sync that changed artists or credits and backfilled on upgrade when relation data exists. Filter the artist lists and the downloaded artist list on it, which keeps partially synced libraries visible and hides display-only rows once credits are known.
This commit is contained in:
+1891
File diff suppressed because it is too large
Load Diff
@@ -93,13 +93,7 @@ private const val DOWNLOADED_ARTIST_ALBUMS =
|
||||
"WHERE song_artists.sourceId = :sourceId AND song_artists.artistId = artists.id " +
|
||||
"AND sd.status = 'Completed' AND songs.albumId IS NOT NULL"
|
||||
|
||||
internal const val ARTIST_CREDITED =
|
||||
"((NOT EXISTS (SELECT 1 FROM album_artists WHERE sourceId = artists.sourceId) " +
|
||||
"AND NOT EXISTS (SELECT 1 FROM song_artists WHERE sourceId = artists.sourceId)) " +
|
||||
"OR EXISTS (SELECT 1 FROM album_artists aa " +
|
||||
"WHERE aa.sourceId = artists.sourceId AND aa.artistId = artists.id) " +
|
||||
"OR EXISTS (SELECT 1 FROM song_artists sa " +
|
||||
"WHERE sa.sourceId = artists.sourceId AND sa.artistId = artists.id))"
|
||||
internal const val ARTIST_CREDITED = "artists.credited = 1"
|
||||
|
||||
internal const val ARTISTS_FILTER =
|
||||
"FROM artists WHERE sourceId = :sourceId " +
|
||||
@@ -121,7 +115,8 @@ internal const val ARTISTS_SELECT =
|
||||
"CASE WHEN :downloadedFilter = 1 THEN (" +
|
||||
"SELECT COUNT(*) FROM ($DOWNLOADED_ARTIST_ALBUMS)" +
|
||||
") ELSE artists.albumCount END AS albumCount, " +
|
||||
"artists.starred, artists.coverArt, artists.playCount, artists.played, artists.songCount, artists.sortName "
|
||||
"artists.starred, artists.coverArt, artists.playCount, artists.played, artists.songCount, artists.sortName, " +
|
||||
"artists.credited "
|
||||
|
||||
internal const val ARTIST_ORDER_BY_NAME = "sortName COLLATE NOCASE, id"
|
||||
internal const val ARTIST_ORDER_BY_NAME_REVERSED = "sortName COLLATE NOCASE DESC, id DESC"
|
||||
@@ -250,11 +245,18 @@ interface LibraryDao {
|
||||
"), counts AS (" +
|
||||
"SELECT artistId AS id, COUNT(*) AS songCount FROM credited GROUP BY artistId" +
|
||||
") " +
|
||||
"UPDATE artists SET songCount = c.songCount " +
|
||||
"FROM (SELECT artists.id AS id, COALESCE(counts.songCount, 0) AS songCount " +
|
||||
"UPDATE artists SET songCount = c.songCount, credited = c.credited " +
|
||||
"FROM (SELECT artists.id AS id, COALESCE(counts.songCount, 0) AS songCount, " +
|
||||
"((NOT EXISTS (SELECT 1 FROM album_artists WHERE sourceId = artists.sourceId) " +
|
||||
"AND NOT EXISTS (SELECT 1 FROM song_artists WHERE sourceId = artists.sourceId)) " +
|
||||
"OR EXISTS (SELECT 1 FROM album_artists aa " +
|
||||
"WHERE aa.sourceId = artists.sourceId AND aa.artistId = artists.id) " +
|
||||
"OR EXISTS (SELECT 1 FROM song_artists sa " +
|
||||
"WHERE sa.sourceId = artists.sourceId AND sa.artistId = artists.id)) AS credited " +
|
||||
"FROM artists LEFT JOIN counts ON counts.id = artists.id " +
|
||||
"WHERE artists.sourceId = :sourceId) c " +
|
||||
"WHERE artists.sourceId = :sourceId AND artists.id = c.id AND artists.songCount != c.songCount",
|
||||
"WHERE artists.sourceId = :sourceId AND artists.id = c.id " +
|
||||
"AND (artists.songCount != c.songCount OR artists.credited != c.credited)",
|
||||
)
|
||||
suspend fun recomputeArtistSongCounts(sourceId: Long)
|
||||
|
||||
@@ -881,7 +883,7 @@ interface LibraryDao {
|
||||
"SELECT artists.* FROM artists " +
|
||||
"JOIN (SELECT id, MAX(downloadedAt) AS lastDownloadedAt FROM ($DOWNLOADED_ARTIST_ROWS) " +
|
||||
"WHERE status = 'Completed' GROUP BY id) dl ON dl.id = artists.id " +
|
||||
"WHERE artists.sourceId = :sourceId " +
|
||||
"WHERE artists.sourceId = :sourceId AND $ARTIST_CREDITED " +
|
||||
"ORDER BY dl.lastDownloadedAt DESC, artists.sortName COLLATE NOCASE, artists.id",
|
||||
)
|
||||
fun downloadedArtists(sourceId: Long): PagingSource<Int, Artist>
|
||||
|
||||
@@ -551,6 +551,22 @@ private suspend fun dropSearchIndex(
|
||||
connection.execSQL("DROP TABLE IF EXISTS `$table`")
|
||||
}
|
||||
|
||||
val MIGRATION_31_32 =
|
||||
object : Migration(31, 32) {
|
||||
override suspend fun migrate(connection: SQLiteConnection) {
|
||||
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `credited` INTEGER NOT NULL DEFAULT 1")
|
||||
connection.execSQL(
|
||||
"UPDATE `artists` SET `credited` = 0 " +
|
||||
"WHERE (EXISTS (SELECT 1 FROM `album_artists` WHERE `sourceId` = `artists`.`sourceId`) " +
|
||||
"OR EXISTS (SELECT 1 FROM `song_artists` WHERE `sourceId` = `artists`.`sourceId`)) " +
|
||||
"AND NOT EXISTS (SELECT 1 FROM `album_artists` aa " +
|
||||
"WHERE aa.`sourceId` = `artists`.`sourceId` AND aa.`artistId` = `artists`.`id`) " +
|
||||
"AND NOT EXISTS (SELECT 1 FROM `song_artists` sa " +
|
||||
"WHERE sa.`sourceId` = `artists`.`sourceId` AND sa.`artistId` = `artists`.`id`)",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Mirrors the FTS5 table and content-sync triggers Room generates for the @Fts5 entities.
|
||||
private suspend fun createSearchIndex(
|
||||
connection: SQLiteConnection,
|
||||
@@ -615,4 +631,5 @@ val MIGRATIONS: Array<Migration> =
|
||||
MIGRATION_28_29,
|
||||
MIGRATION_29_30,
|
||||
MIGRATION_30_31,
|
||||
MIGRATION_31_32,
|
||||
)
|
||||
|
||||
@@ -54,7 +54,7 @@ import com.subtracks.data.model.UpNextEntry
|
||||
SongDownload::class,
|
||||
PendingAction::class,
|
||||
],
|
||||
version = 31,
|
||||
version = 32,
|
||||
exportSchema = true,
|
||||
)
|
||||
@ColumnTypeConverters(
|
||||
|
||||
@@ -91,6 +91,7 @@ data class Artist(
|
||||
val played: Long? = null,
|
||||
@ColumnInfo(defaultValue = "0") val songCount: Long = 0,
|
||||
@ColumnInfo(collate = ColumnInfo.NOCASE, defaultValue = "''") val sortName: String = name,
|
||||
@ColumnInfo(defaultValue = "1") val credited: Boolean = true,
|
||||
)
|
||||
|
||||
@Entity(
|
||||
|
||||
@@ -35,11 +35,13 @@ class SyncService(
|
||||
|
||||
suspend fun sync(): SyncSummary {
|
||||
creditsChanged = false
|
||||
val artistsBefore = dbTotalChanges()
|
||||
syncArtists()
|
||||
val artistsChanged = dbTotalChanges() != artistsBefore
|
||||
syncAlbums()
|
||||
syncSongs()
|
||||
optimize()
|
||||
if (creditsChanged) recomputeArtistSongCounts()
|
||||
if (creditsChanged || artistsChanged) recomputeArtistSongCounts()
|
||||
aggregatePlayData()
|
||||
syncPlaylists()
|
||||
syncPlaylistSongs()
|
||||
@@ -163,6 +165,8 @@ class SyncService(
|
||||
return value
|
||||
}
|
||||
|
||||
private suspend fun dbTotalChanges(): Long = db.useWriterConnection { it.totalChanges() }
|
||||
|
||||
private suspend fun PooledConnection.albumIdsChanged(batch: List<Song>): Boolean {
|
||||
val expected = batch.associate { it.id to it.albumId }.toMutableMap()
|
||||
if (expected.isEmpty()) return false
|
||||
|
||||
@@ -211,6 +211,32 @@ class MigrationsTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theArtistCreditedFlagsHideRowsWithoutCredits() =
|
||||
runTest {
|
||||
driver.open(":memory:").use { migrated ->
|
||||
applySchema(migrated, 31)
|
||||
migrated.execSQL(
|
||||
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 's', 'http://x', 1, 0)",
|
||||
)
|
||||
migrated.execSQL("INSERT INTO artists (sourceId, id, name, albumCount) VALUES (1, 'a1', 'Alpha', 1)")
|
||||
migrated.execSQL("INSERT INTO artists (sourceId, id, name, albumCount) VALUES (1, 'combo', 'Alpha & Beta', 1)")
|
||||
migrated.execSQL(
|
||||
"INSERT INTO albums (sourceId, id, artistId, name, albumArtist, created, songCount) " +
|
||||
"VALUES (1, 'al1', 'a1', 'Album', 'Alpha', 0, 0)",
|
||||
)
|
||||
migrated.execSQL(
|
||||
"INSERT INTO album_artists (sourceId, albumId, position, artistId, name) " +
|
||||
"VALUES (1, 'al1', 0, 'a1', 'Alpha')",
|
||||
)
|
||||
|
||||
MIGRATION_31_32.migrate(migrated)
|
||||
|
||||
assertEquals(1L, long(migrated, "SELECT credited FROM artists WHERE id = 'a1'"))
|
||||
assertEquals(0L, long(migrated, "SELECT credited FROM artists WHERE id = 'combo'"))
|
||||
}
|
||||
}
|
||||
|
||||
private fun text(
|
||||
connection: SQLiteConnection,
|
||||
sql: String,
|
||||
|
||||
@@ -8,6 +8,7 @@ import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import com.subtracks.data.db.SubtracksDatabase
|
||||
import com.subtracks.data.db.artistsDefaultQuery
|
||||
import com.subtracks.data.model.Album
|
||||
import com.subtracks.data.model.Artist
|
||||
import com.subtracks.data.model.ArtistRef
|
||||
@@ -19,6 +20,7 @@ import com.subtracks.data.model.PlaylistSong
|
||||
import com.subtracks.data.model.Song
|
||||
import com.subtracks.data.model.SongDownload
|
||||
import com.subtracks.data.model.Source
|
||||
import com.subtracks.data.prefs.ArtistSort
|
||||
import com.subtracks.data.source.MusicSource
|
||||
import com.subtracks.data.source.StarType
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
@@ -27,7 +29,6 @@ import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -1071,13 +1072,20 @@ class SyncServiceTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
artists = listOf(artist("a1", "Alpha"), artist("combo", "Alpha & Beta")),
|
||||
artists =
|
||||
listOf(
|
||||
artist("a1", "Alpha").copy(playCount = 5, played = 100),
|
||||
artist("combo", "Alpha & Beta").copy(playCount = 5, played = 100),
|
||||
),
|
||||
albums = listOf(album("al1")),
|
||||
songs = listOf(song("s1")),
|
||||
songs = listOf(song("s1").copy(playCount = 5, played = 100)),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
|
||||
assertEquals(false, db.libraryDao().artistOnce(1, "combo")!!.credited)
|
||||
assertEquals(true, db.libraryDao().artistOnce(1, "a1")!!.credited)
|
||||
|
||||
assertEquals(
|
||||
listOf("a1"),
|
||||
db
|
||||
@@ -1086,6 +1094,23 @@ class SyncServiceTest {
|
||||
.allRows()
|
||||
.map { it.id },
|
||||
)
|
||||
val query = artistsDefaultQuery(1, 0, "alpha", ArtistSort.Name, false)
|
||||
assertEquals(
|
||||
listOf("a1"),
|
||||
db
|
||||
.libraryDao()
|
||||
.artistsByName(1, 0, "alpha")
|
||||
.allRows()
|
||||
.map { it.id },
|
||||
)
|
||||
assertEquals(
|
||||
listOf("a1"),
|
||||
db
|
||||
.libraryDao()
|
||||
.artistsPaging(query)
|
||||
.allRows()
|
||||
.map { it.id },
|
||||
)
|
||||
assertEquals(
|
||||
listOf("a1"),
|
||||
db
|
||||
@@ -1094,6 +1119,52 @@ class SyncServiceTest {
|
||||
.first()
|
||||
.map { it.id },
|
||||
)
|
||||
assertEquals(
|
||||
listOf("a1"),
|
||||
db
|
||||
.libraryDao()
|
||||
.homeMostPlayedArtists(1)
|
||||
.allRows()
|
||||
.map { it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun downloadedArtistsHidesNamesThatAreOnlyDisplayStrings() =
|
||||
runTest {
|
||||
insertSource()
|
||||
val source =
|
||||
FakeMusicSource(
|
||||
artists = listOf(artist("a1", "Alpha"), artist("combo", "Alpha & Beta")),
|
||||
albums =
|
||||
listOf(
|
||||
album("al1").copy(
|
||||
artistId = "combo",
|
||||
albumArtist = "Alpha & Beta",
|
||||
artists = listOf(ArtistRef("a1", "Alpha")),
|
||||
),
|
||||
),
|
||||
songs =
|
||||
listOf(
|
||||
song("s1").copy(
|
||||
artistId = "combo",
|
||||
artist = "Alpha & Beta",
|
||||
artists = listOf(ArtistRef("a1", "Alpha")),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
SyncService(db, source).sync()
|
||||
db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Completed))
|
||||
|
||||
assertEquals(
|
||||
listOf("a1"),
|
||||
db
|
||||
.libraryDao()
|
||||
.downloadedArtists(1)
|
||||
.allRows()
|
||||
.map { it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user