Preserve queue deduplication for shared legacy cache paths

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadManager.kt

@@ -75,8 +75,8 @@ class DownloadManager(
     private val retryHandler = Handler(Looper.getMainLooper())
 
     private val downloadQueue = PriorityBlockingQueue<DownloadableTrack>()
-    private val activeDownloads = ConcurrentHashMap<DownloadKey, DownloadTask>()
-    private val failedList = ConcurrentHashMap<DownloadKey, DownloadableTrack>()
+    private val activeDownloads = ConcurrentHashMap<String, DownloadTask>()
+    private val failedList = ConcurrentHashMap<String, DownloadableTrack>()
 
     val observableDownloads = MutableLiveData<List<Track>>()
 
@@ -148,7 +148,7 @@ class DownloadManager(
                 imageLoaderProvider,
                 activeServerProvider
             )
-            activeDownloads[track.key] = downloadTask
+            activeDownloads[track.id] = downloadTask
 
             downloadTask.start()
             listChanged = true
@@ -188,7 +188,7 @@ class DownloadManager(
         when (downloadState) {
             DownloadState.FAILED -> {
                 downloadQueue.remove(item)
-                failedList[item.key] = item
+                failedList[item.id] = item
             }
 
             DownloadState.RETRYING -> {
@@ -200,7 +200,7 @@ class DownloadManager(
         }
 
         if (downloadState.isFinalState()) {
-            activeDownloads.remove(item.key)
+            activeDownloads.remove(item.id)
             processNextTracks()
         }
     }
@@ -234,18 +234,20 @@ class DownloadManager(
 
             // Remove tracks which are currently downloading
             filteredTracks = filteredTracks.filter {
-                !downloadQueue.any { i -> i.key == it.downloadKey() } &&
-                    !activeDownloads.containsKey(it.downloadKey())
+                !downloadQueue.any { i -> i.id == it.id } &&
+                    !activeDownloads.containsKey(it.id)
             }
 
             // The remaining tracks should be added to the download queue
             // By using the counter we ensure that the songs are added in the correct order
             var priority = 0
+            val selectedServerId = activeServerProvider.getSelectedServerId()
             val tracksToDownload =
                 filteredTracks.map {
+                    val sourceServerId = it.downloadSourceServerId(selectedServerId)
                     DownloadableTrack(
-                        it.copy(serverId = it.downloadKey().serverId),
-                        it.downloadKey().serverId,
+                        it.copy(serverId = sourceServerId),
+                        sourceServerId,
                         save,
                         0,
                         if (isHighPriority) priority++ else backgroundPriorityCounter++
@@ -342,13 +344,13 @@ class DownloadManager(
         }
 
         // Update Pinned flag of items in progress
-        downloadQueue.filter { item -> tracks.any { it.downloadKey() == item.key } }
+        downloadQueue.filter { item -> tracks.any { it.id == item.id } }
             .forEach { it.pinned = shouldPin }
         tracks.forEach {
-            activeDownloads[it.downloadKey()]?.downloadTrack?.pinned = shouldPin
+            activeDownloads[it.id]?.downloadTrack?.pinned = shouldPin
         }
         tracks.forEach {
-            failedList[it.downloadKey()]?.pinned = shouldPin
+            failedList[it.id]?.pinned = shouldPin
         }
         return filteredTracks
     }
@@ -362,8 +364,8 @@ class DownloadManager(
     suspend fun cancelForServer(serverId: Int) {
         val tasks = synchronized(this) {
             downloadQueue.removeIf { it.sourceServerId == serverId }
-            failedList.keys.removeIf { it.serverId == serverId }
-            activeDownloads.filterKeys { it.serverId == serverId }.values.toList()
+            failedList.entries.removeIf { it.value.sourceServerId == serverId }
+            activeDownloads.values.filter { it.downloadTrack.sourceServerId == serverId }
         }
         tasks.forEach { it.cancelAndJoin() }
         updateLiveData()
@@ -377,8 +379,8 @@ class DownloadManager(
 
     private suspend fun deleteAsync(track: Track) {
         withContext(Dispatchers.IO) {
-            downloadQueue.get(track.downloadKey())?.let { downloadQueue.remove(it) }
-            failedList.remove(track.downloadKey())
+            downloadQueue.get(track.id)?.let { downloadQueue.remove(it) }
+            failedList.remove(track.id)
             cancelDownload(track)
 
             Storage.delete(track.getPartialFile())
@@ -434,9 +436,9 @@ class DownloadManager(
     private suspend fun unpinAsync(track: Track) {
         withContext(Dispatchers.IO) {
             // Update Pinned flag of items in progress
-            downloadQueue.get(track.downloadKey())?.pinned = false
-            activeDownloads[track.downloadKey()]?.downloadTrack?.pinned = false
-            failedList[track.downloadKey()]?.pinned = false
+            downloadQueue.get(track.id)?.pinned = false
+            activeDownloads[track.id]?.downloadTrack?.pinned = false
+            failedList[track.id]?.pinned = false
 
             val pinnedFile = track.getPinnedFile()
             if (!Storage.isPathExists(pinnedFile)) return@withContext
@@ -476,14 +478,14 @@ class DownloadManager(
         // The activeDownloads lookup below reports DOWNLOADING or RETRYING for tracks
         // that are being downloaded right now; QUEUED is only correct for tracks that
         // are still waiting in the queue.
-        if (downloadQueue.contains(track.downloadKey())) return DownloadState.QUEUED
+        if (downloadQueue.contains(track.id)) return DownloadState.QUEUED
 
-        val downloadableTrack = activeDownloads[track.downloadKey()]?.downloadTrack
+        val downloadableTrack = activeDownloads[track.id]?.downloadTrack
         if (downloadableTrack != null) {
             if (downloadableTrack.tryCount > 0) return DownloadState.RETRYING
             return DownloadState.DOWNLOADING
         }
-        if (failedList[track.downloadKey()] != null) return DownloadState.FAILED
+        if (failedList[track.id] != null) return DownloadState.FAILED
         if (Storage.isPathExists(track.getCompleteFile())) return DownloadState.DONE
         if (Storage.isPathExists(track.getPinnedFile())) return DownloadState.PINNED
         return DownloadState.IDLE
@@ -500,7 +502,7 @@ class DownloadManager(
     }
 
     private fun cancelDownload(track: Track) {
-        activeDownloads[track.downloadKey()]?.cancel()
+        activeDownloads[track.id]?.cancel()
     }
 
     private fun postState(track: Track, state: DownloadState, progress: Int? = null) {
@@ -522,7 +524,7 @@ class DownloadManager(
      */
     fun inFlightDownloads(): List<Track> =
         (downloadQueue.map { it.track } + activeDownloads.values.map { it.downloadTrack.track })
-            .distinctBy { it.downloadKey() }
+            .distinctBy { it.id }
 
     private fun startService() {
         try {
@@ -532,18 +534,15 @@ class DownloadManager(
         }
     }
 
-    private fun Track.downloadKey(): DownloadKey =
-        downloadKey(activeServerProvider.getSelectedServerId())
-
-    fun PriorityBlockingQueue<DownloadableTrack>.get(key: DownloadKey): DownloadableTrack? {
+    fun PriorityBlockingQueue<DownloadableTrack>.get(id: String): DownloadableTrack? {
         for (el in this) {
-            if (el.key == key) return el
+            if (el.id == id) return el
         }
         return null
     }
 
-    fun PriorityBlockingQueue<DownloadableTrack>.contains(key: DownloadKey): Boolean =
-        (this.get(key) != null)
+    fun PriorityBlockingQueue<DownloadableTrack>.contains(id: String): Boolean =
+        (this.get(id) != null)
 }
 
 /**

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadableTrack.kt

@@ -20,7 +20,6 @@ class DownloadableTrack(
     var tryCount: Int,
     var priority: Int
 ) : Identifiable {
-    val key: DownloadKey get() = DownloadKey(sourceServerId, track.id)
     val pinnedFile = track.getPinnedFile()
     val partialFile = track.getPartialFile()
     val completeFile = track.getCompleteFile()
@@ -31,7 +30,5 @@ class DownloadableTrack(
     fun compareTo(other: DownloadableTrack): Int = priority.compareTo(other.priority)
 }
 
-data class DownloadKey(val serverId: Int, val trackId: String)
-
-fun Track.downloadKey(selectedServerId: Int): DownloadKey =
-    DownloadKey(serverId.takeIf { it > 0 } ?: selectedServerId, id)
+fun Track.downloadSourceServerId(selectedServerId: Int): Int =
+    serverId.takeIf { it > 0 } ?: selectedServerId

ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadManagerTest.kt

@@ -150,24 +150,20 @@ class DownloadManagerTest {
     }
 
     @Test
-    fun `same remote track id from different servers has separate queue entries`() {
-        val first = track("shared").copy(serverId = 1)
-        val second = track("shared").copy(serverId = 2)
+    fun `queued item keeps its source after selected server changes`() {
         runBlocking {
-            manager.downloadAsync(listOf(first))
-            manager.downloadAsync(listOf(second))
+            manager.downloadAsync(listOf(track("shared")))
         }
+        whenever(activeServerProvider.getSelectedServerId()).thenReturn(2)
 
-        manager.inFlightDownloads().map { it.serverId }.sorted() shouldBeEqualTo listOf(1, 2)
-        manager.getDownloadState(first) shouldBeEqualTo DownloadState.QUEUED
-        manager.getDownloadState(second) shouldBeEqualTo DownloadState.QUEUED
+        manager.inFlightDownloads().single().serverId shouldBeEqualTo 1
     }
 
     @Test
     fun `deleting one server removes only its queued downloads`() {
         runBlocking {
-            val first = track("shared").copy(serverId = 1)
-            val second = track("shared").copy(serverId = 2)
+            val first = track("first").copy(serverId = 1)
+            val second = track("second").copy(serverId = 2)
             manager.downloadAsync(listOf(first, second))
 
             manager.cancelForServer(1)

ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadSourceTest.kt

@@ -4,18 +4,18 @@ import org.amshove.kluent.shouldBeEqualTo
 import org.junit.Test
 import org.moire.ultrasonic.domain.Track
 
-class DownloadKeyTest {
+class DownloadSourceTest {
     @Test
     fun `track keeps its source server when selection changes`() {
         val track = Track(id = "same-id", serverId = 7)
 
-        track.downloadKey(selectedServerId = 9) shouldBeEqualTo DownloadKey(7, "same-id")
+        track.downloadSourceServerId(selectedServerId = 9) shouldBeEqualTo 7
     }
 
     @Test
     fun `legacy track captures selected server at enqueue`() {
         val track = Track(id = "same-id")
 
-        track.downloadKey(selectedServerId = 7) shouldBeEqualTo DownloadKey(7, "same-id")
+        track.downloadSourceServerId(selectedServerId = 7) shouldBeEqualTo 7
     }
 }