Skip to content

Commit

Permalink
Separate startup listener from ActivityLifecycleListener interface (#977
Browse files Browse the repository at this point in the history
)

## Goal

ActivityLifecycleListener serves two purposes: to provide a hook to for callback invocations when Activities are opened and closed, along with a callback to invoke when app startup is complete. This PR separates the two so that the activity lifecycle one can be added to without indirectly modifying services that really only care about app startup, which is all but two of the services that implement this interface.

For now, we will keep the object that holds all the listeners the same, but eventually, this could be merged with the `StartupTracker` which provides a super set of functionality.

## Testing
Use existing tests to verify functionality, add unit tests to verify this second set of callbacks can be registered properlyl.
  • Loading branch information
bidetofevil authored Jun 20, 2024
2 parents 7a41765 + 1cdf952 commit 3d3566f
Show file tree
Hide file tree
Showing 13 changed files with 75 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import android.widget.Toast
import io.embrace.android.embracesdk.samples.AutomaticVerificationChecker
import io.embrace.android.embracesdk.samples.VerificationActions
import io.embrace.android.embracesdk.samples.VerifyIntegrationException
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ActivityTracker
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateListener
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateService
Expand All @@ -33,7 +32,7 @@ import kotlin.system.exitProcess
*/
internal class EmbraceAutomaticVerification(
private val scheduledExecutorService: ScheduledExecutorService = Executors.newSingleThreadScheduledExecutor()
) : ActivityLifecycleListener, ProcessStateListener {
) : ProcessStateListener {
private val handler = Handler(Looper.getMainLooper())

private var foregroundEventTriggered = false
Expand Down Expand Up @@ -73,7 +72,6 @@ internal class EmbraceAutomaticVerification(
if (!::processStateService.isInitialized) {
processStateService = checkNotNull(Embrace.getImpl().activityService)
}
activityLifecycleTracker.addListener(this)
processStateService.addListener(this)
}

Expand Down Expand Up @@ -336,6 +334,7 @@ internal class EmbraceAutomaticVerification(
private fun logWarning(message: String) {
Embrace.getInstance().internalInterface.logWarning("$TAG $message", null, null)
}

private fun logError(message: String) {
Embrace.getInstance().internalInterface.logError("$TAG $message", null, null, false)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import io.embrace.android.embracesdk.payload.AppInfo
import io.embrace.android.embracesdk.payload.DeviceInfo
import io.embrace.android.embracesdk.payload.DiskUsage
import io.embrace.android.embracesdk.prefs.PreferencesService
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateService
import io.embrace.android.embracesdk.session.lifecycle.StartupListener
import io.embrace.android.embracesdk.worker.BackgroundWorker
import java.io.ByteArrayOutputStream
import java.io.FileInputStream
Expand Down Expand Up @@ -67,7 +67,7 @@ internal class EmbraceMetadataService private constructor(
private val embraceCpuInfoDelegate: CpuInfoDelegate,
private val deviceArchitecture: DeviceArchitecture,
private val logger: EmbLogger
) : MetadataService, ActivityLifecycleListener {
) : MetadataService, StartupListener {

private val statFs = lazy { StatFs(Environment.getDataDirectory().path) }
private var reactNativeBundleId: Future<String?>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import io.embrace.android.embracesdk.internal.utils.Uuid.getEmbUuid
import io.embrace.android.embracesdk.logging.EmbLogger
import io.embrace.android.embracesdk.session.MemoryCleanerListener
import io.embrace.android.embracesdk.session.id.SessionIdTracker
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateListener
import io.embrace.android.embracesdk.session.lifecycle.StartupListener
import io.embrace.android.embracesdk.session.properties.EmbraceSessionProperties
import io.embrace.android.embracesdk.utils.stream
import io.embrace.android.embracesdk.worker.BackgroundWorker
Expand Down Expand Up @@ -43,7 +43,7 @@ internal class EmbraceEventService(
private val logger: EmbLogger,
workerThreadModule: WorkerThreadModule,
private val clock: Clock
) : EventService, ActivityLifecycleListener, ProcessStateListener, MemoryCleanerListener {
) : EventService, ProcessStateListener, MemoryCleanerListener, StartupListener {
private val backgroundWorker: BackgroundWorker

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ internal class ModuleInitBootstrapper(
serviceRegistry.registerActivityListeners(essentialServiceModule.processStateService)
serviceRegistry.registerMemoryCleanerListeners(essentialServiceModule.memoryCleanerService)
serviceRegistry.registerActivityLifecycleListeners(essentialServiceModule.activityLifecycleTracker)
serviceRegistry.registerStartupListener(essentialServiceModule.activityLifecycleTracker)

asyncInitTask.set(initTask)
synchronousInitCompletionMs = initModule.clock.now()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import io.embrace.android.embracesdk.internal.Systrace
import io.embrace.android.embracesdk.internal.clock.Clock
import io.embrace.android.embracesdk.internal.serialization.EmbraceSerializer
import io.embrace.android.embracesdk.internal.utils.Uuid.getEmbUuid
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.StartupListener
import io.embrace.android.embracesdk.worker.BackgroundWorker
import java.util.concurrent.Callable
import java.util.concurrent.Future
Expand All @@ -16,7 +16,7 @@ internal class EmbracePreferencesService(
lazyPrefs: Lazy<SharedPreferences>,
private val clock: Clock,
private val serializer: EmbraceSerializer
) : PreferencesService, ActivityLifecycleListener {
) : PreferencesService, StartupListener {

private val preferences: Future<SharedPreferences>
private val lazyPrefs: Lazy<SharedPreferences>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ActivityTracker
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateListener
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateService
import io.embrace.android.embracesdk.session.lifecycle.StartupListener
import java.io.Closeable
import java.util.concurrent.atomic.AtomicBoolean

Expand All @@ -27,6 +28,7 @@ internal class ServiceRegistry(
val memoryCleanerListeners by lazy { registry.filterIsInstance<MemoryCleanerListener>() }
val processStateListeners by lazy { registry.filterIsInstance<ProcessStateListener>() }
val activityLifecycleListeners by lazy { registry.filterIsInstance<ActivityLifecycleListener>() }
val startupListener by lazy { registry.filterIsInstance<StartupListener>() }

fun registerServices(vararg services: Any?) {
services.forEach(::registerService)
Expand Down Expand Up @@ -62,6 +64,12 @@ internal class ServiceRegistry(
memoryCleanerService::addListener
)

fun registerStartupListener(activityLifecycleTracker: ActivityTracker) =
startupListener.forEachSafe(
"Failed to register application lifecycle listener",
activityLifecycleTracker::addStartupListener
)

// close all of the services in one go. this prevents someone creating a Closeable service
// but forgetting to close it.
override fun close() = closeables.forEachSafe("Failed to close service", Closeable::close)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,4 @@ internal interface ActivityLifecycleListener {
* @param bundle the bundle
*/
fun onActivityCreated(activity: Activity, bundle: Bundle?) {}

/**
* Triggered when the application has completed startup;
*/
fun applicationStartupComplete() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,13 @@ internal class ActivityLifecycleTracker(
/**
* List of listeners that subscribe to activity events.
*/

val listeners = CopyOnWriteArrayList<ActivityLifecycleListener>()

/**
* List of listeners notified when application startup is complete
*/
val startupListeners = CopyOnWriteArrayList<StartupListener>()

/**
* The currently active activity.
*/
Expand Down Expand Up @@ -102,7 +106,7 @@ internal class ActivityLifecycleTracker(
if (!activity.javaClass.isAnnotationPresent(StartupActivity::class.java)) {
// If the activity coming to foreground doesn't have the StartupActivity annotation
// the the SDK will finalize any pending startup moment.
stream(listeners) { listener: ActivityLifecycleListener ->
stream(startupListeners) { listener: StartupListener ->
try {
listener.applicationStartupComplete()
} catch (ex: Exception) {
Expand Down Expand Up @@ -135,18 +139,24 @@ internal class ActivityLifecycleTracker(
}
}

override fun addStartupListener(listener: StartupListener) {
if (!startupListeners.contains(listener)) {
startupListeners.addIfAbsent(listener)
}
}

override fun close() {
try {
logger.logDebug("Shutting down EmbraceActivityService")
logger.logDebug("Shutting down ActivityLifecycleTracker")
application.unregisterActivityLifecycleCallbacks(this)
listeners.clear()
startupListeners.clear()
} catch (ex: Exception) {
logger.logWarning("Error when closing EmbraceActivityService", ex)
logger.logWarning("Error when closing ActivityLifecycleTracker", ex)
}
}

companion object {
private const val ERROR_FAILED_TO_NOTIFY =
"Failed to notify ActivityLifecycleTracker listener"
private const val ERROR_FAILED_TO_NOTIFY = "Failed to notify ActivityLifecycleTracker listener"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ internal interface ActivityTracker : Application.ActivityLifecycleCallbacks, Clo
val foregroundActivity: Activity?

fun addListener(listener: ActivityLifecycleListener)

fun addStartupListener(listener: StartupListener)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package io.embrace.android.embracesdk.session.lifecycle

internal interface StartupListener {
/**
* Triggered when the application has completed startup;
*/
fun applicationStartupComplete() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,23 @@ import android.app.Activity
import android.os.Bundle
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ActivityTracker
import io.embrace.android.embracesdk.session.lifecycle.StartupListener

internal class FakeActivityTracker(
override var foregroundActivity: Activity? = null
) : ActivityTracker {

val listeners = mutableListOf<ActivityLifecycleListener>()
val startupListeners = mutableListOf<StartupListener>()

override fun addListener(listener: ActivityLifecycleListener) {
listeners.add(listener)
}

override fun addStartupListener(listener: StartupListener) {
startupListeners.add(listener)
}

override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
TODO("Not yet implemented")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import io.embrace.android.embracesdk.logging.EmbLoggerImpl
import io.embrace.android.embracesdk.session.MemoryCleanerListener
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ProcessStateListener
import io.embrace.android.embracesdk.session.lifecycle.StartupListener
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
Expand All @@ -27,6 +28,7 @@ internal class ServiceRegistryTest {
assertEquals(expected, registry.processStateListeners)
assertEquals(expected, registry.activityLifecycleListeners)
assertEquals(expected, registry.memoryCleanerListeners)
assertEquals(expected, registry.startupListener)
}

@Test
Expand All @@ -42,7 +44,9 @@ internal class ServiceRegistryTest {

val activityLifecycleTracker = FakeActivityTracker()
registry.registerActivityLifecycleListeners(activityLifecycleTracker)
registry.registerStartupListener(activityLifecycleTracker)
assertEquals(expected, activityLifecycleTracker.listeners)
assertEquals(expected, activityLifecycleTracker.startupListeners)

val memoryCleanerService = FakeMemoryCleanerService()
registry.registerMemoryCleanerListeners(memoryCleanerService)
Expand All @@ -64,7 +68,8 @@ internal class ServiceRegistryTest {
Closeable,
MemoryCleanerListener,
ProcessStateListener,
ActivityLifecycleListener {
ActivityLifecycleListener,
StartupListener {

var closed = false

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import io.embrace.android.embracesdk.fakes.system.mockLooper
import io.embrace.android.embracesdk.logging.EmbLoggerImpl
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.session.lifecycle.ActivityLifecycleTracker
import io.embrace.android.embracesdk.session.lifecycle.StartupListener
import io.mockk.Called
import io.mockk.clearAllMocks
import io.mockk.every
Expand Down Expand Up @@ -140,12 +141,11 @@ internal class ActivityLifecycleTrackerTest {

@Test
fun `verify on activity resumed for a non StartupActivity does trigger listeners`() {
val mockActivityLifecycleListener = mockk<ActivityLifecycleListener>()
activityLifecycleTracker.addListener(mockActivityLifecycleListener)

val mockStartupListener = mockk<StartupListener>()
activityLifecycleTracker.addStartupListener(mockStartupListener)
activityLifecycleTracker.onActivityResumed(TestNonStartupActivity())

verify { mockActivityLifecycleListener.applicationStartupComplete() }
verify { mockStartupListener.applicationStartupComplete() }
}

@Test
Expand Down Expand Up @@ -197,6 +197,17 @@ internal class ActivityLifecycleTrackerTest {
assertEquals(1, activityLifecycleTracker.listeners.size)
}

@Test
fun `verify startup listener is added`() {
// assert empty list first
assertEquals(0, activityLifecycleTracker.startupListeners.size)

val mockStartupListeners = mockk<StartupListener>()
activityLifecycleTracker.addStartupListener(mockStartupListeners)

assertEquals(1, activityLifecycleTracker.startupListeners.size)
}

@Test
fun `verify if listener is already present, then it does not add anything`() {
val mockActivityLifecycleListener = mockk<ActivityLifecycleListener>()
Expand All @@ -223,13 +234,17 @@ internal class ActivityLifecycleTrackerTest {
fun `verify close cleans everything`() {
// add a listener first, so we then check that listener have been cleared
val mockActivityLifecycleListener = mockk<ActivityLifecycleListener>()
val mockStartupListeners = mockk<StartupListener>()
activityLifecycleTracker.addListener(mockActivityLifecycleListener)
activityLifecycleTracker.addStartupListener(mockStartupListeners)

every { application.unregisterActivityLifecycleCallbacks(activityLifecycleTracker) } returns Unit

activityLifecycleTracker.close()

verify { application.unregisterActivityLifecycleCallbacks(activityLifecycleTracker) }
assertTrue(activityLifecycleTracker.listeners.isEmpty())
assertTrue(activityLifecycleTracker.startupListeners.isEmpty())
}

@io.embrace.android.embracesdk.annotation.StartupActivity
Expand Down

0 comments on commit 3d3566f

Please sign in to comment.