From 497b434663e2120bcc7ea2b0b8fef58850b96416 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Wed, 2 Mar 2022 20:40:06 +0100 Subject: [PATCH] Improve timeline dao (#2353) * improve TimelineDao methods * remove @Transaction from cleanup methods --- .../tusky/appstore/CacheUpdater.kt | 11 ++------ .../viewmodel/CachedTimelineViewModel.kt | 7 ++--- .../com/keylesspalace/tusky/db/TimelineDao.kt | 27 ++++++++++++++++--- .../CachedTimelineRemoteMediatorTest.kt | 2 +- .../keylesspalace/tusky/db/TimelineDaoTest.kt | 14 +++++----- 5 files changed, 35 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/appstore/CacheUpdater.kt b/app/src/main/java/com/keylesspalace/tusky/appstore/CacheUpdater.kt index 50f96b26..12cb4a69 100644 --- a/app/src/main/java/com/keylesspalace/tusky/appstore/CacheUpdater.kt +++ b/app/src/main/java/com/keylesspalace/tusky/appstore/CacheUpdater.kt @@ -3,9 +3,7 @@ package com.keylesspalace.tusky.appstore import com.google.gson.Gson import com.keylesspalace.tusky.db.AccountManager import com.keylesspalace.tusky.db.AppDatabase -import io.reactivex.rxjava3.core.Single import io.reactivex.rxjava3.disposables.Disposable -import io.reactivex.rxjava3.schedulers.Schedulers import javax.inject.Inject class CacheUpdater @Inject constructor( @@ -47,12 +45,7 @@ class CacheUpdater @Inject constructor( this.disposable.dispose() } - fun clearForUser(accountId: Long) { - Single.fromCallable { - appDatabase.timelineDao().removeAllForAccount(accountId) - appDatabase.timelineDao().removeAllUsersForAccount(accountId) - } - .subscribeOn(Schedulers.io()) - .subscribe() + suspend fun clearForUser(accountId: Long) { + appDatabase.timelineDao().removeAll(accountId) } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt index 53858d1b..f4bbabe4 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt @@ -70,7 +70,7 @@ class CachedTimelineViewModel @Inject constructor( override val statuses = Pager( config = PagingConfig(pageSize = LOAD_AT_ONCE), remoteMediator = CachedTimelineRemoteMediator(accountManager, api, db, gson), - pagingSourceFactory = { db.timelineDao().getStatusesForAccount(accountManager.activeAccount!!.id) } + pagingSourceFactory = { db.timelineDao().getStatuses(accountManager.activeAccount!!.id) } ).flow .map { pagingData -> pagingData.map { timelineStatus -> @@ -214,10 +214,7 @@ class CachedTimelineViewModel @Inject constructor( override fun fullReload() { viewModelScope.launch { val activeAccount = accountManager.activeAccount!! - db.runInTransaction { - db.timelineDao().removeAllForAccount(activeAccount.id) - db.timelineDao().removeAllUsersForAccount(activeAccount.id) - } + db.timelineDao().removeAll(activeAccount.id) } } diff --git a/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt b/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt index ac712f66..7915ff99 100644 --- a/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt +++ b/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt @@ -51,7 +51,7 @@ LEFT JOIN TimelineAccountEntity rb ON (s.timelineUserId = rb.timelineUserId AND WHERE s.timelineUserId = :account ORDER BY LENGTH(s.serverId) DESC, s.serverId DESC""" ) - abstract fun getStatusesForAccount(account: Long): PagingSource + abstract fun getStatuses(account: Long): PagingSource @Query( """DELETE FROM TimelineStatusEntity WHERE timelineUserId = :accountId AND @@ -86,11 +86,20 @@ WHERE timelineUserId = :accountId AND (serverId = :statusId OR reblogServerId = ) abstract fun removeAllByUser(accountId: Long, userId: String) + /** + * Removes everything in the TimelineStatusEntity and TimelineAccountEntity tables for one user account + * @param accountId id of the account for which to clean tables + */ + suspend fun removeAll(accountId: Long) { + removeAllStatuses(accountId) + removeAllAccounts(accountId) + } + @Query("DELETE FROM TimelineStatusEntity WHERE timelineUserId = :accountId") - abstract fun removeAllForAccount(accountId: Long) + abstract suspend fun removeAllStatuses(accountId: Long) @Query("DELETE FROM TimelineAccountEntity WHERE timelineUserId = :accountId") - abstract fun removeAllUsersForAccount(accountId: Long) + abstract suspend fun removeAllAccounts(accountId: Long) @Query( """DELETE FROM TimelineStatusEntity WHERE timelineUserId = :accountId @@ -98,6 +107,16 @@ AND serverId = :statusId""" ) abstract fun delete(accountId: Long, statusId: String) + /** + * Cleans the TimelineStatusEntity and TimelineAccountEntity tables from old entries. + * @param accountId id of the account for which to clean tables + * @param limit how many statuses to keep + */ + suspend fun cleanup(accountId: Long, limit: Int) { + cleanupStatuses(accountId, limit) + cleanupAccounts(accountId) + } + /** * Cleans the TimelineStatusEntity table from old status entries. * @param accountId id of the account for which to clean statuses @@ -108,7 +127,7 @@ AND serverId = :statusId""" (SELECT serverId FROM TimelineStatusEntity WHERE timelineUserId = :accountId ORDER BY LENGTH(serverId) DESC, serverId DESC LIMIT :limit) """ ) - abstract suspend fun cleanup(accountId: Long, limit: Int) + abstract suspend fun cleanupStatuses(accountId: Long, limit: Int) /** * Cleans the TimelineAccountEntity table from accounts that are no longer referenced in the TimelineStatusEntity table diff --git a/app/src/test/java/com/keylesspalace/tusky/components/timeline/CachedTimelineRemoteMediatorTest.kt b/app/src/test/java/com/keylesspalace/tusky/components/timeline/CachedTimelineRemoteMediatorTest.kt index ad633f3d..4fbeb5d4 100644 --- a/app/src/test/java/com/keylesspalace/tusky/components/timeline/CachedTimelineRemoteMediatorTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/components/timeline/CachedTimelineRemoteMediatorTest.kt @@ -514,7 +514,7 @@ class CachedTimelineRemoteMediatorTest { expected: List, forAccount: Long = 1 ) { - val pagingSource = timelineDao().getStatusesForAccount(forAccount) + val pagingSource = timelineDao().getStatuses(forAccount) val loadResult = runBlocking { pagingSource.load(PagingSource.LoadParams.Refresh(null, 100, false)) diff --git a/app/src/test/java/com/keylesspalace/tusky/db/TimelineDaoTest.kt b/app/src/test/java/com/keylesspalace/tusky/db/TimelineDaoTest.kt index 1ddda3e0..889e5f98 100644 --- a/app/src/test/java/com/keylesspalace/tusky/db/TimelineDaoTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/db/TimelineDaoTest.kt @@ -53,7 +53,7 @@ class TimelineDaoTest { timelineDao.insertStatus(status) } - val pagingSource = timelineDao.getStatusesForAccount(setOne.first.timelineUserId) + val pagingSource = timelineDao.getStatuses(setOne.first.timelineUserId) val loadResult = pagingSource.load(PagingSource.LoadParams.Refresh(null, 2, false)) @@ -96,7 +96,7 @@ class TimelineDaoTest { val loadParams: PagingSource.LoadParams = PagingSource.LoadParams.Refresh(null, 100, false) - val loadedStatuses = (timelineDao.getStatusesForAccount(1).load(loadParams) as PagingSource.LoadResult.Page).data + val loadedStatuses = (timelineDao.getStatuses(1).load(loadParams) as PagingSource.LoadResult.Page).data assertStatuses(statusesAfterCleanup, loadedStatuses) @@ -157,7 +157,7 @@ class TimelineDaoTest { // make sure status 2 is no longer in db - val pagingSource = timelineDao.getStatusesForAccount(1) + val pagingSource = timelineDao.getStatuses(1) val loadResult = pagingSource.load(PagingSource.LoadParams.Refresh(null, 100, false)) @@ -197,8 +197,8 @@ class TimelineDaoTest { val loadParams: PagingSource.LoadParams = PagingSource.LoadParams.Refresh(null, 100, false) - val statusesAccount1 = (timelineDao.getStatusesForAccount(1).load(loadParams) as PagingSource.LoadResult.Page).data - val statusesAccount2 = (timelineDao.getStatusesForAccount(2).load(loadParams) as PagingSource.LoadResult.Page).data + val statusesAccount1 = (timelineDao.getStatuses(1).load(loadParams) as PagingSource.LoadResult.Page).data + val statusesAccount2 = (timelineDao.getStatuses(2).load(loadParams) as PagingSource.LoadResult.Page).data val remainingStatusesAccount1 = listOf( makeStatus(statusId = 100), @@ -269,8 +269,8 @@ class TimelineDaoTest { val loadParams: PagingSource.LoadParams = PagingSource.LoadParams.Refresh(null, 100, false) - val statusesAccount1 = (timelineDao.getStatusesForAccount(1).load(loadParams) as PagingSource.LoadResult.Page).data - val statusesAccount2 = (timelineDao.getStatusesForAccount(2).load(loadParams) as PagingSource.LoadResult.Page).data + val statusesAccount1 = (timelineDao.getStatuses(1).load(loadParams) as PagingSource.LoadResult.Page).data + val statusesAccount2 = (timelineDao.getStatuses(2).load(loadParams) as PagingSource.LoadResult.Page).data assertStatuses(listOf(statusWithBlueDomain, statusWithGreenDomain), statusesAccount1) assertStatuses(listOf(statusWithRedDomainOtherAccount, statusWithBlueDomainOtherAccount), statusesAccount2)