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

Refactor Kotlin code to improve error handling #317

Merged
merged 50 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
373b236
refactor: improve null safety
krocard Nov 8, 2023
a2e29de
refactor: report errors instead of ignoring invalid states
krocard Nov 9, 2023
e0f0244
refactor: move helpers in base module
krocard Nov 10, 2023
5fc530a
refactor: reformat
krocard Nov 10, 2023
09db8f3
fixup! method not called toJson
krocard Nov 10, 2023
2cb51f8
refactor: use getPlayer instead of direct map access
krocard Nov 10, 2023
46a7535
refactor: introduce module accessors
krocard Nov 13, 2023
14fc208
refactor: rename addUIBlock
krocard Nov 13, 2023
b39491e
refactor: introduce throw safe block
krocard Nov 13, 2023
f2ff634
fix: remove extra promise.resolve
krocard Nov 13, 2023
f8e3611
refactor: compile time safe getPlayer
krocard Nov 13, 2023
899d4f0
refactor: add withArray
krocard Nov 13, 2023
f60caee
chore: add documentation
krocard Nov 13, 2023
7b81d83
refactor: make map&array helpers type safe
krocard Nov 13, 2023
508caaa
refactor: cast manager module
krocard Nov 13, 2023
ce3d5b8
chore: prepare release 0.14.0
Nov 14, 2023
5cd38c8
Remove unnecessary dot from CHANGELOG.md
123mpozzi Nov 14, 2023
6883489
Merge pull request #321 from bitmovin/release/v0.14.0
123mpozzi Nov 14, 2023
9c86447
fix: don't return `Unit`
krocard Nov 14, 2023
c969a41
fix: remove unnecessary promise resolve
krocard Nov 14, 2023
598d8bf
fix: factorize with* methods
krocard Nov 14, 2023
efe8065
refactor: move helper method to extension
krocard Nov 14, 2023
d3c9572
refactor: rename PromiseRejectOnExceptionBlock to RejectPromiseOnExce…
krocard Nov 14, 2023
a6528bb
refactor: migrate SourceModule to BitmovinBaseModule
krocard Nov 14, 2023
ef81c51
Merge remote-tracking branch 'origin/main' into refactor--improve-nul…
krocard Nov 14, 2023
fe244b9
refactor: migrate offline module
krocard Nov 17, 2023
9f8abdb
refactor: migrate drm module
krocard Nov 17, 2023
3fd0906
refactor: remove duplicated promise.resolve
krocard Nov 17, 2023
edd2bfa
refactor: remove duplicated promise.resolve
krocard Nov 17, 2023
2aff015
fix: allow duration not propagated
krocard Nov 22, 2023
8d1d05c
fix: don't resolve Unit
krocard Nov 22, 2023
d733d09
refactor: introduce a type safe promise
krocard Nov 29, 2023
4e6f164
fix: code formating
krocard Nov 29, 2023
cb07f50
refactor: prefer early return on failure
krocard Dec 1, 2023
8773de7
fix: reject invalid command
krocard Dec 1, 2023
c30b6fe
fix: formating
krocard Dec 1, 2023
6231fd9
refactor: improve documentation
krocard Dec 1, 2023
f32a92f
fix: reject if analytics is disable
krocard Dec 4, 2023
4373ab6
refactor: mark noop function as inline
krocard Dec 4, 2023
08ab485
fix: made all array non nullable (empty on null)
krocard Dec 4, 2023
2c0d887
fix: serializing methods
krocard Dec 4, 2023
779b1e3
fix: don't skip player creation if config is missing
krocard Dec 4, 2023
d77c955
fix: don't send scaling mode if undefined
krocard Dec 6, 2023
4994562
fix: rename&use runOnMainLooperAndLogException
krocard Dec 6, 2023
d36c14d
fix: log invalid playerId
krocard Dec 6, 2023
ff1148c
fix: error message
krocard Dec 6, 2023
d272236
refactor: avoid creating a handler for each request
krocard Dec 6, 2023
e035a36
refactor: use single line function
krocard Dec 6, 2023
653fcd2
refactor: invalid ref in kdoc
krocard Dec 6, 2023
ae6f3da
fix: inverted condition breaking PiP
krocard Dec 7, 2023
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
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Changelog

## [Unreleased]
## [0.14.0] (2023-11-14)

### Added

- `LiveConfig.minTimeshiftBufferDepth` to control the minimum buffer depth of a stream needed to enable time shifting.
- `LiveConfig.minTimeshiftBufferDepth` to control the minimum buffer depth of a stream needed to enable time shifting
- `Player.buffer` to control buffer preferences and to query the current buffer state
- `DownloadFinishedEvent` to signal when the download of specific content has finished
- `Player.videoQuality`, `Player.availableVideoQualities`, and `VideoDownloadQualityChangedEvent` to query current video qualities and listen to related changes
Expand Down
matamegger marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
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.facebook.react.bridge.*
import com.facebook.react.uimanager.UIManagerModule

private const val MODULE_NAME = "BitmovinBaseModule"

/**
* Base for Bitmovin React modules.
*
* Provides many helper methods that are promise exception safe.
*
* In general, code should not throw while resolving a [Promise]. Instead, [Promise.reject] should be used.
* This doesn't match Kotlin's error style, which uses exception. The helper methods in this class, provide such
* convenience, they can only be called in a context that will catch any Exception and reject the [Promise].
*
*/
abstract class BitmovinBaseModule(
protected val context: ReactApplicationContext,
) : ReactContextBaseJavaModule(context) {
/**
* Runs [block] on the UI thread with [UIManagerModule.addUIBlock] and [TPromise.resolve] [this] with
* its return value. If [block] throws, [Promise.reject] [this] with the [Throwable].
*/
protected inline fun <T, R : T> TPromise<T>.resolveOnUiThread(
crossinline block: RejectPromiseOnExceptionBlock.() -> R,
) {
val uiManager = runAndRejectOnException { uiManager } ?: return
uiManager.addUIBlock {
resolveOnCurrentThread { block() }
}
}

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

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

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

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

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

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

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

/** Run [block], returning it's return value. If [block] throws, [Promise.reject] [this] and return null. */
inline fun <T, R> TPromise<T>.runAndRejectOnException(block: RejectPromiseOnExceptionBlock.() -> R): R? = try {
RejectPromiseOnExceptionBlock.block()
} catch (e: Exception) {
reject(e)
null
}

/**
* [TPromise.resolve] [this] with [block] return value.
* If [block] throws, [Promise.reject] [this] with the [Throwable].
*/
inline fun <T> TPromise<T>.resolveOnCurrentThread(
crossinline block: RejectPromiseOnExceptionBlock.() -> T,
): Unit = runAndRejectOnException { this@resolveOnCurrentThread.resolve(block()) } ?: Unit

/** Receiver of code that can safely throw when resolving a [Promise]. */
object RejectPromiseOnExceptionBlock

/** Compile time wrapper for Promises to type check the resolved type [T]. */
@JvmInline
value class TPromise<T>(val promise: Promise) {
// Promise only support built-in types. Functions that return [Unit] must resolve to `null`.
fun resolve(value: T): Unit = promise.resolve(value.takeUnless { it is Unit })
fun reject(throwable: Throwable) {
Log.e(MODULE_NAME, "Failed to execute Bitmovin method", throwable)
promise.reject(throwable)
}
}

inline val Promise.int get() = TPromise<Int>(this)
krocard marked this conversation as resolved.
Show resolved Hide resolved
inline val Promise.unit get() = TPromise<Unit>(this)
inline val Promise.string get() = TPromise<String>(this)
inline val Promise.double get() = TPromise<Double>(this)
inline val Promise.float get() = TPromise<Float>(this)
inline val Promise.bool get() = TPromise<Boolean>(this)
inline val Promise.map get() = TPromise<ReadableMap>(this)
inline val Promise.array get() = TPromise<ReadableArray>(this)
inline val <T> TPromise<T>.nullable get() = TPromise<T?>(promise)
Original file line number Diff line number Diff line change
@@ -1,70 +1,54 @@
package com.bitmovin.player.reactnative

import com.bitmovin.player.casting.BitmovinCastManager
import com.bitmovin.player.reactnative.converter.JsonConverter
import com.bitmovin.player.reactnative.converter.toCastOptions
import com.facebook.react.bridge.Promise
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.bridge.ReactContextBaseJavaModule
import com.facebook.react.bridge.ReactMethod
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.module.annotations.ReactModule
import com.facebook.react.uimanager.UIManagerModule

private const val MODULE_NAME = "BitmovinCastManagerModule"

@ReactModule(name = MODULE_NAME)
class BitmovinCastManagerModule(
private val context: ReactApplicationContext,
) : ReactContextBaseJavaModule(context) {
class BitmovinCastManagerModule(context: ReactApplicationContext) : BitmovinBaseModule(context) {
override fun getName() = MODULE_NAME

/**
* Returns whether the [BitmovinCastManager] is initialized.
*/
@ReactMethod
fun isInitialized(promise: Promise) = uiManager?.addUIBlock {
promise.resolve(BitmovinCastManager.isInitialized())
fun isInitialized(promise: Promise) = promise.unit.resolveOnUiThread {
BitmovinCastManager.isInitialized()
}

/**
* Initializes the [BitmovinCastManager] with the given options.
*/
@ReactMethod
fun initializeCastManager(options: ReadableMap?, promise: Promise) {
val castOptions = JsonConverter.toCastOptions(options)
uiManager?.addUIBlock {
BitmovinCastManager.initialize(
castOptions?.applicationId,
castOptions?.messageNamespace,
)
promise.resolve(null)
}
fun initializeCastManager(options: ReadableMap?, promise: Promise) = promise.unit.resolveOnUiThread {
val castOptions = options?.toCastOptions()
BitmovinCastManager.initialize(
castOptions?.applicationId,
castOptions?.messageNamespace,
)
}

/**
* Sends a message to the receiver.
*/
@ReactMethod
fun sendMessage(message: String, messageNamespace: String?, promise: Promise) {
uiManager?.addUIBlock {
BitmovinCastManager.getInstance().sendMessage(message, messageNamespace)
promise.resolve(null)
}
fun sendMessage(message: String, messageNamespace: String?, promise: Promise) = promise.unit.resolveOnUiThread {
BitmovinCastManager.getInstance().sendMessage(message, messageNamespace)
}

/**
* Updates the context of the [BitmovinCastManager] to the current activity.
*/
@ReactMethod
fun updateContext(promise: Promise) {
uiManager?.addUIBlock {
BitmovinCastManager.getInstance().updateContext(currentActivity)
promise.resolve(null)
}
fun updateContext(promise: Promise) = promise.unit.resolveOnUiThread {
BitmovinCastManager.getInstance().updateContext(currentActivity)
}

private val uiManager: UIManagerModule?
get() = context.getNativeModule(UIManagerModule::class.java)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,66 +2,50 @@ package com.bitmovin.player.reactnative

import com.bitmovin.player.api.buffer.BufferLevel
import com.bitmovin.player.api.media.MediaType
import com.bitmovin.player.reactnative.converter.JsonConverter
import com.bitmovin.player.reactnative.converter.toBufferType
import com.bitmovin.player.reactnative.converter.toJson
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule
import com.facebook.react.uimanager.UIManagerModule

private const val MODULE_NAME = "BufferModule"
private const val INVALID_BUFFER_TYPE = "Invalid buffer type"

@ReactModule(name = MODULE_NAME)
class BufferModule(private val context: ReactApplicationContext) : ReactContextBaseJavaModule(context) {
class BufferModule(context: ReactApplicationContext) : BitmovinBaseModule(context) {
override fun getName() = MODULE_NAME

/**
* Gets the [BufferLevel] from the Player
* @param nativeId Target player id.
* @param type The [type of buffer][JsonConverter.toBufferType] to return the level for.
* @param type The [type of buffer][toBufferType] to return the level for.
* @param promise JS promise object.
*/
@ReactMethod
fun getLevel(nativeId: NativeId, type: String, promise: Promise) {
uiManager()?.addUIBlock { _ ->
val player = playerModule()?.getPlayer(nativeId) ?: return@addUIBlock
val bufferType = JsonConverter.toBufferType(type)
if (bufferType == null) {
promise.reject("Error: ", "Invalid buffer type")
return@addUIBlock
}
val bufferLevels = RNBufferLevels(
player.buffer.getLevel(bufferType, MediaType.Audio),
player.buffer.getLevel(bufferType, MediaType.Video),
)
JsonConverter.fromRNBufferLevels(bufferLevels).let {
promise.resolve(it)
}
promise.map.resolveOnUiThread {
val player = getPlayer(nativeId)
val bufferType = type.toBufferTypeOrThrow()
RNBufferLevels(
audio = player.buffer.getLevel(bufferType, MediaType.Audio),
video = player.buffer.getLevel(bufferType, MediaType.Video),
).toJson()
}
}

/**
* Sets the target buffer level for the chosen buffer type across all media types.
* @param nativeId Target player id.
* @param type The [type of buffer][JsonConverter.toBufferType] to set the target level for.
* @param type The [type of buffer][toBufferType] to set the target level for.
* @param value The value to set.
*/
@ReactMethod
fun setTargetLevel(nativeId: NativeId, type: String, value: Double) {
uiManager()?.addUIBlock { _ ->
val player = playerModule()?.getPlayer(nativeId) ?: return@addUIBlock
val bufferType = JsonConverter.toBufferType(type) ?: return@addUIBlock
player.buffer.setTargetLevel(bufferType, value)
fun setTargetLevel(nativeId: NativeId, type: String, value: Double, promise: Promise) {
promise.unit.resolveOnUiThread {
getPlayer(nativeId).buffer.setTargetLevel(type.toBufferTypeOrThrow(), value)
}
}

/**
* Helper function that gets the instantiated `UIManagerModule` from modules registry.
*/
private fun uiManager(): UIManagerModule? = context.getNativeModule(UIManagerModule::class.java)

/**
* Helper function that gets the instantiated `PlayerModule` from modules registry.
*/
private fun playerModule(): PlayerModule? = context.getNativeModule(PlayerModule::class.java)
private fun String.toBufferTypeOrThrow() = toBufferType() ?: throw IllegalArgumentException(INVALID_BUFFER_TYPE)
}

/**
Expand Down
Loading