Refactor navigation state so core is impl agnostic #3208
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package io.embrace.android.embracesdk.internal.navigation | ||
|
|
||
| import android.app.Activity | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationControllerEventListener | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingInitListener | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingService | ||
|
|
||
| internal class NavigationTrackingServiceImpl : NavigationTrackingService { | ||
|
|
||
| private var navigationTrackingInitListener: NavigationTrackingInitListener = NoopNavigationTrackingInitListener | ||
| private var navigationControllerEventListener: NavigationControllerEventListener = NoopNavigationControllerEventListener | ||
|
|
||
| override fun setTrackingInitListener(listener: NavigationTrackingInitListener) { | ||
| navigationTrackingInitListener = listener | ||
| } | ||
|
|
||
| override fun setControllerEventListener(listener: NavigationControllerEventListener) { | ||
| navigationControllerEventListener = listener | ||
| } | ||
|
|
||
| override fun trackNavigation(activity: Activity, controller: Any?) { | ||
| navigationTrackingInitListener.trackNavigation(activity, controller) | ||
| } | ||
|
|
||
| override fun onControllerAttached(activity: Activity, timestampMs: Long) { | ||
| navigationControllerEventListener.onControllerAttached(activity, timestampMs) | ||
| } | ||
|
|
||
| override fun onDestinationChange(activity: Activity, screenName: String, timestampMs: Long) { | ||
| navigationControllerEventListener.onDestinationChange(activity, screenName, timestampMs) | ||
| } | ||
| } | ||
|
|
||
| private object NoopNavigationTrackingInitListener : NavigationTrackingInitListener { | ||
| override fun trackNavigation(activity: Activity, controller: Any?) {} | ||
| } | ||
|
|
||
| private object NoopNavigationControllerEventListener : NavigationControllerEventListener { | ||
| override fun onControllerAttached(activity: Activity, timestampMs: Long) {} | ||
| override fun onDestinationChange(activity: Activity, screenName: String, timestampMs: Long) {} | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package io.embrace.android.embracesdk.fakes | ||
|
|
||
| import android.app.Activity | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationControllerEventListener | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingInitListener | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingService | ||
|
|
||
| class FakeNavigationTrackingService : NavigationTrackingService { | ||
| val attachedCalls = mutableListOf<AttachedCall>() | ||
| val destinationChangedCalls = mutableListOf<DestinationChangedCall>() | ||
|
|
||
| override fun setTrackingInitListener(listener: NavigationTrackingInitListener) {} | ||
| override fun setControllerEventListener(listener: NavigationControllerEventListener) {} | ||
| override fun trackNavigation(activity: Activity, controller: Any?) {} | ||
|
|
||
| override fun onControllerAttached(activity: Activity, timestampMs: Long) { | ||
| attachedCalls.add(AttachedCall(activity, timestampMs)) | ||
| } | ||
|
|
||
| override fun onDestinationChange(activity: Activity, screenName: String, timestampMs: Long) { | ||
| destinationChangedCalls.add(DestinationChangedCall(activity, screenName, timestampMs)) | ||
| } | ||
|
|
||
| data class AttachedCall(val activity: Activity, val timestampMs: Long) | ||
| data class DestinationChangedCall(val activity: Activity, val screenName: String, val timestampMs: Long) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package io.embrace.android.embracesdk.internal.arch.navigation | ||
|
|
||
| import android.app.Activity | ||
|
|
||
| /** | ||
| * Listener that receives events related to components that control navigation | ||
| */ | ||
| interface NavigationControllerEventListener { | ||
| /** | ||
| * Called when a component that controls navigation is attached | ||
| */ | ||
| fun onControllerAttached(activity: Activity, timestampMs: Long) | ||
|
|
||
| /** | ||
| * Called when a navigation component attached to the given activity updates its destination | ||
| */ | ||
| fun onDestinationChange(activity: Activity, screenName: String, timestampMs: Long) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package io.embrace.android.embracesdk.internal.arch.navigation | ||
|
|
||
| import android.app.Activity | ||
|
|
||
| /** | ||
| * Receives events related to the initialization of components that control navigation | ||
| */ | ||
| interface NavigationTrackingInitListener { | ||
| /** | ||
| * Track navigation events from a controller associated with the given Activity. If the controller is null, the implementation | ||
| * will try to find the controller within the given activity. | ||
| * | ||
| * The implementation is responsible for casting the controller to whatever interface it needs in order to do its tracking, | ||
| * as the interface is generic and agnostic to any navigation controller implementation details. | ||
| */ | ||
| fun trackNavigation(activity: Activity, controller: Any? = null) | ||
|
Member
There was a problem hiding this comment. What type do we expect the controller to be? Is there any way to make it more specific than
Contributor
Author
There was a problem hiding this comment. For now, it's NavController. And I'd be happy to have that explicitly until there's another type, but then that means we'll have to expose this Compose Navigation dependency at the public API level. I started out with way, but then went to this generic method. It'll be more obvious in my next PR. |
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package io.embrace.android.embracesdk.internal.arch.navigation | ||
|
|
||
| /** | ||
| * Service where navigation controllers can be registered, and when they fire events, they will be dispatched to the listeners. | ||
| */ | ||
| interface NavigationTrackingService : NavigationTrackingInitListener, NavigationControllerEventListener { | ||
| /** | ||
| * Register listener that receives events related to the initialization of components that control navigation | ||
| */ | ||
| fun setTrackingInitListener(listener: NavigationTrackingInitListener) | ||
|
|
||
| /** | ||
| * Register listener that receives events related to components that control navigation | ||
| */ | ||
| fun setControllerEventListener(listener: NavigationControllerEventListener) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package io.embrace.android.embracesdk.internal.instrumentation.compose.navigation | ||
|
|
||
| import io.embrace.android.embracesdk.internal.arch.InstrumentationArgs | ||
| import io.embrace.android.embracesdk.internal.arch.InstrumentationProvider | ||
| import io.embrace.android.embracesdk.internal.arch.datasource.DataSourceState | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingService | ||
|
|
||
| /** | ||
| * Instrumentation provider that creates and registers [NavControllerTracker] with the [NavigationTrackingService]. | ||
| */ | ||
| class ComposeNavigationInstrumentationProvider : InstrumentationProvider { | ||
|
Contributor
Author
There was a problem hiding this comment. Not sure if this is a good way to to use the data source interface because it doesn't actually provide a datasource, but it allows for the dynamic loading of the compose navigation tracking via a new module. Is there a better way to do this? |
||
|
|
||
| override fun register(args: InstrumentationArgs): DataSourceState<*>? { | ||
| val tracker = NavControllerTracker(args.navigationTrackingService, args.clock, args.logger) | ||
| args.navigationTrackingService.setTrackingInitListener(tracker) | ||
| return null | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package io.embrace.android.embracesdk.internal.instrumentation.compose.navigation | ||
|
|
||
| import android.app.Activity | ||
| import androidx.fragment.app.FragmentActivity | ||
| import androidx.navigation.NavController | ||
| import androidx.navigation.NavDestination | ||
| import androidx.navigation.fragment.NavHostFragment | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingInitListener | ||
| import io.embrace.android.embracesdk.internal.arch.navigation.NavigationTrackingService | ||
| import io.embrace.android.embracesdk.internal.clock.Clock | ||
| import io.embrace.android.embracesdk.internal.logging.InternalErrorType | ||
| import io.embrace.android.embracesdk.internal.logging.InternalLogger | ||
|
|
||
| /** | ||
| * Discovers and attaches [NavController.OnDestinationChangedListener] instances to Activities that use a [NavController]. | ||
| * Implements [NavigationTrackingInitListener] so it can be registered with [NavigationTrackingService] tracking events. | ||
| */ | ||
| internal class NavControllerTracker( | ||
| private val navigationTrackingService: NavigationTrackingService, | ||
| private val clock: Clock, | ||
| private val logger: InternalLogger, | ||
| ) : NavigationTrackingInitListener { | ||
|
|
||
| private val trackAttemptStatus = mutableMapOf<Int, Boolean>() | ||
|
|
||
| override fun trackNavigation(activity: Activity, controller: Any?) { | ||
| runCatching { | ||
| val activityId = System.identityHashCode(activity) | ||
| if (trackAttemptStatus[activityId] != true) { | ||
| synchronized(trackAttemptStatus) { | ||
| if (controller == null && trackAttemptStatus.put(activityId, false) == null) { | ||
| findNavController(activity)?.trackForActivity(activity) | ||
| } else if (controller is NavController && trackAttemptStatus[activityId] != true) { | ||
| controller.trackForActivity(activity) | ||
| } | ||
| } | ||
| } | ||
| }.onFailure { | ||
| logger.trackInternalError(InternalErrorType.NAV_CONTROLLER_TRACKING_FAIL, it) | ||
| } | ||
| } | ||
|
|
||
| private fun NavController.trackForActivity(activity: Activity) { | ||
| navigationTrackingService.onControllerAttached(activity, clock.now()) | ||
| addOnDestinationChangedListener { _, destination, _ -> | ||
| navigationTrackingService.onDestinationChange(activity, extractScreenName(destination), clock.now()) | ||
| } | ||
| trackAttemptStatus[System.identityHashCode(activity)] = true | ||
| } | ||
|
|
||
| private fun findNavController(activity: Activity): NavController? { | ||
| if (activity is FragmentActivity) { | ||
| val navHostFragment = activity.supportFragmentManager.fragments | ||
| .firstNotNullOfOrNull { it as? NavHostFragment } | ||
| if (navHostFragment != null) { | ||
| return navHostFragment.navController | ||
| } | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| private fun extractScreenName(destination: NavDestination): String { | ||
| return destination.route | ||
| ?: destination.label?.toString() | ||
| ?: destination.navigatorName | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| io.embrace.android.embracesdk.internal.instrumentation.compose.navigation.ComposeNavigationInstrumentationProvider |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.