From 0011eda8e0adb3dc661213047de5c1cae0ca7b93 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 3 May 2022 15:41:31 +0200 Subject: [PATCH 01/13] Adding changelog entry --- changelog.d/5913.misc | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/5913.misc diff --git a/changelog.d/5913.misc b/changelog.d/5913.misc new file mode 100644 index 0000000000..b0f562f069 --- /dev/null +++ b/changelog.d/5913.misc @@ -0,0 +1,2 @@ +- Notify of the latest known location in LocationTracker to avoid multiple locations at start +- Debounce location updates From 7860173fa22e7480353ef3dedc04903af5eef0d9 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 3 May 2022 16:35:53 +0200 Subject: [PATCH 02/13] Notify of the latest known location among all last known locations --- .../app/features/location/LocationTracker.kt | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 1913f4202d..6f9d4b7870 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -57,34 +57,36 @@ class LocationTracker @Inject constructor( return } - locationManager.allProviders - .takeIf { it.isNotEmpty() } - // Take GPS first - ?.sortedByDescending { if (it == LocationManager.GPS_PROVIDER) 1 else 0 } - ?.forEach { provider -> - Timber.d("## LocationTracker. track location using $provider") + val providers = locationManager.allProviders - // Send last known location without waiting location updates - locationManager.getLastKnownLocation(provider)?.let { lastKnownLocation -> - if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { - Timber.d("## LocationTracker. lastKnownLocation: $lastKnownLocation") - } else { - Timber.d("## LocationTracker. lastKnownLocation: ${lastKnownLocation.provider}") - } - notifyLocation(lastKnownLocation, isLive = false) + if (providers.isEmpty()) { + callbacks.forEach { it.onLocationProviderIsNotAvailable() } + Timber.v("## LocationTracker. There is no location provider available") + } else { + // Take GPS first + providers.sortedByDescending { if (it == LocationManager.GPS_PROVIDER) 1 else 0 } + .mapNotNull { provider -> + Timber.d("## LocationTracker. track location using $provider") + + locationManager.requestLocationUpdates( + provider, + MIN_TIME_TO_UPDATE_LOCATION_MILLIS, + MIN_DISTANCE_TO_UPDATE_LOCATION_METERS, + this + ) + + locationManager.getLastKnownLocation(provider) } - - locationManager.requestLocationUpdates( - provider, - MIN_TIME_TO_UPDATE_LOCATION_MILLIS, - MIN_DISTANCE_TO_UPDATE_LOCATION_METERS, - this - ) - } - ?: run { - callbacks.forEach { it.onLocationProviderIsNotAvailable() } - Timber.v("## LocationTracker. There is no location provider available") - } + .maxByOrNull { location -> location.time } + ?.let { latestKnownLocation -> + if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { + Timber.d("## LocationTracker. lastKnownLocation: $latestKnownLocation") + } else { + Timber.d("## LocationTracker. lastKnownLocation: ${latestKnownLocation.provider}") + } + notifyLocation(latestKnownLocation, isLive = false) + } + } } @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) @@ -125,6 +127,7 @@ class LocationTracker @Inject constructor( } private fun notifyLocation(location: Location, isLive: Boolean) { + Timber.d("## LocationTracker. notify location") when (location.provider) { LocationManager.GPS_PROVIDER -> { hasGpsProviderLiveLocation = isLive From c61412520dff34e746ea8f5562164ffea3f99d0e Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 3 May 2022 16:55:14 +0200 Subject: [PATCH 03/13] Debouncing location updates --- .../app/features/location/LocationTracker.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 6f9d4b7870..2a5dff57d5 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -44,6 +44,8 @@ class LocationTracker @Inject constructor( private var hasGpsProviderLiveLocation = false + private var lastLocationUpdateMillis: Long? = null + private var lastLocation: LocationData? = null @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) @@ -84,7 +86,7 @@ class LocationTracker @Inject constructor( } else { Timber.d("## LocationTracker. lastKnownLocation: ${latestKnownLocation.provider}") } - notifyLocation(latestKnownLocation, isLive = false) + notifyLocation(latestKnownLocation) } } } @@ -123,14 +125,10 @@ class LocationTracker @Inject constructor( } else { Timber.d("## LocationTracker. onLocationChanged: ${location.provider}") } - notifyLocation(location, isLive = true) - } - private fun notifyLocation(location: Location, isLive: Boolean) { - Timber.d("## LocationTracker. notify location") when (location.provider) { LocationManager.GPS_PROVIDER -> { - hasGpsProviderLiveLocation = isLive + hasGpsProviderLiveLocation = true } else -> { if (hasGpsProviderLiveLocation) { @@ -140,9 +138,22 @@ class LocationTracker @Inject constructor( } } } + + val nowMillis = System.currentTimeMillis() + val elapsedMillis = nowMillis - (lastLocationUpdateMillis ?: 0) + if (elapsedMillis >= MIN_TIME_TO_UPDATE_LOCATION_MILLIS) { + lastLocationUpdateMillis = nowMillis + notifyLocation(location) + } else { + Timber.d("## LocationTracker. ignoring location: update is too fast") + } + } + + private fun notifyLocation(location: Location) { + Timber.d("## LocationTracker. notify location") val locationData = location.toLocationData() lastLocation = locationData - callbacks.forEach { it.onLocationUpdate(locationData) } + callbacks.forEach { it.onLocationUpdate(location.toLocationData()) } } override fun onProviderDisabled(provider: String) { From 5e6422b64c95ab7c3e350851f3c8020b783e7a9b Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Fri, 3 Jun 2022 09:52:13 +0200 Subject: [PATCH 04/13] Updating changelog --- changelog.d/5913.misc | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.d/5913.misc b/changelog.d/5913.misc index b0f562f069..24be7194d5 100644 --- a/changelog.d/5913.misc +++ b/changelog.d/5913.misc @@ -1,2 +1,3 @@ - Notify of the latest known location in LocationTracker to avoid multiple locations at start - Debounce location updates +- Improve location providers access From a8c36e5e70534f80b2315a7de92587061583ce4d Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Fri, 3 Jun 2022 11:14:34 +0200 Subject: [PATCH 05/13] Using Debouncer to debounce location updates --- .../app/features/location/LocationTracker.kt | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 2a5dff57d5..66a0cac4bd 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -24,10 +24,15 @@ import androidx.annotation.RequiresPermission import androidx.core.content.getSystemService import androidx.core.location.LocationListenerCompat import im.vector.app.BuildConfig +import im.vector.app.core.utils.Debouncer +import im.vector.app.core.utils.createBackgroundHandler import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton +private const val BKG_HANDLER_NAME = "LocationTracker.BKG_HANDLER_NAME" +private const val LOCATION_DEBOUNCE_ID = "LocationTracker.LOCATION_DEBOUNCE_ID" + @Singleton class LocationTracker @Inject constructor( context: Context @@ -44,14 +49,13 @@ class LocationTracker @Inject constructor( private var hasGpsProviderLiveLocation = false - private var lastLocationUpdateMillis: Long? = null - private var lastLocation: LocationData? = null + private val debouncer = Debouncer(createBackgroundHandler(BKG_HANDLER_NAME)) + @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) fun start() { Timber.d("## LocationTracker. start()") - hasGpsProviderLiveLocation = false if (locationManager == null) { callbacks.forEach { it.onLocationProviderIsNotAvailable() } @@ -96,6 +100,8 @@ class LocationTracker @Inject constructor( Timber.d("## LocationTracker. stop()") locationManager?.removeUpdates(this) callbacks.clear() + debouncer.cancelAll() + hasGpsProviderLiveLocation = false } /** @@ -139,18 +145,18 @@ class LocationTracker @Inject constructor( } } - val nowMillis = System.currentTimeMillis() - val elapsedMillis = nowMillis - (lastLocationUpdateMillis ?: 0) - if (elapsedMillis >= MIN_TIME_TO_UPDATE_LOCATION_MILLIS) { - lastLocationUpdateMillis = nowMillis + debouncer.debounce(LOCATION_DEBOUNCE_ID, MIN_TIME_TO_UPDATE_LOCATION_MILLIS) { notifyLocation(location) - } else { - Timber.d("## LocationTracker. ignoring location: update is too fast") } } private fun notifyLocation(location: Location) { - Timber.d("## LocationTracker. notify location") + if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { + Timber.d("## LocationTracker. notify location: $location") + } else { + Timber.d("## LocationTracker. notify location: ${location.provider}") + } + val locationData = location.toLocationData() lastLocation = locationData callbacks.forEach { it.onLocationUpdate(location.toLocationData()) } From b686d30b1ceae3ff0c3e19a1dbbf237dfd3dfce7 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Fri, 3 Jun 2022 11:53:59 +0200 Subject: [PATCH 06/13] Prioritise providers: Fused, GPS and then others --- .../app/features/location/LocationTracker.kt | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 66a0cac4bd..84836e53c0 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -47,7 +47,8 @@ class LocationTracker @Inject constructor( private val callbacks = mutableListOf() - private var hasGpsProviderLiveLocation = false + private var hasLocationFromFusedProvider = false + private var hasLocationFromGPSProvider = false private var lastLocation: LocationData? = null @@ -70,7 +71,7 @@ class LocationTracker @Inject constructor( Timber.v("## LocationTracker. There is no location provider available") } else { // Take GPS first - providers.sortedByDescending { if (it == LocationManager.GPS_PROVIDER) 1 else 0 } + providers.sortedByDescending { getProviderPriority(it) } .mapNotNull { provider -> Timber.d("## LocationTracker. track location using $provider") @@ -95,13 +96,24 @@ class LocationTracker @Inject constructor( } } + /** + * Compute the priority of the given provider name. + * @return an integer representing the priority: the higher the value, the higher the priority is + */ + private fun getProviderPriority(provider: String): Int = when (provider) { + LocationManager.FUSED_PROVIDER -> 2 + LocationManager.GPS_PROVIDER -> 1 + else -> 0 + } + @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) fun stop() { Timber.d("## LocationTracker. stop()") locationManager?.removeUpdates(this) callbacks.clear() debouncer.cancelAll() - hasGpsProviderLiveLocation = false + hasLocationFromGPSProvider = false + hasLocationFromFusedProvider = false } /** @@ -133,13 +145,23 @@ class LocationTracker @Inject constructor( } when (location.provider) { + LocationManager.FUSED_PROVIDER -> { + hasLocationFromFusedProvider = true + } LocationManager.GPS_PROVIDER -> { - hasGpsProviderLiveLocation = true + if (hasLocationFromFusedProvider) { + hasLocationFromGPSProvider = false + // Ignore this update + Timber.d("## LocationTracker. ignoring location from ${location.provider}, we have location from fused provider") + return + } else { + hasLocationFromGPSProvider = true + } } else -> { - if (hasGpsProviderLiveLocation) { + if (hasLocationFromFusedProvider || hasLocationFromGPSProvider) { // Ignore this update - Timber.d("## LocationTracker. ignoring location from ${location.provider}, we have gps live location") + Timber.d("## LocationTracker. ignoring location from ${location.provider}, we have location from GPS provider") return } } @@ -164,6 +186,10 @@ class LocationTracker @Inject constructor( override fun onProviderDisabled(provider: String) { Timber.d("## LocationTracker. onProviderDisabled: $provider") + when (provider) { + LocationManager.FUSED_PROVIDER -> hasLocationFromFusedProvider = false + LocationManager.GPS_PROVIDER -> hasLocationFromGPSProvider = false + } callbacks.forEach { it.onLocationProviderIsNotAvailable() } } From 45d3fe7c071f2e117bde851b662f3b158d00a2b0 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Fri, 3 Jun 2022 14:11:52 +0200 Subject: [PATCH 07/13] Call no provider available callback only providers list is empty --- .../location/LocationSharingService.kt | 2 +- .../location/LocationSharingViewModel.kt | 2 +- .../app/features/location/LocationTracker.kt | 50 ++++++++++++------- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt index 8b9a1c75ae..62aba9318c 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt @@ -198,7 +198,7 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { ) } - override fun onLocationProviderIsNotAvailable() { + override fun onNoLocationProviderAvailable() { stopForeground(true) stopSelf() } diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt index 9ca340f098..71f59c6fdf 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt @@ -182,7 +182,7 @@ class LocationSharingViewModel @AssistedInject constructor( } } - override fun onLocationProviderIsNotAvailable() { + override fun onNoLocationProviderAvailable() { _viewEvents.post(LocationSharingViewEvents.LocationNotAvailableError) } } diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 84836e53c0..c14ec214f0 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -41,8 +41,15 @@ class LocationTracker @Inject constructor( private val locationManager = context.getSystemService() interface Callback { + /** + * Called on every location update. + */ fun onLocationUpdate(locationData: LocationData) - fun onLocationProviderIsNotAvailable() + + /** + * Called when no location provider is available to request location updates. + */ + fun onNoLocationProviderAvailable() } private val callbacks = mutableListOf() @@ -56,24 +63,24 @@ class LocationTracker @Inject constructor( @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) fun start() { - Timber.d("## LocationTracker. start()") + Timber.d("start()") if (locationManager == null) { - callbacks.forEach { it.onLocationProviderIsNotAvailable() } - Timber.v("## LocationTracker. LocationManager is not available") + callbacks.forEach { it.onNoLocationProviderAvailable() } + Timber.v("LocationManager is not available") return } val providers = locationManager.allProviders if (providers.isEmpty()) { - callbacks.forEach { it.onLocationProviderIsNotAvailable() } - Timber.v("## LocationTracker. There is no location provider available") + callbacks.forEach { it.onNoLocationProviderAvailable() } + Timber.v("There is no location provider available") } else { // Take GPS first providers.sortedByDescending { getProviderPriority(it) } .mapNotNull { provider -> - Timber.d("## LocationTracker. track location using $provider") + Timber.d("track location using $provider") locationManager.requestLocationUpdates( provider, @@ -87,9 +94,9 @@ class LocationTracker @Inject constructor( .maxByOrNull { location -> location.time } ?.let { latestKnownLocation -> if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { - Timber.d("## LocationTracker. lastKnownLocation: $latestKnownLocation") + Timber.d("lastKnownLocation: $latestKnownLocation") } else { - Timber.d("## LocationTracker. lastKnownLocation: ${latestKnownLocation.provider}") + Timber.d("lastKnownLocation: ${latestKnownLocation.provider}") } notifyLocation(latestKnownLocation) } @@ -98,7 +105,7 @@ class LocationTracker @Inject constructor( /** * Compute the priority of the given provider name. - * @return an integer representing the priority: the higher the value, the higher the priority is + * @return an integer representing the priority: the higher the value, the higher the priority is. */ private fun getProviderPriority(provider: String): Int = when (provider) { LocationManager.FUSED_PROVIDER -> 2 @@ -108,7 +115,7 @@ class LocationTracker @Inject constructor( @RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION]) fun stop() { - Timber.d("## LocationTracker. stop()") + Timber.d("stop()") locationManager?.removeUpdates(this) callbacks.clear() debouncer.cancelAll() @@ -139,9 +146,9 @@ class LocationTracker @Inject constructor( override fun onLocationChanged(location: Location) { if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { - Timber.d("## LocationTracker. onLocationChanged: $location") + Timber.d("onLocationChanged: $location") } else { - Timber.d("## LocationTracker. onLocationChanged: ${location.provider}") + Timber.d("onLocationChanged: ${location.provider}") } when (location.provider) { @@ -152,7 +159,7 @@ class LocationTracker @Inject constructor( if (hasLocationFromFusedProvider) { hasLocationFromGPSProvider = false // Ignore this update - Timber.d("## LocationTracker. ignoring location from ${location.provider}, we have location from fused provider") + Timber.d("ignoring location from ${location.provider}, we have location from fused provider") return } else { hasLocationFromGPSProvider = true @@ -161,7 +168,7 @@ class LocationTracker @Inject constructor( else -> { if (hasLocationFromFusedProvider || hasLocationFromGPSProvider) { // Ignore this update - Timber.d("## LocationTracker. ignoring location from ${location.provider}, we have location from GPS provider") + Timber.d("ignoring location from ${location.provider}, we have location from GPS provider") return } } @@ -174,9 +181,9 @@ class LocationTracker @Inject constructor( private fun notifyLocation(location: Location) { if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) { - Timber.d("## LocationTracker. notify location: $location") + Timber.d("notify location: $location") } else { - Timber.d("## LocationTracker. notify location: ${location.provider}") + Timber.d("notify location: ${location.provider}") } val locationData = location.toLocationData() @@ -185,12 +192,17 @@ class LocationTracker @Inject constructor( } override fun onProviderDisabled(provider: String) { - Timber.d("## LocationTracker. onProviderDisabled: $provider") + Timber.d("onProviderDisabled: $provider") when (provider) { LocationManager.FUSED_PROVIDER -> hasLocationFromFusedProvider = false LocationManager.GPS_PROVIDER -> hasLocationFromGPSProvider = false } - callbacks.forEach { it.onLocationProviderIsNotAvailable() } + + locationManager?.allProviders + ?.takeIf { it.isEmpty() } + ?.let { + callbacks.forEach { it.onNoLocationProviderAvailable() } + } } private fun Location.toLocationData(): LocationData { From 260f73b0c247e1e1207be88e5094b7ca01f25a4d Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 7 Jun 2022 17:09:41 +0200 Subject: [PATCH 08/13] Adding unit tests for LocationTracker --- .../location/LocationSharingService.kt | 1 + .../location/LocationSharingViewModel.kt | 1 + .../app/features/location/LocationTracker.kt | 14 +- .../features/location/LocationTrackerTest.kt | 294 ++++++++++++++++++ .../im/vector/app/test/fakes/FakeContext.kt | 2 +- .../im/vector/app/test/fakes/FakeHandler.kt | 25 ++ .../app/test/fakes/FakeLocationManager.kt | 48 +++ 7 files changed, 378 insertions(+), 7 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeHandler.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeLocationManager.kt diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt index 62aba9318c..73a86fd04e 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt @@ -205,6 +205,7 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { private fun destroyMe() { locationTracker.removeCallback(this) + locationTracker.stop() timers.forEach { it.cancel() } timers.clear() stopSelf() diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt index 71f59c6fdf..f049a9400a 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt @@ -114,6 +114,7 @@ class LocationSharingViewModel @AssistedInject constructor( override fun onCleared() { super.onCleared() locationTracker.removeCallback(this) + locationTracker.stop() } override fun handle(action: LocationSharingAction) { diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index c14ec214f0..3e0d60f2c8 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -21,6 +21,7 @@ import android.content.Context import android.location.Location import android.location.LocationManager import androidx.annotation.RequiresPermission +import androidx.annotation.VisibleForTesting import androidx.core.content.getSystemService import androidx.core.location.LocationListenerCompat import im.vector.app.BuildConfig @@ -52,10 +53,14 @@ class LocationTracker @Inject constructor( fun onNoLocationProviderAvailable() } - private val callbacks = mutableListOf() + @VisibleForTesting + val callbacks = mutableListOf() - private var hasLocationFromFusedProvider = false - private var hasLocationFromGPSProvider = false + @VisibleForTesting + var hasLocationFromFusedProvider = false + + @VisibleForTesting + var hasLocationFromGPSProvider = false private var lastLocation: LocationData? = null @@ -139,9 +144,6 @@ class LocationTracker @Inject constructor( fun removeCallback(callback: Callback) { callbacks.remove(callback) - if (callbacks.size == 0) { - stop() - } } override fun onLocationChanged(location: Location) { diff --git a/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt b/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt new file mode 100644 index 0000000000..409647a813 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt @@ -0,0 +1,294 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.location + +import android.content.Context +import android.location.Location +import android.location.LocationManager +import im.vector.app.core.utils.Debouncer +import im.vector.app.core.utils.createBackgroundHandler +import im.vector.app.test.fakes.FakeContext +import im.vector.app.test.fakes.FakeHandler +import im.vector.app.test.fakes.FakeLocationManager +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.slot +import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic +import io.mockk.verify +import io.mockk.verifyOrder +import org.amshove.kluent.shouldBeEqualTo +import org.junit.After +import org.junit.Before +import org.junit.Test + +private const val A_LATITUDE = 1.2 +private const val A_LONGITUDE = 44.0 +private const val AN_ACCURACY = 5.0f + +class LocationTrackerTest { + + private val fakeHandler = FakeHandler() + + init { + mockkConstructor(Debouncer::class) + every { anyConstructed().cancelAll() } just runs + val runnable = slot() + every { anyConstructed().debounce(any(), MIN_TIME_TO_UPDATE_LOCATION_MILLIS, capture(runnable)) } answers { + runnable.captured.run() + true + } + mockkStatic("im.vector.app.core.utils.HandlerKt") + every { createBackgroundHandler(any()) } returns fakeHandler.instance + } + + private val fakeLocationManager = FakeLocationManager() + private val fakeContext = FakeContext().also { + it.givenService(Context.LOCATION_SERVICE, android.location.LocationManager::class.java, fakeLocationManager.instance) + } + + private val locationTracker = LocationTracker(fakeContext.instance) + + @Before + fun setUp() { + fakeLocationManager.givenRemoveUpdates(locationTracker) + } + + @After + fun tearDown() { + unmockkStatic("im.vector.app.core.utils.HandlerKt") + unmockkConstructor(Debouncer::class) + } + + @Test + fun `given available list of providers when starting then location updates are requested in priority order`() { + val providers = listOf(LocationManager.GPS_PROVIDER, LocationManager.FUSED_PROVIDER, LocationManager.NETWORK_PROVIDER) + mockAvailableProviders(providers) + + locationTracker.start() + + verifyOrder { + fakeLocationManager.instance.requestLocationUpdates( + LocationManager.FUSED_PROVIDER, + MIN_TIME_TO_UPDATE_LOCATION_MILLIS, + MIN_DISTANCE_TO_UPDATE_LOCATION_METERS, + locationTracker + ) + fakeLocationManager.instance.requestLocationUpdates( + LocationManager.GPS_PROVIDER, + MIN_TIME_TO_UPDATE_LOCATION_MILLIS, + MIN_DISTANCE_TO_UPDATE_LOCATION_METERS, + locationTracker + ) + fakeLocationManager.instance.requestLocationUpdates( + LocationManager.NETWORK_PROVIDER, + MIN_TIME_TO_UPDATE_LOCATION_MILLIS, + MIN_DISTANCE_TO_UPDATE_LOCATION_METERS, + locationTracker + ) + } + } + + @Test + fun `given available list of providers when list is empty then callbacks are notified`() { + val providers = emptyList() + val callback = mockCallback() + + locationTracker.addCallback(callback) + fakeLocationManager.givenActiveProviders(providers) + + locationTracker.start() + + verify { callback.onNoLocationProviderAvailable() } + locationTracker.removeCallback(callback) + } + + @Test + fun `when adding or removing a callback then it is added into or removed from the list of callbacks`() { + val callback = mockCallback() + + locationTracker.addCallback(callback) + + locationTracker.callbacks.size shouldBeEqualTo 1 + locationTracker.callbacks.first() shouldBeEqualTo callback + + locationTracker.removeCallback(callback) + + locationTracker.callbacks.size shouldBeEqualTo 0 + } + + @Test + fun `when location updates are received from fused provider then fused locations are taken in priority`() { + val providers = listOf(LocationManager.GPS_PROVIDER, LocationManager.FUSED_PROVIDER, LocationManager.NETWORK_PROVIDER) + mockAvailableProviders(providers) + val callback = mockCallback() + locationTracker.addCallback(callback) + locationTracker.start() + + val fusedLocation = mockLocation( + provider = LocationManager.FUSED_PROVIDER, + latitude = 1.0, + longitude = 3.0, + accuracy = 4f + ) + val gpsLocation = mockLocation( + provider = LocationManager.GPS_PROVIDER + ) + + val networkLocation = mockLocation( + provider = LocationManager.NETWORK_PROVIDER + ) + locationTracker.onLocationChanged(fusedLocation) + locationTracker.onLocationChanged(gpsLocation) + locationTracker.onLocationChanged(networkLocation) + + val expectedLocationData = LocationData( + latitude = 1.0, + longitude = 3.0, + uncertainty = 4.0 + ) + verify { callback.onLocationUpdate(expectedLocationData) } + verify { anyConstructed().debounce(any(), MIN_TIME_TO_UPDATE_LOCATION_MILLIS, any()) } + locationTracker.hasLocationFromFusedProvider shouldBeEqualTo true + locationTracker.hasLocationFromGPSProvider shouldBeEqualTo false + } + + @Test + fun `when location updates are received from gps provider then gps locations are taken if none are received from fused provider`() { + val providers = listOf(LocationManager.GPS_PROVIDER, LocationManager.FUSED_PROVIDER, LocationManager.NETWORK_PROVIDER) + mockAvailableProviders(providers) + val callback = mockCallback() + locationTracker.addCallback(callback) + locationTracker.start() + + val gpsLocation = mockLocation( + provider = LocationManager.GPS_PROVIDER, + latitude = 1.0, + longitude = 3.0, + accuracy = 4f + ) + + val networkLocation = mockLocation( + provider = LocationManager.NETWORK_PROVIDER + ) + locationTracker.onLocationChanged(gpsLocation) + locationTracker.onLocationChanged(networkLocation) + + val expectedLocationData = LocationData( + latitude = 1.0, + longitude = 3.0, + uncertainty = 4.0 + ) + verify { callback.onLocationUpdate(expectedLocationData) } + verify { anyConstructed().debounce(any(), MIN_TIME_TO_UPDATE_LOCATION_MILLIS, any()) } + locationTracker.hasLocationFromFusedProvider shouldBeEqualTo false + locationTracker.hasLocationFromGPSProvider shouldBeEqualTo true + } + + @Test + fun `when location updates are received from network provider then network locations are taken if none are received from fused or gps provider`() { + val providers = listOf(LocationManager.GPS_PROVIDER, LocationManager.FUSED_PROVIDER, LocationManager.NETWORK_PROVIDER) + mockAvailableProviders(providers) + val callback = mockCallback() + locationTracker.addCallback(callback) + locationTracker.start() + + val networkLocation = mockLocation( + provider = LocationManager.NETWORK_PROVIDER, + latitude = 1.0, + longitude = 3.0, + accuracy = 4f + ) + locationTracker.onLocationChanged(networkLocation) + + val expectedLocationData = LocationData( + latitude = 1.0, + longitude = 3.0, + uncertainty = 4.0 + ) + verify { callback.onLocationUpdate(expectedLocationData) } + verify { anyConstructed().debounce(any(), MIN_TIME_TO_UPDATE_LOCATION_MILLIS, any()) } + locationTracker.hasLocationFromFusedProvider shouldBeEqualTo false + locationTracker.hasLocationFromGPSProvider shouldBeEqualTo false + } + + @Test + fun `when requesting the last location then last location is notified via callback`() { + val providers = listOf(LocationManager.GPS_PROVIDER) + fakeLocationManager.givenActiveProviders(providers) + val lastLocation = mockLocation(provider = LocationManager.GPS_PROVIDER) + fakeLocationManager.givenLastLocationForProvider(provider = LocationManager.GPS_PROVIDER, location = lastLocation) + fakeLocationManager.givenRequestUpdatesForProvider(provider = LocationManager.GPS_PROVIDER, listener = locationTracker) + val callback = mockCallback() + locationTracker.addCallback(callback) + locationTracker.start() + + locationTracker.requestLastKnownLocation() + + val expectedLocationData = LocationData( + latitude = A_LATITUDE, + longitude = A_LONGITUDE, + uncertainty = AN_ACCURACY.toDouble() + ) + verify { callback.onLocationUpdate(expectedLocationData) } + } + + @Test + fun `when stopping then location updates are stopped and callbacks are cleared`() { + locationTracker.stop() + + verify { fakeLocationManager.instance.removeUpdates(locationTracker) } + verify { anyConstructed().cancelAll() } + locationTracker.callbacks.isEmpty() shouldBeEqualTo true + locationTracker.hasLocationFromGPSProvider shouldBeEqualTo false + locationTracker.hasLocationFromFusedProvider shouldBeEqualTo false + } + + private fun mockAvailableProviders(providers: List) { + fakeLocationManager.givenActiveProviders(providers) + providers.forEach { provider -> + fakeLocationManager.givenLastLocationForProvider(provider = provider, location = null) + fakeLocationManager.givenRequestUpdatesForProvider(provider = provider, listener = locationTracker) + } + } + + private fun mockCallback(): LocationTracker.Callback { + return mockk().also { + every { it.onNoLocationProviderAvailable() } just runs + every { it.onLocationUpdate(any()) } just runs + } + } + + private fun mockLocation( + provider: String, + latitude: Double = A_LATITUDE, + longitude: Double = A_LONGITUDE, + accuracy: Float = AN_ACCURACY + ): Location { + return mockk().also { + every { it.time } returns 123 + every { it.latitude } returns latitude + every { it.longitude } returns longitude + every { it.accuracy } returns accuracy + every { it.provider } returns provider + } + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt index eb491c9e0c..226f6458de 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt @@ -56,7 +56,7 @@ class FakeContext( givenService(Context.CONNECTIVITY_SERVICE, ConnectivityManager::class.java, connectivityManager.instance) } - private fun givenService(name: String, klass: Class, service: T) { + fun givenService(name: String, klass: Class, service: T) { every { instance.getSystemService(name) } returns service every { instance.getSystemService(klass) } returns service } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeHandler.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeHandler.kt new file mode 100644 index 0000000000..11340946ec --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeHandler.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import android.os.Handler +import io.mockk.mockk + +class FakeHandler { + + val instance = mockk() +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeLocationManager.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationManager.kt new file mode 100644 index 0000000000..30c30e6b4a --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationManager.kt @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import android.location.Location +import android.location.LocationListener +import android.location.LocationManager +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.runs + +class FakeLocationManager { + val instance = mockk() + + fun givenActiveProviders(providers: List) { + every { instance.allProviders } returns providers + } + + fun givenRequestUpdatesForProvider( + provider: String, + listener: LocationListener + ) { + every { instance.requestLocationUpdates(provider, any(), any(), listener) } just runs + } + + fun givenRemoveUpdates(listener: LocationListener) { + every { instance.removeUpdates(listener) } just runs + } + + fun givenLastLocationForProvider(provider: String, location: Location?) { + every { instance.getLastKnownLocation(provider) } returns location + } +} From 755d743b063d0773026b68ebe1846d9d10bc1035 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Thu, 9 Jun 2022 09:54:49 +0200 Subject: [PATCH 09/13] Encapsulate callbacks calls into try/catch block --- .../app/features/location/LocationTracker.kt | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 3e0d60f2c8..66005e71c6 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -71,16 +71,16 @@ class LocationTracker @Inject constructor( Timber.d("start()") if (locationManager == null) { - callbacks.forEach { it.onNoLocationProviderAvailable() } Timber.v("LocationManager is not available") + onNoLocationProviderAvailable() return } val providers = locationManager.allProviders if (providers.isEmpty()) { - callbacks.forEach { it.onNoLocationProviderAvailable() } Timber.v("There is no location provider available") + onNoLocationProviderAvailable() } else { // Take GPS first providers.sortedByDescending { getProviderPriority(it) } @@ -133,7 +133,7 @@ class LocationTracker @Inject constructor( * Please ensure adding a callback to receive the value. */ fun requestLastKnownLocation() { - lastLocation?.let { location -> callbacks.forEach { it.onLocationUpdate(location) } } + lastLocation?.let { locationData -> onLocationUpdate(locationData) } } fun addCallback(callback: Callback) { @@ -190,7 +190,7 @@ class LocationTracker @Inject constructor( val locationData = location.toLocationData() lastLocation = locationData - callbacks.forEach { it.onLocationUpdate(location.toLocationData()) } + onLocationUpdate(locationData) } override fun onProviderDisabled(provider: String) { @@ -203,10 +203,31 @@ class LocationTracker @Inject constructor( locationManager?.allProviders ?.takeIf { it.isEmpty() } ?.let { - callbacks.forEach { it.onNoLocationProviderAvailable() } + Timber.e("all providers have been disabled") + onNoLocationProviderAvailable() } } + private fun onNoLocationProviderAvailable() { + callbacks.forEach { + try { + it.onNoLocationProviderAvailable() + } catch (error: Exception) { + Timber.e(error, "error in onNoLocationProviderAvailable callback $it") + } + } + } + + private fun onLocationUpdate(locationData: LocationData) { + callbacks.forEach { + try { + it.onLocationUpdate(locationData) + } catch (error: Exception) { + Timber.e(error, "error in onLocationUpdate callback $it") + } + } + } + private fun Location.toLocationData(): LocationData { return LocationData(latitude, longitude, accuracy.toDouble()) } From 1b88cc39a93da5594f21b8573685e90dd200f321 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Thu, 9 Jun 2022 09:55:27 +0200 Subject: [PATCH 10/13] Use method reference when sorting providers --- .../java/im/vector/app/features/location/LocationTracker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 66005e71c6..c900fc7db5 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -83,7 +83,7 @@ class LocationTracker @Inject constructor( onNoLocationProviderAvailable() } else { // Take GPS first - providers.sortedByDescending { getProviderPriority(it) } + providers.sortedByDescending(::getProviderPriority) .mapNotNull { provider -> Timber.d("track location using $provider") From a1aa337edb771312a4503e7eee331e75327a5a08 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Thu, 9 Jun 2022 10:15:57 +0200 Subject: [PATCH 11/13] Stop location tracking only when callbacks list is empty to avoid non wanting stop --- .../features/location/LocationSharingService.kt | 1 - .../location/LocationSharingViewModel.kt | 1 - .../app/features/location/LocationTracker.kt | 3 +++ .../app/features/location/LocationTrackerTest.kt | 16 ++++++++++++---- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt index 73a86fd04e..62aba9318c 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingService.kt @@ -205,7 +205,6 @@ class LocationSharingService : VectorService(), LocationTracker.Callback { private fun destroyMe() { locationTracker.removeCallback(this) - locationTracker.stop() timers.forEach { it.cancel() } timers.clear() stopSelf() diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt index f049a9400a..71f59c6fdf 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt @@ -114,7 +114,6 @@ class LocationSharingViewModel @AssistedInject constructor( override fun onCleared() { super.onCleared() locationTracker.removeCallback(this) - locationTracker.stop() } override fun handle(action: LocationSharingAction) { diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index c900fc7db5..63508f30d7 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -144,6 +144,9 @@ class LocationTracker @Inject constructor( fun removeCallback(callback: Callback) { callbacks.remove(callback) + if (callbacks.size == 0) { + stop() + } } override fun onLocationChanged(location: Location) { diff --git a/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt b/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt index 409647a813..0a04644856 100644 --- a/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt +++ b/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt @@ -123,16 +123,24 @@ class LocationTrackerTest { @Test fun `when adding or removing a callback then it is added into or removed from the list of callbacks`() { - val callback = mockCallback() + val callback1 = mockCallback() + val callback2 = mockCallback() - locationTracker.addCallback(callback) + locationTracker.addCallback(callback1) + locationTracker.addCallback(callback2) + + locationTracker.callbacks.size shouldBeEqualTo 2 + locationTracker.callbacks.first() shouldBeEqualTo callback1 + locationTracker.callbacks[1] shouldBeEqualTo callback2 + + locationTracker.removeCallback(callback1) locationTracker.callbacks.size shouldBeEqualTo 1 - locationTracker.callbacks.first() shouldBeEqualTo callback - locationTracker.removeCallback(callback) + locationTracker.removeCallback(callback2) locationTracker.callbacks.size shouldBeEqualTo 0 + verify { locationTracker.stop() } } @Test From 162fd0a8403d60b8630967e9c99753312449fc8e Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Mon, 13 Jun 2022 11:35:44 +0200 Subject: [PATCH 12/13] Call unmockAll after each test --- .../features/location/LocationTrackerTest.kt | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt b/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt index 0a04644856..b0f3974226 100644 --- a/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt +++ b/vector/src/test/java/im/vector/app/features/location/LocationTrackerTest.kt @@ -31,8 +31,7 @@ import io.mockk.mockkConstructor import io.mockk.mockkStatic import io.mockk.runs import io.mockk.slot -import io.mockk.unmockkConstructor -import io.mockk.unmockkStatic +import io.mockk.unmockkAll import io.mockk.verify import io.mockk.verifyOrder import org.amshove.kluent.shouldBeEqualTo @@ -47,8 +46,15 @@ private const val AN_ACCURACY = 5.0f class LocationTrackerTest { private val fakeHandler = FakeHandler() + private val fakeLocationManager = FakeLocationManager() + private val fakeContext = FakeContext().also { + it.givenService(Context.LOCATION_SERVICE, android.location.LocationManager::class.java, fakeLocationManager.instance) + } - init { + private lateinit var locationTracker: LocationTracker + + @Before + fun setUp() { mockkConstructor(Debouncer::class) every { anyConstructed().cancelAll() } just runs val runnable = slot() @@ -58,24 +64,13 @@ class LocationTrackerTest { } mockkStatic("im.vector.app.core.utils.HandlerKt") every { createBackgroundHandler(any()) } returns fakeHandler.instance - } - - private val fakeLocationManager = FakeLocationManager() - private val fakeContext = FakeContext().also { - it.givenService(Context.LOCATION_SERVICE, android.location.LocationManager::class.java, fakeLocationManager.instance) - } - - private val locationTracker = LocationTracker(fakeContext.instance) - - @Before - fun setUp() { + locationTracker = LocationTracker(fakeContext.instance) fakeLocationManager.givenRemoveUpdates(locationTracker) } @After fun tearDown() { - unmockkStatic("im.vector.app.core.utils.HandlerKt") - unmockkConstructor(Debouncer::class) + unmockkAll() } @Test From dee5dfd187174ac9c156c1b672abb3ddcea3707d Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Mon, 13 Jun 2022 12:02:21 +0200 Subject: [PATCH 13/13] Add synchronized annotations to protect from concurrent access to callbacks --- .../im/vector/app/features/location/LocationTracker.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt index 63508f30d7..cdf13a7004 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationTracker.kt @@ -122,7 +122,9 @@ class LocationTracker @Inject constructor( fun stop() { Timber.d("stop()") locationManager?.removeUpdates(this) - callbacks.clear() + synchronized(this) { + callbacks.clear() + } debouncer.cancelAll() hasLocationFromGPSProvider = false hasLocationFromFusedProvider = false @@ -136,12 +138,14 @@ class LocationTracker @Inject constructor( lastLocation?.let { locationData -> onLocationUpdate(locationData) } } + @Synchronized fun addCallback(callback: Callback) { if (!callbacks.contains(callback)) { callbacks.add(callback) } } + @Synchronized fun removeCallback(callback: Callback) { callbacks.remove(callback) if (callbacks.size == 0) { @@ -211,6 +215,7 @@ class LocationTracker @Inject constructor( } } + @Synchronized private fun onNoLocationProviderAvailable() { callbacks.forEach { try { @@ -221,6 +226,7 @@ class LocationTracker @Inject constructor( } } + @Synchronized private fun onLocationUpdate(locationData: LocationData) { callbacks.forEach { try {