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 1 commit
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
Prev Previous commit
Next Next commit
refactor: reformat
  • Loading branch information
krocard committed Nov 10, 2023
commit 5fc530a023c21a2e5919b670239b5567a095829b
matamegger marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package com.bitmovin.player.reactnative
import com.facebook.react.bridge.*
import com.facebook.react.uimanager.UIManagerModule

abstract class BitmovinBaseModule(protected val context: ReactApplicationContext) : ReactContextBaseJavaModule(context) {
abstract class BitmovinBaseModule(
protected val context: ReactApplicationContext,
) : ReactContextBaseJavaModule(context) {
/** Run [block] in [UIManagerModule.addUIBlock], forwarding the result to the [promise]. */
protected inline fun <T> addUIBlock(promise: Promise, crossinline block: ()->T) {
protected inline fun <T> addUIBlock(promise: Promise, crossinline block: () -> T) {
val uiManager = runAndRejectOnException(promise) { uiManager() } ?: return
uiManager.addUIBlock {
runAndRejectOnException(promise) {
Expand Down Expand Up @@ -40,10 +42,9 @@ abstract class BitmovinBaseModule(protected val context: ReactApplicationContext
}

/** Run [block], forwarding the return value. If it throws, sets [Promise.reject] and return null. */
inline fun <T> runAndRejectOnException(promise: Promise, crossinline block: ()->T) : T? = try {
inline fun <T> runAndRejectOnException(promise: Promise, crossinline block: () -> T): T? = try {
krocard marked this conversation as resolved.
Show resolved Hide resolved
block()
} catch (e: Exception) {
promise.reject(e)
null
}

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import com.bitmovin.player.api.analytics.AnalyticsApi.Companion.analytics
import com.bitmovin.player.reactnative.converter.toAnalyticsCustomData
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule
import com.facebook.react.uimanager.UIManagerModule

private const val MODULE_NAME = "PlayerAnalyticsModule"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import com.bitmovin.player.reactnative.converter.toJson
import com.bitmovin.player.reactnative.converter.toPlayerConfig
import com.facebook.react.bridge.*
import com.facebook.react.module.annotations.ReactModule
import com.facebook.react.uimanager.UIManagerModule

private const val MODULE_NAME = "PlayerModule"

Expand Down Expand Up @@ -67,7 +66,7 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
nativeId: NativeId,
playerConfigJson: ReadableMap?,
analyticsConfigJson: ReadableMap?,
promise: Promise
promise: Promise,
) {
addUIBlock(promise) {
if (players.containsKey(nativeId)) {
Expand Down Expand Up @@ -113,7 +112,7 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
nativeId: NativeId,
offlineContentManagerBridgeId: String,
options: ReadableMap?,
promise: Promise
promise: Promise,
) {
addUIBlock(promise) {
val offlineSourceConfig = offlineModule().getOfflineContentManagerBridge(offlineContentManagerBridgeId)
Expand Down Expand Up @@ -583,4 +582,4 @@ class PlayerModule(context: ReactApplicationContext) : BitmovinBaseModule(contex
promise.resolve(videoQualities)
}
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sure Hide whitespace is ticked to review this file.

krocard marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ fun ReadableMap.toSubtitleTrack(): SubtitleTrack? {
/**
* Converts any subtitle format name in its mime type representation.
*/
private fun String.toSubtitleMimeType(): String = "text/${this}"
private fun String.toSubtitleMimeType(): String = "text/$this"

/**
* Converts any `SubtitleTrack` into its json representation.
Expand Down Expand Up @@ -669,7 +669,7 @@ fun ReadableMap.toAnalyticsDefaultMetadata(): DefaultMetadata = DefaultMetadata.
*/
fun ReadableMap.toAnalyticsCustomData(): CustomData = CustomData.Builder().apply {
for (n in 1..30) {
this[n] = getString("customData${n}")
this[n] = getString("customData$n")
}
getString("experimentName")?.let {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use withString

setExperimentName(it)
Expand All @@ -681,7 +681,7 @@ fun ReadableMap.toAnalyticsCustomData(): CustomData = CustomData.Builder().apply
*/
fun CustomData.toJson(): WritableMap = Arguments.createMap().also { json ->
for (n in 1..30) {
json.putStringIfNotNull("customData${n}", this[n])
json.putStringIfNotNull("customData$n", this[n])
}
json.putStringIfNotNull("experimentName", experimentName)
}
Expand Down Expand Up @@ -790,16 +790,16 @@ fun BufferType.toJson(): String = when (this) {
}

fun BufferLevel.toJson(): WritableMap = Arguments.createMap().apply {
putDouble("level", level)
putDouble("targetLevel", targetLevel)
putString("media", media.toJson())
putString("type", type.toJson())
}
putDouble("level", level)
putDouble("targetLevel", targetLevel)
putString("media", media.toJson())
putString("type", type.toJson())
}

fun RNBufferLevels.toJson(): WritableMap = Arguments.createMap().apply {
putMap("audio", audio.toJson())
putMap("video", video.toJson())
}
putMap("audio", audio.toJson())
putMap("video", video.toJson())
}

/**
* Maps a JS string into the corresponding [BufferType] value.
Expand Down
krocard marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package com.bitmovin.player.reactnative.extensions

import com.bitmovin.analytics.api.CustomData

operator fun CustomData.get(index: Int) : String? = when (index) {
operator fun CustomData.get(index: Int): String? = when (index) {
1 -> customData1
2 -> customData2
3 -> customData3
Expand Down Expand Up @@ -68,4 +68,4 @@ operator fun CustomData.Builder.set(index: Int, value: String?) = when (index) {
29 -> setCustomData29(value)
30 -> setCustomData30(value)
else -> throw IndexOutOfBoundsException()
}
}
matamegger marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,28 @@ fun ReadableMap.getDoubleOrNull(

inline fun <T> ReadableMap.withDouble(
key: String,
block: (Double) -> T
) : T? = takeIf { hasKey(key) }?.getDouble(key)?.let(block)
block: (Double) -> T,
): T? = takeIf { hasKey(key) }?.getDouble(key)?.let(block)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

factorize:

fun <T> mapAndDo(key: String, get: ReadableMap.() -> T, block: (T) -> R) = takeIf{ hasKey(key) }?.get().let(block)


inline fun <T> ReadableMap.withMap(
key: String,
block: (ReadableMap) -> T
) : T? = takeIf { hasKey(key) }?.getMap(key)?.let(block)
block: (ReadableMap) -> T,
): T? = takeIf { hasKey(key) }?.getMap(key)?.let(block)

inline fun <T> ReadableMap.withInt(
krocard marked this conversation as resolved.
Show resolved Hide resolved
key: String,
block: (Int) -> T
) : T? = takeIf { hasKey(key) }?.getInt(key)?.let(block)
block: (Int) -> T,
): T? = takeIf { hasKey(key) }?.getInt(key)?.let(block)

inline fun <T> ReadableMap.withBoolean(
key: String,
block: (Boolean) -> T
) : T? = takeIf { hasKey(key) }?.getBoolean(key)?.let(block)
block: (Boolean) -> T,
): T? = takeIf { hasKey(key) }?.getBoolean(key)?.let(block)

inline fun <T> ReadableMap.withString(
key: String,
block: (String) -> T
) : T? = getString(key)?.let(block)
block: (String) -> T,
): T? = getString(key)?.let(block)

/**
* Reads the [Boolean] value from the given [ReadableMap] if the [key] is present.
Expand Down