Save the a copy of the notification marker ID locally (#3672)
Not all servers support the marker API. If they don't the user will potentially get duplicate Android notifications. To resolve this, store a copy of the notification marker ID locally as well. Defer to the remote marker if it exists (and is newer than the local marker). Fixes https://github.com/tuskyapp/Tusky/issues/3671
This commit is contained in:
parent
f69c2646ca
commit
d644f6e6f3
4 changed files with 1037 additions and 7 deletions
1002
app/schemas/com.keylesspalace.tusky.db.AppDatabase/51.json
Normal file
1002
app/schemas/com.keylesspalace.tusky.db.AppDatabase/51.json
Normal file
File diff suppressed because it is too large
Load diff
|
@ -102,11 +102,13 @@ class NotificationFetcher @Inject constructor(
|
||||||
* Here, "new" means "notifications with IDs newer than notifications the user has already
|
* Here, "new" means "notifications with IDs newer than notifications the user has already
|
||||||
* seen."
|
* seen."
|
||||||
*
|
*
|
||||||
* The "water mark" for Mastodon Notification IDs are stored in two places.
|
* The "water mark" for Mastodon Notification IDs are stored in three places.
|
||||||
*
|
*
|
||||||
* - acccount.lastNotificationId -- the ID of the top-most notification when the user last
|
* - acccount.lastNotificationId -- the ID of the top-most notification when the user last
|
||||||
* left the Notifications tab.
|
* left the Notifications tab.
|
||||||
* - The Mastodon "marker" API -- the ID of the most recent notification fetched here.
|
* - The Mastodon "marker" API -- the ID of the most recent notification fetched here.
|
||||||
|
* - account.notificationMarkerId -- local version of the value from the Mastodon marker
|
||||||
|
* API, in case the Mastodon server does not implement that API.
|
||||||
*
|
*
|
||||||
* The user may have refreshed the "Notifications" tab and seen notifications newer than the
|
* The user may have refreshed the "Notifications" tab and seen notifications newer than the
|
||||||
* ones that were last fetched here. So `lastNotificationId` takes precedence if it is greater
|
* ones that were last fetched here. So `lastNotificationId` takes precedence if it is greater
|
||||||
|
@ -115,11 +117,21 @@ class NotificationFetcher @Inject constructor(
|
||||||
private fun fetchNewNotifications(account: AccountEntity): List<Notification> {
|
private fun fetchNewNotifications(account: AccountEntity): List<Notification> {
|
||||||
val authHeader = String.format("Bearer %s", account.accessToken)
|
val authHeader = String.format("Bearer %s", account.accessToken)
|
||||||
|
|
||||||
|
// Figure out where to read from. Choose the most recent notification ID from:
|
||||||
|
//
|
||||||
|
// - The Mastodon marker API (if the server supports it)
|
||||||
|
// - account.notificationMarkerId
|
||||||
|
// - account.lastNotificationId
|
||||||
Log.d(TAG, "getting notification marker for ${account.fullName}")
|
Log.d(TAG, "getting notification marker for ${account.fullName}")
|
||||||
val minId = when (val marker = fetchMarker(authHeader, account)) {
|
val remoteMarkerId = fetchMarker(authHeader, account)?.lastReadId ?: "0"
|
||||||
null -> account.lastNotificationId.takeIf { it != "0" }
|
val localMarkerId = account.notificationMarkerId
|
||||||
else -> if (account.lastNotificationId.isLessThan(marker.lastReadId)) marker.lastReadId else account.lastNotificationId
|
val markerId = if (remoteMarkerId.isLessThan(localMarkerId)) localMarkerId else remoteMarkerId
|
||||||
}
|
val readingPosition = account.lastNotificationId
|
||||||
|
|
||||||
|
val minId = 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")
|
Log.d(TAG, "getting Notifications for ${account.fullName}, min_id: $minId")
|
||||||
|
|
||||||
|
@ -139,6 +151,8 @@ class NotificationFetcher @Inject constructor(
|
||||||
domain = account.domain,
|
domain = account.domain,
|
||||||
notificationsLastReadId = newMarkerId
|
notificationsLastReadId = newMarkerId
|
||||||
)
|
)
|
||||||
|
account.notificationMarkerId = newMarkerId
|
||||||
|
accountManager.saveAccount(account)
|
||||||
}
|
}
|
||||||
|
|
||||||
return notifications
|
return notifications
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
|
|
||||||
package com.keylesspalace.tusky.db
|
package com.keylesspalace.tusky.db
|
||||||
|
|
||||||
|
import androidx.room.ColumnInfo
|
||||||
import androidx.room.Entity
|
import androidx.room.Entity
|
||||||
import androidx.room.Index
|
import androidx.room.Index
|
||||||
import androidx.room.PrimaryKey
|
import androidx.room.PrimaryKey
|
||||||
|
@ -70,7 +71,19 @@ data class AccountEntity(
|
||||||
* that media previews are shown as well as downloaded.
|
* that media previews are shown as well as downloaded.
|
||||||
*/
|
*/
|
||||||
var mediaPreviewEnabled: Boolean = true,
|
var mediaPreviewEnabled: Boolean = true,
|
||||||
|
/**
|
||||||
|
* ID of the last notification the user read on the Notification, list, and should be restored
|
||||||
|
* to view when the user returns to the list.
|
||||||
|
*
|
||||||
|
* May not be the ID of the most recent notification if the user has scrolled down the list.
|
||||||
|
*/
|
||||||
var lastNotificationId: String = "0",
|
var lastNotificationId: String = "0",
|
||||||
|
/**
|
||||||
|
* ID of the most recent Mastodon notification that Tusky has fetched to show as an
|
||||||
|
* Android notification.
|
||||||
|
*/
|
||||||
|
@ColumnInfo(defaultValue = "0")
|
||||||
|
var notificationMarkerId: String = "0",
|
||||||
var emojis: List<Emoji> = emptyList(),
|
var emojis: List<Emoji> = emptyList(),
|
||||||
var tabPreferences: List<TabData> = defaultTabs(),
|
var tabPreferences: List<TabData> = defaultTabs(),
|
||||||
var notificationsFilter: String = "[\"follow_request\"]",
|
var notificationsFilter: String = "[\"follow_request\"]",
|
||||||
|
|
|
@ -41,10 +41,11 @@ import java.io.File;
|
||||||
TimelineAccountEntity.class,
|
TimelineAccountEntity.class,
|
||||||
ConversationEntity.class
|
ConversationEntity.class
|
||||||
},
|
},
|
||||||
version = 50,
|
version = 51,
|
||||||
autoMigrations = {
|
autoMigrations = {
|
||||||
@AutoMigration(from = 48, to = 49),
|
@AutoMigration(from = 48, to = 49),
|
||||||
@AutoMigration(from = 49, to = 50, spec = AppDatabase.MIGRATION_49_50.class)
|
@AutoMigration(from = 49, to = 50, spec = AppDatabase.MIGRATION_49_50.class),
|
||||||
|
@AutoMigration(from = 50, to = 51)
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
public abstract class AppDatabase extends RoomDatabase {
|
public abstract class AppDatabase extends RoomDatabase {
|
||||||
|
|
Loading…
Reference in a new issue