diff --git a/changelog/unreleased/bugfixes/7757.md b/changelog/unreleased/bugfixes/7757.md new file mode 100644 index 00000000000..f0ca30832e5 --- /dev/null +++ b/changelog/unreleased/bugfixes/7757.md @@ -0,0 +1 @@ +- Resolved an issue where a crash could occur if telemetry sending settings were changed after creating `MapboxNavigation`. \ No newline at end of file diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt index 79ea03a1087..162cefe62a0 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt @@ -92,7 +92,7 @@ import com.mapbox.navigation.core.routealternatives.RouteAlternativesRequestCall import com.mapbox.navigation.core.routeoptions.RouteOptionsUpdater import com.mapbox.navigation.core.routerefresh.RouteRefreshController import com.mapbox.navigation.core.routerefresh.RouteRefreshControllerProvider -import com.mapbox.navigation.core.telemetry.MapboxNavigationTelemetry +import com.mapbox.navigation.core.telemetry.TelemetryWrapper import com.mapbox.navigation.core.telemetry.events.FeedbackEvent import com.mapbox.navigation.core.telemetry.events.FeedbackHelper import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata @@ -122,8 +122,6 @@ import com.mapbox.navigation.core.trip.session.eh.EHorizonObserver import com.mapbox.navigation.core.trip.session.eh.GraphAccessor import com.mapbox.navigation.core.trip.session.eh.RoadObjectMatcher import com.mapbox.navigation.core.trip.session.eh.RoadObjectsStore -import com.mapbox.navigation.metrics.MapboxMetricsReporter -import com.mapbox.navigation.metrics.internal.TelemetryUtilsDelegate import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator import com.mapbox.navigation.navigator.internal.NavigatorLoader import com.mapbox.navigation.navigator.internal.router.RouterInterfaceAdapter @@ -251,11 +249,15 @@ private const val MAPBOX_NOTIFICATION_ACTION_CHANNEL = "notificationActionButton class MapboxNavigation @VisibleForTesting internal constructor( val navigationOptions: NavigationOptions, private val threadController: ThreadController, + private val telemetryWrapper: TelemetryWrapper = TelemetryWrapper(), ) { - constructor(navigationOptions: NavigationOptions) : this(navigationOptions, ThreadController()) + constructor(navigationOptions: NavigationOptions) : this( + navigationOptions, + ThreadController(), + TelemetryWrapper(), + ) - private val accessToken: String? = navigationOptions.accessToken private val mainJobController = threadController.getMainScopeAndRootJob() private val directionsSession: DirectionsSession private var navigator: MapboxNativeNavigator @@ -469,7 +471,7 @@ class MapboxNavigation @VisibleForTesting internal constructor( val result = MapboxModuleProvider.createModule(MapboxModuleType.NavigationRouter) { paramsProvider( ModuleParams.NavigationRouter( - accessToken + navigationOptions.accessToken ?: throw RuntimeException(MAPBOX_NAVIGATION_TOKEN_EXCEPTION_ROUTER), nativeRouter, threadController, @@ -564,25 +566,11 @@ class MapboxNavigation @VisibleForTesting internal constructor( arrivalProgressObserver ) - ifNonNull(accessToken) { token -> - runInTelemetryContext { telemetry -> - logD( - "MapboxMetricsReporter.init from MapboxNavigation main", - telemetry.LOG_CATEGORY - ) - MapboxMetricsReporter.init( - navigationOptions.applicationContext, - token, - obtainUserAgent() - ) - MapboxMetricsReporter.toggleLogging(navigationOptions.isDebugLoggingEnabled) - telemetry.initialize( - this, - navigationOptions, - MapboxMetricsReporter, - ) - } - } + telemetryWrapper.initialize( + mapboxNavigation = this, + navigationOptions = navigationOptions, + userAgent = obtainUserAgent(), + ) val routeOptionsProvider = RouteOptionsUpdater() @@ -1294,9 +1282,7 @@ class MapboxNavigation @VisibleForTesting internal constructor( @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) routeRefreshController.destroy() routesPreviewController.unregisterAllRoutesPreviewObservers() - runInTelemetryContext { telemetry -> - telemetry.destroy(this@MapboxNavigation) - } + telemetryWrapper.destroy() threadController.cancelAllNonUICoroutines() threadController.cancelAllUICoroutines() ifNonNull(reachabilityObserverId) { @@ -1751,17 +1737,15 @@ class MapboxNavigation @VisibleForTesting internal constructor( feedbackMetadata: FeedbackMetadata?, userFeedbackCallback: UserFeedbackCallback?, ) { - runInTelemetryContext { telemetry -> - telemetry.postUserFeedback( - feedbackType, - description, - feedbackSource, - screenshot, - feedbackSubType, - feedbackMetadata, - userFeedbackCallback, - ) - } + telemetryWrapper.postUserFeedback( + feedbackType, + description, + feedbackSource, + screenshot, + feedbackSubType, + feedbackMetadata, + userFeedbackCallback, + ) } @ExperimentalPreviewMapboxNavigationAPI @@ -1770,13 +1754,11 @@ class MapboxNavigation @VisibleForTesting internal constructor( @CustomEvent.Type customEventType: String, customEventVersion: String, ) { - runInTelemetryContext { telemetry -> - telemetry.postCustomEvent( - payload = payload, - customEventType = customEventType, - customEventVersion = customEventVersion - ) - } + telemetryWrapper.postCustomEvent( + payload = payload, + customEventType = customEventType, + customEventVersion = customEventVersion + ) } /** @@ -1789,9 +1771,7 @@ class MapboxNavigation @VisibleForTesting internal constructor( */ @ExperimentalPreviewMapboxNavigationAPI fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper = - runInTelemetryContext { telemetry -> - telemetry.provideFeedbackMetadataWrapper() - } ?: throw java.lang.IllegalStateException( + telemetryWrapper.provideFeedbackMetadataWrapper() ?: throw java.lang.IllegalStateException( "To get FeedbackMetadataWrapper Telemetry must be enabled" ) @@ -2130,14 +2110,6 @@ class MapboxNavigation @VisibleForTesting internal constructor( } } - private inline fun runInTelemetryContext(func: (MapboxNavigationTelemetry) -> T): T? { - return if (TelemetryUtilsDelegate.getEventsCollectionState()) { - func(MapboxNavigationTelemetry) - } else { - null - } - } - private fun obtainUserAgent(): String { return "$MAPBOX_NAVIGATION_USER_AGENT_BASE/${BuildConfig.MAPBOX_NAVIGATION_VERSION_NAME}" } diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigationProvider.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigationProvider.kt index a2912c6b620..5fd27ed8c40 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigationProvider.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigationProvider.kt @@ -1,7 +1,10 @@ package com.mapbox.navigation.core import androidx.annotation.UiThread +import androidx.annotation.VisibleForTesting import com.mapbox.navigation.base.options.NavigationOptions +import com.mapbox.navigation.core.telemetry.TelemetryWrapper +import com.mapbox.navigation.utils.internal.ThreadController /** * Singleton responsible for ensuring there is only one MapboxNavigation instance. @@ -33,6 +36,22 @@ object MapboxNavigationProvider { return mapboxNavigation!! } + @VisibleForTesting + internal fun create( + navigationOptions: NavigationOptions, + threadController: ThreadController = ThreadController(), + telemetryWrapper: TelemetryWrapper = TelemetryWrapper() + ): MapboxNavigation { + mapboxNavigation?.onDestroy() + mapboxNavigation = MapboxNavigation( + navigationOptions, + threadController, + telemetryWrapper, + ) + + return mapboxNavigation!! + } + /** * Retrieve MapboxNavigation instance. Should be called after [create]. * diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/MapboxNavigationTelemetry.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/MapboxNavigationTelemetry.kt index 3431a459aeb..50e8454064a 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/MapboxNavigationTelemetry.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/MapboxNavigationTelemetry.kt @@ -151,8 +151,10 @@ The class must be initialized before any telemetry events are reported. Attempting to use telemetry before initialization is called will throw an exception. Initialization may be called multiple times, the call is idempotent. The class has two public methods, postUserFeedback() and initialize(). + +TODO(NAVAND-1820) refactor this class. It's hard to test because of statics. */ -internal object MapboxNavigationTelemetry { +internal object MapboxNavigationTelemetry : SdkTelemetry { internal const val LOG_CATEGORY = "MapboxNavigationTelemetry" private const val ONE_SECOND = 1000 @@ -354,10 +356,14 @@ internal object MapboxNavigationTelemetry { log("Valid initialization") } - fun destroy(mapboxNavigation: MapboxNavigation) { + override fun destroy(mapboxNavigation: MapboxNavigation) { telemetryStop() + + // TODO(NAVAND-1820) MapboxMetricsReporter is destroyed here, + // but initialized separately from MapboxNavigationTelemetry log("MapboxMetricsReporter disable") MapboxMetricsReporter.disable() + mapboxNavigation.run { unregisterLocationObserver(locationsCollector) unregisterRouteProgressObserver(routeProgressObserver) @@ -421,7 +427,7 @@ internal object MapboxNavigationTelemetry { userFeedbackCallbacks.remove(userFeedbackCallback) } - fun postCustomEvent( + override fun postCustomEvent( payload: String, customEventType: String, customEventVersion: String @@ -437,7 +443,7 @@ internal object MapboxNavigationTelemetry { } @ExperimentalPreviewMapboxNavigationAPI - fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper { + override fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper { (telemetryState as? NavTelemetryState.Running)?.sessionMetadata?.let { sessionMetadata -> return FeedbackMetadataWrapper( sessionMetadata.navigatorSessionIdentifier, @@ -462,7 +468,7 @@ internal object MapboxNavigationTelemetry { } @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) - fun postUserFeedback( + override fun postUserFeedback( feedbackType: String, description: String, @FeedbackEvent.Source feedbackSource: String, diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/SdkTelemetry.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/SdkTelemetry.kt new file mode 100644 index 00000000000..fce4c98ad2f --- /dev/null +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/SdkTelemetry.kt @@ -0,0 +1,67 @@ +package com.mapbox.navigation.core.telemetry + +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.internal.telemetry.CustomEvent +import com.mapbox.navigation.core.internal.telemetry.UserFeedbackCallback +import com.mapbox.navigation.core.telemetry.events.FeedbackEvent +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadataWrapper + +internal interface SdkTelemetry { + + fun postCustomEvent( + payload: String, + @CustomEvent.Type customEventType: String, + customEventVersion: String, + ) + + @ExperimentalPreviewMapboxNavigationAPI + fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper? + + @ExperimentalPreviewMapboxNavigationAPI + fun postUserFeedback( + feedbackType: String, + description: String, + @FeedbackEvent.Source feedbackSource: String, + screenshot: String?, + feedbackSubType: Array?, + feedbackMetadata: FeedbackMetadata?, + userFeedbackCallback: UserFeedbackCallback?, + ) + + fun destroy(mapboxNavigation: MapboxNavigation) + + companion object { + + val EMPTY = object : SdkTelemetry { + override fun postCustomEvent( + payload: String, + customEventType: String, + customEventVersion: String, + ) { + // do nothing + } + + @ExperimentalPreviewMapboxNavigationAPI + override fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper? = null + + @ExperimentalPreviewMapboxNavigationAPI + override fun postUserFeedback( + feedbackType: String, + description: String, + feedbackSource: String, + screenshot: String?, + feedbackSubType: Array?, + feedbackMetadata: FeedbackMetadata?, + userFeedbackCallback: UserFeedbackCallback?, + ) { + // do nothing + } + + override fun destroy(mapboxNavigation: MapboxNavigation) { + // do nothing + } + } + } +} diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/TelemetryWrapper.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/TelemetryWrapper.kt new file mode 100644 index 00000000000..038149729fd --- /dev/null +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/TelemetryWrapper.kt @@ -0,0 +1,131 @@ +package com.mapbox.navigation.core.telemetry + +import androidx.annotation.UiThread +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.base.options.NavigationOptions +import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.internal.telemetry.CustomEvent +import com.mapbox.navigation.core.internal.telemetry.UserFeedbackCallback +import com.mapbox.navigation.core.telemetry.events.FeedbackEvent +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadataWrapper +import com.mapbox.navigation.metrics.MapboxMetricsReporter +import com.mapbox.navigation.metrics.internal.TelemetryUtilsDelegate +import com.mapbox.navigation.utils.internal.assertDebug +import com.mapbox.navigation.utils.internal.logD + +/** + * Class that manages [MapboxNavigationTelemetry] initialisation. Listens to telemetry state events + * and can enable/disable telemetry in runtime. + * + * TODO(NAVAND-1820) ensure that we have 1-1 relationship between [TelemetryWrapper] and [MapboxNavigationTelemetry] + * [MapboxNavigationTelemetry] is very complex already and needs to be refactored. + */ +@UiThread +internal class TelemetryWrapper { + + private lateinit var mapboxNavigation: MapboxNavigation + private lateinit var navigationOptions: NavigationOptions + private lateinit var userAgent: String + + private var isWrapperInitialized = false + private var sdkTelemetry: SdkTelemetry = SdkTelemetry.EMPTY + + fun initialize( + mapboxNavigation: MapboxNavigation, + navigationOptions: NavigationOptions, + userAgent: String + ) { + assertDebug(!isWrapperInitialized) { + "Already initialized" + } + + if (isWrapperInitialized) { + return + } + + isWrapperInitialized = true + + this.mapboxNavigation = mapboxNavigation + this.navigationOptions = navigationOptions + this.userAgent = userAgent + + if (TelemetryUtilsDelegate.getEventsCollectionState()) { + initializeSdkTelemetry() + } + } + + fun destroy() { + assertDebug(isWrapperInitialized) { + "Initialize object first" + } + + if (!isWrapperInitialized) { + return + } + + isWrapperInitialized = false + + sdkTelemetry.destroy(mapboxNavigation) + sdkTelemetry = SdkTelemetry.EMPTY + } + + fun postCustomEvent( + payload: String, + @CustomEvent.Type customEventType: String, + customEventVersion: String, + ) { + sdkTelemetry.postCustomEvent( + payload = payload, + customEventType = customEventType, + customEventVersion = customEventVersion + ) + } + + @ExperimentalPreviewMapboxNavigationAPI + fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper? { + return sdkTelemetry.provideFeedbackMetadataWrapper() + } + + @ExperimentalPreviewMapboxNavigationAPI + fun postUserFeedback( + feedbackType: String, + description: String, + @FeedbackEvent.Source feedbackSource: String, + screenshot: String, + feedbackSubType: Array?, + feedbackMetadata: FeedbackMetadata?, + userFeedbackCallback: UserFeedbackCallback?, + ) { + sdkTelemetry.postUserFeedback( + feedbackType, + description, + feedbackSource, + screenshot, + feedbackSubType, + feedbackMetadata, + userFeedbackCallback, + ) + } + + private fun initializeSdkTelemetry() { + val token = navigationOptions.accessToken ?: return + + logD( + "MapboxMetricsReporter.init from MapboxNavigation main", + MapboxNavigationTelemetry.LOG_CATEGORY + ) + MapboxMetricsReporter.init( + navigationOptions.applicationContext, + token, + userAgent + ) + MapboxNavigationTelemetry.initialize( + mapboxNavigation, + navigationOptions, + MapboxMetricsReporter, + ) + + sdkTelemetry = MapboxNavigationTelemetry + } +} diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt index 1b462ecc4bb..d8e91c02961 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt @@ -31,6 +31,7 @@ import com.mapbox.navigation.core.routealternatives.RouteAlternativesControllerP import com.mapbox.navigation.core.routerefresh.RouteRefreshController import com.mapbox.navigation.core.routerefresh.RouteRefreshControllerProvider import com.mapbox.navigation.core.telemetry.MapboxNavigationTelemetry +import com.mapbox.navigation.core.telemetry.TelemetryWrapper import com.mapbox.navigation.core.testutil.createRoutesUpdatedResult import com.mapbox.navigation.core.trip.service.TripService import com.mapbox.navigation.core.trip.session.NativeSetRouteValue @@ -106,6 +107,7 @@ internal open class MapboxNavigationBaseTest { val historyRecordingStateHandler: HistoryRecordingStateHandler = mockk(relaxed = true) val developerMetadataAggregator: DeveloperMetadataAggregator = mockk(relaxUnitFun = true) val threadController = mockk(relaxed = true) + val telemetryWrapper = mockk(relaxed = true) val routeProgressDataProvider = mockk(relaxed = true) val routesPreviewController = mockk(relaxed = true) val routesCacheClearer = mockk(relaxed = true) @@ -262,7 +264,7 @@ internal open class MapboxNavigationBaseTest { } fun createMapboxNavigation() { - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) } private fun mockNativeNavigator() { diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt index 9f5d3019dc3..59f8a479461 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt @@ -36,7 +36,9 @@ import com.mapbox.navigation.core.routerefresh.RouteRefreshObserver import com.mapbox.navigation.core.routerefresh.RouteRefresherResult import com.mapbox.navigation.core.routerefresh.RouteRefresherStatus import com.mapbox.navigation.core.routerefresh.RoutesRefresherResult -import com.mapbox.navigation.core.telemetry.MapboxNavigationTelemetry +import com.mapbox.navigation.core.telemetry.events.FeedbackEvent +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadataWrapper import com.mapbox.navigation.core.testutil.createRoutesUpdatedResult import com.mapbox.navigation.core.trip.session.LocationObserver import com.mapbox.navigation.core.trip.session.NativeSetRouteError @@ -47,7 +49,6 @@ import com.mapbox.navigation.core.trip.session.RoadObjectsOnRouteObserver import com.mapbox.navigation.core.trip.session.TripSessionState import com.mapbox.navigation.core.trip.session.TripSessionStateObserver import com.mapbox.navigation.core.trip.session.createSetRouteResult -import com.mapbox.navigation.metrics.internal.TelemetryUtilsDelegate import com.mapbox.navigation.navigator.internal.NavigatorLoader import com.mapbox.navigation.testing.factories.createDirectionsRoute import com.mapbox.navigation.testing.factories.createNavigationRoute @@ -80,6 +81,7 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull +import org.junit.Assert.assertSame import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith @@ -229,7 +231,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { threadController.cancelAllUICoroutines() val navigationOptions = provideNavigationOptions().build() - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) verify(exactly = 2) { tripSession.registerOffRouteObserver(any()) } } @@ -240,7 +242,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { mapboxNavigation.onDestroy() threadController.cancelAllUICoroutines() val navigationOptions = provideNavigationOptions().build() - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) mapboxNavigation.onDestroy() @@ -409,55 +411,87 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { } @Test - fun telemetryIsDisabled() { - every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + fun initializeTelemetryOnSdkInitialisation() { + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) - createMapboxNavigation() - mapboxNavigation.onDestroy() - - verify(exactly = 0) { - MapboxNavigationTelemetry.initialize(any(), any(), any(), any()) + verify(exactly = 1) { + telemetryWrapper.initialize( + mapboxNavigation, + navigationOptions, + "mapbox-navigation-android/${BuildConfig.MAPBOX_NAVIGATION_VERSION_NAME}", + ) } - verify(exactly = 0) { MapboxNavigationTelemetry.destroy(any()) } + } + + @Test + fun telemetryIsEnabledTryToGetFeedbackMetadataWrapper() { + val feedbackMetadataWrapper = mockk(relaxed = true) + every { telemetryWrapper.provideFeedbackMetadataWrapper() } returns feedbackMetadataWrapper + + createMapboxNavigation() + assertSame(feedbackMetadataWrapper, mapboxNavigation.provideFeedbackMetadataWrapper()) } @ExperimentalPreviewMapboxNavigationAPI @Test(expected = IllegalStateException::class) fun telemetryIsDisabledTryToGetFeedbackMetadataWrapper() { - every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + every { telemetryWrapper.provideFeedbackMetadataWrapper() } returns null createMapboxNavigation() mapboxNavigation.provideFeedbackMetadataWrapper() } @ExperimentalPreviewMapboxNavigationAPI - fun telemetryIsDisabledTryToPostFeedback() { - every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false - + @Test + fun forwardPostUserFeedbackCallToTelemetry() { createMapboxNavigation() - mapboxNavigation.postUserFeedback(mockk(), mockk(), mockk(), mockk(), mockk()) - mapboxNavigation.postUserFeedback(mockk(), mockk(), mockk(), mockk(), mockk(), mockk()) + val feedbackSubType = emptyArray() + val feedbackMetadata = mockk() + val feedbackType = "test-type" + val description = "test-description" + val feedbackEvent = FeedbackEvent.REROUTE + val screenshot = "test-screenshot" - verify(exactly = 0) { - MapboxNavigationTelemetry.postUserFeedback( - any(), - any(), - any(), - any(), - any(), - any(), - any(), + mapboxNavigation.postUserFeedback( + feedbackType, + description, + feedbackEvent, + screenshot, + feedbackSubType, + feedbackMetadata, + ) + + verify(exactly = 1) { + telemetryWrapper.postUserFeedback( + feedbackType, + description, + feedbackEvent, + screenshot, + feedbackSubType, + feedbackMetadata, + null, ) } } + @Test + fun forwardPostCustomEventCallToTelemetry() { + createMapboxNavigation() + + mapboxNavigation.postCustomEvent("", NavigationCustomEventType.ANALYTICS, "1.0") + + verify(exactly = 1) { + telemetryWrapper.postCustomEvent("", NavigationCustomEventType.ANALYTICS, "1.0") + } + } + @Test fun unregisterAllTelemetryObservers() { createMapboxNavigation() mapboxNavigation.onDestroy() - verify(exactly = 1) { MapboxNavigationTelemetry.destroy(eq(mapboxNavigation)) } + verify(exactly = 1) { telemetryWrapper.destroy() } } @Test @@ -467,7 +501,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { verifyOrder { tripSession.stop() - MapboxNavigationTelemetry.destroy(mapboxNavigation) + telemetryWrapper.destroy() } } @@ -945,7 +979,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { .routingTilesOptions(RoutingTilesOptions.Builder().build()) .build() - mapboxNavigation = MapboxNavigation(options, threadController) + mapboxNavigation = MapboxNavigation(options, threadController, telemetryWrapper) assertTrue(slot.captured.tilesPath.endsWith(RoutingTilesFiles.TILES_PATH_SUB_DIR)) } @@ -966,7 +1000,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { ) .build() - mapboxNavigation = MapboxNavigation(options, threadController) + mapboxNavigation = MapboxNavigation(options, threadController, telemetryWrapper) assertEquals(slot.captured.endpointConfig!!.dataset, "someUser.osm/truck") } @@ -977,7 +1011,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { val slot = slot() every { NavigatorLoader.createConfig(any(), capture(slot)) } returns mockk() - mapboxNavigation = MapboxNavigation(navigationOptions) + createMapboxNavigation() assertNull(slot.captured.incidentsOptions) } @@ -995,7 +1029,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { ) .build() - mapboxNavigation = MapboxNavigation(options, threadController) + mapboxNavigation = MapboxNavigation(options, threadController, telemetryWrapper) assertEquals(slot.captured.incidentsOptions!!.graph, "graph") assertEquals(slot.captured.incidentsOptions!!.apiUrl, "") @@ -1014,7 +1048,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { ) .build() - mapboxNavigation = MapboxNavigation(options, threadController) + mapboxNavigation = MapboxNavigation(options, threadController, telemetryWrapper) assertEquals(slot.captured.incidentsOptions!!.apiUrl, "apiUrl") assertEquals(slot.captured.incidentsOptions!!.graph, "") @@ -1252,7 +1286,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { ) .build() - mapboxNavigation = MapboxNavigation(options, threadController) + mapboxNavigation = MapboxNavigation(options, threadController, telemetryWrapper) assertEquals(tilesVersion, slot.captured.endpointConfig?.version) assertFalse(slot.captured.endpointConfig?.isFallback!!) @@ -1272,7 +1306,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { ) every { tripSession.getRouteProgress() } returns mockk() - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) val tileConfigSlot = slot() @@ -1307,7 +1341,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { } returns listOf(mockPrimaryNavigationRoute, mockAlternativeNavigationRoute) every { tripSession.getRouteProgress()?.currentLegProgress?.legIndex } returns index - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + createMapboxNavigation() val tileConfigSlot = slot() @@ -1351,7 +1385,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { createSetRouteResult() } - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) fallbackObserverSlot.captured.onFallbackVersionsFound(listOf("version")) @@ -1368,7 +1402,8 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { @Test fun `verify that session state callbacks are always delivered to NavigationSession`() = runBlocking { - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + createMapboxNavigation() + every { directionsSession.initialLegIndex } returns 0 mapboxNavigation.startTripSession() mapboxNavigation.onDestroy() @@ -1386,15 +1421,15 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { @Test(expected = IllegalStateException::class) fun `verify that only one instance of MapboxNavigation can be alive`() = runBlocking { - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) } @Test fun `verify that MapboxNavigation instance can be recreated`() = runBlocking { - val firstInstance = MapboxNavigation(navigationOptions) + val firstInstance = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) firstInstance.onDestroy() - val secondInstance = MapboxNavigation(navigationOptions) + val secondInstance = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) assertNotNull(secondInstance) assertTrue(firstInstance.isDestroyed) @@ -1404,9 +1439,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { @Test(expected = IllegalStateException::class) fun `verify that the old instance is not accessible when a new one is created`() = runBlocking { - val firstInstance = MapboxNavigation(navigationOptions) + val firstInstance = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) firstInstance.onDestroy() - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) firstInstance.startTripSession() } @@ -1417,14 +1452,14 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { localNavigationSession } - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) mapboxNavigation.onDestroy() mapboxNavigation.startTripSession() } @Test(expected = IllegalStateException::class) fun `verify that stopTripSession is not called when destroyed`() = runBlocking { - mapboxNavigation = MapboxNavigation(navigationOptions, threadController) + mapboxNavigation = MapboxNavigation(navigationOptions, threadController, telemetryWrapper) mapboxNavigation.onDestroy() mapboxNavigation.stopTripSession() } @@ -1502,7 +1537,10 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { @Test fun `provider - check if the instance was destroyed outside of the providers scope`() { - val instance = MapboxNavigationProvider.create(navigationOptions) + val instance = MapboxNavigationProvider.create( + navigationOptions = navigationOptions, + telemetryWrapper = telemetryWrapper, + ) instance.onDestroy() @@ -2106,30 +2144,6 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { assertEquals(actual, mapboxNavigation.getRerouteController()) } - @Test - fun `when telemetry is enabled custom event is posted`() = coroutineRule.runBlockingTest { - createMapboxNavigation() - every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true - every { MapboxNavigationTelemetry.postCustomEvent(any(), any(), any()) } just Runs - every { MapboxNavigationTelemetry.destroy(any()) } just Runs - - mapboxNavigation.postCustomEvent("", NavigationCustomEventType.ANALYTICS, "1.0") - - verify(exactly = 1) { MapboxNavigationTelemetry.postCustomEvent(any(), any(), any()) } - } - - @Test - fun `when telemetry is disabled custom event is not posted`() = coroutineRule.runBlockingTest { - createMapboxNavigation() - every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false - every { MapboxNavigationTelemetry.postCustomEvent(any(), any(), any()) } just Runs - every { MapboxNavigationTelemetry.destroy(any()) } just Runs - - mapboxNavigation.postCustomEvent("", NavigationCustomEventType.ANALYTICS, "1.0") - - verify(exactly = 0) { MapboxNavigationTelemetry.postCustomEvent(any(), any(), any()) } - } - @Test fun requestRoadGraphDataUpdate() { val callback = mockk() diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/telemetry/TelemetryWrapperTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/telemetry/TelemetryWrapperTest.kt new file mode 100644 index 00000000000..f0d180d599f --- /dev/null +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/telemetry/TelemetryWrapperTest.kt @@ -0,0 +1,332 @@ +@file:OptIn(ExperimentalPreviewMapboxNavigationAPI::class) + +package com.mapbox.navigation.core.telemetry + +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.base.options.NavigationOptions +import com.mapbox.navigation.core.BuildConfig +import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.internal.telemetry.NavigationCustomEventType +import com.mapbox.navigation.core.internal.telemetry.UserFeedbackCallback +import com.mapbox.navigation.core.telemetry.events.FeedbackEvent +import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata +import com.mapbox.navigation.metrics.MapboxMetricsReporter +import com.mapbox.navigation.metrics.internal.EventsServiceProvider +import com.mapbox.navigation.metrics.internal.TelemetryServiceProvider +import com.mapbox.navigation.metrics.internal.TelemetryUtilsDelegate +import com.mapbox.navigation.utils.internal.logD +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.unmockkObject +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.After +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class TelemetryWrapperTest { + + private val testAccessToken = "test-access-token" + private val userAgent: String = "test-user-agent" + private lateinit var mapboxNavigation: MapboxNavigation + private lateinit var navigationOptions: NavigationOptions + + private lateinit var telemetryWrapper: TelemetryWrapper + + @Before + fun setUp() { + mockkObject(EventsServiceProvider) + every { + EventsServiceProvider.provideEventsService(any()) + } returns mockk(relaxUnitFun = true) + mockkObject(TelemetryServiceProvider) + every { + TelemetryServiceProvider.provideTelemetryService(any()) + } returns mockk(relaxUnitFun = true) + + mockkObject(MapboxNavigationTelemetry) + every { MapboxNavigationTelemetry.initialize(any(), any(), any(), any()) } just runs + every { MapboxNavigationTelemetry.destroy(any()) } just runs + every { + MapboxNavigationTelemetry.postUserFeedback( + any(), + any(), + any(), + any(), + any(), + any(), + any(), + ) + } just runs + every { + MapboxNavigationTelemetry.postCustomEvent( + any(), + any(), + any(), + ) + } just runs + every { + MapboxNavigationTelemetry.provideFeedbackMetadataWrapper() + } returns mockk(relaxed = true) + + mockkObject(TelemetryUtilsDelegate) + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true + mockkObject(MapboxMetricsReporter) + mockkStatic("com.mapbox.navigation.utils.internal.LoggerProviderKt") + every { logD(msg = any(), category = any()) } just Runs + + mapboxNavigation = mockk(relaxed = true) + navigationOptions = mockk(relaxed = true).apply { + every { accessToken } returns testAccessToken + every { applicationContext } returns mockk(relaxed = true) + } + + telemetryWrapper = TelemetryWrapper() + } + + @After + fun tearDown() { + unmockkObject(MapboxNavigationTelemetry) + unmockkObject(TelemetryUtilsDelegate) + unmockkObject(MapboxMetricsReporter) + unmockkStatic("com.mapbox.navigation.utils.internal.LoggerProviderKt") + } + + @Test + fun `throws exception in debug when initialize is called again`() { + if (BuildConfig.DEBUG) { + assertThrows( + "Already initialized", + IllegalStateException::class.java + ) { + telemetryWrapper.initialize() + telemetryWrapper.initialize() + } + } + } + + @Test + fun `throws exception in debug when destroy is called again`() { + if (BuildConfig.DEBUG) { + assertThrows( + "Initialize object first", + IllegalStateException::class.java + ) { + telemetryWrapper.initialize() + telemetryWrapper.destroy() + telemetryWrapper.destroy() + } + } + } + + @Test + fun `throws exception in debug when destroy is called without initialization`() { + if (BuildConfig.DEBUG) { + assertThrows( + "Initialize object first", + IllegalStateException::class.java + ) { + telemetryWrapper.destroy() + } + } + } + + @Test + fun `initializes MapboxNavigationTelemetry on initialization when telemetry is enabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true + + telemetryWrapper.initialize() + + verify(exactly = 1) { + MapboxNavigationTelemetry.initialize( + mapboxNavigation, + navigationOptions, + MapboxMetricsReporter, + any() + ) + } + } + + @Test + fun `doesn't initialize MapboxNavigationTelemetry on initialization when telemetry is disabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + + telemetryWrapper.initialize() + + verify(exactly = 0) { + MapboxNavigationTelemetry.initialize( + mapboxNavigation, + navigationOptions, + MapboxMetricsReporter, + any() + ) + } + } + + @Test + fun `posts custom event when telemetry is enabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true + telemetryWrapper.initialize() + telemetryWrapper.postCustomEvent(PAYLOAD, CUSTOM_EVENT_TYPE, CUSTOM_EVENT_VERSION) + + verify(exactly = 1) { + MapboxNavigationTelemetry.postCustomEvent( + PAYLOAD, + CUSTOM_EVENT_TYPE, + CUSTOM_EVENT_VERSION, + ) + } + } + + @Test + fun `doesn't post custom event when telemetry is disabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + telemetryWrapper.initialize() + telemetryWrapper.postCustomEvent(PAYLOAD, CUSTOM_EVENT_TYPE, CUSTOM_EVENT_VERSION) + + verify(exactly = 0) { + MapboxNavigationTelemetry.postCustomEvent(any(), any(), any()) + } + } + + @Test + fun `provides feedback metadata when telemetry is enabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true + telemetryWrapper.initialize() + + assertNotNull(telemetryWrapper.provideFeedbackMetadataWrapper()) + + verify(exactly = 1) { + MapboxNavigationTelemetry.provideFeedbackMetadataWrapper() + } + } + + @Test + fun `doesn't provide feedback metadata when telemetry is disabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + telemetryWrapper.initialize() + + assertNull(telemetryWrapper.provideFeedbackMetadataWrapper()) + + verify(exactly = 0) { + MapboxNavigationTelemetry.provideFeedbackMetadataWrapper() + } + } + + @Test + fun `posts user feedback when telemetry is enabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true + telemetryWrapper.initialize() + + val feedbackMetadata = mockk(relaxed = true) + val userFeedbackCallback = mockk(relaxed = true) + + telemetryWrapper.postUserFeedback( + FEEDBACK_TYPE, + DESCRIPTION, + FEEDBACK_SOURCE, + SCREENSHOT, + FEEDBACK_SUB_TYPE, + feedbackMetadata, + userFeedbackCallback + ) + + verify(exactly = 1) { + MapboxNavigationTelemetry.postUserFeedback( + FEEDBACK_TYPE, + DESCRIPTION, + FEEDBACK_SOURCE, + SCREENSHOT, + FEEDBACK_SUB_TYPE, + feedbackMetadata, + userFeedbackCallback + ) + } + } + + @Test + fun `doesn't post user feedback when telemetry is disabled`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + telemetryWrapper.initialize() + + telemetryWrapper.postUserFeedback( + FEEDBACK_TYPE, + DESCRIPTION, + FEEDBACK_SOURCE, + SCREENSHOT, + FEEDBACK_SUB_TYPE, + mockk(relaxed = true), + mockk(relaxed = true) + ) + + verify(exactly = 0) { + MapboxNavigationTelemetry.postUserFeedback( + any(), + any(), + any(), + any(), + any(), + any(), + any(), + ) + } + } + + @Test + fun `destroys MapboxNavigationTelemetry on destroy if MapboxNavigationTelemetry was initialized`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns true + + telemetryWrapper.initialize() + telemetryWrapper.destroy() + + verify(exactly = 1) { + MapboxNavigationTelemetry.destroy( + mapboxNavigation + ) + } + } + + @Test + fun `doesn't destroy MapboxNavigationTelemetry on destroy if MapboxNavigationTelemetry was not initialized`() { + every { TelemetryUtilsDelegate.getEventsCollectionState() } returns false + + telemetryWrapper.initialize() + telemetryWrapper.destroy() + + verify(exactly = 0) { + MapboxNavigationTelemetry.destroy( + mapboxNavigation + ) + } + } + + private fun TelemetryWrapper.initialize() { + telemetryWrapper.initialize( + mapboxNavigation, + navigationOptions, + userAgent, + ) + } + + private companion object { + const val PAYLOAD = "test-payload" + const val CUSTOM_EVENT_TYPE = NavigationCustomEventType.ANALYTICS + const val CUSTOM_EVENT_VERSION = "test-version" + const val FEEDBACK_TYPE = "test-feedback-type" + const val DESCRIPTION = "test-description" + const val FEEDBACK_SOURCE = FeedbackEvent.REROUTE + const val SCREENSHOT = "test-screenshot" + val FEEDBACK_SUB_TYPE = arrayOf("test-type") + } +} diff --git a/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/Assertions.kt b/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/Assertions.kt new file mode 100644 index 00000000000..8ac6d13854c --- /dev/null +++ b/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/Assertions.kt @@ -0,0 +1,13 @@ +package com.mapbox.navigation.utils.internal + +import com.mapbox.navigation.utils.BuildConfig + +inline fun assertDebug(value: Boolean, message: () -> Any) { + if (BuildConfig.DEBUG) { + check(value, message) + } + + if (!value) { + logW { message().toString() } + } +}