From 94a2d51272dca479ac51038cfdb548e8c6b772fd Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 27 Dec 2023 11:46:23 -0800 Subject: [PATCH 01/19] feat: offline support Save events on offline and send out when back to online automatically --- android/src/main/AndroidManifest.xml | 1 + .../java/com/amplitude/android/Amplitude.kt | 44 +++++++-- .../AndroidNetworkConnectivityChecker.kt | 46 +++++++++ .../utilities/AndroidNetworkListener.kt | 97 +++++++++++++++++++ .../android/AmplitudeRobolectricTests.kt | 5 +- .../amplitude/android/AmplitudeSessionTest.kt | 4 + .../com/amplitude/android/AmplitudeTest.kt | 5 + .../plugins/AndroidLifecyclePluginTest.kt | 25 +++-- .../amplitude/core/platform/EventPipeline.kt | 14 ++- .../platform/plugins/AmplitudeDestination.kt | 6 +- 10 files changed, 225 insertions(+), 22 deletions(-) create mode 100644 android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt create mode 100644 android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt diff --git a/android/src/main/AndroidManifest.xml b/android/src/main/AndroidManifest.xml index b8490463..a4019a3b 100644 --- a/android/src/main/AndroidManifest.xml +++ b/android/src/main/AndroidManifest.xml @@ -2,4 +2,5 @@ + \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index 79a5ff74..dc5f8996 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -1,5 +1,6 @@ package com.amplitude.android +import AndroidNetworkListener import android.content.Context import com.amplitude.android.migration.ApiKeyStorageMigration import com.amplitude.android.migration.RemnantDataMigration @@ -7,6 +8,7 @@ import com.amplitude.android.plugins.AnalyticsConnectorIdentityPlugin import com.amplitude.android.plugins.AnalyticsConnectorPlugin import com.amplitude.android.plugins.AndroidContextPlugin import com.amplitude.android.plugins.AndroidLifecyclePlugin +import com.amplitude.android.utilities.AndroidNetworkConnectivityChecker import com.amplitude.core.Amplitude import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.plugins.AmplitudeDestination @@ -16,11 +18,21 @@ import com.amplitude.id.IdentityConfiguration import kotlinx.coroutines.launch open class Amplitude( - configuration: Configuration -) : Amplitude(configuration) { - + configuration: Configuration, +) : Amplitude(configuration), AndroidNetworkListener.NetworkChangeCallback { internal var inForeground = false private lateinit var androidContextPlugin: AndroidContextPlugin + private var networkListener: AndroidNetworkListener + private val networkChangeHandler = + object : AndroidNetworkListener.NetworkChangeCallback { + override fun onNetworkAvailable() { + flush() + } + + override fun onNetworkUnavailable() { + // Nothing to do so far + } + } val sessionId: Long get() { @@ -29,6 +41,9 @@ open class Amplitude( init { registerShutdownHook() + networkListener = AndroidNetworkListener((this.configuration as Configuration).context) + networkListener.setNetworkChangeCallback(networkChangeHandler) + networkListener.startListening() } override fun createTimeline(): Timeline { @@ -62,7 +77,7 @@ open class Amplitude( add(AndroidLifecyclePlugin()) add(AnalyticsConnectorIdentityPlugin()) add(AnalyticsConnectorPlugin()) - add(AmplitudeDestination()) + add(AmplitudeDestination(AndroidNetworkConnectivityChecker(this.configuration.context, this.logger))) (timeline as Timeline).start() } @@ -113,11 +128,14 @@ open class Amplitude( } private fun registerShutdownHook() { - Runtime.getRuntime().addShutdownHook(object : Thread() { - override fun run() { - (this@Amplitude.timeline as Timeline).stop() - } - }) + Runtime.getRuntime().addShutdownHook( + object : Thread() { + override fun run() { + (this@Amplitude.timeline as Timeline).stop() + (this@Amplitude.networkListener as AndroidNetworkListener).stopListening() + } + }, + ) } companion object { @@ -139,6 +157,14 @@ open class Amplitude( */ internal const val DUMMY_EXIT_FOREGROUND_EVENT = "dummy_exit_foreground" } + + override fun onNetworkAvailable() { + networkChangeHandler.onNetworkAvailable() + } + + override fun onNetworkUnavailable() { + networkChangeHandler.onNetworkUnavailable() + } } /** * constructor function to build amplitude in dsl format with config options diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt new file mode 100644 index 00000000..9caf41ce --- /dev/null +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt @@ -0,0 +1,46 @@ +package com.amplitude.android.utilities + +import android.annotation.SuppressLint +import android.content.Context +import android.content.pm.PackageManager +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import android.os.Build +import com.amplitude.common.Logger +import com.amplitude.core.platform.NetworkConnectivityChecker + +class AndroidNetworkConnectivityChecker(private val context: Context, private val logger: Logger) : NetworkConnectivityChecker { + companion object { + private const val ACCESS_NETWORK_STATE = "android.permission.ACCESS_NETWORK_STATE" + } + + override suspend fun isConnected(): Boolean { + // Assume connection and proceed. + // Events will be treated like online + // regardless network connectivity + if (!hasPermission(context, ACCESS_NETWORK_STATE)) { + logger.warn("No ACCESS_NETWORK_STATE permission, offline mode is not supported.") + return true + } + + val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + val network = cm.activeNetwork ?: return false + val capabilities = cm.getNetworkCapabilities(network) ?: return false + + return capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) || + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) + } else { + @SuppressLint("MissingPermission") + val networkInfo = cm.activeNetworkInfo + return networkInfo != null && networkInfo.isConnectedOrConnecting + } + } + + private fun hasPermission( + context: Context, + permission: String, + ): Boolean { + return context.checkCallingOrSelfPermission(permission) == PackageManager.PERMISSION_GRANTED + } +} diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt new file mode 100644 index 00000000..8bd16228 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt @@ -0,0 +1,97 @@ +import android.annotation.SuppressLint +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.net.NetworkRequest +import android.os.Build +import java.lang.IllegalArgumentException + +class AndroidNetworkListener(private val context: Context) { + private var networkCallback: NetworkChangeCallback? = null + private var networkCallbackForLowerApiLevels: BroadcastReceiver? = null + private var networkCallbackForHigherApiLevels: ConnectivityManager.NetworkCallback? = null + + interface NetworkChangeCallback { + fun onNetworkAvailable() + + fun onNetworkUnavailable() + } + + fun setNetworkChangeCallback(callback: NetworkChangeCallback) { + this.networkCallback = callback + } + + fun startListening() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + setupNetworkCallback() + } else { + setupBroadcastReceiver() + } + } + + @SuppressLint("NewApi") + private fun setupNetworkCallback() { + val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + val networkRequest = + NetworkRequest.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .build() + + networkCallbackForHigherApiLevels = + object : ConnectivityManager.NetworkCallback() { + override fun onAvailable(network: Network) { + networkCallback?.onNetworkAvailable() + } + + override fun onLost(network: Network) { + networkCallback?.onNetworkUnavailable() + } + } + + connectivityManager.registerNetworkCallback(networkRequest, networkCallbackForHigherApiLevels!!) + } + + private fun setupBroadcastReceiver() { + networkCallbackForLowerApiLevels = + object : BroadcastReceiver() { + override fun onReceive( + context: Context, + intent: Intent, + ) { + if (ConnectivityManager.CONNECTIVITY_ACTION == intent.action) { + val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + val activeNetwork = connectivityManager.activeNetworkInfo + val isConnected = activeNetwork?.isConnectedOrConnecting == true + + if (isConnected) { + networkCallback?.onNetworkAvailable() + } else { + networkCallback?.onNetworkUnavailable() + } + } + } + } + + val filter = IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION) + context.registerReceiver(networkCallbackForLowerApiLevels, filter) + } + + fun stopListening() { + try { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + networkCallbackForHigherApiLevels?.let { connectivityManager.unregisterNetworkCallback(it) } + } else { + networkCallbackForLowerApiLevels?.let { context.unregisterReceiver(it) } + } + } catch (e: IllegalArgumentException) { + // callback was already unregistered. + } catch (e: IllegalStateException) { + // shutdown process is in progress and certain operations are not allowed. + } + } +} diff --git a/android/src/test/java/com/amplitude/android/AmplitudeRobolectricTests.kt b/android/src/test/java/com/amplitude/android/AmplitudeRobolectricTests.kt index aa9cdce6..7cd43a07 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeRobolectricTests.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeRobolectricTests.kt @@ -2,6 +2,7 @@ package com.amplitude.android import android.app.Application import android.content.Context +import android.net.ConnectivityManager import com.amplitude.core.events.BaseEvent import com.amplitude.core.utilities.ConsoleLoggerProvider import com.amplitude.id.IMIdentityStorageProvider @@ -23,6 +24,7 @@ import kotlin.io.path.absolutePathString class AmplitudeRobolectricTests { private lateinit var amplitude: Amplitude private var context: Context? = null + private lateinit var connectivityManager: ConnectivityManager var tempDir = TempDirectory() @@ -30,8 +32,9 @@ class AmplitudeRobolectricTests { @Before fun setup() { context = mockk(relaxed = true) + connectivityManager = mockk(relaxed = true) every { context!!.getDir(any(), any()) } returns File(tempDir.create("data").absolutePathString()) - + every { context!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager amplitude = Amplitude(createConfiguration()) } diff --git a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt index 7aded7e6..9e066ccf 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt @@ -1,6 +1,8 @@ package com.amplitude.android import android.app.Application +import android.content.Context +import android.net.ConnectivityManager import com.amplitude.android.plugins.AndroidLifecyclePlugin import com.amplitude.common.android.AndroidContextProvider import com.amplitude.core.Storage @@ -64,6 +66,8 @@ class AmplitudeSessionTest { private fun createConfiguration(storageProvider: StorageProvider? = null): Configuration { val context = mockk(relaxed = true) + var connectivityManager = mockk(relaxed = true) + every { context!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager return Configuration( apiKey = "api-key", diff --git a/android/src/test/java/com/amplitude/android/AmplitudeTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeTest.kt index 3faf6fcb..4e3120b3 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeTest.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeTest.kt @@ -2,6 +2,7 @@ package com.amplitude.android import android.app.Application import android.content.Context +import android.net.ConnectivityManager import com.amplitude.analytics.connector.AnalyticsConnector import com.amplitude.analytics.connector.Identity import com.amplitude.android.plugins.AndroidLifecyclePlugin @@ -38,10 +39,14 @@ open class StubPlugin : EventPlugin { class AmplitudeTest { private var context: Context? = null private var amplitude: Amplitude? = null + private lateinit var connectivityManager: ConnectivityManager @BeforeEach fun setUp() { context = mockk(relaxed = true) + connectivityManager = mockk(relaxed = true) + every { context!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + mockkStatic(AndroidLifecyclePlugin::class) mockkConstructor(AndroidContextProvider::class) diff --git a/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt b/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt index c30999d0..9f41fb1c 100644 --- a/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt +++ b/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt @@ -2,10 +2,12 @@ package com.amplitude.android.plugins import android.app.Activity import android.app.Application +import android.content.Context import android.content.Intent import android.content.pm.ActivityInfo import android.content.pm.PackageInfo import android.content.pm.PackageManager +import android.net.ConnectivityManager import android.net.Uri import android.os.Bundle import com.amplitude.android.Amplitude @@ -44,6 +46,7 @@ class AndroidLifecyclePluginTest { private val mockedContext = mockk(relaxed = true) private var mockedPackageManager: PackageManager + private lateinit var connectivityManager: ConnectivityManager init { val packageInfo = PackageInfo() @@ -82,15 +85,19 @@ class AndroidLifecyclePluginTest { every { anyConstructed().mostRecentLocation } returns null every { anyConstructed().appSetId } returns "" - configuration = Configuration( - apiKey = "api-key", - context = mockedContext, - storageProvider = InMemoryStorageProvider(), - loggerProvider = ConsoleLoggerProvider(), - identifyInterceptStorageProvider = InMemoryStorageProvider(), - identityStorageProvider = IMIdentityStorageProvider(), - trackingSessionEvents = false, - ) + connectivityManager = mockk(relaxed = true) + every { mockedContext!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + + configuration = + Configuration( + apiKey = "api-key", + context = mockedContext, + storageProvider = InMemoryStorageProvider(), + loggerProvider = ConsoleLoggerProvider(), + identifyInterceptStorageProvider = InMemoryStorageProvider(), + identityStorageProvider = IMIdentityStorageProvider(), + trackingSessionEvents = false, + ) amplitude = Amplitude(configuration) } diff --git a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt index 71d948d6..274727ac 100644 --- a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt +++ b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt @@ -14,8 +14,13 @@ import kotlinx.coroutines.withContext import java.io.FileNotFoundException import java.util.concurrent.atomic.AtomicInteger +interface NetworkConnectivityChecker { + suspend fun isConnected(): Boolean +} + class EventPipeline( - private val amplitude: Amplitude + private val amplitude: Amplitude, + private val networkConnectivityChecker: NetworkConnectivityChecker? = null ) { private val writeChannel: Channel @@ -99,6 +104,13 @@ class EventPipeline( } } + // Skip flush when offline only if + // network connectivity is not null + // and network is not connected. + if (networkConnectivityChecker?.isConnected() == false) { + continue + } + // if flush condition met, generate paths if (eventCount.incrementAndGet() >= getFlushCount() || triggerFlush) { eventCount.set(0) diff --git a/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt b/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt index b4c5d251..d2a6f186 100644 --- a/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt +++ b/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt @@ -7,10 +7,11 @@ import com.amplitude.core.events.IdentifyEvent import com.amplitude.core.events.RevenueEvent import com.amplitude.core.platform.DestinationPlugin import com.amplitude.core.platform.EventPipeline +import com.amplitude.core.platform.NetworkConnectivityChecker import com.amplitude.core.platform.intercept.IdentifyInterceptor import kotlinx.coroutines.launch -class AmplitudeDestination : DestinationPlugin() { +class AmplitudeDestination(private val networkConnectivityChecker: NetworkConnectivityChecker? = null) : DestinationPlugin() { private lateinit var pipeline: EventPipeline private lateinit var identifyInterceptor: IdentifyInterceptor @@ -66,7 +67,8 @@ class AmplitudeDestination : DestinationPlugin() { with(amplitude) { pipeline = EventPipeline( - amplitude + amplitude, + networkConnectivityChecker ) pipeline.start() identifyInterceptor = IdentifyInterceptor( From 6d0680d098ac4f6d257c46fb76c335913e56be81 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 2 Jan 2024 10:23:50 -0800 Subject: [PATCH 02/19] remove network permission --- android/src/main/AndroidManifest.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/android/src/main/AndroidManifest.xml b/android/src/main/AndroidManifest.xml index a4019a3b..b8490463 100644 --- a/android/src/main/AndroidManifest.xml +++ b/android/src/main/AndroidManifest.xml @@ -2,5 +2,4 @@ - \ No newline at end of file From 70e5ff4ce0766363b78bc1733615cfde9f6a79b6 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 2 Jan 2024 14:24:17 -0800 Subject: [PATCH 03/19] remove NetworkChangeCallback interface for Amplitude --- .../src/main/java/com/amplitude/android/Amplitude.kt | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index dc5f8996..40555dfb 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -19,7 +19,7 @@ import kotlinx.coroutines.launch open class Amplitude( configuration: Configuration, -) : Amplitude(configuration), AndroidNetworkListener.NetworkChangeCallback { +) : Amplitude(configuration){ internal var inForeground = false private lateinit var androidContextPlugin: AndroidContextPlugin private var networkListener: AndroidNetworkListener @@ -157,14 +157,6 @@ open class Amplitude( */ internal const val DUMMY_EXIT_FOREGROUND_EVENT = "dummy_exit_foreground" } - - override fun onNetworkAvailable() { - networkChangeHandler.onNetworkAvailable() - } - - override fun onNetworkUnavailable() { - networkChangeHandler.onNetworkUnavailable() - } } /** * constructor function to build amplitude in dsl format with config options From 92d3d7d69651b93e9e41a8cc5f515a826956be09 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 2 Jan 2024 14:26:55 -0800 Subject: [PATCH 04/19] Suppress missing permission lint --- android/src/main/java/com/amplitude/android/Amplitude.kt | 2 +- .../android/utilities/AndroidNetworkConnectivityChecker.kt | 1 + .../amplitude/android/utilities/AndroidNetworkListener.kt | 5 ++++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index 40555dfb..417c43c2 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -19,7 +19,7 @@ import kotlinx.coroutines.launch open class Amplitude( configuration: Configuration, -) : Amplitude(configuration){ +) : Amplitude(configuration) { internal var inForeground = false private lateinit var androidContextPlugin: AndroidContextPlugin private var networkListener: AndroidNetworkListener diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt index 9caf41ce..8b644dd8 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt @@ -14,6 +14,7 @@ class AndroidNetworkConnectivityChecker(private val context: Context, private va private const val ACCESS_NETWORK_STATE = "android.permission.ACCESS_NETWORK_STATE" } + @SuppressLint("MissingPermission") override suspend fun isConnected(): Boolean { // Assume connection and proceed. // Events will be treated like online diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt index 8bd16228..4d86294a 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt @@ -33,7 +33,9 @@ class AndroidNetworkListener(private val context: Context) { } } - @SuppressLint("NewApi") + @SuppressLint("NewApi", "MissingPermission") + // startListening() checks API level + // ACCESS_NETWORK_STATE permission should be added manually by users to enable this feature private fun setupNetworkCallback() { val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager val networkRequest = @@ -58,6 +60,7 @@ class AndroidNetworkListener(private val context: Context) { private fun setupBroadcastReceiver() { networkCallbackForLowerApiLevels = object : BroadcastReceiver() { + @SuppressLint("MissingPermission") override fun onReceive( context: Context, intent: Intent, From 564f3083d29e9a03234aa461e3c9ec4af5566e21 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 2 Jan 2024 14:51:33 -0800 Subject: [PATCH 05/19] Revert "Delete EventPipelineTest.kt" This reverts commit db0dace725a8637f6aeedd4cf0e653bbb3ed2101. --- .../core/platform/EventPipelineTest.kt | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt diff --git a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt new file mode 100644 index 00000000..66131c8f --- /dev/null +++ b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt @@ -0,0 +1,84 @@ +package com.amplitude.core.platform + +import com.amplitude.core.Amplitude +import com.amplitude.core.Configuration +import com.amplitude.core.events.BaseEvent +import com.amplitude.core.utilities.ConsoleLoggerProvider +import com.amplitude.core.utilities.InMemoryStorageProvider +import com.amplitude.id.IMIdentityStorageProvider +import io.mockk.coEvery +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +@ExperimentalCoroutinesApi +class EventPipelineTest { + private lateinit var amplitude: Amplitude + private lateinit var networkConnectivityChecker: NetworkConnectivityChecker + private val config = Configuration( + apiKey = "API_KEY", + flushIntervalMillis = 5, + storageProvider = InMemoryStorageProvider(), + loggerProvider = ConsoleLoggerProvider(), + identifyInterceptStorageProvider = InMemoryStorageProvider(), + identityStorageProvider = IMIdentityStorageProvider() + ) + + @BeforeEach + fun setup() { + amplitude = Amplitude(config) + networkConnectivityChecker = mockk(relaxed = true) + } + + @Test + fun `should not flush when put and offline`() = + runBlocking { + coEvery { networkConnectivityChecker.isConnected() } returns false + val eventPipeline = spyk(EventPipeline(amplitude, networkConnectivityChecker)) + val event = BaseEvent().apply { eventType = "test_event" } + + eventPipeline.start() + eventPipeline.put(event) + delay(6) + + verify(exactly = 0) { eventPipeline.flush() } + } + + @Test + fun `should flush when put and online`() = + runBlocking { + coEvery { networkConnectivityChecker.isConnected() } returns true + val eventPipeline = spyk(EventPipeline(amplitude, networkConnectivityChecker)) + val event = BaseEvent().apply { eventType = "test_event" } + + eventPipeline.start() + eventPipeline.put(event) + delay(6) + + verify(exactly = 1) { eventPipeline.flush() } + } + + @Test + fun `should flush when back to online and an event is tracked`() = + runBlocking { + coEvery { networkConnectivityChecker.isConnected() } returns false + val eventPipeline = spyk(EventPipeline(amplitude, networkConnectivityChecker)) + val event1 = BaseEvent().apply { eventType = "test_event1" } + val event2 = BaseEvent().apply { eventType = "test_event2" } + + eventPipeline.start() + eventPipeline.put(event1) + delay(6) + + coEvery { networkConnectivityChecker.isConnected() } returns true + eventPipeline.put(event2) + delay(6) + + verify(exactly = 1) { eventPipeline.flush() } + } +} From acfab11e948e908571ff40c31f4b9d3e79cc562a Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 3 Jan 2024 09:49:48 -0800 Subject: [PATCH 06/19] revert lint changes --- .../java/com/amplitude/android/Amplitude.kt | 16 +++++++--------- .../plugins/AndroidLifecyclePluginTest.kt | 19 +++++++++---------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index 417c43c2..e33403f8 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -18,7 +18,7 @@ import com.amplitude.id.IdentityConfiguration import kotlinx.coroutines.launch open class Amplitude( - configuration: Configuration, + configuration: Configuration ) : Amplitude(configuration) { internal var inForeground = false private lateinit var androidContextPlugin: AndroidContextPlugin @@ -128,14 +128,12 @@ open class Amplitude( } private fun registerShutdownHook() { - Runtime.getRuntime().addShutdownHook( - object : Thread() { - override fun run() { - (this@Amplitude.timeline as Timeline).stop() - (this@Amplitude.networkListener as AndroidNetworkListener).stopListening() - } - }, - ) + Runtime.getRuntime().addShutdownHook(object : Thread() { + override fun run() { + (this@Amplitude.timeline as Timeline).stop() + this@Amplitude.networkListener.stopListening() + } + }) } companion object { diff --git a/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt b/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt index 9f41fb1c..4cdb4e19 100644 --- a/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt +++ b/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt @@ -88,16 +88,15 @@ class AndroidLifecyclePluginTest { connectivityManager = mockk(relaxed = true) every { mockedContext!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager - configuration = - Configuration( - apiKey = "api-key", - context = mockedContext, - storageProvider = InMemoryStorageProvider(), - loggerProvider = ConsoleLoggerProvider(), - identifyInterceptStorageProvider = InMemoryStorageProvider(), - identityStorageProvider = IMIdentityStorageProvider(), - trackingSessionEvents = false, - ) + configuration = Configuration( + apiKey = "api-key", + context = mockedContext, + storageProvider = InMemoryStorageProvider(), + loggerProvider = ConsoleLoggerProvider(), + identifyInterceptStorageProvider = InMemoryStorageProvider(), + identityStorageProvider = IMIdentityStorageProvider(), + trackingSessionEvents = false, + ) amplitude = Amplitude(configuration) } From 7e95fd0499fd3e834b0ec86758a632478acaf753 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 3 Jan 2024 09:56:16 -0800 Subject: [PATCH 07/19] add docs link to add permission --- .../android/utilities/AndroidNetworkConnectivityChecker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt index 8b644dd8..e5ec8bce 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt @@ -20,7 +20,7 @@ class AndroidNetworkConnectivityChecker(private val context: Context, private va // Events will be treated like online // regardless network connectivity if (!hasPermission(context, ACCESS_NETWORK_STATE)) { - logger.warn("No ACCESS_NETWORK_STATE permission, offline mode is not supported.") + logger.warn("No ACCESS_NETWORK_STATE permission, offline mode is not supported. To enable, add to your AndroidManifest.xml. Learn more at https://www.docs.developers.amplitude.com/data/sdks/android-kotlin/#offline-mode") return true } From bcd0721544028a5036da43f867ca5a8d096974cf Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 3 Jan 2024 13:27:44 -0800 Subject: [PATCH 08/19] add android network checker plugin --- .../java/com/amplitude/android/Amplitude.kt | 9 ++- ...AndroidNetworkConnectivityCheckerPlugin.kt | 26 ++++++++ .../AndroidNetworkConnectivityChecker.kt | 10 ++-- .../java/com/amplitude/core/Configuration.kt | 1 + .../amplitude/core/platform/EventPipeline.kt | 13 +--- .../platform/plugins/AmplitudeDestination.kt | 6 +- .../core/platform/EventPipelineTest.kt | 59 ++++++++----------- 7 files changed, 67 insertions(+), 57 deletions(-) create mode 100644 android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index e33403f8..adcf842d 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -8,7 +8,7 @@ import com.amplitude.android.plugins.AnalyticsConnectorIdentityPlugin import com.amplitude.android.plugins.AnalyticsConnectorPlugin import com.amplitude.android.plugins.AndroidContextPlugin import com.amplitude.android.plugins.AndroidLifecyclePlugin -import com.amplitude.android.utilities.AndroidNetworkConnectivityChecker +import com.amplitude.android.plugins.AndroidNetworkConnectivityCheckerPlugin import com.amplitude.core.Amplitude import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.plugins.AmplitudeDestination @@ -20,17 +20,19 @@ import kotlinx.coroutines.launch open class Amplitude( configuration: Configuration ) : Amplitude(configuration) { + internal var inForeground = false private lateinit var androidContextPlugin: AndroidContextPlugin private var networkListener: AndroidNetworkListener private val networkChangeHandler = object : AndroidNetworkListener.NetworkChangeCallback { override fun onNetworkAvailable() { + configuration.isNetworkConnected = true flush() } override fun onNetworkUnavailable() { - // Nothing to do so far + configuration.isNetworkConnected = false } } @@ -71,13 +73,14 @@ open class Amplitude( } this.createIdentityContainer(identityConfiguration) + add(AndroidNetworkConnectivityCheckerPlugin()) androidContextPlugin = AndroidContextPlugin() add(androidContextPlugin) add(GetAmpliExtrasPlugin()) add(AndroidLifecyclePlugin()) add(AnalyticsConnectorIdentityPlugin()) add(AnalyticsConnectorPlugin()) - add(AmplitudeDestination(AndroidNetworkConnectivityChecker(this.configuration.context, this.logger))) + add(AmplitudeDestination()) (timeline as Timeline).start() } diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt new file mode 100644 index 00000000..e6f415c7 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -0,0 +1,26 @@ +package com.amplitude.android.plugins + +import com.amplitude.android.Configuration +import com.amplitude.android.utilities.AndroidNetworkConnectivityChecker +import com.amplitude.core.Amplitude +import com.amplitude.core.events.BaseEvent +import com.amplitude.core.platform.Plugin +import kotlinx.coroutines.launch + +class AndroidNetworkConnectivityCheckerPlugin : Plugin { + override val type: Plugin.Type = Plugin.Type.Before + override lateinit var amplitude: Amplitude + private lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker + + override fun setup(amplitude: Amplitude) { + super.setup(amplitude) + networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) + } + + override fun execute(event: BaseEvent): BaseEvent? { + amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { + amplitude.configuration.isNetworkConnected = networkConnectivityChecker.isConnected() + } + return super.execute(event) + } +} diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt index e5ec8bce..dbcc8963 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt @@ -7,20 +7,22 @@ import android.net.ConnectivityManager import android.net.NetworkCapabilities import android.os.Build import com.amplitude.common.Logger -import com.amplitude.core.platform.NetworkConnectivityChecker -class AndroidNetworkConnectivityChecker(private val context: Context, private val logger: Logger) : NetworkConnectivityChecker { +class AndroidNetworkConnectivityChecker(private val context: Context, private val logger: Logger) { companion object { private const val ACCESS_NETWORK_STATE = "android.permission.ACCESS_NETWORK_STATE" } @SuppressLint("MissingPermission") - override suspend fun isConnected(): Boolean { + fun isConnected(): Boolean { // Assume connection and proceed. // Events will be treated like online // regardless network connectivity if (!hasPermission(context, ACCESS_NETWORK_STATE)) { - logger.warn("No ACCESS_NETWORK_STATE permission, offline mode is not supported. To enable, add to your AndroidManifest.xml. Learn more at https://www.docs.developers.amplitude.com/data/sdks/android-kotlin/#offline-mode") + logger.warn( + @Suppress("ktlint:standard:max-line-length") + "No ACCESS_NETWORK_STATE permission, offline mode is not supported. To enable, add to your AndroidManifest.xml. Learn more at https://www.docs.developers.amplitude.com/data/sdks/android-kotlin/#offline-mode", + ) return true } diff --git a/core/src/main/java/com/amplitude/core/Configuration.kt b/core/src/main/java/com/amplitude/core/Configuration.kt index 0fa82754..2b0fe501 100644 --- a/core/src/main/java/com/amplitude/core/Configuration.kt +++ b/core/src/main/java/com/amplitude/core/Configuration.kt @@ -30,6 +30,7 @@ open class Configuration @JvmOverloads constructor( open var identifyBatchIntervalMillis: Long = IDENTIFY_BATCH_INTERVAL_MILLIS, open var identifyInterceptStorageProvider: StorageProvider = InMemoryStorageProvider(), open var identityStorageProvider: IdentityStorageProvider = IMIdentityStorageProvider(), + var isNetworkConnected: Boolean = true, ) { companion object { diff --git a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt index 274727ac..c287083f 100644 --- a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt +++ b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt @@ -14,13 +14,8 @@ import kotlinx.coroutines.withContext import java.io.FileNotFoundException import java.util.concurrent.atomic.AtomicInteger -interface NetworkConnectivityChecker { - suspend fun isConnected(): Boolean -} - class EventPipeline( - private val amplitude: Amplitude, - private val networkConnectivityChecker: NetworkConnectivityChecker? = null + private val amplitude: Amplitude ) { private val writeChannel: Channel @@ -104,10 +99,8 @@ class EventPipeline( } } - // Skip flush when offline only if - // network connectivity is not null - // and network is not connected. - if (networkConnectivityChecker?.isConnected() == false) { + // Skip flush when offline + if (!amplitude.configuration.isNetworkConnected) { continue } diff --git a/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt b/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt index d2a6f186..b4c5d251 100644 --- a/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt +++ b/core/src/main/java/com/amplitude/core/platform/plugins/AmplitudeDestination.kt @@ -7,11 +7,10 @@ import com.amplitude.core.events.IdentifyEvent import com.amplitude.core.events.RevenueEvent import com.amplitude.core.platform.DestinationPlugin import com.amplitude.core.platform.EventPipeline -import com.amplitude.core.platform.NetworkConnectivityChecker import com.amplitude.core.platform.intercept.IdentifyInterceptor import kotlinx.coroutines.launch -class AmplitudeDestination(private val networkConnectivityChecker: NetworkConnectivityChecker? = null) : DestinationPlugin() { +class AmplitudeDestination : DestinationPlugin() { private lateinit var pipeline: EventPipeline private lateinit var identifyInterceptor: IdentifyInterceptor @@ -67,8 +66,7 @@ class AmplitudeDestination(private val networkConnectivityChecker: NetworkConnec with(amplitude) { pipeline = EventPipeline( - amplitude, - networkConnectivityChecker + amplitude ) pipeline.start() identifyInterceptor = IdentifyInterceptor( diff --git a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt index 66131c8f..891901bf 100644 --- a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt +++ b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt @@ -2,82 +2,69 @@ package com.amplitude.core.platform import com.amplitude.core.Amplitude import com.amplitude.core.Configuration +import com.amplitude.core.State import com.amplitude.core.events.BaseEvent import com.amplitude.core.utilities.ConsoleLoggerProvider import com.amplitude.core.utilities.InMemoryStorageProvider import com.amplitude.id.IMIdentityStorageProvider -import io.mockk.coEvery -import io.mockk.mockk import io.mockk.spyk import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.delay -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @ExperimentalCoroutinesApi class EventPipelineTest { private lateinit var amplitude: Amplitude - private lateinit var networkConnectivityChecker: NetworkConnectivityChecker + private lateinit var testScope: TestScope + private lateinit var testDispatcher: TestDispatcher private val config = Configuration( apiKey = "API_KEY", - flushIntervalMillis = 5, + flushIntervalMillis = 1, storageProvider = InMemoryStorageProvider(), loggerProvider = ConsoleLoggerProvider(), identifyInterceptStorageProvider = InMemoryStorageProvider(), - identityStorageProvider = IMIdentityStorageProvider() + identityStorageProvider = IMIdentityStorageProvider(), ) @BeforeEach fun setup() { - amplitude = Amplitude(config) - networkConnectivityChecker = mockk(relaxed = true) + testDispatcher = StandardTestDispatcher() + testScope = TestScope(testDispatcher) + amplitude = Amplitude(config, State(), testScope, testDispatcher, testDispatcher, testDispatcher, testDispatcher) } @Test fun `should not flush when put and offline`() = - runBlocking { - coEvery { networkConnectivityChecker.isConnected() } returns false - val eventPipeline = spyk(EventPipeline(amplitude, networkConnectivityChecker)) + runTest(testDispatcher) { + amplitude.isBuilt.await() + amplitude.configuration.isNetworkConnected = false + val eventPipeline = spyk(EventPipeline(amplitude)) val event = BaseEvent().apply { eventType = "test_event" } eventPipeline.start() eventPipeline.put(event) - delay(6) + advanceUntilIdle() verify(exactly = 0) { eventPipeline.flush() } } @Test fun `should flush when put and online`() = - runBlocking { - coEvery { networkConnectivityChecker.isConnected() } returns true - val eventPipeline = spyk(EventPipeline(amplitude, networkConnectivityChecker)) + runTest(testDispatcher) { + amplitude.isBuilt.await() + amplitude.configuration.isNetworkConnected = true + val eventPipeline = spyk(EventPipeline(amplitude)) val event = BaseEvent().apply { eventType = "test_event" } eventPipeline.start() eventPipeline.put(event) - delay(6) - - verify(exactly = 1) { eventPipeline.flush() } - } - - @Test - fun `should flush when back to online and an event is tracked`() = - runBlocking { - coEvery { networkConnectivityChecker.isConnected() } returns false - val eventPipeline = spyk(EventPipeline(amplitude, networkConnectivityChecker)) - val event1 = BaseEvent().apply { eventType = "test_event1" } - val event2 = BaseEvent().apply { eventType = "test_event2" } - - eventPipeline.start() - eventPipeline.put(event1) - delay(6) - - coEvery { networkConnectivityChecker.isConnected() } returns true - eventPipeline.put(event2) - delay(6) + advanceUntilIdle() verify(exactly = 1) { eventPipeline.flush() } } From ea4753e9611e9608ba90548a5dbbb1f04511ddd2 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 3 Jan 2024 21:48:07 -0800 Subject: [PATCH 09/19] Update Configuration.kt --- core/src/main/java/com/amplitude/core/Configuration.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/amplitude/core/Configuration.kt b/core/src/main/java/com/amplitude/core/Configuration.kt index 2b0fe501..85412729 100644 --- a/core/src/main/java/com/amplitude/core/Configuration.kt +++ b/core/src/main/java/com/amplitude/core/Configuration.kt @@ -30,9 +30,10 @@ open class Configuration @JvmOverloads constructor( open var identifyBatchIntervalMillis: Long = IDENTIFY_BATCH_INTERVAL_MILLIS, open var identifyInterceptStorageProvider: StorageProvider = InMemoryStorageProvider(), open var identityStorageProvider: IdentityStorageProvider = IMIdentityStorageProvider(), - var isNetworkConnected: Boolean = true, ) { + var isNetworkConnected: Boolean = true + companion object { const val FLUSH_QUEUE_SIZE = 30 const val FLUSH_INTERVAL_MILLIS = 30 * 1000 // 30s From 9c5702f3821e3e10450a5716aed86426bff0c0f9 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 9 Jan 2024 12:54:36 -0800 Subject: [PATCH 10/19] move network listener to plugin and rename to offline --- .../java/com/amplitude/android/Amplitude.kt | 17 ---------------- ...AndroidNetworkConnectivityCheckerPlugin.kt | 20 ++++++++++++++++++- .../java/com/amplitude/core/Configuration.kt | 2 +- .../amplitude/core/platform/EventPipeline.kt | 2 +- .../core/platform/EventPipelineTest.kt | 4 ++-- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index adcf842d..1f7dd1d5 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -1,6 +1,5 @@ package com.amplitude.android -import AndroidNetworkListener import android.content.Context import com.amplitude.android.migration.ApiKeyStorageMigration import com.amplitude.android.migration.RemnantDataMigration @@ -23,18 +22,6 @@ open class Amplitude( internal var inForeground = false private lateinit var androidContextPlugin: AndroidContextPlugin - private var networkListener: AndroidNetworkListener - private val networkChangeHandler = - object : AndroidNetworkListener.NetworkChangeCallback { - override fun onNetworkAvailable() { - configuration.isNetworkConnected = true - flush() - } - - override fun onNetworkUnavailable() { - configuration.isNetworkConnected = false - } - } val sessionId: Long get() { @@ -43,9 +30,6 @@ open class Amplitude( init { registerShutdownHook() - networkListener = AndroidNetworkListener((this.configuration as Configuration).context) - networkListener.setNetworkChangeCallback(networkChangeHandler) - networkListener.startListening() } override fun createTimeline(): Timeline { @@ -134,7 +118,6 @@ open class Amplitude( Runtime.getRuntime().addShutdownHook(object : Thread() { override fun run() { (this@Amplitude.timeline as Timeline).stop() - this@Amplitude.networkListener.stopListening() } }) } diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt index e6f415c7..5fae28aa 100644 --- a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -1,5 +1,6 @@ package com.amplitude.android.plugins +import AndroidNetworkListener import com.amplitude.android.Configuration import com.amplitude.android.utilities.AndroidNetworkConnectivityChecker import com.amplitude.core.Amplitude @@ -11,15 +12,32 @@ class AndroidNetworkConnectivityCheckerPlugin : Plugin { override val type: Plugin.Type = Plugin.Type.Before override lateinit var amplitude: Amplitude private lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker + private lateinit var networkListener: AndroidNetworkListener + private val networkChangeHandler = + object : AndroidNetworkListener.NetworkChangeCallback { + override fun onNetworkAvailable() { + println("AndroidNetworkListener, onNetworkAvailable") + amplitude.configuration.offline = false + amplitude.flush() + } + + override fun onNetworkUnavailable() { + println("AndroidNetworkListener, onNetworkUnavailable") + amplitude.configuration.offline = true + } + } override fun setup(amplitude: Amplitude) { super.setup(amplitude) networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) + networkListener = AndroidNetworkListener((amplitude.configuration as Configuration).context) + networkListener.setNetworkChangeCallback(networkChangeHandler) + networkListener.startListening() } override fun execute(event: BaseEvent): BaseEvent? { amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { - amplitude.configuration.isNetworkConnected = networkConnectivityChecker.isConnected() + amplitude.configuration.offline = !networkConnectivityChecker.isConnected() } return super.execute(event) } diff --git a/core/src/main/java/com/amplitude/core/Configuration.kt b/core/src/main/java/com/amplitude/core/Configuration.kt index 85412729..5afbf9b7 100644 --- a/core/src/main/java/com/amplitude/core/Configuration.kt +++ b/core/src/main/java/com/amplitude/core/Configuration.kt @@ -32,7 +32,7 @@ open class Configuration @JvmOverloads constructor( open var identityStorageProvider: IdentityStorageProvider = IMIdentityStorageProvider(), ) { - var isNetworkConnected: Boolean = true + var offline: Boolean = false companion object { const val FLUSH_QUEUE_SIZE = 30 diff --git a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt index c287083f..17926fe7 100644 --- a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt +++ b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt @@ -100,7 +100,7 @@ class EventPipeline( } // Skip flush when offline - if (!amplitude.configuration.isNetworkConnected) { + if (amplitude.configuration.offline) { continue } diff --git a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt index 891901bf..8a7888e6 100644 --- a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt +++ b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt @@ -43,7 +43,7 @@ class EventPipelineTest { fun `should not flush when put and offline`() = runTest(testDispatcher) { amplitude.isBuilt.await() - amplitude.configuration.isNetworkConnected = false + amplitude.configuration.offline = true val eventPipeline = spyk(EventPipeline(amplitude)) val event = BaseEvent().apply { eventType = "test_event" } @@ -58,7 +58,7 @@ class EventPipelineTest { fun `should flush when put and online`() = runTest(testDispatcher) { amplitude.isBuilt.await() - amplitude.configuration.isNetworkConnected = true + amplitude.configuration.offline = false val eventPipeline = spyk(EventPipeline(amplitude)) val event = BaseEvent().apply { eventType = "test_event" } From 0acb081f3dc5b2b1580905ae5309e1e43815e0b3 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 10 Jan 2024 15:33:16 -0800 Subject: [PATCH 11/19] offline --- ...AndroidNetworkConnectivityCheckerPlugin.kt | 21 +++--- .../AndroidNetworkConnectivityChecker.kt | 44 ++++++++---- ...oidNetworkConnectivityCheckerPluginTest.kt | 56 +++++++++++++++ .../AndroidNetworkConnectivityCheckerTest.kt | 71 +++++++++++++++++++ .../com/amplitude/core/platform/Plugin.kt | 4 ++ .../com/amplitude/core/platform/Timeline.kt | 4 ++ .../android/sample/MainApplication.java | 5 ++ 7 files changed, 179 insertions(+), 26 deletions(-) create mode 100644 android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt create mode 100644 android/src/test/java/com/amplitude/android/utilities/AndroidNetworkConnectivityCheckerTest.kt diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt index 5fae28aa..3b31541b 100644 --- a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -4,25 +4,24 @@ import AndroidNetworkListener import com.amplitude.android.Configuration import com.amplitude.android.utilities.AndroidNetworkConnectivityChecker import com.amplitude.core.Amplitude -import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.Plugin import kotlinx.coroutines.launch class AndroidNetworkConnectivityCheckerPlugin : Plugin { override val type: Plugin.Type = Plugin.Type.Before override lateinit var amplitude: Amplitude - private lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker - private lateinit var networkListener: AndroidNetworkListener - private val networkChangeHandler = + internal lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker + internal lateinit var networkListener: AndroidNetworkListener + internal val networkChangeHandler = object : AndroidNetworkListener.NetworkChangeCallback { override fun onNetworkAvailable() { - println("AndroidNetworkListener, onNetworkAvailable") + amplitude.logger.debug("AndroidNetworkListener, onNetworkAvailable") amplitude.configuration.offline = false amplitude.flush() } override fun onNetworkUnavailable() { - println("AndroidNetworkListener, onNetworkUnavailable") + amplitude.logger.debug("AndroidNetworkListener, onNetworkUnavailable") amplitude.configuration.offline = true } } @@ -30,15 +29,15 @@ class AndroidNetworkConnectivityCheckerPlugin : Plugin { override fun setup(amplitude: Amplitude) { super.setup(amplitude) networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) + amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { + amplitude.configuration.offline = !networkConnectivityChecker.isConnected() + } networkListener = AndroidNetworkListener((amplitude.configuration as Configuration).context) networkListener.setNetworkChangeCallback(networkChangeHandler) networkListener.startListening() } - override fun execute(event: BaseEvent): BaseEvent? { - amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { - amplitude.configuration.offline = !networkConnectivityChecker.isConnected() - } - return super.execute(event) + override fun teardown() { + networkListener.stopListening() } } diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt index dbcc8963..275f2e42 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkConnectivityChecker.kt @@ -13,30 +13,44 @@ class AndroidNetworkConnectivityChecker(private val context: Context, private va private const val ACCESS_NETWORK_STATE = "android.permission.ACCESS_NETWORK_STATE" } - @SuppressLint("MissingPermission") - fun isConnected(): Boolean { - // Assume connection and proceed. - // Events will be treated like online - // regardless network connectivity - if (!hasPermission(context, ACCESS_NETWORK_STATE)) { + private val hasPermission: Boolean + internal var isMarshmallowAndAbove: Boolean = Build.VERSION.SDK_INT >= Build.VERSION_CODES.M + + init { + hasPermission = hasPermission(context, ACCESS_NETWORK_STATE) + if (!hasPermission) { logger.warn( @Suppress("ktlint:standard:max-line-length") "No ACCESS_NETWORK_STATE permission, offline mode is not supported. To enable, add to your AndroidManifest.xml. Learn more at https://www.docs.developers.amplitude.com/data/sdks/android-kotlin/#offline-mode", ) + } + } + + @SuppressLint("MissingPermission", "NewApi") + fun isConnected(): Boolean { + // Assume connection and proceed. + // Events will be treated like online + // regardless network connectivity + if (!hasPermission) { return true } - val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - val network = cm.activeNetwork ?: return false - val capabilities = cm.getNetworkCapabilities(network) ?: return false + val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) + if (cm is ConnectivityManager) { + if (isMarshmallowAndAbove) { + val network = cm.activeNetwork ?: return false + val capabilities = cm.getNetworkCapabilities(network) ?: return false - return capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) || - capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) + return capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) || + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) + } else { + @SuppressLint("MissingPermission") + val networkInfo = cm.activeNetworkInfo + return networkInfo != null && networkInfo.isConnectedOrConnecting + } } else { - @SuppressLint("MissingPermission") - val networkInfo = cm.activeNetworkInfo - return networkInfo != null && networkInfo.isConnectedOrConnecting + logger.debug("Service is not an instance of ConnectivityManager. Offline mode is not supported") + return true } } diff --git a/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt b/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt new file mode 100644 index 00000000..4e02f3eb --- /dev/null +++ b/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt @@ -0,0 +1,56 @@ +package com.amplitude.android.plugins + +import android.app.Application +import com.amplitude.android.Configuration +import com.amplitude.core.Amplitude +import com.amplitude.core.utilities.ConsoleLoggerProvider +import com.amplitude.core.utilities.InMemoryStorageProvider +import com.amplitude.id.IMIdentityStorageProvider +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import org.junit.Before +import org.junit.Test +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull + +class AndroidNetworkConnectivityCheckerPluginTest { + + private lateinit var amplitude: Amplitude + private lateinit var plugin: AndroidNetworkConnectivityCheckerPlugin + + private val context = mockk(relaxed = true) + + @Before + fun setup() { + amplitude = Amplitude( + Configuration( + apiKey = "api-key", + context = context, + storageProvider = InMemoryStorageProvider(), + loggerProvider = ConsoleLoggerProvider(), + identifyInterceptStorageProvider = InMemoryStorageProvider(), + identityStorageProvider = IMIdentityStorageProvider(), + trackingSessionEvents = false, + ) + ) + plugin = AndroidNetworkConnectivityCheckerPlugin() + } + + @Test + fun `should set up`() { + plugin.setup(amplitude) + assertEquals(amplitude, plugin.amplitude) + assertNotNull(plugin.networkConnectivityChecker) + // Unit tests are run on JVM so default to online + assertEquals(false, amplitude.configuration.offline) + } + + @Test + fun `should teardown correctly`() { + plugin.setup(amplitude) + spyk(plugin.networkListener) + plugin.teardown() + verify { plugin.networkListener.stopListening() } + } +} diff --git a/android/src/test/java/com/amplitude/android/utilities/AndroidNetworkConnectivityCheckerTest.kt b/android/src/test/java/com/amplitude/android/utilities/AndroidNetworkConnectivityCheckerTest.kt new file mode 100644 index 00000000..6f860d06 --- /dev/null +++ b/android/src/test/java/com/amplitude/android/utilities/AndroidNetworkConnectivityCheckerTest.kt @@ -0,0 +1,71 @@ +package com.amplitude.android.utilities + +import android.content.Context +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import android.net.NetworkInfo +import com.amplitude.common.Logger +import io.mockk.every +import io.mockk.mockk +import org.junit.Assert.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +class AndroidNetworkConnectivityCheckerTest { + + private lateinit var context: Context + private lateinit var connectivityManager: ConnectivityManager + private lateinit var networkCapabilities: NetworkCapabilities + private lateinit var networkInfo: NetworkInfo + private lateinit var logger: Logger + private lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker + + @BeforeEach + fun setUp() { + context = mockk() + connectivityManager = mockk() + networkCapabilities = mockk() + networkInfo = mockk() + logger = mockk(relaxed = true) + + every { context.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + every { context.checkCallingOrSelfPermission("android.permission.ACCESS_NETWORK_STATE") } returns 0 + networkConnectivityChecker = AndroidNetworkConnectivityChecker(context, logger) + } + + @Test + fun `should return true when connected to network on devices with API 23 and above`() { + every { connectivityManager.activeNetwork } returns mockk() + every { connectivityManager.getNetworkCapabilities(any()) } returns networkCapabilities + every { networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) } returns true + networkConnectivityChecker.isMarshmallowAndAbove = true + + assertTrue(networkConnectivityChecker.isConnected()) + } + + @Test + fun `should return false when not connected to network on devices with API 23 and above`() { + every { connectivityManager.activeNetwork } returns null + networkConnectivityChecker.isMarshmallowAndAbove = true + + assertFalse(networkConnectivityChecker.isConnected()) + } + + @Test + fun `should return true when connected to network devices with API lower than 23`() { + every { connectivityManager.activeNetworkInfo } returns networkInfo + every { networkInfo.isConnectedOrConnecting } returns true + networkConnectivityChecker.isMarshmallowAndAbove = false + + assertTrue(networkConnectivityChecker.isConnected()) + } + + @Test + fun `should return false when not connected to network devices with API lower than 23`() { + every { connectivityManager.activeNetworkInfo } returns null + networkConnectivityChecker.isMarshmallowAndAbove = false + + assertFalse(networkConnectivityChecker.isConnected()) + } +} diff --git a/core/src/main/java/com/amplitude/core/platform/Plugin.kt b/core/src/main/java/com/amplitude/core/platform/Plugin.kt index 2178103f..557cf2ac 100644 --- a/core/src/main/java/com/amplitude/core/platform/Plugin.kt +++ b/core/src/main/java/com/amplitude/core/platform/Plugin.kt @@ -25,6 +25,10 @@ interface Plugin { fun execute(event: BaseEvent): BaseEvent? { return event } + + fun teardown() { + // Clean up any resources from setup if necessary + } } interface EventPlugin : Plugin { diff --git a/core/src/main/java/com/amplitude/core/platform/Timeline.kt b/core/src/main/java/com/amplitude/core/platform/Timeline.kt index 7b913a74..efb89aec 100644 --- a/core/src/main/java/com/amplitude/core/platform/Timeline.kt +++ b/core/src/main/java/com/amplitude/core/platform/Timeline.kt @@ -42,6 +42,10 @@ open class Timeline { fun remove(plugin: Plugin) { // remove all plugins with this name in every category plugins.forEach { (_, list) -> + val wasRemoved = list.remove(plugin) + if (wasRemoved) { + plugin.teardown() + } list.remove(plugin) } } diff --git a/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java b/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java index 579faa54..bc41bf7a 100644 --- a/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java +++ b/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java @@ -84,4 +84,9 @@ public BaseEvent execute(@NonNull BaseEvent event) { event.getEventProperties().put("custom android event property", "test"); return event; } + + @Override + public void teardown() { + // Clean up any resources from setup if necessary + } } From bbe2b57c91ce4ff53d5a91e76ff5e13b3d42836f Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Thu, 11 Jan 2024 12:28:59 -0800 Subject: [PATCH 12/19] disable offline when null --- .../com/amplitude/android/Configuration.kt | 3 +- ...AndroidNetworkConnectivityCheckerPlugin.kt | 51 +++++++++++-------- ...oidNetworkConnectivityCheckerPluginTest.kt | 32 ++++++++++-- .../java/com/amplitude/core/Configuration.kt | 3 +- .../amplitude/core/platform/EventPipeline.kt | 2 +- .../core/platform/EventPipelineTest.kt | 15 ++++++ 6 files changed, 76 insertions(+), 30 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Configuration.kt b/android/src/main/java/com/amplitude/android/Configuration.kt index 6c536721..bdd1de81 100644 --- a/android/src/main/java/com/amplitude/android/Configuration.kt +++ b/android/src/main/java/com/amplitude/android/Configuration.kt @@ -46,7 +46,8 @@ open class Configuration @JvmOverloads constructor( override var identifyInterceptStorageProvider: StorageProvider = AndroidStorageProvider(), override var identityStorageProvider: IdentityStorageProvider = FileIdentityStorageProvider(), var migrateLegacyData: Boolean = true, -) : Configuration(apiKey, flushQueueSize, flushIntervalMillis, instanceName, optOut, storageProvider, loggerProvider, minIdLength, partnerId, callback, flushMaxRetries, useBatch, serverZone, serverUrl, plan, ingestionMetadata, identifyBatchIntervalMillis, identifyInterceptStorageProvider, identityStorageProvider) { + override var offline: Boolean? = false +) : Configuration(apiKey, flushQueueSize, flushIntervalMillis, instanceName, optOut, storageProvider, loggerProvider, minIdLength, partnerId, callback, flushMaxRetries, useBatch, serverZone, serverUrl, plan, ingestionMetadata, identifyBatchIntervalMillis, identifyInterceptStorageProvider, identityStorageProvider, offline) { companion object { const val MIN_TIME_BETWEEN_SESSIONS_MILLIS: Long = 300000 } diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt index 3b31541b..423fdfa4 100644 --- a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -10,34 +10,41 @@ import kotlinx.coroutines.launch class AndroidNetworkConnectivityCheckerPlugin : Plugin { override val type: Plugin.Type = Plugin.Type.Before override lateinit var amplitude: Amplitude - internal lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker - internal lateinit var networkListener: AndroidNetworkListener - internal val networkChangeHandler = - object : AndroidNetworkListener.NetworkChangeCallback { - override fun onNetworkAvailable() { - amplitude.logger.debug("AndroidNetworkListener, onNetworkAvailable") - amplitude.configuration.offline = false - amplitude.flush() - } - - override fun onNetworkUnavailable() { - amplitude.logger.debug("AndroidNetworkListener, onNetworkUnavailable") - amplitude.configuration.offline = true - } - } + internal var networkConnectivityChecker: AndroidNetworkConnectivityChecker? = null + internal var networkListener: AndroidNetworkListener? = null override fun setup(amplitude: Amplitude) { super.setup(amplitude) - networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) - amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { - amplitude.configuration.offline = !networkConnectivityChecker.isConnected() + if (amplitude.configuration.offline != null) { + amplitude.logger.debug("Installing AndroidNetworkConnectivityPlugin, offline feature should be supported.") + networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) + amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { + amplitude.configuration.offline = !networkConnectivityChecker!!.isConnected() + } + val networkChangeHandler = + object : AndroidNetworkListener.NetworkChangeCallback { + override fun onNetworkAvailable() { + amplitude.logger.debug("AndroidNetworkListener, onNetworkAvailable.") + amplitude.configuration.offline = false + amplitude.flush() + } + + override fun onNetworkUnavailable() { + amplitude.logger.debug("AndroidNetworkListener, onNetworkUnavailable.") + amplitude.configuration.offline = true + } + } + networkListener = AndroidNetworkListener((amplitude.configuration as Configuration).context) + networkListener!!.setNetworkChangeCallback(networkChangeHandler) + networkListener!!.startListening() } - networkListener = AndroidNetworkListener((amplitude.configuration as Configuration).context) - networkListener.setNetworkChangeCallback(networkChangeHandler) - networkListener.startListening() } override fun teardown() { - networkListener.stopListening() + if (amplitude.configuration.offline != null) { + // offline can be changed for custom offline implementation + // so use ?. instead of !! + networkListener?.stopListening() + } } } diff --git a/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt b/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt index 4e02f3eb..25c4d632 100644 --- a/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt +++ b/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt @@ -13,6 +13,7 @@ import org.junit.Before import org.junit.Test import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull class AndroidNetworkConnectivityCheckerPluginTest { @@ -38,19 +39,42 @@ class AndroidNetworkConnectivityCheckerPluginTest { } @Test - fun `should set up`() { + fun `should set up correctly by default`() { + // amplitude.configuration.offline defaults to false plugin.setup(amplitude) assertEquals(amplitude, plugin.amplitude) assertNotNull(plugin.networkConnectivityChecker) // Unit tests are run on JVM so default to online assertEquals(false, amplitude.configuration.offline) + assertNotNull(plugin.networkListener) + } + + @Test + fun `should not set up if offline is disabled`() { + amplitude.configuration.offline = null + plugin.setup(amplitude) + assertEquals(amplitude, plugin.amplitude) + assertNull(plugin.networkConnectivityChecker) + // Unit tests are run on JVM so default to online + assertEquals(null, amplitude.configuration.offline) + assertNull(plugin.networkListener) } @Test fun `should teardown correctly`() { plugin.setup(amplitude) - spyk(plugin.networkListener) - plugin.teardown() - verify { plugin.networkListener.stopListening() } + assertNotNull(plugin.networkListener) + plugin.networkListener?.let { networkListener -> + spyk(networkListener) + plugin.teardown() + verify { networkListener.stopListening() } + } + } + + @Test + fun `should not stop listening if offline is disabled`() { + amplitude.configuration.offline = null + plugin.setup(amplitude) + assertNull(plugin.networkListener) } } diff --git a/core/src/main/java/com/amplitude/core/Configuration.kt b/core/src/main/java/com/amplitude/core/Configuration.kt index 5afbf9b7..cec74bfd 100644 --- a/core/src/main/java/com/amplitude/core/Configuration.kt +++ b/core/src/main/java/com/amplitude/core/Configuration.kt @@ -30,10 +30,9 @@ open class Configuration @JvmOverloads constructor( open var identifyBatchIntervalMillis: Long = IDENTIFY_BATCH_INTERVAL_MILLIS, open var identifyInterceptStorageProvider: StorageProvider = InMemoryStorageProvider(), open var identityStorageProvider: IdentityStorageProvider = IMIdentityStorageProvider(), + open var offline: Boolean? = false ) { - var offline: Boolean = false - companion object { const val FLUSH_QUEUE_SIZE = 30 const val FLUSH_INTERVAL_MILLIS = 30 * 1000 // 30s diff --git a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt index 17926fe7..ebcc232a 100644 --- a/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt +++ b/core/src/main/java/com/amplitude/core/platform/EventPipeline.kt @@ -100,7 +100,7 @@ class EventPipeline( } // Skip flush when offline - if (amplitude.configuration.offline) { + if (amplitude.configuration.offline == true) { continue } diff --git a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt index 8a7888e6..8d60a984 100644 --- a/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt +++ b/core/src/test/kotlin/com/amplitude/core/platform/EventPipelineTest.kt @@ -66,6 +66,21 @@ class EventPipelineTest { eventPipeline.put(event) advanceUntilIdle() + verify(exactly = 1) { eventPipeline.flush() } + } + + @Test + fun `should flush when put and offline is disabled`() = + runTest(testDispatcher) { + amplitude.isBuilt.await() + amplitude.configuration.offline = null + val eventPipeline = spyk(EventPipeline(amplitude)) + val event = BaseEvent().apply { eventType = "test_event" } + + eventPipeline.start() + eventPipeline.put(event) + advanceUntilIdle() + verify(exactly = 1) { eventPipeline.flush() } } } From 6db25e4c2a92b5e3ce31f920aa950ebb44451073 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Fri, 12 Jan 2024 09:24:39 -0800 Subject: [PATCH 13/19] Update Timeline.kt --- core/src/main/java/com/amplitude/core/platform/Timeline.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/com/amplitude/core/platform/Timeline.kt b/core/src/main/java/com/amplitude/core/platform/Timeline.kt index efb89aec..aebe3fea 100644 --- a/core/src/main/java/com/amplitude/core/platform/Timeline.kt +++ b/core/src/main/java/com/amplitude/core/platform/Timeline.kt @@ -46,7 +46,6 @@ open class Timeline { if (wasRemoved) { plugin.teardown() } - list.remove(plugin) } } From 931ba2c140e89adefbbe54056cf9ae4b0dd68a40 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Fri, 12 Jan 2024 12:13:32 -0800 Subject: [PATCH 14/19] should not install plugin if null --- .../java/com/amplitude/android/Amplitude.kt | 4 +- ...AndroidNetworkConnectivityCheckerPlugin.kt | 50 ++++++++----------- ...oidNetworkConnectivityCheckerPluginTest.kt | 19 ------- 3 files changed, 25 insertions(+), 48 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index 1f7dd1d5..c009ec4c 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -57,7 +57,9 @@ open class Amplitude( } this.createIdentityContainer(identityConfiguration) - add(AndroidNetworkConnectivityCheckerPlugin()) + if (this.configuration.offline != null) { + add(AndroidNetworkConnectivityCheckerPlugin()) + } androidContextPlugin = AndroidContextPlugin() add(androidContextPlugin) add(GetAmpliExtrasPlugin()) diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt index 423fdfa4..18699ffe 100644 --- a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -10,41 +10,35 @@ import kotlinx.coroutines.launch class AndroidNetworkConnectivityCheckerPlugin : Plugin { override val type: Plugin.Type = Plugin.Type.Before override lateinit var amplitude: Amplitude - internal var networkConnectivityChecker: AndroidNetworkConnectivityChecker? = null - internal var networkListener: AndroidNetworkListener? = null + internal lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker + internal lateinit var networkListener: AndroidNetworkListener override fun setup(amplitude: Amplitude) { super.setup(amplitude) - if (amplitude.configuration.offline != null) { - amplitude.logger.debug("Installing AndroidNetworkConnectivityPlugin, offline feature should be supported.") - networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) - amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { - amplitude.configuration.offline = !networkConnectivityChecker!!.isConnected() - } - val networkChangeHandler = - object : AndroidNetworkListener.NetworkChangeCallback { - override fun onNetworkAvailable() { - amplitude.logger.debug("AndroidNetworkListener, onNetworkAvailable.") - amplitude.configuration.offline = false - amplitude.flush() - } + amplitude.logger.debug("Installing AndroidNetworkConnectivityPlugin, offline feature should be supported.") + networkConnectivityChecker = AndroidNetworkConnectivityChecker((amplitude.configuration as Configuration).context, amplitude.logger) + amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { + amplitude.configuration.offline = !networkConnectivityChecker.isConnected() + } + val networkChangeHandler = + object : AndroidNetworkListener.NetworkChangeCallback { + override fun onNetworkAvailable() { + amplitude.logger.debug("AndroidNetworkListener, onNetworkAvailable.") + amplitude.configuration.offline = false + amplitude.flush() + } - override fun onNetworkUnavailable() { - amplitude.logger.debug("AndroidNetworkListener, onNetworkUnavailable.") - amplitude.configuration.offline = true - } + override fun onNetworkUnavailable() { + amplitude.logger.debug("AndroidNetworkListener, onNetworkUnavailable.") + amplitude.configuration.offline = true } - networkListener = AndroidNetworkListener((amplitude.configuration as Configuration).context) - networkListener!!.setNetworkChangeCallback(networkChangeHandler) - networkListener!!.startListening() - } + } + networkListener = AndroidNetworkListener((amplitude.configuration as Configuration).context) + networkListener.setNetworkChangeCallback(networkChangeHandler) + networkListener.startListening() } override fun teardown() { - if (amplitude.configuration.offline != null) { - // offline can be changed for custom offline implementation - // so use ?. instead of !! - networkListener?.stopListening() - } + networkListener.stopListening() } } diff --git a/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt b/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt index 25c4d632..f7df5d5d 100644 --- a/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt +++ b/android/src/test/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPluginTest.kt @@ -13,7 +13,6 @@ import org.junit.Before import org.junit.Test import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotNull -import org.junit.jupiter.api.Assertions.assertNull class AndroidNetworkConnectivityCheckerPluginTest { @@ -49,17 +48,6 @@ class AndroidNetworkConnectivityCheckerPluginTest { assertNotNull(plugin.networkListener) } - @Test - fun `should not set up if offline is disabled`() { - amplitude.configuration.offline = null - plugin.setup(amplitude) - assertEquals(amplitude, plugin.amplitude) - assertNull(plugin.networkConnectivityChecker) - // Unit tests are run on JVM so default to online - assertEquals(null, amplitude.configuration.offline) - assertNull(plugin.networkListener) - } - @Test fun `should teardown correctly`() { plugin.setup(amplitude) @@ -70,11 +58,4 @@ class AndroidNetworkConnectivityCheckerPluginTest { verify { networkListener.stopListening() } } } - - @Test - fun `should not stop listening if offline is disabled`() { - amplitude.configuration.offline = null - plugin.setup(amplitude) - assertNull(plugin.networkListener) - } } From 4488d99e6e55aac381065398bc908762f8c2ac9a Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Wed, 17 Jan 2024 15:46:31 -0800 Subject: [PATCH 15/19] add Disable constant --- .../plugins/AndroidNetworkConnectivityCheckerPlugin.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt index 18699ffe..1ed3cf650 100644 --- a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -13,6 +13,10 @@ class AndroidNetworkConnectivityCheckerPlugin : Plugin { internal lateinit var networkConnectivityChecker: AndroidNetworkConnectivityChecker internal lateinit var networkListener: AndroidNetworkListener + companion object { + val Disabled = null + } + override fun setup(amplitude: Amplitude) { super.setup(amplitude) amplitude.logger.debug("Installing AndroidNetworkConnectivityPlugin, offline feature should be supported.") From cbbcd9aec006d32f240ec60d511fd66a32e3ebe8 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Thu, 18 Jan 2024 09:14:36 -0800 Subject: [PATCH 16/19] add package --- .../android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt | 2 +- .../com/amplitude/android/utilities/AndroidNetworkListener.kt | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt index 1ed3cf650..f187669f 100644 --- a/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt +++ b/android/src/main/java/com/amplitude/android/plugins/AndroidNetworkConnectivityCheckerPlugin.kt @@ -1,8 +1,8 @@ package com.amplitude.android.plugins -import AndroidNetworkListener import com.amplitude.android.Configuration import com.amplitude.android.utilities.AndroidNetworkConnectivityChecker +import com.amplitude.android.utilities.AndroidNetworkListener import com.amplitude.core.Amplitude import com.amplitude.core.platform.Plugin import kotlinx.coroutines.launch diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt index 4d86294a..336602cf 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidNetworkListener.kt @@ -1,3 +1,5 @@ +package com.amplitude.android.utilities + import android.annotation.SuppressLint import android.content.BroadcastReceiver import android.content.Context From 7c4d418f6fbbc383075aea3cb2279a41758c744d Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Mon, 22 Jan 2024 15:20:33 -0800 Subject: [PATCH 17/19] Update android/src/main/java/com/amplitude/android/Amplitude.kt Co-authored-by: Justin Fiedler --- android/src/main/java/com/amplitude/android/Amplitude.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index c009ec4c..749d38ba 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -57,7 +57,7 @@ open class Amplitude( } this.createIdentityContainer(identityConfiguration) - if (this.configuration.offline != null) { + if (this.configuration.offline != AndroidNetworkConnectivityCheckerPlugin.Disabled) { add(AndroidNetworkConnectivityCheckerPlugin()) } androidContextPlugin = AndroidContextPlugin() From f01adb912e3ba6be8a1fc12a7a67f726d4b098f1 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 23 Jan 2024 13:19:08 -0800 Subject: [PATCH 18/19] Use default method --- build.gradle | 2 +- .../java/com/amplitude/android/sample/MainApplication.java | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/build.gradle b/build.gradle index adf51f03..04bbf1f2 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ allprojects{ tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all { kotlinOptions { - freeCompilerArgs = ['-Xjvm-default=enable'] + freeCompilerArgs = ['-Xjvm-default=all'] jvmTarget = "1.8" } } diff --git a/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java b/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java index bc41bf7a..50a85fbe 100644 --- a/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java +++ b/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java @@ -85,8 +85,4 @@ public BaseEvent execute(@NonNull BaseEvent event) { return event; } - @Override - public void teardown() { - // Clean up any resources from setup if necessary - } } From 704c7f846e2636038cecb4967593f0400a7ad139 Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Tue, 23 Jan 2024 13:27:34 -0800 Subject: [PATCH 19/19] chore --- android/src/main/java/com/amplitude/android/Configuration.kt | 2 +- core/src/main/java/com/amplitude/core/Configuration.kt | 2 +- .../main/java/com/amplitude/android/sample/MainApplication.java | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Configuration.kt b/android/src/main/java/com/amplitude/android/Configuration.kt index bdd1de81..47de8abc 100644 --- a/android/src/main/java/com/amplitude/android/Configuration.kt +++ b/android/src/main/java/com/amplitude/android/Configuration.kt @@ -46,7 +46,7 @@ open class Configuration @JvmOverloads constructor( override var identifyInterceptStorageProvider: StorageProvider = AndroidStorageProvider(), override var identityStorageProvider: IdentityStorageProvider = FileIdentityStorageProvider(), var migrateLegacyData: Boolean = true, - override var offline: Boolean? = false + override var offline: Boolean? = false, ) : Configuration(apiKey, flushQueueSize, flushIntervalMillis, instanceName, optOut, storageProvider, loggerProvider, minIdLength, partnerId, callback, flushMaxRetries, useBatch, serverZone, serverUrl, plan, ingestionMetadata, identifyBatchIntervalMillis, identifyInterceptStorageProvider, identityStorageProvider, offline) { companion object { const val MIN_TIME_BETWEEN_SESSIONS_MILLIS: Long = 300000 diff --git a/core/src/main/java/com/amplitude/core/Configuration.kt b/core/src/main/java/com/amplitude/core/Configuration.kt index cec74bfd..29b20beb 100644 --- a/core/src/main/java/com/amplitude/core/Configuration.kt +++ b/core/src/main/java/com/amplitude/core/Configuration.kt @@ -30,7 +30,7 @@ open class Configuration @JvmOverloads constructor( open var identifyBatchIntervalMillis: Long = IDENTIFY_BATCH_INTERVAL_MILLIS, open var identifyInterceptStorageProvider: StorageProvider = InMemoryStorageProvider(), open var identityStorageProvider: IdentityStorageProvider = IMIdentityStorageProvider(), - open var offline: Boolean? = false + open var offline: Boolean? = false, ) { companion object { diff --git a/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java b/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java index 50a85fbe..579faa54 100644 --- a/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java +++ b/samples/java-android-app/src/main/java/com/amplitude/android/sample/MainApplication.java @@ -84,5 +84,4 @@ public BaseEvent execute(@NonNull BaseEvent event) { event.getEventProperties().put("custom android event property", "test"); return event; } - }