From 01b3cb3a53b1e08ed26e747ddbddb1e7b891ce9d Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Thu, 1 Jun 2023 19:31:30 +0200 Subject: [PATCH] Fetch all outstanding Mastodon notifications when creating Android notifications (#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 https://github.com/tuskyapp/Tusky/issues/3648 * Build the list using buildList --- .../notifications/NotificationFetcher.kt | 49 +++++++++++++------ .../NotificationsPagingSource.kt | 30 ++++++------ .../tusky/network/MastodonApi.kt | 11 +++-- 3 files changed, 56 insertions(+), 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 index 959bce9b5..89f4222bd 100644 --- 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 { + private suspend fun fetchNewNotifications(account: AccountEntity): List { 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 order, most recent first. Save the newest notification ID - // in the marker. + // 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 + } + } + + // 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/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt index a4d322f78..480d5d681 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt +++ b/app/src/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? { 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 index 4a7162d16..3eb838406 100644 --- a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt +++ b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt @@ -146,15 +146,15 @@ interface MastodonApi { ): Response @GET("api/v1/markers") - fun markersWithAuth( + suspend fun markersWithAuth( @Header("Authorization") auth: String, @Header(DOMAIN_HEADER) domain: String, @Query("timeline[]") timelines: List - ): Single> + ): Map @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, @@ -162,11 +162,12 @@ interface MastodonApi { ): NetworkResult @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> + ): Response> @POST("api/v1/notifications/clear") suspend fun clearNotifications(): Response