mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 07:29:14 +01:00
library: keep artist art maintenance self-healing and cheap
Move change detection into a desired CTE with UPDATE ... FROM so mass updates do not turn the trailing WHERE into a quadratic correlated scan, and so no-op passes write zero rows. Fold the trigger optimization into the 35->36 migration and re-check/rewrite it from onOpen, so a future migration cannot silently restore the full UPDATE triggers. Re-resolve artist detail art when the active source becomes ready, and cover artist search renames in a Room-level test.
This commit is contained in:
-1962
File diff suppressed because it is too large
Load Diff
@@ -118,6 +118,10 @@ internal object ArtistSearchTriggerOptimization : RoomDatabase.Callback() {
|
|||||||
override suspend fun onCreate(connection: SQLiteConnection) {
|
override suspend fun onCreate(connection: SQLiteConnection) {
|
||||||
optimizeArtistSearchTriggers(connection)
|
optimizeArtistSearchTriggers(connection)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override suspend fun onOpen(connection: SQLiteConnection) {
|
||||||
|
if (!artistSearchTriggersOptimized(connection)) optimizeArtistSearchTriggers(connection)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
internal object SqliteLifecycleLogging : RoomDatabase.Callback() {
|
internal object SqliteLifecycleLogging : RoomDatabase.Callback() {
|
||||||
|
|||||||
@@ -225,15 +225,15 @@ internal const val ARTIST_ART_UPDATE_SQL =
|
|||||||
"ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_downloaded), " +
|
"ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_downloaded), " +
|
||||||
"downloaded_covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
|
"downloaded_covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
|
||||||
"SELECT artistId, coverArt, coverRank FROM downloaded_deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
|
"SELECT artistId, coverArt, coverRank FROM downloaded_deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
|
||||||
") GROUP BY artistId) " +
|
") GROUP BY artistId), " +
|
||||||
"UPDATE artists SET " +
|
"desired AS (SELECT artists.rowid AS rowid, " +
|
||||||
"coverArts = COALESCE((SELECT coverArts FROM covers WHERE covers.artistId = artists.id), ''), " +
|
"COALESCE((SELECT coverArts FROM covers WHERE covers.artistId = artists.id), '') AS coverArts, " +
|
||||||
"downloadedCoverArts = COALESCE((SELECT coverArts FROM downloaded_covers " +
|
"COALESCE((SELECT coverArts FROM downloaded_covers " +
|
||||||
"WHERE downloaded_covers.artistId = artists.id), '') " +
|
"WHERE downloaded_covers.artistId = artists.id), '') AS downloadedCoverArts " +
|
||||||
"WHERE sourceId = :sourceId AND (" +
|
"FROM artists WHERE artists.sourceId = :sourceId) " +
|
||||||
"coverArts != COALESCE((SELECT coverArts FROM covers WHERE covers.artistId = artists.id), '') " +
|
"UPDATE artists SET coverArts = desired.coverArts, downloadedCoverArts = desired.downloadedCoverArts " +
|
||||||
"OR downloadedCoverArts != COALESCE((SELECT coverArts FROM downloaded_covers " +
|
"FROM desired WHERE artists.rowid = desired.rowid AND " +
|
||||||
"WHERE downloaded_covers.artistId = artists.id), ''))"
|
"(artists.coverArts != desired.coverArts OR artists.downloadedCoverArts != desired.downloadedCoverArts)"
|
||||||
|
|
||||||
internal const val DOWNLOADED_ARTIST_ART_UPDATE_SQL =
|
internal const val DOWNLOADED_ARTIST_ART_UPDATE_SQL =
|
||||||
"WITH downloaded_albums AS (SELECT DISTINCT artistId, albumId FROM ($DOWNLOADED_COVER_ALBUM_ROWS) " +
|
"WITH downloaded_albums AS (SELECT DISTINCT artistId, albumId FROM ($DOWNLOADED_COVER_ALBUM_ROWS) " +
|
||||||
@@ -248,12 +248,14 @@ internal const val DOWNLOADED_ARTIST_ART_UPDATE_SQL =
|
|||||||
"ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_downloaded), " +
|
"ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_downloaded), " +
|
||||||
"downloaded_covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
|
"downloaded_covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
|
||||||
"SELECT artistId, coverArt, coverRank FROM downloaded_deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
|
"SELECT artistId, coverArt, coverRank FROM downloaded_deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
|
||||||
") GROUP BY artistId) " +
|
") GROUP BY artistId), " +
|
||||||
"UPDATE artists SET downloadedCoverArts = COALESCE((SELECT coverArts FROM downloaded_covers " +
|
"desired AS (SELECT artists.rowid AS rowid, " +
|
||||||
"WHERE downloaded_covers.artistId = artists.id), '') " +
|
"COALESCE((SELECT coverArts FROM downloaded_covers " +
|
||||||
"WHERE sourceId = :sourceId AND id IN (:artistIds) AND " +
|
"WHERE downloaded_covers.artistId = artists.id), '') AS downloadedCoverArts " +
|
||||||
"downloadedCoverArts != COALESCE((SELECT coverArts FROM downloaded_covers " +
|
"FROM artists WHERE artists.sourceId = :sourceId AND artists.id IN (:artistIds)) " +
|
||||||
"WHERE downloaded_covers.artistId = artists.id), '')"
|
"UPDATE artists SET downloadedCoverArts = desired.downloadedCoverArts " +
|
||||||
|
"FROM desired WHERE artists.rowid = desired.rowid AND " +
|
||||||
|
"artists.downloadedCoverArts != desired.downloadedCoverArts"
|
||||||
|
|
||||||
internal const val ARTISTS_FILTER =
|
internal const val ARTISTS_FILTER =
|
||||||
"FROM artists WHERE sourceId = :sourceId " +
|
"FROM artists WHERE sourceId = :sourceId " +
|
||||||
|
|||||||
@@ -701,15 +701,19 @@ val MIGRATION_35_36 =
|
|||||||
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `artistImageUrl` TEXT")
|
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `artistImageUrl` TEXT")
|
||||||
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `coverArts` TEXT NOT NULL DEFAULT ''")
|
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `coverArts` TEXT NOT NULL DEFAULT ''")
|
||||||
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `downloadedCoverArts` TEXT NOT NULL DEFAULT ''")
|
connection.execSQL("ALTER TABLE `artists` ADD COLUMN `downloadedCoverArts` TEXT NOT NULL DEFAULT ''")
|
||||||
|
optimizeArtistSearchTriggers(connection)
|
||||||
backfillArtistArt(connection)
|
backfillArtistArt(connection)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
val MIGRATION_36_37 =
|
internal fun artistSearchTriggersOptimized(connection: SQLiteConnection): Boolean {
|
||||||
object : Migration(36, 37) {
|
val triggers = mutableListOf<String>()
|
||||||
override suspend fun migrate(connection: SQLiteConnection) {
|
connection.prepare("SELECT sql FROM sqlite_master WHERE type = 'trigger' AND name IN (?, ?)").use { statement ->
|
||||||
optimizeArtistSearchTriggers(connection)
|
statement.bindText(1, "room_fts_content_sync_artist_search_BEFORE_UPDATE")
|
||||||
|
statement.bindText(2, "room_fts_content_sync_artist_search_AFTER_UPDATE")
|
||||||
|
while (statement.step()) triggers += statement.getText(0)
|
||||||
}
|
}
|
||||||
|
return triggers.size == 2 && triggers.all { it.contains("UPDATE OF `name`, `sortName`") }
|
||||||
}
|
}
|
||||||
|
|
||||||
internal fun optimizeArtistSearchTriggers(connection: SQLiteConnection) {
|
internal fun optimizeArtistSearchTriggers(connection: SQLiteConnection) {
|
||||||
@@ -778,5 +782,4 @@ val MIGRATIONS: Array<Migration> =
|
|||||||
MIGRATION_33_34,
|
MIGRATION_33_34,
|
||||||
MIGRATION_34_35,
|
MIGRATION_34_35,
|
||||||
MIGRATION_35_36,
|
MIGRATION_35_36,
|
||||||
MIGRATION_36_37,
|
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -57,7 +57,7 @@ import com.subtracks.data.model.UpNextEntry
|
|||||||
SongDownload::class,
|
SongDownload::class,
|
||||||
PendingAction::class,
|
PendingAction::class,
|
||||||
],
|
],
|
||||||
version = 37,
|
version = 36,
|
||||||
exportSchema = true,
|
exportSchema = true,
|
||||||
)
|
)
|
||||||
@ColumnTypeConverters(
|
@ColumnTypeConverters(
|
||||||
|
|||||||
@@ -70,6 +70,9 @@ class SourceRepository(
|
|||||||
private val _offline = MutableStateFlow(false)
|
private val _offline = MutableStateFlow(false)
|
||||||
val offline: StateFlow<Boolean> = _offline
|
val offline: StateFlow<Boolean> = _offline
|
||||||
|
|
||||||
|
private val _activeSource = MutableStateFlow<Long?>(null)
|
||||||
|
val activeSource: StateFlow<Long?> = _activeSource
|
||||||
|
|
||||||
fun downloadsAllowedOverMetered(): Boolean = allowMeteredDownloads
|
fun downloadsAllowedOverMetered(): Boolean = allowMeteredDownloads
|
||||||
|
|
||||||
fun setOfflineMode(enabled: Boolean) {
|
fun setOfflineMode(enabled: Boolean) {
|
||||||
@@ -110,6 +113,7 @@ class SourceRepository(
|
|||||||
Log.i("net", "active source -> ${config?.name ?: "none"}")
|
Log.i("net", "active source -> ${config?.name ?: "none"}")
|
||||||
}
|
}
|
||||||
active = config?.let { ActiveSource(it.id, it.toMusicSource(quality, concurrency)) }
|
active = config?.let { ActiveSource(it.id, it.toMusicSource(quality, concurrency)) }
|
||||||
|
_activeSource.value = config?.id
|
||||||
_quality.value = quality
|
_quality.value = quality
|
||||||
if (sourceChanged && config != null && config.useTokenAuth && !_offline.value) {
|
if (sourceChanged && config != null && config.useTokenAuth && !_offline.value) {
|
||||||
scope.launch {
|
scope.launch {
|
||||||
|
|||||||
@@ -17,7 +17,6 @@ import com.subtracks.ui.components.ArtistArt
|
|||||||
import com.subtracks.ui.components.artistArt
|
import com.subtracks.ui.components.artistArt
|
||||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||||
import kotlinx.coroutines.Job
|
import kotlinx.coroutines.Job
|
||||||
import kotlinx.coroutines.delay
|
|
||||||
import kotlinx.coroutines.flow.Flow
|
import kotlinx.coroutines.flow.Flow
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.SharingStarted
|
import kotlinx.coroutines.flow.SharingStarted
|
||||||
@@ -143,8 +142,13 @@ class ArtistDetailViewModel(
|
|||||||
|
|
||||||
init {
|
init {
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
combine(artist.filterNotNull(), sourceRepository.offline) { loaded, _ -> loaded }
|
combine(
|
||||||
.collectLatest { loaded ->
|
artist.filterNotNull(),
|
||||||
|
sourceRepository.offline,
|
||||||
|
sourceRepository.activeSource,
|
||||||
|
) { loaded, _, activeSource -> loaded to activeSource }
|
||||||
|
.collectLatest { (loaded, activeSource) ->
|
||||||
|
if (activeSource == null) return@collectLatest
|
||||||
val hero = sourceRepository.artistArt(loaded, forIcon = false)
|
val hero = sourceRepository.artistArt(loaded, forIcon = false)
|
||||||
val icon = sourceRepository.artistArt(loaded, forIcon = true)
|
val icon = sourceRepository.artistArt(loaded, forIcon = true)
|
||||||
_art.value = hero.image ?: hero.covers.firstOrNull()
|
_art.value = hero.image ?: hero.covers.firstOrNull()
|
||||||
|
|||||||
@@ -682,6 +682,24 @@ class LibraryDaoTest {
|
|||||||
assertEquals(listOf("art-2", "art-1"), dao.artistOnce(sourceId, "ar-1")!!.downloadedCoverArts)
|
assertEquals(listOf("art-2", "art-1"), dao.artistOnce(sourceId, "ar-1")!!.downloadedCoverArts)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun artistNameUpdatesKeepTheSearchIndexValid() =
|
||||||
|
runTest {
|
||||||
|
val sourceId = source()
|
||||||
|
val dao = db.libraryDao()
|
||||||
|
dao.upsertArtists(listOf(artist(sourceId, "ar-1", "Artist", albumCount = 0)))
|
||||||
|
dao.upsertArtists(listOf(artist(sourceId, "ar-1", "Renamed", albumCount = 0)))
|
||||||
|
|
||||||
|
val hits =
|
||||||
|
db.useReaderConnection { connection ->
|
||||||
|
connection.usePrepared("SELECT COUNT(*) FROM artist_search WHERE artist_search MATCH 'Renamed'") {
|
||||||
|
it.step()
|
||||||
|
it.getLong(0)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assertEquals(1L, hits)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun artistArtDeduplicatesCoversBeforeApplyingTheFourCoverBudget() =
|
fun artistArtDeduplicatesCoversBeforeApplyingTheFourCoverBudget() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -222,9 +222,10 @@ class MigrationsTest {
|
|||||||
fun theArtistSearchTriggersIgnoreArtOnlyUpdates() =
|
fun theArtistSearchTriggersIgnoreArtOnlyUpdates() =
|
||||||
runTest {
|
runTest {
|
||||||
driver.open(":memory:").use { migrated ->
|
driver.open(":memory:").use { migrated ->
|
||||||
applySchema(migrated, 36)
|
applySchema(migrated, 35)
|
||||||
MIGRATION_36_37.migrate(migrated)
|
MIGRATION_35_36.migrate(migrated)
|
||||||
|
|
||||||
|
assertTrue(artistSearchTriggersOptimized(migrated))
|
||||||
assertTrue(
|
assertTrue(
|
||||||
triggerSql(migrated, "room_fts_content_sync_artist_search_BEFORE_UPDATE")
|
triggerSql(migrated, "room_fts_content_sync_artist_search_BEFORE_UPDATE")
|
||||||
.contains("UPDATE OF `name`, `sortName`"),
|
.contains("UPDATE OF `name`, `sortName`"),
|
||||||
@@ -240,7 +241,7 @@ class MigrationsTest {
|
|||||||
fun theFreshDatabaseCallbackOptimizesTheArtistSearchTriggers() =
|
fun theFreshDatabaseCallbackOptimizesTheArtistSearchTriggers() =
|
||||||
runTest {
|
runTest {
|
||||||
driver.open(":memory:").use { connection ->
|
driver.open(":memory:").use { connection ->
|
||||||
applySchema(connection, 37)
|
applySchema(connection, 36)
|
||||||
connection.execSQL(
|
connection.execSQL(
|
||||||
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 's', 'http://x', 1, 0)",
|
"INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 's', 'http://x', 1, 0)",
|
||||||
)
|
)
|
||||||
@@ -249,15 +250,12 @@ class MigrationsTest {
|
|||||||
"VALUES (1, 'ar1', 'Artist', 0, NULL)",
|
"VALUES (1, 'ar1', 'Artist', 0, NULL)",
|
||||||
)
|
)
|
||||||
|
|
||||||
ArtistSearchTriggerOptimization.onCreate(connection)
|
ArtistSearchTriggerOptimization.onOpen(connection)
|
||||||
|
assertTrue(artistSearchTriggersOptimized(connection))
|
||||||
val before = long(connection, "SELECT total_changes()")
|
val before = long(connection, "SELECT total_changes()")
|
||||||
connection.execSQL("UPDATE artists SET coverArts = 'a' WHERE id = 'ar1'")
|
connection.execSQL("UPDATE artists SET coverArts = 'a' WHERE id = 'ar1'")
|
||||||
|
|
||||||
assertEquals(before + 1, long(connection, "SELECT total_changes()"))
|
assertEquals(before + 1, long(connection, "SELECT total_changes()"))
|
||||||
assertTrue(
|
|
||||||
triggerSql(connection, "room_fts_content_sync_artist_search_AFTER_UPDATE")
|
|
||||||
.contains("UPDATE OF `name`, `sortName`"),
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -546,6 +544,10 @@ class MigrationsTest {
|
|||||||
for (i in 0 until indices.length()) {
|
for (i in 0 until indices.length()) {
|
||||||
connection.execSQL(indices.getJSONObject(i).getString("createSql").replace("\${TABLE_NAME}", name))
|
connection.execSQL(indices.getJSONObject(i).getString("createSql").replace("\${TABLE_NAME}", name))
|
||||||
}
|
}
|
||||||
|
val triggers = entity.optJSONArray("contentSyncTriggers") ?: continue
|
||||||
|
for (i in 0 until triggers.length()) {
|
||||||
|
connection.execSQL(triggers.getString(i))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -39,7 +39,12 @@ class QueryTimingMeasurementTest {
|
|||||||
fun setUp() {
|
fun setUp() {
|
||||||
val context = ApplicationProvider.getApplicationContext<Context>()
|
val context = ApplicationProvider.getApplicationContext<Context>()
|
||||||
val driver = LoggingSQLiteDriver(BundledSQLiteDriver()) { executions += it }
|
val driver = LoggingSQLiteDriver(BundledSQLiteDriver()) { executions += it }
|
||||||
db = Room.inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java).setDriver(driver).build()
|
db =
|
||||||
|
Room
|
||||||
|
.inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java)
|
||||||
|
.setDriver(driver)
|
||||||
|
.addCallback(ArtistSearchTriggerOptimization)
|
||||||
|
.build()
|
||||||
}
|
}
|
||||||
|
|
||||||
@After
|
@After
|
||||||
|
|||||||
@@ -285,6 +285,15 @@ class SourceRepositoryTest {
|
|||||||
assertTrue("expected the stored file, was ${ref.url}", ref.url.startsWith("file:"))
|
assertTrue("expected the stored file, was ${ref.url}", ref.url.startsWith("file:"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun activeSourceBecomesReadyAfterThePreferencesLoad() =
|
||||||
|
runBlocking {
|
||||||
|
repository.addSource("nav", "http://a.example/", "u", "p", false)
|
||||||
|
|
||||||
|
val active = withTimeout(TEST_TIMEOUT_MS) { repository.activeSource.first { it != null } }
|
||||||
|
assertEquals(repository.activeSourceId().first(), active)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun artistImageUsesTheNetworkOnlineAndOnlyCachedFilesOffline() =
|
fun artistImageUsesTheNetworkOnlineAndOnlyCachedFilesOffline() =
|
||||||
runBlocking {
|
runBlocking {
|
||||||
|
|||||||
Reference in New Issue
Block a user