Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import kotlinx.coroutines.test.UnconfinedTestDispatcher
class ScopeProviderStub private constructor(
override val eventBusScope: TestScope,
override val lifecycleListenerScope: TestScope,
override val inAppLifecycleScope: TestScope
override val inAppLifecycleScope: TestScope,
override val locationScope: TestScope
) : ScopeProvider {

@Suppress("FunctionName")
Expand All @@ -18,13 +19,15 @@ class ScopeProviderStub private constructor(
fun Unconfined(): ScopeProviderStub = ScopeProviderStub(
eventBusScope = TestScope(UnconfinedTestDispatcher()),
lifecycleListenerScope = TestScope(UnconfinedTestDispatcher()),
inAppLifecycleScope = TestScope(UnconfinedTestDispatcher())
inAppLifecycleScope = TestScope(UnconfinedTestDispatcher()),
locationScope = TestScope(UnconfinedTestDispatcher())
)

fun Standard(): ScopeProviderStub = ScopeProviderStub(
eventBusScope = TestScope(StandardTestDispatcher()),
lifecycleListenerScope = TestScope(StandardTestDispatcher()),
inAppLifecycleScope = TestScope(StandardTestDispatcher())
inAppLifecycleScope = TestScope(StandardTestDispatcher()),
locationScope = TestScope(StandardTestDispatcher())
)
}
}
2 changes: 2 additions & 0 deletions core/api/core.api
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public abstract interface class io/customer/sdk/core/util/ScopeProvider {
public abstract fun getEventBusScope ()Lkotlinx/coroutines/CoroutineScope;
public abstract fun getInAppLifecycleScope ()Lkotlinx/coroutines/CoroutineScope;
public abstract fun getLifecycleListenerScope ()Lkotlinx/coroutines/CoroutineScope;
public abstract fun getLocationScope ()Lkotlinx/coroutines/CoroutineScope;
}

public final class io/customer/sdk/core/util/SdkDispatchers : io/customer/sdk/core/util/DispatchersProvider {
Expand All @@ -264,6 +265,7 @@ public final class io/customer/sdk/core/util/SdkScopeProvider : io/customer/sdk/
public fun getEventBusScope ()Lkotlinx/coroutines/CoroutineScope;
public fun getInAppLifecycleScope ()Lkotlinx/coroutines/CoroutineScope;
public fun getLifecycleListenerScope ()Lkotlinx/coroutines/CoroutineScope;
public fun getLocationScope ()Lkotlinx/coroutines/CoroutineScope;
}

public abstract class io/customer/sdk/data/model/Region {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ interface ScopeProvider {
val eventBusScope: CoroutineScope
val lifecycleListenerScope: CoroutineScope
val inAppLifecycleScope: CoroutineScope
val locationScope: CoroutineScope
}

class SdkScopeProvider(private val dispatchers: DispatchersProvider) : ScopeProvider {
Expand All @@ -16,4 +17,6 @@ class SdkScopeProvider(private val dispatchers: DispatchersProvider) : ScopeProv
get() = CoroutineScope(dispatchers.default + SupervisorJob())
override val inAppLifecycleScope: CoroutineScope
get() = CoroutineScope(dispatchers.default + SupervisorJob())
override val locationScope: CoroutineScope
get() = CoroutineScope(dispatchers.default + SupervisorJob())
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,23 +134,21 @@ class ContextPluginBehaviorTest : JUnitTest(dispatcher = StandardTestDispatcher(
executor.shutdown()

// For each event executed by SDK, verify writer token that was active during the event's execution
val sortedWrites = writerLog.entries.sortedBy { it.key }
val mismatches = outputReaderPlugin.trackEvents.mapNotNull { event ->
val executionStartTime = event.context.getStringAtPath("test.executionStartTime")?.toLong().shouldNotBeNull()
val executionEndTime = event.context.getStringAtPath("test.executionEndTime")?.toLong().shouldNotBeNull()
val actualToken = event.context.deviceToken

// Find the latest write before the event execution end time
val latestWriteBeforeEvent = writerLog
.filterKeys { it <= executionEndTime }
.maxByOrNull { it.key }
// Find the newest write after the latest write
// This is because the writer might have written a new token after the event was executed
// So having a newer token is valid
val nextWriteAfterLatest = writerLog
.filterKeys { it > (latestWriteBeforeEvent?.key ?: Long.MAX_VALUE) }
.minByOrNull { it.key }
// Valid tokens are the latest write before the event and the next write after the latest
val validTokens = setOfNotNull(latestWriteBeforeEvent?.value, nextWriteAfterLatest?.value)
// Find the index of the latest write logged before the event finished.
// Note: registerDeviceToken sets the @Volatile field BEFORE writerLog
// records the timestamp, so on slow CI the actual token may be several
// writes ahead of the latest log entry. We allow a window of up to 3
// entries beyond the latest logged write to account for this gap.
val latestBeforeIndex = sortedWrites.indexOfLast { it.key <= executionEndTime }
val windowStart = latestBeforeIndex.coerceAtLeast(0)
val windowEnd = (latestBeforeIndex + 3).coerceAtMost(sortedWrites.size - 1)
val validTokens = (windowStart..windowEnd).map { sortedWrites[it].value }.toSet()

// If the actual token is not in valid tokens, it's a mismatch
if (actualToken !in validTokens) {
Expand Down
2 changes: 1 addition & 1 deletion location/api/location.api
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public final class io/customer/location/LocationModuleConfig$Builder : io/custom
}

public abstract interface class io/customer/location/LocationServices {
public abstract fun requestLocationUpdateOnce ()V
public abstract fun requestLocationUpdate ()V
public abstract fun setLastKnownLocation (DD)V
public abstract fun setLastKnownLocation (Landroid/location/Location;)V
public abstract fun stopLocationUpdates ()V
Expand Down
3 changes: 3 additions & 0 deletions location/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

<!-- Coarse location is sufficient for city/timezone level tracking -->
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this have any impact on customers' apps? Or was this added because only customers who want to use the location feature will include this module and should already be fine with the permission?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will only be added for the users who adds the module


</manifest>
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package io.customer.location

import io.customer.location.provider.LocationProvider
import io.customer.location.provider.LocationRequestException
import io.customer.location.type.LocationGranularity
import io.customer.sdk.communication.Event
import io.customer.sdk.communication.EventBus
import io.customer.sdk.core.util.Logger
import kotlinx.coroutines.CancellationException

/**
* Coordinates location requests: one-shot only.
* Uses an injected [LocationProvider] for platform location access.
*/
internal class LocationOrchestrator(
private val config: LocationModuleConfig,
private val logger: Logger,
private val eventBus: EventBus,
private val locationProvider: LocationProvider
) {

suspend fun requestLocationUpdate() {
if (!config.enableLocationTracking) {
logger.debug("Location tracking is disabled, ignoring requestLocationUpdate.")
return
}

val authStatus = locationProvider.currentAuthorizationStatus()
if (!authStatus.isAuthorized) {
logger.debug("Location permission not granted ($authStatus), ignoring request.")
return
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant authorization check in orchestrator and provider

Low Severity

LocationOrchestrator.requestLocationUpdate() calls locationProvider.currentAuthorizationStatus() to check permissions, then immediately calls locationProvider.requestLocation(), which internally calls currentAuthorizationStatus() again. Every one-shot location request performs the same permission check twice. This duplicated logic increases maintenance burden — if the authorization check evolves, it needs consistent updates in both places.

Additional Locations (1)

Fix in Cursor Fix in Web


try {
val snapshot = locationProvider.requestLocation(
granularity = LocationGranularity.DEFAULT
)
postLocation(snapshot.latitude, snapshot.longitude)
} catch (e: CancellationException) {
logger.debug("Location request was cancelled.")
throw e
} catch (e: LocationRequestException) {
logger.debug("Location request failed: ${e.error}")
} catch (e: Exception) {
logger.error("Location request failed with unexpected error: ${e.message}")
}
}

private fun postLocation(latitude: Double, longitude: Double) {
logger.debug("Tracking location: lat=$latitude, lng=$longitude")
val locationData = Event.LocationData(
latitude = latitude,
longitude = longitude
)
eventBus.publish(Event.TrackLocationEvent(location = locationData))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ interface LocationServices {
* The SDK does not request location permission. The host app must request
* runtime permissions and only call this when permission is granted.
*/
fun requestLocationUpdateOnce()
fun requestLocationUpdate()

/**
* Cancels any in-flight location request.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,28 @@ import android.location.Location
import io.customer.sdk.communication.Event
import io.customer.sdk.communication.EventBus
import io.customer.sdk.core.util.Logger
import java.util.concurrent.locks.ReentrantLock
import kotlin.concurrent.withLock
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

/**
* Real implementation of [LocationServices].
* Handles manual location setting with validation and config checks.
*
* SDK-managed location (requestLocationUpdateOnce) will be implemented in a future PR.
* Handles manual location setting with validation and config checks,
* and SDK-managed one-shot location via [LocationOrchestrator].
*/
internal class LocationServicesImpl(
private val config: LocationModuleConfig,
private val logger: Logger,
private val eventBus: EventBus
private val eventBus: EventBus,
private val orchestrator: LocationOrchestrator,
private val scope: CoroutineScope
) : LocationServices {

private val lock = ReentrantLock()
private var currentLocationJob: Job? = null

override fun setLastKnownLocation(latitude: Double, longitude: Double) {
if (!config.enableLocationTracking) {
logger.debug("Location tracking is disabled, ignoring setLastKnownLocation.")
Expand All @@ -41,14 +50,35 @@ internal class LocationServicesImpl(
setLastKnownLocation(location.latitude, location.longitude)
}

override fun requestLocationUpdateOnce() {
// Will be implemented in the SDK-managed location PR
logger.debug("requestLocationUpdateOnce is not yet implemented.")
override fun requestLocationUpdate() {
lock.withLock {
// Cancel any previous in-flight request
currentLocationJob?.cancel()

currentLocationJob = scope.launch {
val thisJob = coroutineContext[Job]
try {
orchestrator.requestLocationUpdate()
} finally {
lock.withLock {
if (currentLocationJob === thisJob) {
currentLocationJob = null
}
}
}
}
}
}

override fun stopLocationUpdates() {
// Will be implemented in the SDK-managed location PR
logger.debug("stopLocationUpdates is not yet implemented.")
val job: Job?
lock.withLock {
job = currentLocationJob
currentLocationJob = null
}
// Cancelling the job triggers invokeOnCancellation in FusedLocationProvider's
// suspendCancellableCoroutine, which cancels the CancellationTokenSource.
job?.cancel()
}

companion object {
Expand Down
26 changes: 22 additions & 4 deletions location/src/main/kotlin/io/customer/location/ModuleLocation.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.customer.location

import android.location.Location
import io.customer.location.provider.FusedLocationProvider
import io.customer.sdk.core.di.SDKComponent
import io.customer.sdk.core.module.CustomerIOModule
import io.customer.sdk.core.util.Logger
Expand All @@ -26,8 +27,11 @@ import io.customer.sdk.core.util.Logger
*
* CustomerIO.initialize(config)
*
* // Then use the location services
* // Manual location from host app
* ModuleLocation.instance().locationServices.setLastKnownLocation(37.7749, -122.4194)
*
* // SDK-managed one-shot location
* ModuleLocation.instance().locationServices.requestLocationUpdate()
* ```
*/
class ModuleLocation @JvmOverloads constructor(
Expand All @@ -52,10 +56,24 @@ class ModuleLocation @JvmOverloads constructor(
get() = _locationServices ?: UninitializedLocationServices(SDKComponent.logger)

override fun initialize() {
val logger = SDKComponent.logger
val eventBus = SDKComponent.eventBus
val context = SDKComponent.android().applicationContext

val locationProvider = FusedLocationProvider(context)
val orchestrator = LocationOrchestrator(
config = moduleConfig,
logger = logger,
eventBus = eventBus,
locationProvider = locationProvider
)

_locationServices = LocationServicesImpl(
config = moduleConfig,
logger = SDKComponent.logger,
eventBus = SDKComponent.eventBus
logger = logger,
eventBus = eventBus,
orchestrator = orchestrator,
scope = SDKComponent.scopeProvider.locationScope
)
}

Expand Down Expand Up @@ -92,7 +110,7 @@ private class UninitializedLocationServices(

override fun setLastKnownLocation(location: Location) = logNotInitialized()

override fun requestLocationUpdateOnce() = logNotInitialized()
override fun requestLocationUpdate() = logNotInitialized()

override fun stopLocationUpdates() = logNotInitialized()
}
Loading
Loading