diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadManager.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadManager.kt index 628b80f6..b1021600 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadManager.kt +++ b/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() - private val activeDownloads = ConcurrentHashMap() - private val failedList = ConcurrentHashMap() + private val activeDownloads = ConcurrentHashMap() + private val failedList = ConcurrentHashMap() val observableDownloads = MutableLiveData>() @@ -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 = (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.get(key: DownloadKey): DownloadableTrack? { + fun PriorityBlockingQueue.get(id: String): DownloadableTrack? { for (el in this) { - if (el.key == key) return el + if (el.id == id) return el } return null } - fun PriorityBlockingQueue.contains(key: DownloadKey): Boolean = - (this.get(key) != null) + fun PriorityBlockingQueue.contains(id: String): Boolean = + (this.get(id) != null) } /** diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadableTrack.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadableTrack.kt index 29f9090a..3dc59846 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadableTrack.kt +++ b/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 diff --git a/ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadManagerTest.kt b/ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadManagerTest.kt index 967f6581..6ba5eef5 100644 --- a/ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadManagerTest.kt +++ b/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) diff --git a/ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadKeyTest.kt b/ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadSourceTest.kt similarity index 66% rename from ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadKeyTest.kt rename to ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadSourceTest.kt index 350a0723..e820a138 100644 --- a/ultrasonic/src/test/kotlin/org/moire/ultrasonic/service/DownloadKeyTest.kt +++ b/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 } }