Skip to content

Commit

Permalink
fix(android): bugfixes for different SDK versions
Browse files Browse the repository at this point in the history
  • Loading branch information
abhaysood committed Nov 16, 2023
1 parent 61b189f commit 32cdbfa
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import sh.measure.android.gestures.ScrollEvent
import sh.measure.android.lifecycle.ActivityLifecycleEvent
import sh.measure.android.lifecycle.ApplicationLifecycleEvent
import sh.measure.android.lifecycle.FragmentLifecycleEvent
import sh.measure.android.network_change.NetworkChangeEvent

@Suppress("MemberVisibilityCanBePrivate")
internal class FakeEventTracker: EventTracker {
Expand All @@ -25,6 +26,7 @@ internal class FakeEventTracker: EventTracker {
val trackedApplicationLifecycleEvents = mutableListOf<ApplicationLifecycleEvent>()
val trackedColdLaunchEvents = mutableListOf<ColdLaunchEvent>()
val trackedWarmLaunchEvents = mutableListOf<WarmLaunchEvent>()
val trackedNetworkChangeEvents = mutableListOf<NetworkChangeEvent>()
val trackedHotLaunchEvents = mutableListOf<HotLaunchEvent>()
val trackedAttachments = mutableListOf<AttachmentInfo>()

Expand Down Expand Up @@ -72,6 +74,10 @@ internal class FakeEventTracker: EventTracker {
trackedHotLaunchEvents.add(event)
}

override fun trackNetworkChange(event: NetworkChangeEvent) {
trackedNetworkChangeEvents.add(event)
}

override fun storeAttachment(attachmentInfo: AttachmentInfo) {
trackedAttachments.add(attachmentInfo)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ internal class MeasureEventTracker(
}

override fun trackNetworkChange(event: NetworkChangeEvent) {
logger.log(LogLevel.Debug, "Tracking network change $event")
logger.log(LogLevel.Error, "Tracking network change ${event.network_generation}, ${event.network_type}, ${event.carrier_name}")
sessionController.storeEvent(event.toEvent())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,21 @@ internal class NetworkChangesCollector(
val previousNetworkType = currentNetworkType
val previousNetworkGeneration = currentNetworkGeneration
val newNetworkGeneration = getNetworkGenerationIfAvailable(newNetworkType)
val carrierName = getNetworkOperatorName(newNetworkType)

// for Android O+, the callback is called as soon as it's registered. However, we
// only want to track changes.
// This also means, the first change event will contain non-null previous states
// for Android O+.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
if (currentNetworkType == null) {
currentNetworkType = newNetworkType
currentNetworkGeneration = newNetworkGeneration
return
}
}

// only track significant changes
if (!shouldTrackNetworkChange(
newNetworkType,
previousNetworkType,
Expand All @@ -115,7 +129,7 @@ internal class NetworkChangesCollector(
) {
return
}
val carrierName = getNetworkOperatorName(newNetworkType)

eventTracker.trackNetworkChange(
NetworkChangeEvent(
previous_network_type = previousNetworkType,
Expand Down Expand Up @@ -170,21 +184,19 @@ internal class NetworkChangesCollector(
newNetworkGeneration: String?,
previousNetworkGeneration: String?
): Boolean {
when {
previousNetworkType == null || previousNetworkType == NetworkType.NO_NETWORK -> {
return true
}

return when {
// track if network type has changed
previousNetworkType != newNetworkType -> {
return true
true
}

// track if network type is cellular, but network generation has changed
newNetworkType == NetworkType.CELLULAR && newNetworkGeneration != null && newNetworkGeneration != previousNetworkGeneration -> {
return true
true
}

else -> {
return false
false
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ data class Resource(
val app_unique_id: String? = null, // package name,
val network_type: String? = null,
val network_generation: String? = null,
val carrier_name: String? = null,
val network_carrier_name: String? = null,
val measure_sdk_version: String? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal class ResourceFactoryImpl(
app_unique_id = context.packageName,
network_type = networkType,
network_generation = getNetworkGeneration(networkType),
carrier_name = getCarrierName(networkType),
network_carrier_name = getCarrierName(networkType),
measure_sdk_version = getMeasureVersion(),
)
}
Expand All @@ -71,6 +71,7 @@ internal class ResourceFactoryImpl(
if (networkType != NetworkType.CELLULAR) return null
systemServiceProvider.telephonyManager?.networkOperatorName.let {
if (it.isNullOrBlank()) return null
logger.log(LogLevel.Error, "Carrier name: $it")
return it
}
}
Expand Down Expand Up @@ -128,9 +129,10 @@ internal class ResourceFactoryImpl(
private fun getNetworkGeneration(networkType: String?): String? {
if (networkType != NetworkType.CELLULAR) return null
return if (hasPhoneStatePermission(context)) {
systemServiceProvider.telephonyManager?.getNetworkGeneration()
val networkGeneration = systemServiceProvider.telephonyManager?.getNetworkGeneration()
logger.log(LogLevel.Error, "Network generation: $networkGeneration")
networkGeneration
} else {
logger.log(LogLevel.Debug, "No permission to access phone state") // TODO improve msg
null
}
}
Expand All @@ -142,9 +144,13 @@ internal class ResourceFactoryImpl(
return null
}
return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
getNetworkTypeAboveApi23(connectivityManager)
val networkTypeAboveApi23 = getNetworkTypeAboveApi23(connectivityManager)
logger.log(LogLevel.Error, "Network type: $networkTypeAboveApi23")
networkTypeAboveApi23
} else {
getNetworkTypeBelowApi23(connectivityManager)
val networkTypeBelowApi23 = getNetworkTypeBelowApi23(connectivityManager)
logger.log(LogLevel.Error, "Network type: $networkTypeBelowApi23")
networkTypeBelowApi23
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ private fun fakeResource() = Resource(
app_unique_id = "app_unique_id",
network_type = NetworkType.WIFI,
network_generation = NetworkGeneration.FIFTH_GEN,
carrier_name = "Android",
network_carrier_name = "Android",
measure_sdk_version = "measure_sdk_version"
)
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import android.app.Application
import android.content.Context
import android.net.ConnectivityManager
import android.net.NetworkCapabilities
import android.os.Build
import android.telephony.TelephonyManager
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.junit.Assert
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito
import org.mockito.Mockito.mock
import org.mockito.kotlin.verify
import org.robolectric.RuntimeEnvironment
Expand Down Expand Up @@ -97,12 +99,13 @@ class NetworkChangesCollectorTest {
}

@Test
@Config(sdk = [23])
@Config(sdk = [23, 33])
fun `NetworkChangesCollector tracks change to cellular network with carrier_name and network_generation`() {
shadowOf(context as Application).grantPermissions(Manifest.permission.ACCESS_NETWORK_STATE)
shadowOf(context as Application).grantPermissions(Manifest.permission.READ_PHONE_STATE)
shadowOf(telephonyManager).setNetworkOperatorName("Test Carrier")
setNetworkTypeInTelephonyManager(networkType = TelephonyManager.NETWORK_TYPE_NR)
var previousNetworkType: String? = null

NetworkChangesCollector(
context = context,
Expand All @@ -113,11 +116,16 @@ class NetworkChangesCollectorTest {
systemServiceProvider = systemServiceProvider,
).register()

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
// first change is discarded for Android O and above
triggerNetworkCapabilitiesChange(addTransportType = NetworkCapabilities.TRANSPORT_WIFI)
previousNetworkType = NetworkType.WIFI
}
triggerNetworkCapabilitiesChange(addTransportType = NetworkCapabilities.TRANSPORT_CELLULAR)

verify(eventTracker).trackNetworkChange(
NetworkChangeEvent(
previous_network_type = null,
previous_network_type = previousNetworkType,
network_type = NetworkType.CELLULAR,
previous_network_generation = null,
network_generation = NetworkGeneration.FIFTH_GEN,
Expand Down Expand Up @@ -159,6 +167,28 @@ class NetworkChangesCollectorTest {
)
}

@Test
@Config(sdk = [26, 33])
fun `NetworkChangesCollector discards first change for SDK 26 and above`() {
shadowOf(context as Application).grantPermissions(Manifest.permission.ACCESS_NETWORK_STATE)
shadowOf(context as Application).grantPermissions(Manifest.permission.READ_BASIC_PHONE_STATE)
shadowOf(telephonyManager).setNetworkOperatorName("Test Carrier")
setNetworkTypeInTelephonyManager(networkType = TelephonyManager.NETWORK_TYPE_NR)

NetworkChangesCollector(
context = context,
logger = logger,
eventTracker = eventTracker,
timeProvider = timeProvider,
currentThread = currentThread,
systemServiceProvider = systemServiceProvider,
).register()

triggerNetworkCapabilitiesChange(addTransportType = NetworkCapabilities.TRANSPORT_CELLULAR)

Mockito.verifyNoInteractions(eventTracker)
}

@Test
@Config(sdk = [33])
fun `NetworkChangesCollector tracks change to cellular network with carrier_name & network_generation if READ_BASIC_PHONE_STATE permission is available`() {
Expand All @@ -176,11 +206,14 @@ class NetworkChangesCollectorTest {
systemServiceProvider = systemServiceProvider,
).register()

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
triggerNetworkCapabilitiesChange(addTransportType = NetworkCapabilities.TRANSPORT_WIFI)
}
triggerNetworkCapabilitiesChange(addTransportType = NetworkCapabilities.TRANSPORT_CELLULAR)

verify(eventTracker).trackNetworkChange(
NetworkChangeEvent(
previous_network_type = null,
previous_network_type = NetworkType.WIFI,
network_type = NetworkType.CELLULAR,
previous_network_generation = null,
network_generation = NetworkGeneration.FIFTH_GEN,
Expand Down Expand Up @@ -281,7 +314,6 @@ class NetworkChangesCollectorTest {
assertTrue(shouldTrackChange)
}


@Test
fun `NetworkChangesCollector should track network change for cellular network when previous gen is not equal to new gen`() {
// Simulate a previous cellular network type with a different previous and new generation
Expand Down Expand Up @@ -312,7 +344,6 @@ class NetworkChangesCollectorTest {
assertTrue(shouldTrackChange)
}


@Test
fun `NetworkChangesCollector should not track network change for same network type and generation`() {
// Simulate a cellular network type with the same previous and new generation
Expand Down Expand Up @@ -405,7 +436,7 @@ class NetworkChangesCollectorTest {

@Test
@Config(sdk = [23])
fun `NetworkChangesCollector tracks network change event with previous network when network is lost`() {
fun `NetworkChangesCollector discards first change with previous network when network is lost`() {
shadowOf(context as Application).grantPermissions(Manifest.permission.ACCESS_NETWORK_STATE)
shadowOf(context as Application).grantPermissions(Manifest.permission.READ_PHONE_STATE)
shadowOf(telephonyManager).setNetworkOperatorName("Test Carrier")
Expand Down

0 comments on commit 32cdbfa

Please sign in to comment.