fix: restore synced positions by chapter
Some checks are pending
Build / build (push) Waiting to run

This commit is contained in:
Chmouel Boudjnah 2026-09-10 16:03:17 +02:00
parent 82c17a8286
commit 33cfd5931e
9 changed files with 824 additions and 34 deletions

View file

@ -45,6 +45,7 @@ import com.chmouel.liseur.domain.needsReconciling
import com.chmouel.liseur.domain.readingStatusFor
import com.chmouel.liseur.domain.reconcileReadingState
import com.chmouel.liseur.reader.progress.ExactLocatorAnchor
import com.chmouel.liseur.reader.progress.ResourceAnchor
import java.io.IOException
import org.json.JSONArray
import org.json.JSONObject
@ -192,10 +193,13 @@ class LiseurSyncPositionSync(
val stored = progressDao.get(bookUrl)
val exact = head.locatorJson.takeIf {
head.editionSha != null &&
head.editionSha == alias.editionSha &&
ExactLocatorAnchor.isExactJson(it)
sameEdition(head.editionSha, alias) && ExactLocatorAnchor.isExactJson(it)
}
// What the reader is *told* comes from the exact anchor; where
// they are actually put comes from the ladder. A peer that named
// the chapter but not the passage is an approximate answer, and
// it is still a far better one than a percentage.
val restorable = restorableLocator(head.locatorJson, head.editionSha, alias)
return PreviewOutcome.Ready(
SyncPreview(
local = stored?.totalProgression,
@ -214,7 +218,7 @@ class LiseurSyncPositionSync(
accountKey = account.peerId,
remoteStatus = head.progression
.let { ReadingStatus.forProgression(it).wireName },
remoteLocatorJson = exact,
remoteLocatorJson = restorable,
localRevision = stored?.localRevision,
resolvable = !ours,
),
@ -248,7 +252,7 @@ class LiseurSyncPositionSync(
),
accountKey = account.peerId,
remoteStatus = state.pendingStatus,
remoteLocatorJson = exact,
remoteLocatorJson = state.restorableLocatorFor(alias),
localRevision = progressDao.get(bookUrl)?.localRevision,
).takeIf { !it.agrees }
}
@ -287,7 +291,7 @@ class LiseurSyncPositionSync(
val alias = bookDao.getByUrl(bookUrl)
?.let { works.cached(it, account.peerId) }
val locator = state.exactLocatorFor(alias)
val locator = state.restorableLocatorFor(alias)
var applied = false
var accountMatches = false
@ -1156,7 +1160,19 @@ class LiseurSyncPositionSync(
exactRemote,
),
)
return act(account, book, alias, stored, state, exactRemote, decision)
// The merge is decided on the exact anchor, which is the only
// thing that can say two places are the same passage. What a
// pull then *stores* is the ladder's answer, so a peer that
// named only the chapter is still restored to it.
return act(
account,
book,
alias,
stored,
state,
state?.restorableLocatorFor(alias),
decision,
)
}
private suspend fun act(
@ -1165,7 +1181,7 @@ class LiseurSyncPositionSync(
alias: WorkAlias,
stored: ReadingProgress?,
state: SyncPeerState?,
exactRemote: String?,
restorableRemote: String?,
decision: SyncDecision,
): Reconciled {
val at = now()
@ -1198,7 +1214,7 @@ class LiseurSyncPositionSync(
progression = progression,
status = decision.state.status.wireName,
now = at,
locatorJson = exactRemote,
locatorJson = restorableRemote,
remoteUpdatedAt = state?.pendingUpdatedAt,
)
if (applied) {
@ -2010,11 +2026,32 @@ class LiseurSyncPositionSync(
/** Exact placement is safe only for byte-identical editions. */
private fun SyncPeerState.exactLocatorFor(alias: WorkAlias?): String? =
pendingLocatorJson.takeIf {
pendingEditionSha != null &&
pendingEditionSha == alias?.editionSha &&
ExactLocatorAnchor.isExactJson(it)
sameEdition(pendingEditionSha, alias) && ExactLocatorAnchor.isExactJson(it)
}
/**
* The best place in the peer's locator this device can actually put
* the reader — the exact passage where there is one, and the
* resource with its own progression where there is not.
*
* Kept apart from [exactLocatorFor] because the two answer different
* questions. That one asks "may the reader be told this is the exact
* spot", and its answer drives the excerpt, the confidence and the
* agreement test. This one asks "what is the best place to open at",
* and its answer is what gets *stored*. Conflating them is what made
* a percentage the only fallback: a peer that names the chapter and
* how far through it the reader was had that thrown away, and the
* position was rebuilt from a whole-book fraction the two clients
* do not even compute the same way.
*
* Still bounded by the edition. A locator recorded against other
* bytes names a resource this copy may not have, at an offset that
* means nothing here, so it is no more restorable than an exact
* anchor would be. See ADR-0028.
*/
private fun SyncPeerState.restorableLocatorFor(alias: WorkAlias?): String? =
restorableLocator(pendingLocatorJson, pendingEditionSha, alias)
private companion object {
/**
* The answers to a session batch that say something about one
@ -2100,3 +2137,37 @@ class LiseurSyncPositionSync(
const val CONFLICT = "conflict"
}
}
/**
* Whether a locator recorded against [editionSha] describes this copy.
*
* An unknown edition is not a matching one. A peer that did not say
* which bytes it was reading may be reading a different translation
* under the same work, and a resource path or an offset from that book
* is not a place in this one.
*/
private fun sameEdition(editionSha: String?, alias: WorkAlias?): Boolean =
editionSha != null && editionSha == alias?.editionSha
/**
* The best place [locatorJson] can put the reader in this copy.
*
* The ladder, in one place so every caller descends it identically: the
* exact passage, else the resource and how far through it the reader
* was, else nothing — and "nothing" leaves the whole-book progression,
* which travels beside the locator and is what the reader gets when
* this copy and the peer's are not the same bytes.
*/
private fun restorableLocator(
locatorJson: String?,
editionSha: String?,
alias: WorkAlias?,
): String? {
if (!sameEdition(editionSha, alias)) return null
if (ExactLocatorAnchor.isExactJson(locatorJson)) return locatorJson
// Rebuilt rather than passed through: what arrives may carry a
// quote that no longer resolves and a page number from another
// client's pagination, and both would be read here as things they
// are not.
return ResourceAnchor.sanitize(locatorJson)?.toJSON()?.toString()
}

View file

@ -426,6 +426,7 @@ class ReaderActivity : FragmentActivity() {
resolvePercent = viewModel::resolvePercent,
locatorAtOrBeforeProgression =
viewModel::locatorAtOrBeforeProgression,
resourceTargetFor = viewModel::resourceTargetFor,
prepareLocator = viewModel::prepareLocator,
onApproximateResume = viewModel::onApproximateResume,
)

View file

@ -125,6 +125,7 @@ import com.chmouel.liseur.reader.footnotes.FootnoteLayout
import com.chmouel.liseur.data.settings.FooterMode
import com.chmouel.liseur.data.settings.ColumnMode
import com.chmouel.liseur.data.settings.PageTurnStyle
import com.chmouel.liseur.reader.progress.ResourceAnchor
import com.chmouel.liseur.data.settings.ReadingFont
import com.chmouel.liseur.data.settings.ReaderPrefs
import com.chmouel.liseur.data.settings.readingCssFor
@ -729,8 +730,16 @@ fun ReaderScreen(
settleLayout()
if (ExactLocatorAnchor.verify(nav, locator)) return
}
val progression = locator.locations.totalProgression ?: return
val fallback = onProgressAction.locatorAtOrBeforeProgression(progression) ?: return
// The quote did not land. The chapter it named is still the best
// thing known about where the reader was: falling straight to a
// whole-book fraction can move them to a different chapter
// entirely, because that fraction may have been computed by the
// other client, which does not compute it the same way.
val fallback = onProgressAction.resourceTargetFor(locator)
?: locator.locations.totalProgression
?.takeIf(ResourceAnchor::isFraction)
?.let(onProgressAction.locatorAtOrBeforeProgression)
?: return
retargetGate(fallback)
pendingPositionEvent = event
val fallbackToken = moves.issue(
@ -1086,8 +1095,13 @@ fun ReaderScreen(
val requested = restoreTarget ?: return@LaunchedEffect
if (!ExactLocatorAnchor.isExact(requested)) return@LaunchedEffect
if (openingExactAnchorArrived(nav, requested)) return@LaunchedEffect
val progression = requested.locations.totalProgression ?: return@LaunchedEffect
val fallback = onProgressAction.locatorAtOrBeforeProgression(progression)
// The chapter before the percentage, for the reason given in
// navigate(): the whole-book fraction is the only rung that can
// land in the wrong chapter.
val fallback = onProgressAction.resourceTargetFor(requested)
?: requested.locations.totalProgression
?.takeIf(ResourceAnchor::isFraction)
?.let(onProgressAction.locatorAtOrBeforeProgression)
?: return@LaunchedEffect
pendingPositionEvent = NavigatorPositionEvent.FRAGMENT_RECREATION
// Retarget rather than release. This navigation is asynchronous
@ -3273,6 +3287,7 @@ class ReaderProgressActions(
val currentPercent: () -> Int,
val resolvePercent: (String) -> GoToDestination?,
val locatorAtOrBeforeProgression: (Double) -> Locator?,
val resourceTargetFor: (Locator) -> Locator?,
val prepareLocator: (Locator) -> Locator,
val onApproximateResume: () -> Unit,
) {

View file

@ -78,6 +78,7 @@ import com.chmouel.liseur.reader.footnotes.FootnoteResolver
import com.chmouel.liseur.reader.progress.ReaderProgress
import com.chmouel.liseur.reader.progress.ReadingPace
import com.chmouel.liseur.reader.progress.ReadingSpeedEstimator
import com.chmouel.liseur.reader.progress.ResourceAnchor
import com.chmouel.liseur.reader.progress.StableBookProgress
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
@ -681,6 +682,7 @@ class ReaderViewModel(
// settled on the next open; the book still opens now.
if (outcome != ResolveOutcome.Done || !takeRemote) return
positionsFor(publication)
this@ReaderViewModel.publication = publication
offerWayBack(localLocator, here, preview)
}
@ -696,12 +698,16 @@ class ReaderViewModel(
preview: SyncPreview,
) {
val positions = bookPositions?.takeIf { it.isUsable } ?: return
val locator = exactLocal?.takeIf(ExactLocatorAnchor::isExact)
?: positions.locatorAtOrBeforeProgression(progression)
?: return
val position = positions.resolve(locator)?.position
val locator = ResourceAnchor.resumeTarget(
saved = exactLocal,
totalProgression = progression,
readingOrder = readingOrderPaths(),
byProgression = positions::locatorAtOrBeforeProgression,
) ?: return
val prepared = prepareLocator(locator)
val position = positions.resolve(prepared)?.position
_jumpBack.value = JumpBack(
locator = prepareLocator(locator),
locator = prepared,
position = position,
fromSync = true,
excerpt = preview.excerpt,
@ -723,11 +729,16 @@ class ReaderViewModel(
/** Navigates to the position that was offered, not a later database snapshot. */
private suspend fun goToRemotePosition(preview: SyncPreview) {
val positions = positionsFor(publication ?: return)
val exact = preview.remoteLocatorJson
val remoteLocator = preview.remoteLocatorJson
?.let { runCatching { Locator.fromJSON(JSONObject(it)) }.getOrNull() }
?.takeIf(ExactLocatorAnchor::isExact)
val exact = remoteLocator?.takeIf(ExactLocatorAnchor::isExact)
val target = exact
?: preview.remote?.let(positions::locatorAtOrBeforeProgression)
?: ResourceAnchor.approximateTarget(
saved = remoteLocator,
totalProgression = preview.remote,
readingOrder = readingOrderPaths(),
byProgression = positions::locatorAtOrBeforeProgression,
)
?: return
onJump()
_jumpBack.value = _jumpBack.value?.copy(
@ -749,6 +760,7 @@ class ReaderViewModel(
private var publication: Publication? = null
private var bookPositions: BookPositions? = null
private var readingOrderPaths: List<String>? = null
private var goToPageResolver: GoToPageResolver? = null
private var speed = ReadingSpeedEstimator()
private var jumpBackTimer: Job? = null
@ -973,6 +985,7 @@ class ReaderViewModel(
?: ReadingPace.Unknown,
)
val positions = positionsFor(publication)
this@ReaderViewModel.publication = publication
if (pulledAutomatically != null) {
val localLocator = beforeSync?.locatorJson
?.let { runCatching { Locator.fromJSON(JSONObject(it)) }.getOrNull() }
@ -985,17 +998,19 @@ class ReaderViewModel(
val savedLocator = stored?.locatorJson
?.let { runCatching { Locator.fromJSON(JSONObject(it)) }.getOrNull() }
// Unmarked Readium text may describe the following synthetic
// position. It is approximate and deliberately resumes at or
// before the stable progression instead.
val initialLocator = if (ExactLocatorAnchor.isExact(savedLocator)) {
savedLocator
} else {
stored?.totalProgression?.let(positions::locatorAtOrBeforeProgression)
?: savedLocator
}?.let(::prepareLocator)
// position, so it is not treated as exact. What is left of
// such a locator is still the chapter and the place in it,
// which is tried before the whole-book progression: that
// last rung can land in a different chapter altogether when
// the fraction was written down by the other client.
val initialLocator = ResourceAnchor.resumeTarget(
saved = savedLocator,
totalProgression = stored?.totalProgression,
readingOrder = readingOrderPaths(),
byProgression = positions::locatorAtOrBeforeProgression,
)?.let(::prepareLocator)
lastLocator = initialLocator
library.markOpened(bookId)
this@ReaderViewModel.publication = publication
_state.value = UiState.Ready(
publication = publication,
navigatorFactory = EpubNavigatorFactory(publication),
@ -1254,6 +1269,31 @@ class ReaderViewModel(
fun locatorAtOrBeforeProgression(progression: Double): Locator? =
bookPositions?.locatorAtOrBeforeProgression(progression)
/**
* The chapter a peer named, if this book has it.
*
* Offered to the reading chrome so that a quote which will not
* verify falls back to the right chapter rather than straight to a
* whole-book percentage, which can land in a different one.
*/
fun resourceTargetFor(locator: Locator): Locator? {
val target = ResourceAnchor.targetIn(locator, readingOrderPaths()) ?: return null
val stable = bookPositions?.resolve(target) ?: return target
return target.copy(
locations = target.locations.copy(totalProgression = stable.progression),
)
}
/** The opened publication's spine paths, computed once per book. */
private fun readingOrderPaths(): List<String> {
readingOrderPaths?.let { return it }
val paths = publication?.readingOrder
?.map { ResourceAnchor.path(it.url().toString()) }
?: return emptyList()
readingOrderPaths = paths
return paths
}
fun onApproximateResume() {
_jumpBack.value = _jumpBack.value?.copy(confidence = ResumeConfidence.APPROXIMATE)
}

View file

@ -42,6 +42,10 @@ internal object ResourceAddress {
*/
fun href(webUrl: String?): String? = relative(webUrl)
/** A publication-relative path suitable for comparing two href spellings. */
fun canonicalPath(raw: String?): String? =
path(raw)
private fun relative(raw: String?): String? {
val address = raw?.substringBefore('#')?.substringBefore('?')?.trim() ?: return null
if (address.isEmpty()) return null

View file

@ -0,0 +1,165 @@
package com.chmouel.liseur.reader.progress
import com.chmouel.liseur.reader.ResourceAddress
import org.json.JSONObject
import org.readium.r2.shared.publication.Locator
import org.readium.r2.shared.util.Url
/**
* The place in a book a peer named, kept when the exact word cannot be.
*
* An exact anchor ([ExactLocatorAnchor]) names a passage and survives a
* different font, a different column count and a different screen. When
* it cannot be resolved — the other client never captured one, the quote
* has moved, the selector no longer matches — what is left is still far
* better than a percentage: the resource the reader was in, and how far
* through it they were.
*
* That matters most in the books this exists for. A whole-book
* progression is a fraction of *the whole book*, and the two clients
* compute it differently: this one interpolates Readium's synthetic
* positions, the web reader interpolates the server's position list by
* byte weight. The two disagree by a little on every book, and by more
* on one whose resources are unevenly sized — which is the ordinary
* shape of an EPUB 2 with few, large spine items. Turning that fraction
* back into a place lands in roughly the right region of the book. The
* resource and its own progression land in the right chapter, at the
* right point of it, whoever wrote them down.
*
* So this is deliberately *not* a smaller exact anchor. It is the
* coarser rung below one, and everything that reads it must go on
* calling the result approximate.
*/
object ResourceAnchor {
/**
* The resource-relative place in [locatorJson], with nothing else.
*
* Everything an anchor could be mistaken for is dropped rather than
* carried: the text quote and its selector (which no longer
* describe a passage this device can verify), the CFI fragments
* (which this client cannot resolve at all), and the synthetic
* `position` (which is the *writing* client's pagination — Readium's
* count and the web reader's are two different numbers for the same
* spot, so a foreign one indexes into the wrong place here).
*
* Null when there is no usable resource-relative place, which is not
* a failure: it simply means the whole-book progression is the best
* that can be done.
*/
fun sanitize(locatorJson: String?): Locator? =
parse(locatorJson)?.let(::sanitize)
fun sanitize(locator: Locator?): Locator? {
val locator = locator ?: return null
if (locator.href.toString().isBlank()) return null
// The within-resource fraction is the whole point: without it
// there is no "how far into the chapter" to restore, and the
// resource alone would send the reader to its first paragraph
// when they were most of the way through it.
val progression = fraction(locator.locations.progression) ?: return null
// copy() keeps the href and the media type and replaces the rest
// outright, which is what makes this a whitelist: a locations
// field invented after this was written is dropped by default
// rather than carried by an omission here.
return locator.copy(
locations = Locator.Locations(
progression = progression,
totalProgression = fraction(locator.locations.totalProgression),
),
text = Locator.Text(),
)
}
/**
* Whether [locatorJson] names a resource-relative place at all.
*
* Cheaper than sanitising when the answer is all that is wanted, and
* it asks exactly the same question, so the two cannot disagree.
*/
fun isResourceJson(locatorJson: String?): Boolean = sanitize(locatorJson) != null
/**
* The sanitised place in [locator], if this publication has that
* resource.
*
* [readingOrder] is the opened publication's spine paths. A locator
* that names a resource this book does not have is not a place in
* it — it came from another edition, or from a client that spells
* the path differently — and following it would leave the navigator
* nowhere. Checked here rather than at the sync boundary because
* only the open publication can answer it.
*/
fun targetIn(locator: Locator?, readingOrder: Collection<String>): Locator? {
val resource = sanitize(locator) ?: return null
val target = ResourceAddress.canonicalPath(resource.href.toString()) ?: return null
val matches = readingOrder.filter {
ResourceAddress.canonicalPath(it) == target
}
if (matches.size != 1) return null
val localHref = matches.single().let { Url(it) ?: Url("/$it") } ?: return null
return resource.copy(href = localHref)
}
/**
* Where to reopen: the exact passage, else the chapter, else the
* percentage.
*
* The order is the point, and it lives here so that opening a book,
* accepting a catch-up offer and recovering from a quote that would
* not verify cannot drift apart. The percentage is last because it
* is the only rung that can land in the wrong chapter.
*/
fun resumeTarget(
saved: Locator?,
totalProgression: Double?,
readingOrder: Collection<String>,
byProgression: (Double) -> Locator?,
): Locator? = saved?.takeIf(ExactLocatorAnchor::isExact)
?: approximateTarget(saved, totalProgression, readingOrder, byProgression)
?: saved
/**
* The rungs below the exact one, for a caller whose exact attempt
* has already been made and failed.
*/
fun approximateTarget(
saved: Locator?,
totalProgression: Double?,
readingOrder: Collection<String>,
byProgression: (Double) -> Locator?,
): Locator? = targetIn(saved, readingOrder)
?: (totalProgression ?: saved?.locations?.totalProgression)
?.takeIf(::isFraction)
?.let(byProgression)
/** A spine path, without the fragment or query an anchor carries. */
fun path(href: String): String = href.substringBefore('#').substringBefore('?')
/** Whether a value can be used as a whole-book or resource fraction. */
fun isFraction(value: Double?): Boolean =
value?.let(::fraction) != null
/**
* A number that was really written down, told apart from one that
* was not.
*
* Out of range is refused rather than clamped. A peer that sends
* `1.7` has not told this device the reader is at the end of the
* chapter; it has told it something impossible, and treating that as
* the end would move the reader on the strength of a bug. Zero is
* legitimate and must survive: it is the top of the chapter.
*/
private fun fraction(value: Double?): Double? {
val value = value ?: return null
if (!value.isFinite()) return null
// Written this way round so a NaN is refused, as in SyncOps.
if (!(value >= 0.0 && value <= 1.0)) return null
return value
}
private fun parse(json: String?): Locator? {
if (json.isNullOrEmpty() || json == "{}") return null
return runCatching { Locator.fromJSON(JSONObject(json)) }.getOrNull()
}
}

View file

@ -19,6 +19,7 @@ import com.chmouel.liseur.data.remote.SyncFailure
import com.chmouel.liseur.data.remote.SyncOutcome
import com.chmouel.liseur.data.remote.LiveIdentity
import com.chmouel.liseur.data.remote.LiveTopic
import com.chmouel.liseur.reader.progress.ExactLocatorAnchor
import java.io.File
import java.net.InetAddress
import javax.crypto.KeyGenerator
@ -606,6 +607,128 @@ class LiseurSyncPositionSyncTest {
assertEquals(ResumeConfidence.EXACT, preview.confidence)
}
@Test
fun `a peer that names the chapter has it kept, not reduced to a percentage`() = runTest {
// Issue #184. The web reader records the resource it is in and
// how far through it the reader is, but not the app's exact
// passage anchor. That locator used to be discarded outright,
// leaving the whole-book percentage as the only thing to reopen
// on — and the two clients do not compute that percentage the
// same way, so the book reopened somewhere near the place
// instead of at it. The chapter is kept now.
connect()
db.bookDao().upsert(local())
alias(editionSha = "sha-local")
db.syncPeerStateDao().persistPending(
bookUrl = LOCAL,
peerId = peer(),
progression = 0.31,
status = "reading",
remoteUpdatedAt = NOW,
locatorJson = resourceLocator(
href = "/chapter-9.xhtml",
progression = 0.74,
totalProgression = 0.31,
),
editionSha = "sha-local",
)
db.readingProgressDao().recordLocal(LOCAL, LOCATOR, 0.2, null, "reading", NOW)
assertEquals(ResolveOutcome.Done, sync().takeRemotePosition(LOCAL, 1))
val stored = JSONObject(db.readingProgressDao().get(LOCAL)!!.locatorJson!!)
assertEquals("/chapter-9.xhtml", stored.getString("href"))
assertEquals(0.74, stored.getJSONObject("locations").getDouble("progression"), 0.0)
}
@Test
fun `a kept chapter is not dressed up as the exact passage`() = runTest {
// The chapter is worth restoring and worth saying so about. It
// is not the reader's word, so it must not raise the exact
// confidence or offer an excerpt of a passage nobody captured.
connect()
db.bookDao().upsert(local())
alias(editionSha = "sha-local")
db.syncPeerStateDao().persistPending(
bookUrl = LOCAL,
peerId = peer(),
progression = 0.31,
status = "reading",
remoteUpdatedAt = NOW,
locatorJson = resourceLocator(),
editionSha = "sha-local",
)
db.readingProgressDao().recordLocal(LOCAL, LOCATOR, 0.2, null, "reading", NOW)
val preview = sync().preservedConflict(LOCAL, null)!!
assertEquals(ResumeConfidence.APPROXIMATE, preview.confidence)
assertNull(preview.excerpt)
// Nothing exact on either side, so the percentages decide, as
// they always did.
assertNull(preview.exactPositionAgreement)
assertEquals("/chapter-9.xhtml", JSONObject(preview.remoteLocatorJson!!).getString("href"))
}
@Test
fun `a chapter from another edition is still refused`() = runTest {
// A resource path and an offset in someone else's bytes are not
// a place in this book. Keeping the ladder inside the edition
// check is what stops this being a regression of ADR-0028.
connect()
db.bookDao().upsert(local())
alias(editionSha = "sha-local")
db.syncPeerStateDao().persistPending(
bookUrl = LOCAL,
peerId = peer(),
progression = 0.31,
status = "reading",
remoteUpdatedAt = NOW,
locatorJson = resourceLocator(),
editionSha = "sha-remote",
)
db.readingProgressDao().recordLocal(LOCAL, LOCATOR, 0.2, null, "reading", NOW)
assertEquals(ResolveOutcome.Done, sync().takeRemotePosition(LOCAL, 1))
assertEquals("{}", db.readingProgressDao().get(LOCAL)?.locatorJson)
}
@Test
fun `a foreign page number and a stale quote do not ride along`() = runTest {
// The peer's `position` is its own pagination and its `text` is
// a quote this device never verified. Stored as they arrive,
// the first indexes into the wrong place and the second reads
// as an exact anchor on the next run.
connect()
db.bookDao().upsert(local())
alias(editionSha = "sha-local")
db.syncPeerStateDao().persistPending(
bookUrl = LOCAL,
peerId = peer(),
progression = 0.31,
status = "reading",
remoteUpdatedAt = NOW,
locatorJson = """
{"href":"/chapter-9.xhtml","type":"application/xhtml+xml",
"locations":{"progression":0.74,"totalProgression":0.31,
"position":812,"fragments":["epubcfi(/6/14!/4/2/2)"]},
"text":{"highlight":"a quote from another engine"}}
""".trimIndent(),
editionSha = "sha-local",
)
db.readingProgressDao().recordLocal(LOCAL, LOCATOR, 0.2, null, "reading", NOW)
assertEquals(ResolveOutcome.Done, sync().takeRemotePosition(LOCAL, 1))
val stored = db.readingProgressDao().get(LOCAL)!!.locatorJson!!
val locations = JSONObject(stored).getJSONObject("locations")
assertFalse(stored, locations.has("position"))
assertFalse(stored, locations.has("fragments"))
assertFalse(stored, JSONObject(stored).has("text"))
assertFalse(stored, ExactLocatorAnchor.isExactJson(stored))
assertEquals(0.74, locations.getDouble("progression"), 0.0)
}
@Test
fun `different edition and legacy locators fall back to progression`() = runTest {
connect()
@ -2411,6 +2534,22 @@ class LiseurSyncPositionSyncTest {
.put("text", JSONObject().put("highlight", highlight))
.toString()
/** What the web reader can say: a resource and how far into it. */
private fun resourceLocator(
href: String = "/chapter-9.xhtml",
progression: Double = 0.74,
totalProgression: Double = 0.31,
): String = JSONObject()
.put("href", href)
.put("type", "application/xhtml+xml")
.put(
"locations",
JSONObject()
.put("progression", progression)
.put("totalProgression", totalProgression),
)
.toString()
/** Every request the server has seen, including earlier runs'. */
private fun requests(): List<RecordedRequest> {
while (seen.size < server.requestCount) seen += server.takeRequest()

View file

@ -0,0 +1,335 @@
package com.chmouel.liseur.reader.progress
import org.json.JSONObject
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.readium.r2.shared.publication.Locator
import org.readium.r2.shared.util.Url
import com.chmouel.liseur.reader.ResourceAddress
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
@Config(sdk = [35], application = android.app.Application::class)
@RunWith(RobolectricTestRunner::class)
class ResourceAnchorTest {
@Test
fun `the resource and its own progression survive`() {
val sanitized = ResourceAnchor.sanitize(
locator(progression = 0.62, totalProgression = 0.23).toJSON().toString(),
)
assertNotNull(sanitized)
assertEquals("/chapter-7.xhtml", sanitized?.href.toString())
assertEquals(0.62, sanitized?.locations?.progression ?: 0.0, 0.0)
assertEquals(0.23, sanitized?.locations?.totalProgression ?: 0.0, 0.0)
}
@Test
fun `a foreign pagination and a stale quote are dropped, not carried`() {
// Everything here describes the *writing* client: its synthetic
// page number, its engine's CFI, and a quote this device has no
// way to confirm. Restoring against any of them is how a
// fallback ends up more wrong than the percentage it replaced.
val foreign = locator().copy(
locations = Locator.Locations(
fragments = listOf("epubcfi(/6/14!/4/2/2[p9]/1:0)"),
progression = 0.62,
position = 812,
totalProgression = 0.23,
otherLocations = mapOf(
ExactLocatorAnchor.MARKER to 1,
ExactLocatorAnchor.CSS_SELECTOR to "body > p:nth-of-type(9)",
),
),
text = Locator.Text(before = "before", highlight = "moved", after = "after"),
)
val sanitized = requireNotNull(ResourceAnchor.sanitize(foreign.toJSON().toString()))
assertEquals(emptyList<String>(), sanitized.locations.fragments)
assertNull(sanitized.locations.position)
assertEquals(emptyMap<String, Any>(), sanitized.locations.otherLocations)
assertNull(sanitized.text.highlight)
// What it is *for* is still there.
assertEquals(0.62, sanitized.locations.progression ?: 0.0, 0.0)
// And it must not read as an exact anchor afterwards, or the
// reader would be told the passage was restored exactly.
assertFalse(ExactLocatorAnchor.isExact(sanitized))
}
@Test
fun `the top of a chapter is a place, not a missing one`() {
val top = ResourceAnchor.sanitize(
locator(progression = 0.0, totalProgression = 0.0).toJSON().toString(),
)
assertNotNull(top)
assertEquals(0.0, top?.locations?.progression ?: -1.0, 0.0)
}
@Test
fun `a progression that is not a fraction is refused rather than clamped`() {
// Clamping would move the reader to the end of a chapter on the
// strength of another client's bug. There is no place here.
//
// Written as raw JSON because that is how these actually arrive.
// org.json refuses to *emit* a NaN, so a client whose fraction
// went wrong sends `null` (what JSON.stringify does with one) or
// the string "NaN" — which is just a string until something asks
// it for a double. SyncOps guards the same door for the same
// reason.
val broken = listOf("1.7", "-0.2", "\"NaN\"", "null", "\"\"", "\"later\"")
for (value in broken) {
val json = """
{"href":"/chapter-7.xhtml","type":"application/xhtml+xml",
"locations":{"progression":$value,"totalProgression":0.23}}
""".trimIndent()
assertNull(value, ResourceAnchor.sanitize(json))
}
}
@Test
fun `a broken whole-book percentage does not cost the chapter`() {
// The resource progression is the part that matters here, and it
// is sound. Dropping the whole place because the *other* number
// is nonsense would be refusing the good half of the answer.
val json = """
{"href":"/chapter-7.xhtml","type":"application/xhtml+xml",
"locations":{"progression":0.62,"totalProgression":4.2}}
""".trimIndent()
val sanitized = requireNotNull(ResourceAnchor.sanitize(json))
assertEquals(0.62, sanitized.locations.progression ?: 0.0, 0.0)
assertNull(sanitized.locations.totalProgression)
}
@Test
fun `a locator with no resource progression has no resource place`() {
val percentageOnly = JSONObject()
.put("href", "/chapter-7.xhtml")
.put("type", "application/xhtml+xml")
.put("locations", JSONObject().put("totalProgression", 0.23))
assertNull(ResourceAnchor.sanitize(percentageOnly.toString()))
assertFalse(ResourceAnchor.isResourceJson(percentageOnly.toString()))
}
@Test
fun `nothing at all is not a place`() {
assertNull(ResourceAnchor.sanitize(null as String?))
assertNull(ResourceAnchor.sanitize(""))
assertNull(ResourceAnchor.sanitize("{}"))
assertNull(ResourceAnchor.sanitize("not json"))
assertFalse(ResourceAnchor.isResourceJson(null))
}
@Test
fun `an exact anchor is also a resource place`() {
// The ladder descends: an anchor whose quote will not resolve
// must still leave the chapter behind it, or the fallback is the
// percentage again.
val exact = ExactLocatorAnchor.mark(
locator(progression = 0.62),
ViewportTextAnchor("#chapter", "before ", "word", " after"),
)
assertTrue(ExactLocatorAnchor.isExact(exact))
val sanitized = requireNotNull(ResourceAnchor.sanitize(exact.toJSON().toString()))
assertEquals("/chapter-7.xhtml", sanitized.href.toString())
assertEquals(0.62, sanitized.locations.progression ?: 0.0, 0.0)
}
@Test
fun `a chapter this book does not have is not a place in it`() {
val spine = setOf("/chapter-7.xhtml")
assertNotNull(ResourceAnchor.targetIn(locator(), spine))
assertNull(ResourceAnchor.targetIn(locator(), setOf("/other.xhtml")))
assertNull(ResourceAnchor.targetIn(locator(), emptySet()))
}
@Test
fun `equivalent href spellings resolve to the opened spine spelling`() {
val foreign = locator().copy(
href = requireNotNull(Url("/OEBPS/ch2%20long.xhtml#part")),
)
assertEquals("OEBPS/ch2 long.xhtml", ResourceAddress.canonicalPath(foreign.href.toString()))
assertEquals("OEBPS/ch2 long.xhtml", ResourceAddress.canonicalPath("OEBPS/ch2%20long.xhtml"))
val target = ResourceAnchor.targetIn(foreign, setOf("OEBPS/ch2%20long.xhtml"))
assertEquals("OEBPS/ch2%20long.xhtml", target?.href.toString())
assertEquals(0.62, target?.locations?.progression ?: 0.0, 0.0)
}
@Test
fun `duplicate spine resources are too ambiguous for the resource rung`() {
assertNull(
ResourceAnchor.targetIn(
locator(),
listOf("/chapter-7.xhtml", "/chapter-7.xhtml"),
),
)
}
@Test
fun `whole-book fractions must stay in range`() {
assertTrue(ResourceAnchor.isFraction(0.0))
assertTrue(ResourceAnchor.isFraction(1.0))
assertFalse(ResourceAnchor.isFraction(-0.1))
assertFalse(ResourceAnchor.isFraction(1.1))
assertFalse(ResourceAnchor.isFraction(Double.NaN))
}
@Test
fun `a fragment on the href still names its resource`() {
// A peer's anchor may carry an id. The spine is keyed by path.
val withFragment = requireNotNull(
Locator.fromJSON(
JSONObject()
.put("href", "/chapter-7.xhtml#p42")
.put("type", "application/xhtml+xml")
.put("locations", JSONObject().put("progression", 0.4)),
),
)
assertNotNull(ResourceAnchor.targetIn(withFragment, setOf("/chapter-7.xhtml")))
}
@Test
fun `the chapter is tried before the percentage`() {
// Issue #184: the percentage is the only rung that can land in
// the wrong chapter, so it must be the last one tried.
var percentageAsked = false
val target = ResourceAnchor.resumeTarget(
saved = locator(),
totalProgression = 0.23,
readingOrder = setOf("/chapter-7.xhtml"),
byProgression = { percentageAsked = true; null },
)
assertEquals("/chapter-7.xhtml", target?.href.toString())
assertEquals(0.62, target?.locations?.progression ?: 0.0, 0.0)
assertFalse(percentageAsked)
}
@Test
fun `a chapter from elsewhere gives way to the percentage`() {
val elsewhere = requireNotNull(
Locator.fromJSON(
JSONObject()
.put("href", "/from-another-book.xhtml")
.put("type", "application/xhtml+xml")
.put("locations", JSONObject().put("progression", 0.4)),
),
)
val fromPercentage = locator(progression = 0.1, totalProgression = 0.23)
val target = ResourceAnchor.resumeTarget(
saved = elsewhere,
totalProgression = 0.23,
readingOrder = setOf("/chapter-7.xhtml"),
byProgression = { fromPercentage },
)
assertEquals(fromPercentage, target)
}
@Test
fun `an exact anchor is taken as it is, whole`() {
// The ladder must not sanitise the rung it is not on: an exact
// anchor needs its quote and selector to be verifiable.
val exact = requireNotNull(
Locator.fromJSON(
JSONObject()
.put("href", "/chapter-7.xhtml")
.put("type", "application/xhtml+xml")
.put(
"locations",
JSONObject()
.put("progression", 0.62)
.put("cssSelector", "#p42")
.put("liseurAnchor", 1),
)
.put("text", JSONObject().put("highlight", "the word")),
),
)
val target = ResourceAnchor.resumeTarget(
saved = exact,
totalProgression = 0.23,
readingOrder = setOf("/chapter-7.xhtml"),
byProgression = { null },
)
assertEquals(exact, target)
assertTrue(ExactLocatorAnchor.isExact(target))
}
@Test
fun `nothing usable leaves the caller what it had`() {
// Preserving the old behaviour: a locator nobody can improve on
// is still passed through rather than dropped.
val bare = requireNotNull(
Locator.fromJSON(
JSONObject()
.put("href", "/chapter-7.xhtml")
.put("type", "application/xhtml+xml"),
),
)
assertEquals(
bare,
ResourceAnchor.resumeTarget(bare, null, setOf("/chapter-7.xhtml")) { null },
)
assertNull(ResourceAnchor.resumeTarget(null, 0.5, emptySet()) { null })
}
@Test
fun `a peer's own total progression is used when nothing else says`() {
var asked: Double? = null
ResourceAnchor.approximateTarget(
saved = locator(totalProgression = 0.44),
totalProgression = null,
readingOrder = emptySet(),
byProgression = { asked = it; null },
)
assertEquals(0.44, asked ?: -1.0, 0.0)
}
@Test
fun `a locator written by the browser reader is read as exact here`() {
// The literal bytes liseur-sync's reader-anchor.js emits. The two
// readers agree on this shape or they cannot use each other's
// anchors at all, so it is worth pinning rather than describing.
val fromBrowser = """
{"href":"OEBPS/ch1.xhtml","type":"application/xhtml+xml","title":"Moby-Dick",
"locations":{"fragments":[],"progression":0.25,"totalProgression":0.1,
"position":2,"liseurAnchor":1,"cssSelector":"body > p:nth-of-type(3)"},
"text":{"before":"and then ","highlight":"Ishmael","after":" said nothing"}}
""".trimIndent()
assertTrue(ExactLocatorAnchor.isExactJson(fromBrowser))
// And when its quote will not resolve, the chapter underneath is
// still there to fall back to, unpolluted by the browser's own
// spine index.
val target = ResourceAnchor.sanitize(fromBrowser)
assertEquals("OEBPS/ch1.xhtml", target?.href.toString())
assertEquals(0.25, target?.locations?.progression ?: -1.0, 0.0)
assertNull(target?.locations?.position)
assertNull(target?.text?.highlight)
}
private fun locator(
progression: Double = 0.62,
totalProgression: Double = 0.23,
): Locator = requireNotNull( Locator.fromJSON(
JSONObject()
.put("href", "/chapter-7.xhtml")
.put("type", "application/xhtml+xml")
.put(
"locations",
JSONObject()
.put("progression", progression)
.put("totalProgression", totalProgression),
),
),
)
}

View file

@ -239,13 +239,33 @@ per-server capability detection.
|--------|-----------|--------|
| Komga | Exact position in chapter | Full Readium locator, with position snapping |
| calibre-web | Page-level at best | Percentage (`totalProgression`) via the Kobo protocol |
| liseur-sync | Exact position in chapter | Full Readium locator via the op log |
| liseur-sync | Exact position in chapter, in the same file | Full Readium locator via the op log |
| Grimmory | Page-level at best | Percentage via KOReader's kosync, paired alongside the catalog |
| Any kosync server | Page-level at best | Same partner: it speaks the generic protocol, so a stock kosync server pairs the same way |
Every sync goes through the shared `reconcileReadingState` merge
logic in `domain/ReadingStateMerge.kt`.
A locator is only followed into the file it was written against. Ops
carry the edition's SHA-256 (`edition_sha`), and a position from an
edition this device cannot confirm it holds is used as a percentage and
nothing more — the same paragraph is not at the same offset in another
translation or another conversion of the book.
Within a matching edition the restoration descends three rungs, in
`reader/progress/ResourceAnchor.kt`:
1. **The passage.** A locator this app marked as exact
(`liseurAnchor`): a block selector and a bounded quote, verified to
be on screen after the chapter has laid out.
2. **The chapter.** The resource and the fraction *within* it. Kept
whenever the exact rung is absent or will not verify, including
positions from clients that do not provide a usable exact anchor.
3. **The percentage.** The whole-book fraction, and the only rung that
can land in the wrong chapter: two engines compute that fraction
from different things, and on a book with a few large chapters they
disagree by pages.
The kosync partner is not a kind of server: it is paired *alongside* a
connected one, covers that server's downloaded books (matched by
KOReader's partial MD5 of the file), and has its own lifecycle.