queue: preserve fragmented origin entry identity

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')"
     ]
   }
 }

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`), " +

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,
             )
     }
 }

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 {
+                        withOriginEntryIds(dao.upNextEntries().map { it.toQueueEntry() }).map { entry ->
-                            val entry = it.toQueueEntry()
                             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<ResolvedQueueEntry>,
         targetIndex: Int,
     ): Int {
-        val origin = entries[targetIndex].entry.origin
+        val targetKey = entries[targetIndex].entry.originEntryKey()
-        val targetWhole = entries[targetIndex].entry.isWholeOrigin()
+        val groups = mutableListOf<OriginEntryKey>()
-        var occurrence = 0
-        var fragmentSeen = false
         for (index in 0..targetIndex) {
             val entry = entries[index].entry
-            if (entry.origin != origin) continue
+            if (entry.origin != targetKey.origin) continue
-            if (entry.isWholeOrigin()) {
+            val key = entry.originEntryKey()
-                if (index == targetIndex) return occurrence
+            if (key !in groups) groups += key
-                occurrence++
-            } else if (!fragmentSeen) {
-                fragmentSeen = true
-                if (!targetWhole) return occurrence
-                occurrence++
         }
-        }
+        return groups.indexOf(targetKey)
-        return occurrence
     }
 
     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<QueueEntry>): List<QueueEntry> {
+        if (entries.all { it.originEntryId != null }) return entries
+        val nextIds = mutableMapOf<QueueOrigin, Long>()
+        for (entry in entries) {
+            val id = entry.originEntryId ?: continue
+            nextIds[entry.origin] = maxOf(nextIds[entry.origin] ?: 0L, id + 1)
+        }
+        val fragmentIds = mutableMapOf<QueueOrigin, Long>()
+        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<QueueEntry>,
         changed: Set<QueueOrigin>,
     ): List<QueueEntry> {
         val result = mutableListOf<QueueEntry>()
-        val reset = mutableSetOf<QueueOrigin>()
+        val reset = mutableSetOf<OriginEntryKey>()
         for (entry in entries) {
             val origin = entry.origin
-            val whole = entry.isWholeOrigin()
             if (origin !in changed) {
                 result += entry
-            } else if (whole || reset.add(origin)) {
+            } else if (reset.add(entry.originEntryKey())) {
-                result += QueueEntry(position = 0, sourceId = origin.sourceId, kind = origin.kind, refId = origin.refId)
+                result +=
+                    QueueEntry(
+                        position = 0,
+                        sourceId = origin.sourceId,
+                        kind = origin.kind,
+                        refId = origin.refId,
+                        originEntryId = entry.originEntryId,
+                    )
             }
         }
         return result
@@ -774,16 +806,14 @@ class QueueRepository(
         origin: QueueOrigin,
     ): List<OriginEntryGroup> {
         val groups = mutableListOf<OriginEntryGroup>()
-        var fragmentIndex: Int? = null
+        val groupIndices = mutableMapOf<OriginEntryKey, Int>()
         var start = 0L
         for (resolved in entries) {
             if (resolved.entry.origin == origin) {
-                if (resolved.entry.isWholeOrigin()) {
+                val key = resolved.entry.originEntryKey()
-                    groups += OriginEntryGroup(listOf(start), listOf(resolved))
+                val index = groupIndices[key]
-                } else {
-                    val index = fragmentIndex
                 if (index == null) {
-                        fragmentIndex = groups.size
+                    groupIndices[key] = groups.size
                     groups += OriginEntryGroup(listOf(start), listOf(resolved))
                 } else {
                     groups[index] =
@@ -793,7 +823,6 @@ class QueueRepository(
                         )
                 }
             }
-            }
             start += resolved.length
         }
         return groups
@@ -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
 }

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 {