Commit 8bfc8934a6
Unsigned
Layout: unified · split
Hutch/Networking/MailingListActivity.swift added +135
| @@ -0,0 +1,135 @@ | |||
| 1 | import Foundation | ||
| 2 | |||
| 3 | // MARK: - Response types (file-private to avoid @MainActor Decodable issues) | ||
| 4 | |||
| 5 | private struct ListEmailsResponse: Decodable, Sendable { | ||
| 6 | let list: ListEmailsPayload? | ||
| 7 | } | ||
| 8 | |||
| 9 | private struct ListEmailsPayload: Decodable, Sendable { | ||
| 10 | let emails: ListEmailPage | ||
| 11 | } | ||
| 12 | |||
| 13 | private struct ListEmailPage: Decodable, Sendable { | ||
| 14 | let results: [ListEmailPayload] | ||
| 15 | let cursor: String? | ||
| 16 | } | ||
| 17 | |||
| 18 | private struct ListEmailPayload: Decodable, Sendable { | ||
| 19 | /// When sr.ht received the mail. Unlike `date`, which comes from the sender's | ||
| 20 | /// Date: header and is both nullable and not to be trusted, this is | ||
| 21 | /// server-authoritative. | ||
| 22 | let received: Date | ||
| 23 | let thread: ListEmailThread | ||
| 24 | } | ||
| 25 | |||
| 26 | private struct ListEmailThread: Decodable, Sendable { | ||
| 27 | let root: ListEmailThreadRoot | ||
| 28 | } | ||
| 29 | |||
| 30 | private struct ListEmailThreadRoot: Decodable, Sendable { | ||
| 31 | let id: Int | ||
| 32 | } | ||
| 33 | |||
| 34 | // MARK: - Activity | ||
| 35 | |||
| 36 | /// When each thread on a mailing list last received mail. | ||
| 37 | /// | ||
| 38 | /// `Thread.updated` cannot answer this. Despite its name, and despite the schema | ||
| 39 | /// describing threads as ordered "most recently bumped", it is the root email's | ||
| 40 | /// insert time and never advances when a reply arrives — sr.ht reports `updated` | ||
| 41 | /// seven seconds after `root.date` on a thread carrying four replies. Anything | ||
| 42 | /// built on it silently treats thread creation as activity. | ||
| 43 | /// | ||
| 44 | /// `MailingList.emails` is reverse-chronological arrival data, so it can. | ||
| 45 | struct MailingListActivity: Sendable { | ||
| 46 | private let newestByRootEmailID: [Int: Date] | ||
| 47 | |||
| 48 | init(newestByRootEmailID: [Int: Date] = [:]) { | ||
| 49 | self.newestByRootEmailID = newestByRootEmailID | ||
| 50 | } | ||
| 51 | |||
| 52 | /// The newest arrival in the thread rooted at `rootEmailID`. | ||
| 53 | /// | ||
| 54 | /// Falls back to `fallback` for threads with nothing inside the scanned | ||
| 55 | /// window, which are by definition older than the cutoff and therefore read. | ||
| 56 | func lastActivity(rootEmailID: Int, fallback: Date) -> Date { | ||
| 57 | guard let newest = newestByRootEmailID[rootEmailID] else { return fallback } | ||
| 58 | return max(newest, fallback) | ||
| 59 | } | ||
| 60 | } | ||
| 61 | |||
| 62 | enum MailingListActivityLoader { | ||
| 63 | |||
| 64 | private static let listEmailsQuery = """ | ||
| 65 | query listActivity($rid: ID!, $cursor: Cursor) { | ||
| 66 | list(rid: $rid) { | ||
| 67 | emails(cursor: $cursor) { | ||
| 68 | results { | ||
| 69 | received | ||
| 70 | thread { root { id } } | ||
| 71 | } | ||
| 72 | cursor | ||
| 73 | } | ||
| 74 | } | ||
| 75 | } | ||
| 76 | """ | ||
| 77 | |||
| 78 | /// Scans the list's mail newest-first and stops once it is older than | ||
| 79 | /// `cutoff`, so a quiet list costs a single page and a busy one costs only | ||
| 80 | /// what has arrived since. | ||
| 81 | /// | ||
| 82 | /// `maxPages` bounds the scan. An account carrying pre-existing read state has | ||
| 83 | /// a `distantPast` cutoff, which would otherwise walk the entire archive; | ||
| 84 | /// threads beyond the window keep their fallback date and stay read, which is | ||
| 85 | /// what they already were. | ||
| 86 | /// | ||
| 87 | /// Returns empty activity on failure rather than throwing: unread is a | ||
| 88 | /// decoration, and losing it should not fail the thread list around it. | ||
| 89 | static func load( | ||
| 90 | client: SRHTClient, | ||
| 91 | listRID: String, | ||
| 92 | since cutoff: Date, | ||
| 93 | maxPages: Int = 3 | ||
| 94 | ) async -> MailingListActivity { | ||
| 95 | var newest: [Int: Date] = [:] | ||
| 96 | var cursor: String? | ||
| 97 | var pagesFetched = 0 | ||
| 98 | |||
| 99 | while pagesFetched < maxPages { | ||
| 100 | var variables: [String: any Sendable] = ["rid": listRID] | ||
| 101 | if let cursor { | ||
| 102 | variables["cursor"] = cursor | ||
| 103 | } | ||
| 104 | |||
| 105 | let response: ListEmailsResponse | ||
| 106 | do { | ||
| 107 | response = try await client.execute( | ||
| 108 | service: .lists, | ||
| 109 | query: listEmailsQuery, | ||
| 110 | variables: variables, | ||
| 111 | responseType: ListEmailsResponse.self | ||
| 112 | ) | ||
| 113 | } catch { | ||
| 114 | return MailingListActivity(newestByRootEmailID: newest) | ||
| 115 | } | ||
| 116 | |||
| 117 | guard let page = response.list?.emails else { break } | ||
| 118 | pagesFetched += 1 | ||
| 119 | |||
| 120 | for email in page.results { | ||
| 121 | let rootID = email.thread.root.id | ||
| 122 | if let existing = newest[rootID], existing >= email.received { continue } | ||
| 123 | newest[rootID] = email.received | ||
| 124 | } | ||
| 125 | |||
| 126 | // Reverse chronological, so once a page ends older than the cutoff | ||
| 127 | // nothing further back can matter. | ||
| 128 | if let oldest = page.results.map(\.received).min(), oldest <= cutoff { break } | ||
| 129 | guard let next = page.cursor, !next.isEmpty else { break } | ||
| 130 | cursor = next | ||
| 131 | } | ||
| 132 | |||
| 133 | return MailingListActivity(newestByRootEmailID: newest) | ||
| 134 | } | ||
| 135 | } | ||
Hutch/Views/Home/HomeViewModel.swift +11 −2
| @@ -819,6 +819,14 @@ final class HomeViewModel { | |||
| 819 | var cursor: String? | 819 | var cursor: String? |
| 820 | var unreadThreads: [InboxThreadSummary] = [] | 820 | var unreadThreads: [InboxThreadSummary] = [] |
| 821 | 821 | ||
| 822 | // thread.updated is the root email's insert time and never advances when a | ||
| 823 | // reply lands, so activity has to come from the list's mail feed. | ||
| 824 | let activity = await MailingListActivityLoader.load( | ||
| 825 | client: client, | ||
| 826 | listRID: mailingList.rid, | ||
| 827 | since: InboxReadStateStore.baseline(defaults: defaults) ?? .distantPast | ||
| 828 | ) | ||
| 829 | |||
| 822 | while true { | 830 | while true { |
| 823 | var variables: [String: any Sendable] = ["rid": mailingList.rid] | 831 | var variables: [String: any Sendable] = ["rid": mailingList.rid] |
| 824 | if let cursor { | 832 | if let cursor { |
| @@ -838,6 +846,7 @@ final class HomeViewModel { | |||
| 838 | let response = cached.value | 846 | let response = cached.value |
| 839 | 847 | ||
| 840 | let unreadThreadSummaries = response.list.threads.results.compactMap { thread -> InboxThreadSummary? in | 848 | let unreadThreadSummaries = response.list.threads.results.compactMap { thread -> InboxThreadSummary? in |
| 849 | let lastActivityAt = activity.lastActivity(rootEmailID: thread.root.id, fallback: thread.updated) | ||
| 841 | let summary = InboxThreadSummary( | 850 | let summary = InboxThreadSummary( |
| 842 | rootEmailID: thread.root.id, | 851 | rootEmailID: thread.root.id, |
| 843 | rootMessageID: thread.root.messageID, | 852 | rootMessageID: thread.root.messageID, |
| @@ -849,13 +858,13 @@ final class HomeViewModel { | |||
| 849 | listOwner: mailingList.owner, | 858 | listOwner: mailingList.owner, |
| 850 | subject: thread.subject, | 859 | subject: thread.subject, |
| 851 | latestSender: thread.sender, | 860 | latestSender: thread.sender, |
| 852 | lastActivityAt: thread.updated, | 861 | lastActivityAt: lastActivityAt, |
| 853 | messageCount: thread.replies + 1, | 862 | messageCount: thread.replies + 1, |
| 854 | repo: InboxThreadUtilities.deriveRepositoryName(from: mailingList.name), | 863 | repo: InboxThreadUtilities.deriveRepositoryName(from: mailingList.name), |
| 855 | containsPatch: thread.root.patch != nil || thread.subject.localizedCaseInsensitiveContains("[patch"), | 864 | containsPatch: thread.root.patch != nil || thread.subject.localizedCaseInsensitiveContains("[patch"), |
| 856 | isUnread: InboxReadStateStore.isUnread( | 865 | isUnread: InboxReadStateStore.isUnread( |
| 857 | threadID: "\(mailingList.rid)#\(InboxThreadSummary.normalizationKey(for: thread.subject))", | 866 | threadID: "\(mailingList.rid)#\(InboxThreadSummary.normalizationKey(for: thread.subject))", |
| 858 | lastActivityAt: thread.updated, | 867 | lastActivityAt: lastActivityAt, |
| 859 | defaults: defaults | 868 | defaults: defaults |
| 860 | ) | 869 | ) |
| 861 | ) | 870 | ) |
Hutch/Views/Projects/ProjectMailingListView.swift +17 −4
| @@ -84,8 +84,14 @@ final class MailingListDetailViewModel { | |||
| 84 | responseType: ProjectMailingListThreadsResponse.self | 84 | responseType: ProjectMailingListThreadsResponse.self |
| 85 | ) | 85 | ) |
| 86 | 86 | ||
| 87 | let activity = await MailingListActivityLoader.load( | ||
| 88 | client: client, | ||
| 89 | listRID: mailingList.rid, | ||
| 90 | since: InboxReadStateStore.baseline(defaults: defaults) ?? .distantPast | ||
| 91 | ) | ||
| 92 | |||
| 87 | threads = deduplicateThreads( | 93 | threads = deduplicateThreads( |
| 88 | response.list.threads.results.map(makeSummary(from:)) | 94 | response.list.threads.results.map { makeSummary(from: $0, activity: activity) } |
| 89 | ) | 95 | ) |
| 90 | } catch { | 96 | } catch { |
| 91 | self.error = "Failed to load mailing list" | 97 | self.error = "Failed to load mailing list" |
| @@ -138,7 +144,10 @@ final class MailingListDetailViewModel { | |||
| 138 | NeedsAttentionSnapshotStore.adjustUnreadInboxThreads(by: -unreadThreads.count, accountID: accountID) | 144 | NeedsAttentionSnapshotStore.adjustUnreadInboxThreads(by: -unreadThreads.count, accountID: accountID) |
| 139 | } | 145 | } |
| 140 | 146 | ||
| 141 | private func makeSummary(from thread: ProjectMailingListThreadPayload) -> InboxThreadSummary { | 147 | private func makeSummary( |
| 148 | from thread: ProjectMailingListThreadPayload, | ||
| 149 | activity: MailingListActivity | ||
| 150 | ) -> InboxThreadSummary { | ||
| 142 | let normalizedSubject = thread.subject | 151 | let normalizedSubject = thread.subject |
| 143 | .replacingOccurrences(of: #"\s+"#, with: " ", options: .regularExpression) | 152 | .replacingOccurrences(of: #"\s+"#, with: " ", options: .regularExpression) |
| 144 | .trimmingCharacters(in: .whitespacesAndNewlines) | 153 | .trimmingCharacters(in: .whitespacesAndNewlines) |
| @@ -146,6 +155,10 @@ final class MailingListDetailViewModel { | |||
| 146 | .lowercased() | 155 | .lowercased() |
| 147 | let threadID = "\(mailingList.rid)#\(normalizedSubject)" | 156 | let threadID = "\(mailingList.rid)#\(normalizedSubject)" |
| 148 | 157 | ||
| 158 | // thread.updated is the root email's insert time and never advances when a | ||
| 159 | // reply lands, so activity has to come from the list's mail feed. | ||
| 160 | let lastActivityAt = activity.lastActivity(rootEmailID: thread.root.id, fallback: thread.updated) | ||
| 161 | |||
| 149 | return InboxThreadSummary( | 162 | return InboxThreadSummary( |
| 150 | rootEmailID: thread.root.id, | 163 | rootEmailID: thread.root.id, |
| 151 | rootMessageID: thread.root.messageID, | 164 | rootMessageID: thread.root.messageID, |
| @@ -157,11 +170,11 @@ final class MailingListDetailViewModel { | |||
| 157 | listOwner: mailingList.owner, | 170 | listOwner: mailingList.owner, |
| 158 | subject: thread.subject, | 171 | subject: thread.subject, |
| 159 | latestSender: thread.sender, | 172 | latestSender: thread.sender, |
| 160 | lastActivityAt: thread.updated, | 173 | lastActivityAt: lastActivityAt, |
| 161 | messageCount: thread.replies + 1, | 174 | messageCount: thread.replies + 1, |
| 162 | repo: nil, | 175 | repo: nil, |
| 163 | containsPatch: thread.root.patch != nil || thread.subject.localizedCaseInsensitiveContains("[patch"), | 176 | containsPatch: thread.root.patch != nil || thread.subject.localizedCaseInsensitiveContains("[patch"), |
| 164 | isUnread: InboxReadStateStore.isUnread(threadID: threadID, lastActivityAt: thread.updated, defaults: defaults) | 177 | isUnread: InboxReadStateStore.isUnread(threadID: threadID, lastActivityAt: lastActivityAt, defaults: defaults) |
| 165 | ) | 178 | ) |
| 166 | } | 179 | } |
| 167 | 180 | ||
HutchTests/MailingListActivityTests.swift added +77
| @@ -0,0 +1,77 @@ | |||
| 1 | import Foundation | ||
| 2 | import Testing | ||
| 3 | @testable import Hutch | ||
| 4 | |||
| 5 | struct MailingListActivityTests { | ||
| 6 | |||
| 7 | @Test | ||
| 8 | func usesTheNewestArrivalOverTheRootTimestamp() { | ||
| 9 | // The case that started this: sr.ht reports thread.updated seven seconds | ||
| 10 | // after the root email on a thread carrying four replies, so the fallback | ||
| 11 | // must lose to real arrival data. | ||
| 12 | let rootInsert = Date(timeIntervalSince1970: 1_000) | ||
| 13 | let newestReply = Date(timeIntervalSince1970: 5_000) | ||
| 14 | let activity = MailingListActivity(newestByRootEmailID: [42: newestReply]) | ||
| 15 | |||
| 16 | #expect(activity.lastActivity(rootEmailID: 42, fallback: rootInsert) == newestReply) | ||
| 17 | } | ||
| 18 | |||
| 19 | @Test | ||
| 20 | func fallsBackForThreadsOutsideTheScannedWindow() { | ||
| 21 | // Threads with nothing new are absent from the feed scan; they keep the | ||
| 22 | // root timestamp, which is older than any cutoff and so reads as read. | ||
| 23 | let rootInsert = Date(timeIntervalSince1970: 1_000) | ||
| 24 | let activity = MailingListActivity(newestByRootEmailID: [:]) | ||
| 25 | |||
| 26 | #expect(activity.lastActivity(rootEmailID: 42, fallback: rootInsert) == rootInsert) | ||
| 27 | } | ||
| 28 | |||
| 29 | @Test | ||
| 30 | func neverGoesBackwardsFromTheFallback() { | ||
| 31 | // A root inserted after the newest scanned reply must not age the thread | ||
| 32 | // backwards. | ||
| 33 | let rootInsert = Date(timeIntervalSince1970: 9_000) | ||
| 34 | let staleReply = Date(timeIntervalSince1970: 5_000) | ||
| 35 | let activity = MailingListActivity(newestByRootEmailID: [42: staleReply]) | ||
| 36 | |||
| 37 | #expect(activity.lastActivity(rootEmailID: 42, fallback: rootInsert) == rootInsert) | ||
| 38 | } | ||
| 39 | |||
| 40 | @Test | ||
| 41 | func tracksThreadsIndependently() { | ||
| 42 | let activity = MailingListActivity(newestByRootEmailID: [ | ||
| 43 | 1: Date(timeIntervalSince1970: 5_000), | ||
| 44 | 2: Date(timeIntervalSince1970: 7_000) | ||
| 45 | ]) | ||
| 46 | let fallback = Date(timeIntervalSince1970: 1_000) | ||
| 47 | |||
| 48 | #expect(activity.lastActivity(rootEmailID: 1, fallback: fallback) == Date(timeIntervalSince1970: 5_000)) | ||
| 49 | #expect(activity.lastActivity(rootEmailID: 2, fallback: fallback) == Date(timeIntervalSince1970: 7_000)) | ||
| 50 | #expect(activity.lastActivity(rootEmailID: 3, fallback: fallback) == fallback) | ||
| 51 | } | ||
| 52 | |||
| 53 | @Test | ||
| 54 | func newMailInAnOldThreadReadsAsUnread() { | ||
| 55 | let suiteName = "MailingListActivityTests-\(UUID().uuidString)" | ||
| 56 | let defaults = UserDefaults(suiteName: suiteName)! | ||
| 57 | defer { defaults.removePersistentDomain(forName: suiteName) } | ||
| 58 | |||
| 59 | let signIn = Date(timeIntervalSince1970: 5_000) | ||
| 60 | InboxReadStateStore.establishBaselineIfNeeded(now: signIn, defaults: defaults) | ||
| 61 | |||
| 62 | // A thread rooted long before sign-in, with a reply after it. Keyed on | ||
| 63 | // thread.updated this reads as read, which was the bug. | ||
| 64 | let rootInsert = Date(timeIntervalSince1970: 1_000) | ||
| 65 | let replyAfterSignIn = Date(timeIntervalSince1970: 6_000) | ||
| 66 | let activity = MailingListActivity(newestByRootEmailID: [42: replyAfterSignIn]) | ||
| 67 | let lastActivityAt = activity.lastActivity(rootEmailID: 42, fallback: rootInsert) | ||
| 68 | |||
| 69 | #expect( | ||
| 70 | InboxReadStateStore.isUnread( | ||
| 71 | threadID: "list#old thread", | ||
| 72 | lastActivityAt: lastActivityAt, | ||
| 73 | defaults: defaults | ||
| 74 | ) | ||
| 75 | ) | ||
| 76 | } | ||
| 77 | } | ||