refactor: Use type converters instead calling moshi.adapter by hand (#1134)

A few places in the code were 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.
This commit is contained in:
Nik Clayton 2024-11-25 21:25:28 +01:00 committed by GitHub
parent 2ebb77e85f
commit 982963b3b2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 71 additions and 94 deletions

View File

@ -2,9 +2,6 @@ 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
@ -12,12 +9,10 @@ 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)

View File

@ -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

View File

@ -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,9 +245,9 @@ 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)
}
@ -257,7 +255,6 @@ class CachedTimelineRemoteMediator(
TimelineStatusEntity.from(
status,
timelineUserId = pachliAccountId,
moshi = moshi,
),
)
}

View File

@ -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,
)

View File

@ -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,

View File

@ -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,

View File

@ -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(

View File

@ -178,7 +178,6 @@ abstract class CachedTimelineViewModelTestBase {
accountManager,
statusDisplayOptionsRepository,
sharedPreferencesRepository,
moshi,
)
}
}

View File

@ -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,

View File

@ -163,7 +163,6 @@ class ViewThreadViewModelTest {
eventHub,
accountManager,
timelineDao,
moshi,
cachedTimelineRepository,
statusDisplayOptionsRepository,
)

View File

@ -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) }
}

View File

@ -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)

View File

@ -32,8 +32,6 @@ import app.pachli.core.network.model.HashTag
import app.pachli.core.network.model.Poll
import app.pachli.core.network.model.Status
import app.pachli.core.network.model.TimelineAccount
import com.squareup.moshi.Moshi
import com.squareup.moshi.adapter
import java.time.Instant
import java.util.Date
@ -76,7 +74,7 @@ data class TimelineStatusEntity(
val content: String?,
val createdAt: Long,
val editedAt: Long?,
val emojis: String?,
val emojis: List<Emoji>?,
val reblogsCount: Int,
val favouritesCount: Int,
val repliesCount: Int,
@ -86,23 +84,22 @@ data class TimelineStatusEntity(
val sensitive: Boolean,
val spoilerText: String,
val visibility: Status.Visibility,
val attachments: String?,
val mentions: String?,
val tags: String?,
val application: String?,
val attachments: List<Attachment>?,
val mentions: List<Status.Mention>?,
val tags: List<HashTag>?,
val application: Status.Application?,
// if it has a reblogged status, it's id is stored here
val reblogServerId: String?,
val reblogAccountId: String?,
val poll: String?,
val poll: Poll?,
val muted: Boolean?,
val pinned: Boolean,
val card: String?,
val card: Card?,
val language: String?,
val filtered: List<FilterResult>?,
) {
companion object {
@OptIn(ExperimentalStdlibApi::class)
fun from(status: Status, timelineUserId: Long, moshi: Moshi) = TimelineStatusEntity(
fun from(status: Status, timelineUserId: Long) = TimelineStatusEntity(
serverId = status.id,
url = status.actionableStatus.url,
timelineUserId = timelineUserId,
@ -112,7 +109,7 @@ data class TimelineStatusEntity(
content = status.actionableStatus.content,
createdAt = status.actionableStatus.createdAt.time,
editedAt = status.actionableStatus.editedAt?.time,
emojis = moshi.adapter<List<Emoji>>().toJson(status.actionableStatus.emojis),
emojis = status.actionableStatus.emojis,
reblogsCount = status.actionableStatus.reblogsCount,
favouritesCount = status.actionableStatus.favouritesCount,
reblogged = status.actionableStatus.reblogged,
@ -121,16 +118,16 @@ data class TimelineStatusEntity(
sensitive = status.actionableStatus.sensitive,
spoilerText = status.actionableStatus.spoilerText,
visibility = status.actionableStatus.visibility,
attachments = moshi.adapter<List<Attachment>>().toJson(status.actionableStatus.attachments),
mentions = moshi.adapter<List<Status.Mention>>().toJson(status.actionableStatus.mentions),
tags = moshi.adapter<List<HashTag>>().toJson(status.actionableStatus.tags),
application = moshi.adapter<Status.Application?>().toJson(status.actionableStatus.application),
attachments = status.actionableStatus.attachments,
mentions = status.actionableStatus.mentions,
tags = status.actionableStatus.tags,
application = status.actionableStatus.application,
reblogServerId = status.reblog?.id,
reblogAccountId = status.reblog?.let { status.account.id },
poll = moshi.adapter<Poll>().toJson(status.actionableStatus.poll),
poll = status.actionableStatus.poll,
muted = status.actionableStatus.muted,
pinned = status.actionableStatus.pinned == true,
card = moshi.adapter<Card?>().toJson(status.actionableStatus.card),
card = status.actionableStatus.card,
repliesCount = status.actionableStatus.repliesCount,
language = status.actionableStatus.language,
filtered = status.actionableStatus.filtered,
@ -166,12 +163,11 @@ data class TimelineAccountEntity(
val displayName: String,
val url: String,
val avatar: String,
val emojis: String,
val emojis: List<Emoji>,
val bot: Boolean,
val createdAt: Instant?,
) {
@OptIn(ExperimentalStdlibApi::class)
fun toTimelineAccount(moshi: Moshi): TimelineAccount {
fun toTimelineAccount(): TimelineAccount {
return TimelineAccount(
id = serverId,
localUsername = localUsername,
@ -181,14 +177,13 @@ data class TimelineAccountEntity(
url = url,
avatar = avatar,
bot = bot,
emojis = moshi.adapter<List<Emoji>>().fromJson(emojis),
emojis = emojis,
createdAt = createdAt,
)
}
companion object {
@OptIn(ExperimentalStdlibApi::class)
fun from(timelineAccount: TimelineAccount, accountId: Long, moshi: Moshi) = TimelineAccountEntity(
fun from(timelineAccount: TimelineAccount, accountId: Long) = TimelineAccountEntity(
serverId = timelineAccount.id,
timelineUserId = accountId,
localUsername = timelineAccount.localUsername,
@ -196,7 +191,7 @@ data class TimelineAccountEntity(
displayName = timelineAccount.name,
url = timelineAccount.url,
avatar = timelineAccount.avatar,
emojis = moshi.adapter<List<Emoji>>().toJson(timelineAccount.emojis),
emojis = timelineAccount.emojis.orEmpty(),
bot = timelineAccount.bot,
createdAt = timelineAccount.createdAt,
)
@ -251,28 +246,20 @@ data class TimelineStatusWithAccount(
@Embedded(prefix = "t_")
val translatedStatus: TranslatedStatusEntity? = null,
) {
@OptIn(ExperimentalStdlibApi::class)
fun toStatus(moshi: Moshi): Status {
val attachments: List<Attachment> = status.attachments?.let {
moshi.adapter<List<Attachment>?>().fromJson(it)
} ?: emptyList()
val mentions: List<Status.Mention> = status.mentions?.let {
moshi.adapter<List<Status.Mention>?>().fromJson(it)
} ?: emptyList()
val tags: List<HashTag>? = status.tags?.let {
moshi.adapter<List<HashTag>?>().fromJson(it)
}
val application = status.application?.let { moshi.adapter<Status.Application>().fromJson(it) }
val emojis: List<Emoji> = status.emojis?.let { moshi.adapter<List<Emoji>?>().fromJson(it) }
?: emptyList()
val poll: Poll? = status.poll?.let { moshi.adapter<Poll?>().fromJson(it) }
val card: Card? = status.card?.let { moshi.adapter<Card?>().fromJson(it) }
fun toStatus(): Status {
val attachments: List<Attachment> = status.attachments.orEmpty()
val mentions: List<Status.Mention> = status.mentions.orEmpty()
val tags: List<HashTag>? = status.tags
val application = status.application
val emojis: List<Emoji> = status.emojis.orEmpty()
val poll: Poll? = status.poll
val card: Card? = status.card
val reblog = status.reblogServerId?.let { id ->
Status(
id = id,
url = status.url,
account = account.toTimelineAccount(moshi),
account = account.toTimelineAccount(),
inReplyToId = status.inReplyToId,
inReplyToAccountId = status.inReplyToAccountId,
reblog = null,
@ -306,7 +293,7 @@ data class TimelineStatusWithAccount(
id = status.serverId,
// no url for reblogs
url = null,
account = reblogAccount!!.toTimelineAccount(moshi),
account = reblogAccount!!.toTimelineAccount(),
inReplyToId = null,
inReplyToAccountId = null,
reblog = reblog,
@ -339,7 +326,7 @@ data class TimelineStatusWithAccount(
Status(
id = status.serverId,
url = status.url,
account = account.toTimelineAccount(moshi),
account = account.toTimelineAccount(),
inReplyToId = status.inReplyToId,
inReplyToAccountId = status.inReplyToAccountId,
reblog = null,

View File

@ -23,6 +23,9 @@ import app.pachli.core.database.AppDatabase
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.Card
import app.pachli.core.network.model.Emoji
import app.pachli.core.network.model.PreviewCardKind
import app.pachli.core.network.model.Status
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
@ -332,7 +335,7 @@ class TimelineDaoTest {
displayName = "displayName",
url = "blah",
avatar = "avatar",
emojis = "[\"pachli\": \"http://pachli.cool/emoji.jpg\"]",
emojis = listOf(Emoji("pachli", "http://pachli.cool/emoji.jpg", "", null)),
bot = false,
createdAt = null,
)
@ -346,7 +349,7 @@ class TimelineDaoTest {
displayName = "RdisplayName",
url = "Rblah",
avatar = "Ravatar",
emojis = "[]",
emojis = emptyList(),
bot = false,
createdAt = null,
)
@ -356,7 +359,7 @@ class TimelineDaoTest {
val card = when (cardUrl) {
null -> null
else -> "{ url: \"$cardUrl\" }"
else -> Card(cardUrl, "", "", PreviewCardKind.LINK, providerName = "", providerUrl = "")
}
val even = accountId % 2 == 0L
val status = TimelineStatusEntity(
@ -369,7 +372,7 @@ class TimelineDaoTest {
content = "Content!$statusId",
createdAt = createdAt,
editedAt = null,
emojis = "emojis$statusId",
emojis = emptyList(),
reblogsCount = 1 * statusId.toInt(),
favouritesCount = 2 * statusId.toInt(),
repliesCount = 3 * statusId.toInt(),
@ -379,10 +382,10 @@ class TimelineDaoTest {
sensitive = even,
spoilerText = "spoiler$statusId",
visibility = Status.Visibility.PRIVATE,
attachments = "attachments$accountId",
mentions = "mentions$accountId",
tags = "tags$accountId",
application = "application$accountId",
attachments = null,
mentions = null,
tags = null,
application = null,
reblogServerId = if (reblog) (statusId * 100).toString() else null,
reblogAccountId = reblogAuthor?.serverId,
poll = null,