mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-15 00:42:54 +01:00
queue: leave ambiguous migrated fragments unreset
This commit is contained in:
@@ -505,8 +505,8 @@ class QueueRepository(
|
|||||||
upNextAnchor: Long,
|
upNextAnchor: Long,
|
||||||
) {
|
) {
|
||||||
invalidateLibraryCache()
|
invalidateLibraryCache()
|
||||||
writeLocked(entries)
|
writeLocked(withOriginEntryIds(entries))
|
||||||
writeUpNextLocked(upNext)
|
writeUpNextLocked(withOriginEntryIds(upNext))
|
||||||
setShuffleLocked(shuffleSeed)
|
setShuffleLocked(shuffleSeed)
|
||||||
cursorMutex.withLock {
|
cursorMutex.withLock {
|
||||||
dao.setCursor(cursorRow().copy(queuePosition = 0, positionMs = 0, upNextAnchor = upNextAnchor))
|
dao.setCursor(cursorRow().copy(queuePosition = 0, positionMs = 0, upNextAnchor = upNextAnchor))
|
||||||
@@ -520,9 +520,12 @@ class QueueRepository(
|
|||||||
val loaded =
|
val loaded =
|
||||||
db.useReaderConnection { transactor ->
|
db.useReaderConnection { transactor ->
|
||||||
transactor.deferredTransaction {
|
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 =
|
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))
|
ResolvedQueueEntry(entry, lengthOf(entry, libraryRevision))
|
||||||
}
|
}
|
||||||
val row = dao.cursor()
|
val row = dao.cursor()
|
||||||
@@ -637,16 +640,10 @@ class QueueRepository(
|
|||||||
db.useWriterConnection { connection ->
|
db.useWriterConnection { connection ->
|
||||||
connection.immediateTransaction {
|
connection.immediateTransaction {
|
||||||
dao.clear()
|
dao.clear()
|
||||||
dao.insert(
|
dao.insert(entries.mapIndexed { index, entry -> entry.copy(id = 0, position = index.toLong()) })
|
||||||
withOriginEntryIds(entries).mapIndexed { index, entry ->
|
|
||||||
entry.copy(id = 0, position = index.toLong())
|
|
||||||
},
|
|
||||||
)
|
|
||||||
dao.clearUpNext()
|
dao.clearUpNext()
|
||||||
dao.insertUpNext(
|
dao.insertUpNext(
|
||||||
withOriginEntryIds(upNext).mapIndexed { index, entry ->
|
upNext.mapIndexed { index, entry -> UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) },
|
||||||
UpNextEntry.from(entry).copy(id = 0, position = index.toLong())
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
dao.setCursor(
|
dao.setCursor(
|
||||||
row.copy(
|
row.copy(
|
||||||
@@ -751,16 +748,23 @@ class QueueRepository(
|
|||||||
return kind == origin.kind && refId == origin.refId && rangeStart == null && rangeEnd == null
|
return kind == origin.kind && refId == origin.refId && rangeStart == null && rangeEnd == null
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun withOriginEntryIds(entries: List<QueueEntry>): List<QueueEntry> {
|
private fun QueueEntry.shouldAssignOriginEntryId(assignFragmentIds: Boolean) =
|
||||||
if (entries.all { it.originEntryId != null }) return entries
|
originEntryId == null && (assignFragmentIds || isWholeOrigin())
|
||||||
|
|
||||||
|
private fun withOriginEntryIds(
|
||||||
|
entries: List<QueueEntry>,
|
||||||
|
existingEntries: List<QueueEntry> = emptyList(),
|
||||||
|
assignFragmentIds: Boolean = true,
|
||||||
|
): List<QueueEntry> {
|
||||||
|
if (entries.none { it.shouldAssignOriginEntryId(assignFragmentIds) }) return entries
|
||||||
val nextIds = mutableMapOf<QueueOrigin, Long>()
|
val nextIds = mutableMapOf<QueueOrigin, Long>()
|
||||||
for (entry in entries) {
|
for (entry in existingEntries + entries) {
|
||||||
val id = entry.originEntryId ?: continue
|
val id = entry.originEntryId ?: continue
|
||||||
nextIds[entry.origin] = maxOf(nextIds[entry.origin] ?: 0L, id + 1)
|
nextIds[entry.origin] = maxOf(nextIds[entry.origin] ?: 0L, id + 1)
|
||||||
}
|
}
|
||||||
val fragmentIds = mutableMapOf<QueueOrigin, Long>()
|
val fragmentIds = mutableMapOf<QueueOrigin, Long>()
|
||||||
return entries.map { entry ->
|
return entries.map { entry ->
|
||||||
if (entry.originEntryId != null) return@map entry
|
if (!entry.shouldAssignOriginEntryId(assignFragmentIds)) return@map entry
|
||||||
val origin = entry.origin
|
val origin = entry.origin
|
||||||
val whole = entry.isWholeOrigin()
|
val whole = entry.isWholeOrigin()
|
||||||
val existingId = if (whole) null else fragmentIds[origin]
|
val existingId = if (whole) null else fragmentIds[origin]
|
||||||
@@ -781,7 +785,7 @@ class QueueRepository(
|
|||||||
val reset = mutableSetOf<OriginEntryKey>()
|
val reset = mutableSetOf<OriginEntryKey>()
|
||||||
for (entry in entries) {
|
for (entry in entries) {
|
||||||
val origin = entry.origin
|
val origin = entry.origin
|
||||||
if (origin !in changed) {
|
if (origin !in changed || entry.originEntryId == null) {
|
||||||
result += entry
|
result += entry
|
||||||
} else if (reset.add(entry.originEntryKey())) {
|
} else if (reset.add(entry.originEntryKey())) {
|
||||||
result +=
|
result +=
|
||||||
@@ -941,7 +945,8 @@ class QueueRepository(
|
|||||||
list.size
|
list.size
|
||||||
}
|
}
|
||||||
}.coerceIn(0, 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 =
|
val anchor =
|
||||||
if (current.upNextSize == 0L) {
|
if (current.upNextSize == 0L) {
|
||||||
val play = current.anchorContextPlay(currentPosition)
|
val play = current.anchorContextPlay(currentPosition)
|
||||||
@@ -1078,11 +1083,7 @@ class QueueRepository(
|
|||||||
db.useWriterConnection { transactor ->
|
db.useWriterConnection { transactor ->
|
||||||
transactor.immediateTransaction {
|
transactor.immediateTransaction {
|
||||||
dao.clear()
|
dao.clear()
|
||||||
dao.insert(
|
dao.insert(entries.mapIndexed { index, entry -> entry.copy(id = 0, position = index.toLong()) })
|
||||||
withOriginEntryIds(entries).mapIndexed { index, entry ->
|
|
||||||
entry.copy(id = 0, position = index.toLong())
|
|
||||||
},
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
queueVersion.incrementAndGet()
|
queueVersion.incrementAndGet()
|
||||||
@@ -1093,9 +1094,7 @@ class QueueRepository(
|
|||||||
transactor.immediateTransaction {
|
transactor.immediateTransaction {
|
||||||
dao.clearUpNext()
|
dao.clearUpNext()
|
||||||
dao.insertUpNext(
|
dao.insertUpNext(
|
||||||
withOriginEntryIds(entries).mapIndexed { index, entry ->
|
entries.mapIndexed { index, entry -> UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) },
|
||||||
UpNextEntry.from(entry).copy(id = 0, position = index.toLong())
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,9 +1,15 @@
|
|||||||
package com.subtracks.data.db
|
package com.subtracks.data.db
|
||||||
|
|
||||||
|
import android.content.Context
|
||||||
|
import androidx.room3.Room
|
||||||
import androidx.sqlite.SQLiteConnection
|
import androidx.sqlite.SQLiteConnection
|
||||||
import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
import androidx.sqlite.driver.bundled.BundledSQLiteDriver
|
||||||
import androidx.sqlite.execSQL
|
import androidx.sqlite.execSQL
|
||||||
|
import androidx.test.core.app.ApplicationProvider
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
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 kotlinx.coroutines.test.runTest
|
||||||
import org.json.JSONObject
|
import org.json.JSONObject
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
@@ -483,6 +489,59 @@ class MigrationsTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun migratedFragmentedDuplicateEntriesAreLeftUnresetWhenTheirIdentityIsUnknown() =
|
||||||
|
runTest {
|
||||||
|
val context = ApplicationProvider.getApplicationContext<Context>()
|
||||||
|
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<SubtracksDatabase>(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(
|
private fun text(
|
||||||
connection: SQLiteConnection,
|
connection: SQLiteConnection,
|
||||||
sql: String,
|
sql: String,
|
||||||
|
|||||||
Reference in New Issue
Block a user