From 940fe634c4b00d174a6e3c7c00890be39ffc7cc9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Nov 2021 12:43:11 +0000 Subject: [PATCH] fixing notifications not dismissing when the in memory state becomes out of sync - lazily loads the initial notification state as we rely on a current session to exist - extracts all the notification state manipulation to a separate state class --- .../java/im/vector/app/VectorApplication.kt | 3 +- .../im/vector/app/features/MainActivity.kt | 1 - .../NotificationDrawerManager.kt | 130 +++++------------- .../notifications/NotificationState.kt | 121 ++++++++++++++++ 4 files changed, 155 insertions(+), 100 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt diff --git a/vector/src/main/java/im/vector/app/VectorApplication.kt b/vector/src/main/java/im/vector/app/VectorApplication.kt index c1d9eef125..46f155a8d9 100644 --- a/vector/src/main/java/im/vector/app/VectorApplication.kt +++ b/vector/src/main/java/im/vector/app/VectorApplication.kt @@ -175,8 +175,7 @@ class VectorApplication : } override fun onPause(owner: LifecycleOwner) { - Timber.i("App entered background") // call persistInfo - notificationDrawerManager.persistInfo() + Timber.i("App entered background") FcmHelper.onEnterBackground(appContext, vectorPreferences, activeSessionHolder) } }) diff --git a/vector/src/main/java/im/vector/app/features/MainActivity.kt b/vector/src/main/java/im/vector/app/features/MainActivity.kt index 8e0995b426..f4c737c942 100644 --- a/vector/src/main/java/im/vector/app/features/MainActivity.kt +++ b/vector/src/main/java/im/vector/app/features/MainActivity.kt @@ -116,7 +116,6 @@ class MainActivity : VectorBaseActivity(), UnlockedActivity private fun clearNotifications() { // Dismiss all notifications notificationDrawerManager.clearAllEvents() - notificationDrawerManager.persistInfo() // Also clear the dynamic shortcuts shortcutsHandler.clearShortcuts() diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt index e86a65c3fe..2d65aa0c24 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt @@ -29,8 +29,6 @@ import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.content.ContentUrlResolver import org.matrix.android.sdk.api.util.toMatrixItem import timber.log.Timber -import java.io.File -import java.io.FileOutputStream import javax.inject.Inject import javax.inject.Singleton @@ -50,42 +48,24 @@ class NotificationDrawerManager @Inject constructor(private val context: Context private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY) private var backgroundHandler: Handler - init { - handlerThread.start() - backgroundHandler = Handler(handlerThread.looper) - } - - /** - * The notifiable events to render - * this is our source of truth for notifications, any changes to this list will be rendered as notifications - * when events are removed the previously rendered notifications will be cancelled - * when adding or updating, the notifications will be notified - * - * Events are unique by their properties, we should be careful not to insert multiple events with the same event-id - */ - private val queuedEvents = loadEventInfo() - - /** - * The last known rendered notifiable events - * we keep track of them in order to know which events have been removed from the eventList - * allowing us to cancel any notifications previous displayed by now removed events - */ - private var renderedEvents = emptyList>() - private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) - private var currentRoomId: String? = null - // TODO Multi-session: this will have to be improved private val currentSession: Session? get() = activeSessionDataSource.currentValue?.orNull() + /** + * Lazily initializes the NotificationState as we rely on having a current session in order to fetch the persisted queue of events + */ + private val notificationState by lazy { NotificationState.createInitialNotificationState(context, currentSession) } + private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) + private var currentRoomId: String? = null + private val firstThrottler = FirstThrottler(200) + private var useCompleteNotificationFormat = vectorPreferences.useCompleteNotificationFormat() - /** - * An in memory FIFO cache of the seen events. - * Acts as a notification debouncer to stop already dismissed push notifications from - * displaying again when the /sync response is delayed. - */ - private val seenEventIds = CircularCache.create(cacheSize = 25) + init { + handlerThread.start() + backgroundHandler = Handler(handlerThread.looper) + } /** Should be called as soon as a new event is ready to be displayed. @@ -106,7 +86,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}") } - add(notifiableEvent, seenEventIds) + add(notifiableEvent, notificationState.seenEventIds) } /** @@ -142,14 +122,12 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } fun updateEvents(action: NotificationDrawerManager.(NotificationEventQueue) -> Unit) { - synchronized(queuedEvents) { - action(this, queuedEvents) + notificationState.updateQueuedEvents(this) { queuedEvents, _ -> + action(queuedEvents) } refreshNotificationDrawer() } - private var firstThrottler = FirstThrottler(200) - private fun refreshNotificationDrawer() { // Implement last throttler val canHandle = firstThrottler.canHandle() @@ -171,85 +149,43 @@ class NotificationDrawerManager @Inject constructor(private val context: Context @WorkerThread private fun refreshNotificationDrawerBg() { Timber.v("refreshNotificationDrawerBg()") - val eventsToRender = synchronized(queuedEvents) { + val eventsToRender = notificationState.updateQueuedEvents(this) { queuedEvents, renderedEvents -> notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, renderedEvents).also { queuedEvents.clearAndAdd(it.onlyKeptEvents()) } } - if (renderedEvents == eventsToRender) { + if (notificationState.hasAlreadyRendered(eventsToRender)) { Timber.d("Skipping notification update due to event list not changing") } else { - renderedEvents = eventsToRender + notificationState.clearAndAddRenderedEvents(eventsToRender) val session = currentSession ?: return - val user = session.getUser(session.myUserId) - // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash - val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId - val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail( - contentUrl = user?.avatarUrl, - width = avatarSize, - height = avatarSize, - method = ContentUrlResolver.ThumbnailMethod.SCALE - ) - notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender) + renderEvents(session, eventsToRender) + notificationState.persistState(context, session) } } + private fun renderEvents(session: Session, eventsToRender: List>) { + val user = session.getUser(session.myUserId) + // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash + val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId + val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail( + contentUrl = user?.avatarUrl, + width = avatarSize, + height = avatarSize, + method = ContentUrlResolver.ThumbnailMethod.SCALE + ) + notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender) + } + fun shouldIgnoreMessageEventInRoom(roomId: String?): Boolean { return currentRoomId != null && roomId == currentRoomId } - fun persistInfo() { - synchronized(queuedEvents) { - if (queuedEvents.isEmpty()) { - deleteCachedRoomNotifications() - return - } - try { - val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) - if (!file.exists()) file.createNewFile() - FileOutputStream(file).use { - currentSession?.securelyStoreObject(queuedEvents.rawEvents(), KEY_ALIAS_SECRET_STORAGE, it) - } - } catch (e: Throwable) { - Timber.e(e, "## Failed to save cached notification info") - } - } - } - - private fun loadEventInfo(): NotificationEventQueue { - try { - val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) - if (file.exists()) { - file.inputStream().use { - val events: ArrayList? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE) - if (events != null) { - return NotificationEventQueue(events.toMutableList()) - } - } - } - } catch (e: Throwable) { - Timber.e(e, "## Failed to load cached notification info") - } - return NotificationEventQueue() - } - - private fun deleteCachedRoomNotifications() { - val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) - if (file.exists()) { - file.delete() - } - } - companion object { const val SUMMARY_NOTIFICATION_ID = 0 const val ROOM_MESSAGES_NOTIFICATION_ID = 1 const val ROOM_EVENT_NOTIFICATION_ID = 2 const val ROOM_INVITATION_NOTIFICATION_ID = 3 - - // TODO Mutliaccount - private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache" - - private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr" } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt new file mode 100644 index 0000000000..62a64b1d96 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt @@ -0,0 +1,121 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.notifications + +import android.content.Context +import org.matrix.android.sdk.api.session.Session +import timber.log.Timber +import java.io.File +import java.io.FileOutputStream + +// TODO Mutliaccount +private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache" +private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr" + +class NotificationState( + /** + * The notifiable events to render + * this is our source of truth for notifications, any changes to this list will be rendered as notifications + * when events are removed the previously rendered notifications will be cancelled + * when adding or updating, the notifications will be notified + * + * Events are unique by their properties, we should be careful not to insert multiple events with the same event-id + */ + private val queuedEvents: NotificationEventQueue, + + /** + * The last known rendered notifiable events + * we keep track of them in order to know which events have been removed from the eventList + * allowing us to cancel any notifications previous displayed by now removed events + */ + private val renderedEvents: MutableList>, + + /** + * An in memory FIFO cache of the seen events. + * Acts as a notification debouncer to stop already dismissed push notifications from + * displaying again when the /sync response is delayed. + */ + val seenEventIds: CircularCache +) { + + fun updateQueuedEvents(drawerManager: NotificationDrawerManager, action: NotificationDrawerManager.(NotificationEventQueue, List>) -> T): T { + return synchronized(queuedEvents) { + action(drawerManager, queuedEvents, renderedEvents) + } + } + + fun clearAndAddRenderedEvents(eventsToRender: List>) { + renderedEvents.clear() + renderedEvents.addAll(eventsToRender) + } + + fun hasAlreadyRendered(eventsToRender: List>) = renderedEvents == eventsToRender + + fun persistState(context: Context, currentSession: Session) { + synchronized(queuedEvents) { + if (queuedEvents.isEmpty()) { + deleteCachedRoomNotifications(context) + return + } + try { + val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) + if (!file.exists()) file.createNewFile() + FileOutputStream(file).use { + currentSession.securelyStoreObject(queuedEvents.rawEvents(), KEY_ALIAS_SECRET_STORAGE, it) + } + } catch (e: Throwable) { + Timber.e(e, "## Failed to save cached notification info") + } + } + } + + private fun deleteCachedRoomNotifications(context: Context) { + val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) + if (file.exists()) { + file.delete() + } + } + + companion object { + + fun createInitialNotificationState(context: Context, currentSession: Session?): NotificationState { + val queuedEvents = loadEventInfo(context, currentSession) + return NotificationState( + queuedEvents = queuedEvents, + renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList(), + seenEventIds = CircularCache.create(cacheSize = 25) + ) + } + + private fun loadEventInfo(context: Context, currentSession: Session?): NotificationEventQueue { + try { + val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) + if (file.exists()) { + file.inputStream().use { + val events: ArrayList? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE) + if (events != null) { + return NotificationEventQueue(events.toMutableList()) + } + } + } + } catch (e: Throwable) { + Timber.e(e, "## Failed to load cached notification info") + } + return NotificationEventQueue() + } + } +}