library: finish the credit query review nits

Put the download-visibility comment on the constant it describes, keep the
no-scan guards beside the seek assertions, and give the identical-sync test a
positive control for both the artist aggregate and the album play recompute.
This commit is contained in:
austinried
2026-10-19 02:03:05 +00:00
parent 0ac67a124d
commit 824579c938
3 changed files with 10 additions and 3 deletions
@@ -112,13 +112,13 @@ internal const val CREDITED_ALBUM_ROWS =
"JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " +
"WHERE song_artists.sourceId = :sourceId" "WHERE song_artists.sourceId = :sourceId"
// Download visibility follows the credit path: an album credit includes the album when any song is
// downloaded, while a track credit includes it only when the credited track is downloaded.
internal const val ALBUMS_FOR_ARTIST = internal const val ALBUMS_FOR_ARTIST =
"SELECT albums.* FROM (SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) WHERE artistId = :artistId) credits " + "SELECT albums.* FROM (SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) WHERE artistId = :artistId) credits " +
"JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " + "JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " +
"ORDER BY year DESC, sortName COLLATE NOCASE, id" "ORDER BY year DESC, sortName COLLATE NOCASE, id"
// Download visibility follows the credit path: an album credit includes the album when any song is
// downloaded, while a track credit includes it only when the credited track is downloaded.
private const val DOWNLOADED_ARTIST_ALBUMS = private const val DOWNLOADED_ARTIST_ALBUMS =
"SELECT albums.id FROM albums " + "SELECT albums.id FROM albums " +
"WHERE albums.sourceId = :sourceId AND albums.artistId = artists.id " + "WHERE albums.sourceId = :sourceId AND albums.artistId = artists.id " +
@@ -871,6 +871,8 @@ class LibraryDaoTest {
plan, plan,
plan.contains("USING INDEX index_song_artists_sourceId_artistId (sourceId=? AND artistId=?)"), plan.contains("USING INDEX index_song_artists_sourceId_artistId (sourceId=? AND artistId=?)"),
) )
assertFalse(plan, plan.contains("SCAN albums"))
assertFalse(plan, plan.contains("SCAN songs"))
} }
private suspend fun plan(sql: String): String = private suspend fun plan(sql: String): String =
@@ -1765,7 +1765,7 @@ class SyncServiceTest {
} }
@Test @Test
fun anIdenticalSyncSkipsTheArtistAggregateQuery() = fun anIdenticalSyncSkipsTheAggregateQueries() =
runTest { runTest {
val context = ApplicationProvider.getApplicationContext<Context>() val context = ApplicationProvider.getApplicationContext<Context>()
val executions = Collections.synchronizedList(mutableListOf<SqlExecution>()) val executions = Collections.synchronizedList(mutableListOf<SqlExecution>())
@@ -1786,11 +1786,16 @@ class SyncServiceTest {
) )
SyncService(loggingDb, source).sync() SyncService(loggingDb, source).sync()
val firstSyncExecutions = synchronized(executions) { executions.toList() }
assertTrue(firstSyncExecutions.any { it.sql.contains("credited_albums") })
assertTrue(firstSyncExecutions.any { it.sql.contains("UPDATE albums SET playCount") })
synchronized(executions) { executions.clear() } synchronized(executions) { executions.clear() }
SyncService(loggingDb, source).sync() SyncService(loggingDb, source).sync()
val secondSyncExecutions = synchronized(executions) { executions.toList() } val secondSyncExecutions = synchronized(executions) { executions.toList() }
assertTrue(secondSyncExecutions.none { it.sql.contains("credited_albums") }) assertTrue(secondSyncExecutions.none { it.sql.contains("credited_albums") })
assertTrue(secondSyncExecutions.none { it.sql.contains("UPDATE albums SET playCount") })
} finally { } finally {
loggingDb.close() loggingDb.close()
} }