Skip to content

Commit

Permalink
refactor: Use a single transaction for all remote mediator operations (…
Browse files Browse the repository at this point in the history 
…#1256)

Previous code used a transaction for updates to the database, but didn't
do the earlier reads in the same transaction. Theoretically this could
race.

Guard against this by using a single transaction for a complete remote
mediator operation (refresh, prepend, append).
  • Loading branch information
@nikclayton
nikclayton authored Feb 1, 2025
1 parent 26b1645 commit ecb0f7c
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 84 deletions.
90 changes: 45 additions & 45 deletions app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,58 +58,58 @@ class CachedTimelineRemoteMediator(
Timber.d("load(), account ID: %d, LoadType = %s", pachliAccountId, loadType)

return try {
val response = when (loadType) {
LoadType.REFRESH -> {
// Ignore the provided state, always try and fetch from the remote
// REFRESH key.
val statusId = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.REFRESH,
)?.key
Timber.d("Loading from item: %s", statusId)
getInitialPage(statusId, state.config.pageSize)
}
transactionProvider {
val response = when (loadType) {
LoadType.REFRESH -> {
// Ignore the provided state, always try and fetch from the remote
// REFRESH key.
val statusId = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.REFRESH,
)?.key
Timber.d("Loading from item: %s", statusId)
getInitialPage(statusId, state.config.pageSize)
}

LoadType.PREPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.PREV,
) ?: return MediatorResult.Success(endOfPaginationReached = true)
Timber.d("Loading from remoteKey: %s", rke)
mastodonApi.homeTimeline(minId = rke.key, limit = state.config.pageSize)
}
LoadType.PREPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.PREV,
) ?: return@transactionProvider MediatorResult.Success(endOfPaginationReached = true)
Timber.d("Loading from remoteKey: %s", rke)
mastodonApi.homeTimeline(minId = rke.key, limit = state.config.pageSize)
}

LoadType.APPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.NEXT,
) ?: return MediatorResult.Success(endOfPaginationReached = true)
Timber.d("Loading from remoteKey: %s", rke)
mastodonApi.homeTimeline(maxId = rke.key, limit = state.config.pageSize)
LoadType.APPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.NEXT,
) ?: return@transactionProvider MediatorResult.Success(endOfPaginationReached = true)
Timber.d("Loading from remoteKey: %s", rke)
mastodonApi.homeTimeline(maxId = rke.key, limit = state.config.pageSize)
}
}
}

val statuses = response.body()
if (!response.isSuccessful || statuses == null) {
return MediatorResult.Error(HttpException(response))
}
val statuses = response.body()
if (!response.isSuccessful || statuses == null) {
return@transactionProvider MediatorResult.Error(HttpException(response))
}

Timber.d("%d - # statuses loaded", statuses.size)
Timber.d("%d - # statuses loaded", statuses.size)

// This request succeeded with no new data, and pagination ends (unless this is a
// REFRESH, which must always set endOfPaginationReached to false).
if (statuses.isEmpty()) {
return MediatorResult.Success(endOfPaginationReached = loadType != LoadType.REFRESH)
}
// This request succeeded with no new data, and pagination ends (unless this is a
// REFRESH, which must always set endOfPaginationReached to false).
if (statuses.isEmpty()) {
return@transactionProvider MediatorResult.Success(endOfPaginationReached = loadType != LoadType.REFRESH)
}

Timber.d(" %s..%s", statuses.first().id, statuses.last().id)
Timber.d(" %s..%s", statuses.first().id, statuses.last().id)

val links = Links.from(response.headers()["link"])
val links = Links.from(response.headers()["link"])

transactionProvider {
when (loadType) {
LoadType.REFRESH -> {
remoteKeyDao.deletePrevNext(pachliAccountId, RKE_TIMELINE_ID)
Expand Down Expand Up @@ -159,9 +159,9 @@ class CachedTimelineRemoteMediator(
}
}
insertStatuses(pachliAccountId, statuses)
}

return MediatorResult.Success(endOfPaginationReached = false)
MediatorResult.Success(endOfPaginationReached = false)
}
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "Error loading, LoadType = %s", loadType)
Expand Down
77 changes: 38 additions & 39 deletions .../main/kotlin/app/pachli/core/data/repository/notifications/NotificationsRemoteMediator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,52 +62,51 @@ class NotificationsRemoteMediator(
private val notificationDao: NotificationDao,
private val statusDao: StatusDao,
) : RemoteMediator<Int, NotificationData>() {

override suspend fun load(loadType: LoadType, state: PagingState<Int, NotificationData>): MediatorResult {
return try {
val response = when (loadType) {
LoadType.REFRESH -> {
// Ignore the provided state, always try and fetch from the remote
// REFRESH key.
val notificationId = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.REFRESH,
)?.key
getInitialPage(notificationId, state.config.pageSize)
}
transactionProvider {
val response = when (loadType) {
LoadType.REFRESH -> {
// Ignore the provided state, always try and fetch from the remote
// REFRESH key.
val notificationId = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.REFRESH,
)?.key
getInitialPage(notificationId, state.config.pageSize)
}

LoadType.PREPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.PREV,
) ?: return MediatorResult.Success(endOfPaginationReached = true)
mastodonApi.notifications(minId = rke.key, limit = state.config.pageSize)
}
LoadType.PREPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.PREV,
) ?: return@transactionProvider MediatorResult.Success(endOfPaginationReached = true)
mastodonApi.notifications(minId = rke.key, limit = state.config.pageSize)
}

LoadType.APPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.NEXT,
) ?: return MediatorResult.Success(endOfPaginationReached = true)
mastodonApi.notifications(maxId = rke.key, limit = state.config.pageSize)
LoadType.APPEND -> {
val rke = remoteKeyDao.remoteKeyForKind(
pachliAccountId,
RKE_TIMELINE_ID,
RemoteKeyKind.NEXT,
) ?: return@transactionProvider MediatorResult.Success(endOfPaginationReached = true)
mastodonApi.notifications(maxId = rke.key, limit = state.config.pageSize)
}
}
}

val notifications = response.body()
if (!response.isSuccessful || notifications == null) {
return MediatorResult.Error(HttpException(response))
}
val notifications = response.body()
if (!response.isSuccessful || notifications == null) {
return@transactionProvider MediatorResult.Error(HttpException(response))
}

if (notifications.isEmpty()) {
return MediatorResult.Success(endOfPaginationReached = loadType != LoadType.REFRESH)
}
if (notifications.isEmpty()) {
return@transactionProvider MediatorResult.Success(endOfPaginationReached = loadType != LoadType.REFRESH)
}

val links = Links.from(response.headers()["link"])
val links = Links.from(response.headers()["link"])

transactionProvider {
when (loadType) {
LoadType.REFRESH -> {
remoteKeyDao.deletePrevNext(pachliAccountId, RKE_TIMELINE_ID)
Expand Down Expand Up @@ -156,9 +155,9 @@ class NotificationsRemoteMediator(
}

upsertNotifications(pachliAccountId, notifications)
}

MediatorResult.Success(endOfPaginationReached = false)
MediatorResult.Success(endOfPaginationReached = false)
}
} catch (e: Exception) {
currentCoroutineContext().ensureActive()
Timber.e(e, "error loading, loadtype = %s", loadType)
Expand Down

0 comments on commit ecb0f7c

Please sign in to comment.