diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 349507a47..321e938d2 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -25,7 +25,7 @@ object Config { val leakCanary = "com.squareup.leakcanary:leakcanary-android:2.1" val lifecycleVersion = "2.2.0" - val lifecycleProcessor = "androidx.lifecycle:lifecycle-process:$lifecycleVersion" + val lifecycleProcess = "androidx.lifecycle:lifecycle-process:$lifecycleVersion" val lifecycleCommonJava8 = "androidx.lifecycle:lifecycle-common-java8:$lifecycleVersion" } @@ -38,6 +38,7 @@ object Config { val androidxJunit = "androidx.test.ext:junit:1.1.1" val robolectric = "org.robolectric:robolectric:4.3.1" val mockitoKotlin = "com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0" + val awaitility = "org.awaitility:awaitility-kotlin:4.0.2" } object QualityPlugins { diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index 04df63fab..9af23d4b9 100644 --- a/sentry-android-core/build.gradle.kts +++ b/sentry-android-core/build.gradle.kts @@ -79,7 +79,7 @@ dependencies { implementation(Config.Libs.gson) // lifecycle processor, session tracking - implementation(Config.Libs.lifecycleProcessor) + implementation(Config.Libs.lifecycleProcess) implementation(Config.Libs.lifecycleCommonJava8) compileOnly(Config.CompileOnly.nopen) @@ -96,6 +96,7 @@ dependencies { testImplementation(Config.TestLibs.androidxRunner) testImplementation(Config.TestLibs.androidxJunit) testImplementation(Config.TestLibs.mockitoKotlin) + testImplementation(Config.TestLibs.awaitility) } //TODO: move thse blocks to parent gradle file, DRY diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index 6b5abd83b..36956e622 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -5,10 +5,14 @@ import io.sentry.core.Breadcrumb; import io.sentry.core.IHub; import io.sentry.core.SentryLevel; +import io.sentry.core.transport.CurrentDateProvider; +import io.sentry.core.transport.ICurrentDateProvider; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; final class LifecycleWatcher implements DefaultLifecycleObserver { @@ -21,16 +25,34 @@ final class LifecycleWatcher implements DefaultLifecycleObserver { private final @NotNull IHub hub; private final boolean enableSessionTracking; private final boolean enableAppLifecycleBreadcrumbs; + private final @NotNull AtomicBoolean runningSession = new AtomicBoolean(); + + private final @NotNull ICurrentDateProvider currentDateProvider; LifecycleWatcher( final @NotNull IHub hub, final long sessionIntervalMillis, final boolean enableSessionTracking, final boolean enableAppLifecycleBreadcrumbs) { + this( + hub, + sessionIntervalMillis, + enableSessionTracking, + enableAppLifecycleBreadcrumbs, + CurrentDateProvider.getInstance()); + } + + LifecycleWatcher( + final @NotNull IHub hub, + final long sessionIntervalMillis, + final boolean enableSessionTracking, + final boolean enableAppLifecycleBreadcrumbs, + final @NotNull ICurrentDateProvider currentDateProvider) { this.sessionIntervalMillis = sessionIntervalMillis; this.enableSessionTracking = enableSessionTracking; this.enableAppLifecycleBreadcrumbs = enableAppLifecycleBreadcrumbs; this.hub = hub; + this.currentDateProvider = currentDateProvider; } // App goes to foreground @@ -42,12 +64,13 @@ public void onStart(final @NotNull LifecycleOwner owner) { private void startSession() { if (enableSessionTracking) { - final long currentTimeMillis = System.currentTimeMillis(); + final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis(); cancelTask(); if (lastStartedSession == 0L || (lastStartedSession + sessionIntervalMillis) <= currentTimeMillis) { addSessionBreadcrumb("start"); hub.startSession(); + runningSession.set(true); } lastStartedSession = currentTimeMillis; } @@ -72,6 +95,7 @@ private void scheduleEndSession() { public void run() { addSessionBreadcrumb("end"); hub.endSession(); + runningSession.set(false); } }; @@ -81,6 +105,7 @@ public void run() { private void cancelTask() { if (timerTask != null) { timerTask.cancel(); + timerTask = null; } } @@ -103,4 +128,16 @@ private void addSessionBreadcrumb(final @NotNull String state) { breadcrumb.setLevel(SentryLevel.INFO); hub.addBreadcrumb(breadcrumb); } + + @TestOnly + @NotNull + AtomicBoolean isRunningSession() { + return runningSession; + } + + @TestOnly + @Nullable + TimerTask getTimerTask() { + return timerTask; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt index 61f8283b0..3b1091af8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt @@ -1,93 +1,105 @@ package io.sentry.android.core +import androidx.lifecycle.LifecycleOwner import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever import io.sentry.core.Breadcrumb import io.sentry.core.IHub import io.sentry.core.SentryLevel -import kotlin.test.Ignore +import io.sentry.core.transport.ICurrentDateProvider import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import org.awaitility.kotlin.await class LifecycleWatcherTest { + private class Fixture { + val ownerMock = mock() + val hub = mock() + val dateProvider = mock() + + fun getSUT(sessionIntervalMillis: Long = 0L, enableSessionTracking: Boolean = true, enableAppLifecycleBreadcrumbs: Boolean = true): LifecycleWatcher { + return LifecycleWatcher(hub, sessionIntervalMillis, enableSessionTracking, enableAppLifecycleBreadcrumbs, dateProvider) + } + } + + private val fixture = Fixture() + @Test fun `if last started session is 0, start new session`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 100L, true, false) - watcher.onStart(mock()) - verify(hub).startSession() + val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub).startSession() } @Test fun `if last started session is after interval, start new session`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 100L, true, false) - watcher.onStart(mock()) - Thread.sleep(150L) - watcher.onStart(mock()) - verify(hub, times(2)).startSession() + val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) + whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L, 2L) + watcher.onStart(fixture.ownerMock) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub, times(2)).startSession() } @Test fun `if last started session is before interval, it should not start a new session`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 1000L, true, false) - watcher.onStart(mock()) - Thread.sleep(100) - watcher.onStart(mock()) - verify(hub).startSession() + val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) + whenever(fixture.dateProvider.currentTimeMillis).thenReturn(2L, 1L) + watcher.onStart(fixture.ownerMock) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub).startSession() } - @Ignore("for some reason this is flaky only on appveyor") @Test fun `if app goes to background, end session after interval`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 100L, true, false) - watcher.onStart(mock()) - watcher.onStop(mock()) - Thread.sleep(500L) - verify(hub).endSession() + val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + watcher.onStop(fixture.ownerMock) + await.untilFalse(watcher.isRunningSession) + verify(fixture.hub).endSession() } @Test fun `if app goes to background and foreground again, dont end the session`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 1000L, true, false) - watcher.onStart(mock()) - watcher.onStop(mock()) - Thread.sleep(150) - watcher.onStart(mock()) - verify(hub, never()).endSession() + val watcher = fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + + watcher.onStop(fixture.ownerMock) + assertNotNull(watcher.timerTask) + + watcher.onStart(fixture.ownerMock) + assertNull(watcher.timerTask) + + verify(fixture.hub, never()).endSession() } @Test fun `When session tracking is disabled, do not start session`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 1000L, false, false) - watcher.onStart(mock()) - verify(hub, never()).startSession() + val watcher = fixture.getSUT(enableSessionTracking = false, enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub, never()).startSession() } @Test fun `When session tracking is disabled, do not end session`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, false) - watcher.onStart(mock()) - verify(hub, never()).endSession() + val watcher = fixture.getSUT(enableSessionTracking = false, enableAppLifecycleBreadcrumbs = false) + watcher.onStop(fixture.ownerMock) + assertNull(watcher.timerTask) + verify(fixture.hub, never()).endSession() } @Test fun `When session tracking is enabled, add breadcrumb on start`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, true, false) - watcher.onStart(mock()) - Thread.sleep(150) - verify(hub).addBreadcrumb(check { + val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub).addBreadcrumb(check { assertEquals("app.lifecycle", it.category) assertEquals("session", it.type) assertEquals(SentryLevel.INFO, it.level) @@ -97,11 +109,11 @@ class LifecycleWatcherTest { @Test fun `When session tracking is enabled, add breadcrumb on stop`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, true, false) - watcher.onStop(mock()) - Thread.sleep(150) - verify(hub).addBreadcrumb(check { + val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) + watcher.isRunningSession.set(true) + watcher.onStop(fixture.ownerMock) + await.untilFalse(watcher.isRunningSession) + verify(fixture.hub).addBreadcrumb(check { assertEquals("app.lifecycle", it.category) assertEquals("session", it.type) assertEquals(SentryLevel.INFO, it.level) @@ -111,26 +123,24 @@ class LifecycleWatcherTest { @Test fun `When session tracking is disabled, do not add breadcrumb on start`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, false) - watcher.onStart(mock()) - verify(hub, never()).addBreadcrumb(any()) + val watcher = fixture.getSUT(enableSessionTracking = false, enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub, never()).addBreadcrumb(any()) } @Test fun `When session tracking is disabled, do not add breadcrumb on stop`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, false) - watcher.onStop(mock()) - verify(hub, never()).addBreadcrumb(any()) + val watcher = fixture.getSUT(enableSessionTracking = false, enableAppLifecycleBreadcrumbs = false) + watcher.onStop(fixture.ownerMock) + assertNull(watcher.timerTask) + verify(fixture.hub, never()).addBreadcrumb(any()) } @Test fun `When app lifecycle breadcrumbs is enabled, add breadcrumb on start`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, true) - watcher.onStart(mock()) - verify(hub).addBreadcrumb(check { + val watcher = fixture.getSUT(enableSessionTracking = false) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub).addBreadcrumb(check { assertEquals("app.lifecycle", it.category) assertEquals("navigation", it.type) assertEquals(SentryLevel.INFO, it.level) @@ -140,18 +150,16 @@ class LifecycleWatcherTest { @Test fun `When app lifecycle breadcrumbs is disabled, do not add breadcrumb on start`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, false) - watcher.onStart(mock()) - verify(hub, never()).addBreadcrumb(any()) + val watcher = fixture.getSUT(enableSessionTracking = false, enableAppLifecycleBreadcrumbs = false) + watcher.onStart(fixture.ownerMock) + verify(fixture.hub, never()).addBreadcrumb(any()) } @Test fun `When app lifecycle breadcrumbs is enabled, add breadcrumb on stop`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, true) - watcher.onStop(mock()) - verify(hub).addBreadcrumb(check { + val watcher = fixture.getSUT(enableSessionTracking = false) + watcher.onStop(fixture.ownerMock) + verify(fixture.hub).addBreadcrumb(check { assertEquals("app.lifecycle", it.category) assertEquals("navigation", it.type) assertEquals(SentryLevel.INFO, it.level) @@ -161,9 +169,8 @@ class LifecycleWatcherTest { @Test fun `When app lifecycle breadcrumbs is disabled, do not add breadcrumb on stop`() { - val hub = mock() - val watcher = LifecycleWatcher(hub, 0L, false, false) - watcher.onStop(mock()) - verify(hub, never()).addBreadcrumb(any()) + val watcher = fixture.getSUT(enableSessionTracking = false, enableAppLifecycleBreadcrumbs = false) + watcher.onStop(fixture.ownerMock) + verify(fixture.hub, never()).addBreadcrumb(any()) } } diff --git a/sentry-core/src/main/java/io/sentry/core/transport/CurrentDateProvider.java b/sentry-core/src/main/java/io/sentry/core/transport/CurrentDateProvider.java index fe8feef0d..289b6e048 100644 --- a/sentry-core/src/main/java/io/sentry/core/transport/CurrentDateProvider.java +++ b/sentry-core/src/main/java/io/sentry/core/transport/CurrentDateProvider.java @@ -1,6 +1,17 @@ package io.sentry.core.transport; -final class CurrentDateProvider implements ICurrentDateProvider { +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class CurrentDateProvider implements ICurrentDateProvider { + + private static final ICurrentDateProvider instance = new CurrentDateProvider(); + + public static ICurrentDateProvider getInstance() { + return instance; + } + + private CurrentDateProvider() {} @Override public final long getCurrentTimeMillis() { diff --git a/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java b/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java index 4af74aa21..d509ef023 100644 --- a/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java +++ b/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java @@ -111,7 +111,7 @@ public HttpTransport( readTimeoutMillis, bypassSecurity, sentryUrl, - new CurrentDateProvider()); + CurrentDateProvider.getInstance()); } HttpTransport( diff --git a/sentry-core/src/main/java/io/sentry/core/transport/ICurrentDateProvider.java b/sentry-core/src/main/java/io/sentry/core/transport/ICurrentDateProvider.java index 902d2fad8..a4f681002 100644 --- a/sentry-core/src/main/java/io/sentry/core/transport/ICurrentDateProvider.java +++ b/sentry-core/src/main/java/io/sentry/core/transport/ICurrentDateProvider.java @@ -1,7 +1,10 @@ package io.sentry.core.transport; +import org.jetbrains.annotations.ApiStatus; + /** Date Provider to make the Transport unit testable */ -interface ICurrentDateProvider { +@ApiStatus.Internal +public interface ICurrentDateProvider { /** * Returns the current time in millis