Inject SyncDispatcher over Hilt (#784)

* Inject SyncDispatcher over Hilt

* Use setWorkerFactory for TestListenableWorkerBuilder

* Correctly inject SyncDispatcher over an AssistedFactory

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

@@ -15,6 +15,8 @@ import androidx.test.platform.app.InstrumentationRegistry
 import androidx.work.Configuration
 import androidx.work.ListenableWorker
 import androidx.work.WorkManager
+import androidx.work.WorkerFactory
+import androidx.work.WorkerParameters
 import androidx.work.testing.TestListenableWorkerBuilder
 import androidx.work.testing.WorkManagerTestInitHelper
 import androidx.work.workDataOf
@@ -23,6 +25,7 @@ import at.bitfire.davdroid.TestUtils.workScheduledOrRunning
 import at.bitfire.davdroid.db.Credentials
 import at.bitfire.davdroid.settings.AccountSettings
 import at.bitfire.davdroid.ui.NotificationUtils
+import dagger.assisted.AssistedFactory
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
 import io.mockk.junit4.MockKRule
@@ -36,6 +39,7 @@ import org.junit.Before
 import org.junit.BeforeClass
 import org.junit.Rule
 import org.junit.Test
+import javax.inject.Inject
 
 @HiltAndroidTest
 class PeriodicSyncWorkerTest {
@@ -74,11 +78,19 @@ class PeriodicSyncWorkerTest {
 
     }
 
+    @AssistedFactory
+    interface PeriodicSyncWorkerFactory {
+        fun create(appContext: Context, workerParams: WorkerParameters): PeriodicSyncWorker
+    }
+
     @get:Rule
     val hiltRule = HiltAndroidRule(this)
     @get:Rule
     val mockkRule = MockKRule(this)
 
+    @Inject
+    lateinit var syncWorkerFactory: PeriodicSyncWorkerFactory
+
     @Before
     fun inject() {
         hiltRule.inject()
@@ -116,7 +128,12 @@ class PeriodicSyncWorkerTest {
         mockkObject(workManager)
 
         // run test worker, expect failure
-        val testWorker = TestListenableWorkerBuilder<PeriodicSyncWorker>(context, inputData).build()
+        val testWorker = TestListenableWorkerBuilder<PeriodicSyncWorker>(context, inputData)
+            .setWorkerFactory(object: WorkerFactory() {
+                override fun createWorker(appContext: Context, workerClassName: String, workerParameters: WorkerParameters) =
+                    syncWorkerFactory.create(appContext, workerParameters)
+            })
+            .build()
         val result = runBlocking {
             testWorker.doWork()
         }

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

@@ -31,6 +31,7 @@ import at.bitfire.davdroid.ui.NotificationUtils.notifyIfPossible
 import at.bitfire.davdroid.ui.account.WifiPermissionsActivity
 import at.bitfire.davdroid.util.PermissionUtils
 import at.bitfire.ical4android.TaskProvider
+import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
@@ -41,7 +42,8 @@ import java.util.logging.Level
 
 abstract class BaseSyncWorker(
     appContext: Context,
-    val workerParams: WorkerParameters
+    private val workerParams: WorkerParameters,
+    private val syncDispatcher: CoroutineDispatcher,
 ) : CoroutineWorker(appContext, workerParams) {
 
     companion object {
@@ -178,7 +180,6 @@ abstract class BaseSyncWorker(
     }
 
 
-    private val dispatcher = SyncWorkDispatcher.getInstance(applicationContext)
     private val notificationManager = NotificationManagerCompat.from(applicationContext)
 
 
@@ -240,7 +241,7 @@ abstract class BaseSyncWorker(
         account: Account,
         authority: String,
         accountSettings: AccountSettings
-    ): Result = withContext(dispatcher) {
+    ): Result = withContext(syncDispatcher) {
         Logger.log.info("Running ${javaClass.name}: account=$account, authority=$authority")
 
         // What are we going to sync? Select syncer based on authority

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

@@ -40,8 +40,9 @@ import java.util.logging.Level
 @HiltWorker
 class OneTimeSyncWorker @AssistedInject constructor(
     @Assisted appContext: Context,
-    @Assisted workerParams: WorkerParameters
-) : BaseSyncWorker(appContext, workerParams) {
+    @Assisted workerParams: WorkerParameters,
+    syncDispatcher: SyncDispatcher
+) : BaseSyncWorker(appContext, workerParams, syncDispatcher.dispatcher) {
 
     companion object {
 

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

@@ -37,8 +37,9 @@ import java.util.concurrent.TimeUnit
 @HiltWorker
 class PeriodicSyncWorker @AssistedInject constructor(
     @Assisted appContext: Context,
-    @Assisted workerParams: WorkerParameters
-) : BaseSyncWorker(appContext, workerParams) {
+    @Assisted workerParams: WorkerParameters,
+    syncDispatcher: SyncDispatcher
+) : BaseSyncWorker(appContext, workerParams, syncDispatcher.dispatcher) {
 
     companion object {
 

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

@@ -4,37 +4,33 @@
 
 package at.bitfire.davdroid.syncadapter
 
-import android.content.Context
+import android.app.Application
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.asCoroutineDispatcher
 import java.util.concurrent.LinkedBlockingQueue
 import java.util.concurrent.ThreadFactory
 import java.util.concurrent.ThreadPoolExecutor
 import java.util.concurrent.TimeUnit
+import javax.inject.Inject
+import javax.inject.Singleton
 
-object SyncWorkDispatcher {
+/**
+ * Creates a [CoroutineDispatcher] with multiple threads that guarantees that the threads
+ * have set their contextClassLoader to the application context's class loader.
+ *
+ * We use our own dispatcher to
+ *
+ * - make sure that all threads have [Thread.getContextClassLoader] set, which is required for ical4j (because it uses [ServiceLoader]),
+ * - control the global number of sync threads.
+ */
+@Singleton
+class SyncDispatcher @Inject constructor(
+    context: Application
+) {
 
-    private var _dispatcher: CoroutineDispatcher? = null
+    val dispatcher = createDispatcher(context.classLoader)
 
-    /**
-     * We use our own dispatcher to
-     *
-     *   - make sure that all threads have [Thread.getContextClassLoader] set,
-     *     which is required for dav4jvm and ical4j (because they rely on [ServiceLoader]),
-     *   - control the global number of sync worker threads.
-     */
-    @Synchronized
-    fun getInstance(context: Context): CoroutineDispatcher {
-        // prefer cached work dispatcher
-        _dispatcher?.let { return it }
-
-        val newDispatcher = createDispatcher(context.applicationContext.classLoader)
-        _dispatcher = newDispatcher
-
-        return newDispatcher
-    }
-
-    private fun createDispatcher(classLoader: ClassLoader) =
+    private fun createDispatcher(classLoader: ClassLoader): CoroutineDispatcher =
         ThreadPoolExecutor(
             0, Runtime.getRuntime().availableProcessors(),
             10, TimeUnit.SECONDS, LinkedBlockingQueue(),