From 845d5768200cd1fb6304f1649f578c7cf81f016c Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 6 Nov 2026 22:20:24 -0500 Subject: [PATCH] fix(chat): keep stored history when a newest page misses it Opening a chat after missing 50 or more messages deleted every stored row, because the newest page did not overlap them and an unmarked gap would have rendered as contiguous. A 10,000-message chat was left with 50 rows and re-paged from the network. Record the gap instead (schema 66, conversation_history_gap): keep the rows below it, stop every transcript read at the gap's upper bound, and fill it by paging GetMessages back from that bound until a page reaches the stored rows. A second miss before the fill drops only the run between the two gaps. An empty page below the gap drops the rows under it, since the server holds nothing older. The fill pages GetMessages rather than streaming GetDelta forward: event_sequence is a message's current-state version (an edit or reaction bumps it on an old id), so a cursor taken from a stored row does not mark where the gap starts. --- .../Controllers/ConversationController.swift | 52 ++--- .../Screens/Conversation/MessageLoader.swift | 10 +- .../Database+Conversations.swift | 170 +++++++++++++--- .../Sources/FlipcashStore/Database.swift | 2 +- .../Sources/FlipcashStore/Schema.swift | 25 +++ .../ConversationHistoryGapTests.swift | 192 ++++++++++++++++++ .../TestSupport/MockConversations.swift | 17 ++ 7 files changed, 407 insertions(+), 61 deletions(-) create mode 100644 FlipcashTests/ConversationHistoryGapTests.swift diff --git a/Flipcash/Core/Controllers/ConversationController.swift b/Flipcash/Core/Controllers/ConversationController.swift index f4bfc1fbc..653d03b41 100644 --- a/Flipcash/Core/Controllers/ConversationController.swift +++ b/Flipcash/Core/Controllers/ConversationController.swift @@ -1000,10 +1000,9 @@ final class ConversationController { let (reconciled, pairs) = reconciledForPersist(messages, in: conversationID) let floor = messages.map(\.eventSequence).filter({ $0 > 0 }).min() ?? 0 // Page + floor cursor land atomically (a failed write must not seat the cursor), and a - // non-overlapping page drops the stale older epoch first. + // non-overlapping page records a history gap instead of joining the older rows. let ok = persist(operation: "reset-resync") { - try dropStaleEpochIfNeeded(before: messages, in: conversationID) - try database.persistMessages(reconciled, cursor: floor, conversationID: conversationID) + try database.persistNewestPage(reconciled, cursor: floor, conversationID: conversationID) } if ok { commitReconciled(pairs, in: conversationID) @@ -1790,17 +1789,6 @@ final class ConversationController { } } - /// A freshly fetched newest page that does not overlap the retained history proves nothing about - /// contiguity — the interior gap would render seamlessly stitched and can never be fetched (older - /// paging anchors below the *oldest* persisted row). Drop the stale older epoch; it re-pages from - /// the server on scroll. Must run before the page itself is persisted. - private func dropStaleEpochIfNeeded(before page: [ConversationMessage], in conversationID: ConversationID) throws { - guard let oldestOfPage = page.map(\.id.value).min(), - let newestPersisted = try database.newestMessageID(conversationID: conversationID), - newestPersisted.value < oldestOfPage else { return } - try database.deleteMessages(conversationID: conversationID) - } - /// Recompute the feed row's preview from the newest persisted *visible* message (the store no longer /// holds the confirmed transcript to derive it from). func refreshFeedPreview(for conversationID: ConversationID) { @@ -2337,6 +2325,13 @@ final class ConversationController { ((try? database.messagesWindow(conversationID: conversationID, before: nil, limit: limit)) ?? []).first?.id.value } + /// The history gap in `conversationID`'s stored rows, or nil when they are contiguous. Observes + /// `messageRevision`, since every write that opens, narrows or closes a gap bumps it. + func historyGap(in conversationID: ConversationID) -> HistoryGap? { + _ = messageRevision // observe: re-read when a confirmed DB write lands + return (try? database.historyGap(conversationID: conversationID)) ?? nil + } + /// The persisted id `step` rows older than `before` — the loader's next window anchor — or nil when /// no older history is persisted locally (time to page the server). func olderAnchor(for conversationID: ConversationID, before: UInt64, step: Int) -> UInt64? { @@ -2384,13 +2379,13 @@ final class ConversationController { // head — fetching only genuinely newer messages, appended at the tail — instead of a // `GetDelta(after: 0)` that re-pulls the whole history and prepends it, knocking the // transcript off the bottom on first open. Page + cursor land atomically (a failed write - // must not seat the cursor past messages that never persisted), and a non-overlapping page - // drops the stale older epoch first. + // must not seat the cursor past messages that never persisted). A page that does not + // overlap the stored rows records a history gap: the older rows stay, the transcript reads + // only from the page up, and paging back fills the gap until it reaches them. let (reconciled, pairs) = reconciledForPersist(messages, in: conversationID) let head = messages.map(\.eventSequence).max() ?? 0 let ok = persist(operation: "load-messages") { - try dropStaleEpochIfNeeded(before: messages, in: conversationID) - try database.persistMessages(reconciled, cursor: head, conversationID: conversationID) + try database.persistNewestPage(reconciled, cursor: head, conversationID: conversationID) } if ok { commitReconciled(pairs, in: conversationID) @@ -2439,13 +2434,16 @@ final class ConversationController { olderPageState[conversationID]?.isLoading ?? false } - /// Pages strictly older than the oldest loaded message, prepends it to the in-memory window, and + /// Pages strictly older than the oldest readable message, prepends it to the in-memory window, and /// persists it — retention is on, so paged-in history stays in the DB and a reopen reads it locally - /// instead of re-fetching. No-ops while a page is in flight or once history is exhausted. + /// instead of re-fetching. Above a history gap this pages before the gap's upper bound, so each + /// page fills the gap from the top until it reaches the stored rows below it. No-ops while a page + /// is in flight or once history is exhausted. func loadOlderMessages(for conversationID: ConversationID) async { guard canRead(conversationID) else { return } guard hasMoreOlderMessages(for: conversationID), !isLoadingOlderMessages(for: conversationID) else { return } - // Page before the oldest PERSISTED id — the DB holds all viewed history; the store may be trimmed. + // Page before the oldest PERSISTED readable id — the DB holds all viewed history; the store may + // be trimmed. Rows below a history gap are not readable, so this is the gap's upper bound. guard let oldest = (try? database.oldestMessageID(conversationID: conversationID)).flatMap({ $0 }) else { return } olderPageState[conversationID, default: OlderPageState()].isLoading = true @@ -2453,16 +2451,22 @@ final class ConversationController { do { let older = try await messaging.getMessages(owner: owner, conversationID: conversationID, before: oldest) + let hadGap = (try? database.historyGap(conversationID: conversationID)) != nil if older.isEmpty { olderPageState[conversationID]?.hasMore = false + // Nothing older on the server: rows below a gap are not part of this history. + if hadGap { + _ = persist(operation: "load-older") { try database.persistOlderPage([], conversationID: conversationID) } + } } else { // Retention is on, so persist the paged-in history: the next open reads it from the DB - // instead of re-fetching. Prepended history must not play insertion transitions — the - // revision bump and the resulting re-read ride an animation-suppressed transaction. + // instead of re-fetching. The same write narrows or closes a history gap. Prepended + // history must not play insertion transitions — the revision bump and the resulting + // re-read ride an animation-suppressed transaction. var transaction = Transaction() transaction.disablesAnimations = true withTransaction(transaction) { - _ = persist(operation: "load-older") { try database.upsertConversationMessages(older, conversationID: conversationID) } + _ = persist(operation: "load-older") { try database.persistOlderPage(older, conversationID: conversationID) } } } } catch { diff --git a/Flipcash/Core/Screens/Conversation/MessageLoader.swift b/Flipcash/Core/Screens/Conversation/MessageLoader.swift index 7baa36ca7..fd214d3b2 100644 --- a/Flipcash/Core/Screens/Conversation/MessageLoader.swift +++ b/Flipcash/Core/Screens/Conversation/MessageLoader.swift @@ -38,10 +38,10 @@ final class MessageLoader { } /// Whether a rendered window of `windowCount` messages is the entire locally-known history: - /// nothing paged back and the window under its limit. The count is passed in because - /// `messages` is a DB read the caller already holds. + /// nothing paged back, the window under its limit, and no history gap under it. The count is + /// passed in because `messages` is a DB read the caller already holds. func isEntireHistory(windowCount: Int) -> Bool { - startID == nil && windowCount < Self.initialWindow + startID == nil && windowCount < Self.initialWindow && controller.historyGap(in: conversationID) == nil } /// Brings `messageID` into the rendered window, returning whether it is now there. @@ -50,11 +50,13 @@ final class MessageLoader { /// forward, so anchoring at or just before the target is enough. The anchor sits `leadIn` /// messages early where history allows, so the transcript can center the target instead of /// pinning it as the cut-off top row. Nothing is fetched — a message the - /// local database has never seen returns `false` and the caller leaves the transcript alone. + /// local database has never seen returns `false` and the caller leaves the transcript alone, as + /// does one stored below a history gap, which no window reaches until paging fills the gap. @discardableResult func reveal(_ messageID: MessageID) -> Bool { if messages.contains(where: { $0.id == messageID }) { return true } guard controller.persistedMessage(messageID, in: conversationID) != nil else { return false } + if let gap = controller.historyGap(in: conversationID), messageID.value < gap.above.value { return false } startID = controller.olderAnchor(for: conversationID, before: messageID.value, step: Self.leadIn) ?? messageID.value return true } diff --git a/FlipcashCore/Sources/FlipcashStore/Database+Conversations.swift b/FlipcashCore/Sources/FlipcashStore/Database+Conversations.swift index 26257f61c..35bc00330 100644 --- a/FlipcashCore/Sources/FlipcashStore/Database+Conversations.swift +++ b/FlipcashCore/Sources/FlipcashStore/Database+Conversations.swift @@ -188,11 +188,11 @@ nonisolated extension Database { } /// A bounded window of a conversation's messages, oldest-first: the newest `limit` when `before` is - /// nil, otherwise the `limit` messages immediately older than `before`. Index-backed by the - /// composite `(conversationId, id)` primary key — no scan, no sort. + /// nil, otherwise the `limit` messages immediately older than `before`. Stops at a history gap. + /// Index-backed by the composite `(conversationId, id)` primary key — no scan, no sort. public func messagesWindow(conversationID: ConversationID, before: MessageID? = nil, limit: Int) throws -> [ConversationMessage] { let m = ConversationMessageTable() - var query = m.table.filter(m.conversationId == conversationID.data) + var query = m.table.filter(m.conversationId == conversationID.data && m.id >= (try historyFloor(conversationID))) if let before { query = query.filter(m.id < before.value) } @@ -202,22 +202,23 @@ nonisolated extension Database { /// Every message from `startID` (inclusive) to the newest, oldest-first — the id-anchored window. /// Anchoring by id means an arriving message grows the window at the tail instead of sliding the - /// oldest revealed row out from under a reader who has scrolled up. + /// oldest revealed row out from under a reader who has scrolled up. Stops at a history gap. public func messages(conversationID: ConversationID, from startID: UInt64) throws -> [ConversationMessage] { let m = ConversationMessageTable() + let floor = max(startID, try historyFloor(conversationID)) let rows = try reader.prepareRowIterator( - m.table.filter(m.conversationId == conversationID.data && m.id >= startID).order(m.id.asc) + m.table.filter(m.conversationId == conversationID.data && m.id >= floor).order(m.id.asc) ) return try rows.map { conversationMessage(from: $0) }.compactMap { $0 } } /// Every message in the half-open id interval `(after, through]`, oldest-first — the window a /// read-pointer advance just crossed. A nil `after` means the pointer had never been set, so the - /// whole stored history up to `through` counts as newly read. Index-backed by the composite - /// `(conversationId, id)` primary key. + /// whole stored history up to `through` counts as newly read. Stops at a history gap, since rows + /// below one were never on screen. Index-backed by the composite `(conversationId, id)` primary key. public func messages(conversationID: ConversationID, after: MessageID?, through: MessageID) throws -> [ConversationMessage] { let m = ConversationMessageTable() - var query = m.table.filter(m.conversationId == conversationID.data && m.id <= through.value) + var query = m.table.filter(m.conversationId == conversationID.data && m.id <= through.value && m.id >= (try historyFloor(conversationID))) if let after { query = query.filter(m.id > after.value) } @@ -226,10 +227,10 @@ nonisolated extension Database { } /// The id `step` rows older than `before` — the next anchor when the reader pages back — falling - /// back to the oldest available older row; nil when nothing older is persisted. + /// back to the oldest available older row; nil when nothing older is persisted above a history gap. public func olderAnchor(conversationID: ConversationID, before: UInt64, step: Int) throws -> UInt64? { let m = ConversationMessageTable() - let older = m.table.filter(m.conversationId == conversationID.data && m.id < before) + let older = m.table.filter(m.conversationId == conversationID.data && m.id < before && m.id >= (try historyFloor(conversationID))) if let row = try reader.pluck(older.order(m.id.desc).limit(1, offset: step - 1)) { return row[m.id] } @@ -245,13 +246,15 @@ nonisolated extension Database { /// The number of stored messages after `after` that someone other than `selfUserID` sent and /// that are not deleted — the count the unread divider shows. A row with no sender counts as - /// someone else's, except a system message, which is never unread. Index-backed by the composite `(conversationId, id)` primary key. + /// someone else's, except a system message, which is never unread. Stops at a history gap, like + /// every read the transcript makes. Index-backed by the composite `(conversationId, id)` primary key. public func inboundMessageCount(conversationID: ConversationID, after: MessageID, excludingSender selfUserID: UUID) throws -> Int { let m = ConversationMessageTable() return try reader.scalar( m.table.filter( m.conversationId == conversationID.data && m.id > after.value + && m.id >= (try historyFloor(conversationID)) && m.kind != 2 && m.kind != 6 && (m.senderId == nil || m.senderId != selfUserID) @@ -289,13 +292,15 @@ nonisolated extension Database { /// The oldest stored message id after `after` that someone other than `selfUserID` sent, deleted /// or not, or nil when there is none — where the unread divider goes. A row with no sender counts - /// as someone else's, except a system message, so the divider never lands above one. + /// as someone else's, except a system message, so the divider never lands above one. Stops at a + /// history gap, so the divider never points at a row no window reaches. public func firstInboundMessageID(conversationID: ConversationID, after: MessageID, excludingSender selfUserID: UUID) throws -> UInt64? { let m = ConversationMessageTable() return try reader.pluck( m.table.filter( m.conversationId == conversationID.data && m.id > after.value + && m.id >= (try historyFloor(conversationID)) && m.kind != 6 && (m.senderId == nil || m.senderId != selfUserID) ).order(m.id.asc) @@ -303,30 +308,31 @@ nonisolated extension Database { } /// The newest stored message id at or below `through`, or nil when none is stored — the row the - /// unread divider sits under when the READ pointer's own message is gone. + /// unread divider sits under when the READ pointer's own message is gone. Stops at a history gap. public func newestMessageID(conversationID: ConversationID, through: MessageID) throws -> UInt64? { let m = ConversationMessageTable() return try reader.pluck( - m.table.filter(m.conversationId == conversationID.data && m.id <= through.value).order(m.id.desc) + m.table.filter(m.conversationId == conversationID.data && m.id <= through.value && m.id >= (try historyFloor(conversationID))).order(m.id.desc) ).map { $0[m.id] } } - /// The ids of up to `limit` stored messages older than `before` (or the newest when nil), newest first. + /// The ids of up to `limit` stored messages older than `before` (or the newest when nil), newest + /// first, stopping at a history gap. public func messageIDs(conversationID: ConversationID, before: UInt64? = nil, limit: Int) throws -> [MessageID] { let m = ConversationMessageTable() - var query = m.table.select(m.id).filter(m.conversationId == conversationID.data) + var query = m.table.select(m.id).filter(m.conversationId == conversationID.data && m.id >= (try historyFloor(conversationID))) if let before { query = query.filter(m.id < before) } return try reader.prepareRowIterator(query.order(m.id.desc).limit(limit)).map { MessageID(value: $0[m.id]) } } - /// The oldest persisted message id for a conversation, or nil when none is cached — the anchor for - /// paging genuinely older history from the server. + /// The oldest persisted message id above any history gap, or nil when none is cached — the anchor + /// for paging older history from the server, which fills the gap when there is one. public func oldestMessageID(conversationID: ConversationID) throws -> MessageID? { let m = ConversationMessageTable() return try reader.pluck( - m.table.filter(m.conversationId == conversationID.data).order(m.id.asc) + m.table.filter(m.conversationId == conversationID.data && m.id >= (try historyFloor(conversationID))).order(m.id.asc) ).map { MessageID(value: $0[m.id]) } } @@ -461,32 +467,120 @@ nonisolated extension Database { @discardableResult public func persistMessages(_ messages: [ConversationMessage], cursor: UInt64, conversationID: ConversationID) throws -> Bool { try write { writer in - let c = ConversationTable() var changed = false // IMMEDIATE: reads the current cursor before updating it (see replaceConversationFeed). try writer.transaction(.immediate) { - for message in messages { - changed = try writeMessage(message, conversationId: conversationID.data) || changed - } - if cursor > 0 { - let scoped = c.table.filter(c.id == conversationID.data) - let current = try writer.pluck(scoped).flatMap { $0[c.catchupCursor] } ?? 0 - if cursor > current { - try writer.run(scoped.update(c.catchupCursor <- cursor)) + changed = try writeMessages(messages, cursor: cursor, conversationID: conversationID, writer: writer) + } + return changed + } + } + + /// The body of ``persistMessages(_:cursor:conversationID:)``, for callers already inside a transaction. + private func writeMessages(_ messages: [ConversationMessage], cursor: UInt64, conversationID: ConversationID, writer: Connection) throws -> Bool { + let c = ConversationTable() + var changed = false + for message in messages { + changed = try writeMessage(message, conversationId: conversationID.data) || changed + } + if cursor > 0 { + let scoped = c.table.filter(c.id == conversationID.data) + let current = try writer.pluck(scoped).flatMap { $0[c.catchupCursor] } ?? 0 + if cursor > current { + try writer.run(scoped.update(c.catchupCursor <- cursor)) + } + } + return changed + } + + // MARK: - History gap + + /// The bounds of a conversation's history gap, or nil when its stored history is contiguous. + public func historyGap(conversationID: ConversationID) throws -> HistoryGap? { + let g = ConversationHistoryGapTable() + return try reader.pluck(g.table.filter(g.conversationId == conversationID.data)).map { + HistoryGap(below: MessageID(value: $0[g.belowId]), above: MessageID(value: $0[g.aboveId])) + } + } + + /// The lowest id the transcript may read: the gap's upper bound, or 0 when there is no gap. Rows + /// below it are kept but are not contiguous with the rows above, so no window may join them. + func historyFloor(_ conversationID: ConversationID) throws -> UInt64 { + try historyGap(conversationID: conversationID)?.above.value ?? 0 + } + + /// Persists a newest page (and its cursor) the way ``persistMessages(_:cursor:conversationID:)`` + /// does, first recording a history gap when the page does not overlap the stored rows. The older + /// rows are kept, not deleted, so paging back can reconnect to them instead of re-fetching them. + /// A page that misses an unfilled gap's upper run as well drops that run — it is bounded by gaps + /// on both sides — and moves the gap's upper bound to the new page, keeping its lower bound. + @discardableResult + public func persistNewestPage(_ messages: [ConversationMessage], cursor: UInt64, conversationID: ConversationID) throws -> Bool { + try write { writer in + let m = ConversationMessageTable() + let g = ConversationHistoryGapTable() + var changed = false + try writer.transaction(.immediate) { + if let pageOldest = messages.map(\.id.value).min(), + let newestStored = try writer.pluck( + m.table.select(m.id).filter(m.conversationId == conversationID.data).order(m.id.desc) + )?[m.id], + newestStored < pageOldest { + let existing = try writer.pluck(g.table.filter(g.conversationId == conversationID.data)) + let below = existing?[g.belowId] ?? newestStored + if existing != nil { + try writer.run(m.table.filter(m.conversationId == conversationID.data && m.id > below && m.id < pageOldest).delete()) } + try writer.run(g.table.insert(or: .replace, + g.conversationId <- conversationID.data, + g.belowId <- below, + g.aboveId <- pageOldest + )) } + changed = try writeMessages(messages, cursor: cursor, conversationID: conversationID, writer: writer) } return changed } } - /// Deletes a conversation's persisted messages — used when a freshly fetched newest page does not - /// overlap the retained history, so a stale older epoch can't render seamlessly stitched to the new - /// page across an unfetchable gap. + /// Persists a page of older history fetched before the transcript's oldest readable row, and + /// narrows the history gap when there is one, in one transaction so a reader never sees the page + /// without the floor that exposes it. A page reaching the gap's lower bound closes it; a page that + /// stops short moves the upper bound down to the page; an empty page means the server holds + /// nothing older, so the rows below the gap are not part of this history and are dropped. + public func persistOlderPage(_ messages: [ConversationMessage], conversationID: ConversationID) throws { + try write { writer in + let m = ConversationMessageTable() + let g = ConversationHistoryGapTable() + try writer.transaction(.immediate) { + for message in messages { + try writeMessage(message, conversationId: conversationID.data) + } + let scoped = g.table.filter(g.conversationId == conversationID.data) + guard let gap = try writer.pluck(scoped) else { return } + guard let pageOldest = messages.map(\.id.value).min() else { + try writer.run(m.table.filter(m.conversationId == conversationID.data && m.id < gap[g.aboveId]).delete()) + try writer.run(scoped.delete()) + return + } + if pageOldest <= gap[g.belowId] { + try writer.run(scoped.delete()) + } else if pageOldest < gap[g.aboveId] { + try writer.run(scoped.update(g.aboveId <- pageOldest)) + } + } + } + } + + /// Deletes a conversation's persisted messages and any history gap between them. public func deleteMessages(conversationID: ConversationID) throws { try write { writer in let m = ConversationMessageTable() - try writer.run(m.table.filter(m.conversationId == conversationID.data).delete()) + let g = ConversationHistoryGapTable() + try writer.transaction(.immediate) { + try writer.run(m.table.filter(m.conversationId == conversationID.data).delete()) + try writer.run(g.table.filter(g.conversationId == conversationID.data).delete()) + } } } @@ -1064,3 +1158,15 @@ nonisolated extension Database { ) } } + +/// A break in a conversation's stored history: `below` is the newest stored id under it, `above` the +/// oldest id of the contiguous run the transcript reads. +nonisolated public struct HistoryGap: Equatable, Sendable { + public let below: MessageID + public let above: MessageID + + public init(below: MessageID, above: MessageID) { + self.below = below + self.above = above + } +} diff --git a/FlipcashCore/Sources/FlipcashStore/Database.swift b/FlipcashCore/Sources/FlipcashStore/Database.swift index 889f7ab7d..5f7bc94c9 100644 --- a/FlipcashCore/Sources/FlipcashStore/Database.swift +++ b/FlipcashCore/Sources/FlipcashStore/Database.swift @@ -221,7 +221,7 @@ nonisolated open class Database: @unchecked Sendable { /// the notification service extension needs the same number to decide whether the store on disk /// is one it understands, and an extension cannot read the app's `Info.plist` — separate bundles. /// Both targets link this module, so they cannot disagree. - public static let schemaVersion = 65 + public static let schemaVersion = 66 /// Removes the store and the write-ahead log files beside it. /// diff --git a/FlipcashCore/Sources/FlipcashStore/Schema.swift b/FlipcashCore/Sources/FlipcashStore/Schema.swift index 95914b275..713527396 100644 --- a/FlipcashCore/Sources/FlipcashStore/Schema.swift +++ b/FlipcashCore/Sources/FlipcashStore/Schema.swift @@ -392,6 +392,21 @@ nonisolated public struct GroupMembershipTable: Sendable { public let conversationId = Expression ("conversationId") } +/// A break in a conversation's stored history, left when a newest page arrived that did not overlap +/// the rows already stored. Rows at or above `aboveId` are the contiguous run the transcript reads; +/// rows at or below `belowId` are older history kept for when paging back reaches them. At most one +/// row per conversation. +nonisolated public struct ConversationHistoryGapTable: Sendable { + public static let name = "conversation_history_gap" + + public init() {} + + public let table = Table(Self.name) + public let conversationId = Expression ("conversationId") + public let belowId = Expression ("belowId") + public let aboveId = Expression ("aboveId") +} + nonisolated public struct ConversationMemberTable: Sendable { public static let name = "conversation_member" @@ -843,6 +858,16 @@ nonisolated extension Database { }) } + let historyGapTable = ConversationHistoryGapTable() + + try writer.transaction(.immediate) { + try writer.run(historyGapTable.table.create(ifNotExists: true, withoutRowid: true) { t in + t.column(historyGapTable.conversationId, primaryKey: true) + t.column(historyGapTable.belowId) + t.column(historyGapTable.aboveId) + }) + } + let blocklistTable = BlocklistTable() try writer.transaction(.immediate) { diff --git a/FlipcashTests/ConversationHistoryGapTests.swift b/FlipcashTests/ConversationHistoryGapTests.swift new file mode 100644 index 000000000..79dca9fe0 --- /dev/null +++ b/FlipcashTests/ConversationHistoryGapTests.swift @@ -0,0 +1,192 @@ +// +// ConversationHistoryGapTests.swift +// FlipcashTests +// + +import Testing +import Foundation +import FlipcashCore +import FlipcashStore +@testable import Flipcash + +/// A newest page that does not overlap the stored rows leaves a gap between them. The rows below are +/// kept, the transcript reads only from the page up, and paging back fills the gap. +@MainActor +@Suite("Conversation history gap") +struct ConversationHistoryGapTests { + + private let id = ConversationID.test(1) + + private func makeController(_ server: MockConversations, database: Database) -> ConversationController { + ConversationController( + fetching: server, membership: server, viewerSettings: server, messaging: server, streaming: server, + contactNaming: MockDMContactNaming(), + database: database, + owner: .generate()!, selfUserID: UUID(), + typingHeartbeatInterval: .seconds(3), incomingTypingExpiry: .seconds(10) + ) + } + + private func message(_ id: UInt64) -> ConversationMessage { + ConversationMessage(id: MessageID(value: id), senderID: nil, content: .text("m\(id)"), date: Date(timeIntervalSince1970: TimeInterval(id)), unreadSeq: id, eventSequence: id) + } + + private func messages(_ ids: ClosedRange) -> [ConversationMessage] { + ids.map(message) + } + + private func storedIDs(_ database: Database) throws -> [UInt64] { + try database.getConversationMessages(conversationID: id).map(\.id.value).sorted() + } + + /// Stores 1...200, then opens the chat to a server whose newest page is 301...350: 100 missed. + private func openAfterMissing(_ database: Database) async throws -> (MockConversations, ConversationController) { + try database.upsertConversationMessages(messages(1...200), conversationID: id) + let server = MockConversations() + server.messages = messages(301...350) + server.olderHistory = messages(1...350) + let controller = makeController(server, database: database) + #expect(await controller.loadMessages(for: id)) + return (server, controller) + } + + @Test("a newest page that misses the stored rows keeps them and records the gap") + func gapKeepsRows() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (_, controller) = try await openAfterMissing(database) + + #expect(try storedIDs(database) == Array(1...200) + Array(301...350)) + #expect(try database.historyGap(conversationID: id) == HistoryGap(below: MessageID(value: 200), above: MessageID(value: 301))) + // The transcript reads only the run above the gap. + #expect(controller.messages(for: id).map(\.id.value) == Array(301...350)) + } + + @Test("the first window over a gap holds only the page, not stored rows from below it") + func firstWindowStopsAtGap() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (_, controller) = try await openAfterMissing(database) + let loader = MessageLoader(conversationID: id, controller: controller) + + // The opening window asks for 60 rows; the 10 below the gap must not stitch on. + #expect(loader.messages.map(\.id.value) == Array(301...350)) + #expect(!loader.isEntireHistory(windowCount: loader.messages.count)) + #expect(controller.olderAnchor(for: id, before: 301, step: 40) == nil) + } + + @Test("paging back fills the gap, then clears it, with every message stored once") + func pagingFillsGap() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (server, controller) = try await openAfterMissing(database) + + await controller.loadOlderMessages(for: id) + #expect(try database.historyGap(conversationID: id)?.above == MessageID(value: 251)) + await controller.loadOlderMessages(for: id) + #expect(try database.historyGap(conversationID: id)?.above == MessageID(value: 201)) + // The next page reaches the stored rows and closes the gap. + await controller.loadOlderMessages(for: id) + + #expect(server.olderQueries.map(\.value) == [301, 251, 201]) + #expect(try database.historyGap(conversationID: id) == nil) + #expect(try storedIDs(database) == Array(1...350)) + // The rows below the old gap are readable again, contiguous with the page. + #expect(controller.olderAnchor(for: id, before: 201, step: 40) == 161) + #expect(controller.hasMoreOlderMessages(for: id)) + } + + @Test("a failed fill keeps the gap and the next page resumes it") + func failedFillRetries() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (server, controller) = try await openAfterMissing(database) + server.olderError = URLError(.notConnectedToInternet) + + await controller.loadOlderMessages(for: id) + #expect(try database.historyGap(conversationID: id) == HistoryGap(below: MessageID(value: 200), above: MessageID(value: 301))) + #expect(try storedIDs(database) == Array(1...200) + Array(301...350)) + + server.olderError = nil + await controller.loadOlderMessages(for: id) + #expect(server.olderQueries.map(\.value) == [301, 301]) + #expect(try database.historyGap(conversationID: id)?.above == MessageID(value: 251)) + } + + @Test("a page that overlaps the stored rows records no gap and drops nothing") + func overlapUnchanged() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + try database.upsertConversationMessages(messages(1...200), conversationID: id) + let server = MockConversations() + server.messages = messages(180...229) + let controller = makeController(server, database: database) + + #expect(await controller.loadMessages(for: id)) + + #expect(try database.historyGap(conversationID: id) == nil) + #expect(try storedIDs(database) == Array(1...229)) + } + + @Test("a second miss before the gap fills drops the run between the gaps and keeps the oldest rows") + func secondGapDropsIsland() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (server, controller) = try await openAfterMissing(database) + server.messages = messages(501...550) + #expect(await controller.loadMessages(for: id)) + + #expect(try storedIDs(database) == Array(1...200) + Array(501...550)) + #expect(try database.historyGap(conversationID: id) == HistoryGap(below: MessageID(value: 200), above: MessageID(value: 501))) + #expect(controller.messages(for: id).map(\.id.value) == Array(501...550)) + } + + @Test("an empty page below the gap means the rows under it are not this chat's history") + func emptyPageBelowGapDropsOrphans() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (server, controller) = try await openAfterMissing(database) + server.olderHistory = messages(301...350) + + await controller.loadOlderMessages(for: id) + + #expect(try database.historyGap(conversationID: id) == nil) + #expect(try storedIDs(database) == Array(301...350)) + #expect(!controller.hasMoreOlderMessages(for: id)) + } + + @Test("a read pointer below the gap counts and places nothing from the rows under it") + func readPointerBelowGap() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (_, controller) = try await openAfterMissing(database) + + // 1...200 have no sender, so they count as inbound; only the 50 above the gap may. + #expect(controller.inboundMessageCount(after: MessageID(value: 100), in: id) == 50) + #expect(try database.firstInboundMessageID(conversationID: id, after: MessageID(value: 100), excludingSender: UUID()) == 301) + } + + @Test("revealing a stored message below the gap pages the gap closed first") + func revealBelowGap() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + let (_, controller) = try await openAfterMissing(database) + let loader = MessageLoader(conversationID: id, controller: controller) + + // Stored, but no window reaches it across the gap. + #expect(!loader.reveal(MessageID(value: 100))) + + #expect(await loader.revealPaging(MessageID(value: 100))) + #expect(try database.historyGap(conversationID: id) == nil) + #expect(loader.messages.contains { $0.id.value == 100 }) + #expect(loader.messages.map(\.id.value).last == 350) + } +} diff --git a/FlipcashTests/TestSupport/MockConversations.swift b/FlipcashTests/TestSupport/MockConversations.swift index 611b365a8..cb5636e02 100644 --- a/FlipcashTests/TestSupport/MockConversations.swift +++ b/FlipcashTests/TestSupport/MockConversations.swift @@ -149,6 +149,19 @@ final class MockConversations: ConversationFetching, ConversationMembership, Con get { lock.withLock { _olderMessages } } set { lock.withLock { _olderMessages = newValue } } } + /// When set, older pages are cut from this server history: the newest 50 messages below `before`, + /// like the real paging. Takes precedence over `olderMessages`. + var olderHistory: [ConversationMessage]? { + get { lock.withLock { _olderHistory } } + set { lock.withLock { _olderHistory = newValue } } + } + private var _olderHistory: [ConversationMessage]? + /// When set, older-page `getMessages` throws this instead of returning a page. + var olderError: Error? { + get { lock.withLock { _olderError } } + set { lock.withLock { _olderError = newValue } } + } + private var _olderError: Error? /// The `before` cursors `getMessages` was paged with. var olderQueries: [MessageID] { lock.withLock { _olderQueries } } /// Messages `getMessage` returns by id; an id missing here reads as not found. @@ -432,6 +445,10 @@ final class MockConversations: ConversationFetching, ConversationMembership, Con return messages } lock.withLock { _olderQueries.append(before) } + if let error = olderError { throw error } + if let history = olderHistory { + return Array(history.filter { $0.id.value < before.value }.sorted { $0.id.value < $1.id.value }.suffix(50)) + } return olderMessages }