fix: location sharing without gms when not moving [WPB-9724] 🍒 (#3143)

Co-authored-by: Michał Saleniuk <[email protected]>
Co-authored-by: Michał Saleniuk <[email protected]>

app/build.gradle.kts

@@ -1,5 +1,3 @@
-import scripts.Variants_gradle
-
 /*
  * Wire
  * Copyright (C) 2024 Wire Swiss GmbH

app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt

@@ -17,16 +17,17 @@
  */
 package com.wire.android.ui.home.messagecomposer.location
 
-import android.content.Context
 import javax.inject.Inject
 import javax.inject.Singleton
 
 @Singleton
-class LocationPickerHelperFlavor @Inject constructor(context: Context) : LocationPickerHelper(context) {
+class LocationPickerHelperFlavor @Inject constructor(
+    private val locationPickerHelper: LocationPickerHelper,
+) {
     suspend fun getLocation(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) {
-        getLocationWithoutGms(
+        locationPickerHelper.getLocationWithoutGms(
             onSuccess = onSuccess,
-            onError = onError
+            onError = onError,
         )
     }
 }

app/src/main/kotlin/com/wire/android/di/AppModule.kt

@@ -20,12 +20,14 @@ package com.wire.android.di
 
 import android.app.NotificationManager
 import android.content.Context
+import android.location.Geocoder
 import android.media.AudioAttributes
 import android.media.MediaPlayer
 import androidx.core.app.NotificationManagerCompat
 import com.wire.android.BuildConfig
 import com.wire.android.mapper.MessageResourceProvider
 import com.wire.android.ui.home.appLock.CurrentTimestampProvider
+import com.wire.android.ui.home.messagecomposer.location.LocationPickerParameters
 import com.wire.android.util.dispatchers.DefaultDispatcherProvider
 import com.wire.android.util.dispatchers.DispatcherProvider
 import dagger.Module
@@ -82,4 +84,11 @@ object AppModule {
     @Singleton
     @Provides
     fun provideCurrentTimestampProvider(): CurrentTimestampProvider = { System.currentTimeMillis() }
+
+    @Provides
+    fun provideGeocoder(appContext: Context): Geocoder = Geocoder(appContext)
+
+    @Singleton
+    @Provides
+    fun provideLocationPickerParameters(): LocationPickerParameters = LocationPickerParameters()
 }

app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt

@@ -0,0 +1,37 @@
+/*
+ * Wire
+ * Copyright (C) 2024 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+package com.wire.android.ui.home.messagecomposer.location
+
+import android.location.Geocoder
+import android.location.Location
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class GeocoderHelper @Inject constructor(private val geocoder: Geocoder) {
+
+    @Suppress("TooGenericExceptionCaught")
+    fun getGeoLocatedAddress(location: Location): GeoLocatedAddress =
+        try {
+            geocoder.getFromLocation(location.latitude, location.longitude, 1).orEmpty()
+        } catch (e: Exception) {
+            emptyList()
+        }.let { addressList ->
+            GeoLocatedAddress(addressList.firstOrNull(), location)
+        }
+}

app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt

@@ -19,35 +19,59 @@ package com.wire.android.ui.home.messagecomposer.location
 
 import android.annotation.SuppressLint
 import android.content.Context
-import android.location.Geocoder
 import android.location.Location
 import android.location.LocationListener
 import android.location.LocationManager
+import android.os.Build
+import android.os.CancellationSignal
+import androidx.annotation.VisibleForTesting
 import androidx.core.location.LocationManagerCompat
 import com.wire.android.AppJsonStyledLogger
+import com.wire.android.di.ApplicationScope
+import com.wire.android.ui.home.appLock.CurrentTimestampProvider
 import com.wire.kalium.logger.KaliumLogLevel
 import dagger.hilt.android.qualifiers.ApplicationContext
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.CoroutineStart
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
+import java.util.function.Consumer
 import javax.inject.Inject
+import javax.inject.Singleton
+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.minutes
+import kotlin.time.Duration.Companion.seconds
 
-open class LocationPickerHelper @Inject constructor(@ApplicationContext val context: Context) {
+@SuppressLint("MissingPermission")
+@Singleton
+class LocationPickerHelper @Inject constructor(
+    @ApplicationContext private val context: Context,
+    @ApplicationScope private val scope: CoroutineScope,
+    private val currentTimestampProvider: CurrentTimestampProvider,
+    private val geocoderHelper: GeocoderHelper,
+    private val parameters: LocationPickerParameters,
+) {
 
-    @SuppressLint("MissingPermission")
-    protected fun getLocationWithoutGms(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) {
+    @VisibleForTesting
+    fun getLocationWithoutGms(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) {
         if (isLocationServicesEnabled()) {
             AppJsonStyledLogger.log(
                 level = KaliumLogLevel.INFO,
                 leadingMessage = "GetLocation",
                 jsonStringKeyValues = mapOf("isUsingGms" to false)
             )
             val locationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
-            val networkLocationListener: LocationListener = object : LocationListener {
-                override fun onLocationChanged(location: Location) {
-                    val address = Geocoder(context).getFromLocation(location.latitude, location.longitude, 1).orEmpty()
-                    onSuccess(GeoLocatedAddress(address.firstOrNull(), location))
-                    locationManager.removeUpdates(this) // important step, otherwise it will keep listening for location changes
+            locationManager.getLastKnownLocation(LocationManager.FUSED_PROVIDER).let { lastLocation ->
+                if (
+                    lastLocation != null
+                        && currentTimestampProvider() - lastLocation.time <= parameters.lastLocationTimeLimit.inWholeMilliseconds
+                ) {
+                    // use last known location if present and not older than given limit
+                    onSuccess(geocoderHelper.getGeoLocatedAddress(lastLocation))
+                } else {
+                    locationManager.requestCurrentLocationWithoutGms(onSuccess, onError)
                 }
             }
-            locationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, 0, 0f, networkLocationListener)
         } else {
             AppJsonStyledLogger.log(
                 level = KaliumLogLevel.WARN,
@@ -61,8 +85,45 @@ open class LocationPickerHelper @Inject constructor(@ApplicationContext val cont
         }
     }
 
-    protected fun isLocationServicesEnabled(): Boolean {
+    private fun LocationManager.requestCurrentLocationWithoutGms(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) {
+        val cancellationSignal = CancellationSignal()
+        val timeoutJob = scope.launch(start = CoroutineStart.LAZY) {
+            delay(parameters.requestLocationTimeout)
+            cancellationSignal.cancel()
+            onError()
+        }
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
+            val executor = context.mainExecutor
+            val consumer: Consumer<Location?> = Consumer { location ->
+                timeoutJob.cancel()
+                if (location != null) {
+                    onSuccess(geocoderHelper.getGeoLocatedAddress(location))
+                } else {
+                    onError()
+                }
+            }
+            this.getCurrentLocation(LocationManager.FUSED_PROVIDER, cancellationSignal, executor, consumer)
+        } else {
+            val listener = LocationListener { location ->
+                timeoutJob.cancel()
+                onSuccess(geocoderHelper.getGeoLocatedAddress(location))
+            }
+            cancellationSignal.setOnCancelListener {
+                this.removeUpdates(listener)
+            }
+            this.requestSingleUpdate(LocationManager.FUSED_PROVIDER, listener, null)
+        }
+        timeoutJob.start()
+    }
+
+    internal fun isLocationServicesEnabled(): Boolean {
         val locationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
         return LocationManagerCompat.isLocationEnabled(locationManager)
     }
 }
+
+data class LocationPickerParameters(
+    val lastLocationTimeLimit: Duration = 1.minutes,
+    val requestLocationTimeout: Duration = 10.seconds,
+)

app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt

@@ -19,7 +19,6 @@ package com.wire.android.ui.home.messagecomposer.location
 
 import android.annotation.SuppressLint
 import android.content.Context
-import android.location.Geocoder
 import com.google.android.gms.location.LocationServices
 import com.google.android.gms.location.Priority
 import com.google.android.gms.tasks.CancellationTokenSource
@@ -31,16 +30,19 @@ import javax.inject.Inject
 import javax.inject.Singleton
 
 @Singleton
-class LocationPickerHelperFlavor @Inject constructor(context: Context) : LocationPickerHelper(context) {
-
+class LocationPickerHelperFlavor @Inject constructor(
+    private val context: Context,
+    private val geocoderHelper: GeocoderHelper,
+    private val locationPickerHelper: LocationPickerHelper,
+) {
     suspend fun getLocation(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) {
         if (context.isGoogleServicesAvailable()) {
             getLocationWithGms(
                 onSuccess = onSuccess,
                 onError = onError
             )
         } else {
-            getLocationWithoutGms(
+            locationPickerHelper.getLocationWithoutGms(
                 onSuccess = onSuccess,
                 onError = onError
             )
@@ -53,7 +55,7 @@ class LocationPickerHelperFlavor @Inject constructor(context: Context) : Locatio
      */
     @SuppressLint("MissingPermission")
     private suspend fun getLocationWithGms(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) {
-        if (isLocationServicesEnabled()) {
+        if (locationPickerHelper.isLocationServicesEnabled()) {
             AppJsonStyledLogger.log(
                 level = KaliumLogLevel.INFO,
                 leadingMessage = "GetLocation",
@@ -62,8 +64,7 @@ class LocationPickerHelperFlavor @Inject constructor(context: Context) : Locatio
             val locationProvider = LocationServices.getFusedLocationProviderClient(context)
             val currentLocation =
                 locationProvider.getCurrentLocation(Priority.PRIORITY_HIGH_ACCURACY, CancellationTokenSource().token).await()
-            val address = Geocoder(context).getFromLocation(currentLocation.latitude, currentLocation.longitude, 1).orEmpty()
-            onSuccess(GeoLocatedAddress(address.firstOrNull(), currentLocation))
+            onSuccess(geocoderHelper.getGeoLocatedAddress(currentLocation))
         } else {
             AppJsonStyledLogger.log(
                 level = KaliumLogLevel.WARN,

app/src/test/kotlin/com/wire/android/mapper/UserTypeMapperTest.kt

@@ -46,7 +46,7 @@ class UserTypeMapperTest {
     }
 
     @Test
-    fun `given internal as a user type correctly map to none as membership`() {
+    fun `given internal as a user type correctly map to standard as membership`() {
         val result = userTypeMapper.toMembership(UserType.INTERNAL)
         assertEquals(Membership.Standard, result)
     }

app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt

@@ -55,13 +55,14 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 
 @OptIn(ExperimentalCoroutinesApi::class)
-@ExtendWith(CoroutineTestExtension::class)
-@ExtendWith(NavigationTestExtension::class)
+@ExtendWith(CoroutineTestExtension::class, NavigationTestExtension::class)
 class EditGuestAccessViewModelTest {
 
+    val dispatcher = TestDispatcherProvider()
+
     @Test
     fun `given updateConversationAccessRole use case runs successfully, when trying to enable guest access, then enable guest access`() =
-        runTest {
+        runTest(dispatcher.default()) {
             // given
             val (arrangement, editGuestAccessViewModel) = Arrangement()
                 .withUpdateConversationAccessRoleResult(UpdateConversationAccessRoleUseCase.Result.Success)
@@ -78,7 +79,7 @@ class EditGuestAccessViewModelTest {
 
     @Test
     fun `given a failure when running updateConversationAccessRole, when trying to enable guest access, then do not enable guest access`() =
-        runTest {
+        runTest(dispatcher.default()) {
             // given
             val (arrangement, editGuestAccessViewModel) = Arrangement()
                 .withUpdateConversationAccessRoleResult(
@@ -96,7 +97,7 @@ class EditGuestAccessViewModelTest {
 
     @Test
     fun `given guest access is activated, when trying to disable guest access, then display dialog before disabling guest access`() =
-        runTest {
+        runTest(dispatcher.default()) {
             // given
             val (arrangement, editGuestAccessViewModel) = Arrangement()
                 .withUpdateConversationAccessRoleResult(UpdateConversationAccessRoleUseCase.Result.Success)
@@ -107,12 +108,12 @@ class EditGuestAccessViewModelTest {
             editGuestAccessViewModel.updateGuestAccess(false)
 
             // then
-            coVerify(inverse = true) { arrangement.updateConversationAccessRoleUseCase(any(), any(), any()) }
+            coVerify(inverse = true) { arrangement.updateConversationAccessRole(any(), any(), any()) }
             assertEquals(true, editGuestAccessViewModel.editGuestAccessState.shouldShowGuestAccessChangeConfirmationDialog)
         }
 
     @Test
-    fun `given useCase runs with success, when_generating guest link, then invoke it once`() = runTest {
+    fun `given useCase runs with success, when_generating guest link, then invoke it once`() = runTest(dispatcher.default()) {
         // given
         val (arrangement, editGuestAccessViewModel) = Arrangement()
             .withGenerateGuestRoomResult(GenerateGuestRoomLinkResult.Success)
@@ -128,7 +129,7 @@ class EditGuestAccessViewModelTest {
     }
 
     @Test
-    fun `given useCase runs with failure, when generating guest link, then show dialog error`() = runTest {
+    fun `given useCase runs with failure, when generating guest link, then show dialog error`() = runTest(dispatcher.default()) {
         // given
         val (arrangement, editGuestAccessViewModel) = Arrangement()
             .withGenerateGuestRoomResult(
@@ -145,7 +146,7 @@ class EditGuestAccessViewModelTest {
     }
 
     @Test
-    fun `given useCase runs with success, when revoking guest link, then invoke it once`() = runTest {
+    fun `given useCase runs with success, when revoking guest link, then invoke it once`() = runTest(dispatcher.default()) {
         // given
         val (arrangement, editGuestAccessViewModel) = Arrangement()
             .withRevokeGuestRoomLinkResult(RevokeGuestRoomLinkResult.Success)
@@ -161,7 +162,7 @@ class EditGuestAccessViewModelTest {
     }
 
     @Test
-    fun `given useCase runs with failure when revoking guest link then show dialog error`() = runTest {
+    fun `given useCase runs with failure when revoking guest link then show dialog error`() = runTest(dispatcher.default()) {
         // given
         val (arrangement, editGuestAccessViewModel) = Arrangement()
             .withRevokeGuestRoomLinkResult(RevokeGuestRoomLinkResult.Failure(CoreFailure.MissingClientRegistration))
@@ -179,7 +180,7 @@ class EditGuestAccessViewModelTest {
 
     @Test
     fun `given updateConversationAccessRole use case runs successfully, when trying to disable guest access, then disable guest access`() =
-        runTest {
+        runTest(dispatcher.default()) {
             // given
             val (arrangement, editGuestAccessViewModel) = Arrangement()
                 .withUpdateConversationAccessRoleResult(UpdateConversationAccessRoleUseCase.Result.Success)
@@ -196,7 +197,7 @@ class EditGuestAccessViewModelTest {
 
     @Test
     fun `given a failure running updateConversationAccessRole, when trying to disable guest access, then do not disable guest access`() =
-        runTest {
+        runTest(dispatcher.default()) {
             // given
             val (arrangement, editGuestAccessViewModel) = Arrangement()
                 .withUpdateConversationAccessRoleResult(
@@ -217,9 +218,6 @@ class EditGuestAccessViewModelTest {
         @MockK
         lateinit var savedStateHandle: SavedStateHandle
 
-        @MockK
-        lateinit var updateConversationAccessRoleUseCase: UpdateConversationAccessRoleUseCase
-
         @MockK
         lateinit var observeConversationDetails: ObserveConversationDetailsUseCase