Make public playlist immutable (only Downloader can touch it)

Remove external usage of playlist_revision

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

@@ -220,6 +220,6 @@ class DownloadListModel(application: Application) : GenericListModel(application
     private val downloader by inject<Downloader>()
 
     fun getList(): LiveData<List<DownloadFile>> {
-        return downloader.observableList
+        return downloader.observableDownloads
     }
 }

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

@@ -35,6 +35,7 @@ import android.widget.TextView
 import android.widget.ViewFlipper
 import androidx.core.view.isVisible
 import androidx.fragment.app.Fragment
+import androidx.lifecycle.lifecycleScope
 import androidx.navigation.Navigation
 import com.mobeta.android.dslv.DragSortListView
 import com.mobeta.android.dslv.DragSortListView.DragSortListener
@@ -49,6 +50,7 @@ import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.TimeUnit
 import kotlin.math.abs
 import kotlin.math.max
+import kotlinx.coroutines.launch
 import org.koin.android.ext.android.inject
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.get
@@ -66,6 +68,7 @@ import org.moire.ultrasonic.service.DownloadFile
 import org.moire.ultrasonic.service.LocalMediaPlayer
 import org.moire.ultrasonic.service.MediaPlayerController
 import org.moire.ultrasonic.service.MusicServiceFactory.getMusicService
+import org.moire.ultrasonic.service.RxBus
 import org.moire.ultrasonic.subsonic.ImageLoaderProvider
 import org.moire.ultrasonic.subsonic.NetworkAndStorageChecker
 import org.moire.ultrasonic.subsonic.ShareHandler
@@ -88,8 +91,6 @@ import timber.log.Timber
  */
 @Suppress("LargeClass", "TooManyFunctions", "MagicNumber")
 class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinComponent {
-    // Settings
-    private var currentRevision: Long = 0
     private var swipeDistance = 0
     private var swipeVelocity = 0
     private var jukeboxAvailable = false
@@ -419,13 +420,21 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon
                 }
             }
         )
-        Thread {
+
+        // Observe playlist changes and update the UI
+        RxBus.playlistObservable.subscribe {
+            onPlaylistChanged()
+        }
+
+        // Query the Jukebox state off-thread
+        viewLifecycleOwner.lifecycleScope.launch {
             try {
                 jukeboxAvailable = mediaPlayerController.isJukeboxAvailable
             } catch (all: Exception) {
                 Timber.e(all)
             }
-        }.start()
+        }
+
         view.setOnTouchListener { _, event -> gestureScanner.onTouchEvent(event) }
     }
 
@@ -797,9 +806,6 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon
     private fun update(cancel: CancellationToken?) {
         if (cancel!!.isCancellationRequested) return
         val mediaPlayerController = mediaPlayerController
-        if (currentRevision != mediaPlayerController.playListUpdateRevision) {
-            onPlaylistChanged()
-        }
         if (currentPlaying != mediaPlayerController.currentPlaying) {
             onCurrentChanged()
         }
@@ -914,7 +920,6 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon
 
         emptyTextView.isVisible = list.isEmpty()
 
-        currentRevision = mediaPlayerController.playListUpdateRevision
         when (mediaPlayerController.repeatMode) {
             RepeatMode.OFF -> repeatButton.setImageDrawable(
                 Util.getDrawableFromAttribute(

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

@@ -30,14 +30,15 @@ class Downloader(
     private val externalStorageMonitor: ExternalStorageMonitor,
     private val localMediaPlayer: LocalMediaPlayer
 ) : KoinComponent {
-    val playlist: MutableList<DownloadFile> = ArrayList()
+
+    private val playlist = mutableListOf<DownloadFile>()
 
     var started: Boolean = false
 
-    private val downloadQueue: PriorityQueue<DownloadFile> = PriorityQueue<DownloadFile>()
+    private val downloadQueue = PriorityQueue<DownloadFile>()
-    private val activelyDownloading: MutableList<DownloadFile> = ArrayList()
+    private val activelyDownloading = mutableListOf<DownloadFile>()
 
-    val observableList: MutableLiveData<List<DownloadFile>> = MutableLiveData<List<DownloadFile>>()
+    val observableDownloads = MutableLiveData<List<DownloadFile>>()
 
     private val jukeboxMediaPlayer: JukeboxMediaPlayer by inject()
 
@@ -46,7 +47,7 @@ class Downloader(
     private var executorService: ScheduledExecutorService? = null
     private var wifiLock: WifiManager.WifiLock? = null
 
-    var playlistUpdateRevision: Long = 0
+    private var playlistUpdateRevision: Long = 0
         private set(value) {
             field = value
             RxBus.playlistPublisher.onNext(playlist)
@@ -65,7 +66,7 @@ class Downloader(
         stop()
         clearPlaylist()
         clearBackground()
-        observableList.value = listOf()
+        observableDownloads.value = listOf()
         Timber.i("Downloader destroyed")
     }
 
@@ -183,7 +184,7 @@ class Downloader(
     }
 
     private fun updateLiveData() {
-        observableList.postValue(downloads)
+        observableDownloads.postValue(downloads)
     }
 
     private fun startDownloadOnService(task: DownloadFile) {
@@ -268,6 +269,10 @@ class Downloader(
             return temp.distinct().sorted()
         }
 
+    // Public facing playlist (immutable)
+    @Synchronized
+    fun getPlaylist(): List<DownloadFile> = playlist
+
     @Synchronized
     fun clearPlaylist() {
         playlist.clear()

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

@@ -180,7 +180,7 @@ class MediaPlayerController(
         downloader.addToPlaylist(filteredSongs, save, autoPlay, playNext, newPlaylist)
         jukeboxMediaPlayer.updatePlaylist()
         if (shuffle) shuffle()
-        val isLastTrack = (downloader.playlist.size - 1 == downloader.currentPlayingIndex)
+        val isLastTrack = (downloader.getPlaylist().size - 1 == downloader.currentPlayingIndex)
 
         if (!playNext && !autoPlay && isLastTrack) {
             val mediaPlayerService = runningInstance
@@ -190,15 +190,15 @@ class MediaPlayerController(
         if (autoPlay) {
             play(0)
         } else {
-            if (localMediaPlayer.currentPlaying == null && downloader.playlist.size > 0) {
+            if (localMediaPlayer.currentPlaying == null && downloader.getPlaylist().isNotEmpty()) {
-                localMediaPlayer.currentPlaying = downloader.playlist[0]
+                localMediaPlayer.currentPlaying = downloader.getPlaylist()[0]
-                downloader.playlist[0].setPlaying(true)
+                downloader.getPlaylist()[0].setPlaying(true)
             }
             downloader.checkDownloads()
         }
 
         playbackStateSerializer.serialize(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex,
             playerPosition
         )
@@ -210,7 +210,7 @@ class MediaPlayerController(
         val filteredSongs = songs.filterNotNull()
         downloader.downloadBackground(filteredSongs, save)
         playbackStateSerializer.serialize(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex,
             playerPosition
         )
@@ -241,7 +241,7 @@ class MediaPlayerController(
     fun shuffle() {
         downloader.shuffle()
         playbackStateSerializer.serialize(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex,
             playerPosition
         )
@@ -270,7 +270,7 @@ class MediaPlayerController(
             downloader.clearPlaylist()
             if (serialize) {
                 playbackStateSerializer.serialize(
-                    downloader.playlist,
+                    downloader.getPlaylist(),
                     downloader.currentPlayingIndex, playerPosition
                 )
             }
@@ -281,16 +281,11 @@ class MediaPlayerController(
     @Synchronized
     fun clearIncomplete() {
         reset()
-        val iterator = downloader.playlist.iterator()
+
-        while (iterator.hasNext()) {
+        downloader.clearIncomplete()
-            val downloadFile = iterator.next()
-            if (!downloadFile.isCompleteFileAvailable) {
-                iterator.remove()
-            }
-        }
 
         playbackStateSerializer.serialize(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex,
             playerPosition
         )
@@ -307,7 +302,7 @@ class MediaPlayerController(
         downloader.removeFromPlaylist(downloadFile)
 
         playbackStateSerializer.serialize(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex,
             playerPosition
         )
@@ -359,12 +354,12 @@ class MediaPlayerController(
             when (repeatMode) {
                 RepeatMode.SINGLE, RepeatMode.OFF -> {
                     // Play next if exists
-                    if (index + 1 >= 0 && index + 1 < downloader.playlist.size) {
+                    if (index + 1 >= 0 && index + 1 < downloader.getPlaylist().size) {
                         play(index + 1)
                     }
                 }
                 RepeatMode.ALL -> {
-                    play((index + 1) % downloader.playlist.size)
+                    play((index + 1) % downloader.getPlaylist().size)
                 }
                 else -> {
                 }
@@ -492,16 +487,13 @@ class MediaPlayerController(
         }
 
     val playlistSize: Int
-        get() = downloader.playlist.size
+        get() = downloader.getPlaylist().size
 
     val currentPlayingNumberOnPlaylist: Int
         get() = downloader.currentPlayingIndex
 
     val playList: List<DownloadFile>
-        get() = downloader.playlist
+        get() = downloader.getPlaylist()
-
-    val playListUpdateRevision: Long
-        get() = downloader.playlistUpdateRevision
 
     val playListDuration: Long
         get() = downloader.downloadListDuration

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

@@ -70,7 +70,7 @@ class MediaPlayerLifecycleSupport : KoinComponent {
             // Work-around: Serialize again, as the restore() method creates a
             // serialization without current playing info.
             playbackStateSerializer.serialize(
-                downloader.playlist,
+                downloader.getPlaylist(),
                 downloader.currentPlayingIndex,
                 mediaPlayerController.playerPosition
             )
@@ -87,7 +87,7 @@ class MediaPlayerLifecycleSupport : KoinComponent {
         if (!created) return
 
         playbackStateSerializer.serializeNow(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex,
             mediaPlayerController.playerPosition
         )

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

@@ -87,7 +87,7 @@ class MediaPlayerService : Service() {
 
         localMediaPlayer.onPrepared = {
             playbackStateSerializer.serialize(
-                downloader.playlist,
+                downloader.getPlaylist(),
                 downloader.currentPlayingIndex,
                 playerPosition
             )
@@ -198,7 +198,7 @@ class MediaPlayerService : Service() {
     @Synchronized
     fun setCurrentPlaying(currentPlayingIndex: Int) {
         try {
-            localMediaPlayer.setCurrentPlaying(downloader.playlist[currentPlayingIndex])
+            localMediaPlayer.setCurrentPlaying(downloader.getPlaylist()[currentPlayingIndex])
         } catch (ignored: IndexOutOfBoundsException) {
         }
     }
@@ -215,7 +215,7 @@ class MediaPlayerService : Service() {
         if (index != -1) {
             when (Settings.repeatMode) {
                 RepeatMode.OFF -> index += 1
-                RepeatMode.ALL -> index = (index + 1) % downloader.playlist.size
+                RepeatMode.ALL -> index = (index + 1) % downloader.getPlaylist().size
                 RepeatMode.SINGLE -> {
                 }
                 else -> {
@@ -224,8 +224,8 @@ class MediaPlayerService : Service() {
         }
 
         localMediaPlayer.clearNextPlaying(false)
-        if (index < downloader.playlist.size && index != -1) {
+        if (index < downloader.getPlaylist().size && index != -1) {
-            localMediaPlayer.setNextPlaying(downloader.playlist[index])
+            localMediaPlayer.setNextPlaying(downloader.getPlaylist()[index])
         } else {
             localMediaPlayer.clearNextPlaying(true)
         }
@@ -278,7 +278,7 @@ class MediaPlayerService : Service() {
     @Synchronized
     fun play(index: Int, start: Boolean) {
         Timber.v("play requested for %d", index)
-        if (index < 0 || index >= downloader.playlist.size) {
+        if (index < 0 || index >= downloader.getPlaylist().size) {
             resetPlayback()
         } else {
             setCurrentPlaying(index)
@@ -286,7 +286,7 @@ class MediaPlayerService : Service() {
                 if (jukeboxMediaPlayer.isEnabled) {
                     jukeboxMediaPlayer.skip(index, 0)
                 } else {
-                    localMediaPlayer.play(downloader.playlist[index])
+                    localMediaPlayer.play(downloader.getPlaylist()[index])
                 }
             }
             downloader.checkDownloads()
@@ -299,7 +299,7 @@ class MediaPlayerService : Service() {
         localMediaPlayer.reset()
         localMediaPlayer.setCurrentPlaying(null)
         playbackStateSerializer.serialize(
-            downloader.playlist,
+            downloader.getPlaylist(),
             downloader.currentPlayingIndex, playerPosition
         )
     }
@@ -368,7 +368,7 @@ class MediaPlayerService : Service() {
             when {
                 playerState === PlayerState.PAUSED -> {
                     playbackStateSerializer.serialize(
-                        downloader.playlist, downloader.currentPlayingIndex, playerPosition
+                        downloader.getPlaylist(), downloader.currentPlayingIndex, playerPosition
                     )
                 }
                 playerState === PlayerState.STARTED -> {
@@ -382,8 +382,8 @@ class MediaPlayerService : Service() {
             Util.broadcastPlaybackStatusChange(context, playerState)
             Util.broadcastA2dpPlayStatusChange(
                 context, playerState, song,
-                downloader.playlist.size,
+                downloader.getPlaylist().size,
-                downloader.playlist.indexOf(currentPlaying) + 1, playerPosition
+                downloader.getPlaylist().indexOf(currentPlaying) + 1, playerPosition
             )
         } else {
             // State didn't change, only the track
@@ -434,7 +434,7 @@ class MediaPlayerService : Service() {
             if (index != -1) {
                 when (Settings.repeatMode) {
                     RepeatMode.OFF -> {
-                        if (index + 1 < 0 || index + 1 >= downloader.playlist.size) {
+                        if (index + 1 < 0 || index + 1 >= downloader.getPlaylist().size) {
                             if (Settings.shouldClearPlaylist) {
                                 clear(true)
                                 jukeboxMediaPlayer.updatePlaylist()
@@ -445,7 +445,7 @@ class MediaPlayerService : Service() {
                         }
                     }
                     RepeatMode.ALL -> {
-                        play((index + 1) % downloader.playlist.size)
+                        play((index + 1) % downloader.getPlaylist().size)
                     }
                     RepeatMode.SINGLE -> play(index)
                     else -> {
@@ -464,7 +464,7 @@ class MediaPlayerService : Service() {
         setNextPlaying()
         if (serialize) {
             playbackStateSerializer.serialize(
-                downloader.playlist,
+                downloader.getPlaylist(),
                 downloader.currentPlayingIndex, playerPosition
             )
         }