Merge pull request #5946 from k9mail/update_new_mail_notifications

Add support for updating existing notifications
This commit is contained in:
cketti 2022-03-08 22:07:30 +01:00 committed by GitHub
commit 2607b24b3f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 95 additions and 36 deletions

View file

@ -27,8 +27,10 @@ internal class NewMailNotificationController(
fun addNewMailNotification(account: Account, message: LocalMessage, silent: Boolean) {
val notificationData = newMailNotificationManager.addNewMailNotification(account, message, silent)
if (notificationData != null) {
processNewMailNotificationData(notificationData)
}
}
fun removeNewMailNotifications(
account: Account,

View file

@ -38,10 +38,10 @@ internal class NewMailNotificationManager(
)
}
fun addNewMailNotification(account: Account, message: LocalMessage, silent: Boolean): NewMailNotificationData {
fun addNewMailNotification(account: Account, message: LocalMessage, silent: Boolean): NewMailNotificationData? {
val content = contentCreator.createFromMessage(account, message)
val result = notificationRepository.addNotification(account, content, timestamp = now())
val result = notificationRepository.addNotification(account, content, timestamp = now()) ?: return null
val singleNotificationData = createSingleNotificationData(
account = account,

View file

@ -2,7 +2,6 @@ package com.fsck.k9.notification
import com.fsck.k9.Account
import com.fsck.k9.controller.MessageReference
import com.fsck.k9.core.BuildConfig
internal const val MAX_NUMBER_OF_NEW_MESSAGE_NOTIFICATIONS = 8
@ -30,15 +29,45 @@ internal class NotificationDataStore {
}
@Synchronized
fun addNotification(account: Account, content: NotificationContent, timestamp: Long): AddNotificationResult {
fun addNotification(account: Account, content: NotificationContent, timestamp: Long): AddNotificationResult? {
val notificationData = getNotificationData(account)
val messageReference = content.messageReference
if (BuildConfig.DEBUG && notificationData.contains(messageReference)) {
throw AssertionError("Notification for message $messageReference already exists")
val activeNotification = notificationData.activeNotifications.firstOrNull { notificationHolder ->
notificationHolder.content.messageReference == messageReference
}
val inactiveNotification = notificationData.inactiveNotifications.firstOrNull { inactiveNotificationHolder ->
inactiveNotificationHolder.content.messageReference == messageReference
}
return if (notificationData.isMaxNumberOfActiveNotificationsReached) {
return if (activeNotification != null) {
val newActiveNotification = activeNotification.copy(content = content)
val notificationHolder = activeNotification.copy(
content = content
)
val operations = emptyList<NotificationStoreOperation>()
val newActiveNotifications = notificationData.activeNotifications
.replace(activeNotification, newActiveNotification)
val newNotificationData = notificationData.copy(
activeNotifications = newActiveNotifications
)
notificationDataMap[account.uuid] = newNotificationData
AddNotificationResult.newNotification(newNotificationData, operations, notificationHolder)
} else if (inactiveNotification != null) {
val newInactiveNotification = inactiveNotification.copy(content = content)
val newInactiveNotifications = notificationData.inactiveNotifications
.replace(inactiveNotification, newInactiveNotification)
val newNotificationData = notificationData.copy(
inactiveNotifications = newInactiveNotifications
)
notificationDataMap[account.uuid] = newNotificationData
null
} else if (notificationData.isMaxNumberOfActiveNotificationsReached) {
val lastNotificationHolder = notificationData.activeNotifications.last()
val inactiveNotificationHolder = lastNotificationHolder.toInactiveNotificationHolder()
@ -175,14 +204,15 @@ internal class NotificationDataStore {
throw AssertionError("getNewNotificationId() called with no free notification ID")
}
private fun NotificationData.contains(messageReference: MessageReference): Boolean {
return activeNotifications.any { it.content.messageReference == messageReference } ||
inactiveNotifications.any { it.content.messageReference == messageReference }
}
private fun NotificationHolder.toInactiveNotificationHolder() = InactiveNotificationHolder(timestamp, content)
private fun InactiveNotificationHolder.toNotificationHolder(notificationId: Int): NotificationHolder {
return NotificationHolder(notificationId, timestamp, content)
}
private fun <T> List<T>.replace(old: T, new: T): List<T> {
return map { element ->
if (element === old) new else element
}
}
}

View file

@ -37,8 +37,8 @@ internal class NotificationRepository(
}
@Synchronized
fun addNotification(account: Account, content: NotificationContent, timestamp: Long): AddNotificationResult {
return notificationDataStore.addNotification(account, content, timestamp).also { result ->
fun addNotification(account: Account, content: NotificationContent, timestamp: Long): AddNotificationResult? {
return notificationDataStore.addNotification(account, content, timestamp)?.also { result ->
persistNotificationDataStoreChanges(
account = account,
operations = result.notificationStoreOperations,

View file

@ -49,6 +49,7 @@ class NewMailNotificationManagerTest {
val result = manager.addNewMailNotification(account, message, silent = false)
assertNotNull(result)
assertThat(result.singleNotificationData.first().content).isEqualTo(
NotificationContent(
messageReference = createMessageReference("msg-1"),
@ -85,6 +86,7 @@ class NewMailNotificationManagerTest {
val result = manager.addNewMailNotification(account, messageTwo, silent = false)
assertNotNull(result)
assertThat(result.singleNotificationData.first().content).isEqualTo(
NotificationContent(
messageReference = createMessageReference("msg-2"),
@ -121,6 +123,7 @@ class NewMailNotificationManagerTest {
val result = manager.addNewMailNotification(account, message, silent = false)
assertNotNull(result)
val notificationId = NotificationIds.getSingleMessageNotificationId(account, index = 0)
assertThat(result.cancelNotificationIds).isEqualTo(listOf(notificationId))
assertThat(result.singleNotificationData.first().notificationId).isEqualTo(notificationId)
@ -196,6 +199,7 @@ class NewMailNotificationManagerTest {
messageUid = "msg-2"
)
val dataTwo = manager.addNewMailNotification(account, messageTwo, silent = true)
assertNotNull(dataTwo)
val notificationIdTwo = dataTwo.singleNotificationData.first().notificationId
val messageThree = addMessageToNotificationContentCreator(
sender = "Alice",

View file

@ -3,12 +3,8 @@ package com.fsck.k9.notification
import com.fsck.k9.Account
import com.fsck.k9.RobolectricTest
import com.fsck.k9.controller.MessageReference
import com.fsck.k9.core.BuildConfig
import com.google.common.truth.Truth.assertThat
import kotlin.test.assertNotNull
import org.junit.Assert.fail
import org.junit.Assume.assumeFalse
import org.junit.Assume.assumeTrue
import org.junit.Test
private const val ACCOUNT_UUID = "1-2-3"
@ -26,6 +22,7 @@ class NotificationDataStoreTest : RobolectricTest() {
val result = notificationDataStore.addNotification(account, content, TIMESTAMP)
assertNotNull(result)
assertThat(result.shouldCancelNotification).isFalse()
val holder = result.notificationHolder
@ -48,6 +45,7 @@ class NotificationDataStoreTest : RobolectricTest() {
val result = notificationDataStore.addNotification(account, createNotificationContent("9"), TIMESTAMP)
assertNotNull(result)
assertThat(result.shouldCancelNotification).isTrue()
assertThat(result.cancelNotificationId).isEqualTo(NotificationIds.getSingleMessageNotificationId(account, 0))
}
@ -107,19 +105,23 @@ class NotificationDataStoreTest : RobolectricTest() {
fun testNewMessagesCount() {
val contentOne = createNotificationContent("1")
val resultOne = notificationDataStore.addNotification(account, contentOne, TIMESTAMP)
assertNotNull(resultOne)
assertThat(resultOne.notificationData.newMessagesCount).isEqualTo(1)
val contentTwo = createNotificationContent("2")
val resultTwo = notificationDataStore.addNotification(account, contentTwo, TIMESTAMP)
assertNotNull(resultTwo)
assertThat(resultTwo.notificationData.newMessagesCount).isEqualTo(2)
}
@Test
fun testIsSingleMessageNotification() {
val resultOne = notificationDataStore.addNotification(account, createNotificationContent("1"), TIMESTAMP)
assertNotNull(resultOne)
assertThat(resultOne.notificationData.isSingleMessageNotification).isTrue()
val resultTwo = notificationDataStore.addNotification(account, createNotificationContent("2"), TIMESTAMP)
assertNotNull(resultTwo)
assertThat(resultTwo.notificationData.isSingleMessageNotification).isFalse()
}
@ -128,37 +130,58 @@ class NotificationDataStoreTest : RobolectricTest() {
val content = createNotificationContent("1")
val addResult = notificationDataStore.addNotification(account, content, TIMESTAMP)
assertNotNull(addResult)
assertThat(addResult.notificationData.activeNotifications.first()).isEqualTo(addResult.notificationHolder)
}
@Test
fun `adding notification for same message twice should throw in debug build`() {
assumeTrue(BuildConfig.DEBUG)
fun `adding notification for message with active notification should update notification`() {
val content1 = createNotificationContent("1")
val content2 = createNotificationContent("1")
notificationDataStore.addNotification(account, content1, TIMESTAMP)
try {
notificationDataStore.addNotification(account, content2, TIMESTAMP)
fail("Exception expected")
} catch (e: AssertionError) {
assertThat(e).hasMessageThat().matches("Notification for message .+ already exists")
val resultOne = notificationDataStore.addNotification(account, content1, TIMESTAMP)
val resultTwo = notificationDataStore.addNotification(account, content2, TIMESTAMP)
assertNotNull(resultOne)
assertNotNull(resultTwo)
assertThat(resultTwo.notificationData.activeNotifications).hasSize(1)
assertThat(resultTwo.notificationData.activeNotifications.first().content).isSameInstanceAs(content2)
assertThat(resultTwo.notificationStoreOperations).isEmpty()
with(resultTwo.notificationHolder) {
assertThat(notificationId).isEqualTo(resultOne.notificationHolder.notificationId)
assertThat(timestamp).isEqualTo(resultOne.notificationHolder.timestamp)
assertThat(content).isSameInstanceAs(content2)
}
assertThat(resultTwo.shouldCancelNotification).isFalse()
}
@Test
fun `adding notification for same message twice should add another notification in release build`() {
assumeFalse(BuildConfig.DEBUG)
fun `adding notification for message with inactive notification should update notificationData`() {
notificationDataStore.addNotification(account, createNotificationContent("1"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("2"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("3"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("4"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("5"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("6"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("7"), TIMESTAMP)
notificationDataStore.addNotification(account, createNotificationContent("8"), TIMESTAMP)
val latestNotificationContent = createNotificationContent("9")
notificationDataStore.addNotification(account, latestNotificationContent, TIMESTAMP)
val content = createNotificationContent("1")
val content1 = createNotificationContent("1")
val content2 = createNotificationContent("1")
val resultOne = notificationDataStore.addNotification(account, content, TIMESTAMP)
val addResult1 = notificationDataStore.addNotification(account, content1, TIMESTAMP)
val addResult2 = notificationDataStore.addNotification(account, content2, TIMESTAMP)
assertThat(resultOne).isNull()
assertThat(addResult1.notificationHolder.notificationId)
.isNotEqualTo(addResult2.notificationHolder.notificationId)
val resultTwo = notificationDataStore.removeNotifications(account) {
listOf(latestNotificationContent.messageReference)
}
assertNotNull(resultTwo)
val notificationHolder = resultTwo.notificationData.activeNotifications.first { notificationHolder ->
notificationHolder.content.messageReference == content.messageReference
}
assertThat(notificationHolder.content).isSameInstanceAs(content)
}
private fun createAccount(): Account {