Fetch all outstanding Mastodon notifications when creating Android no…

…tifications (#3700) * Fetch all outstanding Mastodon notifications when creating Android notifications Previous code fetched the oldest page of unfetched Mastodon notifications. If you had more than a page of Mastodon notifications you'd get Android notifications for that page, then ~ 15 minutes later Android notifications for the next page, and so on. This code fetches all the outstanding notifications at once. If this results in more than 40 total notifications the list is still trimmed so that a maximum of 40 Android notifications is displayed. Fixes #3648 * Build the list using buildList
tuskyapp · Jun 1, 2023 · 01b3cb3 · 01b3cb3
1 parent 346dabf
commit 01b3cb3
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 34 deletions.
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
@@ -11,6 +11,7 @@ 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 kotlinx.coroutines.runBlocking
 import javax.inject.Inject
 import kotlin.math.min
 
@@ -35,10 +36,12 @@ class NotificationFetcher @Inject constructor(
                     val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
 
                     // Create sorted list of new notifications
-                    val notifications = fetchNewNotifications(account)
-                        .filter { filterNotification(notificationManager, account, it) }
-                        .sortedWith(compareBy({ it.id.length }, { it.id })) // oldest notifications first
-                        .toMutableList()
+                    val notifications = runBlocking { // OK, because in a worker thread
+                        fetchNewNotifications(account)
+                            .filter { filterNotification(notificationManager, account, it) }
+                            .sortedWith(compareBy({ it.id.length }, { it.id })) // oldest notifications first
+                            .toMutableList()
+                    }
 
                     // There's a maximum limit on the number of notifications an Android app
                     // can display. If the total number of notifications (current notifications,
@@ -114,7 +117,7 @@ class NotificationFetcher @Inject constructor(
      * ones that were last fetched here. So `lastNotificationId` takes precedence if it is greater
      * than the marker.
      */
-    private fun fetchNewNotifications(account: AccountEntity): List<Notification> {
+    private suspend fun fetchNewNotifications(account: AccountEntity): List<Notification> {
         val authHeader = String.format("Bearer %s", account.accessToken)
 
         // Figure out where to read from. Choose the most recent notification ID from:
@@ -128,21 +131,37 @@ class NotificationFetcher @Inject constructor(
         val markerId = if (remoteMarkerId.isLessThan(localMarkerId)) localMarkerId else remoteMarkerId
         val readingPosition = account.lastNotificationId
 
-        val minId = if (readingPosition.isLessThan(markerId)) markerId else readingPosition
+        var minId: String? = if (readingPosition.isLessThan(markerId)) markerId else readingPosition
         Log.d(TAG, "  remoteMarkerId: $remoteMarkerId")
         Log.d(TAG, "  localMarkerId: $localMarkerId")
         Log.d(TAG, "  readingPosition: $readingPosition")
 
         Log.d(TAG, "getting Notifications for ${account.fullName}, min_id: $minId")
 
-        val notifications = mastodonApi.notificationsWithAuth(
-            authHeader,
-            account.domain,
-            minId
-        ).blockingGet()
+        // Fetch all outstanding notifications
+        val notifications = buildList {
+            while (minId != null) {
+                val response = mastodonApi.notificationsWithAuth(
+                    authHeader,
+                    account.domain,
+                    minId = minId
+                )
+                if (!response.isSuccessful) break
+
+                // Notifications are returned in the page in order, newest first,
+                // (https://github.com/mastodon/documentation/issues/1226), insert the
+                // new page at the head of the list.
+                response.body()?.let { addAll(0, it) }
+
+                // Get the previous page, which will be chronologically newer
+                // notifications. If it doesn't exist this is null and the loop
+                // will exit.
+                val links = Links.from(response.headers()["link"])
+                minId = links.prev
+            }
+        }
 
-        // Notifications are returned in order, most recent first. Save the newest notification ID
-        // in the marker.
+        // Save the newest notification ID in the marker.
         notifications.firstOrNull()?.let {
             val newMarkerId = notifications.first().id
             Log.d(TAG, "updating notification marker for ${account.fullName} to: $newMarkerId")
@@ -158,13 +177,13 @@ class NotificationFetcher @Inject constructor(
         return notifications
     }
 
-    private fun fetchMarker(authHeader: String, account: AccountEntity): Marker? {
+    private suspend 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 for ${account.fullName}: $notificationMarker")
             notificationMarker

diff --git a/...c/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt b/...c/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt
@@ -31,7 +31,21 @@ import retrofit2.Response
 import javax.inject.Inject
 
 /** Models next/prev links from the "Links" header in an API response */
-data class Links(val next: String?, val prev: String?)
+data class Links(val next: String?, val prev: String?) {
+    companion object {
+        fun from(linkHeader: String?): Links {
+            val links = HttpHeaderLink.parse(linkHeader)
+            return Links(
+                next = HttpHeaderLink.findByRelationType(links, "next")?.uri?.getQueryParameter(
+                    "max_id"
+                ),
+                prev = HttpHeaderLink.findByRelationType(links, "prev")?.uri?.getQueryParameter(
+                    "min_id"
+                )
+            )
+        }
+    }
+}
 
 /** [PagingSource] for Mastodon Notifications, identified by the Notification ID */
 class NotificationsPagingSource @Inject constructor(
@@ -79,7 +93,7 @@ class NotificationsPagingSource @Inject constructor(
                 return LoadResult.Error(Throwable("HTTP $code: $msg"))
             }
 
-            val links = getPageLinks(response.headers()["link"])
+            val links = Links.from(response.headers()["link"])
             return LoadResult.Page(
                 data = response.body()!!,
                 nextKey = links.next,
@@ -188,18 +202,6 @@ class NotificationsPagingSource @Inject constructor(
         )
     }
 
-    private fun getPageLinks(linkHeader: String?): Links {
-        val links = HttpHeaderLink.parse(linkHeader)
-        return Links(
-            next = HttpHeaderLink.findByRelationType(links, "next")?.uri?.getQueryParameter(
-                "max_id"
-            ),
-            prev = HttpHeaderLink.findByRelationType(links, "prev")?.uri?.getQueryParameter(
-                "min_id"
-            )
-        )
-    }
-
     override fun getRefreshKey(state: PagingState<String, Notification>): String? {
         return state.anchorPosition?.let { anchorPosition ->
             val anchorPage = state.closestPageToPosition(anchorPosition)

diff --git a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt
@@ -146,27 +146,28 @@ interface MastodonApi {
     ): Response<Notification>
 
     @GET("api/v1/markers")
-    fun markersWithAuth(
+    suspend fun markersWithAuth(
         @Header("Authorization") auth: String,
         @Header(DOMAIN_HEADER) domain: String,
         @Query("timeline[]") timelines: List<String>
-    ): Single<Map<String, Marker>>
+    ): Map<String, Marker>
 
     @FormUrlEncoded
     @POST("api/v1/markers")
-    fun updateMarkersWithAuth(
+    suspend fun updateMarkersWithAuth(
         @Header("Authorization") auth: String,
         @Header(DOMAIN_HEADER) domain: String,
         @Field("home[last_read_id]") homeLastReadId: String? = null,
         @Field("notifications[last_read_id]") notificationsLastReadId: String? = null
     ): NetworkResult<Unit>
 
     @GET("api/v1/notifications")
-    fun notificationsWithAuth(
+    suspend fun notificationsWithAuth(
         @Header("Authorization") auth: String,
         @Header(DOMAIN_HEADER) domain: String,
+        /** Return results immediately newer than this ID */
         @Query("min_id") minId: String?
-    ): Single<List<Notification>>
+    ): Response<List<Notification>>
 
     @POST("api/v1/notifications/clear")
     suspend fun clearNotifications(): Response<ResponseBody>