Have network/WiFi check in both PeriodicSyncWorker and OneTimeSyncWorker; add tests

- OneTimeSyncWorker is also started by sync framework, so it should take network restrictions into consideration
- add "manual" flag for manual syncs that ignore network restrictions
This commit is contained in:
Ricki Hirner 2024-03-15 11:30:54 +01:00
parent fe833759ee
commit a43dbb5cff
9 changed files with 79 additions and 64 deletions

View file

@ -7,9 +7,12 @@ package at.bitfire.davdroid.syncadapter
import android.accounts.Account
import android.accounts.AccountManager
import android.content.ContentResolver
import android.net.wifi.WifiInfo
import android.net.wifi.WifiManager
import android.provider.CalendarContract
import android.provider.ContactsContract
import android.util.Log
import androidx.core.content.getSystemService
import androidx.test.platform.app.InstrumentationRegistry
import androidx.work.Configuration
import androidx.work.testing.WorkManagerTestInitHelper
@ -18,12 +21,14 @@ import at.bitfire.davdroid.db.Credentials
import at.bitfire.davdroid.network.ConnectionUtils
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.ui.NotificationUtils
import at.bitfire.davdroid.util.PermissionUtils
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.every
import io.mockk.junit4.MockKRule
import io.mockk.mockk
import io.mockk.mockkObject
import io.mockk.spyk
import org.junit.After
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
@ -107,13 +112,39 @@ class BaseSyncWorkerTest {
@Test
fun testWifiConditionsMet_correctWifiSsid() {
// TODO: Write test
fun testCorrectWifiSsid_CorrectWiFiSsid() {
val accountSettings = AccountSettings(context, account)
mockkObject(accountSettings)
every { accountSettings.getSyncWifiOnlySSIDs() } returns listOf("SampleWiFi1","ConnectedWiFi")
mockkObject(PermissionUtils)
every { PermissionUtils.canAccessWifiSsid(any()) } returns true
val wifiManager = context.getSystemService<WifiManager>()!!
mockkObject(wifiManager)
every { wifiManager.connectionInfo } returns spyk<WifiInfo>().apply {
every { ssid } returns "ConnectedWiFi"
}
assertTrue(BaseSyncWorker.correctWifiSsid(context, accountSettings))
}
@Test
fun testWifiConditionsMet_wrongWifiSsid() {
// TODO: Write test
fun testCorrectWifiSsid_WrongWiFiSsid() {
val accountSettings = AccountSettings(context, account)
mockkObject(accountSettings)
every { accountSettings.getSyncWifiOnlySSIDs() } returns listOf("SampleWiFi1","SampleWiFi2")
mockkObject(PermissionUtils)
every { PermissionUtils.canAccessWifiSsid(any()) } returns true
val wifiManager = context.getSystemService<WifiManager>()!!
mockkObject(wifiManager)
every { wifiManager.connectionInfo } returns spyk<WifiInfo>().apply {
every { ssid } returns "ConnectedWiFi"
}
assertFalse(BaseSyncWorker.correctWifiSsid(context, accountSettings))
}
}

View file

@ -106,9 +106,9 @@ class PeriodicSyncWorkerTest {
// Run PeriodicSyncWorker as TestWorker
val inputData = workDataOf(
BaseSyncWorker.ARG_AUTHORITY to CalendarContract.AUTHORITY,
BaseSyncWorker.ARG_ACCOUNT_NAME to invalidAccount.name,
BaseSyncWorker.ARG_ACCOUNT_TYPE to invalidAccount.type
BaseSyncWorker.INPUT_AUTHORITY to CalendarContract.AUTHORITY,
BaseSyncWorker.INPUT_ACCOUNT_NAME to invalidAccount.name,
BaseSyncWorker.INPUT_ACCOUNT_TYPE to invalidAccount.type
)
// mock WorkManager to observe cancellation call

View file

@ -44,9 +44,12 @@ abstract class BaseSyncWorker(
companion object {
// common worker input parameters
const val ARG_ACCOUNT_NAME = "accountName"
const val ARG_ACCOUNT_TYPE = "accountType"
const val ARG_AUTHORITY = "authority"
const val INPUT_ACCOUNT_NAME = "accountName"
const val INPUT_ACCOUNT_TYPE = "accountType"
const val INPUT_AUTHORITY = "authority"
/** set to true for user-initiated sync that skips network checks */
const val INPUT_MANUAL = "manual"
/**
* How often this work will be retried to run after soft (network) errors.
@ -174,10 +177,10 @@ abstract class BaseSyncWorker(
override suspend fun doWork(): Result {
// ensure we got the required arguments
val account = Account(
inputData.getString(ARG_ACCOUNT_NAME) ?: throw IllegalArgumentException("$ARG_ACCOUNT_NAME required"),
inputData.getString(ARG_ACCOUNT_TYPE) ?: throw IllegalArgumentException("$ARG_ACCOUNT_TYPE required")
inputData.getString(INPUT_ACCOUNT_NAME) ?: throw IllegalArgumentException("$INPUT_ACCOUNT_NAME required"),
inputData.getString(INPUT_ACCOUNT_TYPE) ?: throw IllegalArgumentException("$INPUT_ACCOUNT_TYPE required")
)
val authority = inputData.getString(ARG_AUTHORITY) ?: throw IllegalArgumentException("$ARG_AUTHORITY required")
val authority = inputData.getString(INPUT_AUTHORITY) ?: throw IllegalArgumentException("$INPUT_AUTHORITY required")
val syncTag = commonTag(account, authority)
Logger.log.info("${javaClass.simpleName} called for $syncTag")
@ -200,12 +203,22 @@ abstract class BaseSyncWorker(
return Result.failure()
}
// check internet connection
val ignoreVpns = accountSettings.getIgnoreVpns()
val connectivityManager = applicationContext.getSystemService<ConnectivityManager>()!!
if (!ConnectionUtils.internetAvailable(connectivityManager, ignoreVpns)) {
Logger.log.info("WorkManager started SyncWorker without Internet connection. Aborting.")
return Result.failure()
if (inputData.getBoolean(INPUT_MANUAL, false))
Logger.log.info("Manual sync, skipping network checks")
else {
// check internet connection
val ignoreVpns = accountSettings.getIgnoreVpns()
val connectivityManager = applicationContext.getSystemService<ConnectivityManager>()!!
if (!ConnectionUtils.internetAvailable(connectivityManager, ignoreVpns)) {
Logger.log.info("WorkManager started SyncWorker without Internet connection. Aborting.")
return Result.success()
}
// check WiFi restriction
if (!wifiConditionsMet(applicationContext, accountSettings)) {
Logger.log.info("WiFi conditions not met. Won't run periodic sync.")
return Result.success()
}
}
return doSyncWork(account, authority, accountSettings)
@ -220,7 +233,7 @@ abstract class BaseSyncWorker(
authority: String,
accountSettings: AccountSettings
): Result = withContext(dispatcher) {
Logger.log.info("Running sync worker: account=$account, authority=$authority")
Logger.log.info("Running ${javaClass.name}: account=$account, authority=$authority")
// What are we going to sync? Select syncer based on authority
val syncer: Syncer = when (authority) {

View file

@ -71,11 +71,12 @@ class OneTimeSyncWorker @AssistedInject constructor(
fun enqueueAllAuthorities(
context: Context,
account: Account,
manual: Boolean = false,
@ArgResync resync: Int = NO_RESYNC,
upload: Boolean = false
) {
for (authority in SyncUtils.syncAuthorities(context))
enqueue(context, account, authority, resync = resync, upload = upload)
enqueue(context, account, authority, manual = manual, resync = resync, upload = upload)
}
/**
@ -83,6 +84,7 @@ class OneTimeSyncWorker @AssistedInject constructor(
*
* @param account account to sync
* @param authority authority to sync (for instance: [CalendarContract.AUTHORITY])
* @param manual user-initiated sync (ignores network checks)
* @param resync whether to request (full) re-synchronization or not
* @param upload see [ContentResolver.SYNC_EXTRAS_UPLOAD] used only for contacts sync
* and android 7 workaround
@ -92,14 +94,17 @@ class OneTimeSyncWorker @AssistedInject constructor(
context: Context,
account: Account,
authority: String,
manual: Boolean = false,
@ArgResync resync: Int = NO_RESYNC,
upload: Boolean = false
): String {
// Worker arguments
val argumentsBuilder = Data.Builder()
.putString(ARG_AUTHORITY, authority)
.putString(ARG_ACCOUNT_NAME, account.name)
.putString(ARG_ACCOUNT_TYPE, account.type)
.putString(INPUT_AUTHORITY, authority)
.putString(INPUT_ACCOUNT_NAME, account.name)
.putString(INPUT_ACCOUNT_TYPE, account.type)
if (manual)
argumentsBuilder.putBoolean(INPUT_MANUAL, true)
if (resync != NO_RESYNC)
argumentsBuilder.putInt(ARG_RESYNC, resync)
argumentsBuilder.putBoolean(ARG_UPLOAD, upload)
@ -158,9 +163,4 @@ class OneTimeSyncWorker @AssistedInject constructor(
return ForegroundInfo(NotificationUtils.NOTIFY_SYNC_EXPEDITED, notification)
}
override suspend fun doWork(): Result {
return super.doWork()
}
}

View file

@ -16,8 +16,6 @@ import androidx.work.Operation
import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager
import androidx.work.WorkerParameters
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.settings.AccountSettings
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import java.util.concurrent.TimeUnit
@ -66,9 +64,9 @@ class PeriodicSyncWorker @AssistedInject constructor(
*/
fun enable(context: Context, account: Account, authority: String, interval: Long, syncWifiOnly: Boolean): Operation {
val arguments = Data.Builder()
.putString(ARG_AUTHORITY, authority)
.putString(ARG_ACCOUNT_NAME, account.name)
.putString(ARG_ACCOUNT_TYPE, account.type)
.putString(INPUT_AUTHORITY, authority)
.putString(INPUT_ACCOUNT_NAME, account.name)
.putString(INPUT_ACCOUNT_TYPE, account.type)
.build()
val constraints = Constraints.Builder()
.setRequiredNetworkType(
@ -105,19 +103,4 @@ class PeriodicSyncWorker @AssistedInject constructor(
}
override suspend fun doSyncWork(
account: Account,
authority: String,
accountSettings: AccountSettings
): Result {
Logger.log.info("Running periodic sync for account=$account, authority=$authority")
if (!wifiConditionsMet(applicationContext, accountSettings)) {
Logger.log.info("WiFi conditions not met. Won't run periodic sync.")
return Result.failure()
}
return super.doSyncWork(account, authority, accountSettings)
}
}

View file

@ -413,7 +413,7 @@ class AccountsActivity: AppCompatActivity() {
// Enqueue sync worker for all accounts and authorities. Will sync once internet is available
for (account in allAccounts())
OneTimeSyncWorker.enqueueAllAuthorities(context, account)
OneTimeSyncWorker.enqueueAllAuthorities(context, account, manual = true)
}

View file

@ -179,7 +179,7 @@ class AccountActivity : AppCompatActivity() {
}
},
onSync = {
OneTimeSyncWorker.enqueueAllAuthorities(this, model.account)
OneTimeSyncWorker.enqueueAllAuthorities(this, model.account, manual = true)
},
onAccountSettings = {
val intent = Intent(this, AccountSettingsActivity::class.java)

View file

@ -285,7 +285,7 @@ class AccountModel @AssistedInject constructor(
}
// synchronize again
OneTimeSyncWorker.enqueueAllAuthorities(context, newAccount)
OneTimeSyncWorker.enqueueAllAuthorities(context, newAccount, manual = true)
}

View file

@ -73,18 +73,6 @@ object PermissionUtils {
locationAvailable
}
/**
* Whether this app declares the given permission (regardless of whether it has been granted or not).
*
* @param permission permission to check
*
* @return *true* if this app declares [permission] in the manifest; *false* otherwise
*/
fun declaresPermission(packageManager: PackageManager, permission: String): Boolean {
val info = packageManager.getPackageInfo(BuildConfig.APPLICATION_ID, PackageManager.GET_PERMISSIONS)
return info.requestedPermissions.contains(permission)
}
/**
* Checks whether at least one of the given permissions is granted.
*