From e4c10f1ca4dad6e7751ff160704d1ade237cdc11 Mon Sep 17 00:00:00 2001 From: Ivan Kupalov Date: Sun, 20 Sep 2020 18:43:28 +0200 Subject: [PATCH] Improve notifications fetching (#1930) * Improve notifications fetching - Only fetch notifications up to the latest fetched one - Use timeline markers to avoid showing already seen notifications * Apply some of the suggestions --- .../com/keylesspalace/tusky/MainActivity.kt | 7 +- .../notifications/NotificationFetcher.kt | 82 +++++++++++++++++++ .../notifications/NotificationWorker.kt | 71 +++------------- .../components/notifications/Notifier.kt | 20 +++++ .../com/keylesspalace/tusky/di/AppModule.kt | 8 +- .../com/keylesspalace/tusky/entity/Marker.kt | 15 ++++ .../tusky/network/MastodonApi.kt | 12 ++- 7 files changed, 150 insertions(+), 65 deletions(-) create mode 100644 app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationFetcher.kt create mode 100644 app/src/main/java/com/keylesspalace/tusky/components/notifications/Notifier.kt create mode 100644 app/src/main/java/com/keylesspalace/tusky/entity/Marker.kt diff --git a/app/src/main/java/com/keylesspalace/tusky/MainActivity.kt b/app/src/main/java/com/keylesspalace/tusky/MainActivity.kt index 59b4e937..085269f6 100644 --- a/app/src/main/java/com/keylesspalace/tusky/MainActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/MainActivity.kt @@ -425,7 +425,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje } ) - if(addSearchButton) { + if (addSearchButton) { mainDrawer.addItemsAtPosition(4, primaryDrawerItem { nameRes = R.string.action_search @@ -457,7 +457,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje private fun setupTabs(selectNotificationTab: Boolean) { - val activeTabLayout = if(preferences.getString("mainNavPosition", "top") == "bottom") { + val activeTabLayout = if (preferences.getString("mainNavPosition", "top") == "bottom") { val actionBarSize = ThemeUtils.getDimension(this, R.attr.actionBarSize) val fabMargin = resources.getDimensionPixelSize(R.dimen.fabMargin) (composeButton.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = actionBarSize + fabMargin @@ -621,10 +621,11 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje .transform( RoundedCorners(resources.getDimensionPixelSize(R.dimen.avatar_radius_36dp)) ) - .into(object : CustomTarget(){ + .into(object : CustomTarget() { override fun onResourceReady(resource: Drawable, transition: Transition?) { mainToolbar.navigationIcon = resource } + override fun onLoadCleared(placeholder: Drawable?) { mainToolbar.navigationIcon = placeholder } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationFetcher.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationFetcher.kt new file mode 100644 index 00000000..394a6846 --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationFetcher.kt @@ -0,0 +1,82 @@ +package com.keylesspalace.tusky.components.notifications + +import android.util.Log +import com.keylesspalace.tusky.db.AccountEntity +import com.keylesspalace.tusky.db.AccountManager +import com.keylesspalace.tusky.entity.Marker +import com.keylesspalace.tusky.entity.Notification +import com.keylesspalace.tusky.network.MastodonApi +import com.keylesspalace.tusky.util.isLessThan +import javax.inject.Inject + +class NotificationFetcher @Inject constructor( + private val mastodonApi: MastodonApi, + private val accountManager: AccountManager, + private val notifier: Notifier +) { + fun fetchAndShow() { + for (account in accountManager.getAllAccountsOrderedByActive()) { + if (account.notificationsEnabled) { + try { + val notifications = fetchNotifications(account) + notifications.forEachIndexed { index, notification -> + notifier.show(notification, account, index == 0) + } + accountManager.saveAccount(account) + } catch (e: Exception) { + Log.w(TAG, "Error while fetching notifications", e) + } + } + } + } + + private fun fetchNotifications(account: AccountEntity): MutableList { + val authHeader = String.format("Bearer %s", account.accessToken) + // We fetch marker to not load/show notifications which user has already seen + val marker = fetchMarker(authHeader, account) + if (marker != null && account.lastNotificationId.isLessThan(marker.lastReadId)) { + account.lastNotificationId = marker.lastReadId + } + Log.d(TAG, "getting Notifications for " + account.fullName) + val notifications = mastodonApi.notificationsWithAuth( + authHeader, + account.domain, + account.lastNotificationId + ).blockingGet() + + val newId = account.lastNotificationId + var newestId = "" + val result = mutableListOf() + for (notification in notifications.reversed()) { + val currentId = notification.id + if (newestId.isLessThan(currentId)) { + newestId = currentId + account.lastNotificationId = currentId + } + if (newId.isLessThan(currentId)) { + result.add(notification) + } + } + return result + } + + private fun fetchMarker(authHeader: String, account: AccountEntity): Marker? { + return try { + val allMarkers = mastodonApi.markersWithAuth( + authHeader, + account.domain, + listOf("notifications") + ).blockingGet() + val notificationMarker = allMarkers["notifications"] + Log.d(TAG, "Fetched marker: $notificationMarker") + notificationMarker + } catch (e: Exception) { + Log.e(TAG, "Failed to fetch marker", e) + null + } + } + + companion object { + const val TAG = "NotificationFetcher" + } +} \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationWorker.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationWorker.kt index 9abcd3ac..ae7d4d3f 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationWorker.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationWorker.kt @@ -16,82 +16,35 @@ package com.keylesspalace.tusky.components.notifications import android.content.Context -import android.util.Log import androidx.work.ListenableWorker import androidx.work.Worker import androidx.work.WorkerFactory import androidx.work.WorkerParameters -import com.keylesspalace.tusky.db.AccountEntity -import com.keylesspalace.tusky.db.AccountManager -import com.keylesspalace.tusky.entity.Notification -import com.keylesspalace.tusky.network.MastodonApi -import com.keylesspalace.tusky.util.isLessThan -import java.io.IOException import javax.inject.Inject class NotificationWorker( - private val context: Context, + context: Context, params: WorkerParameters, - private val mastodonApi: MastodonApi, - private val accountManager: AccountManager + private val notificationsFetcher: NotificationFetcher ) : Worker(context, params) { override fun doWork(): Result { - val accountList = accountManager.getAllAccountsOrderedByActive() - for (account in accountList) { - if (account.notificationsEnabled) { - try { - Log.d(TAG, "getting Notifications for " + account.fullName) - val notificationsResponse = mastodonApi.notificationsWithAuth( - String.format("Bearer %s", account.accessToken), - account.domain - ).execute() - val notifications = notificationsResponse.body() - if (notificationsResponse.isSuccessful && notifications != null) { - onNotificationsReceived(account, notifications) - } else { - Log.w(TAG, "error receiving notifications") - } - } catch (e: IOException) { - Log.w(TAG, "error receiving notifications", e) - } - } - } + notificationsFetcher.fetchAndShow() return Result.success() } - - private fun onNotificationsReceived(account: AccountEntity, notificationList: List) { - val newId = account.lastNotificationId - var newestId = "" - var isFirstOfBatch = true - notificationList.reversed().forEach { notification -> - val currentId = notification.id - if (newestId.isLessThan(currentId)) { - newestId = currentId - } - if (newId.isLessThan(currentId)) { - NotificationHelper.make(context, notification, account, isFirstOfBatch) - isFirstOfBatch = false - } - } - account.lastNotificationId = newestId - accountManager.saveAccount(account) - } - - companion object { - private const val TAG = "NotificationWorker" - } - } class NotificationWorkerFactory @Inject constructor( - val api: MastodonApi, - val accountManager: AccountManager -): WorkerFactory() { + private val notificationsFetcher: NotificationFetcher +) : WorkerFactory() { - override fun createWorker(appContext: Context, workerClassName: String, workerParameters: WorkerParameters): ListenableWorker? { - if(workerClassName == NotificationWorker::class.java.name) { - return NotificationWorker(appContext, workerParameters, api, accountManager) + override fun createWorker( + appContext: Context, + workerClassName: String, + workerParameters: WorkerParameters + ): ListenableWorker? { + if (workerClassName == NotificationWorker::class.java.name) { + return NotificationWorker(appContext, workerParameters, notificationsFetcher) } return null } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/Notifier.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/Notifier.kt new file mode 100644 index 00000000..35c33a9b --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/Notifier.kt @@ -0,0 +1,20 @@ +package com.keylesspalace.tusky.components.notifications + +import android.content.Context +import com.keylesspalace.tusky.db.AccountEntity +import com.keylesspalace.tusky.entity.Notification + +/** + * Shows notifications. + */ +interface Notifier { + fun show(notification: Notification, account: AccountEntity, isFirstInBatch: Boolean) +} + +class SystemNotifier( + private val context: Context +) : Notifier { + override fun show(notification: Notification, account: AccountEntity, isFirstInBatch: Boolean) { + NotificationHelper.make(context, notification, account, isFirstInBatch) + } +} \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt b/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt index e87ef289..aedcc3ae 100644 --- a/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt +++ b/app/src/main/java/com/keylesspalace/tusky/di/AppModule.kt @@ -25,6 +25,8 @@ import androidx.room.Room import com.keylesspalace.tusky.TuskyApplication import com.keylesspalace.tusky.appstore.EventHub import com.keylesspalace.tusky.appstore.EventHubImpl +import com.keylesspalace.tusky.components.notifications.Notifier +import com.keylesspalace.tusky.components.notifications.SystemNotifier import com.keylesspalace.tusky.db.AppDatabase import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.network.TimelineCases @@ -79,7 +81,11 @@ class AppModule { AppDatabase.MIGRATION_16_17, AppDatabase.MIGRATION_17_18, AppDatabase.MIGRATION_18_19, AppDatabase.MIGRATION_19_20, AppDatabase.MIGRATION_20_21, AppDatabase.MIGRATION_21_22, AppDatabase.MIGRATION_22_23) - .build() + .build() } + @Provides + @Singleton + fun notifier(context: Context): Notifier = SystemNotifier(context) + } \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/entity/Marker.kt b/app/src/main/java/com/keylesspalace/tusky/entity/Marker.kt new file mode 100644 index 00000000..16fd9e31 --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/entity/Marker.kt @@ -0,0 +1,15 @@ +package com.keylesspalace.tusky.entity + +import com.google.gson.annotations.SerializedName +import java.util.* + +/** + * API type for saving the scroll position of a timeline. + */ +data class Marker( + @SerializedName("last_read_id") + val lastReadId: String, + val version: Int, + @SerializedName("updated_at") + val updatedAt: Date +) \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt index 767fe257..d5b71ac1 100644 --- a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt +++ b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt @@ -99,11 +99,19 @@ interface MastodonApi { @Query("exclude_types[]") excludes: Set? ): Call> + @GET("api/v1/markers") + fun markersWithAuth( + @Header("Authorization") auth: String, + @Header(DOMAIN_HEADER) domain: String, + @Query("timeline[]") timelines: List + ): Single> + @GET("api/v1/notifications") fun notificationsWithAuth( @Header("Authorization") auth: String, - @Header(DOMAIN_HEADER) domain: String - ): Call> + @Header(DOMAIN_HEADER) domain: String, + @Query("since_id") sinceId: String? + ): Single> @POST("api/v1/notifications/clear") fun clearNotifications(): Call