Skip to content

SyncWorker: use Provider for lazy injection #925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 19, 2024
Merged

SyncWorker: use Provider for lazy injection #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a790dd6
SyncWorker: use Provider for lazy injection
rfc2822 Jul 19, 2024
1419522
Use Provider to lazyily inject SyncAdapter
rfc2822 Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions app/src/androidTest/kotlin/at/bitfire/davdroid/sync/worker/TestSyncWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,15 @@ package at.bitfire.davdroid.sync.worker

import android.content.Context
import androidx.work.WorkerParameters
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.sync.SyncConditions
import at.bitfire.davdroid.ui.NotificationRegistry
import dagger.hilt.android.qualifiers.ApplicationContext
import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import javax.inject.Inject

class TestSyncWorker @Inject constructor(
@ApplicationContext context: Context,
accountSettingsFactory: AccountSettings.Factory,
notificationRegistry: NotificationRegistry,
syncConditionsFactory: SyncConditions.Factory
@ApplicationContext context: Context
): BaseSyncWorker(
context,
workerParams = mockk<WorkerParameters>(),
accountSettingsFactory,
notificationRegistry,
syncConditionsFactory,
syncDispatcher = Dispatchers.Main
)
26 changes: 15 additions & 11 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/SyncAdapterServices.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,24 @@ import at.bitfire.davdroid.R
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.sync.worker.OneTimeSyncWorker
import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.android.AndroidEntryPoint
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.cancel
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import java.util.logging.Level
import javax.inject.Inject
import javax.inject.Provider

abstract class SyncAdapterService: Service() {

@EntryPoint
@InstallIn(SingletonComponent::class)
interface SyncAdapterServiceEntryPoint {
fun syncAdapter(): SyncAdapter
}
@Inject
lateinit var syncAdapter: Provider<SyncAdapter>

override fun onBind(intent: Intent?): IBinder {
val entryPoint = EntryPointAccessors.fromApplication<SyncAdapterServiceEntryPoint>(this)
return entryPoint.syncAdapter().syncAdapterBinder
return syncAdapter.get().syncAdapterBinder
}

/**
Expand Down Expand Up @@ -150,8 +144,18 @@ abstract class SyncAdapterService: Service() {
}

// exported sync adapter services; we need a separate class for each authority

@AndroidEntryPoint
class CalendarsSyncAdapterService: SyncAdapterService()

@AndroidEntryPoint
class ContactsSyncAdapterService: SyncAdapterService()

@AndroidEntryPoint
class JtxSyncAdapterService: SyncAdapterService()

@AndroidEntryPoint
class OpenTasksSyncAdapterService: SyncAdapterService()

@AndroidEntryPoint
class TasksOrgSyncAdapterService: SyncAdapterService()
80 changes: 43 additions & 37 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/worker/BaseSyncWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import androidx.work.WorkQuery
import androidx.work.WorkerParameters
import at.bitfire.davdroid.InvalidAccountException
import at.bitfire.davdroid.R
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.sync.AddressBookSyncer
import at.bitfire.davdroid.sync.CalendarSyncer
Expand All @@ -30,24 +29,20 @@ import at.bitfire.davdroid.sync.Syncer
import at.bitfire.davdroid.sync.TaskSyncer
import at.bitfire.davdroid.ui.NotificationRegistry
import at.bitfire.ical4android.TaskProvider
import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.components.SingletonComponent
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.runInterruptible
import kotlinx.coroutines.withContext
import java.util.Collections
import java.util.logging.Logger
import javax.inject.Inject
import javax.inject.Provider

abstract class BaseSyncWorker(
context: Context,
private val workerParams: WorkerParameters,
private val accountSettingsFactory: AccountSettings.Factory,
private val notificationRegistry: NotificationRegistry,
private val syncConditionsFactory: SyncConditions.Factory,
private val syncDispatcher: CoroutineDispatcher
) : CoroutineWorker(context, workerParams) {

Expand Down Expand Up @@ -124,19 +119,29 @@ abstract class BaseSyncWorker(

}

// We don't inject the Syncers in our constructor because that would generate
// every syncer object regardless of whether it's even used for the synced authority (useless overhead).
@EntryPoint
@InstallIn(SingletonComponent::class)
interface BaseSyncWorkerEntryPoint {
fun addressBookSyncer(): AddressBookSyncer
fun calendarSyncer(): CalendarSyncer
fun jtxSyncer(): JtxSyncer
fun taskSyncer(): TaskSyncer
}
private val entryPoint = EntryPointAccessors.fromApplication<BaseSyncWorkerEntryPoint>(context)
@Inject
lateinit var accountSettingsFactory: AccountSettings.Factory

@Inject
lateinit var addressBookSyncer: Provider<AddressBookSyncer>

@Inject
lateinit var calendarSyncer: Provider<CalendarSyncer>

@Inject
lateinit var jtxSyncer: Provider<JtxSyncer>

@Inject
lateinit var logger: Logger

@Inject
lateinit var notificationRegistry: NotificationRegistry

@Inject
lateinit var syncConditionsFactory: SyncConditions.Factory

private val notificationManager = NotificationManagerCompat.from(applicationContext)
@Inject
lateinit var taskSyncer: Provider<TaskSyncer>


override suspend fun doWork(): Result {
Expand All @@ -148,10 +153,10 @@ abstract class BaseSyncWorker(
val authority = inputData.getString(INPUT_AUTHORITY) ?: throw IllegalArgumentException("$INPUT_AUTHORITY required")

val syncTag = commonTag(account, authority)
Logger.log.info("${javaClass.simpleName} called for $syncTag")
logger.info("${javaClass.simpleName} called for $syncTag")

if (!runningSyncs.add(syncTag)) {
Logger.log.info("There's already another worker running for $syncTag, skipping")
logger.info("There's already another worker running for $syncTag, skipping")
return Result.success()
}

Expand All @@ -160,7 +165,7 @@ abstract class BaseSyncWorker(
accountSettingsFactory.forAccount(account)
} catch (e: InvalidAccountException) {
val workId = workerParams.id
Logger.log.warning("Account $account doesn't exist anymore, cancelling worker $workId")
logger.warning("Account $account doesn't exist anymore, cancelling worker $workId")

val workManager = WorkManager.getInstance(applicationContext)
workManager.cancelWorkById(workId)
Expand All @@ -169,26 +174,26 @@ abstract class BaseSyncWorker(
}

if (inputData.getBoolean(INPUT_MANUAL, false))
Logger.log.info("Manual sync, skipping network checks")
logger.info("Manual sync, skipping network checks")
else {
val syncConditions = syncConditionsFactory.create(accountSettings)

// check internet connection
if (!syncConditions.internetAvailable()) {
Logger.log.info("WorkManager started SyncWorker without Internet connection. Aborting.")
logger.info("WorkManager started SyncWorker without Internet connection. Aborting.")
return Result.success()
}

// check WiFi restriction
if (!syncConditions.wifiConditionsMet()) {
Logger.log.info("WiFi conditions not met. Won't run periodic sync.")
logger.info("WiFi conditions not met. Won't run periodic sync.")
return Result.success()
}
}

return doSyncWork(account, authority, accountSettings)
} finally {
Logger.log.info("${javaClass.simpleName} finished for $syncTag")
logger.info("${javaClass.simpleName} finished for $syncTag")
runningSyncs -= syncTag
}
}
Expand All @@ -198,19 +203,19 @@ abstract class BaseSyncWorker(
authority: String,
accountSettings: AccountSettings
): Result = withContext(syncDispatcher) {
Logger.log.info("Running ${javaClass.name}: account=$account, authority=$authority")
logger.info("Running ${javaClass.name}: account=$account, authority=$authority")

// What are we going to sync? Select syncer based on authority
val syncer: Syncer = when (authority) {
applicationContext.getString(R.string.address_books_authority) ->
entryPoint.addressBookSyncer()
addressBookSyncer.get()
CalendarContract.AUTHORITY ->
entryPoint.calendarSyncer()
calendarSyncer.get()
TaskProvider.ProviderName.JtxBoard.authority ->
entryPoint.jtxSyncer()
jtxSyncer.get()
TaskProvider.ProviderName.OpenTasks.authority,
TaskProvider.ProviderName.TasksOrg.authority ->
entryPoint.taskSyncer()
taskSyncer.get()
else ->
throw IllegalArgumentException("Invalid authority $authority")
}
Expand Down Expand Up @@ -243,21 +248,21 @@ abstract class BaseSyncWorker(

// On soft errors the sync is retried a few times before considered failed
if (result.hasSoftError()) {
Logger.log.warning("Soft error while syncing: result=$result, stats=${result.stats}")
logger.warning("Soft error while syncing: result=$result, stats=${result.stats}")
if (runAttemptCount < MAX_RUN_ATTEMPTS) {
val blockDuration = result.delayUntil - System.currentTimeMillis() / 1000
Logger.log.warning("Waiting for $blockDuration seconds, before retrying ...")
logger.warning("Waiting for $blockDuration seconds, before retrying ...")

// We block the SyncWorker here so that it won't be started by the sync framework immediately again.
// This should be replaced by proper work scheduling as soon as we don't depend on the sync framework anymore.
if (blockDuration > 0)
delay(blockDuration * 1000)

Logger.log.warning("Retrying on soft error (attempt $runAttemptCount of $MAX_RUN_ATTEMPTS)")
logger.warning("Retrying on soft error (attempt $runAttemptCount of $MAX_RUN_ATTEMPTS)")
return@withContext Result.retry()
}

Logger.log.warning("Max retries on soft errors reached ($runAttemptCount of $MAX_RUN_ATTEMPTS). Treating as failed")
logger.warning("Max retries on soft errors reached ($runAttemptCount of $MAX_RUN_ATTEMPTS). Treating as failed")

notificationRegistry.notifyIfPossible(NotificationRegistry.NOTIFY_SYNC_ERROR, tag = softErrorNotificationTag) {
NotificationCompat.Builder(applicationContext, NotificationRegistry.CHANNEL_SYNC_IO_ERRORS)
Expand All @@ -275,6 +280,7 @@ abstract class BaseSyncWorker(
}

// If no soft error found, dismiss sync error notification
val notificationManager = NotificationManagerCompat.from(applicationContext)
notificationManager.cancel(
softErrorNotificationTag,
NotificationRegistry.NOTIFY_SYNC_ERROR
Expand All @@ -283,7 +289,7 @@ abstract class BaseSyncWorker(
// On a hard error - fail with an error message
// Note: SyncManager should have notified the user
if (result.hasHardError()) {
Logger.log.warning("Hard error while syncing: result=$result, stats=${result.stats}")
logger.warning("Hard error while syncing: result=$result, stats=${result.stats}")
return@withContext Result.failure(syncResult)
}
}
Expand Down
5 changes: 1 addition & 4 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/worker/OneTimeSyncWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,8 @@ import java.util.logging.Level
class OneTimeSyncWorker @AssistedInject constructor(
@Assisted appContext: Context,
@Assisted workerParams: WorkerParameters,
accountSettingsFactory: AccountSettings.Factory,
notificationRegistry: NotificationRegistry,
syncConditionsFactory: SyncConditions.Factory,
syncDispatcher: SyncDispatcher
) : BaseSyncWorker(appContext, workerParams, accountSettingsFactory, notificationRegistry, syncConditionsFactory, syncDispatcher.dispatcher) {
) : BaseSyncWorker(appContext, workerParams, syncDispatcher.dispatcher) {

companion object {

Expand Down
8 changes: 1 addition & 7 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/worker/PeriodicSyncWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ import androidx.work.Operation
import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager
import androidx.work.WorkerParameters
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.sync.SyncConditions
import at.bitfire.davdroid.sync.SyncDispatcher
import at.bitfire.davdroid.ui.NotificationRegistry
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import java.util.concurrent.TimeUnit
Expand All @@ -44,11 +41,8 @@ import java.util.concurrent.TimeUnit
class PeriodicSyncWorker @AssistedInject constructor(
@Assisted appContext: Context,
@Assisted workerParams: WorkerParameters,
accountSettingsFactory: AccountSettings.Factory,
notificationRegistry: NotificationRegistry,
syncConditionsFactory: SyncConditions.Factory,
syncDispatcher: SyncDispatcher
) : BaseSyncWorker(appContext, workerParams, accountSettingsFactory, notificationRegistry, syncConditionsFactory, syncDispatcher.dispatcher) {
) : BaseSyncWorker(appContext, workerParams, syncDispatcher.dispatcher) {

companion object {

Expand Down
Loading