From 6ae5055f9c2e7025a3ae882a00ea7994fdd145da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 27 Jun 2024 14:26:48 +0200 Subject: [PATCH 1/8] fix: location sharing without gms when not moving [WPB-9724] --- app/build.gradle.kts | 2 + .../location/LocationPickerHelperFlavor.kt | 10 +- .../kotlin/com/wire/android/di/AppModule.kt | 9 + .../location/LocationPickerHelper.kt | 86 ++++++- .../location/LocationPickerHelperFlavor.kt | 15 +- .../LocationPickerHelperFlavorTest.kt | 194 +++++++++++++++ .../location/LocationPickerHelperTest.kt | 229 ++++++++++++++++++ gradle/libs.versions.toml | 3 + 8 files changed, 529 insertions(+), 19 deletions(-) create mode 100644 app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt create mode 100644 app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 067b4b33943..a5d193f7f5a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -201,6 +201,8 @@ dependencies { testImplementation(libs.okio.fakeFileSystem) testRuntimeOnly(libs.junit5.engine) testImplementation(libs.androidx.paging.testing) + testImplementation(libs.robolectric) + testRuntimeOnly(libs.junit.vintage.engine) // Acceptance/Functional tests dependencies androidTestImplementation(libs.androidx.test.runner) diff --git a/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt b/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt index f50538d7437..0343dbc321c 100644 --- a/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt +++ b/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt @@ -18,15 +18,19 @@ package com.wire.android.ui.home.messagecomposer.location import android.content.Context +import com.wire.android.di.ApplicationScope import javax.inject.Inject import javax.inject.Singleton +import kotlinx.coroutines.CoroutineScope @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, ) } } diff --git a/app/src/main/kotlin/com/wire/android/di/AppModule.kt b/app/src/main/kotlin/com/wire/android/di/AppModule.kt index c73e8ecf7e9..2d64387d68e 100644 --- a/app/src/main/kotlin/com/wire/android/di/AppModule.kt +++ b/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() } diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt index f66fa5aec8a..a951724be06 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt @@ -23,16 +23,38 @@ 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 geocoder: Geocoder, + 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, @@ -40,14 +62,16 @@ open class LocationPickerHelper @Inject constructor(@ApplicationContext val cont 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 + ) { + onSuccess(lastLocation.toGeoLocatedAddress()) // use last known location if present and not older than given limit + } else { + locationManager.requestCurrentLocationWithoutGms(onSuccess, onError) } } - locationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, 0, 0f, networkLocationListener) } else { AppJsonStyledLogger.log( level = KaliumLogLevel.WARN, @@ -61,8 +85,50 @@ 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 = Consumer { location -> + timeoutJob.cancel() + if (location != null) { + onSuccess(location.toGeoLocatedAddress()) + } else { + onError() + } + } + this.getCurrentLocation(LocationManager.FUSED_PROVIDER, cancellationSignal, executor, consumer) + } else { + val listener = LocationListener { location -> + timeoutJob.cancel() + onSuccess(location.toGeoLocatedAddress()) + } + 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) } + + private fun Location.toGeoLocatedAddress(): GeoLocatedAddress = + geocoder.getFromLocation(latitude, longitude, 1).orEmpty().let { addressList -> + GeoLocatedAddress(addressList.firstOrNull(), this) + } } + +data class LocationPickerParameters( + val lastLocationTimeLimit: Duration = 1.minutes, + val requestLocationTimeout: Duration = 10.seconds, +) diff --git a/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt b/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt index 829aa040835..c8b36f65ca7 100644 --- a/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt +++ b/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt @@ -24,13 +24,16 @@ import com.google.android.gms.location.LocationServices import com.google.android.gms.location.Priority import com.google.android.gms.tasks.CancellationTokenSource import com.wire.android.util.extension.isGoogleServicesAvailable +import kotlinx.coroutines.tasks.await import javax.inject.Inject import javax.inject.Singleton -import kotlinx.coroutines.tasks.await @Singleton -class LocationPickerHelperFlavor @Inject constructor(context: Context) : LocationPickerHelper(context) { - +class LocationPickerHelperFlavor @Inject constructor( + private val context: Context, + private val geocoder: Geocoder, + private val locationPickerHelper: LocationPickerHelper, +) { suspend fun getLocation(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) { if (context.isGoogleServicesAvailable()) { getLocationWithGms( @@ -38,7 +41,7 @@ class LocationPickerHelperFlavor @Inject constructor(context: Context) : Locatio onError = onError ) } else { - getLocationWithoutGms( + locationPickerHelper.getLocationWithoutGms( onSuccess = onSuccess, onError = onError ) @@ -51,11 +54,11 @@ class LocationPickerHelperFlavor @Inject constructor(context: Context) : Locatio */ @SuppressLint("MissingPermission") private suspend fun getLocationWithGms(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) { - if (isLocationServicesEnabled()) { + if (locationPickerHelper.isLocationServicesEnabled()) { 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() + val address = geocoder.getFromLocation(currentLocation.latitude, currentLocation.longitude, 1).orEmpty() onSuccess(GeoLocatedAddress(address.firstOrNull(), currentLocation)) } else { onError() diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt new file mode 100644 index 00000000000..c7c3823d102 --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt @@ -0,0 +1,194 @@ +/* + * 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.content.Context +import android.location.Address +import android.location.Geocoder +import android.location.Location +import android.location.LocationManager +import androidx.core.location.LocationManagerCompat +import com.google.android.gms.location.FusedLocationProviderClient +import com.google.android.gms.location.LocationServices +import com.google.android.gms.tasks.CancellationToken +import com.google.android.gms.tasks.CancellationTokenSource +import com.google.android.gms.tasks.Task +import com.wire.android.config.CoroutineTestExtension +import com.wire.android.util.extension.isGoogleServicesAvailable +import io.mockk.MockKAnnotations +import io.mockk.MockKMatcherScope +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic +import kotlinx.coroutines.tasks.await +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith + +@ExtendWith(CoroutineTestExtension::class) +class LocationPickerHelperFlavorTest { + + private val dispatcher = StandardTestDispatcher() + + @Test + fun `given GMS not available, when getting location, then execute getLocationWithoutGms`() = + runTest(dispatcher) { + // given + val (arrangement, locationPickerHelperFlavor) = Arrangement() + .withIsGoogleServicesAvailable(false) + .arrange() + + // when + locationPickerHelperFlavor.getLocation(onSuccess = arrangement.onSuccess, onError = arrangement.onError) + + // then + coVerify(exactly = 1) { + arrangement.locationPickerHelper.getLocationWithoutGms(any(), any()) + } + } + + @Test + fun `given GMS available and location service disabled, when getting location, then execute onError`() = + runTest(dispatcher) { + // given + val (arrangement, locationPickerHelperFlavor) = Arrangement() + .withIsGoogleServicesAvailable(true) + .withIsLocationServiceEnabled(false) + .arrange() + + // when + locationPickerHelperFlavor.getLocation(onSuccess = arrangement.onSuccess, onError = arrangement.onError) + + // then + coVerify(exactly = 0) { + arrangement.onSuccess(any()) + } + coVerify(exactly = 1) { + arrangement.onError() + } + } + + @Test + fun `given GMS available and location service enabled, when getting location, then execute onSuccess with location`() = + runTest(dispatcher) { + // given + val location = mockLocation(latitude = 1.0, longitude = 1.0) + val address = mockAddress(addressFirstLine = "address") + val (arrangement, locationPickerHelperFlavor) = Arrangement() + .withIsGoogleServicesAvailable(true) + .withIsLocationServiceEnabled(true) + .withGetCurrentLocation(location) + .withGeocoderGetFromLocation(1.0, 1.0, address) + .arrange() + + // when + locationPickerHelperFlavor.getLocation(onSuccess = arrangement.onSuccess, onError = arrangement.onError) + + // then + coVerify(exactly = 1) { + arrangement.onSuccess(match { it.location == location && it.address == address }) + } + coVerify(exactly = 0) { + arrangement.onError() + } + } + + private fun MockKMatcherScope.match(expected: GeoLocatedAddress): GeoLocatedAddress = + match { + it.location.latitude == expected.location.latitude && + it.location.longitude == expected.location.longitude && + it.address?.getAddressLine(0) == expected.address?.getAddressLine(0) + } + + inner class Arrangement { + + @MockK + private lateinit var context: Context + + @MockK + private lateinit var locationManager: LocationManager + + @MockK + private lateinit var fusedLocationProviderClient: FusedLocationProviderClient + + @MockK + private lateinit var geocoder: Geocoder + + @MockK + lateinit var locationPickerHelper: LocationPickerHelper + + val onSuccess: (GeoLocatedAddress) -> Unit = mockk() + val onError: () -> Unit = mockk() + + private val locationPickerHelperFlavor by lazy { + LocationPickerHelperFlavor( + context = context, + geocoder = geocoder, + locationPickerHelper = locationPickerHelper, + ) + } + + init { + MockKAnnotations.init(this, relaxUnitFun = true) + mockkStatic(LocationServices::getFusedLocationProviderClient) + coEvery { LocationServices.getFusedLocationProviderClient(context) } returns fusedLocationProviderClient + coEvery { context.getSystemService(Context.LOCATION_SERVICE) } returns locationManager + coEvery { onSuccess(any()) } returns Unit + coEvery { onError() } returns Unit + coEvery { locationPickerHelper.getLocationWithoutGms(any(), any()) } returns Unit + } + + fun withIsGoogleServicesAvailable(isAvailable: Boolean) = apply { + mockkStatic("com.wire.android.util.extension.GoogleServicesKt") + coEvery { context.isGoogleServicesAvailable() } returns isAvailable + } + + fun withIsLocationServiceEnabled(isEnabled: Boolean) = apply { + coEvery { locationPickerHelper.isLocationServicesEnabled() } returns isEnabled + } + + fun withGetCurrentLocation(location: Location) = apply { + val task: Task = mockk() + mockkStatic("kotlinx.coroutines.tasks.TasksKt") + coEvery { task.await() } returns location + mockkConstructor(CancellationTokenSource::class) + coEvery { anyConstructed().token } returns mockk() + coEvery { fusedLocationProviderClient.getCurrentLocation(any(), any()) } returns task + } + + fun withGeocoderGetFromLocation(latitude: Double, longitude: Double, result: Address) = apply { + coEvery { geocoder.getFromLocation(latitude, longitude, 1) } returns listOf(result) + } + + fun arrange() = this to locationPickerHelperFlavor + } + + fun mockLocation(latitude: Double, longitude: Double) = mockk().let { + coEvery { it.latitude } returns latitude + coEvery { it.longitude } returns longitude + it + } + + fun mockAddress(addressFirstLine: String) = mockk
().also { + coEvery { it.getAddressLine(0) } returns addressFirstLine + } +} diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt new file mode 100644 index 00000000000..8e46d002f88 --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt @@ -0,0 +1,229 @@ +/* + * 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.app.Application +import android.content.Context +import android.location.Address +import android.location.Geocoder +import android.location.Location +import android.location.LocationManager +import android.os.Build +import android.os.Looper +import android.os.SystemClock +import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.amshove.kluent.internal.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowLocationManager +import org.robolectric.shadows.ShadowSystemClock +import java.util.Locale +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import kotlin.time.Duration +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.nanoseconds +import kotlin.time.Duration.Companion.seconds +import kotlin.time.toJavaDuration + +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class) +class LocationPickerHelperTest { + + private val dispatcher = StandardTestDispatcher() + + @Test + @Config(sdk = [Build.VERSION_CODES.P, Build.VERSION_CODES.R]) + fun `given last location not too old, then emit last location`() = runTest(dispatcher) { + // given + val resultHandler = ResultHandler() + val (arrangement, locationPickerHelper) = Arrangement().arrange() + val location = Location(LocationManager.FUSED_PROVIDER).apply { + latitude = 1.0 + longitude = 1.0 + elapsedRealtimeNanos = arrangement.lastLocationTimeLimit.inWholeNanoseconds - 1.seconds.inWholeNanoseconds + time = dispatcher.scheduler.currentTime - elapsedRealtimeNanos.nanoseconds.inWholeMilliseconds + bearing = 0f + } + arrangement.updateLocation(location) + shadowOf(Looper.getMainLooper()).idle() + + // when + locationPickerHelper.getLocationWithoutGms(resultHandler::onSuccess, resultHandler::onError) + + // then + resultHandler.assert(expectedErrorCount = 0, expectedLocations = listOf(GeoLocatedAddress(arrangement.address, location))) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.P, Build.VERSION_CODES.R]) + fun `given last location too old, when new location comes before timeout, then emit new location`() = runTest(dispatcher) { + // given + val resultHandler = ResultHandler() + val (arrangement, locationPickerHelper) = Arrangement().arrange() + val lastLocation = Location(LocationManager.FUSED_PROVIDER).apply { + latitude = 1.0 + longitude = 1.0 + elapsedRealtimeNanos = arrangement.lastLocationTimeLimit.inWholeNanoseconds + 1.seconds.inWholeNanoseconds + time = dispatcher.scheduler.currentTime - elapsedRealtimeNanos.nanoseconds.inWholeMilliseconds + } + arrangement.updateLocation(lastLocation) + shadowOf(Looper.getMainLooper()).idle() + + // when + locationPickerHelper.getLocationWithoutGms(resultHandler::onSuccess, resultHandler::onError) + advanceTimeBy(arrangement.requestLocationTimeout - 1.seconds) + + val newLocation = Location(LocationManager.FUSED_PROVIDER).apply { + latitude = 2.0 + longitude = 2.0 + elapsedRealtimeNanos = 0 + time = dispatcher.scheduler.currentTime + } + arrangement.updateLocation(newLocation) + shadowOf(Looper.getMainLooper()).idle() + + // then + resultHandler.assert(expectedErrorCount = 0, expectedLocations = listOf(GeoLocatedAddress(arrangement.address, newLocation))) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.P, Build.VERSION_CODES.R]) + fun `given last location too old, when new location times out, then emit error`() = runTest(dispatcher) { + // given + val resultHandler = ResultHandler() + val (arrangement, locationPickerHelper) = Arrangement().arrange() + val lastLocation = Location(LocationManager.FUSED_PROVIDER).apply { + latitude = 1.0 + longitude = 1.0 + elapsedRealtimeNanos = arrangement.lastLocationTimeLimit.inWholeNanoseconds + 1.seconds.inWholeNanoseconds + time = dispatcher.scheduler.currentTime - elapsedRealtimeNanos.nanoseconds.inWholeMilliseconds + } + arrangement.updateLocation(lastLocation) + shadowOf(Looper.getMainLooper()).idle() + + // when + locationPickerHelper.getLocationWithoutGms(resultHandler::onSuccess, resultHandler::onError) + advanceTimeBy(arrangement.requestLocationTimeout + 1.seconds) + + // then + resultHandler.assert(expectedErrorCount = 1, expectedLocations = emptyList()) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.R]) // null location can happen only for R and above after some timeout + fun `given no last location, when new location is null, then emit error`() = runTest(dispatcher) { + // given + val resultHandler = ResultHandler() + val (arrangement, locationPickerHelper) = Arrangement( + requestLocationTimeout = 1.minutes + ) + .arrange() + + // when + locationPickerHelper.getLocationWithoutGms(resultHandler::onSuccess, resultHandler::onError) + + shadowOf(Looper.getMainLooper()) + .idleFor(shadowOf(Looper.getMainLooper()).lastScheduledTaskTime) // this is how the timeout is simulated + + // then + resultHandler.assert(expectedErrorCount = 1, expectedLocations = emptyList()) + } + + inner class Arrangement( + val lastLocationTimeLimit: Duration = 1.minutes, + val requestLocationTimeout: Duration = 10.seconds + ) { + private val context: Context = ApplicationProvider.getApplicationContext() + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + dispatcher) + private val geocoder: Geocoder = Geocoder(context) + private val locationManager: LocationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager + val address = Address(Locale.getDefault()).apply { + setAddressLine(0, "address") + } + + private val locationPickerHelper by lazy { + LocationPickerHelper( + context = context, + scope = scope, + currentTimestampProvider = dispatcher.scheduler::currentTime, + geocoder = geocoder, + parameters = LocationPickerParameters( + lastLocationTimeLimit = lastLocationTimeLimit, + requestLocationTimeout = requestLocationTimeout + ), + ) + } + + init { + shadowOf(geocoder).setFromLocation(listOf(address)) + shadowOf(locationManager).setProviderEnabled(LocationManager.FUSED_PROVIDER, true) + + // update the system clock to not start with 0 and prevent from having negative time for some locations + dispatcher.scheduler.advanceTimeBy(1.hours) + ShadowSystemClock.advanceBy(1.hours.toJavaDuration()) + } + + fun updateLocation(location: Location) = apply { + shadowOf(locationManager).simulateLocation(location) + } + + + fun arrange() = this to locationPickerHelper + } + + class ResultHandler { + private val locations = ConcurrentLinkedQueue() + private val errorCount = AtomicInteger(0) + + fun onSuccess(geoLocatedAddress: GeoLocatedAddress) { + locations.add(geoLocatedAddress) + } + + fun onError() { + errorCount.incrementAndGet() + } + + fun assert(expectedErrorCount: Int = 0, expectedLocations: List = emptyList()) { + assertEquals(expectedErrorCount, errorCount.get()) + assertEquals(expectedLocations.size, locations.size) + locations.forEachIndexed { index, geoLocatedAddress -> + assertEquals(expectedLocations[index].address, geoLocatedAddress.address) + assertEquals(expectedLocations[index].location.latitude, geoLocatedAddress.location.latitude) + assertEquals(expectedLocations[index].location.longitude, geoLocatedAddress.location.longitude) + assertEquals(expectedLocations[index].location.time, geoLocatedAddress.location.time) + } + } + } +} + + diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 85a672045d9..d04831736fc 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -90,6 +90,7 @@ kluent = "1.73" mockk = "1.13.9" okio = "3.6.0" turbine = "1.0.0" +robolectric = "4.12.2" [plugins] # 3rd Party plugins @@ -245,3 +246,5 @@ mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockk" } mockk-core = { module = "io.mockk:mockk", version.ref = "mockk" } okio-fakeFileSystem = { module = "com.squareup.okio:okio-fakefilesystem", version.ref = "okio" } turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" } +robolectric = { module = "org.robolectric:robolectric", version.ref = "robolectric" } +junit-vintage-engine = { module = "org.junit.vintage:junit-vintage-engine", version.ref = "junit5" } # needed for tests that use Robolectric because it doesn't yet support JUnit5 From 854d25df99074e0651d81bbb4a91791fce122c5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 27 Jun 2024 14:27:32 +0200 Subject: [PATCH 2/8] remove unused imports --- .../location/LocationPickerHelperFlavorTest.kt | 1 - .../messagecomposer/location/LocationPickerHelperTest.kt | 7 ------- 2 files changed, 8 deletions(-) diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt index c7c3823d102..6a2ec276cfb 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt @@ -22,7 +22,6 @@ import android.location.Address import android.location.Geocoder import android.location.Location import android.location.LocationManager -import androidx.core.location.LocationManagerCompat import com.google.android.gms.location.FusedLocationProviderClient import com.google.android.gms.location.LocationServices import com.google.android.gms.tasks.CancellationToken diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt index 8e46d002f88..1b3998ce3b8 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt @@ -25,31 +25,24 @@ import android.location.Location import android.location.LocationManager import android.os.Build import android.os.Looper -import android.os.SystemClock import androidx.test.core.app.ApplicationProvider import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.advanceTimeBy -import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest -import kotlinx.coroutines.test.setMain import org.amshove.kluent.internal.assertEquals import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config -import org.robolectric.shadows.ShadowLocationManager import org.robolectric.shadows.ShadowSystemClock import java.util.Locale import java.util.concurrent.ConcurrentLinkedQueue -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger import kotlin.time.Duration import kotlin.time.Duration.Companion.hours -import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.nanoseconds import kotlin.time.Duration.Companion.seconds From 1892010775287d8e7b92629bccdd44e8e98b973d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 27 Jun 2024 14:39:15 +0200 Subject: [PATCH 3/8] remove unused function --- .../location/LocationPickerHelperFlavorTest.kt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt index 6a2ec276cfb..bdfcebea8a0 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt @@ -30,7 +30,6 @@ import com.google.android.gms.tasks.Task import com.wire.android.config.CoroutineTestExtension import com.wire.android.util.extension.isGoogleServicesAvailable import io.mockk.MockKAnnotations -import io.mockk.MockKMatcherScope import io.mockk.coEvery import io.mockk.coVerify import io.mockk.impl.annotations.MockK @@ -111,13 +110,6 @@ class LocationPickerHelperFlavorTest { } } - private fun MockKMatcherScope.match(expected: GeoLocatedAddress): GeoLocatedAddress = - match { - it.location.latitude == expected.location.latitude && - it.location.longitude == expected.location.longitude && - it.address?.getAddressLine(0) == expected.address?.getAddressLine(0) - } - inner class Arrangement { @MockK From cb2ab9dd6e5d8fa79c904d1ebe499182215f8478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 27 Jun 2024 14:49:14 +0200 Subject: [PATCH 4/8] fix detekt issues --- .../messagecomposer/location/LocationPickerHelperFlavor.kt | 3 --- .../messagecomposer/location/LocationPickerHelperTest.kt | 7 ++----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt b/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt index 0343dbc321c..fe07d70e78d 100644 --- a/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt +++ b/app/src/foss/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt @@ -17,11 +17,8 @@ */ package com.wire.android.ui.home.messagecomposer.location -import android.content.Context -import com.wire.android.di.ApplicationScope import javax.inject.Inject import javax.inject.Singleton -import kotlinx.coroutines.CoroutineScope @Singleton class LocationPickerHelperFlavor @Inject constructor( diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt index 1b3998ce3b8..d3bb6e8c5f7 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt @@ -153,8 +153,8 @@ class LocationPickerHelperTest { } inner class Arrangement( - val lastLocationTimeLimit: Duration = 1.minutes, - val requestLocationTimeout: Duration = 10.seconds + val lastLocationTimeLimit: Duration = 1.minutes, + val requestLocationTimeout: Duration = 10.seconds ) { private val context: Context = ApplicationProvider.getApplicationContext() private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + dispatcher) @@ -190,7 +190,6 @@ class LocationPickerHelperTest { shadowOf(locationManager).simulateLocation(location) } - fun arrange() = this to locationPickerHelper } @@ -218,5 +217,3 @@ class LocationPickerHelperTest { } } } - - From f8815aeab9eabd2a60e15bb5709efd25a5abfe27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 27 Jun 2024 17:15:06 +0200 Subject: [PATCH 5/8] change all other unit tests to use junit5 --- .../EditGuestAccessViewModel.kt | 2 + .../wire/android/mapper/UserTypeMapperTest.kt | 6 +- .../android/migration/MigrationMapperTest.kt | 2 +- .../EditGuestAccessViewModelTest.kt | 86 +++++++++++++------ 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModel.kt index 7082c01184a..322d8623ec1 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModel.kt @@ -123,6 +123,8 @@ class EditGuestAccessViewModel @Inject constructor( conversationDetailsFlow, isSelfAdminFlow ) { conversationDetails, isSelfAnAdmin -> + isSelfAnAdmin to conversationDetails + }.collect { (isSelfAnAdmin, conversationDetails) -> val isGuestAllowed = conversationDetails.conversation.isGuestAllowed() || conversationDetails.conversation.isNonTeamMemberAllowed() diff --git a/app/src/test/kotlin/com/wire/android/mapper/UserTypeMapperTest.kt b/app/src/test/kotlin/com/wire/android/mapper/UserTypeMapperTest.kt index 07ccf3a4dcc..54718bc081c 100644 --- a/app/src/test/kotlin/com/wire/android/mapper/UserTypeMapperTest.kt +++ b/app/src/test/kotlin/com/wire/android/mapper/UserTypeMapperTest.kt @@ -21,7 +21,7 @@ package com.wire.android.mapper import com.wire.android.ui.home.conversationslist.model.Membership import com.wire.kalium.logic.data.user.type.UserType import org.amshove.kluent.internal.assertEquals -import org.junit.Test +import org.junit.jupiter.api.Test class UserTypeMapperTest { @@ -46,9 +46,9 @@ 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.None, result) + assertEquals(Membership.Standard, result) } } diff --git a/app/src/test/kotlin/com/wire/android/migration/MigrationMapperTest.kt b/app/src/test/kotlin/com/wire/android/migration/MigrationMapperTest.kt index 9790587c059..654bf2d5b89 100644 --- a/app/src/test/kotlin/com/wire/android/migration/MigrationMapperTest.kt +++ b/app/src/test/kotlin/com/wire/android/migration/MigrationMapperTest.kt @@ -21,8 +21,8 @@ import com.wire.android.config.CoroutineTestExtension import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.user.UserId import org.amshove.kluent.internal.assertEquals -import org.junit.Test import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith @ExtendWith(CoroutineTestExtension::class) diff --git a/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt index 82515173b0a..00f7a97e2de 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt @@ -22,12 +22,17 @@ package com.wire.android.ui.home.conversations.details.editguestaccess import androidx.lifecycle.SavedStateHandle import com.wire.android.config.CoroutineTestExtension +import com.wire.android.config.NavigationTestExtension import com.wire.android.config.TestDispatcherProvider import com.wire.android.framework.TestConversation +import com.wire.android.framework.TestConversationDetails +import com.wire.android.ui.home.conversations.details.participants.model.ConversationParticipantsData import com.wire.android.ui.home.conversations.details.participants.usecase.ObserveParticipantsForConversationUseCase import com.wire.android.ui.navArgs import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.NetworkFailure +import com.wire.kalium.logic.configuration.GuestRoomLinkStatus +import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.feature.conversation.ObserveConversationDetailsUseCase import com.wire.kalium.logic.feature.conversation.UpdateConversationAccessRoleUseCase import com.wire.kalium.logic.feature.conversation.guestroomlink.CanCreatePasswordProtectedLinksUseCase @@ -37,22 +42,27 @@ import com.wire.kalium.logic.feature.conversation.guestroomlink.ObserveGuestRoom import com.wire.kalium.logic.feature.conversation.guestroomlink.RevokeGuestRoomLinkResult import com.wire.kalium.logic.feature.conversation.guestroomlink.RevokeGuestRoomLinkUseCase import com.wire.kalium.logic.feature.user.guestroomlink.ObserveGuestRoomLinkFeatureFlagUseCase +import com.wire.kalium.logic.functional.Either +import io.mockk.MockKAnnotations import io.mockk.coEvery import io.mockk.coVerify -import io.mockk.every import io.mockk.impl.annotations.MockK import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.consumeAsFlow +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.amshove.kluent.internal.assertEquals -import org.junit.Before -import org.junit.Test +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith -// TODO test not working, fix it @OptIn(ExperimentalCoroutinesApi::class) -@ExtendWith(CoroutineTestExtension::class) +@ExtendWith(CoroutineTestExtension::class, NavigationTestExtension::class) class EditGuestAccessViewModelTest { + val dispatcher = TestDispatcherProvider() + @MockK private lateinit var savedStateHandle: SavedStateHandle @@ -65,9 +75,6 @@ class EditGuestAccessViewModelTest { @MockK lateinit var observeConversationMembers: ObserveParticipantsForConversationUseCase - @MockK - lateinit var updateConversationAccessRole: UpdateConversationAccessRoleUseCase - @MockK lateinit var generateGuestRoomLink: GenerateGuestRoomLinkUseCase @@ -85,33 +92,50 @@ class EditGuestAccessViewModelTest { private lateinit var editGuestAccessViewModel: EditGuestAccessViewModel - @Before + private val conversationDetailsChannel = Channel(capacity = Channel.UNLIMITED) + + @BeforeEach fun setUp() { + MockKAnnotations.init(this, relaxUnitFun = true) + coEvery { savedStateHandle.navArgs() } returns EditGuestAccessNavArgs( + conversationId = TestConversation.ID, + editGuessAccessParams = EditGuestAccessParams( + isGuestAccessAllowed = true, + isServicesAllowed = true, + isUpdatingGuestAccessAllowed = true + ) + ) + coEvery { + observeConversationDetails(any()) + } returns conversationDetailsChannel.consumeAsFlow() + coEvery { + observeConversationMembers(any()) + } returns flowOf(ConversationParticipantsData()) + coEvery { + observeGuestRoomLink(any()) + } returns flowOf(Either.Right(null)) + coEvery { + observeGuestRoomLinkFeatureFlag() + } returns flowOf(GuestRoomLinkStatus(null, null)) + editGuestAccessViewModel = EditGuestAccessViewModel( - dispatcher = TestDispatcherProvider(), observeConversationDetails = observeConversationDetails, observeConversationMembers = observeConversationMembers, - updateConversationAccessRole = updateConversationAccessRole, + updateConversationAccessRole = updateConversationAccessRoleUseCase, generateGuestRoomLink = generateGuestRoomLink, revokeGuestRoomLink = revokeGuestRoomLink, observeGuestRoomLink = observeGuestRoomLink, savedStateHandle = savedStateHandle, observeGuestRoomLinkFeatureFlag = observeGuestRoomLinkFeatureFlag, - canCreatePasswordProtectedLinks = canCreatePasswordProtectedLinks - ) - every { savedStateHandle.navArgs() } returns EditGuestAccessNavArgs( - conversationId = TestConversation.ID, - editGuessAccessParams = EditGuestAccessParams( - isGuestAccessAllowed = true, - isServicesAllowed = true, - isUpdatingGuestAccessAllowed = true - ) + canCreatePasswordProtectedLinks = canCreatePasswordProtectedLinks, + dispatcher = dispatcher, ) + conversationDetailsChannel.trySend(ObserveConversationDetailsUseCase.Result.Success(TestConversationDetails.GROUP)) } @Test fun `given updateConversationAccessRole use case runs successfully, when trying to enable guest access, then enable guest access`() = - runTest { + runTest(dispatcher.default()) { editGuestAccessViewModel.editGuestAccessState = editGuestAccessViewModel.editGuestAccessState.copy(isGuestAccessAllowed = false) coEvery { updateConversationAccessRoleUseCase(any(), any(), any()) @@ -163,7 +187,7 @@ class EditGuestAccessViewModelTest { } @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()) { coEvery { generateGuestRoomLink.invoke(any(), any()) } returns GenerateGuestRoomLinkResult.Success @@ -176,7 +200,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()) { coEvery { generateGuestRoomLink(any(), any()) } returns GenerateGuestRoomLinkResult.Failure(NetworkFailure.NoNetworkConnection(null)) @@ -190,7 +214,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()) { coEvery { revokeGuestRoomLink(any()) } returns RevokeGuestRoomLinkResult.Success @@ -204,7 +228,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()) { coEvery { revokeGuestRoomLink(any()) } returns RevokeGuestRoomLinkResult.Failure(CoreFailure.MissingClientRegistration) @@ -220,11 +244,19 @@ class EditGuestAccessViewModelTest { @Test fun `given updateConversationAccessRole use case runs successfully, when trying to disable guest access, then disable guest access`() = - runTest { + runTest(dispatcher.default()) { editGuestAccessViewModel.editGuestAccessState = editGuestAccessViewModel.editGuestAccessState.copy(isGuestAccessAllowed = true) coEvery { updateConversationAccessRoleUseCase(any(), any(), any()) - } returns UpdateConversationAccessRoleUseCase.Result.Success + } coAnswers { + val accessRoles = secondArg>() + val newConversationDetails = TestConversationDetails.GROUP.copy( + conversation = TestConversationDetails.GROUP.conversation.copy(accessRole = accessRoles.toList()) + ) + // mock emitting updated conversation details with new access roles + conversationDetailsChannel.send(ObserveConversationDetailsUseCase.Result.Success(newConversationDetails)) + UpdateConversationAccessRoleUseCase.Result.Success + } editGuestAccessViewModel.onGuestDialogConfirm() From 8adf4228ab1b6942caeec2be5d66fad7d605a25e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 27 Jun 2024 17:50:29 +0200 Subject: [PATCH 6/8] fix UncaughtExceptionsBeforeTest --- .../details/editguestaccess/EditGuestAccessViewModelTest.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt index 00f7a97e2de..6134137016f 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/conversations/details/editguestaccess/EditGuestAccessViewModelTest.kt @@ -117,6 +117,9 @@ class EditGuestAccessViewModelTest { coEvery { observeGuestRoomLinkFeatureFlag() } returns flowOf(GuestRoomLinkStatus(null, null)) + coEvery { + canCreatePasswordProtectedLinks() + } returns true editGuestAccessViewModel = EditGuestAccessViewModel( observeConversationDetails = observeConversationDetails, From 64077c899488b51cbf218283419535973909b119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 28 Jun 2024 10:54:59 +0200 Subject: [PATCH 7/8] create helper for geocoder and handle exceptions --- .../location/GeocoderHelper.kt | 36 +++++++ .../location/LocationPickerHelper.kt | 15 +-- .../location/LocationPickerHelperFlavor.kt | 6 +- .../location/GeocoderHelperTest.kt | 97 +++++++++++++++++++ .../LocationPickerHelperFlavorTest.kt | 27 +++--- .../location/LocationPickerHelperTest.kt | 3 +- 6 files changed, 155 insertions(+), 29 deletions(-) create mode 100644 app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt create mode 100644 app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelperTest.kt diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt new file mode 100644 index 00000000000..5ff587b03ae --- /dev/null +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt @@ -0,0 +1,36 @@ +/* + * 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) { + + fun getGeoLocatedAddress(location: Location): GeoLocatedAddress = + try { + geocoder.getFromLocation(location.latitude, location.longitude, 1).orEmpty() + } catch (e: Exception) { + emptyList() + }.let { addressList -> + GeoLocatedAddress(addressList.firstOrNull(), location) + } +} diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt index a951724be06..5836283a942 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelper.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 android.location.Location import android.location.LocationListener import android.location.LocationManager @@ -49,7 +48,7 @@ class LocationPickerHelper @Inject constructor( @ApplicationContext private val context: Context, @ApplicationScope private val scope: CoroutineScope, private val currentTimestampProvider: CurrentTimestampProvider, - private val geocoder: Geocoder, + private val geocoderHelper: GeocoderHelper, private val parameters: LocationPickerParameters, ) { @@ -67,7 +66,8 @@ class LocationPickerHelper @Inject constructor( lastLocation != null && currentTimestampProvider() - lastLocation.time <= parameters.lastLocationTimeLimit.inWholeMilliseconds ) { - onSuccess(lastLocation.toGeoLocatedAddress()) // use last known location if present and not older than given limit + // use last known location if present and not older than given limit + onSuccess(geocoderHelper.getGeoLocatedAddress(lastLocation)) } else { locationManager.requestCurrentLocationWithoutGms(onSuccess, onError) } @@ -98,7 +98,7 @@ class LocationPickerHelper @Inject constructor( val consumer: Consumer = Consumer { location -> timeoutJob.cancel() if (location != null) { - onSuccess(location.toGeoLocatedAddress()) + onSuccess(geocoderHelper.getGeoLocatedAddress(location)) } else { onError() } @@ -107,7 +107,7 @@ class LocationPickerHelper @Inject constructor( } else { val listener = LocationListener { location -> timeoutJob.cancel() - onSuccess(location.toGeoLocatedAddress()) + onSuccess(geocoderHelper.getGeoLocatedAddress(location)) } cancellationSignal.setOnCancelListener { this.removeUpdates(listener) @@ -121,11 +121,6 @@ class LocationPickerHelper @Inject constructor( val locationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager return LocationManagerCompat.isLocationEnabled(locationManager) } - - private fun Location.toGeoLocatedAddress(): GeoLocatedAddress = - geocoder.getFromLocation(latitude, longitude, 1).orEmpty().let { addressList -> - GeoLocatedAddress(addressList.firstOrNull(), this) - } } data class LocationPickerParameters( diff --git a/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt b/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt index c8b36f65ca7..15ba9467775 100644 --- a/app/src/nonfree/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavor.kt +++ b/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,7 +30,7 @@ import javax.inject.Singleton @Singleton class LocationPickerHelperFlavor @Inject constructor( private val context: Context, - private val geocoder: Geocoder, + private val geocoderHelper: GeocoderHelper, private val locationPickerHelper: LocationPickerHelper, ) { suspend fun getLocation(onSuccess: (GeoLocatedAddress) -> Unit, onError: () -> Unit) { @@ -58,8 +57,7 @@ class LocationPickerHelperFlavor @Inject constructor( val locationProvider = LocationServices.getFusedLocationProviderClient(context) val currentLocation = locationProvider.getCurrentLocation(Priority.PRIORITY_HIGH_ACCURACY, CancellationTokenSource().token).await() - val address = geocoder.getFromLocation(currentLocation.latitude, currentLocation.longitude, 1).orEmpty() - onSuccess(GeoLocatedAddress(address.firstOrNull(), currentLocation)) + onSuccess(geocoderHelper.getGeoLocatedAddress(currentLocation)) } else { onError() } diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelperTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelperTest.kt new file mode 100644 index 00000000000..23cd5603817 --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelperTest.kt @@ -0,0 +1,97 @@ +/* + * 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.Address +import android.location.Geocoder +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.test.runTest +import okio.IOException +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class GeocoderHelperTest { + + @Test + fun `given non-null result, when getting geocoder address, then return result with address`() = runTest { + // given + val location = mockLocation(latitude = 1.0, longitude = 1.0) + val address = mockAddress(addressFirstLine = "address") + val (_, geocoderHelper) = Arrangement() + .withGetFromLocation(1.0, 1.0, address) + .arrange() + + // when + val result = geocoderHelper.getGeoLocatedAddress(location) + + // then + assertEquals(address, result.address) + } + + @Test + fun `given empty result, when getting geocoder address, then return result without address`() = runTest { + // given + val location = mockLocation(latitude = 1.0, longitude = 1.0) + val (_, geocoderHelper) = Arrangement() + .withGetFromLocation(1.0, 1.0, null) + .arrange() + + // when + val result = geocoderHelper.getGeoLocatedAddress(location) + + // then + assertEquals(null, result.address) + } + + @Test + fun `given failure, when getting geocoder address, then return result without address`() = runTest { + // given + val location = mockLocation(latitude = 1.0, longitude = 1.0) + val (_, geocoderHelper) = Arrangement() + .withGetFromLocationFailure() + .arrange() + + // when + val result = geocoderHelper.getGeoLocatedAddress(location) + + // then + assertEquals(null, result.address) + } + + inner class Arrangement { + + @MockK + lateinit var geocoder: Geocoder + + init { + MockKAnnotations.init(this, relaxUnitFun = true) + } + + fun withGetFromLocation(latitude: Double, longitude: Double, result: Address?) = apply { + coEvery { geocoder.getFromLocation(latitude, longitude, 1) } returns listOfNotNull(result) + } + + fun withGetFromLocationFailure() = apply { + coEvery { geocoder.getFromLocation(any(), any(), any()) } throws IOException() + } + + fun arrange() = this to GeocoderHelper(geocoder) + } +} diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt index bdfcebea8a0..467b8ad282a 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperFlavorTest.kt @@ -19,7 +19,6 @@ package com.wire.android.ui.home.messagecomposer.location import android.content.Context import android.location.Address -import android.location.Geocoder import android.location.Location import android.location.LocationManager import com.google.android.gms.location.FusedLocationProviderClient @@ -95,7 +94,7 @@ class LocationPickerHelperFlavorTest { .withIsGoogleServicesAvailable(true) .withIsLocationServiceEnabled(true) .withGetCurrentLocation(location) - .withGeocoderGetFromLocation(1.0, 1.0, address) + .withGetGeoLocatedAddress(location, address) .arrange() // when @@ -122,7 +121,7 @@ class LocationPickerHelperFlavorTest { private lateinit var fusedLocationProviderClient: FusedLocationProviderClient @MockK - private lateinit var geocoder: Geocoder + private lateinit var geocoderHelper: GeocoderHelper @MockK lateinit var locationPickerHelper: LocationPickerHelper @@ -133,7 +132,7 @@ class LocationPickerHelperFlavorTest { private val locationPickerHelperFlavor by lazy { LocationPickerHelperFlavor( context = context, - geocoder = geocoder, + geocoderHelper = geocoderHelper, locationPickerHelper = locationPickerHelper, ) } @@ -166,20 +165,20 @@ class LocationPickerHelperFlavorTest { coEvery { fusedLocationProviderClient.getCurrentLocation(any(), any()) } returns task } - fun withGeocoderGetFromLocation(latitude: Double, longitude: Double, result: Address) = apply { - coEvery { geocoder.getFromLocation(latitude, longitude, 1) } returns listOf(result) + fun withGetGeoLocatedAddress(location: Location, result: Address) = apply { + coEvery { geocoderHelper.getGeoLocatedAddress(location) } returns GeoLocatedAddress(result, location) } fun arrange() = this to locationPickerHelperFlavor } +} - fun mockLocation(latitude: Double, longitude: Double) = mockk().let { - coEvery { it.latitude } returns latitude - coEvery { it.longitude } returns longitude - it - } +fun mockLocation(latitude: Double, longitude: Double) = mockk().let { + coEvery { it.latitude } returns latitude + coEvery { it.longitude } returns longitude + it +} - fun mockAddress(addressFirstLine: String) = mockk
().also { - coEvery { it.getAddressLine(0) } returns addressFirstLine - } +fun mockAddress(addressFirstLine: String) = mockk
().also { + coEvery { it.getAddressLine(0) } returns addressFirstLine } diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt index d3bb6e8c5f7..bcc30d16b97 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/location/LocationPickerHelperTest.kt @@ -159,6 +159,7 @@ class LocationPickerHelperTest { private val context: Context = ApplicationProvider.getApplicationContext() private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + dispatcher) private val geocoder: Geocoder = Geocoder(context) + private val geocoderHelper: GeocoderHelper = GeocoderHelper(geocoder) private val locationManager: LocationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager val address = Address(Locale.getDefault()).apply { setAddressLine(0, "address") @@ -169,7 +170,7 @@ class LocationPickerHelperTest { context = context, scope = scope, currentTimestampProvider = dispatcher.scheduler::currentTime, - geocoder = geocoder, + geocoderHelper = geocoderHelper, parameters = LocationPickerParameters( lastLocationTimeLimit = lastLocationTimeLimit, requestLocationTimeout = requestLocationTimeout From 0789dc7afd25a2bd3f444ca6f80f384c7119c7c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Fri, 28 Jun 2024 11:02:37 +0200 Subject: [PATCH 8/8] fix detekt --- .../android/ui/home/messagecomposer/location/GeocoderHelper.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt index 5ff587b03ae..b738d2d65d9 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/location/GeocoderHelper.kt @@ -25,6 +25,7 @@ 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()