Merge pull request #4733 from k9mail/fix_mail_sync

Improve periodic mail sync
This commit is contained in:
cketti 2020-05-07 19:03:22 +02:00 committed by GitHub
commit 1f3525ba49
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 150 additions and 78 deletions

View file

@ -189,6 +189,7 @@ public class Account implements BaseAccount {
private boolean remoteSearchFullText;
private int remoteSearchNumResults;
private boolean uploadSentMessages;
private long lastSyncTime;
private boolean changedVisibleLimits = false;
@ -1125,6 +1126,14 @@ public class Account implements BaseAccount {
remoteSearchFullText = val;
}
public synchronized long getLastSyncTime() {
return lastSyncTime;
}
public synchronized void setLastSyncTime(long lastSyncTime) {
this.lastSyncTime = lastSyncTime;
}
boolean isChangedVisibleLimits() {
return changedVisibleLimits;
}

View file

@ -162,6 +162,7 @@ class AccountPreferenceSerializer(
isMarkMessageAsReadOnView = storage.getBoolean("$accountUuid.markMessageAsReadOnView", true)
isMarkMessageAsReadOnDelete = storage.getBoolean("$accountUuid.markMessageAsReadOnDelete", true)
isAlwaysShowCcBcc = storage.getBoolean("$accountUuid.alwaysShowCcBcc", false)
lastSyncTime = storage.getLong("$accountUuid.lastSyncTime", 0L)
// Use email address as account description if necessary
if (description == null) {
@ -321,6 +322,7 @@ class AccountPreferenceSerializer(
editor.putString("$accountUuid.ringtone", notificationSetting.ringtone)
editor.putBoolean("$accountUuid.led", notificationSetting.isLedEnabled)
editor.putInt("$accountUuid.ledColor", notificationSetting.ledColor)
editor.putLong("$accountUuid.lastSyncTime", lastSyncTime)
for (type in NetworkType.values()) {
val useCompression = compressionMap[type]
@ -443,6 +445,7 @@ class AccountPreferenceSerializer(
editor.remove("$accountUuid.archiveFolderId")
editor.remove("$accountUuid.spamFolderId")
editor.remove("$accountUuid.autoExpandFolderId")
editor.remove("$accountUuid.lastSyncTime")
for (type in NetworkType.values()) {
editor.remove("$accountUuid.useCompression." + type.name)
@ -581,6 +584,7 @@ class AccountPreferenceSerializer(
isMarkMessageAsReadOnView = true
isMarkMessageAsReadOnDelete = true
isAlwaysShowCcBcc = false
lastSyncTime = 0L
setArchiveFolderId(null, SpecialFolderSelection.AUTOMATIC)
setDraftsFolderId(null, SpecialFolderSelection.AUTOMATIC)

View file

@ -56,6 +56,7 @@ import com.fsck.k9.controller.MessagingControllerCommands.PendingMoveOrCopy;
import com.fsck.k9.controller.MessagingControllerCommands.PendingSetFlag;
import com.fsck.k9.controller.ProgressBodyFactory.ProgressListener;
import com.fsck.k9.helper.Contacts;
import com.fsck.k9.helper.MutableBoolean;
import com.fsck.k9.mail.Address;
import com.fsck.k9.mail.AuthenticationFailedException;
import com.fsck.k9.mail.CertificateValidationException;
@ -2264,23 +2265,39 @@ public class MessagingController {
context.startActivity(chooserIntent);
}
public void checkMailBlocking(Account account) {
public boolean performPeriodicMailSync(Account account) {
final CountDownLatch latch = new CountDownLatch(1);
checkMail(context, account, true, false, new SimpleMessagingListener() {
MutableBoolean syncError = new MutableBoolean(false);
checkMail(context, account, false, false, new SimpleMessagingListener() {
@Override
public void checkMailFinished(Context context, Account account) {
latch.countDown();
}
@Override
public void synchronizeMailboxFailed(Account account, long folderId, String message) {
syncError.setValue(true);
}
});
Timber.v("checkMailBlocking(%s) about to await latch release", account.getDescription());
Timber.v("performPeriodicMailSync(%s) about to await latch release", account.getDescription());
try {
latch.await();
Timber.v("checkMailBlocking(%s) got latch release", account.getDescription());
Timber.v("performPeriodicMailSync(%s) got latch release", account.getDescription());
} catch (Exception e) {
Timber.e(e, "Interrupted while awaiting latch release");
}
boolean success = !syncError.getValue();
if (success) {
long now = System.currentTimeMillis();
Timber.v("Account %s successfully synced @ %tc", account, now);
account.setLastSyncTime(now);
Preferences.getPreferences(context).saveAccount(account);
}
return success;
}
/**
@ -2357,11 +2374,6 @@ public class MessagingController {
Timber.i("Skipping synchronizing unavailable account %s", account.getDescription());
return;
}
final long accountInterval = account.getAutomaticCheckIntervalMinutes() * 60 * 1000;
if (!ignoreLastCheckedTime && accountInterval <= 0) {
Timber.i("Skipping synchronizing account %s", account.getDescription());
return;
}
Timber.i("Synchronizing account %s", account.getDescription());
@ -2403,7 +2415,7 @@ public class MessagingController {
continue;
}
synchronizeFolder(account, folder, ignoreLastCheckedTime, accountInterval, listener);
synchronizeFolder(account, folder, ignoreLastCheckedTime, listener);
}
} catch (MessagingException e) {
Timber.e(e, "Unable to synchronize account %s", account.getName());
@ -2425,57 +2437,47 @@ public class MessagingController {
}
private void synchronizeFolder(Account account, LocalFolder folder, boolean ignoreLastCheckedTime,
MessagingListener listener) {
putBackground("sync" + folder.getServerId(), null, () -> {
synchronizeFolderInBackground(account, folder, ignoreLastCheckedTime, listener);
});
}
private void synchronizeFolder(
final Account account,
final LocalFolder folder,
final boolean ignoreLastCheckedTime,
final long accountInterval,
final MessagingListener listener) {
private void synchronizeFolderInBackground(Account account, LocalFolder folder, boolean ignoreLastCheckedTime,
MessagingListener listener) {
Timber.v("Folder %s was last synced @ %tc", folder.getServerId(), folder.getLastChecked());
if (!ignoreLastCheckedTime && folder.getLastChecked() > System.currentTimeMillis() - accountInterval) {
Timber.v("Not syncing folder %s, previously synced @ %tc which would be too recent for the account " +
"period", folder.getServerId(), folder.getLastChecked());
return;
if (!ignoreLastCheckedTime) {
long lastCheckedTime = folder.getLastChecked();
long now = System.currentTimeMillis();
if (lastCheckedTime > now) {
// The time this folder was last checked lies in the future. We better ignore this and sync now.
} else {
long syncInterval = account.getAutomaticCheckIntervalMinutes() * 60L * 1000L;
long nextSyncTime = lastCheckedTime + syncInterval;
if (nextSyncTime > now) {
Timber.v("Not syncing folder %s, previously synced @ %tc which would be too recent for the " +
"account sync interval", folder.getServerId(), lastCheckedTime);
return;
}
}
}
putBackground("sync" + folder.getServerId(), null, new Runnable() {
@Override
public void run() {
LocalFolder tLocalFolder = null;
try {
// In case multiple Commands get enqueued, don't run more than
// once
final LocalStore localStore = localStoreProvider.getInstance(account);
tLocalFolder = localStore.getFolder(folder.getServerId());
tLocalFolder.open();
if (!ignoreLastCheckedTime && tLocalFolder.getLastChecked() >
(System.currentTimeMillis() - accountInterval)) {
Timber.v("Not running Command for folder %s, previously synced @ %tc which would " +
"be too recent for the account period",
folder.getServerId(), folder.getLastChecked());
return;
}
showFetchingMailNotificationIfNecessary(account, folder);
try {
synchronizeMailboxSynchronous(account, folder.getDatabaseId(), listener);
} finally {
clearFetchingMailNotificationIfNecessary(account);
}
} catch (Exception e) {
Timber.e(e, "Exception while processing folder %s:%s",
account.getDescription(), folder.getServerId());
} finally {
closeFolder(tLocalFolder);
}
}
}
);
try {
showFetchingMailNotificationIfNecessary(account, folder);
try {
synchronizeMailboxSynchronous(account, folder.getDatabaseId(), listener);
long now = System.currentTimeMillis();
folder.setLastChecked(now);
} finally {
clearFetchingMailNotificationIfNecessary(account);
}
} catch (Exception e) {
Timber.e(e, "Exception while processing folder %s:%s", account.getDescription(), folder.getServerId());
}
}
private void showFetchingMailNotificationIfNecessary(Account account, LocalFolder folder) {

View file

@ -0,0 +1,3 @@
package com.fsck.k9.helper
class MutableBoolean(var value: Boolean)

View file

@ -1,6 +1,7 @@
package com.fsck.k9.job
import androidx.work.WorkManager
import com.fsck.k9.Account
import com.fsck.k9.Preferences
import timber.log.Timber
@ -14,7 +15,16 @@ class K9JobManager(
scheduleMailSync()
}
fun scheduleMailSync() {
fun scheduleMailSync(account: Account) {
mailSyncWorkerManager.cancelMailSync(account)
mailSyncWorkerManager.scheduleMailSync(account)
}
fun schedulePusherRefresh() {
// Push is temporarily disabled. See GH-4253
}
private fun scheduleMailSync() {
cancelAllMailSyncJobs()
preferences.availableAccounts?.forEach { account ->
@ -22,10 +32,6 @@ class K9JobManager(
}
}
fun schedulePusherRefresh() {
// Push is temporarily disabled. See GH-4253
}
private fun cancelAllMailSyncJobs() {
Timber.v("canceling mail sync job")
workManager.cancelAllWorkByTag(MailSyncWorkerManager.MAIL_SYNC_TAG)

View file

@ -1,6 +1,7 @@
package com.fsck.k9.job
import androidx.work.WorkerFactory
import com.fsck.k9.Clock
import org.koin.dsl.module
val jobModule = module {
@ -8,5 +9,5 @@ val jobModule = module {
single<WorkerFactory> { K9WorkerFactory(get(), get()) }
single { get<WorkManagerProvider>().getWorkManager() }
single { K9JobManager(get(), get(), get()) }
factory { MailSyncWorkerManager(get()) }
factory { MailSyncWorkerManager(get(), Clock.INSTANCE) }
}

View file

@ -4,6 +4,7 @@ import android.content.ContentResolver
import android.content.Context
import androidx.work.Worker
import androidx.work.WorkerParameters
import com.fsck.k9.Account
import com.fsck.k9.K9
import com.fsck.k9.Preferences
import com.fsck.k9.controller.MessagingController
@ -27,11 +28,20 @@ class MailSyncWorker(
return Result.success()
}
preferences.getAccount(accountUuid)?.let { account ->
messagingController.checkMailBlocking(account)
val account = preferences.getAccount(accountUuid)
if (account == null) {
Timber.e("Account %s not found. Can't perform mail sync.", accountUuid)
return Result.failure()
}
return Result.success()
if (account.isPeriodicMailSyncDisabled) {
Timber.d("Periodic mail sync has been disabled for this account. Skipping mail sync.")
return Result.success()
}
val success = messagingController.performPeriodicMailSync(account)
return if (success) Result.success() else Result.retry()
}
private fun isBackgroundSyncDisabled(): Boolean {
@ -42,6 +52,9 @@ class MailSyncWorker(
}
}
private val Account.isPeriodicMailSyncDisabled
get() = automaticCheckIntervalMinutes <= 0
companion object {
const val EXTRA_ACCOUNT_UUID = "accountUuid"
}

View file

@ -1,5 +1,6 @@
package com.fsck.k9.job
import androidx.work.BackoffPolicy
import androidx.work.Constraints
import androidx.work.ExistingPeriodicWorkPolicy
import androidx.work.NetworkType
@ -7,40 +8,72 @@ import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager
import androidx.work.workDataOf
import com.fsck.k9.Account
import java.util.concurrent.TimeUnit
import com.fsck.k9.Clock
import com.fsck.k9.K9
import java.time.Duration
import timber.log.Timber
class MailSyncWorkerManager(private val workManager: WorkManager) {
class MailSyncWorkerManager(private val workManager: WorkManager, val clock: Clock) {
fun cancelMailSync(account: Account) {
Timber.v("Canceling mail sync worker for %s", account.description)
val uniqueWorkName = createUniqueWorkName(account.uuid)
workManager.cancelUniqueWork(uniqueWorkName)
}
fun scheduleMailSync(account: Account) {
getSyncIntervalInMinutesIfEnabled(account)?.let { syncInterval ->
if (isNeverSyncInBackground()) return
getSyncIntervalIfEnabled(account)?.let { syncInterval ->
Timber.v("Scheduling mail sync worker for %s", account.description)
Timber.v(" sync interval: %d minutes", syncInterval.toMinutes())
val constraints = Constraints.Builder()
.setRequiredNetworkType(NetworkType.CONNECTED)
.setRequiresStorageNotLow(true)
.build()
.setRequiredNetworkType(NetworkType.CONNECTED)
.setRequiresStorageNotLow(true)
.build()
val lastSyncTime = account.lastSyncTime
Timber.v(" last sync time: %tc", lastSyncTime)
val initialDelay = calculateInitialDelay(lastSyncTime, syncInterval)
Timber.v(" initial delay: %d minutes", initialDelay.toMinutes())
val data = workDataOf(MailSyncWorker.EXTRA_ACCOUNT_UUID to account.uuid)
val mailSyncRequest = PeriodicWorkRequestBuilder<MailSyncWorker>(syncInterval, TimeUnit.MINUTES)
.setConstraints(constraints)
.setInputData(data)
.addTag(MAIL_SYNC_TAG)
.build()
val mailSyncRequest = PeriodicWorkRequestBuilder<MailSyncWorker>(syncInterval)
.setInitialDelay(initialDelay)
.setBackoffCriteria(BackoffPolicy.EXPONENTIAL, INITIAL_BACKOFF_DELAY)
.setConstraints(constraints)
.setInputData(data)
.addTag(MAIL_SYNC_TAG)
.build()
val uniqueWorkName = createUniqueWorkName(account.uuid)
workManager.enqueueUniquePeriodicWork(uniqueWorkName, ExistingPeriodicWorkPolicy.REPLACE, mailSyncRequest)
}
}
private fun getSyncIntervalInMinutesIfEnabled(account: Account): Long? {
private fun isNeverSyncInBackground() = K9.backgroundOps == K9.BACKGROUND_OPS.NEVER
private fun getSyncIntervalIfEnabled(account: Account): Duration? {
val intervalMinutes = account.automaticCheckIntervalMinutes
if (intervalMinutes <= Account.INTERVAL_MINUTES_NEVER) {
return null
}
return intervalMinutes.toLong()
return Duration.ofMinutes(intervalMinutes.toLong())
}
private fun calculateInitialDelay(lastSyncTime: Long, syncInterval: Duration): Duration {
val now = clock.time
val nextSyncTime = lastSyncTime + syncInterval.toMillis()
return if (lastSyncTime > now || nextSyncTime <= now) {
Duration.ZERO
} else {
Duration.ofMillis(nextSyncTime - now)
}
}
private fun createUniqueWorkName(accountUuid: String): String {
@ -49,5 +82,6 @@ class MailSyncWorkerManager(private val workManager: WorkManager) {
companion object {
const val MAIL_SYNC_TAG = "MailSync"
private val INITIAL_BACKOFF_DELAY = Duration.ofMinutes(5)
}
}

View file

@ -215,7 +215,7 @@ class AccountSettingsDataStore(
}
private fun reschedulePoll() {
jobManager.scheduleMailSync()
jobManager.scheduleMailSync(account)
}
private fun restartPushers() {