From a41f77d9076092d508de8d575f508c5b86fda869 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 18 Oct 2026 07:00:04 +0000 Subject: [PATCH] library: fix credited fallback and stale marker writes Restore the both-tables-empty fallback in the credit recompute, skip redundant marker writes, append albumless tracks after album tracks, and make the paged artist query observe sources. Add tests for an interrupted sync and the partial-relation migration guard. --- .../com/subtracks/data/db/LibraryDao.kt | 5 +- .../kotlin/com/subtracks/data/db/QueueDao.kt | 2 +- .../com/subtracks/data/db/SourcesDao.kt | 9 +++- .../subtracks/data/repo/SourceRepository.kt | 4 +- .../com/subtracks/data/db/MigrationsTest.kt | 26 +++++++++++ .../subtracks/data/sync/SyncServiceTest.kt | 46 ++++++++++++++++++- 6 files changed, 85 insertions(+), 7 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 9707ab96..058dd23e 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -18,6 +18,7 @@ import com.subtracks.data.model.Playlist import com.subtracks.data.model.PlaylistSong import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.Song +import com.subtracks.data.model.Source import kotlinx.coroutines.flow.Flow internal const val PLAYLIST_SONGS_SELECT = @@ -250,7 +251,7 @@ interface LibraryDao { "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) " + - "OR NOT EXISTS (SELECT 1 FROM song_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 " + @@ -567,7 +568,7 @@ interface LibraryDao { @RawQuery(observedEntities = [Album::class]) fun albumsPaging(query: RoomRawQuery): PagingSource - @RawQuery(observedEntities = [Artist::class]) + @RawQuery(observedEntities = [Artist::class, Source::class]) fun artistsPaging(query: RoomRawQuery): PagingSource @RawQuery(observedEntities = [Playlist::class]) diff --git a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt index 69605597..cfedc066 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/QueueDao.kt @@ -38,7 +38,7 @@ private const val ARTIST_SONGS_SELECT = "WHERE songs.sourceId = :sourceId AND songs.id IN $ARTIST_SONG_IDS" private const val ARTIST_SONGS_ORDER = - " ORDER BY albums.year DESC, albums.sortName COLLATE NOCASE, albums.id, songs.disc, songs.track, songs.id" + " ORDER BY albums.id IS NULL, albums.year DESC, albums.sortName COLLATE NOCASE, albums.id, songs.disc, songs.track, songs.id" private const val SONG_SQL = "SELECT songs.*, albums.coverArt AS coverArt FROM songs " + diff --git a/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt b/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt index 2b3adfeb..e290b35b 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt @@ -32,12 +32,19 @@ interface SourcesDao { @Query("SELECT creditsStale FROM sources WHERE id = :sourceId") suspend fun creditsStale(sourceId: Long): Boolean? - @Query("UPDATE sources SET creditsStale = :stale WHERE id = :sourceId") + @Query("UPDATE sources SET creditsStale = :stale WHERE id = :sourceId AND creditsStale != :stale") suspend fun setCreditsStale( sourceId: Long, stale: Boolean, ) + @Query("UPDATE sources SET name = :name, address = :address WHERE id = :sourceId") + suspend fun updateSourceDetails( + sourceId: Long, + name: String, + address: String, + ) + @Insert suspend fun insertSource(source: Source): Long diff --git a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt index a5b14b9f..a0ff04b3 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt @@ -220,8 +220,8 @@ class SourceRepository( useTokenAuth: Boolean, ) { val dao = db.sourcesDao() - val source = dao.sourceOnce(id) ?: return - dao.upsertSource(source.copy(name = name.trim(), address = normalizeAddress(address))) + if (dao.sourceOnce(id) == null) return + dao.updateSourceDetails(id, name.trim(), normalizeAddress(address)) dao.upsertSubsonicSource( SubsonicSource( sourceId = id, diff --git a/app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt b/app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt index b4eff2ca..c3cc3533 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt @@ -246,6 +246,32 @@ class MigrationsTest { } } + @Test + fun theCreditedBackfillKeepsArtistsVisibleWhileRelationsAreIncomplete() = + 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(1L, long(migrated, "SELECT credited FROM artists WHERE id = 'combo'")) + } + } + private fun text( connection: SQLiteConnection, sql: String, diff --git a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt index e68dc3b5..c8cfa42c 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt @@ -1167,6 +1167,44 @@ class SyncServiceTest { ) } + @Test + fun anInterruptedSyncLeavesTheCreditMarkerStale() = + runTest { + insertSource() + val source = + FakeMusicSource( + artists = listOf(artist("a1", "Alpha"), artist("combo", "Alpha & Beta")), + albums = listOf(album("al1")), + songs = listOf(song("s1")), + songsFailure = java.io.IOException("boom"), + ) + + runCatching { SyncService(db, source).sync() } + + assertEquals(true, db.sourcesDao().creditsStale(1)) + assertEquals( + listOf("a1", "combo"), + db + .libraryDao() + .artistsByName(1, 0, "") + .allRows() + .map { it.id }, + ) + + source.songsFailure = null + SyncService(db, source).sync() + + assertEquals(false, db.sourcesDao().creditsStale(1)) + assertEquals( + listOf("a1"), + db + .libraryDao() + .artistsByName(1, 0, "") + .allRows() + .map { it.id }, + ) + } + @Test fun aStaleCreditMarkerForcesARecomputeOnTheNextSync() = runTest { @@ -1513,6 +1551,7 @@ private class FakeMusicSource( var songs: List = emptyList(), var playlists: List = emptyList(), var playlistSongs: List = emptyList(), + var songsFailure: Throwable? = null, private val batchSize: Int = 1000, ) : MusicSource { val requestedPlaylistIds = mutableListOf>() @@ -1535,7 +1574,12 @@ private class FakeMusicSource( override fun albums(): Flow> = batches(albums) - override fun songs(): Flow> = batches(songs) + override fun songs(): Flow> = + if (songsFailure == null) { + batches(songs) + } else { + flow { throw songsFailure!! } + } override fun playlists(): Flow> = batches(playlists)