More correct usage of expedited workers (#566)

* Add foreground notification type to expedited workers (required for Android 14)

* Make SyncWorker a long-running worker

* Don't use expedited SyncWorker for everything; handle foreground service launch restriction

* AddressBookSyncer: only request expedited for sub-jobs when parent job is expedited, too

* RefreshCollectionsWorker is not long-running -> no foreground service type needed

* Fix tests

* Don't use foreground service type in ForegroundInfo

* Make SyncWorker not long-running

Author: rfc2822

app/src/androidTestOse/kotlin/at/bitfire/davdroid/syncadapter/SyncWorkerTest.kt

@@ -72,7 +72,7 @@ class SyncWorkerTest {
 
     @Test
     fun testEnqueue_enqueuesWorker() {
-        SyncWorker.enqueue(context, account, CalendarContract.AUTHORITY)
+        SyncWorker.enqueue(context, account, CalendarContract.AUTHORITY, true)
         val workerName = SyncWorker.workerName(account, CalendarContract.AUTHORITY)
         assertTrue(workScheduledOrRunningOrSuccessful(context, workerName))
     }

app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt

@@ -84,6 +84,10 @@ import kotlin.collections.*
  *  - adds resources if new ones are detected
  *  - removes resources if not found 40x (delete locally)
  *
+ * Expedited: yes (always initiated by user)
+ *
+ * Long-running: no
+ *
  * @throws IllegalArgumentException when there's no service with the given service ID
  */
 @HiltWorker
@@ -577,4 +581,4 @@ class RefreshCollectionsWorker @AssistedInject constructor(
         }
     }
 
-}
+}

app/src/main/kotlin/at/bitfire/davdroid/syncadapter/AddressBookSyncer.kt

@@ -30,7 +30,10 @@ import java.util.logging.Level
 /**
  * Sync logic for address books
  */
-class AddressBookSyncer(context: Context): Syncer(context) {
+class AddressBookSyncer(
+    context: Context,
+    private val expedited: Boolean
+) : Syncer(context) {
 
     @EntryPoint
     @InstallIn(SingletonComponent::class)
@@ -53,7 +56,7 @@ class AddressBookSyncer(context: Context): Syncer(context) {
             if (updateLocalAddressBooks(account, syncResult))
                 for (addressBookAccount in LocalAddressBook.findAll(context, null, account).map { it.account }) {
                     Logger.log.log(Level.INFO, "Running sync for address book", addressBookAccount)
-                    SyncWorker.enqueue(context, addressBookAccount, ContactsContract.AUTHORITY)
+                    SyncWorker.enqueue(context, addressBookAccount, ContactsContract.AUTHORITY, expedited = expedited)
                 }
         } catch (e: Exception) {
             Logger.log.log(Level.SEVERE, "Couldn't sync address books", e)

app/src/main/kotlin/at/bitfire/davdroid/syncadapter/PeriodicSyncWorker.kt

@@ -32,6 +32,10 @@ import java.util.concurrent.TimeUnit
  * The different periodic sync workers each carry a unique work name composed of the account and
  * authority which they are responsible for. For each account there will be multiple dedicated periodic
  * sync workers for each authority. See [PeriodicSyncWorker.workerName] for more information.
+ *
+ * Deferrable: yes (PeriodicWorkRequest)
+ *
+ * Long-running: no
  */
 @HiltWorker
 class PeriodicSyncWorker @AssistedInject constructor(
@@ -121,7 +125,7 @@ class PeriodicSyncWorker @AssistedInject constructor(
 
         // Just request immediate sync
         Logger.log.info("Requesting immediate sync")
-        SyncWorker.enqueue(applicationContext, account, authority)
+        SyncWorker.enqueue(applicationContext, account, authority, expedited = false)
         return Result.success()
     }
 }

app/src/main/kotlin/at/bitfire/davdroid/syncadapter/SyncAdapterServices.kt

@@ -79,7 +79,7 @@ abstract class SyncAdapterService: Service() {
             }
 
             Logger.log.fine("Sync framework now starting SyncWorker")
-            val workerName = SyncWorker.enqueue(context, account, authority, upload = upload)
+            val workerName = SyncWorker.enqueue(context, account, authority, expedited = true, upload = upload)
 
             // Block the onPerformSync method to simulate an ongoing sync
             Logger.log.fine("Blocking sync framework until SyncWorker finishes")

app/src/main/kotlin/at/bitfire/davdroid/syncadapter/SyncWorker.kt

@@ -63,13 +63,17 @@ import java.util.logging.Level
  *
  * By enqueuing this worker ([SyncWorker.enqueue]) a sync will be started immediately (as soon as
  * possible). Currently, there are three scenarios starting a sync:
+ *
  * 1) *manual sync*: User presses an in-app sync button and enqueues this worker directly.
  * 2) *periodic sync*: User defines time interval to sync in app settings. The [PeriodicSyncWorker] runs
  * in the background and enqueues this worker when due.
  * 3) *content-triggered sync*: User changes a calendar event, task or contact, or presses a sync
  * button in one of the responsible apps. The [SyncAdapterService] is notified of this and enqueues
  * this worker.
  *
+ * Expedited: when run manually
+ *
+ * Long-running: no
  */
 @HiltWorker
 class SyncWorker @AssistedInject constructor(
@@ -84,6 +88,9 @@ class SyncWorker @AssistedInject constructor(
         internal const val ARG_ACCOUNT_TYPE = "accountType"
         internal const val ARG_AUTHORITY = "authority"
 
+        /** Boolean. Set to `true` when the job was requested as expedited job. */
+        private const val ARG_EXPEDITED = "expedited"
+
         private const val ARG_UPLOAD = "upload"
 
         private const val ARG_RESYNC = "resync"
@@ -129,7 +136,7 @@ class SyncWorker @AssistedInject constructor(
             upload: Boolean = false
         ) {
             for (authority in SyncUtils.syncAuthorities(context))
-                enqueue(context, account, authority, resync, upload)
+                enqueue(context, account, authority, expedited = true, resync = resync, upload = upload)
         }
 
         /**
@@ -146,6 +153,7 @@ class SyncWorker @AssistedInject constructor(
             context: Context,
             account: Account,
             authority: String,
+            expedited: Boolean,
             @ArgResync resync: Int = NO_RESYNC,
             upload: Boolean = false
         ): String {
@@ -154,6 +162,8 @@ class SyncWorker @AssistedInject constructor(
                 .putString(ARG_AUTHORITY, authority)
                 .putString(ARG_ACCOUNT_NAME, account.name)
                 .putString(ARG_ACCOUNT_TYPE, account.type)
+            if (expedited)
+                argumentsBuilder.putBoolean(ARG_EXPEDITED, true)
             if (resync != NO_RESYNC)
                 argumentsBuilder.putInt(ARG_RESYNC, resync)
             argumentsBuilder.putBoolean(ARG_UPLOAD, upload)
@@ -165,7 +175,6 @@ class SyncWorker @AssistedInject constructor(
             val workRequest = OneTimeWorkRequestBuilder<SyncWorker>()
                 .addTag(workerName(account, authority))
                 .setInputData(argumentsBuilder.build())
-                .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
                 .setBackoffCriteria(
                     BackoffPolicy.EXPONENTIAL,
                     WorkRequest.DEFAULT_BACKOFF_DELAY_MILLIS,   // 30 sec
@@ -179,17 +188,20 @@ class SyncWorker @AssistedInject constructor(
                         addTag(workerName(mainAccount, authority))
                     }
                 }
-                .build()
+
+            if (expedited)
+                workRequest.setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
 
             // enqueue and start syncing
             val name = workerName(account, authority)
-            Logger.log.log(Level.INFO, "Enqueueing unique worker: $name, with tags: ${workRequest.tags}")
+            val request = workRequest.build()
+            Logger.log.log(Level.INFO, "Enqueueing unique worker: $name, expedited = $expedited, tags = ${request.tags}")
             WorkManager.getInstance(context).enqueueUniqueWork(
                 name,
                 ExistingWorkPolicy.KEEP,    // If sync is already running, just continue.
                                             // Existing retried work will not be replaced (for instance when
                                             // PeriodicSyncWorker enqueues another scheduled sync).
-                workRequest
+                request
             )
             return name
         }
@@ -305,8 +317,9 @@ class SyncWorker @AssistedInject constructor(
             inputData.getString(ARG_ACCOUNT_TYPE) ?: throw IllegalArgumentException("$ARG_ACCOUNT_TYPE required")
         )
         val authority = inputData.getString(ARG_AUTHORITY) ?: throw IllegalArgumentException("$ARG_AUTHORITY required")
+        val expedited = inputData.getBoolean(ARG_EXPEDITED, false)
 
-        // Check internet connection
+        // check internet connection
         val ignoreVpns = AccountSettings(applicationContext, account).getIgnoreVpns()
         val connectivityManager = applicationContext.getSystemService<ConnectivityManager>()!!
         if (!internetAvailable(connectivityManager, ignoreVpns)) {
@@ -319,7 +332,7 @@ class SyncWorker @AssistedInject constructor(
         // What are we going to sync? Select syncer based on authority
         val syncer: Syncer = when (authority) {
             applicationContext.getString(R.string.address_books_authority) ->
-                AddressBookSyncer(applicationContext)
+                AddressBookSyncer(applicationContext, expedited)
             CalendarContract.AUTHORITY ->
                 CalendarSyncer(applicationContext)
             ContactsContract.AUTHORITY ->
@@ -438,6 +451,7 @@ class SyncWorker @AssistedInject constructor(
             .setCategory(NotificationCompat.CATEGORY_STATUS)
             .setOngoing(true)
             .setPriority(NotificationCompat.PRIORITY_LOW)
+            .setForegroundServiceBehavior(NotificationCompat.FOREGROUND_SERVICE_DEFERRED)
             .build()
         return ForegroundInfo(NotificationUtils.NOTIFY_SYNC_EXPEDITED, notification)
     }

app/src/main/kotlin/at/bitfire/davdroid/ui/account/SettingsActivity.kt

@@ -603,7 +603,7 @@ class SettingsActivity: AppCompatActivity() {
          */
         private fun resync(authority: String, fullResync: Boolean) {
             val resync = if (fullResync) SyncWorker.FULL_RESYNC else SyncWorker.RESYNC
-            SyncWorker.enqueue(getApplication(), account, authority, resync)
+            SyncWorker.enqueue(getApplication(), account, authority, expedited = true, resync = resync)
         }
 
     }