Removes setMediaListAndPlay, fixes #496 (#834)

google · Dec 14, 2022 · 730d4ac · 730d4ac
1 parent 1906b9c
commit 730d4ac
Show file tree

Hide file tree

Showing 11 changed files with 4 additions and 88 deletions.
diff --git a/media-data/api/current.api b/media-data/api/current.api
@@ -330,7 +330,6 @@ package com.google.android.horologist.media.data.repository {
     method public void setMedia(com.google.android.horologist.media.model.Media media);
     method public void setMediaList(java.util.List<com.google.android.horologist.media.model.Media> mediaList);
     method public void setMediaList(java.util.List<com.google.android.horologist.media.model.Media> mediaList, int index, kotlin.time.Duration? position);
-    method public void setMediaListAndPlay(java.util.List<com.google.android.horologist.media.model.Media> mediaList, int index);
     method public void setPlaybackSpeed(float speed);
     method public void setShuffleModeEnabled(boolean shuffleModeEnabled);
     method public void skipToNextMedia();

diff --git a/...src/main/java/com/google/android/horologist/media/data/repository/PlayerRepositoryImpl.kt b/...src/main/java/com/google/android/horologist/media/data/repository/PlayerRepositoryImpl.kt
@@ -20,7 +20,6 @@ import android.util.Log
 import androidx.media3.common.C
 import androidx.media3.common.MediaItem
 import androidx.media3.common.Player
-import androidx.media3.common.Timeline
 import com.google.android.horologist.media.data.ExperimentalHorologistMediaDataApi
 import com.google.android.horologist.media.data.mapper.MediaExtrasMapperNoopImpl
 import com.google.android.horologist.media.data.mapper.MediaItemExtrasMapperNoopImpl
@@ -95,9 +94,6 @@ public class PlayerRepositoryImpl(
     private var _seekForwardIncrement = MutableStateFlow<Duration?>(null)
     override val seekForwardIncrement: StateFlow<Duration?> get() = _seekForwardIncrement
 
-    // https://github.com/google/horologist/issues/496
-    private var mediaIndexToSeekTo: Int? = null
-
     private val listener = object : Player.Listener {
         private val eventHandlers = mapOf(
             Player.EVENT_AVAILABLE_COMMANDS_CHANGED to ::updateAvailableCommands,
@@ -107,8 +103,6 @@ public class PlayerRepositoryImpl(
             Player.EVENT_SEEK_BACK_INCREMENT_CHANGED to ::updateSeekBackIncrement,
             Player.EVENT_SEEK_FORWARD_INCREMENT_CHANGED to ::updateSeekForwardIncrement,
             Player.EVENT_MEDIA_METADATA_CHANGED to ::updateCurrentMedia,
-            // Moved below until https://github.com/google/horologist/issues/496 solved
-//            Player.EVENT_TIMELINE_CHANGED to ::updateTimeline,
 
             // Reason for handling these events here, instead of using individual callbacks
             // (onIsLoadingChanged, onIsPlayingChanged, onPlaybackStateChanged, etc):
@@ -131,10 +125,6 @@ public class PlayerRepositoryImpl(
                 }
             }
         }
-
-        override fun onTimelineChanged(timeline: Timeline, reason: Int) {
-            updateTimeline(player.value!!)
-        }
     }
 
     private fun updatePlaybackSpeed(player: Player) {
@@ -174,16 +164,6 @@ public class PlayerRepositoryImpl(
         _seekForwardIncrement.value = player.seekForwardIncrement.toDuration(DurationUnit.MILLISECONDS)
     }
 
-    // TODO https://github.com/google/horologist/issues/496
-    private fun updateTimeline(player: Player) {
-        mediaIndexToSeekTo?.let { index ->
-            player.seekTo(index, 0)
-            player.prepare()
-            player.play()
-            mediaIndexToSeekTo = null
-        }
-    }
-
     /**
      * Connect this repository to the player including listening to events.
      */
@@ -346,16 +326,6 @@ public class PlayerRepositoryImpl(
         }
     }
 
-    override fun setMediaListAndPlay(mediaList: List<Media>, index: Int) {
-        checkNotClosed()
-
-        player.value?.let {
-            it.setMediaItems(mediaList.map(mediaItemMapper::map))
-
-            mediaIndexToSeekTo = index
-        }
-    }
-
     override fun addMedia(media: Media) {
         checkNotClosed()
 

diff --git a/...java/com/google/android/horologist/media/data/repository/PlayerRepositoryImplCloseTest.kt b/...java/com/google/android/horologist/media/data/repository/PlayerRepositoryImplCloseTest.kt
@@ -109,9 +109,6 @@ class PlayerRepositoryImplCloseTest(
             param("setMediaList") { sut: PlayerRepositoryImpl, _: Context ->
                 sut.setMediaList(listOf(getDummyMedia()))
             },
-            param("setMediaListAndPlay") { sut: PlayerRepositoryImpl, _: Context ->
-                sut.setMediaListAndPlay(listOf(getDummyMedia(), getDummyMedia()), 1)
-            },
             param("addMedia") { sut: PlayerRepositoryImpl, _: Context ->
                 sut.addMedia(getDummyMedia())
             },

diff --git a/...m/google/android/horologist/media/data/repository/PlayerRepositoryImplNotConnectedTest.kt b/...m/google/android/horologist/media/data/repository/PlayerRepositoryImplNotConnectedTest.kt
@@ -110,9 +110,6 @@ class PlayerRepositoryImplNotConnectedTest(
             param("setMedia") { sut: PlayerRepositoryImpl ->
                 sut.setMedia(getDummyMedia())
             },
-            param("setMediaListAndPlay") { sut: PlayerRepositoryImpl ->
-                sut.setMediaListAndPlay(listOf(getDummyMedia(), getDummyMedia()), 1)
-            },
             param("setMediaList") { sut: PlayerRepositoryImpl ->
                 sut.setMediaList(listOf(getDummyMedia()))
             },

diff --git a/...test/java/com/google/android/horologist/media/data/repository/PlayerRepositoryImplTest.kt b/...test/java/com/google/android/horologist/media/data/repository/PlayerRepositoryImplTest.kt
@@ -37,7 +37,6 @@ import com.google.common.truth.Truth.assertThat
 import org.junit.Assert.assertThrows
 import org.junit.Assert.assertTrue
 import org.junit.Before
-import org.junit.Ignore
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
@@ -582,36 +581,6 @@ class PlayerRepositoryImplTest {
         assertThat(sut.player.value?.currentPosition).isEqualTo(6000)
     }
 
-    @Ignore("Fix test once this issue is fixed: https://github.com/androidx/media/issues/85")
-    @Test
-    fun `given NO previous MediaList is set when setMediaListAndPlay then state is correct`() {
-        // given
-        val player = TestExoPlayerBuilder(context).build()
-        sut.connect(player) {}
-
-        val media1 = getStubMedia("id1")
-        val media2 = getStubMedia("id2")
-        val mediaList = listOf(media1, media2)
-
-        // when
-        sut.setMediaListAndPlay(mediaList, 1)
-        runUntilPendingCommandsAreFullyHandled(player)
-
-        // then
-        assertThat(sut.getMediaCount()).isEqualTo(2)
-        assertThat(sut.getMediaAt(0)).isEqualTo(media1)
-        assertThat(sut.getMediaAt(1)).isEqualTo(media2)
-        assertThat(sut.currentState.value).isEqualTo(PlayerState.Playing)
-        assertThat(sut.currentMedia.value).isEqualTo(media2)
-        assertThat(sut.playbackSpeed.value).isEqualTo(1f)
-        assertThat(sut.shuffleModeEnabled.value).isFalse()
-        assertThat(sut.player.value).isSameInstanceAs(player)
-        assertThat(sut.mediaPosition.value).isEqualTo(MediaPosition.Unknown)
-        assertThat(sut.availableCommands.value).containsExactlyElementsIn(
-            listOf(Command.PlayPause, Command.SkipToPreviousMedia, Command.SetShuffle)
-        )
-    }
-
     @Test
     fun `given NO previous Media is set when addMedia then state is correct`() {
         // given

diff --git a/...ain/java/com/google/android/horologist/mediasample/ui/entity/UampEntityScreenViewModel.kt b/...ain/java/com/google/android/horologist/mediasample/ui/entity/UampEntityScreenViewModel.kt
@@ -88,7 +88,9 @@ class UampEntityScreenViewModel @Inject constructor(
                 settingsRepository.edit { it.copy { currentMediaListId = playlistId } }
             }
             playerRepository.setShuffleModeEnabled(shuffled)
-            playerRepository.setMediaListAndPlay(playlistDownload.playlist.mediaList, index)
+            playerRepository.setMediaList(playlistDownload.playlist.mediaList, index)
+            playerRepository.prepare()
+            playerRepository.play()
         }
     }
 

diff --git a/...m/google/android/horologist/mediasample/ui/entity/UampStreamingPlaylistScreenViewModel.kt b/...m/google/android/horologist/mediasample/ui/entity/UampStreamingPlaylistScreenViewModel.kt
@@ -88,7 +88,7 @@ class UampStreamingPlaylistScreenViewModel @Inject constructor(
                 settingsRepository.edit { it.copy { currentMediaListId = playlistId } }
             }
             playerRepository.setShuffleModeEnabled(shuffled)
-            playerRepository.setMediaListAndPlay(playlistDownload.playlist.mediaList, index)
+            playerRepository.setMediaList(playlistDownload.playlist.mediaList, index)
         }
     }
 }
diff --git a/...dTest/java/com/google/android/horologist/test/toolbox/testdoubles/FakePlayerRepository.kt b/...dTest/java/com/google/android/horologist/test/toolbox/testdoubles/FakePlayerRepository.kt
@@ -121,10 +121,6 @@ class FakePlayerRepository() : PlayerRepository {
         _mediaPosition.value = position?.let { MediaPosition.create(it, 10.seconds) }
     }
 
-    override fun setMediaListAndPlay(mediaList: List<Media>, index: Int) {
-        // do nothing
-    }
-
     override fun addMedia(media: Media) {
         // do nothing
     }

diff --git a/.../test/java/com/google/android/horologist/test/toolbox/testdoubles/MockPlayerRepository.kt b/.../test/java/com/google/android/horologist/test/toolbox/testdoubles/MockPlayerRepository.kt
@@ -114,10 +114,6 @@ class MockPlayerRepository(
         // do nothing
     }
 
-    override fun setMediaListAndPlay(mediaList: List<Media>, index: Int) {
-        // do nothing
-    }
-
     override fun addMedia(media: Media) {
         // do nothing
     }

diff --git a/media/api/current.api b/media/api/current.api
@@ -202,7 +202,6 @@ package com.google.android.horologist.media.repository {
     method public void setMedia(com.google.android.horologist.media.model.Media media);
     method public void setMediaList(java.util.List<com.google.android.horologist.media.model.Media> mediaList);
     method public void setMediaList(java.util.List<com.google.android.horologist.media.model.Media> mediaList, int index, optional kotlin.time.Duration? position);
-    method public void setMediaListAndPlay(java.util.List<com.google.android.horologist.media.model.Media> mediaList, int index);
     method public void setPlaybackSpeed(float speed);
     method public void setShuffleModeEnabled(boolean shuffleModeEnabled);
     method public void skipToNextMedia();

diff --git a/media/src/main/java/com/google/android/horologist/media/repository/PlayerRepository.kt b/media/src/main/java/com/google/android/horologist/media/repository/PlayerRepository.kt
@@ -147,15 +147,6 @@ public interface PlayerRepository {
 
     public fun setMediaList(mediaList: List<Media>, index: Int, position: Duration? = null)
 
-    /**
-     * Clears the playlist, adds the specified [Media] list and plays the [Media] at the position of
-     * the index passed as param.
-     *
-     * @param mediaList The new [Media] list.
-     * @param index The new position.
-     */
-    public fun setMediaListAndPlay(mediaList: List<Media>, index: Int)
-
     /**
      * Adds a [Media] to the end of the playlist.
      */