Synchronized now playing state on outside promise

- Avoid timing out test to gather additional data
- Used static list of `ResolvablePlaybackHandler`

Author: davidvedvick

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/playback/engine/PlaybackEngine.kt

@@ -22,11 +22,11 @@ import com.lasthopesoftware.bluewater.client.playback.nowplaying.storage.ManageN
 import com.lasthopesoftware.bluewater.client.playback.nowplaying.storage.NowPlaying
 import com.lasthopesoftware.bluewater.client.playback.playlist.ManagePlaylistPlayback
 import com.lasthopesoftware.bluewater.shared.lazyLogger
-import com.lasthopesoftware.policies.ratelimiting.PromisingRateLimiter
 import com.lasthopesoftware.promises.ContinuingResult
 import com.lasthopesoftware.promises.extensions.ProgressingPromise
 import com.lasthopesoftware.promises.extensions.keepPromise
 import com.lasthopesoftware.promises.extensions.onEachEventually
+import com.lasthopesoftware.promises.extensions.regardless
 import com.lasthopesoftware.promises.extensions.toPromise
 import com.lasthopesoftware.promises.extensions.unitResponse
 import com.namehillsoftware.handoff.errors.RejectionDropper
@@ -69,7 +69,10 @@ class PlaybackEngine(
 
 	private val activeLibraryId = AtomicReference(markerLibraryId)
 
-	private val nowPlayingStateSync = PromisingRateLimiter<NowPlaying?>(1)
+	private val promisedNowPlayingStateSync = Any()
+
+	@Volatile
+	private var promisedNowPlayingState = Promise.empty<NowPlaying?>()
 
 	private val promisedPlayback = AtomicReference<ProgressingPromise<PositionedPlayingFile, Unit>?>(null)
 
@@ -480,13 +483,9 @@ class PlaybackEngine(
 		return playlistPlayback.resume()
 	}
 
-	private fun serializedPlayerUpdate() = nowPlayingStateSync.limit {
-		playbackBootstrapper.updateFromState(activeLibraryId.get())
-	}
+	private fun serializedPlayerUpdate() = updateStateSynchronously { playbackBootstrapper.updateFromState(activeLibraryId.get()) }
 
-	private fun promiseActiveNowPlaying() = nowPlayingStateSync.limit {
-		nowPlayingRepository.promiseNowPlaying(activeLibraryId.get())
-	}
+	private fun promiseActiveNowPlaying() = updateStateSynchronously { nowPlayingRepository.promiseNowPlaying(activeLibraryId.get()) }
 
 	private fun saveState(
 		libraryId: LibraryId,
@@ -508,7 +507,7 @@ class PlaybackEngine(
 	private inline fun saveState(
 		libraryId: LibraryId,
 		crossinline updateFunc: NowPlaying.() -> NowPlaying
-	): Promise<NowPlaying?> = nowPlayingStateSync.limit {
+	): Promise<NowPlaying?> = updateStateSynchronously {
 		nowPlayingRepository.promiseNowPlaying(libraryId).eventually {
 			it?.let { np ->
 				val updatedNowPlaying = updateFunc(np)
@@ -519,7 +518,25 @@ class PlaybackEngine(
 	}
 
 	@Suppress("UNCHECKED_CAST")
-	private fun saveState(nowPlaying: NowPlaying): Promise<NowPlaying?> = nowPlayingStateSync.limit {
+	private fun saveState(nowPlaying: NowPlaying): Promise<NowPlaying?> = updateStateSynchronously {
 		nowPlayingRepository.updateNowPlaying(nowPlaying) as Promise<NowPlaying?>
 	}
+
+	/**
+     * Updates the `promisedNowPlayingState` with the result of the provided `updateFunc`.
+     *
+     * This function ensures that the `promisedNowPlayingState` is updated synchronously.
+     * Each new update is chained to the previous promise, guaranteeing that state
+     * updates are applied in the order they are initiated.
+     *
+     * @param updateFunc A function that returns a [Promise] for a nullable [NowPlaying] state.
+     *   This function is responsible for fetching or modifying the state.
+     * @return A [Promise] that resolves with the current [NowPlaying] state after the update
+     *   function has been scheduled. To wait for the update to complete, chain to the returned promise.
+     */
+    private inline fun updateStateSynchronously(crossinline updateFunc: () -> Promise<NowPlaying?>): Promise<NowPlaying?> = synchronized(promisedNowPlayingStateSync) {
+		promisedNowPlayingState
+			.regardless { updateFunc() }
+			.also { promisedNowPlayingState = it }
+	}
 }

projectBlueWater/src/test/java/com/lasthopesoftware/bluewater/client/playback/engine/GivenAPlayingPlaybackEngine/When changing tracks many times.kt

@@ -11,12 +11,20 @@ import com.lasthopesoftware.bluewater.client.playback.engine.bootstrap.ManagedPl
 import com.lasthopesoftware.bluewater.client.playback.engine.preparation.PreparedPlaybackQueueResourceManagement
 import com.lasthopesoftware.bluewater.client.playback.file.PositionedFile
 import com.lasthopesoftware.bluewater.client.playback.file.PositionedPlayingFile
-import com.lasthopesoftware.bluewater.client.playback.file.preparation.FakeMappedPlayableFilePreparationSourceProvider
+import com.lasthopesoftware.bluewater.client.playback.file.fakes.FakePreparedPlayableFile
+import com.lasthopesoftware.bluewater.client.playback.file.fakes.ResolvablePlaybackHandler
 import com.lasthopesoftware.bluewater.client.playback.file.preparation.queues.CompletingFileQueueProvider
 import com.lasthopesoftware.bluewater.client.playback.nowplaying.storage.NowPlayingRepository
 import com.lasthopesoftware.bluewater.client.playback.volume.PlaylistVolumeManager
+import com.lasthopesoftware.bluewater.shared.promises.extensions.DeferredPromise
 import com.lasthopesoftware.bluewater.shared.promises.extensions.toExpiringFuture
+import com.lasthopesoftware.promises.ForwardedResponse.Companion.thenForward
+import com.lasthopesoftware.promises.extensions.keepPromise
+import com.lasthopesoftware.promises.PromiseDelay
+import com.lasthopesoftware.promises.toFuture
 import com.namehillsoftware.handoff.promises.Promise
+import io.mockk.every
+import io.mockk.mockk
 import org.assertj.core.api.Assertions.assertThat
 import org.joda.time.Duration
 import org.junit.jupiter.api.BeforeAll
@@ -30,19 +38,29 @@ class `When changing tracks many times` {
 	}
 
 	private val mut by lazy {
-		val fakePlaybackPreparerProvider = FakeMappedPlayableFilePreparationSourceProvider(
-			listOf(
-				ServiceFile("1"),
-				ServiceFile("2"),
-				ServiceFile("3"),
-				ServiceFile("4"),
-				ServiceFile("5")
-			)
-		)
+		val deferredPlaybackHandlers = listOf(
+			ServiceFile("1"),
+			ServiceFile("2"),
+			ServiceFile("3"),
+			ServiceFile("4"),
+			ServiceFile("5")
+		).associateWith {
+			val playbackHandler = ResolvablePlaybackHandler()
+			Pair(playbackHandler, DeferredPromise(FakePreparedPlayableFile(playbackHandler)))
+		}
+
 		val library = Library(id = libraryId)
 		val libraryProvider = FakeLibraryRepository(library)
-		val preparedPlaybackQueueResourceManagement =
-			PreparedPlaybackQueueResourceManagement(fakePlaybackPreparerProvider, FakePlaybackQueueConfiguration())
+		val preparedPlaybackQueueResourceManagement = PreparedPlaybackQueueResourceManagement(
+			mockk {
+				every { providePlayableFilePreparationSource() } returns mockk {
+					every { promisePreparedPlaybackFile(LibraryId(libraryId), any(), any()) }  answers {
+						deferredPlaybackHandlers[secondArg()]?.second.keepPromise().thenForward()
+					}
+				}
+			},
+			FakePlaybackQueueConfiguration()
+		)
 		val repository = NowPlayingRepository(
 			FakeSelectedLibraryProvider(),
 			libraryProvider,
@@ -61,7 +79,7 @@ class `When changing tracks many times` {
 				playbackBootstrapper,
 				playbackBootstrapper,
 			)
-		Pair(fakePlaybackPreparerProvider, playbackEngine)
+		Pair(deferredPlaybackHandlers, playbackEngine)
 	}
 
 	private var nextSwitchedFile: PositionedFile? = null
@@ -70,7 +88,7 @@ class `When changing tracks many times` {
 
 	@BeforeAll
 	fun act() {
-		val (fakePlaybackPreparerProvider, playbackEngine) = mut
+		val (preparedFiles, playbackEngine) = mut
 
 		val promisedFinalFile = Promise {
 			playbackEngine
@@ -82,7 +100,7 @@ class `When changing tracks many times` {
 				}
 		}
 
-		val playlist = fakePlaybackPreparerProvider.deferredResolutions.keys.toList()
+		val playlist = preparedFiles.keys.toList()
 
 		val promisedStart = playbackEngine
 			.startPlaylist(
@@ -91,7 +109,8 @@ class `When changing tracks many times` {
 				1
 			)
 
-		val playingPlaybackHandler = fakePlaybackPreparerProvider.deferredResolutions[ServiceFile("2")]?.resolve()
+		val (playingPlaybackHandler, resolvablePlaybackHandler) = preparedFiles.getValue(ServiceFile("2"))
+		resolvablePlaybackHandler.resolve()
 		promisedStart.toExpiringFuture().get()
 
 		val promisedChanges = Promise.whenAll(
@@ -101,19 +120,22 @@ class `When changing tracks many times` {
 		)
 
 		// Resolve the skipped tracks as well to ensure that they aren't the last switched track
-		fakePlaybackPreparerProvider.deferredResolutions[playlist[2]]?.resolve()
+		preparedFiles[playlist[2]]?.second?.resolve()
 
-		val finalPreparableFile = fakePlaybackPreparerProvider.deferredResolutions[playlist[4]]
-		finalPreparableFile?.resolve()
+		val finalPreparableFile = preparedFiles[playlist[4]]
+		finalPreparableFile?.second?.resolve()
 
 		nextSwitchedFile = promisedChanges.toExpiringFuture().get()?.lastOrNull()?.second
 
-		// Resolve the first skipped tracks afterward to ensure a cancellation is tested.
-		fakePlaybackPreparerProvider.deferredResolutions[playlist[0]]?.resolve()
+		// Resolve the first skipped track afterward to ensure a cancellation is tested.
+		preparedFiles[playlist[0]]?.second?.resolve()
 
-		playingPlaybackHandler?.resolve()
+		playingPlaybackHandler.resolve()
 
-		latestFile = promisedFinalFile.toExpiringFuture().get()
+		latestFile = Promise.whenAny(
+			promisedFinalFile,
+			PromiseDelay.delay(Duration.standardSeconds(10))
+		).toFuture().get()
 	}
 
 	@Test

projectBlueWater/src/test/java/com/lasthopesoftware/bluewater/shared/promises/extensions/DeferredPromise.kt

@@ -4,8 +4,9 @@ import com.namehillsoftware.handoff.cancellation.CancellationResponse
 import com.namehillsoftware.handoff.promises.Promise
 
 open class DeferredPromise<Resolution> : Promise<Resolution>, CancellationResponse {
-    private val resolution: Resolution?
-    private val error: Throwable?
+    val resolution: Resolution?
+    val error: Throwable?
+
 	var isCancelled = false
 		private set