Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify getters by always throwing #347

Open
wants to merge 2 commits into
base: PRN-92/refactor--remove-rejectpromiseonexceptionblock
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
package com.bitmovin.player.reactnative

import android.util.Log
import com.bitmovin.player.api.Player
import com.bitmovin.player.api.source.Source
import com.bitmovin.player.reactnative.extensions.drmModule
import com.bitmovin.player.reactnative.extensions.offlineModule
import com.bitmovin.player.reactnative.extensions.playerModule
import com.bitmovin.player.reactnative.extensions.sourceModule
import com.bitmovin.player.reactnative.extensions.uiManagerModule
import com.bitmovin.player.reactnative.offline.OfflineContentManagerBridge
import com.facebook.react.bridge.*
import com.facebook.react.uimanager.UIManagerModule

Expand All @@ -32,42 +25,11 @@ abstract class BitmovinBaseModule(
* its return value. If [block] throws, [Promise.reject] [this] with the [Throwable].
*/
protected inline fun <T, R : T> TPromise<T>.resolveOnUiThread(crossinline block: () -> R) {
val uiManager = runAndRejectOnException { uiManager } ?: return
val uiManager = runAndRejectOnException { context.uiManagerModule } ?: return
uiManager.addUIBlock {
resolveOnCurrentThread { block() }
}
}

protected val playerModule: PlayerModule get() = context.playerModule
?: throw IllegalArgumentException("PlayerModule not found")

protected val uiManager: UIManagerModule get() = context.uiManagerModule
?: throw IllegalStateException("UIManager not found")

protected val sourceModule: SourceModule get() = context.sourceModule
?: throw IllegalStateException("SourceModule not found")

protected val offlineModule: OfflineModule get() = context.offlineModule
?: throw IllegalStateException("OfflineModule not found")

protected val drmModule: DrmModule get() = context.drmModule
?: throw IllegalStateException("DrmModule not found")

fun getPlayer(
nativeId: NativeId,
playerModule: PlayerModule = this.playerModule,
): Player = playerModule.getPlayerOrNull(nativeId) ?: throw IllegalArgumentException("Invalid PlayerId $nativeId")

fun getSource(
nativeId: NativeId,
sourceModule: SourceModule = this.sourceModule,
): Source = sourceModule.getSourceOrNull(nativeId) ?: throw IllegalArgumentException("Invalid SourceId $nativeId")

fun getOfflineContentManagerBridge(
nativeId: NativeId,
offlineModule: OfflineModule = this.offlineModule,
): OfflineContentManagerBridge = offlineModule.getOfflineContentManagerBridgeOrNull(nativeId)
?: throw IllegalArgumentException("Invalid offline content manager bridge id: $nativeId")
}

/** Compile time wrapper for Promises to type check the resolved type [T]. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.bitmovin.player.api.buffer.BufferLevel
import com.bitmovin.player.api.media.MediaType
import com.bitmovin.player.reactnative.converter.toBufferType
import com.bitmovin.player.reactnative.converter.toJson
import com.bitmovin.player.reactnative.extensions.playerModule
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule

Expand All @@ -23,7 +24,7 @@ class BufferModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
@ReactMethod
fun getLevel(nativeId: NativeId, type: String, promise: Promise) {
promise.map.resolveOnUiThread {
val player = getPlayer(nativeId)
val player = context.playerModule.getPlayer(nativeId)
val bufferType = type.toBufferTypeOrThrow()
RNBufferLevels(
audio = player.buffer.getLevel(bufferType, MediaType.Audio),
Expand All @@ -41,7 +42,7 @@ class BufferModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
@ReactMethod
fun setTargetLevel(nativeId: NativeId, type: String, value: Double, promise: Promise) {
promise.unit.resolveOnUiThread {
getPlayer(nativeId).buffer.setTargetLevel(type.toBufferTypeOrThrow(), value)
context.playerModule.getPlayer(nativeId).buffer.setTargetLevel(type.toBufferTypeOrThrow(), value)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ class OfflineModule(context: ReactApplicationContext) : BitmovinBaseModule(conte
/**
* Fetches the `OfflineManager` instance associated with `nativeId` from the internal offline managers.
*/
fun getOfflineContentManagerBridgeOrNull(
fun getOfflineContentManagerBridge(
nativeId: NativeId,
): OfflineContentManagerBridge? = offlineContentManagerBridges[nativeId]
): OfflineContentManagerBridge = offlineContentManagerBridges[nativeId]
?: throw IllegalArgumentException("Invalid offline content manager bridge id: $nativeId")

/**
* Callback when a new NativeEventEmitter is created from the Typescript layer.
Expand Down Expand Up @@ -65,7 +66,7 @@ class OfflineModule(context: ReactApplicationContext) : BitmovinBaseModule(conte
val sourceConfig = config.getMap("sourceConfig")?.toSourceConfig()
?: throw IllegalArgumentException("Invalid source config")

sourceConfig.drmConfig = context.drmModule?.getConfig(drmNativeId)
sourceConfig.drmConfig = context.drmModule.getConfig(drmNativeId)

offlineContentManagerBridges[nativeId] = OfflineContentManagerBridge(
nativeId,
Expand Down Expand Up @@ -235,9 +236,7 @@ class OfflineModule(context: ReactApplicationContext) : BitmovinBaseModule(conte
private inline fun <T> TPromise<T>.resolveWithBridge(
nativeId: NativeId,
crossinline block: OfflineContentManagerBridge.() -> T,
) {
resolveOnCurrentThread {
getOfflineContentManagerBridge(nativeId, this@OfflineModule).block()
}
) = resolveOnCurrentThread {
getOfflineContentManagerBridge(nativeId).block()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.bitmovin.player.reactnative
import com.bitmovin.player.api.analytics.AnalyticsApi
import com.bitmovin.player.api.analytics.AnalyticsApi.Companion.analytics
import com.bitmovin.player.reactnative.converter.toAnalyticsCustomData
import com.bitmovin.player.reactnative.extensions.playerModule
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule

Expand Down Expand Up @@ -44,7 +45,7 @@ class PlayerAnalyticsModule(context: ReactApplicationContext) : BitmovinBaseModu
playerId: NativeId,
crossinline block: AnalyticsApi.() -> T,
) = resolveOnUiThread {
val analytics = getPlayer(playerId).analytics
val analytics = context.playerModule.getPlayer(playerId).analytics
?: throw IllegalStateException("Analytics is disabled for player $playerId")
analytics.block()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import com.bitmovin.player.reactnative.converter.toAnalyticsDefaultMetadata
import com.bitmovin.player.reactnative.converter.toJson
import com.bitmovin.player.reactnative.converter.toPlayerConfig
import com.bitmovin.player.reactnative.extensions.mapToReactArray
import com.bitmovin.player.reactnative.extensions.offlineModule
import com.bitmovin.player.reactnative.extensions.sourceModule
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule
import java.security.InvalidParameterException
Expand All @@ -32,7 +34,8 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
/**
* Fetches the `Player` instance associated with [nativeId] from the internal players.
*/
fun getPlayerOrNull(nativeId: NativeId): Player? = players[nativeId]
fun getPlayer(nativeId: NativeId): Player = players[nativeId]
?: throw InvalidParameterException("Invalid player id: $nativeId")

/**
* Creates a new `Player` instance inside the internal players using the provided `config` object.
Expand Down Expand Up @@ -91,7 +94,7 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
@ReactMethod
fun loadSource(nativeId: NativeId, sourceNativeId: String, promise: Promise) {
promise.unit.resolveOnUiThreadWithPlayer(nativeId) {
load(getSource(sourceNativeId))
load(context.sourceModule.getSource(sourceNativeId))
}
}

Expand All @@ -109,7 +112,8 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
promise: Promise,
) {
promise.unit.resolveOnUiThreadWithPlayer(nativeId) {
val offlineContentManagerBridge = getOfflineContentManagerBridge(offlineContentManagerBridgeId)
val offlineContentManagerBridge =
context.offlineModule.getOfflineContentManagerBridge(offlineContentManagerBridgeId)
val offlineSourceConfig = offlineContentManagerBridge.offlineContentManager.offlineSourceConfig
load(offlineSourceConfig ?: throw IllegalStateException("Offline source has no config"))
}
Expand Down Expand Up @@ -562,5 +566,5 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
private inline fun <T> TPromise<T>.resolveOnUiThreadWithPlayer(
nativeId: NativeId,
crossinline block: Player.() -> T,
) = resolveOnUiThread { getPlayer(nativeId, this@PlayerModule).block() }
) = resolveOnUiThread { getPlayer(nativeId).block() }
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ import com.bitmovin.player.api.ui.ScalingMode
import com.bitmovin.player.api.ui.UiConfig
import com.bitmovin.player.reactnative.converter.toRNPlayerViewConfigWrapper
import com.bitmovin.player.reactnative.converter.toRNStyleConfigWrapperFromPlayerConfig
import com.bitmovin.player.reactnative.extensions.customMessageHandlerModule
import com.bitmovin.player.reactnative.extensions.fullscreenHandlerModule
import com.bitmovin.player.reactnative.extensions.getBooleanOrNull
import com.bitmovin.player.reactnative.extensions.getModule
import com.bitmovin.player.reactnative.extensions.playerModule
import com.bitmovin.player.reactnative.ui.CustomMessageHandlerModule
import com.bitmovin.player.reactnative.ui.FullscreenHandlerModule
import com.bitmovin.player.reactnative.ui.RNPictureInPictureHandler
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule
Expand Down Expand Up @@ -46,6 +45,7 @@ class RNPlayerViewManager(private val context: ReactApplicationContext) : Simple
override fun getName() = MODULE_NAME

private var customMessageHandlerBridgeId: NativeId? = null
private val handler = Handler(Looper.getMainLooper())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private val handler = Handler(Looper.getMainLooper())
private val mainHandler = Handler(Looper.getMainLooper())


/**
* The component's native view factory. RN may call this method multiple times
Expand Down Expand Up @@ -181,53 +181,51 @@ class RNPlayerViewManager(private val context: ReactApplicationContext) : Simple
}

private fun attachFullscreenBridge(view: RNPlayerView, fullscreenBridgeId: NativeId) {
Handler(Looper.getMainLooper()).post {
view.playerView?.setFullscreenHandler(
context.getModule<FullscreenHandlerModule>()?.getInstance(fullscreenBridgeId),
handler.postAndLogException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is already in the develop branch, could you rebase this branch please?

view.getPlayerViewOrThrow().setFullscreenHandler(
context.fullscreenHandlerModule.getInstance(fullscreenBridgeId),
)
}
}

private fun setFullscreen(view: RNPlayerView, isFullscreenRequested: Boolean) {
Handler(Looper.getMainLooper()).post {
val playerView = view.playerView ?: return@post
if (playerView.isFullscreen == isFullscreenRequested) return@post
if (isFullscreenRequested) {
playerView.enterFullscreen()
} else {
playerView.exitFullscreen()
handler.postAndLogException {
with(view.getPlayerViewOrThrow()) {
when {
isFullscreen == isFullscreenRequested -> Unit // No changes
isFullscreenRequested -> enterFullscreen()
!isFullscreenRequested -> exitFullscreen()
}
}
}
}

private fun setPictureInPicture(view: RNPlayerView, isPictureInPictureRequested: Boolean) {
runInMainLooperAndLogException {
val playerView = view.playerView ?: throw IllegalStateException("The player view is not yet created")
if (playerView.isPictureInPicture != isPictureInPictureRequested) return@runInMainLooperAndLogException
if (isPictureInPictureRequested) {
playerView.enterPictureInPicture()
} else {
playerView.exitPictureInPicture()
handler.postAndLogException {
with(view.getPlayerViewOrThrow()) {
when {
isPictureInPicture == isPictureInPictureRequested -> Unit // No changes
isPictureInPictureRequested -> enterPictureInPicture()
!isPictureInPictureRequested -> exitPictureInPicture()
}
}
}
}

private fun setScalingMode(view: RNPlayerView, scalingMode: String) {
Handler(Looper.getMainLooper()).post {
view.playerView?.scalingMode = ScalingMode.valueOf(scalingMode)
handler.postAndLogException {
view.getPlayerViewOrThrow().scalingMode = ScalingMode.valueOf(scalingMode)
}
}

private fun setCustomMessageHandlerBridgeId(view: RNPlayerView, customMessageHandlerBridgeId: NativeId) {
this.customMessageHandlerBridgeId = customMessageHandlerBridgeId
attachCustomMessageHandlerBridge(view)
attachCustomMessageHandlerBridge(view, customMessageHandlerBridgeId)
}

private fun attachCustomMessageHandlerBridge(view: RNPlayerView) {
view.playerView?.setCustomMessageHandler(
context.getModule<CustomMessageHandlerModule>()
?.getInstance(customMessageHandlerBridgeId)
?.customMessageHandler,
private fun attachCustomMessageHandlerBridge(view: RNPlayerView, customMessageHandlerBridgeId: NativeId) {
view.getPlayerViewOrThrow().setCustomMessageHandler(
context.customMessageHandlerModule.getInstance(customMessageHandlerBridgeId).customMessageHandler,
)
}

Expand All @@ -237,9 +235,8 @@ class RNPlayerViewManager(private val context: ReactApplicationContext) : Simple
* @param playerId `Player` instance id inside `PlayerModule`'s registry.
*/
private fun attachPlayer(view: RNPlayerView, playerId: NativeId, playerConfig: ReadableMap?) {
runInMainLooperAndLogException {
val player = playerId.let { context.playerModule?.getPlayerOrNull(it) }
?: throw InvalidParameterException("Cannot create a PlayerView, invalid playerId was passed.")
handler.postAndLogException {
val player = context.playerModule.getPlayer(playerId)
val playbackConfig = playerConfig?.getMap("playbackConfig")
val isPictureInPictureEnabled = view.config?.pictureInPictureConfig?.isEnabled == true ||
playbackConfig?.getBooleanOrNull("isPictureInPictureEnabled") == true
Expand Down Expand Up @@ -270,7 +267,7 @@ class RNPlayerViewManager(private val context: ReactApplicationContext) : Simple
LayoutParams.MATCH_PARENT,
)
view.setPlayerView(playerView)
attachCustomMessageHandlerBridge(view)
customMessageHandlerBridgeId?.let { attachCustomMessageHandlerBridge(view, it) }
}

if (rnStyleConfigWrapper?.styleConfig?.isUiEnabled != false &&
Expand All @@ -285,13 +282,15 @@ class RNPlayerViewManager(private val context: ReactApplicationContext) : Simple
}
}

private inline fun runInMainLooperAndLogException(crossinline block: () -> Unit) {
Handler(Looper.getMainLooper()).post {
try {
block()
} catch (e: Exception) {
Log.e(MODULE_NAME, "Error while command", e)
}
/** Post and log any exceptions instead of crashing the app. */
private inline fun Handler.postAndLogException(crossinline block: () -> Unit) = post {
try {
block()
} catch (e: Exception) {
Log.e(MODULE_NAME, "Error while executing command", e)
}
}

private fun RNPlayerView.getPlayerViewOrThrow() = playerView
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I quite like the require naming convention that androidx libraries use: https://developer.android.com/reference/kotlin/androidx/fragment/app/Fragment#requireActivity()

Suggested change
private fun RNPlayerView.getPlayerViewOrThrow() = playerView
private fun RNPlayerView.requirePlayerView() = playerView

?: throw IllegalStateException("The player view is not yet created")
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.bitmovin.player.api.source.Source
import com.bitmovin.player.reactnative.converter.toAnalyticsSourceMetadata
import com.bitmovin.player.reactnative.converter.toJson
import com.bitmovin.player.reactnative.converter.toSourceConfig
import com.bitmovin.player.reactnative.extensions.drmModule
import com.bitmovin.player.reactnative.extensions.toMap
import com.bitmovin.player.reactnative.extensions.toReadableMap
import com.facebook.react.bridge.Promise
Expand All @@ -29,9 +30,10 @@ class SourceModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
override fun getName() = MODULE_NAME

/**
* Fetches the [Source] instance associated with [nativeId] from internal sources or null.
* Fetches the [Source] instance associated with [nativeId] from internal sources or throw.
*/
fun getSourceOrNull(nativeId: NativeId): Source? = sources[nativeId]
fun getSource(nativeId: NativeId): Source = sources[nativeId]
?: throw IllegalArgumentException("Invalid SourceId $nativeId")

/**
* Creates a new `Source` instance inside the internal sources using the provided
Expand Down Expand Up @@ -76,7 +78,7 @@ class SourceModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
analyticsSourceMetadata: ReadableMap?,
promise: Promise,
) = promise.unit.resolveOnUiThread {
val drmConfig = drmNativeId?.let { drmModule.getConfig(it) }
val drmConfig = context.drmModule.getConfig(drmNativeId)
val sourceConfig = config?.toSourceConfig() ?: throw InvalidParameterException("Invalid SourceConfig")
val sourceMetadata = analyticsSourceMetadata?.toAnalyticsSourceMetadata()
if (sources.containsKey(nativeId)) {
Expand Down Expand Up @@ -180,5 +182,5 @@ class SourceModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
private inline fun <T> TPromise<T>.resolveOnUiThreadWithSource(
nativeId: NativeId,
crossinline block: Source.() -> T,
) = resolveOnUiThread { getSource(nativeId, this@SourceModule).block() }
) = resolveOnUiThread { getSource(nativeId).block() }
}
Loading