Drop the legacy chapter model and narrow last page read to an Int

eu.kanade.tachiyomi.data.database.models.Chapter predates SQLDelight. It
survived because it implements SChapter, so ReaderChapter could hand the
same object to the database and to the source.

ReaderChapter now holds the domain Chapter and converts at the three
points that actually need a source model: getPageList, getFormat and
getChapterUrl. The domain model is immutable, so the reader replaces the
chapter rather than writing through it, behind an update function so the
field cannot be reassigned from outside. Equality stays on the chapter it
was loaded with, which keeps the identity comparisons in the viewers
stable while progress is recorded against it.

updateChapterProgress no longer mutates the chapter and reads the values
straight back out to build the update it persists; it computes them once
and passes them. Marking a chapter read moves out of the function named
for completing it and into the same place the page is recorded.

Removing the interface also drops the nullable id it carried, along with
three non-null assertions and a Float to Double conversion that the
compiler had been warning about.

A page position is an index into a chapter's pages, so it cannot exceed
Int range, but SQLite has one integer type and the column came back as a
Long. That width reached the domain model and then every caller either
carried it or converted, and the viewers had to narrow it again to index
a list with.

Declaring the column INTEGER AS Int moves the narrowing into a column
adapter, so it happens once where the value stops being a database
number. No migration: SQLite still stores an INTEGER, only the type
SQLDelight generates changes.

Backups are the exception and keep converting, because the field is on
the wire as int64 and has to stay that way. That is also the only place
the value arrives from outside the app, where a malformed file could
carry something that does not fit and truncate to a negative index, so
it is clamped rather than cast.

app/src/main/java/eu/kanade/domain/chapter/model/Chapter.kt

@@ -1,13 +1,10 @@
 package eu.kanade.domain.chapter.model
 
-import eu.kanade.tachiyomi.data.database.models.ChapterImpl
 import eu.kanade.tachiyomi.source.model.SChapter
 import mihon.core.common.extensions.toEpochMillisOrZero
 import mihon.core.common.extensions.toInstantOrNull
 import tachiyomi.domain.chapter.model.Chapter
-import eu.kanade.tachiyomi.data.database.models.Chapter as DbChapter
 
-// TODO: Remove when all deps are migrated
 fun Chapter.toSChapter(): SChapter {
     return SChapter.create().also {
         it.url = url
@@ -29,19 +26,3 @@ fun Chapter.copyFromSChapter(sChapter: SChapter): Chapter {
         memo = sChapter.memo,
     )
 }
-
-fun Chapter.toDbChapter(): DbChapter = ChapterImpl().also {
-    it.id = id
-    it.manga_id = mangaId
-    it.url = url
-    it.name = name
-    it.scanlator = scanlator
-    it.read = read
-    it.bookmark = bookmark
-    it.last_page_read = lastPageRead.toInt()
-    it.date_fetch = dateFetch.toEpochMilliseconds()
-    it.date_upload = dateUpload.toEpochMillisOrZero()
-    it.chapter_number = chapterNumber.toFloat()
-    it.source_order = sourceOrder.toInt()
-    it.memo = memo
-}

app/src/main/java/eu/kanade/presentation/manga/MangaScreen.kt

@@ -722,7 +722,7 @@ private fun SharedMangaBottomActionMenu(
         }.takeIf { selected.fastAny { !it.chapter.read } },
         onMarkAsUnreadClicked = {
             onMultiMarkAsReadClicked(selected.fastMap { it.chapter }, false)
-        }.takeIf { selected.fastAny { it.chapter.read || it.chapter.lastPageRead > 0L } },
+        }.takeIf { selected.fastAny { it.chapter.read || it.chapter.lastPageRead > 0 } },
         onMarkPreviousAsReadClicked = {
             onMarkPreviousAsReadClicked(selected[0].chapter)
         }.takeIf { selected.size == 1 },

app/src/main/java/eu/kanade/presentation/reader/ChapterTransition.kt

@@ -30,7 +30,6 @@ import androidx.compose.ui.tooling.preview.PreviewLightDark
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
 import eu.kanade.presentation.theme.TachiyomiPreviewTheme
-import eu.kanade.tachiyomi.data.database.models.toDomainChapter
 import eu.kanade.tachiyomi.ui.reader.model.ChapterTransition
 import eu.kanade.tachiyomi.ui.reader.model.ReaderChapter
 import mihon.icons.materialsymbols.MaterialSymbols
@@ -50,8 +49,8 @@ fun ChapterTransition(
     currChapterDownloaded: Boolean,
     goingToChapterDownloaded: Boolean,
 ) {
-    val currChapter = transition.from.chapter.toDomainChapter()
-    val goingToChapter = transition.to?.chapter?.toDomainChapter()
+    val currChapter = transition.from.chapter
+    val goingToChapter = transition.to?.chapter
 
     ProvideTextStyle(MaterialTheme.typography.bodyMedium) {
         when (transition) {

app/src/main/java/eu/kanade/presentation/updates/UpdatesScreen.kt

@@ -235,7 +235,7 @@ private fun UpdatesBottomBar(
         }.takeIf { selected.fastAny { !it.update.read } },
         onMarkAsUnreadClicked = {
             onMultiMarkAsReadClicked(selected, false)
-        }.takeIf { selected.fastAny { it.update.read || it.update.lastPageRead > 0L } },
+        }.takeIf { selected.fastAny { it.update.read || it.update.lastPageRead > 0 } },
         onDownloadClicked = {
             onDownloadChapter(selected, ChapterDownloadAction.START)
         }.takeIf {

app/src/main/java/eu/kanade/tachiyomi/data/backup/models/BackupChapter.kt

@@ -38,7 +38,8 @@ class BackupChapter(
             scanlator = this@BackupChapter.scanlator,
             read = this@BackupChapter.read,
             bookmark = this@BackupChapter.bookmark,
-            lastPageRead = this@BackupChapter.lastPageRead,
+            // Only untrusted source of this value, and a truncating narrow can land negative
+            lastPageRead = this@BackupChapter.lastPageRead.coerceIn(0L, Int.MAX_VALUE.toLong()).toInt(),
             dateFetch = Instant.fromEpochMilliseconds(this@BackupChapter.dateFetch),
             dateUpload = this@BackupChapter.dateUpload.toInstantOrNull(),
             sourceOrder = this@BackupChapter.sourceOrder,
@@ -54,7 +55,7 @@ fun Chapter.toBackupChapter() = BackupChapter(
     scanlator = scanlator,
     read = read,
     bookmark = bookmark,
-    lastPageRead = lastPageRead,
+    lastPageRead = lastPageRead.toLong(),
     dateFetch = dateFetch.toEpochMilliseconds(),
     dateUpload = dateUpload.toEpochMillisOrZero(),
     sourceOrder = sourceOrder,

app/src/main/java/eu/kanade/tachiyomi/data/database/models/Chapter.kt

@@ -1,48 +0,0 @@
-@file:Suppress("PropertyName")
-
-package eu.kanade.tachiyomi.data.database.models
-
-import eu.kanade.tachiyomi.source.model.SChapter
-import mihon.core.common.extensions.toInstantOrNull
-import java.io.Serializable
-import kotlin.time.Instant
-import tachiyomi.domain.chapter.model.Chapter as DomainChapter
-
-interface Chapter : SChapter, Serializable {
-
-    var id: Long?
-
-    var manga_id: Long?
-
-    var read: Boolean
-
-    var bookmark: Boolean
-
-    var last_page_read: Int
-
-    var date_fetch: Long
-
-    var source_order: Int
-}
-
-val Chapter.isRecognizedNumber: Boolean
-    get() = chapter_number >= 0f
-
-fun Chapter.toDomainChapter(): DomainChapter? {
-    if (id == null || manga_id == null) return null
-    return DomainChapter(
-        id = id!!,
-        mangaId = manga_id!!,
-        read = read,
-        bookmark = bookmark,
-        lastPageRead = last_page_read.toLong(),
-        dateFetch = Instant.fromEpochMilliseconds(date_fetch),
-        sourceOrder = source_order.toLong(),
-        url = url,
-        name = name,
-        dateUpload = date_upload.toInstantOrNull(),
-        chapterNumber = chapter_number.toDouble(),
-        scanlator = scanlator,
-        memo = memo,
-    )
-}

app/src/main/java/eu/kanade/tachiyomi/data/database/models/ChapterImpl.kt

@@ -1,48 +0,0 @@
-@file:Suppress("PropertyName")
-
-package eu.kanade.tachiyomi.data.database.models
-
-import kotlinx.serialization.json.JsonObject
-import mihon.core.common.extensions.EMPTY
-
-class ChapterImpl : Chapter {
-
-    override var id: Long? = null
-
-    override var manga_id: Long? = null
-
-    override lateinit var url: String
-
-    override lateinit var name: String
-
-    override var scanlator: String? = null
-
-    override var read: Boolean = false
-
-    override var bookmark: Boolean = false
-
-    override var last_page_read: Int = 0
-
-    override var date_fetch: Long = 0
-
-    override var date_upload: Long = 0
-
-    override var chapter_number: Float = 0f
-
-    override var source_order: Int = 0
-
-    override var memo: JsonObject = JsonObject.EMPTY
-
-    override fun equals(other: Any?): Boolean {
-        if (this === other) return true
-        if (other == null || javaClass != other.javaClass) return false
-
-        val chapter = other as Chapter
-        if (url != chapter.url) return false
-        return id == chapter.id
-    }
-
-    override fun hashCode(): Int {
-        return url.hashCode() + id.hashCode()
-    }
-}

app/src/main/java/eu/kanade/tachiyomi/ui/reader/ReaderViewModel.kt

@@ -17,7 +17,7 @@ import dev.zacsweers.metro.ContributesIntoMap
 import dev.zacsweers.metrox.viewmodel.ViewModelAssistedFactory
 import dev.zacsweers.metrox.viewmodel.ViewModelAssistedFactoryKey
 import eu.kanade.domain.base.BasePreferences
-import eu.kanade.domain.chapter.model.toDbChapter
+import eu.kanade.domain.chapter.model.toSChapter
 import eu.kanade.domain.manga.interactor.SetMangaViewerFlags
 import eu.kanade.domain.manga.interactor.UpdateManga
 import eu.kanade.domain.manga.model.readerOrientation
@@ -27,7 +27,6 @@ import eu.kanade.domain.track.interactor.TrackChapter
 import eu.kanade.domain.track.service.TrackPreferences
 import eu.kanade.tachiyomi.data.cache.ChapterCache
 import eu.kanade.tachiyomi.data.cache.CoverCache
-import eu.kanade.tachiyomi.data.database.models.toDomainChapter
 import eu.kanade.tachiyomi.data.download.DownloadCache
 import eu.kanade.tachiyomi.data.download.DownloadManager
 import eu.kanade.tachiyomi.data.download.DownloadProvider
@@ -274,7 +273,6 @@ class ReaderViewModel(
                     this
                 }
             }
-            .map { it.toDbChapter() }
             .map(::ReaderChapter)
     }
 
@@ -291,9 +289,9 @@ class ReaderViewModel(
                     // Restore from SavedState
                     currentChapter.requestedPage = chapterPageIndex
                 } else if (!currentChapter.chapter.read) {
-                    currentChapter.requestedPage = currentChapter.chapter.last_page_read
+                    currentChapter.requestedPage = currentChapter.chapter.lastPageRead
                 }
-                chapterId = currentChapter.chapter.id!!
+                chapterId = currentChapter.chapter.id
             }
             .launchIn(viewModelScope)
 
@@ -524,9 +522,9 @@ class ReaderViewModel(
             )
             if (!isNextChapterDownloaded) return@launchIO
 
-            val chaptersToDownload = getNextChapters.await(manga.id, nextChapter.id!!).run {
+            val chaptersToDownload = getNextChapters.await(manga.id, nextChapter.id).run {
                 if (readerPreferences.skipDupe.get()) {
-                    removeDuplicates(nextChapter.toDomainChapter()!!)
+                    removeDuplicates(nextChapter)
                 } else {
                     this
                 }
@@ -544,7 +542,7 @@ class ReaderViewModel(
      * if setting is enabled and [currentChapter] is queued for download
      */
     private fun cancelQueuedDownloads(currentChapter: ReaderChapter): Download? {
-        return downloadManager.getQueuedDownloadOrNull(currentChapter.chapter.id!!)?.also {
+        return downloadManager.getQueuedDownloadOrNull(currentChapter.chapter.id)?.also {
             downloadManager.cancelQueuedDownloads(listOf(it))
         }
     }
@@ -574,14 +572,14 @@ class ReaderViewModel(
                 .filter {
                     it.id != chapterToDelete.chapter.id &&
                         it.isRecognizedNumber &&
-                        it.chapterNumber.toFloat() == chapterToDelete.chapter.chapter_number
+                        it.chapterNumber == chapterToDelete.chapter.chapterNumber
                 }
                 // The list is loaded once, before these were marked read along with chapterToDelete
                 .map { it.copy(read = true) }
         } else {
             emptyList()
         }
-        enqueueDeleteChapters(listOf(chapterToDelete.chapter.toDomainChapter()!!) + duplicateChapters)
+        enqueueDeleteChapters(listOf(chapterToDelete.chapter) + duplicateChapters)
     }
 
     /**
@@ -598,23 +596,23 @@ class ReaderViewModel(
         chapterPageIndex = pageIndex
 
         if (!incognitoMode && page.status !is Page.State.Error) {
-            readerChapter.chapter.last_page_read = pageIndex
+            val read = readerChapter.chapter.read || readerChapter.pages?.lastIndex == pageIndex
+            readerChapter.update { it.copy(lastPageRead = pageIndex, read = read) }
 
             if (readerChapter.pages?.lastIndex == pageIndex) {
                 updateChapterProgressOnComplete(readerChapter)
             }
 
             updateChapter.await(
-                ChapterUpdate(readerChapter.chapter.id!!) {
-                    read = readerChapter.chapter.read
-                    lastPageRead = readerChapter.chapter.last_page_read.toLong()
+                ChapterUpdate(readerChapter.chapter.id) {
+                    this.read = read
+                    lastPageRead = pageIndex
                 },
             )
         }
     }
 
     private suspend fun updateChapterProgressOnComplete(readerChapter: ReaderChapter) {
-        readerChapter.chapter.read = true
         updateTrackChapterRead(readerChapter)
         deleteChapterIfNeeded(readerChapter)
 
@@ -627,7 +625,7 @@ class ReaderViewModel(
                 if (
                     !chapter.read &&
                     chapter.isRecognizedNumber &&
-                    chapter.chapterNumber.toFloat() == readerChapter.chapter.chapter_number
+                    chapter.chapterNumber == readerChapter.chapter.chapterNumber
                 ) {
                     ChapterUpdate(chapter.id) { read = true }
                 } else {
@@ -648,7 +646,7 @@ class ReaderViewModel(
         getCurrentChapter()?.let { readerChapter ->
             if (incognitoMode) return@let
 
-            val chapterId = readerChapter.chapter.id!!
+            val chapterId = readerChapter.chapter.id
             val endTime = Clock.System.now()
             val sessionReadDuration = chapterReadStartTime?.let { (endTime - it).inWholeMilliseconds } ?: 0
             chapterReadStartTime = null
@@ -683,7 +681,7 @@ class ReaderViewModel(
     fun getSource() = state.value.source as? HttpSource
 
     fun getChapterUrl(): String? {
-        val sChapter = getCurrentChapter()?.chapter ?: return null
+        val sChapter = getCurrentChapter()?.chapter?.toSChapter() ?: return null
         val source = getSource() ?: return null
 
         return try {
@@ -698,13 +696,13 @@ class ReaderViewModel(
      * Bookmarks the currently active chapter.
      */
     fun toggleChapterBookmark() {
-        val chapter = getCurrentChapter()?.chapter ?: return
-        val bookmarked = !chapter.bookmark
-        chapter.bookmark = bookmarked
+        val readerChapter = getCurrentChapter() ?: return
+        val bookmarked = !readerChapter.chapter.bookmark
+        readerChapter.update { it.copy(bookmark = bookmarked) }
 
         viewModelScope.launchNonCancellable {
             updateChapter.await(
-                ChapterUpdate(chapter.id!!) {
+                ChapterUpdate(readerChapter.chapter.id) {
                     bookmark = bookmarked
                 },
             )
@@ -740,7 +738,7 @@ class ReaderViewModel(
             if (currChapters != null) {
                 // Save current page
                 val currChapter = currChapters.currChapter
-                currChapter.requestedPage = currChapter.chapter.last_page_read
+                currChapter.requestedPage = currChapter.chapter.lastPageRead
 
                 mutableState.update {
                     it.copy(
@@ -776,7 +774,7 @@ class ReaderViewModel(
             if (currChapters != null) {
                 // Save current page
                 val currChapter = currChapters.currChapter
-                currChapter.requestedPage = currChapter.chapter.last_page_read
+                currChapter.requestedPage = currChapter.chapter.lastPageRead
 
                 mutableState.update {
                     it.copy(
@@ -969,7 +967,7 @@ class ReaderViewModel(
         val manga = manga ?: return
 
         viewModelScope.launchNonCancellable {
-            trackChapter.await(context, manga.id, readerChapter.chapter.chapter_number.toDouble())
+            trackChapter.await(context, manga.id, readerChapter.chapter.chapterNumber)
         }
     }
 

app/src/main/java/eu/kanade/tachiyomi/ui/reader/loader/ChapterLoader.kt

@@ -1,6 +1,7 @@
 package eu.kanade.tachiyomi.ui.reader.loader
 
 import android.content.Context
+import eu.kanade.domain.chapter.model.toSChapter
 import eu.kanade.tachiyomi.data.cache.ChapterCache
 import eu.kanade.tachiyomi.data.download.DownloadManager
 import eu.kanade.tachiyomi.data.download.DownloadProvider
@@ -56,7 +57,7 @@ class ChapterLoader(
                 // If the chapter is partially read, set the starting page to the last the user read
                 // otherwise use the requested page.
                 if (!chapter.chapter.read) {
-                    chapter.requestedPage = chapter.chapter.last_page_read
+                    chapter.requestedPage = chapter.chapter.lastPageRead
                 }
 
                 chapter.state = ReaderChapter.State.Loaded(pages)
@@ -94,7 +95,7 @@ class ChapterLoader(
                 downloadManager,
                 downloadProvider,
             )
-            source is LocalSource -> source.getFormat(chapter.chapter).let { format ->
+            source is LocalSource -> source.getFormat(chapter.chapter.toSChapter()).let { format ->
                 when (format) {
                     is Format.Directory -> DirectoryPageLoader(format.file)
                     is Format.Archive -> ArchivePageLoader(format.file.archiveReader(context))

app/src/main/java/eu/kanade/tachiyomi/ui/reader/loader/DownloadPageLoader.kt

@@ -3,7 +3,6 @@ package eu.kanade.tachiyomi.ui.reader.loader
 import android.content.Context
 import android.net.Uri
 import com.hippo.unifile.UniFile
-import eu.kanade.tachiyomi.data.database.models.toDomainChapter
 import eu.kanade.tachiyomi.data.download.DownloadManager
 import eu.kanade.tachiyomi.data.download.DownloadProvider
 import eu.kanade.tachiyomi.source.Source
@@ -58,7 +57,7 @@ internal class DownloadPageLoader(
     }
 
     private fun getPagesFromDirectory(): List<ReaderPage> {
-        val pages = downloadManager.buildPageList(source, manga, chapter.chapter.toDomainChapter()!!)
+        val pages = downloadManager.buildPageList(source, manga, chapter.chapter)
         return pages.map { page ->
             ReaderPage(page.index, page.url, page.imageUrl) {
                 context.contentResolver.openInputStream(page.uri ?: Uri.EMPTY)!!

app/src/main/java/eu/kanade/tachiyomi/ui/reader/loader/HttpPageLoader.kt

@@ -1,7 +1,7 @@
 package eu.kanade.tachiyomi.ui.reader.loader
 
+import eu.kanade.domain.chapter.model.toSChapter
 import eu.kanade.tachiyomi.data.cache.ChapterCache
-import eu.kanade.tachiyomi.data.database.models.toDomainChapter
 import eu.kanade.tachiyomi.source.model.Page
 import eu.kanade.tachiyomi.source.online.HttpSource
 import eu.kanade.tachiyomi.ui.reader.model.ReaderChapter
@@ -66,12 +66,12 @@ internal class HttpPageLoader(
      */
     override suspend fun getPages(): List<ReaderPage> {
         val pages = try {
-            chapterCache.getPageListFromCache(chapter.chapter.toDomainChapter()!!)
+            chapterCache.getPageListFromCache(chapter.chapter)
         } catch (e: Throwable) {
             if (e is CancellationException) {
                 throw e
             }
-            source.getPageList(chapter.chapter)
+            source.getPageList(chapter.chapter.toSChapter())
         }
         return pages.mapIndexed { index, page ->
             // Don't trust sources and use our own indexing
@@ -136,7 +136,7 @@ internal class HttpPageLoader(
                 try {
                     // Convert to pages without reader information
                     val pagesToSave = pages.map { Page(it.index, it.url, it.imageUrl) }
-                    chapterCache.putPageListToCache(chapter.chapter.toDomainChapter()!!, pagesToSave)
+                    chapterCache.putPageListToCache(chapter.chapter, pagesToSave)
                 } catch (e: Throwable) {
                     if (e is CancellationException) {
                         throw e

app/src/main/java/eu/kanade/tachiyomi/ui/reader/model/ReaderChapter.kt

@@ -1,12 +1,19 @@
 package eu.kanade.tachiyomi.ui.reader.model
 
-import eu.kanade.domain.chapter.model.toDbChapter
-import eu.kanade.tachiyomi.data.database.models.Chapter
 import eu.kanade.tachiyomi.ui.reader.loader.PageLoader
 import kotlinx.coroutines.flow.MutableStateFlow
 import tachiyomi.core.common.util.system.logcat
+import tachiyomi.domain.chapter.model.Chapter
 
-data class ReaderChapter(val chapter: Chapter) {
+data class ReaderChapter(private val loadedChapter: Chapter) {
+
+    /**
+     * The chapter as it currently stands, which drifts from [loadedChapter] as the reader records
+     * progress against it. Equality stays on the chapter it was loaded with, so a chapter held by
+     * the viewers keeps comparing equal to itself while being read.
+     */
+    var chapter: Chapter = loadedChapter
+        private set
 
     val stateFlow = MutableStateFlow<State>(State.Wait)
     var state: State
@@ -24,7 +31,9 @@ data class ReaderChapter(val chapter: Chapter) {
 
     private var references = 0
 
-    constructor(chapter: tachiyomi.domain.chapter.model.Chapter) : this(chapter.toDbChapter())
+    fun update(transform: (Chapter) -> Chapter) {
+        chapter = transform(chapter)
+    }
 
     fun ref() {
         references++

app/src/main/java/eu/kanade/tachiyomi/ui/reader/viewer/MissingChapters.kt

@@ -1,12 +1,11 @@
 package eu.kanade.tachiyomi.ui.reader.viewer
 
-import eu.kanade.tachiyomi.data.database.models.toDomainChapter
 import eu.kanade.tachiyomi.ui.reader.model.ReaderChapter
 import tachiyomi.domain.chapter.service.calculateChapterGap as domainCalculateChapterGap
 
 fun calculateChapterGap(higherReaderChapter: ReaderChapter?, lowerReaderChapter: ReaderChapter?): Int {
     return domainCalculateChapterGap(
-        higherReaderChapter?.chapter?.toDomainChapter(),
-        lowerReaderChapter?.chapter?.toDomainChapter(),
+        higherReaderChapter?.chapter,
+        lowerReaderChapter?.chapter,
     )
 }

app/src/main/java/eu/kanade/tachiyomi/ui/reader/viewer/webtoon/WebtoonViewer.kt

@@ -211,7 +211,7 @@ class WebtoonViewer(val activity: ReaderActivity, val isContinuous: Boolean = tr
             val nextItem = adapter.items.getOrNull(adapter.items.size - 1)
             val transitionChapter = (nextItem as? ChapterTransition.Next)?.to ?: (nextItem as?ReaderPage)?.chapter
             if (transitionChapter != null) {
-                logcat { "Requesting to preload chapter ${transitionChapter.chapter.chapter_number}" }
+                logcat { "Requesting to preload chapter ${transitionChapter.chapter.chapterNumber}" }
                 activity.requestPreloadChapter(transitionChapter)
             }
         }