diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index 19950e8c..0b3d593a 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -505,8 +505,8 @@ class QueueRepository( upNextAnchor: Long, ) { invalidateLibraryCache() - writeLocked(entries) - writeUpNextLocked(upNext) + writeLocked(withOriginEntryIds(entries)) + writeUpNextLocked(withOriginEntryIds(upNext)) setShuffleLocked(shuffleSeed) cursorMutex.withLock { dao.setCursor(cursorRow().copy(queuePosition = 0, positionMs = 0, upNextAnchor = upNextAnchor)) @@ -520,9 +520,12 @@ class QueueRepository( val loaded = db.useReaderConnection { transactor -> transactor.deferredTransaction { - val entries = withOriginEntryIds(dao.entries()).map { ResolvedQueueEntry(it, lengthOf(it, libraryRevision)) } + val entries = + withOriginEntryIds(dao.entries(), assignFragmentIds = false).map { + ResolvedQueueEntry(it, lengthOf(it, libraryRevision)) + } val upNext = - withOriginEntryIds(dao.upNextEntries().map { it.toQueueEntry() }).map { entry -> + withOriginEntryIds(dao.upNextEntries().map { it.toQueueEntry() }, assignFragmentIds = false).map { entry -> ResolvedQueueEntry(entry, lengthOf(entry, libraryRevision)) } val row = dao.cursor() @@ -637,16 +640,10 @@ class QueueRepository( db.useWriterConnection { connection -> connection.immediateTransaction { dao.clear() - dao.insert( - withOriginEntryIds(entries).mapIndexed { index, entry -> - entry.copy(id = 0, position = index.toLong()) - }, - ) + dao.insert(entries.mapIndexed { index, entry -> entry.copy(id = 0, position = index.toLong()) }) dao.clearUpNext() dao.insertUpNext( - withOriginEntryIds(upNext).mapIndexed { index, entry -> - UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) - }, + upNext.mapIndexed { index, entry -> UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) }, ) dao.setCursor( row.copy( @@ -751,16 +748,23 @@ class QueueRepository( return kind == origin.kind && refId == origin.refId && rangeStart == null && rangeEnd == null } - private fun withOriginEntryIds(entries: List): List { - if (entries.all { it.originEntryId != null }) return entries + private fun QueueEntry.shouldAssignOriginEntryId(assignFragmentIds: Boolean) = + originEntryId == null && (assignFragmentIds || isWholeOrigin()) + + private fun withOriginEntryIds( + entries: List, + existingEntries: List = emptyList(), + assignFragmentIds: Boolean = true, + ): List { + if (entries.none { it.shouldAssignOriginEntryId(assignFragmentIds) }) return entries val nextIds = mutableMapOf() - for (entry in entries) { + for (entry in existingEntries + entries) { val id = entry.originEntryId ?: continue nextIds[entry.origin] = maxOf(nextIds[entry.origin] ?: 0L, id + 1) } val fragmentIds = mutableMapOf() return entries.map { entry -> - if (entry.originEntryId != null) return@map entry + if (!entry.shouldAssignOriginEntryId(assignFragmentIds)) return@map entry val origin = entry.origin val whole = entry.isWholeOrigin() val existingId = if (whole) null else fragmentIds[origin] @@ -781,7 +785,7 @@ class QueueRepository( val reset = mutableSetOf() for (entry in entries) { val origin = entry.origin - if (origin !in changed) { + if (origin !in changed || entry.originEntryId == null) { result += entry } else if (reset.add(entry.originEntryKey())) { result += @@ -941,7 +945,8 @@ class QueueRepository( list.size } }.coerceIn(0, list.size) - list.add(insertIndex, entry.copy(id = 0, position = 0)) + val newEntry = withOriginEntryIds(listOf(entry.copy(id = 0, position = 0)), list).single() + list.add(insertIndex, newEntry) val anchor = if (current.upNextSize == 0L) { val play = current.anchorContextPlay(currentPosition) @@ -1078,11 +1083,7 @@ class QueueRepository( db.useWriterConnection { transactor -> transactor.immediateTransaction { dao.clear() - dao.insert( - withOriginEntryIds(entries).mapIndexed { index, entry -> - entry.copy(id = 0, position = index.toLong()) - }, - ) + dao.insert(entries.mapIndexed { index, entry -> entry.copy(id = 0, position = index.toLong()) }) } } queueVersion.incrementAndGet() @@ -1093,9 +1094,7 @@ class QueueRepository( transactor.immediateTransaction { dao.clearUpNext() dao.insertUpNext( - withOriginEntryIds(entries).mapIndexed { index, entry -> - UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) - }, + entries.mapIndexed { index, entry -> UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) }, ) } } 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 8d4d48dd..06745823 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/MigrationsTest.kt @@ -1,9 +1,15 @@ package com.subtracks.data.db +import android.content.Context +import androidx.room3.Room import androidx.sqlite.SQLiteConnection import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.sqlite.execSQL +import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.QueueKind +import com.subtracks.data.model.QueueOrigin +import com.subtracks.data.repo.QueueRepository import kotlinx.coroutines.test.runTest import org.json.JSONObject import org.junit.Assert.assertEquals @@ -483,6 +489,59 @@ class MigrationsTest { } } + @Test + fun migratedFragmentedDuplicateEntriesAreLeftUnresetWhenTheirIdentityIsUnknown() = + runTest { + val context = ApplicationProvider.getApplicationContext() + val name = "migration-38-queue-${System.nanoTime()}.db" + val databaseFile = context.getDatabasePath(name) + databaseFile.parentFile?.mkdirs() + driver.open(databaseFile.absolutePath).use { migrated -> + applySchema(migrated, 38) + migrated.execSQL("PRAGMA user_version = 38") + migrated.execSQL( + "INSERT INTO sources (id, name, address, isActive, createdAt) VALUES (1, 'server', 'http://localhost', 1, 0)", + ) + migrated.execSQL( + "INSERT INTO songs (sourceId, id, title) VALUES " + + "(1, 's1', 'One'), (1, 's2', 'Two'), (1, 's3', 'Three')", + ) + migrated.execSQL( + "INSERT INTO playlist_songs (sourceId, playlistId, songId, position) VALUES " + + "(1, 'p1', 's1', 0), (1, 'p1', 's2', 1), (1, 'p1', 's3', 2)", + ) + migrated.execSQL( + "INSERT INTO queue_entries (position, sourceId, kind, refId, rangeStart, rangeEnd) VALUES " + + "(0, 1, 'Playlist', 'p1', 0, 0), (1, 1, 'Playlist', 'p1', 2, 2), " + + "(2, 1, 'Playlist', 'p1', 0, 0), (3, 1, 'Playlist', 'p1', 2, 2)", + ) + } + val db = + Room + .databaseBuilder(context, name) + .setDriver(driver) + .addMigrations(MIGRATION_38_39) + .build() + try { + val repository = QueueRepository(db) + val before = repository.snapshot() + val point = repository.captureResetPoint() + + repository.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point) + + val after = repository.snapshot() + assertEquals(4, after.entries.size) + assertEquals(before.entries, after.entries) + assertEquals( + listOf("s1", "s3", "s1", "s3"), + (0 until after.size).map { repository.itemAt(after, it)?.song?.id }, + ) + } finally { + db.close() + context.deleteDatabase(name) + } + } + private fun text( connection: SQLiteConnection, sql: String,