From 320a9db5977c3dc1aa97897f2480aaca5ddacd28 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:20:50 +0000 Subject: [PATCH] stop syncing from rewriting unchanged rows --- .../com/subtracks/data/db/LibraryDao.kt | 8 +- .../com/subtracks/data/sync/SyncService.kt | 132 ++++++++++++++++-- .../subtracks/data/sync/SyncServiceTest.kt | 57 ++++++++ 3 files changed, 184 insertions(+), 13 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 acef4ec6..57e54a21 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -82,8 +82,12 @@ interface LibraryDao { ids: Collection, ) - @Query("DELETE FROM playlist_songs WHERE sourceId = :sourceId") - suspend fun deletePlaylistSongs(sourceId: Long) + @Query("DELETE FROM playlist_songs WHERE sourceId = :sourceId AND playlistId = :playlistId AND position >= :position") + suspend fun deletePlaylistSongsFrom( + sourceId: Long, + playlistId: String, + position: Long, + ) @Query("SELECT * FROM artists WHERE sourceId = :sourceId ORDER BY name COLLATE NOCASE") fun artists(sourceId: Long): PagingSource diff --git a/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt b/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt index 84b98d8d..7f884493 100644 --- a/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt +++ b/app/src/main/kotlin/com/subtracks/data/sync/SyncService.kt @@ -1,9 +1,16 @@ package com.subtracks.data.sync +import androidx.room3.PooledConnection import androidx.room3.immediateTransaction import androidx.room3.useWriterConnection +import androidx.sqlite.SQLiteStatement import com.subtracks.data.db.SubtracksDatabase +import com.subtracks.data.model.Album +import com.subtracks.data.model.Artist +import com.subtracks.data.model.Playlist +import com.subtracks.data.model.PlaylistSong import com.subtracks.data.model.SearchIndex +import com.subtracks.data.model.Song import com.subtracks.data.source.MusicSource import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -24,19 +31,28 @@ class SyncService( db.useWriterConnection { transactor -> transactor.immediateTransaction { + upsertChanged("artists", artistColumns, primaryKey, artists) { it.values() } + upsertChanged("albums", albumColumns, primaryKey, albums) { it.values() } + upsertChanged("songs", songColumns, primaryKey, songs) { it.values() } + upsertChanged("playlists", playlistColumns, primaryKey, playlists) { it.values() } + upsertChanged("playlist_songs", playlistSongColumns, playlistSongKey, playlistSongs) { it.values() } + val library = db.libraryDao() - library.upsertArtists(artists) - library.upsertAlbums(albums) - library.upsertSongs(songs) - library.upsertPlaylists(playlists) + val existingArtistIds = library.artistIds(source.id) + val existingAlbumIds = library.albumIds(source.id) + val existingSongIds = library.songIds(source.id) + val existingPlaylistIds = library.playlistIds(source.id) - deleteMissing(library.artistIds(source.id), artists.map { it.id }) { library.deleteArtists(source.id, it) } - deleteMissing(library.albumIds(source.id), albums.map { it.id }) { library.deleteAlbums(source.id, it) } - deleteMissing(library.songIds(source.id), songs.map { it.id }) { library.deleteSongs(source.id, it) } - deleteMissing(library.playlistIds(source.id), playlists.map { it.id }) { library.deletePlaylists(source.id, it) } + prune(existingArtistIds, artists.map { it.id }) { library.deleteArtists(source.id, it) } + prune(existingAlbumIds, albums.map { it.id }) { library.deleteAlbums(source.id, it) } + prune(existingSongIds, songs.map { it.id }) { library.deleteSongs(source.id, it) } + prune(existingPlaylistIds, playlists.map { it.id }) { library.deletePlaylists(source.id, it) } - library.deletePlaylistSongs(source.id) - library.upsertPlaylistSongs(playlistSongs) + val playlistSongCounts = playlistSongs.groupingBy { it.playlistId }.eachCount() + playlists.forEach { playlist -> + library.deletePlaylistSongsFrom(source.id, playlist.id, (playlistSongCounts[playlist.id] ?: 0).toLong()) + } + (existingPlaylistIds - playlists.map { it.id }).forEach { library.deletePlaylistSongsFrom(source.id, it, 0) } val search = db.searchDao() val sourceKey = source.id.toString() @@ -53,7 +69,7 @@ class SyncService( } } - private suspend fun deleteMissing( + private suspend fun prune( existing: List, current: List, delete: suspend (Collection) -> Unit, @@ -65,3 +81,97 @@ class SyncService( .forEach { delete(it) } } } + +private val primaryKey = listOf("sourceId", "id") + +private val artistColumns = listOf("sourceId", "id", "name", "albumCount", "starred", "coverArt") + +private val albumColumns = + listOf( + "sourceId", + "id", + "artistId", + "name", + "albumArtist", + "created", + "coverArt", + "genre", + "year", + "starred", + "songCount", + "frequentRank", + "recentRank", + ) + +private val songColumns = + listOf( + "sourceId", + "id", + "albumId", + "artistId", + "title", + "album", + "artist", + "duration", + "track", + "disc", + "starred", + "genre", + ) + +private val playlistColumns = listOf("sourceId", "id", "name", "comment", "coverArt", "songCount", "created") + +private val playlistSongColumns = listOf("sourceId", "playlistId", "songId", "position") + +private val playlistSongKey = listOf("sourceId", "playlistId", "position") + +private fun Artist.values() = listOf(sourceId, id, name, albumCount, starred, coverArt) + +private fun Album.values() = + listOf(sourceId, id, artistId, name, albumArtist, created, coverArt, genre, year, starred, songCount, frequentRank, recentRank) + +private fun Song.values() = listOf(sourceId, id, albumId, artistId, title, album, artist, duration, track, disc, starred, genre) + +private fun Playlist.values() = listOf(sourceId, id, name, comment, coverArt, songCount, created) + +private fun PlaylistSong.values() = listOf(sourceId, playlistId, songId, position) + +private suspend fun PooledConnection.upsertChanged( + table: String, + columns: List, + key: List, + rows: List, + values: (T) -> List, +) { + if (rows.isEmpty()) return + val updates = columns.filterNot { it in key } + val sql = + "INSERT INTO $table (${columns.joinToString()}) VALUES (${columns.joinToString { "?" }}) " + + "ON CONFLICT(${key.joinToString()}) DO UPDATE SET " + + updates.joinToString { "$it = excluded.$it" } + + " WHERE " + + updates.joinToString(" OR ") { "$table.$it IS NOT excluded.$it" } + usePrepared(sql) { statement -> + rows.forEach { row -> + values(row).forEachIndexed { index, value -> statement.bind(index + 1, value) } + statement.step() + statement.reset() + statement.clearBindings() + } + } +} + +private fun SQLiteStatement.bind( + index: Int, + value: Any?, +) { + when (value) { + null -> bindNull(index) + is Long -> bindLong(index, value) + is Int -> bindLong(index, value.toLong()) + is Boolean -> bindLong(index, if (value) 1L else 0L) + is Double -> bindDouble(index, value) + is String -> bindText(index, value) + else -> error("Cannot bind ${value::class}") + } +} 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 24efd3e5..7ef2a9ca 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt @@ -14,6 +14,7 @@ import com.subtracks.data.model.PlaylistSong import com.subtracks.data.model.Song import com.subtracks.data.model.Source import com.subtracks.data.source.MusicSource +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals @@ -103,6 +104,62 @@ class SyncServiceTest { ) } + @Test + fun aSecondIdenticalSyncLeavesTheLibraryUntouched() = + runTest { + insertSource() + val source = + FakeMusicSource( + artists = listOf(artist("a1"), artist("a2")), + albums = listOf(album("al1")), + songs = listOf(song("s1")), + playlists = listOf(playlist("p1")), + playlistSongs = listOf(PlaylistSong(1, "p1", "s1", 0)), + ) + + SyncService(db, source).sync() + SyncService(db, source).sync() + + assertEquals(2, db.libraryDao().artistIds(1).size) + assertEquals(1, db.libraryDao().albumIds(1).size) + assertEquals(1, db.libraryDao().songIds(1).size) + assertEquals(1, db.libraryDao().playlistIds(1).size) + assertEquals( + 1, + db + .libraryDao() + .playlistSongs(1, "p1") + .allRows() + .size, + ) + } + + @Test + fun albumsAreUpsertedAndPruned() = + runTest { + insertSource() + val source = FakeMusicSource(albums = listOf(album("al1"), album("al2"))) + + SyncService(db, source).sync() + SyncService(db, source).sync() + + assertEquals(listOf("al1", "al2"), db.libraryDao().albumIds(1).sorted()) + + source.albums = listOf(album("al1").copy(name = "Renamed")) + + SyncService(db, source).sync() + + assertEquals(listOf("al1"), db.libraryDao().albumIds(1)) + assertEquals( + "Renamed", + db + .libraryDao() + .album(1, "al1") + .first() + ?.name, + ) + } + @Test fun searchIndexMatchesSubstrings() = runTest {