From b9085af44cc120ab37abcdd3da4ca038973ea3e8 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Thu, 12 Nov 2026 10:17:54 +0000 Subject: [PATCH] queue: preserve fragmented origin entry identity --- .../39.json | 18 ++- .../com/subtracks/data/db/Migrations.kt | 2 + .../kotlin/com/subtracks/data/model/Models.kt | 17 ++- .../subtracks/data/repo/QueueRepository.kt | 113 ++++++++++++------ .../data/repo/QueueRepositoryTest.kt | 53 ++++++++ 5 files changed, 160 insertions(+), 43 deletions(-) diff --git a/app/schemas/com.subtracks.data.db.SubtracksDatabase/39.json b/app/schemas/com.subtracks.data.db.SubtracksDatabase/39.json index fbc1eacf..c221856c 100644 --- a/app/schemas/com.subtracks.data.db.SubtracksDatabase/39.json +++ b/app/schemas/com.subtracks.data.db.SubtracksDatabase/39.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 39, - "identityHash": "ee91ebd8ca8608584ff44c68b6d9f224", + "identityHash": "6e9243b8186efefb9a63425559da6cdc", "entities": [ { "tableName": "sources", @@ -1515,7 +1515,7 @@ }, { "tableName": "queue_entries", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `position` INTEGER NOT NULL, `sourceId` INTEGER NOT NULL, `kind` TEXT NOT NULL, `refId` TEXT NOT NULL, `rangeStart` INTEGER, `rangeEnd` INTEGER, `originKind` TEXT, `originRefId` TEXT, `originOccurrence` INTEGER, FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `position` INTEGER NOT NULL, `sourceId` INTEGER NOT NULL, `kind` TEXT NOT NULL, `refId` TEXT NOT NULL, `rangeStart` INTEGER, `rangeEnd` INTEGER, `originKind` TEXT, `originRefId` TEXT, `originOccurrence` INTEGER, `originEntryId` INTEGER, FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", "fields": [ { "fieldPath": "id", @@ -1571,6 +1571,11 @@ "fieldPath": "originOccurrence", "columnName": "originOccurrence", "affinity": "INTEGER" + }, + { + "fieldPath": "originEntryId", + "columnName": "originEntryId", + "affinity": "INTEGER" } ], "primaryKey": { @@ -1615,7 +1620,7 @@ }, { "tableName": "up_next_entries", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `position` INTEGER NOT NULL, `sourceId` INTEGER NOT NULL, `kind` TEXT NOT NULL, `refId` TEXT NOT NULL, `rangeStart` INTEGER, `rangeEnd` INTEGER, `originKind` TEXT, `originRefId` TEXT, `originOccurrence` INTEGER, FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `position` INTEGER NOT NULL, `sourceId` INTEGER NOT NULL, `kind` TEXT NOT NULL, `refId` TEXT NOT NULL, `rangeStart` INTEGER, `rangeEnd` INTEGER, `originKind` TEXT, `originRefId` TEXT, `originOccurrence` INTEGER, `originEntryId` INTEGER, FOREIGN KEY(`sourceId`) REFERENCES `sources`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", "fields": [ { "fieldPath": "id", @@ -1671,6 +1676,11 @@ "fieldPath": "originOccurrence", "columnName": "originOccurrence", "affinity": "INTEGER" + }, + { + "fieldPath": "originEntryId", + "columnName": "originEntryId", + "affinity": "INTEGER" } ], "primaryKey": { @@ -2076,7 +2086,7 @@ ], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'ee91ebd8ca8608584ff44c68b6d9f224')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '6e9243b8186efefb9a63425559da6cdc')" ] } } \ No newline at end of file diff --git a/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt b/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt index 8487abed..9b7f19f2 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/Migrations.kt @@ -736,9 +736,11 @@ val MIGRATION_38_39 = connection.execSQL("ALTER TABLE `queue_entries` ADD COLUMN `originKind` TEXT") connection.execSQL("ALTER TABLE `queue_entries` ADD COLUMN `originRefId` TEXT") connection.execSQL("ALTER TABLE `queue_entries` ADD COLUMN `originOccurrence` INTEGER") + connection.execSQL("ALTER TABLE `queue_entries` ADD COLUMN `originEntryId` INTEGER") connection.execSQL("ALTER TABLE `up_next_entries` ADD COLUMN `originKind` TEXT") connection.execSQL("ALTER TABLE `up_next_entries` ADD COLUMN `originRefId` TEXT") connection.execSQL("ALTER TABLE `up_next_entries` ADD COLUMN `originOccurrence` INTEGER") + connection.execSQL("ALTER TABLE `up_next_entries` ADD COLUMN `originEntryId` INTEGER") connection.execSQL( "CREATE TABLE IF NOT EXISTS `pending_sync_origins` (`sourceId` INTEGER NOT NULL, `kind` TEXT NOT NULL, " + "`refId` TEXT NOT NULL, PRIMARY KEY(`sourceId`, `kind`, `refId`), " + diff --git a/app/src/main/kotlin/com/subtracks/data/model/Models.kt b/app/src/main/kotlin/com/subtracks/data/model/Models.kt index 8f92bded..62926532 100644 --- a/app/src/main/kotlin/com/subtracks/data/model/Models.kt +++ b/app/src/main/kotlin/com/subtracks/data/model/Models.kt @@ -496,6 +496,7 @@ data class QueueEntry( val originKind: QueueKind? = null, val originRefId: String? = null, val originOccurrence: Long? = null, + val originEntryId: Long? = null, ) { val offset: Long get() = rangeStart ?: 0 @@ -527,13 +528,26 @@ data class UpNextEntry( val originKind: QueueKind? = null, val originRefId: String? = null, val originOccurrence: Long? = null, + val originEntryId: Long? = null, ) { val offset: Long get() = rangeStart ?: 0 val count: Long? get() = rangeEnd?.let { (it - offset + 1).coerceAtLeast(0) } fun toQueueEntry(): QueueEntry = - QueueEntry(id, position, sourceId, kind, refId, rangeStart, rangeEnd, originKind, originRefId, originOccurrence) + QueueEntry( + id, + position, + sourceId, + kind, + refId, + rangeStart, + rangeEnd, + originKind, + originRefId, + originOccurrence, + originEntryId, + ) companion object { fun from(entry: QueueEntry) = @@ -548,6 +562,7 @@ data class UpNextEntry( entry.originKind, entry.originRefId, entry.originOccurrence, + entry.originEntryId, ) } } 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 46c07d63..7db5d99f 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -519,10 +519,9 @@ class QueueRepository( val loaded = db.useReaderConnection { transactor -> transactor.deferredTransaction { - val entries = dao.entries().map { ResolvedQueueEntry(it, lengthOf(it, libraryRevision)) } + val entries = withOriginEntryIds(dao.entries()).map { ResolvedQueueEntry(it, lengthOf(it, libraryRevision)) } val upNext = - dao.upNextEntries().map { - val entry = it.toQueueEntry() + withOriginEntryIds(dao.upNextEntries().map { it.toQueueEntry() }).map { entry -> ResolvedQueueEntry(entry, lengthOf(entry, libraryRevision)) } val row = dao.cursor() @@ -637,10 +636,16 @@ class QueueRepository( db.useWriterConnection { connection -> connection.immediateTransaction { dao.clear() - dao.insert(entries.mapIndexed { index, entry -> entry.copy(id = 0, position = index.toLong()) }) + dao.insert( + withOriginEntryIds(entries).mapIndexed { index, entry -> + entry.copy(id = 0, position = index.toLong()) + }, + ) dao.clearUpNext() dao.insertUpNext( - upNext.mapIndexed { index, entry -> UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) }, + withOriginEntryIds(upNext).mapIndexed { index, entry -> + UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) + }, ) dao.setCursor( row.copy( @@ -718,27 +723,26 @@ class QueueRepository( return QueueResetItem(origin, resolvedSongId, position, upNext, entryOccurrence, occurrence) } + private data class OriginEntryKey( + val origin: QueueOrigin, + val entryId: Long?, + ) + + private fun QueueEntry.originEntryKey() = OriginEntryKey(origin, originEntryId) + private fun resetEntryOccurrence( entries: List, targetIndex: Int, ): Int { - val origin = entries[targetIndex].entry.origin - val targetWhole = entries[targetIndex].entry.isWholeOrigin() - var occurrence = 0 - var fragmentSeen = false + val targetKey = entries[targetIndex].entry.originEntryKey() + val groups = mutableListOf() for (index in 0..targetIndex) { val entry = entries[index].entry - if (entry.origin != origin) continue - if (entry.isWholeOrigin()) { - if (index == targetIndex) return occurrence - occurrence++ - } else if (!fragmentSeen) { - fragmentSeen = true - if (!targetWhole) return occurrence - occurrence++ - } + if (entry.origin != targetKey.origin) continue + val key = entry.originEntryKey() + if (key !in groups) groups += key } - return occurrence + return groups.indexOf(targetKey) } private fun QueueEntry.isWholeOrigin(): Boolean { @@ -746,19 +750,47 @@ 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 + val nextIds = mutableMapOf() + for (entry in 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 + val origin = entry.origin + val whole = entry.isWholeOrigin() + val existingId = if (whole) null else fragmentIds[origin] + val id = existingId ?: (nextIds[origin] ?: 0L) + if (existingId == null) { + nextIds[origin] = id + 1 + if (!whole) fragmentIds[origin] = id + } + entry.copy(originEntryId = id) + } + } + private fun canonicalEntries( entries: List, changed: Set, ): List { val result = mutableListOf() - val reset = mutableSetOf() + val reset = mutableSetOf() for (entry in entries) { val origin = entry.origin - val whole = entry.isWholeOrigin() if (origin !in changed) { result += entry - } else if (whole || reset.add(origin)) { - result += QueueEntry(position = 0, sourceId = origin.sourceId, kind = origin.kind, refId = origin.refId) + } else if (reset.add(entry.originEntryKey())) { + result += + QueueEntry( + position = 0, + sourceId = origin.sourceId, + kind = origin.kind, + refId = origin.refId, + originEntryId = entry.originEntryId, + ) } } return result @@ -774,24 +806,21 @@ class QueueRepository( origin: QueueOrigin, ): List { val groups = mutableListOf() - var fragmentIndex: Int? = null + val groupIndices = mutableMapOf() var start = 0L for (resolved in entries) { if (resolved.entry.origin == origin) { - if (resolved.entry.isWholeOrigin()) { + val key = resolved.entry.originEntryKey() + val index = groupIndices[key] + if (index == null) { + groupIndices[key] = groups.size groups += OriginEntryGroup(listOf(start), listOf(resolved)) } else { - val index = fragmentIndex - if (index == null) { - fragmentIndex = groups.size - groups += OriginEntryGroup(listOf(start), listOf(resolved)) - } else { - groups[index] = - groups[index].copy( - starts = groups[index].starts + start, - entries = groups[index].entries + resolved, - ) - } + groups[index] = + groups[index].copy( + starts = groups[index].starts + start, + entries = groups[index].entries + resolved, + ) } } start += resolved.length @@ -1041,7 +1070,11 @@ class QueueRepository( db.useWriterConnection { transactor -> transactor.immediateTransaction { dao.clear() - dao.insert(entries.mapIndexed { index, entry -> entry.copy(id = 0, position = index.toLong()) }) + dao.insert( + withOriginEntryIds(entries).mapIndexed { index, entry -> + entry.copy(id = 0, position = index.toLong()) + }, + ) } } queueVersion.incrementAndGet() @@ -1052,7 +1085,7 @@ class QueueRepository( transactor.immediateTransaction { dao.clearUpNext() dao.insertUpNext( - entries.mapIndexed { index, entry -> + withOriginEntryIds(entries).mapIndexed { index, entry -> UpNextEntry.from(entry).copy(id = 0, position = index.toLong()) }, ) @@ -1348,6 +1381,7 @@ class QueueRepository( originKind = source.origin.kind, originRefId = source.origin.refId, originOccurrence = resetItem?.occurrence ?: source.originOccurrence, + originEntryId = source.originEntryId, ) } @@ -1447,5 +1481,8 @@ class QueueRepository( entry.kind == next.entry.kind && entry.sourceId == next.entry.sourceId && entry.refId == next.entry.refId && + entry.origin == next.entry.origin && + entry.originOccurrence == next.entry.originOccurrence && + entry.originEntryId == next.entry.originEntryId && entry.offset + length == next.entry.offset } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt index d44305e3..1681d42f 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt @@ -613,6 +613,59 @@ class QueueRepositoryTest { assertEquals(3L, repository.snapshot().upNextAnchor) } + @Test + fun aChangedOriginResetRestoresFullyFragmentedDuplicateEntries() = + runTest { + seedLibrary() + setPlaylist("p1", "s1", "s2") + val initial = + repository.snapshotAfter(listOf(repository.playlistEntry(1, "p1"), repository.playlistEntry(1, "p1"))) + repository.removeAt(initial, 0) + repository.removeAt(repository.snapshot(), 1) + repository = QueueRepository(db) + repository.setCursor(1) + repository.setPosition(42_000L) + val point = repository.captureResetPoint() + + repository.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p1")), point) + + val updated = repository.snapshot() + assertEquals(2, updated.entries.size) + assertTrue(updated.entries.all { it.entry.rangeStart == null && it.entry.rangeEnd == null }) + assertEquals(listOf("s1", "s2", "s1", "s2"), resolveAll(updated)) + assertEquals(3L, repository.remapPosition(updated, point.current!!)) + assertEquals(3L, repository.cursor()) + assertEquals(42_000L, repository.cursorPositionMs()) + } + + @Test + fun anUnrelatedOriginResetRemapsWithinTheSecondFullyFragmentedDuplicateEntry() = + runTest { + seedLibrary() + db.libraryDao().upsertSongs(listOf(song("s0", "al1", track = 0, album = "First Album"))) + setPlaylist("p1", "s1", "s2") + setPlaylist("p2", "s4") + val initial = + repository.snapshotAfter( + listOf( + repository.playlistEntry(1, "p2"), + repository.playlistEntry(1, "p1"), + repository.playlistEntry(1, "p1"), + ), + ) + repository.removeAt(initial, 1) + repository.removeAt(repository.snapshot(), 2) + repository.setCursor(2) + val point = repository.captureResetPoint() + + setPlaylist("p2", "s0", "s4") + repository.resetChangedOrigins(setOf(QueueOrigin(QueueKind.Playlist, 1, "p2")), point) + + val updated = repository.snapshot() + assertEquals(3L, repository.remapPosition(updated, point.current!!)) + assertEquals(3L, repository.cursor()) + } + @Test fun aResetRemapsTheSecondOccurrenceOfASongWithinAPlaylist() = runTest {