refactor: Use type converters instead calling moshi.adapter by hand

A few places in the code where calling `moshi.adapter` to marshall to/from strings in the database where type converters either already exist, or are straightforward to create. Create the missing type converters, and use them throughout. This simplifies several places where a Moshi instance no longer needs to be passed through several layers of method calls. Since this doesn't change the underlying database representation of the data there's no need to bump the database version number.
pachli · Nov 25, 2024 · e8eeaea · e8eeaea
1 parent 2ebb77e
commit e8eeaea
Show file tree

Hide file tree

Showing 13 changed files with 60 additions and 86 deletions.
diff --git a/app/src/main/java/app/pachli/appstore/CacheUpdater.kt b/app/src/main/java/app/pachli/appstore/CacheUpdater.kt
@@ -2,22 +2,17 @@ package app.pachli.appstore
 
 import app.pachli.core.data.repository.AccountManager
 import app.pachli.core.database.dao.TimelineDao
-import app.pachli.core.network.model.Poll
-import com.squareup.moshi.Moshi
-import com.squareup.moshi.adapter
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.launch
 
-@OptIn(ExperimentalStdlibApi::class)
 class CacheUpdater @Inject constructor(
     eventHub: EventHub,
     accountManager: AccountManager,
     timelineDao: TimelineDao,
-    moshi: Moshi,
 ) {
     private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
 
@@ -37,8 +32,7 @@ class CacheUpdater @Inject constructor(
                     is StatusDeletedEvent ->
                         timelineDao.delete(accountId, event.statusId)
                     is PollVoteEvent -> {
-                        val pollString = moshi.adapter<Poll>().toJson(event.poll)
-                        timelineDao.setVoted(accountId, event.statusId, pollString)
+                        timelineDao.setVoted(accountId, event.statusId, event.poll)
                     }
                     is PinEvent ->
                         timelineDao.setPinned(accountId, event.statusId, event.pinned)

diff --git a/app/src/main/java/app/pachli/components/timeline/CachedTimelineRepository.kt b/app/src/main/java/app/pachli/components/timeline/CachedTimelineRepository.kt
@@ -39,7 +39,6 @@ import app.pachli.core.network.retrofit.MastodonApi
 import app.pachli.viewdata.StatusViewData
 import at.connyduck.calladapter.networkresult.NetworkResult
 import at.connyduck.calladapter.networkresult.fold
-import com.squareup.moshi.Moshi
 import javax.inject.Inject
 import javax.inject.Singleton
 import kotlinx.coroutines.CoroutineScope
@@ -63,7 +62,6 @@ class CachedTimelineRepository @Inject constructor(
     val timelineDao: TimelineDao,
     private val remoteKeyDao: RemoteKeyDao,
     private val translatedStatusDao: TranslatedStatusDao,
-    private val moshi: Moshi,
     @ApplicationScope private val externalScope: CoroutineScope,
 ) {
     private var factory: InvalidatingPagingSourceFactory<Int, TimelineStatusWithAccount>? = null
@@ -109,7 +107,6 @@ class CachedTimelineRepository @Inject constructor(
                 transactionProvider,
                 timelineDao,
                 remoteKeyDao,
-                moshi,
             ),
             pagingSourceFactory = factory!!,
         ).flow

diff --git a/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt b/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt
@@ -35,7 +35,6 @@ import app.pachli.core.database.model.TimelineStatusWithAccount
 import app.pachli.core.network.model.Links
 import app.pachli.core.network.model.Status
 import app.pachli.core.network.retrofit.MastodonApi
-import com.squareup.moshi.Moshi
 import kotlinx.coroutines.async
 import kotlinx.coroutines.coroutineScope
 import okhttp3.Headers
@@ -52,7 +51,6 @@ class CachedTimelineRemoteMediator(
     private val transactionProvider: TransactionProvider,
     private val timelineDao: TimelineDao,
     private val remoteKeyDao: RemoteKeyDao,
-    private val moshi: Moshi,
 ) : RemoteMediator<Int, TimelineStatusWithAccount>() {
     override suspend fun load(
         loadType: LoadType,
@@ -247,17 +245,16 @@ class CachedTimelineRemoteMediator(
     @Transaction
     private suspend fun insertStatuses(pachliAccountId: Long, statuses: List<Status>) {
         for (status in statuses) {
-            timelineDao.insertAccount(TimelineAccountEntity.from(status.account, pachliAccountId, moshi))
+            timelineDao.insertAccount(TimelineAccountEntity.from(status.account, pachliAccountId))
             status.reblog?.account?.let {
-                val account = TimelineAccountEntity.from(it, pachliAccountId, moshi)
+                val account = TimelineAccountEntity.from(it, pachliAccountId)
                 timelineDao.insertAccount(account)
             }
 
             timelineDao.insertStatus(
                 TimelineStatusEntity.from(
                     status,
                     timelineUserId = pachliAccountId,
-                    moshi = moshi,
                 ),
             )
         }

diff --git a/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineViewModel.kt b/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineViewModel.kt
@@ -37,7 +37,6 @@ import app.pachli.core.network.model.Poll
 import app.pachli.core.preferences.SharedPreferencesRepository
 import app.pachli.usecase.TimelineCases
 import app.pachli.viewdata.StatusViewData
-import com.squareup.moshi.Moshi
 import dagger.hilt.android.lifecycle.HiltViewModel
 import javax.inject.Inject
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -60,7 +59,6 @@ class CachedTimelineViewModel @Inject constructor(
     accountManager: AccountManager,
     statusDisplayOptionsRepository: StatusDisplayOptionsRepository,
     sharedPreferencesRepository: SharedPreferencesRepository,
-    private val moshi: Moshi,
 ) : TimelineViewModel(
     savedStateHandle,
     timelineCases,
@@ -91,7 +89,6 @@ class CachedTimelineViewModel @Inject constructor(
                     .map {
                         StatusViewData.from(
                             it,
-                            moshi,
                             isExpanded = activeAccount.alwaysOpenSpoiler,
                             isShowingContent = activeAccount.alwaysShowSensitiveMedia,
                         )

diff --git a/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt b/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt
@@ -48,7 +48,6 @@ import app.pachli.viewdata.StatusViewData
 import at.connyduck.calladapter.networkresult.fold
 import at.connyduck.calladapter.networkresult.getOrElse
 import at.connyduck.calladapter.networkresult.getOrThrow
-import com.squareup.moshi.Moshi
 import dagger.hilt.android.lifecycle.HiltViewModel
 import javax.inject.Inject
 import kotlinx.coroutines.Job
@@ -73,7 +72,6 @@ class ViewThreadViewModel @Inject constructor(
     eventHub: EventHub,
     private val accountManager: AccountManager,
     private val timelineDao: TimelineDao,
-    private val moshi: Moshi,
     private val repository: CachedTimelineRepository,
     statusDisplayOptionsRepository: StatusDisplayOptionsRepository,
 ) : ViewModel() {
@@ -141,7 +139,7 @@ class ViewThreadViewModel @Inject constructor(
 
             var detailedStatus = if (timelineStatusWithAccount != null) {
                 Timber.d("Loaded status from local timeline")
-                val status = timelineStatusWithAccount.toStatus(moshi)
+                val status = timelineStatusWithAccount.toStatus()
 
                 // Return the correct status, depending on which one matched. If you do not do
                 // this the status IDs will be different between the status that's displayed with
@@ -160,7 +158,6 @@ class ViewThreadViewModel @Inject constructor(
                 } else {
                     StatusViewData.from(
                         timelineStatusWithAccount,
-                        moshi,
                         isExpanded = account.alwaysOpenSpoiler,
                         isShowingContent = (account.alwaysShowSensitiveMedia || !status.actionableStatus.sensitive),
                         isDetailed = true,

diff --git a/app/src/main/java/app/pachli/viewdata/StatusViewData.kt b/app/src/main/java/app/pachli/viewdata/StatusViewData.kt
@@ -28,7 +28,6 @@ import app.pachli.core.network.model.Status
 import app.pachli.core.network.parseAsMastodonHtml
 import app.pachli.core.network.replaceCrashingCharacters
 import app.pachli.util.shouldTrimStatus
-import com.squareup.moshi.Moshi
 
 /**
  * Interface for the data shown when viewing a status, or something that wraps
@@ -270,13 +269,12 @@ data class StatusViewData(
 
         fun from(
             timelineStatusWithAccount: TimelineStatusWithAccount,
-            moshi: Moshi,
             isExpanded: Boolean,
             isShowingContent: Boolean,
             isDetailed: Boolean = false,
             translationState: TranslationState = TranslationState.SHOW_ORIGINAL,
         ): StatusViewData {
-            val status = timelineStatusWithAccount.toStatus(moshi)
+            val status = timelineStatusWithAccount.toStatus()
             return StatusViewData(
                 status = status,
                 translation = timelineStatusWithAccount.translatedStatus,

diff --git a/app/src/test/java/app/pachli/components/timeline/CachedTimelineRemoteMediatorTest.kt b/app/src/test/java/app/pachli/components/timeline/CachedTimelineRemoteMediatorTest.kt
@@ -95,15 +95,14 @@ class CachedTimelineRemoteMediatorTest {
     fun `should return error when network call returns error code`() {
         val remoteMediator = CachedTimelineRemoteMediator(
             initialKey = null,
-            pachliAccountId = activeAccount.id,
             api = mock {
                 onBlocking { homeTimeline(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) } doReturn Response.error(500, "".toResponseBody())
             },
+            pachliAccountId = activeAccount.id,
             factory = pagingSourceFactory,
             transactionProvider = transactionProvider,
             timelineDao = db.timelineDao(),
             remoteKeyDao = db.remoteKeyDao(),
-            moshi = moshi,
         )
 
         val result = runBlocking { remoteMediator.load(LoadType.REFRESH, state()) }
@@ -118,15 +117,14 @@ class CachedTimelineRemoteMediatorTest {
     fun `should return error when network call fails`() {
         val remoteMediator = CachedTimelineRemoteMediator(
             initialKey = null,
-            pachliAccountId = activeAccount.id,
             api = mock {
                 onBlocking { homeTimeline(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) } doThrow IOException()
             },
+            pachliAccountId = activeAccount.id,
             factory = pagingSourceFactory,
             transactionProvider = transactionProvider,
             timelineDao = db.timelineDao(),
             remoteKeyDao = db.remoteKeyDao(),
-            moshi = moshi,
         )
 
         val result = runBlocking { remoteMediator.load(LoadType.REFRESH, state()) }
@@ -140,13 +138,12 @@ class CachedTimelineRemoteMediatorTest {
     fun `should not prepend statuses`() {
         val remoteMediator = CachedTimelineRemoteMediator(
             initialKey = null,
-            pachliAccountId = activeAccount.id,
             api = mock(),
+            pachliAccountId = activeAccount.id,
             factory = pagingSourceFactory,
             transactionProvider = transactionProvider,
             timelineDao = db.timelineDao(),
             remoteKeyDao = db.remoteKeyDao(),
-            moshi = moshi,
         )
 
         val state = state(
@@ -172,7 +169,6 @@ class CachedTimelineRemoteMediatorTest {
     fun `should not try to refresh already cached statuses when db is empty`() {
         val remoteMediator = CachedTimelineRemoteMediator(
             initialKey = null,
-            pachliAccountId = activeAccount.id,
             api = mock {
                 onBlocking { homeTimeline(limit = 20) } doReturn Response.success(
                     listOf(
@@ -182,11 +178,11 @@ class CachedTimelineRemoteMediatorTest {
                     ),
                 )
             },
+            pachliAccountId = activeAccount.id,
             factory = pagingSourceFactory,
             transactionProvider = transactionProvider,
             timelineDao = db.timelineDao(),
             remoteKeyDao = db.remoteKeyDao(),
-            moshi = moshi,
         )
 
         val state = state(
@@ -227,7 +223,6 @@ class CachedTimelineRemoteMediatorTest {
 
         val remoteMediator = CachedTimelineRemoteMediator(
             initialKey = null,
-            pachliAccountId = activeAccount.id,
             api = mock {
                 onBlocking { homeTimeline(limit = 20) } doReturn Response.success(
                     listOf(
@@ -236,11 +231,11 @@ class CachedTimelineRemoteMediatorTest {
                     ),
                 )
             },
+            pachliAccountId = activeAccount.id,
             factory = pagingSourceFactory,
             transactionProvider = transactionProvider,
             timelineDao = db.timelineDao(),
             remoteKeyDao = db.remoteKeyDao(),
-            moshi = moshi,
         )
 
         val state = state(
@@ -284,7 +279,6 @@ class CachedTimelineRemoteMediatorTest {
 
         val remoteMediator = CachedTimelineRemoteMediator(
             initialKey = null,
-            pachliAccountId = activeAccount.id,
             api = mock {
                 onBlocking { homeTimeline(maxId = "5", limit = 20) } doReturn Response.success(
                     listOf(
@@ -297,11 +291,11 @@ class CachedTimelineRemoteMediatorTest {
                     ).build(),
                 )
             },
+            pachliAccountId = activeAccount.id,
             factory = pagingSourceFactory,
             transactionProvider = transactionProvider,
             timelineDao = db.timelineDao(),
             remoteKeyDao = db.remoteKeyDao(),
-            moshi = moshi,
         )
 
         val state = state(

diff --git a/app/src/test/java/app/pachli/components/timeline/CachedTimelineViewModelTestBase.kt b/app/src/test/java/app/pachli/components/timeline/CachedTimelineViewModelTestBase.kt
@@ -178,7 +178,6 @@ abstract class CachedTimelineViewModelTestBase {
             accountManager,
             statusDisplayOptionsRepository,
             sharedPreferencesRepository,
-            moshi,
         )
     }
 }
diff --git a/app/src/test/java/app/pachli/components/timeline/StatusMocker.kt b/app/src/test/java/app/pachli/components/timeline/StatusMocker.kt
@@ -115,12 +115,10 @@ fun mockStatusEntityWithAccount(
         status = TimelineStatusEntity.from(
             mockedStatus,
             timelineUserId = userId,
-            moshi = moshi,
         ),
         account = TimelineAccountEntity.from(
             mockedStatus.account,
             accountId = userId,
-            moshi = moshi,
         ),
         viewData = StatusViewDataEntity(
             serverId = id,

diff --git a/app/src/test/java/app/pachli/components/viewthread/ViewThreadViewModelTest.kt b/app/src/test/java/app/pachli/components/viewthread/ViewThreadViewModelTest.kt
@@ -163,7 +163,6 @@ class ViewThreadViewModelTest {
             eventHub,
             accountManager,
             timelineDao,
-            moshi,
             cachedTimelineRepository,
             statusDisplayOptionsRepository,
         )

diff --git a/core/database/src/main/kotlin/app/pachli/core/database/Converters.kt b/core/database/src/main/kotlin/app/pachli/core/database/Converters.kt
@@ -25,6 +25,7 @@ import app.pachli.core.model.ServerOperation
 import app.pachli.core.model.Timeline
 import app.pachli.core.network.model.Announcement
 import app.pachli.core.network.model.Attachment
+import app.pachli.core.network.model.Card
 import app.pachli.core.network.model.Emoji
 import app.pachli.core.network.model.FilterResult
 import app.pachli.core.network.model.HashTag
@@ -284,4 +285,16 @@ class Converters @Inject constructor(
 
     @TypeConverter
     fun jsonToAnnouncement(s: String?) = s?.let { moshi.adapter<Announcement>().fromJson(it) }
+
+    @TypeConverter
+    fun applicationToJson(application: Status.Application): String = moshi.adapter<Status.Application>().toJson(application)
+
+    @TypeConverter
+    fun jsonToApplication(s: String?) = s?.let { moshi.adapter<Status.Application>().fromJson(it) }
+
+    @TypeConverter
+    fun cardToJson(card: Card): String = moshi.adapter<Card>().toJson(card)
+
+    @TypeConverter
+    fun jsonToCard(s: String?) = s?.let { moshi.adapter<Card>().fromJson(it) }
 }
diff --git a/core/database/src/main/kotlin/app/pachli/core/database/dao/TimelineDao.kt b/core/database/src/main/kotlin/app/pachli/core/database/dao/TimelineDao.kt
@@ -24,13 +24,17 @@ import androidx.room.MapColumn
 import androidx.room.OnConflictStrategy.Companion.REPLACE
 import androidx.room.Query
 import androidx.room.Transaction
+import androidx.room.TypeConverters
 import androidx.room.Upsert
+import app.pachli.core.database.Converters
 import app.pachli.core.database.model.StatusViewDataEntity
 import app.pachli.core.database.model.TimelineAccountEntity
 import app.pachli.core.database.model.TimelineStatusEntity
 import app.pachli.core.database.model.TimelineStatusWithAccount
+import app.pachli.core.network.model.Poll
 
 @Dao
+@TypeConverters(Converters::class)
 abstract class TimelineDao {
 
     @Insert(onConflict = REPLACE)
@@ -255,7 +259,7 @@ AND serverId = :statusId""",
         """UPDATE TimelineStatusEntity SET poll = :poll
 WHERE timelineUserId = :accountId AND (serverId = :statusId OR reblogServerId = :statusId)""",
     )
-    abstract suspend fun setVoted(accountId: Long, statusId: String, poll: String)
+    abstract suspend fun setVoted(accountId: Long, statusId: String, poll: Poll)
 
     @Upsert
     abstract suspend fun upsertStatusViewData(svd: StatusViewDataEntity)