From 64971007f5cb6c6560a48c96f0158997e4834a93 Mon Sep 17 00:00:00 2001 From: Sujan Kota Date: Mon, 14 Jan 2019 09:56:13 -0800 Subject: [PATCH 1/2] Make notification for new messages as an interface and move the implementation to the K9NotificationStrategy.kt --- .../java/com/fsck/k9/controller/KoinModule.kt | 2 +- .../k9/controller/MessagingController.java | 84 +++---------------- .../com/fsck/k9/mailstore/LocalFolder.java | 11 +++ .../k9/notification/NotificationStrategy.kt | 17 ++++ .../k9/notification/K9NotificationStrategy.kt | 69 +++++++++++++++ .../com/fsck/k9/notification/KoinModule.kt | 1 + 6 files changed, 109 insertions(+), 75 deletions(-) create mode 100644 app/core/src/main/java/com/fsck/k9/notification/NotificationStrategy.kt create mode 100644 app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt diff --git a/app/core/src/main/java/com/fsck/k9/controller/KoinModule.kt b/app/core/src/main/java/com/fsck/k9/controller/KoinModule.kt index 5d55f64f1..ce22855a8 100644 --- a/app/core/src/main/java/com/fsck/k9/controller/KoinModule.kt +++ b/app/core/src/main/java/com/fsck/k9/controller/KoinModule.kt @@ -3,6 +3,6 @@ package com.fsck.k9.controller import org.koin.dsl.module.applicationContext val controllerModule = applicationContext { - bean { MessagingController(get(), get(), get(), get(), get(), get(), get(), get("controllerExtensions")) } + bean { MessagingController(get(), get(), get(), get(), get(), get(), get(), get(), get("controllerExtensions")) } bean { DefaultAccountStatsCollector(get(), get(), get()) as AccountStatsCollector } } diff --git a/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java b/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java index 9cf46f676..5bd4061d6 100644 --- a/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java +++ b/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java @@ -83,6 +83,7 @@ import com.fsck.k9.mailstore.OutboxStateRepository; import com.fsck.k9.mailstore.SendState; import com.fsck.k9.mailstore.UnavailableStorageException; import com.fsck.k9.notification.NotificationController; +import com.fsck.k9.notification.NotificationStrategy; import com.fsck.k9.power.TracingPowerManager; import com.fsck.k9.power.TracingPowerManager.TracingWakeLock; import com.fsck.k9.search.LocalSearch; @@ -119,6 +120,7 @@ public class MessagingController { private final Context context; private final Contacts contacts; private final NotificationController notificationController; + private final NotificationStrategy notificationStrategy; private final LocalStoreProvider localStoreProvider; private final BackendManager backendManager; @@ -143,11 +145,13 @@ public class MessagingController { MessagingController(Context context, NotificationController notificationController, + NotificationStrategy notificationStrategy, LocalStoreProvider localStoreProvider, Contacts contacts, AccountStatsCollector accountStatsCollector, CoreResourceProvider resourceProvider, BackendManager backendManager, List controllerExtensions) { this.context = context; this.notificationController = notificationController; + this.notificationStrategy = notificationStrategy; this.localStoreProvider = localStoreProvider; this.contacts = contacts; this.accountStatsCollector = accountStatsCollector; @@ -2515,7 +2519,7 @@ public class MessagingController { Folder.FolderClass fDisplayClass = folder.getDisplayClass(); Folder.FolderClass fSyncClass = folder.getSyncClass(); - if (modeMismatch(aDisplayMode, fDisplayClass)) { + if (LocalFolder.isModeMismatch(aDisplayMode, fDisplayClass)) { // Never sync a folder that isn't displayed /* if (K9.DEBUG) { @@ -2527,7 +2531,7 @@ public class MessagingController { continue; } - if (modeMismatch(aSyncMode, fSyncClass)) { + if (LocalFolder.isModeMismatch(aSyncMode, fSyncClass)) { // Do not sync folders in the wrong class /* if (K9.DEBUG) { @@ -2709,63 +2713,6 @@ public class MessagingController { }); } - - public boolean shouldNotifyForMessage(Account account, LocalFolder localFolder, Message message, - boolean isOldMessage) { - // If we don't even have an account name, don't show the notification. - // (This happens during initial account setup) - if (account.getName() == null) { - return false; - } - - if (K9.isQuietTime() && !K9.isNotificationDuringQuietTimeEnabled()) { - return false; - } - - // Do not notify if the user does not have notifications enabled or if the message has - // been read. - if (!account.isNotifyNewMail() || message.isSet(Flag.SEEN) || isOldMessage) { - return false; - } - - Account.FolderMode aDisplayMode = account.getFolderDisplayMode(); - Account.FolderMode aNotifyMode = account.getFolderNotifyNewMailMode(); - Folder.FolderClass fDisplayClass = localFolder.getDisplayClass(); - Folder.FolderClass fNotifyClass = localFolder.getNotifyClass(); - - if (modeMismatch(aDisplayMode, fDisplayClass)) { - // Never notify a folder that isn't displayed - return false; - } - - if (modeMismatch(aNotifyMode, fNotifyClass)) { - // Do not notify folders in the wrong class - return false; - } - - // No notification for new messages in Trash, Drafts, Spam or Sent folder. - // But do notify if it's the INBOX (see issue 1817). - Folder folder = message.getFolder(); - if (folder != null) { - String folderServerId = folder.getServerId(); - if (!folderServerId.equals(account.getInboxFolder()) && - (folderServerId.equals(account.getTrashFolder()) - || folderServerId.equals(account.getDraftsFolder()) - || folderServerId.equals(account.getSpamFolder()) - || folderServerId.equals(account.getSentFolder()))) { - return false; - } - } - - // Don't notify if the sender address matches one of our identities and the user chose not - // to be notified for such messages. - if (account.isAnIdentity(message.getFrom()) && !account.isNotifySelfNewMail()) { - return false; - } - - return !account.isNotifyContactsMailOnly() || contacts.isAnyInContacts(message.getFrom()); - } - public void deleteAccount(Account account) { notificationController.clearNewMailNotifications(account); memorizingMessagingListener.removeAccount(account); @@ -2826,17 +2773,6 @@ public class MessagingController { return id; } - private boolean modeMismatch(Account.FolderMode aMode, Folder.FolderClass fMode) { - return aMode == Account.FolderMode.NONE - || (aMode == Account.FolderMode.FIRST_CLASS && - fMode != Folder.FolderClass.FIRST_CLASS) - || (aMode == Account.FolderMode.FIRST_AND_SECOND_CLASS && - fMode != Folder.FolderClass.FIRST_CLASS && - fMode != Folder.FolderClass.SECOND_CLASS) - || (aMode == Account.FolderMode.NOT_SECOND_CLASS && - fMode == Folder.FolderClass.SECOND_CLASS); - } - private static AtomicInteger sequencing = new AtomicInteger(0); private static class Command implements Comparable { @@ -2903,7 +2839,7 @@ public class MessagingController { Folder.FolderClass fDisplayClass = folder.getDisplayClass(); Folder.FolderClass fPushClass = folder.getPushClass(); - if (modeMismatch(aDisplayMode, fDisplayClass)) { + if (LocalFolder.isModeMismatch(aDisplayMode, fDisplayClass)) { // Never push a folder that isn't displayed /* if (K9.DEBUG) { @@ -2915,7 +2851,7 @@ public class MessagingController { continue; } - if (modeMismatch(aPushMode, fPushClass)) { + if (LocalFolder.isModeMismatch(aPushMode, fPushClass)) { // Do not push folders in the wrong class /* if (K9.DEBUG) { @@ -3149,7 +3085,7 @@ public class MessagingController { // Send a notification of this message LocalMessage message = loadMessage(folderServerId, messageServerId); LocalFolder localFolder = message.getFolder(); - if (shouldNotifyForMessage(account, localFolder, message, isOldMessage)) { + if (notificationStrategy.shouldNotifyForMessage(account, localFolder, message, isOldMessage)) { // Notify with the localMessage so that we don't have to recalculate the content preview. notificationController.addNewMailNotification(account, message, previousUnreadMessageCount); } @@ -3176,7 +3112,7 @@ public class MessagingController { syncRemovedMessage(folderServerId, message.getUid()); } else { LocalFolder localFolder = message.getFolder(); - if (shouldNotifyForMessage(account, localFolder, message, false)) { + if (notificationStrategy.shouldNotifyForMessage(account, localFolder, message, false)) { shouldBeNotifiedOf = true; } } diff --git a/app/core/src/main/java/com/fsck/k9/mailstore/LocalFolder.java b/app/core/src/main/java/com/fsck/k9/mailstore/LocalFolder.java index 2def2b1a2..6a7861f24 100644 --- a/app/core/src/main/java/com/fsck/k9/mailstore/LocalFolder.java +++ b/app/core/src/main/java/com/fsck/k9/mailstore/LocalFolder.java @@ -2429,4 +2429,15 @@ public class LocalFolder extends Folder { return databaseName; } } + + public static boolean isModeMismatch(Account.FolderMode aMode, Folder.FolderClass fMode) { + return aMode == Account.FolderMode.NONE + || (aMode == Account.FolderMode.FIRST_CLASS && + fMode != Folder.FolderClass.FIRST_CLASS) + || (aMode == Account.FolderMode.FIRST_AND_SECOND_CLASS && + fMode != Folder.FolderClass.FIRST_CLASS && + fMode != Folder.FolderClass.SECOND_CLASS) + || (aMode == Account.FolderMode.NOT_SECOND_CLASS && + fMode == Folder.FolderClass.SECOND_CLASS); + } } diff --git a/app/core/src/main/java/com/fsck/k9/notification/NotificationStrategy.kt b/app/core/src/main/java/com/fsck/k9/notification/NotificationStrategy.kt new file mode 100644 index 000000000..345d2888d --- /dev/null +++ b/app/core/src/main/java/com/fsck/k9/notification/NotificationStrategy.kt @@ -0,0 +1,17 @@ +package com.fsck.k9.notification + +import com.fsck.k9.Account +import com.fsck.k9.K9 +import com.fsck.k9.helper.Contacts +import com.fsck.k9.mail.Flag +import com.fsck.k9.mail.Folder +import com.fsck.k9.mailstore.LocalFolder +import com.fsck.k9.mail.Message + +interface NotificationStrategy { + + fun shouldNotifyForMessage(account: Account, + localFolder: LocalFolder, + message: Message, + isOldMessage:Boolean):Boolean +} \ No newline at end of file diff --git a/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt b/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt new file mode 100644 index 000000000..c4e8371fd --- /dev/null +++ b/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt @@ -0,0 +1,69 @@ +package com.fsck.k9.notification + +import com.fsck.k9.Account +import com.fsck.k9.K9 +import com.fsck.k9.helper.Contacts +import com.fsck.k9.mail.Flag +import com.fsck.k9.mail.Message +import com.fsck.k9.mailstore.LocalFolder + +class K9NotificationStrategy(val contacts: Contacts) : NotificationStrategy { + + override fun shouldNotifyForMessage(account: Account, + localFolder: LocalFolder, + message: Message, + isOldMessage:Boolean):Boolean { + + // If we don't even have an account name, don't show the notification. + // (This happens during initial account setup) + if (account.name == null) { + return false + } + + if (K9.isQuietTime() && !K9.isNotificationDuringQuietTimeEnabled()) { + return false + } + + // Do not notify if the user does not have notifications enabled or if the message has + // been read. + if (!account.isNotifyNewMail || message.isSet(Flag.SEEN) || isOldMessage) { + return false + } + + val aDisplayMode = account.folderDisplayMode + val aNotifyMode = account.folderNotifyNewMailMode + val fDisplayClass = localFolder.displayClass + val fNotifyClass = localFolder.notifyClass + + if (LocalFolder.isModeMismatch(aDisplayMode, fDisplayClass)) { + // Never notify a folder that isn't displayed + return false + } + + if (LocalFolder.isModeMismatch(aNotifyMode, fNotifyClass)) { + // Do not notify folders in the wrong class + return false + } + + // No notification for new messages in Trash, Drafts, Spam or Sent folder. + // But do notify if it's the INBOX (see issue 1817). + val folder = message.folder + if (folder != null) { + val folderServerId = folder.serverId + if (folderServerId != account.inboxFolder && (folderServerId == account.trashFolder + || folderServerId == account.draftsFolder + || folderServerId == account.spamFolder + || folderServerId == account.sentFolder)) { + return false + } + } + + // Don't notify if the sender address matches one of our identities and the user chose not + // to be notified for such messages. + return if (account.isAnIdentity(message.from) && !account.isNotifySelfNewMail) { + false + } else !account.isNotifyContactsMailOnly || contacts.isAnyInContacts(message.from) + + } +} + diff --git a/app/k9mail/src/main/java/com/fsck/k9/notification/KoinModule.kt b/app/k9mail/src/main/java/com/fsck/k9/notification/KoinModule.kt index 15822b162..9d7a3abf7 100644 --- a/app/k9mail/src/main/java/com/fsck/k9/notification/KoinModule.kt +++ b/app/k9mail/src/main/java/com/fsck/k9/notification/KoinModule.kt @@ -5,4 +5,5 @@ import org.koin.dsl.module.applicationContext val notificationModule = applicationContext { bean { K9NotificationActionCreator(get()) as NotificationActionCreator } bean { K9NotificationResourceProvider(get()) as NotificationResourceProvider } + bean { K9NotificationStrategy(get()) as NotificationStrategy } } From 8e5e6d66a64e8bd42cb8d4b5acda3ce48ef32471 Mon Sep 17 00:00:00 2001 From: Sujan Kota Date: Mon, 14 Jan 2019 10:41:44 -0800 Subject: [PATCH 2/2] Fix the unit tests --- .../com/fsck/k9/controller/MessagingControllerTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/core/src/test/java/com/fsck/k9/controller/MessagingControllerTest.java b/app/core/src/test/java/com/fsck/k9/controller/MessagingControllerTest.java index 154c5f521..87f03a71f 100644 --- a/app/core/src/test/java/com/fsck/k9/controller/MessagingControllerTest.java +++ b/app/core/src/test/java/com/fsck/k9/controller/MessagingControllerTest.java @@ -38,6 +38,7 @@ import com.fsck.k9.mailstore.OutboxStateRepository; import com.fsck.k9.mailstore.SendState; import com.fsck.k9.mailstore.UnavailableStorageException; import com.fsck.k9.notification.NotificationController; +import com.fsck.k9.notification.NotificationStrategy; import com.fsck.k9.search.LocalSearch; import com.fsck.k9.search.SearchAccount; import org.jetbrains.annotations.NotNull; @@ -106,6 +107,8 @@ public class MessagingControllerTest extends K9RobolectricTest { private LocalStore localStore; @Mock private NotificationController notificationController; + @Mock + private NotificationStrategy notificationStrategy; @Captor private ArgumentCaptor> localFolderListCaptor; @Captor @@ -147,7 +150,8 @@ public class MessagingControllerTest extends K9RobolectricTest { MockitoAnnotations.initMocks(this); appContext = RuntimeEnvironment.application; - controller = new MessagingController(appContext, notificationController, localStoreProvider, contacts, + controller = new MessagingController(appContext, notificationController, notificationStrategy, + localStoreProvider, contacts, accountStatsCollector, mock(CoreResourceProvider.class), backendManager, Collections.emptyList());