Merge branch 'fixUnpin' into 'develop'

Fix unpin

Closes #1259

See merge request ultrasonic/ultrasonic!1074

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SearchFragment.kt

@@ -140,7 +140,11 @@ class SearchFragment : MultiListFragment<Identifiable>(), KoinComponent {
     private fun downloadBackground(save: Boolean, songs: List<Track?>) {
         val onValid = Runnable {
             networkAndStorageChecker.warnIfNetworkOrStorageUnavailable()
-            DownloadService.download(songs.filterNotNull(), save)
+            DownloadService.download(
+                songs.filterNotNull(),
+                save = save,
+                updateSaveFlag = true
+            )
         }
         onValid.run()
     }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/PlaybackService.kt

@@ -325,7 +325,7 @@ class PlaybackService :
         ).map { it.toTrack() }
 
         launch {
-            DownloadService.download(nextSongs, save = false, isHighPriority = true)
+            DownloadService.download(nextSongs, isHighPriority = true)
         }
     }
 

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

@@ -274,46 +274,26 @@ class DownloadService : Service(), KoinComponent {
         @Synchronized
         fun download(
             tracks: List<Track>,
-            save: Boolean,
+            save: Boolean = false,
-            isHighPriority: Boolean = false
+            isHighPriority: Boolean = false,
+            updateSaveFlag: Boolean = false
         ) {
             CoroutineScope(Dispatchers.IO).launch {
 
-                // First handle and filter out those tracks that are already completed
+                // Remove tracks which are already downloaded and update the save flag
-                var filteredTracks: List<Track>
+                // if needed
-                if (save) {
+                var filteredTracks = if (updateSaveFlag) {
-                    tracks.filter { Storage.isPathExists(it.getCompleteFile()) }.forEach { track ->
+                    setSaveFlagForTracks(save, tracks)
-                        Storage.getFromPath(track.getCompleteFile())?.let {
-                            Storage.renameOrDeleteIfAlreadyExists(it, track.getPinnedFile())
-                            postState(track, DownloadState.PINNED)
-                        }
-                    }
-                    filteredTracks = tracks.filter { !Storage.isPathExists(it.getPinnedFile()) }
                 } else {
-                    tracks.filter { Storage.isPathExists(it.getPinnedFile()) }.forEach { track ->
+                    removeDownloadedTracksFromList(tracks)
-                        Storage.getFromPath(track.getPinnedFile())?.let {
-                            Storage.renameOrDeleteIfAlreadyExists(it, track.getCompleteFile())
-                            postState(track, DownloadState.DONE)
-                        }
-                    }
-                    filteredTracks = tracks.filter { !Storage.isPathExists(it.getCompleteFile()) }
-                }
-
-                // Update Pinned flag of items in progress
-                downloadQueue.filter { item -> tracks.any { it.id == item.id } }
-                    .forEach { it.pinned = save }
-                tracks.forEach {
-                    activeDownloads[it.id]?.downloadTrack?.pinned = save
-                }
-                tracks.forEach {
-                    failedList[it.id]?.pinned = save
                 }
 
+                // Remove tracks which are currently downloading
                 filteredTracks = filteredTracks.filter {
                     !downloadQueue.any { i -> i.id == it.id } && !activeDownloads.containsKey(it.id)
                 }
 
-                // The remainder tracks should be added to the download queue
+                // 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 tracksToDownload =
@@ -334,6 +314,69 @@ class DownloadService : Service(), KoinComponent {
             }
         }
 
+        private fun removeDownloadedTracksFromList(tracks: List<Track>): List<Track> {
+            return tracks.filter { track ->
+                val pinnedFile = Storage.getFromPath(track.getPinnedFile())
+                val completeFile = Storage.getFromPath(track.getCompleteFile())
+
+                completeFile?.let {
+                    postState(track, DownloadState.DONE)
+                    false
+                }
+                pinnedFile?.let {
+                    postState(track, DownloadState.PINNED)
+                    false
+                }
+                true
+            }
+        }
+
+        private fun setSaveFlagForTracks(
+            shouldPin: Boolean,
+            tracks: List<Track>
+        ): List<Track> {
+            // Walk through the tracks. If a track is pinned or complete and needs to be changed
+            // to the other state, rename it, but don't return it, thereby excluding it from
+            // further processing.
+            // If it is neither pinned nor saved, return it, so that it can be processed.
+            val filteredTracks: List<Track> = tracks.map { track ->
+                val pinnedFile = Storage.getFromPath(track.getPinnedFile())
+                val completeFile = Storage.getFromPath(track.getCompleteFile())
+
+                if (shouldPin) {
+                    pinnedFile?.let {
+                        null
+                    }
+                    completeFile?.let {
+                        Storage.renameOrDeleteIfAlreadyExists(it, track.getPinnedFile())
+                        postState(track, DownloadState.PINNED)
+                        null
+                    }
+                } else {
+                    completeFile?.let {
+                        null
+                    }
+                    pinnedFile?.let {
+                        Storage.renameOrDeleteIfAlreadyExists(it, track.getCompleteFile())
+                        postState(track, DownloadState.DONE)
+                        null
+                    }
+                }
+                track
+            }
+
+            // Update Pinned flag of items in progress
+            downloadQueue.filter { item -> tracks.any { it.id == item.id } }
+                .forEach { it.pinned = shouldPin }
+            tracks.forEach {
+                activeDownloads[it.id]?.downloadTrack?.pinned = shouldPin
+            }
+            tracks.forEach {
+                failedList[it.id]?.pinned = shouldPin
+            }
+            return filteredTracks
+        }
+
         fun requestStop() {
             val context = UApp.applicationContext()
             val intent = Intent(context, DownloadService::class.java)

ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/DownloadHandler.kt

@@ -53,8 +53,16 @@ class DownloadHandler(
             withContext(Dispatchers.Main) {
                 // If we are just downloading tracks we don't need to add them to the controller
                 when (action) {
-                    DownloadAction.DOWNLOAD -> DownloadService.download(tracksToDownload, false)
+                    DownloadAction.DOWNLOAD -> DownloadService.download(
-                    DownloadAction.PIN -> DownloadService.download(tracksToDownload, true)
+                        tracksToDownload,
+                        save = false,
+                        updateSaveFlag = true
+                    )
+                    DownloadAction.PIN -> DownloadService.download(
+                        tracksToDownload,
+                        save = true,
+                        updateSaveFlag = true
+                    )
                     DownloadAction.UNPIN -> DownloadService.unpin(tracksToDownload)
                     DownloadAction.DELETE -> DownloadService.delete(tracksToDownload)
                 }