From 28387360bacf02a64523a70344d82664cfebc5bb Mon Sep 17 00:00:00 2001 From: Arthur Zettler Date: Thu, 29 Nov 2018 14:15:38 -0200 Subject: [PATCH 1/5] refactor(loader): make plugins and playback lists private in Loader --- clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt b/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt index e4b032b3..ba1c2e1b 100644 --- a/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt +++ b/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt @@ -11,7 +11,7 @@ import kotlin.reflect.full.primaryConstructor class Loader(extraPlugins: List> = emptyList(), extraPlaybacks: List> = emptyList()) { companion object { - @JvmStatic val registeredPlugins = mutableMapOf>() + @JvmStatic private val registeredPlugins = mutableMapOf>() @JvmStatic private val registeredPlaybacks= mutableListOf>() @JvmStatic @@ -63,9 +63,9 @@ class Loader(extraPlugins: List> = emptyList(), extraPlayback private val externalPlaybacks = mutableListOf>() - val availablePlugins = mutableMapOf>() + private val availablePlugins = mutableMapOf>() - val availablePlaybacks = mutableListOf>() + private val availablePlaybacks = mutableListOf>() init { for (pluginClass in registeredPlugins.values) { From c04676c1ef7d670599d72f5c12e2a6df0b06fe22 Mon Sep 17 00:00:00 2001 From: Arthur Zettler Date: Thu, 29 Nov 2018 14:16:41 -0200 Subject: [PATCH 2/5] feat(loader): create unregister plugins method in Loader --- .../kotlin/io/clappr/player/plugin/Loader.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt b/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt index ba1c2e1b..ab73a1f6 100644 --- a/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt +++ b/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt @@ -29,13 +29,24 @@ class Loader(extraPlugins: List> = emptyList(), extraPlayback val pluginName = (pluginClass.companionObjectInstance as? NamedType)?.name pluginName?.let { if (pluginName.isNotEmpty()) { - registeredPlugins.put(pluginName, pluginClass) + registeredPlugins[pluginName] = pluginClass return true } } return false } + @JvmStatic + fun unregisterPlugin(pluginClass: KClass): Boolean { + val pluginName = (pluginClass.companionObjectInstance as? NamedType)?.name + pluginName?.let { + if (it.isNotEmpty() && registeredPlugins.containsKey(it)) { + return registeredPlugins.remove(it) != null + } + } + return false + } + @JvmStatic fun registerPlayback(playbackClass: KClass): Boolean { val playbackName = (playbackClass.companionObjectInstance as? NamedType)?.name @@ -98,12 +109,13 @@ class Loader(extraPlugins: List> = emptyList(), extraPlayback fun loadPlayback(source: String, mimeType: String? = null, options: Options): Playback? { var playback: Playback? = null + try { val playbackClass = registeredPlaybacks.first { supportsSource(it, source, mimeType) } val constructor = playbackClass.primaryConstructor playback = constructor?.call(source, mimeType, options) as? Playback - } catch (e: Exception) { - } + } catch (e: Exception) { } + return playback } @@ -120,19 +132,18 @@ class Loader(extraPlugins: List> = emptyList(), extraPlayback val name : String? = (pluginClass.companionObjectInstance as? NamedType)?.name name?.let { if (name.isNotEmpty()) { - availablePlugins.put(name, pluginClass) + availablePlugins[name] = pluginClass } } } private fun loadPlugin(component: BaseObject, pluginClass: KClass) : Plugin? { var plugin: Plugin? = null - val constructor = pluginClass.primaryConstructor + try { plugin = constructor?.call(component) as? Plugin - } catch (e: Exception) { - } + } catch (e: Exception) { } return plugin } From 2f33e2e3ad0dc9565391bb3612ba72bdb103a276 Mon Sep 17 00:00:00 2001 From: Arthur Zettler Date: Thu, 29 Nov 2018 14:17:37 -0200 Subject: [PATCH 3/5] test(loader): update tests to handle private plugins and playbacks list --- .../io/clappr/player/plugin/LoaderTest.kt | 135 ++++++++++++++---- 1 file changed, 108 insertions(+), 27 deletions(-) diff --git a/clappr/src/test/kotlin/io/clappr/player/plugin/LoaderTest.kt b/clappr/src/test/kotlin/io/clappr/player/plugin/LoaderTest.kt index 3c33afc9..03275259 100644 --- a/clappr/src/test/kotlin/io/clappr/player/plugin/LoaderTest.kt +++ b/clappr/src/test/kotlin/io/clappr/player/plugin/LoaderTest.kt @@ -2,9 +2,9 @@ package io.clappr.player.plugin import io.clappr.player.BuildConfig import io.clappr.player.base.BaseObject -import io.clappr.player.base.ContainerTest import io.clappr.player.base.NamedType import io.clappr.player.base.Options +import io.clappr.player.components.Container import io.clappr.player.components.Core import io.clappr.player.components.Playback import io.clappr.player.components.PlaybackSupportInterface @@ -20,22 +20,21 @@ import org.robolectric.shadows.ShadowApplication import kotlin.reflect.KClass @RunWith(RobolectricTestRunner::class) -@Config(constants = BuildConfig::class, sdk = intArrayOf(23)) +@Config(constants = BuildConfig::class, sdk = [23]) class LoaderTest { - class TestPlugin : Plugin(BaseObject()) { + class TestPlugin(baseObject: BaseObject) : Plugin(baseObject) { companion object: NamedType { override val name = "testplugin" } } - class SameNameTestPlugin : Plugin(BaseObject()) { + class SameNameTestPlugin(baseObject: BaseObject) : Plugin(baseObject) { companion object: NamedType { override val name = "testplugin" } } - class NoNameTestPlugin : Plugin(BaseObject()) { - } + class NoNameTestPlugin(baseObject: BaseObject) : Plugin(baseObject) class TestCorePlugin(core: Core) : CorePlugin(core) { companion object: NamedType { @@ -74,84 +73,159 @@ class LoaderTest { @Before fun setup() { BaseObject.context = ShadowApplication.getInstance().applicationContext + Loader.clearPlaybacks() Loader.clearPlugins() } @Test fun shouldHaveAnEmptyInitialPluginList() { + val expectedListSize = 0 + val loader = Loader() - assertTrue("default plugins should be empty", loader.availablePlugins.isEmpty()) + val corePlugins = loader.loadPlugins(Core(loader, Options())) + val containerPlugins = loader.loadPlugins(Container(loader, Options())) + + assertEquals(expectedListSize, corePlugins.size) + assertEquals(expectedListSize, containerPlugins.size) } @Test fun shouldAllowRegisteringPlugins() { + val expectedLoadedPluginsListSize = 1 + val expectedLoadedPluginName = "coreplugin" + Loader.registerPlugin(TestCorePlugin::class) + val loader = Loader() - assertTrue("no plugin have been registered", loader.availablePlugins.isNotEmpty()) + val loadedPlugins = loader.loadPlugins(Core(loader, Options())) + + assertEquals(expectedLoadedPluginsListSize, loadedPlugins.size) + assertEquals(expectedLoadedPluginName, loadedPlugins[0].name) } @Test - fun shouldAddExternalPlugins() { + fun shouldAllowUnregisteringPlugins() { + val expectedLoadedPluginsListSize = 0 + + Loader.registerPlugin(TestCorePlugin::class) + val didUnregistered = Loader.unregisterPlugin(TestCorePlugin::class) + val loader = Loader() - val externalPlugins = listOf>(TestPlugin::class) - val loaderExternal = Loader(externalPlugins) - assertTrue("no external plugin have been added", (loaderExternal.availablePlugins.size - loader.availablePlugins.size) == 1) + val loadedPlugins = loader.loadPlugins(Core(loader, Options())) + + assertTrue("plugin still registered", didUnregistered) + assertEquals(expectedLoadedPluginsListSize, loadedPlugins.size) + } + + @Test + fun shouldNotUnregisterNotRegisteredPlugin() { + val didUnregistered = Loader.unregisterPlugin(TestPlugin::class) + + assertFalse("plugin should not be unregistered", didUnregistered) + } + + @Test + fun shouldAddExternalPlugins() { + val expectedLoadedPluginsListSize = 1 + val expectedLoadedPluginName = "testplugin" + + val loaderExternal = Loader(listOf>(TestPlugin::class)) + val loadedPlugins = loaderExternal.loadPlugins(BaseObject()) + + assertEquals(expectedLoadedPluginsListSize, loadedPlugins.size) + assertEquals(expectedLoadedPluginName, loadedPlugins[0].name) } @Test fun shouldDisregardExternalPluginsWithoutName() { - val loader = Loader() + val expectedLoadedPluginsListSize = 0 val externalPlugins = listOf>(NoNameTestPlugin::class) + val loaderExternal = Loader(externalPlugins) - assertTrue("nameless external plugin added", loaderExternal.availablePlugins.size == loader.availablePlugins.size) + val loadedPlugins = loaderExternal.loadPlugins(BaseObject()) + + assertEquals(expectedLoadedPluginsListSize, loadedPlugins.size) } @Test fun externalPluginShouldReplaceDefaultPlugin() { + val expectedLoadedPluginsListSize = 1 + val expectedLoadedPluginName = "coreplugin" + Loader.registerPlugin(CorePlugin::class) + val loader = Loader() - assertNotNull("no default coreplugin: ${loader.availablePlugins}", loader.availablePlugins["coreplugin"]) - val externalPlugins = listOf>(TestCorePlugin::class) - val loaderExternal = Loader(externalPlugins) - assertFalse("no external plugin replace", loader.availablePlugins["coreplugin"] == loaderExternal.availablePlugins["coreplugin"]) - assertTrue("invalid external plugin", TestCorePlugin::class == loaderExternal.availablePlugins["coreplugin"]) + val loadedPlugins = loader.loadPlugins(Core(loader, Options())) + + assertEquals(expectedLoadedPluginsListSize, loadedPlugins.size) + assertEquals(expectedLoadedPluginName, loadedPlugins[0].name) + + val loaderExternal = Loader(listOf>(TestCorePlugin::class)) + val loadedExternalPlugins = loaderExternal.loadPlugins(Core(loaderExternal, Options())) + + assertEquals(expectedLoadedPluginsListSize, loadedExternalPlugins.size) + assertEquals(expectedLoadedPluginName, loadedExternalPlugins[0].name) + assertTrue("invalid external plugin", TestCorePlugin::class == loadedExternalPlugins[0]::class) } @Test fun shouldOverwritePluginWithDuplicateNames() { + val expectedLoadedPluginsListSize = 1 + val expectedLoadedPluginName = "testplugin" + Loader.registerPlugin(TestPlugin::class) Loader.registerPlugin(SameNameTestPlugin::class) + val loader = Loader() - assertTrue("should not have duplicate plugins", loader.availablePlugins.size == 1) + val loadedPlugins = loader.loadPlugins(BaseObject()) + + assertEquals(expectedLoadedPluginsListSize, loadedPlugins.size) + assertEquals(expectedLoadedPluginName, loadedPlugins[0].name) } @Test fun shouldHaveAnEmptyInitialPlaybackList() { val loader = Loader() - assertTrue("default playbacks should be empty", loader.availablePlaybacks.isEmpty()) + + val loadedDashPlayback = loader.loadPlayback("123.mpd", "video", Options()) + val loadedHLSPlayback = loader.loadPlayback("123.m3u8", "video", Options()) + val loadedMP4Playback = loader.loadPlayback("123.mp4", "video", Options()) + + assertNull("no playback should be loaded", loadedDashPlayback) + assertNull("no playback should be loaded", loadedHLSPlayback) + assertNull("no playback should be loaded", loadedMP4Playback) } @Test fun shouldAllowRegisteringPlaybacks() { Loader.registerPlayback(TestPlaybackMp4::class) + val loader = Loader() - assertTrue("default playbacks should not be empty", loader.availablePlaybacks.isNotEmpty()) + val loadedMP4Playback = loader.loadPlayback("123.mp4", "video", Options()) + + assertNotNull("mp4 playback should not be empty", loadedMP4Playback) } @Test fun shouldOverwritePlaybacksWithDuplicateNames() { Loader.registerPlayback(TestPlaybackMp4::class) Loader.registerPlayback(TestDuplicatePlayback::class) + val loader = Loader() - assertTrue("should not have duplicate playbacks", loader.availablePlaybacks.size == 1) + val loadedPlayback = loader.loadPlayback("123.mp4", "video", Options()) + + assertNotNull("playback should not be empty", loadedPlayback) + assertEquals(TestDuplicatePlayback::class, loadedPlayback!!::class) } @Test fun shouldInstantiatePlayback() { Loader.registerPlayback(TestPlaybackAny::class) + val loader = Loader() val playback = loader.loadPlayback("some-source.mp4", null, Options()) + assertNotNull("should have loaded playback", playback) } @@ -159,12 +233,15 @@ class LoaderTest { fun shouldInstantiatePlaybackWhichCanPlaySource() { Loader.registerPlayback(TestPlaybackMp4::class) Loader.registerPlayback(TestPlaybackDash::class) + val loader = Loader() var playback = loader.loadPlayback("some-source.mp4", null, Options()) + assertNotNull("should have loaded playback", playback) assertTrue("should load mp4 playback", playback is TestPlaybackMp4) playback = loader.loadPlayback("some-source.mpd", null, Options()) + assertNotNull("should have loaded playback", playback) assertTrue("should load dash playback", playback is TestPlaybackDash) } @@ -172,22 +249,26 @@ class LoaderTest { @Test fun shouldInstantiateFirstPlaybackInRegisteredListWhenThereAreMoreThanOneThatCanPlaySource() { Loader.registerPlayback(NoOpPlayback::class) - var loader = Loader() + + val loader = Loader() var playback = loader.loadPlayback("some-source.mp4", null, Options()) + assertNotNull("should have loaded playback", playback) assertTrue("should load no-op playback", playback is NoOpPlayback) Loader.registerPlayback(TestPlaybackMp4::class) - loader = loader + playback = loader.loadPlayback("some-source.mp4", null, Options()) + assertNotNull("should have loaded playback", playback) assertTrue("should load mp4 playback", playback is TestPlaybackMp4) } @Test fun shouldReturnNullForNoPlayback() { - var loader = Loader() - var playback = loader.loadPlayback("some-source.mp4", null, Options()) + val loader = Loader() + val playback = loader.loadPlayback("some-source.mp4", null, Options()) + assertNull("should not have loaded playback", playback) } } From 27772917899ef0adeb921bb065c440ac72dd925f Mon Sep 17 00:00:00 2001 From: Arthur Zettler Date: Mon, 3 Dec 2018 10:28:32 -0200 Subject: [PATCH 4/5] refactor(loader_visibility): make unregister plugin more kotlin like --- .../main/kotlin/io/clappr/player/plugin/Loader.kt | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt b/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt index ab73a1f6..b0a11320 100644 --- a/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt +++ b/clappr/src/main/kotlin/io/clappr/player/plugin/Loader.kt @@ -37,15 +37,12 @@ class Loader(extraPlugins: List> = emptyList(), extraPlayback } @JvmStatic - fun unregisterPlugin(pluginClass: KClass): Boolean { - val pluginName = (pluginClass.companionObjectInstance as? NamedType)?.name - pluginName?.let { - if (it.isNotEmpty() && registeredPlugins.containsKey(it)) { - return registeredPlugins.remove(it) != null + fun unregisterPlugin(pluginClass: KClass) = + (pluginClass.companionObjectInstance as? NamedType)?.run { + name?.takeIf { it.isNotEmpty() && registeredPlugins.containsKey(it) }?.let { + registeredPlugins.remove(it) != null } - } - return false - } + } ?: false @JvmStatic fun registerPlayback(playbackClass: KClass): Boolean { From fd0af4fab922497c9f5cdeea6ba0d05d9eef26c6 Mon Sep 17 00:00:00 2001 From: Video Players Date: Mon, 3 Dec 2018 12:32:20 -0200 Subject: [PATCH 5/5] chore(release): bump version --- clappr/build.gradle | 2 +- .../io.clappr.player.plugin/-loader/index.md | 14 +------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/clappr/build.gradle b/clappr/build.gradle index a570d37a..a50b8e17 100644 --- a/clappr/build.gradle +++ b/clappr/build.gradle @@ -20,7 +20,7 @@ apply plugin: 'org.jetbrains.dokka-android' apply from: 'versioning.gradle' group = 'io.clappr.player' -version = '0.10.5' +version = '0.10.6' def publishAttrs = buildPublishAttrs(version) diff --git a/doc/clappr/io.clappr.player.plugin/-loader/index.md b/doc/clappr/io.clappr.player.plugin/-loader/index.md index 1f2b0653..3da4bbb9 100644 --- a/doc/clappr/io.clappr.player.plugin/-loader/index.md +++ b/doc/clappr/io.clappr.player.plugin/-loader/index.md @@ -10,13 +10,6 @@ |---|---| | [<init>](-init-.md) | `Loader(extraPlugins: `[`List`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/index.html)`<`[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`> = emptyList(), extraPlaybacks: `[`List`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/index.html)`<`[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`> = emptyList())` | -### Properties - -| Name | Summary | -|---|---| -| [availablePlaybacks](available-playbacks.md) | `val availablePlaybacks: `[`MutableList`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-mutable-list/index.html)`<`[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`>` | -| [availablePlugins](available-plugins.md) | `val availablePlugins: `[`MutableMap`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-mutable-map/index.html)`<`[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`, `[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`>` | - ### Functions | Name | Summary | @@ -24,12 +17,6 @@ | [loadPlayback](load-playback.md) | `fun loadPlayback(source: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`, mimeType: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`? = null, options: `[`Options`](../../io.clappr.player.base/-options/index.md)`): `[`Playback`](../../io.clappr.player.components/-playback/index.md)`?` | | [loadPlugins](load-plugins.md) | `fun loadPlugins(context: `[`BaseObject`](../../io.clappr.player.base/-base-object/index.md)`): `[`List`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/index.html)`<`[`Plugin`](../-plugin/index.md)`>` | -### Companion Object Properties - -| Name | Summary | -|---|---| -| [registeredPlugins](registered-plugins.md) | `val registeredPlugins: `[`MutableMap`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-mutable-map/index.html)`<`[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`, `[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`>` | - ### Companion Object Functions | Name | Summary | @@ -39,3 +26,4 @@ | [registerPlayback](register-playback.md) | `fun registerPlayback(playbackClass: `[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`): `[`Boolean`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-boolean/index.html) | | [registerPlugin](register-plugin.md) | `fun registerPlugin(pluginClass: `[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`): `[`Boolean`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-boolean/index.html) | | [supportsSource](supports-source.md) | `fun supportsSource(playbackClass: `[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`, source: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`, mimeType: `[`String`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html)`? = null): `[`Boolean`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-boolean/index.html) | +| [unregisterPlugin](unregister-plugin.md) | `fun unregisterPlugin(pluginClass: `[`KClass`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-class/index.html)`): `[`Boolean`](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-boolean/index.html) |