From 8dbc73acda55dcbe23258472c6b035d21b6ce32d Mon Sep 17 00:00:00 2001 From: Kyle Madsen Date: Tue, 6 Sep 2022 11:52:43 -0700 Subject: [PATCH] Make requireMapboxNavigation attach during initialization --- CHANGELOG.md | 1 - libnavigation-core/api/current.txt | 2 +- .../core/lifecycle/RequireMapboxNavigation.kt | 37 ++++++++++--------- .../lifecycle/RequireMapboxNavigationTest.kt | 12 ++++++ .../view/InactiveRouteStylingActivity.kt | 1 - 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5c7358cb19..2a2e3cddf99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ Mapbox welcomes participation and contributions from everyone. ## Unreleased #### Features - Improved the route refresh feature to also refresh _closures_ (`RouteLeg#closures`). [#6274](https://github.com/mapbox/mapbox-navigation-android/pull/6274) - - Added `requireMapboxNavigation` to offer a simple way to use `MapboxNavigationApp`. [#6233](https://github.com/mapbox/mapbox-navigation-android/pull/6233) #### Bug fixes and improvements diff --git a/libnavigation-core/api/current.txt b/libnavigation-core/api/current.txt index b36d2562eee..ef6b71499d3 100644 --- a/libnavigation-core/api/current.txt +++ b/libnavigation-core/api/current.txt @@ -338,7 +338,7 @@ package com.mapbox.navigation.core.lifecycle { } public final class RequireMapboxNavigation { - method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static kotlin.properties.ReadOnlyProperty requireMapboxNavigation(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onCreatedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onStartedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onResumedObserver = null, kotlin.jvm.functions.Function0? onInitialize = null); + method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static kotlin.properties.ReadOnlyProperty requireMapboxNavigation(androidx.lifecycle.LifecycleOwner, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onCreatedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onStartedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onResumedObserver = null, kotlin.jvm.functions.Function0? onInitialize = null); } } diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt index 1bc2656815e..662385a272b 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt @@ -7,12 +7,13 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp.lifecycleOwner import kotlin.properties.ReadOnlyProperty import kotlin.reflect.KProperty /** - * Extension function to make it simple to create the [RequireMapboxNavigationDelegate]. Below are - * a couple examples of how you may use the delegate. + * Extension function to make it simple to create the [RequireMapboxNavigationDelegate]. + * Below are a couple examples of how you may use the delegate. * * Default can be used when [MapboxNavigationApp] is setup elsewhere. * ``` @@ -51,12 +52,13 @@ import kotlin.reflect.KProperty * @see [RequireMapboxNavigationDelegate] for more details. */ @ExperimentalPreviewMapboxNavigationAPI -fun requireMapboxNavigation( +fun LifecycleOwner.requireMapboxNavigation( onCreatedObserver: MapboxNavigationObserver? = null, onStartedObserver: MapboxNavigationObserver? = null, onResumedObserver: MapboxNavigationObserver? = null, onInitialize: (() -> Unit)? = null, -): ReadOnlyProperty = RequireMapboxNavigationDelegate( +): ReadOnlyProperty = RequireMapboxNavigationDelegate( + lifecycleOwner = this, onCreatedObserver = onCreatedObserver, onStartedObserver = onStartedObserver, onResumedObserver = onResumedObserver, @@ -74,22 +76,23 @@ fun requireMapboxNavigation( * [MapboxNavigation] instance can be re-created with [MapboxNavigationApp.disable], or if all * [MapboxNavigationApp.attach] lifecycles are destroyed. * + * @param lifecycleOwner: LifecycleOwner * @param onCreatedObserver registered to the [Lifecycle.State.CREATED] lifecycle * @param onStartedObserver registered to the [Lifecycle.State.STARTED] lifecycle * @param onResumedObserver registered to the [Lifecycle.State.RESUMED] lifecycle - * @param onInitialize called when the property is read for the first time + * @param onInitialize called when the [lifecycleOwner] is [Lifecycle.State.CREATED] */ internal class RequireMapboxNavigationDelegate( + lifecycleOwner: LifecycleOwner, private val onCreatedObserver: MapboxNavigationObserver? = null, private val onStartedObserver: MapboxNavigationObserver? = null, private val onResumedObserver: MapboxNavigationObserver? = null, private val onInitialize: (() -> Unit)? = null -) : ReadOnlyProperty { - - private lateinit var lifecycleOwner: LifecycleOwner +) : ReadOnlyProperty { private val lifecycleObserver = object : DefaultLifecycleObserver { override fun onCreate(owner: LifecycleOwner) { + onInitialize?.invoke() onCreatedObserver?.let { MapboxNavigationApp.registerObserver(it) } } @@ -114,21 +117,19 @@ internal class RequireMapboxNavigationDelegate( } } + init { + MapboxNavigationApp.attach(lifecycleOwner) + lifecycleOwner.lifecycle.addObserver(lifecycleObserver) + } + /** - * Returns an instance of [MapboxNavigation], the first retrieval will attach the [thisRef] - * to [MapboxNavigationApp.attach]. If [MapboxNavigationApp.isSetup] is false after all - * observers and initializers, this property getter will crash. + * Returns an instance of [MapboxNavigation]. If [MapboxNavigationApp.isSetup] is false after + * all observers and initializers, this property getter will crash. * * @param thisRef - the [LifecycleOwner] that needs access to [MapboxNavigation]. * @param property - ignored */ - override fun getValue(thisRef: LifecycleOwner, property: KProperty<*>): MapboxNavigation { - if (!this::lifecycleOwner.isInitialized) { - onInitialize?.invoke() - this.lifecycleOwner = thisRef - MapboxNavigationApp.attach(lifecycleOwner) - lifecycleOwner.lifecycle.addObserver(lifecycleObserver) - } + override fun getValue(thisRef: Any, property: KProperty<*>): MapboxNavigation { val mapboxNavigation = MapboxNavigationApp.current() checkNotNull(mapboxNavigation) { "MapboxNavigation cannot be null. Ensure that MapboxNavigationApp is setup and an" + diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt index 2d8ad731d18..7a2e6e97d75 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt @@ -81,6 +81,18 @@ class RequireMapboxNavigationTest { sut.mapboxNavigation } + @Test + fun `onInitialize is called before first reference to the delegate`() { + mockMapboxNavigationAppBehavior() + val mockOnInitialize: (() -> Unit) = mockk(relaxed = true) + val sut = SystemUnderTest(onInitialize = mockOnInitialize) + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + + verify { mockOnInitialize.invoke() } + } + @Test fun `multiple delegate references are the same instance`() { mockMapboxNavigationAppBehavior() diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt index 7b4986ccb3c..813d8ebe1dc 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt @@ -93,7 +93,6 @@ class InactiveRouteStylingActivity : AppCompatActivity() { private val mapboxNavigation by requireMapboxNavigation( onCreatedObserver = object : MapboxNavigationObserver { override fun onAttached(mapboxNavigation: MapboxNavigation) { - mapboxNavigation.registerLocationObserver(locationObserver) mapboxNavigation.registerRouteProgressObserver(routeProgressObserver) mapboxNavigation.registerRouteProgressObserver(replayProgressObserver)