vrr/liseur
2
0
Fork 0
mirror of https://github.com/chmouel/liseur.git synced 2026-10-19 01:11:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix: Separate note timestamps from overall annotation updates

Browse source 
Note timestamps previously relied on the general annotation update time,
which caused changes like highlight recoloring or peer syncing to
falsely display notes as edited. Dedicated note timestamps were
introduced so edit indicators only reflected actual text revisions.

Signed-off-by: Chmouel Boudjnah <chmouel@chmouel.com>
This commit is contained in:
Chmouel Boudjnah 2026-09-08 10:05:54 +02:00
parent 19cc68316d
commit 27989d8024
12 changed files with 1708 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

1519
app/schemas/com.chmouel.liseur.data.db.LiseurDatabase/50.json Normal file
View file

File diff suppressed because it is too large Load diff

4
app/src/main/kotlin/com/chmouel/liseur/data/db/BookAnnotation.kt
View file

@ -45,6 +45,10 @@ data class BookAnnotation(
@ColumnInfo(name = "position") val position: Int? = null,
@ColumnInfo(name = "total_progression") val totalProgression: Double? = null,
@ColumnInfo(name = "created_at") val createdAt: Long,
/** When the note body was first written, in epoch milliseconds. */
@ColumnInfo(name = "note_created_at") val noteCreatedAt: Long? = null,
/** When the note body was last changed after creation, in epoch microseconds. */
@ColumnInfo(name = "note_updated_at") val noteUpdatedAt: Long? = null,
/**
* When this mark was last changed, in epoch **microseconds**.
*

25
app/src/main/kotlin/com/chmouel/liseur/data/db/LiseurDatabase.kt
View file

@ -29,7 +29,7 @@ import androidx.sqlite.execSQL
SessionRefusal::class,
SessionTransmission::class,
],
version = 49,
version = 50,
exportSchema = true,
)
abstract class LiseurDatabase : RoomDatabase() {
@ -1337,6 +1337,28 @@ abstract class LiseurDatabase : RoomDatabase() {
}
}
/**
* Separates a note body's own dates from the mark-wide sync stamp.
*
* Existing rows cannot say whether `updated_at` moved because the
* reader edited the note, recoloured the mark, or a peer changed
* another field. Keeping only the note creation date avoids showing
* old false "Edited" stamps as fact.
*/
val MIGRATION_49_50 = object : Migration(49, 50) {
override fun migrate(connection: SQLiteConnection) {
connection.execSQL("ALTER TABLE annotations ADD COLUMN note_created_at INTEGER")
connection.execSQL("ALTER TABLE annotations ADD COLUMN note_updated_at INTEGER")
connection.execSQL(
"""
UPDATE annotations
SET note_created_at = created_at
WHERE note IS NOT NULL AND TRIM(note) != ''
""".trimIndent(),
)
}
}
val MIGRATIONS: Array<Migration> get() = arrayOf(
MIGRATION_1_2,
MIGRATION_2_3,
@ -1386,6 +1408,7 @@ abstract class LiseurDatabase : RoomDatabase() {
MIGRATION_46_47,
MIGRATION_47_48,
MIGRATION_48_49,
MIGRATION_49_50,
)
}
}

20
app/src/main/kotlin/com/chmouel/liseur/data/liseursync/AnnotationWire.kt
View file

@ -377,6 +377,22 @@ object AnnotationWire {
record.body.isNotEmpty() -> AnnotationKind.NOTE
else -> AnnotationKind.HIGHLIGHT
}
val note = record.body.takeIf { it.isNotEmpty() }
val noteCreatedAt = if (note == null) {
null
} else {
existing?.noteCreatedAt ?: if (existing?.note == null) {
record.clientTsMicros / 1000
} else {
existing.createdAt
}
}
val noteUpdatedAt = when {
note == null -> null
existing?.note == null -> null
existing.note == note -> existing.noteUpdatedAt
else -> record.clientTsMicros
}
return BookAnnotation(
id = record.id,
bookId = bookId,
@ -398,7 +414,7 @@ object AnnotationWire {
?.takeIf { truncate(it, MAX_EXCERPT_BYTES) == excerpt }
?: excerpt
},
note = record.body.takeIf { it.isNotEmpty() },
note = note,
tint = record.color.takeIf { !bookNote && it.isNotEmpty() }?.uppercase(Locale.ROOT),
chapter = if (bookNote) {
null
@ -412,6 +428,8 @@ object AnnotationWire {
},
totalProgression = if (bookNote) null else record.progression,
createdAt = existing?.createdAt ?: (record.clientTsMicros / 1000),
noteCreatedAt = noteCreatedAt,
noteUpdatedAt = noteUpdatedAt,
updatedAt = record.clientTsMicros,
)
}

7
app/src/main/kotlin/com/chmouel/liseur/domain/AnnotationBackup.kt
View file

@ -70,6 +70,8 @@ fun encodeAnnotationBackup(books: List<BackedUpBook>): String {
a.position?.let { put("position", it) }
a.totalProgression?.let { put("progression", it) }
put("created_at", a.createdAt)
a.noteCreatedAt?.let { put("note_created_at", it) }
a.noteUpdatedAt?.let { put("note_updated_at", it) }
},
)
}
@ -120,6 +122,8 @@ fun decodeAnnotationBackup(json: String): BackupContents {
position = if (m.has("position")) m.optInt("position") else null,
totalProgression = if (m.has("progression")) m.optDouble("progression") else null,
createdAt = m.optLong("created_at"),
noteCreatedAt = m.optLongOrNull("note_created_at"),
noteUpdatedAt = m.optLongOrNull("note_updated_at"),
)
}
out += BackedUpBook(
@ -165,6 +169,9 @@ fun matchBackedUpBook(
private fun JSONObject.optStringOrNull(key: String): String? =
if (isNull(key)) null else optString(key).takeIf { it.isNotEmpty() }
private fun JSONObject.optLongOrNull(key: String): Long? =
if (!has(key) || isNull(key)) null else optLong(key)
/**
* How much of a backup would land on books that are actually here.
*

28
app/src/main/kotlin/com/chmouel/liseur/reader/ReaderViewModel.kt
View file

@ -1405,6 +1405,8 @@ class ReaderViewModel(
annotation(locator, AnnotationKind.HIGHLIGHT).copy(
id = existing?.id ?: UUID.randomUUID().toString(),
note = existing?.note,
noteCreatedAt = existing?.noteCreatedAt,
noteUpdatedAt = existing?.noteUpdatedAt,
tint = tint.name,
createdAt = existing?.createdAt ?: System.currentTimeMillis(),
kind = existing?.kind ?: AnnotationKind.HIGHLIGHT.name,
@ -1415,12 +1417,25 @@ class ReaderViewModel(
/** Attaches the reader's own words to a passage. */
fun addNote(locator: Locator, note: String, existingId: String? = null) {
val existing = existingId?.let { id -> annotations.value.firstOrNull { it.id == id } }
val now = System.currentTimeMillis()
val noteCreatedAt = when {
existing?.noteCreatedAt != null -> existing.noteCreatedAt
existing?.note == null -> now
else -> existing.createdAt
}
val noteUpdatedAt = when {
existing?.note == null -> null
existing.note == note -> existing.noteUpdatedAt
else -> now * 1000
}
save(
annotation(locator, AnnotationKind.NOTE).copy(
id = existing?.id ?: UUID.randomUUID().toString(),
note = note,
noteCreatedAt = noteCreatedAt,
noteUpdatedAt = noteUpdatedAt,
tint = existing?.tint ?: highlightPalette.value.passageNoteTint.name,
createdAt = existing?.createdAt ?: System.currentTimeMillis(),
createdAt = existing?.createdAt ?: now,
),
)
}
@ -1428,6 +1443,13 @@ class ReaderViewModel(
/** Writes a thought about the book itself, without inventing a place for it. */
fun saveBookNote(note: String, existingId: String? = null) {
val existing = existingId?.let { id -> annotations.value.firstOrNull { it.id == id } }
val now = System.currentTimeMillis()
val noteCreatedAt = existing?.noteCreatedAt ?: now
val noteUpdatedAt = when {
existing?.note == null -> null
existing.note == note -> existing.noteUpdatedAt
else -> now * 1000
}
save(
BookAnnotation(
id = existing?.id ?: UUID.randomUUID().toString(),
@ -1435,7 +1457,9 @@ class ReaderViewModel(
kind = AnnotationKind.BOOK_NOTE.name,
locatorJson = "",
note = note,
createdAt = existing?.createdAt ?: System.currentTimeMillis(),
createdAt = existing?.createdAt ?: now,
noteCreatedAt = noteCreatedAt,
noteUpdatedAt = noteUpdatedAt,
),
)
}

8
app/src/main/kotlin/com/chmouel/liseur/reader/annotations/NoteSheet.kt
View file

@ -215,12 +215,14 @@ fun NoteSheet(
private fun stamps(annotation: BookAnnotation): String {
val context = LocalContext.current
val parts = mutableListOf<String>()
val noteCreatedAt = annotation.noteCreatedAt ?: annotation.createdAt
val noteUpdatedAt = annotation.noteUpdatedAt
annotation.chapter?.takeIf { it.isNotBlank() }?.let(parts::add)
parts += stringResource(R.string.annotation_note_added, dated(context, annotation.createdAt))
if (NoteText.edited(annotation.createdAt, annotation.updatedAt)) {
parts += stringResource(R.string.annotation_note_added, dated(context, noteCreatedAt))
if (noteUpdatedAt != null && NoteText.edited(noteCreatedAt, noteUpdatedAt)) {
parts += stringResource(
R.string.annotation_note_edited,
dated(context, annotation.updatedAt / 1000),
dated(context, noteUpdatedAt / 1000),
)
}
return parts.joinToString(" \u00B7 ")

12
app/src/main/kotlin/com/chmouel/liseur/reader/annotations/NoteText.kt
View file

@ -7,13 +7,13 @@ object NoteText {
/**
* Whether a note was reworked after it was written.
*
* A mark's two stamps are not in the same unit: `created_at` is in
* milliseconds and `updated_at` in microseconds, since the latter goes
* to liseur-sync verbatim. Saving a note stamps it a moment after
* creating it, so anything inside a minute is the same sitting.
* A note's two stamps are not in the same unit: `note_created_at` is
* in milliseconds and `note_updated_at` in microseconds, matching the
* mark-wide sync stamp precision. Saving a note stamps it a moment
* after creating it, so anything inside a minute is the same sitting.
*/
fun edited(createdAtMs: Long, updatedAtMicros: Long): Boolean {
if (updatedAtMicros <= 0L) return false
fun edited(createdAtMs: Long, updatedAtMicros: Long?): Boolean {
if (updatedAtMicros == null || updatedAtMicros <= 0L) return false
val updatedAtMs = TimeUnit.MICROSECONDS.toMillis(updatedAtMicros)
return updatedAtMs - createdAtMs > TimeUnit.MINUTES.toMillis(1)
}

39
app/src/test/kotlin/com/chmouel/liseur/data/db/MigrationTest.kt
View file

@ -1126,10 +1126,47 @@ class MigrationTest {
}
}
@Test
fun `annotation note dates split from mark update dates on upgrade`() {
helper.createDatabase(TEST_DB, 49).use { old ->
old.execSQL(
"""
INSERT INTO annotations
(id, book_id, kind, locator_json, text, note, tint, chapter,
position, total_progression, created_at, updated_at)
VALUES
('note-1', 'book', 'NOTE', '{}', 'passage', 'words', 'YELLOW', 'One',
1, 0.25, 1000, 3000000),
('highlight-1', 'book', 'HIGHLIGHT', '{}', 'passage', NULL, 'BLUE', 'One',
2, 0.50, 2000, 4000000)
""".trimIndent(),
)
}
helper.runMigrationsAndValidate(TEST_DB, LATEST, true, *LiseurDatabase.MIGRATIONS).use { db ->
db.query(
"""
SELECT id, note_created_at, note_updated_at
FROM annotations
ORDER BY id
""".trimIndent(),
).use { cursor ->
assertTrue(cursor.moveToFirst())
assertEquals("highlight-1", cursor.getString(0))
assertTrue(cursor.isNull(1))
assertTrue(cursor.isNull(2))
assertTrue(cursor.moveToNext())
assertEquals("note-1", cursor.getString(0))
assertEquals(1000, cursor.getLong(1))
assertTrue(cursor.isNull(2))
}
}
}
private companion object {
const val TEST_DB = "migration-test.db"
/** Kept in step with the `version` on [LiseurDatabase]. */
const val LATEST = 49
const val LATEST = 50
}
}

44
app/src/test/kotlin/com/chmouel/liseur/data/liseursync/AnnotationWireTest.kt
View file

@ -59,10 +59,10 @@ class AnnotationWireTest {
val record = AnnotationWire.record(
server(kind = "highlight", body = "worth arguing with"),
)!!
assertEquals(
AnnotationKind.NOTE.name,
AnnotationWire.toAnnotation(record, BOOK, null).kind,
)
val landed = AnnotationWire.toAnnotation(record, BOOK, null)
assertEquals(AnnotationKind.NOTE.name, landed.kind)
assertEquals(MICROS / 1000, landed.noteCreatedAt)
assertNull(landed.noteUpdatedAt)
}
@Test
@ -108,6 +108,38 @@ class AnnotationWireTest {
assertFalse(JSONObject(item!!.json).has("color"))
}
@Test
fun `a remote recolour does not become a note edit`() {
val existing = mark(
kind = AnnotationKind.NOTE,
note = "worth arguing with",
noteCreatedAt = 1_700_000_000_000,
noteUpdatedAt = null,
)
val record = AnnotationWire.record(server(body = "worth arguing with", color = "blue"))!!
val landed = AnnotationWire.toAnnotation(record, BOOK, existing)
assertEquals(existing.noteCreatedAt, landed.noteCreatedAt)
assertNull(landed.noteUpdatedAt)
}
@Test
fun `a remote body change advances the note edit stamp`() {
val existing = mark(
kind = AnnotationKind.NOTE,
note = "old thought",
noteCreatedAt = 1_700_000_000_000,
noteUpdatedAt = null,
)
val record = AnnotationWire.record(server(body = "new thought"))!!
val landed = AnnotationWire.toAnnotation(record, BOOK, existing)
assertEquals(existing.noteCreatedAt, landed.noteCreatedAt)
assertEquals(MICROS, landed.noteUpdatedAt)
}
// -- Fingerprints ------------------------------------------------------
@Test
@ -505,6 +537,8 @@ class AnnotationWireTest {
text: String? = "a passage",
note: String? = null,
tint: String? = "YELLOW",
noteCreatedAt: Long? = null,
noteUpdatedAt: Long? = null,
updatedAt: Long = MICROS,
) = BookAnnotation(
id = id,
@ -518,6 +552,8 @@ class AnnotationWireTest {
position = 12,
totalProgression = 0.25,
createdAt = 1_709_294_400_000,
noteCreatedAt = noteCreatedAt,
noteUpdatedAt = noteUpdatedAt,
updatedAt = updatedAt,
)

15
app/src/test/kotlin/com/chmouel/liseur/domain/AnnotationBackupTest.kt
View file

@ -66,6 +66,21 @@ class AnnotationBackupTest {
assertEquals(full, back.single().annotations.single())
}
@Test
fun `note timestamps survive a backup`() {
val noted = full.copy(
kind = AnnotationKind.NOTE.name,
noteCreatedAt = 1_700_000_010_000,
noteUpdatedAt = 1_700_000_020_000_000,
)
val back = roundTrip(
listOf(BackedUpBook("calibre:uuid-1", "A Book", "An Author", listOf(noted))),
)
assertEquals(noted, back.single().annotations.single())
}
@Test
fun `a bookmark with nothing written on it comes back empty, not blank`() {
val back = roundTrip(listOf(BackedUpBook("calibre:uuid-1", null, null, listOf(bare))))

5
app/src/test/kotlin/com/chmouel/liseur/reader/annotations/NoteTextTest.kt
View file

@ -25,6 +25,11 @@ class NoteTextTest {
assertFalse(NoteText.edited(createdMs, 0L))
}
@Test
fun `a row with no note edit stamp is not edited`() {
assertFalse(NoteText.edited(createdMs, null))
}
@Test
fun `microseconds are not mistaken for milliseconds`() {
// The same instant in both units must read as the same sitting.