refactor: throw on invalid RN input

PRN-92

android/src/main/java/com/bitmovin/player/reactnative/BitmovinBaseModule.kt

@@ -22,7 +22,6 @@ private const val MODULE_NAME = "BitmovinBaseModule"
  * 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,

android/src/main/java/com/bitmovin/player/reactnative/BufferModule.kt

@@ -1,6 +1,7 @@
 package com.bitmovin.player.reactnative
 
 import com.bitmovin.player.api.buffer.BufferLevel
+import com.bitmovin.player.api.buffer.BufferType
 import com.bitmovin.player.api.media.MediaType
 import com.bitmovin.player.reactnative.converter.toBufferType
 import com.bitmovin.player.reactnative.converter.toJson
@@ -17,14 +18,14 @@ class BufferModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
     /**
      * Gets the [BufferLevel] from the Player
      * @param nativeId Target player id.
-     * @param type The [type of buffer][toBufferType] to return the level for.
+     * @param type The [type of buffer][BufferType] to return the level for.
      * @param promise JS promise object.
      */
     @ReactMethod
     fun getLevel(nativeId: NativeId, type: String, promise: Promise) {
         promise.map.resolveOnUiThread {
             val player = getPlayer(nativeId)
-            val bufferType = type.toBufferTypeOrThrow()
+            val bufferType = type.toBufferType()
             RNBufferLevels(
                 audio = player.buffer.getLevel(bufferType, MediaType.Audio),
                 video = player.buffer.getLevel(bufferType, MediaType.Video),
@@ -41,11 +42,9 @@ 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)
+            getPlayer(nativeId).buffer.setTargetLevel(type.toBufferType(), value)
         }
     }
-
-    private fun String.toBufferTypeOrThrow() = toBufferType() ?: throw IllegalArgumentException(INVALID_BUFFER_TYPE)
 }
 
 /**

android/src/main/java/com/bitmovin/player/reactnative/DrmModule.kt

@@ -79,10 +79,10 @@ class DrmModule(context: ReactApplicationContext) : BitmovinBaseModule(context)
             if (drmConfigs.containsKey(nativeId)) {
                 throw InvalidParameterException("NativeId already exists $nativeId")
             }
-            val widevineConfig = config.toWidevineConfig() ?: throw InvalidParameterException("Invalid widevine config")
-            widevineConfig.prepareMessageCallback = buildPrepareMessageCallback(nativeId, config)
-            widevineConfig.prepareLicenseCallback = buildPrepareLicense(nativeId, config)
-            drmConfigs[nativeId] = widevineConfig
+            drmConfigs[nativeId] = config.toWidevineConfig().apply {
+                prepareMessageCallback = buildPrepareMessageCallback(nativeId, config)
+                prepareLicenseCallback = buildPrepareLicense(nativeId, config)
+            }
         }
     }
 

android/src/main/java/com/bitmovin/player/reactnative/RNPlayerView.kt

@@ -327,7 +327,7 @@ data class RNPlayerViewConfigWrapper(
 
 data class RNStyleConfigWrapper(
     val styleConfig: StyleConfig?,
-    val userInterfaceType: UserInterfaceType,
+    val userInterfaceType: UserInterfaceType?,
 )
 
 enum class UserInterfaceType {

android/src/main/java/com/bitmovin/player/reactnative/converter/JsonConverter.kt

@@ -52,9 +52,12 @@ import com.bitmovin.player.reactnative.RNPlayerViewConfigWrapper
 import com.bitmovin.player.reactnative.RNStyleConfigWrapper
 import com.bitmovin.player.reactnative.UserInterfaceType
 import com.bitmovin.player.reactnative.extensions.get
+import com.bitmovin.player.reactnative.extensions.getArrayOrThrow
 import com.bitmovin.player.reactnative.extensions.getBooleanOrNull
 import com.bitmovin.player.reactnative.extensions.getDoubleOrNull
+import com.bitmovin.player.reactnative.extensions.getMapOrThrow
 import com.bitmovin.player.reactnative.extensions.getName
+import com.bitmovin.player.reactnative.extensions.getStringOrThrow
 import com.bitmovin.player.reactnative.extensions.mapToReactArray
 import com.bitmovin.player.reactnative.extensions.putBoolean
 import com.bitmovin.player.reactnative.extensions.putDouble
@@ -72,6 +75,7 @@ import com.bitmovin.player.reactnative.extensions.withString
 import com.bitmovin.player.reactnative.extensions.withStringArray
 import com.bitmovin.player.reactnative.ui.RNPictureInPictureHandler.PictureInPictureConfig
 import com.facebook.react.bridge.*
+import java.security.InvalidParameterException
 import java.util.UUID
 
 /**
@@ -127,10 +131,10 @@ fun ReadableMap.toSourceOptions(): SourceOptions = SourceOptions(
 /**
  * Converts an arbitrary `json` to `TimelineReferencePoint`.
  */
-private fun String.toTimelineReferencePoint(): TimelineReferencePoint? = when (this) {
+private fun String.toTimelineReferencePoint(): TimelineReferencePoint = when (this) {
     "start" -> TimelineReferencePoint.Start
     "end" -> TimelineReferencePoint.End
-    else -> null
+    else -> throw InvalidParameterException("Unknown timeline reference point $this")
 }
 
 /**
@@ -186,75 +190,72 @@ fun ReadableMap.toTweaksConfig(): TweaksConfig = TweaksConfig().apply {
 /**
  * Converts any JS object into an `AdvertisingConfig` object.
  */
-fun ReadableMap.toAdvertisingConfig(): AdvertisingConfig? {
+fun ReadableMap.toAdvertisingConfig(): AdvertisingConfig {
     return AdvertisingConfig(
-        getArray("schedule")?.toMapList()?.mapNotNull { it?.toAdItem() } ?: return null,
+        getArrayOrThrow("schedule").toMapList().checkNoNull().map { it.toAdItem() },
     )
 }
 
 /**
  * Converts any JS object into an `AdItem` object.
  */
-fun ReadableMap.toAdItem(): AdItem? {
+fun ReadableMap.toAdItem(): AdItem {
     return AdItem(
-        sources = getArray("sources") ?.toMapList()?.mapNotNull { it?.toAdSource() }?.toTypedArray() ?: return null,
+        sources = getArrayOrThrow("sources").toMapList().checkNoNull().map { it.toAdSource() }.toTypedArray(),
         position = getString("position") ?: "pre",
     )
 }
 
 /**
  * Converts any JS object into an `AdSource` object.
  */
-fun ReadableMap.toAdSource(): AdSource? {
+fun ReadableMap.toAdSource(): AdSource {
     return AdSource(
-        type = getString("type")?.toAdSourceType() ?: return null,
-        tag = getString("tag") ?: return null,
+        type = getStringOrThrow("type").toAdSourceType(),
+        tag = getStringOrThrow("tag"),
     )
 }
 
 /**
  * Converts any JS string into an `AdSourceType` enum value.
  */
-private fun String.toAdSourceType(): AdSourceType? = when (this) {
+private fun String.toAdSourceType(): AdSourceType = when (this) {
     "ima" -> AdSourceType.Ima
     "progressive" -> AdSourceType.Progressive
     "unknown" -> AdSourceType.Unknown
-    else -> null
+    else -> throw InvalidParameterException("Unknown AdSourceType $this")
 }
 
 /**
  * Converts an arbitrary `json` to `SourceConfig`.
  */
-fun ReadableMap.toSourceConfig(): SourceConfig? {
-    val url = getString("url") ?: return null
-    val type = getString("type")?.toSourceType() ?: return null
-    return SourceConfig(url, type).apply {
-        withString("title") { title = it }
-        withString("description") { description = it }
-        withString("poster") { posterSource = it }
-        withBoolean("isPosterPersistent") { isPosterPersistent = it }
-        withArray("subtitleTracks") { subtitleTracks ->
-            for (i in 0 until subtitleTracks.size()) {
-                subtitleTracks.getMap(i).toSubtitleTrack()?.let {
-                    addSubtitleTrack(it)
-                }
-            }
+fun ReadableMap.toSourceConfig(): SourceConfig = SourceConfig(
+    url = getStringOrThrow("url"),
+    type = getStringOrThrow("type").toSourceType(),
+).apply {
+    withString("title") { title = it }
+    withString("description") { description = it }
+    withString("poster") { posterSource = it }
+    withBoolean("isPosterPersistent") { isPosterPersistent = it }
+    withArray("subtitleTracks") { subtitleTracks ->
+        subtitleTracks.toMapList().forEach {
+            addSubtitleTrack(it.toSubtitleTrack())
         }
-        withString("thumbnailTrack") { thumbnailTrack = it.toThumbnailTrack() }
-        withMap("metadata") { metadata = it.toMap() }
-        withMap("options") { options = it.toSourceOptions() }
     }
+    withString("thumbnailTrack") { thumbnailTrack = it.toThumbnailTrack() }
+    withMap("metadata") { metadata = it.toMap() }
+    withMap("options") { options = it.toSourceOptions() }
 }
 
 /**
  * Converts an arbitrary `json` to `SourceType`.
  */
-fun String.toSourceType(): SourceType? = when (this) {
+fun String.toSourceType(): SourceType = when (this) {
     "dash" -> SourceType.Dash
     "hls" -> SourceType.Hls
     "smooth" -> SourceType.Smooth
     "progressive" -> SourceType.Progressive
-    else -> null
+    else -> throw InvalidParameterException("Unknown source type $this")
 }
 
 /**
@@ -488,7 +489,7 @@ fun ReadableMap.toCastOptions(): BitmovinCastManagerOptions = BitmovinCastManage
 /**
  * Converts an arbitrary `json` to `WidevineConfig`.
  */
-fun ReadableMap.toWidevineConfig(): WidevineConfig? = getMap("widevine")?.run {
+fun ReadableMap.toWidevineConfig(): WidevineConfig = getMapOrThrow("widevine").run {
     WidevineConfig(getString("licenseUrl")).apply {
         withString("preferredSecurityLevel") { preferredSecurityLevel = it }
         withBoolean("shouldKeepDrmSessionsAlive") { shouldKeepDrmSessionsAlive = it }
@@ -515,10 +516,10 @@ fun AudioTrack.toJson(): WritableMap = Arguments.createMap().apply {
 /**
  * Converts an arbitrary `json` into a `SubtitleTrack`.
  */
-fun ReadableMap.toSubtitleTrack(): SubtitleTrack? {
+fun ReadableMap.toSubtitleTrack(): SubtitleTrack {
     return SubtitleTrack(
-        url = getString("url") ?: return null,
-        label = getString("label") ?: return null,
+        url = getStringOrThrow("url"),
+        label = getStringOrThrow("label"),
         id = getString("identifier") ?: UUID.randomUUID().toString(),
         isDefault = getBooleanOrNull("isDefault") ?: false,
         language = getString("language"),
@@ -626,12 +627,10 @@ fun AdQuartile.toJson(): String = when (this) {
 /**
  * Converts an arbitrary json object into a `BitmovinAnalyticsConfig`.
  */
-fun ReadableMap.toAnalyticsConfig(): AnalyticsConfig? = getString("licenseKey")
-    ?.let { AnalyticsConfig.Builder(it) }
-    ?.apply {
-        withBoolean("adTrackingDisabled") { setAdTrackingDisabled(it) }
-        withBoolean("randomizeUserId") { setRandomizeUserId(it) }
-    }?.build()
+fun ReadableMap.toAnalyticsConfig(): AnalyticsConfig = AnalyticsConfig.Builder(getStringOrThrow("licenseKey")).apply {
+    withBoolean("adTrackingDisabled") { setAdTrackingDisabled(it) }
+    withBoolean("randomizeUserId") { setRandomizeUserId(it) }
+}.build()
 
 /**
  * Converts an arbitrary json object into an analytics `DefaultMetadata`.
@@ -736,12 +735,11 @@ fun toPlayerViewConfig(json: ReadableMap) = PlayerViewConfig(
     ),
 )
 
-private fun ReadableMap.toUserInterfaceTypeFromPlayerConfig(): UserInterfaceType? =
-    when (getMap("styleConfig")?.getString("userInterfaceType")) {
-        "Subtitle" -> UserInterfaceType.Subtitle
-        "Bitmovin" -> UserInterfaceType.Bitmovin
-        else -> null
-    }
+private fun String.toUserInterfaceType(): UserInterfaceType = when (this) {
+    "Subtitle" -> UserInterfaceType.Subtitle
+    "Bitmovin" -> UserInterfaceType.Bitmovin
+    else -> throw InvalidParameterException("Unknown user interface $this")
+}
 
 /**
  * Converts the [this@toRNPlayerViewConfigWrapper] to a `RNPlayerViewConfig` object.
@@ -751,10 +749,10 @@ fun ReadableMap.toRNPlayerViewConfigWrapper() = RNPlayerViewConfigWrapper(
     pictureInPictureConfig = getMap("pictureInPictureConfig")?.toPictureInPictureConfig(),
 )
 
-fun ReadableMap.toRNStyleConfigWrapperFromPlayerConfig(): RNStyleConfigWrapper? {
-    return RNStyleConfigWrapper(
+fun ReadableMap.toRNStyleConfigWrapperFromPlayerConfig(): RNStyleConfigWrapper? = getMap("styleConfig")?.run {
+    RNStyleConfigWrapper(
         styleConfig = toStyleConfig(),
-        userInterfaceType = toUserInterfaceTypeFromPlayerConfig() ?: return null,
+        userInterfaceType = getString("userInterfaceType")?.toUserInterfaceType(),
     )
 }
 
@@ -796,19 +794,19 @@ fun RNBufferLevels.toJson(): WritableMap = Arguments.createMap().apply {
 /**
  * Maps a JS string into the corresponding [BufferType] value.
  */
-fun String.toBufferType(): BufferType? = when (this) {
+fun String.toBufferType(): BufferType = when (this) {
     "forwardDuration" -> BufferType.ForwardDuration
     "backwardDuration" -> BufferType.BackwardDuration
-    else -> null
+    else -> throw InvalidParameterException("Unknown buffer type $this")
 }
 
 /**
  * Maps a JS string into the corresponding [MediaType] value.
  */
-fun String.toMediaType(): MediaType? = when (this) {
+fun String.toMediaType(): MediaType = when (this) {
     "audio" -> MediaType.Audio
     "video" -> MediaType.Video
-    else -> null
+    else -> throw InvalidParameterException("Unknown media type $this")
 }
 
 /**
@@ -821,3 +819,7 @@ private fun CastPayload.toJson(): WritableMap = Arguments.createMap().apply {
 }
 
 private fun WritableMap.putStringIfNotNull(name: String, value: String?) = value?.let { putString(name, value) }
+
+private fun <T> List<T?>.checkNoNull(): List<T> = map {
+    it ?: throw InvalidParameterException("Unexpected null in array")
+}

android/src/main/java/com/bitmovin/player/reactnative/extensions/ReadableArray.kt

@@ -2,16 +2,12 @@ package com.bitmovin.player.reactnative.extensions
 
 import com.facebook.react.bridge.*
 
-inline fun <T> ReadableArray.toList(convert: (Dynamic) -> T): List<T?> = (0 until size()).map { i ->
-    convert(getDynamic(i))
+inline fun <T> ReadableArray.toList(getter: ReadableArray.(Int) -> T): List<T> = (0 until size()).map { i ->
+    getter(i)
 }
 
-fun ReadableArray.toBooleanList() = toList { it.asBoolean() }
-fun ReadableArray.toStringList() = toList { it.asString() }
-fun ReadableArray.toDoubleList() = toList { it.asDouble() }
-fun ReadableArray.toIntList() = toList { it.asInt() }
-fun ReadableArray.toListOfArrays() = toList { it.asArray() }
-fun ReadableArray.toMapList() = toList { it.asMap() }
+fun ReadableArray.toStringList() = toList(ReadableArray::getString)
+fun ReadableArray.toMapList() = toList(ReadableArray::getMap)
 
 inline fun <T> List<T>.mapToReactArray(
     transform: (T) -> WritableMap,

android/src/main/java/com/bitmovin/player/reactnative/extensions/ReadableMapExtension.kt

@@ -1,15 +1,15 @@
 package com.bitmovin.player.reactnative.extensions
 
 import com.facebook.react.bridge.*
+import java.security.InvalidParameterException
 
 fun ReadableMap.getBooleanOrNull(key: String): Boolean? = getValueOrNull(key, ReadableMap::getBoolean)
 fun ReadableMap.getIntOrNull(key: String): Int? = getValueOrNull(key, ReadableMap::getInt)
 fun ReadableMap.getDoubleOrNull(key: String): Double? = getValueOrNull(key, ReadableMap::getDouble)
 
-inline fun <T> ReadableMap.getValueOrNull(
-    key: String,
-    get: ReadableMap.(String) -> T?,
-) = takeIf { hasKey(key) }?.get(key)
+fun ReadableMap.getArrayOrThrow(key: String) = getObjectOrThrow(key, "array", ReadableMap::getArray)
+fun ReadableMap.getMapOrThrow(key: String) = getObjectOrThrow(key, "map", ReadableMap::getMap)
+fun ReadableMap.getStringOrThrow(key: String) = getObjectOrThrow(key, "string", ReadableMap::getString)
 
 inline fun <T> ReadableMap.withDouble(
     key: String,
@@ -48,10 +48,23 @@ inline fun <T> ReadableMap.withStringArray(
 
 fun ReadableMap.getStringArray(it: String): List<String?>? = getArray(it)?.toStringList()
 
+inline fun <reified T> ReadableMap.toMap(): Map<String, T> = toHashMap().mapValues { it.value as T }
+
+/** Private helper, do not use directly. */
 inline fun <T, R> ReadableMap.mapValue(
     key: String,
     get: ReadableMap.(String) -> T?,
     block: (T) -> R,
 ) = getValueOrNull(key, get)?.let(block)
 
-inline fun <reified T> ReadableMap.toMap(): Map<String, T> = toHashMap().mapValues { it.value as T }
+/** Private helper, do not use directly. */
+inline fun <T> ReadableMap.getValueOrNull(
+    key: String,
+    get: ReadableMap.(String) -> T?,
+) = takeIf { hasKey(key) }?.get(key)
+
+private inline fun <T> ReadableMap.getObjectOrThrow(
+    key: String,
+    name: String,
+    getter: ReadableMap.(String) -> T?,
+) = getter(key) ?: throw InvalidParameterException("Missing $name $key")