Skip to content

Commit 9fbfdac

Browse files
committed
Replace per-conversation expiration timers with single next-expiration timer (#509)
## Summary Replaces the per-conversation `Task.sleep` timers added in #475 with a proper single-timer architecture in `ExpiredConversationsWorker`. ## Problem PR #475 added per-conversation `Task.sleep` tasks in `ScheduledExplosionManager` to detect when conversations expire while the app is foregrounded. This approach had several issues: 1. **Wrong responsibility** — `ScheduledExplosionManager` handles local push notifications. Expiration cleanup belongs in `ExpiredConversationsWorker`. 2. **Per-conversation tasks don't scale** — 10 scheduled explosions = 10 sleeping tasks. 3. **Duplicates existing logic** — `ExpiredConversationsWorker` already knows how to query and clean up expired conversations, it just needed a trigger. ## Solution `ExpiredConversationsWorker` now maintains a **single timer** for the next expiring conversation: 1. After processing expired conversations, queries DB for the soonest future `expiresAt` 2. Schedules one `Task.sleep` for that duration 3. When the timer fires, processes all newly-expired conversations and reschedules The timer is cancelled and rescheduled whenever: - A new explosion is scheduled (`.conversationScheduledExplosion`) - The app becomes active (`didBecomeActive`) - A conversation expires (`.conversationExpired`) ## Changes - **`ScheduledExplosionManager`**: Removed all expiration task logic (`_expirationTasks`, `scheduleExpirationTask`, `postConversationExpired`). Now only handles local push notification scheduling. - **`ExpiredConversationsWorker`**: Added single `nextExpirationTask` timer with `fetchNextExpiration()` DB query. Now also observes `.conversationScheduledExplosion`. - **Tests**: Moved expiration tests to new `ExpiredConversationsWorkerTests`. All existing `ScheduledExplosionManager` notification tests still pass. Stacks on #475. <!-- Macroscope's pull request summary starts here --> <!-- Macroscope will only edit the content between these invisible markers, and the markers themselves will not be visible in the GitHub rendered markdown. --> <!-- If you delete either of the start / end markers from your PR's description, Macroscope will append its summary at the bottom of the description. --> > [!NOTE] > ### Replace per-conversation expiration timers with a single buffered next-expiration Task in `ExpiredConversationsWorker.scheduleNextExpirationCheck` with a 0.5s wake buffer > Move expiration timing from `ScheduledExplosionManager` to `ExpiredConversationsWorker`, add `checkAndReschedule` and a single `nextExpirationTask`, and gate progress indicator completion animation via `HoldToConfirmStyleConfig.poofProgressIndicatorOnComplete`. > > #### 📍Where to Start > Start with `ExpiredConversationsWorker.checkAndReschedule` in [ExpiredConversationsWorker.swift](https://github.com/xmtplabs/convos-ios/pull/509/files#diff-0fe27ce7c3c36513ccb0460c6a57e998999c277b68bf21f48fbf24d6b935a616). > > <!-- Macroscope's review summary starts here --> > > <sup><a href="https://app.macroscope.com">Macroscope</a> summarized 6fd1648.</sup> > <!-- Macroscope's review summary ends here --> > <!-- macroscope-ui-refresh --> <!-- Macroscope's pull request summary ends here -->
1 parent db88524 commit 9fbfdac

5 files changed

Lines changed: 338 additions & 130 deletions

File tree

Convos/Shared Views/HoldToConfirmButton.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ struct HoldToConfirmStyleConfig {
2525

2626
// Progress indicator
2727
var showProgressIndicator: Bool = true
28+
var poofProgressIndicatorOnComplete: Bool = true
2829

2930
static let `default`: HoldToConfirmStyleConfig = .init()
3031
}
@@ -65,6 +66,10 @@ struct HoldToConfirmPrimitiveStyle: PrimitiveButtonStyle {
6566
@State private var didFire: Bool = false
6667
@State private var frozenProgress: Double = 0
6768

69+
private var shouldPoof: Bool {
70+
config.poofProgressIndicatorOnComplete && didFire
71+
}
72+
6873
var body: some View {
6974
TimelineView(.animation(paused: !isPressing)) { context in
7075
let currentProgress: Double = {
@@ -96,6 +101,10 @@ struct HoldToConfirmPrimitiveStyle: PrimitiveButtonStyle {
96101
.fill(config.progressIndicatorColor)
97102
.scaleEffect(currentProgress)
98103
)
104+
.scaleEffect(shouldPoof ? 2.0 : 1.0)
105+
.blur(radius: shouldPoof ? 8 : 0)
106+
.opacity(shouldPoof ? 0 : 1)
107+
.animation(.easeOut(duration: 0.25), value: shouldPoof)
99108
Spacer()
100109
}
101110
.padding(config.progressIndicatorPadding)

ConvosCore/Sources/ConvosCore/Storage/Workers/ExpiredConversationsWorker.swift

Lines changed: 93 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,22 @@ import GRDB
33

44
public protocol ExpiredConversationsWorkerProtocol {}
55

6+
/// Monitors conversations with scheduled explosions and triggers cleanup when they expire.
7+
///
8+
/// Uses a single timer targeting the next expiring conversation rather than per-conversation
9+
/// timers. After processing expired conversations, queries the database for the soonest
10+
/// future expiresAt and schedules one task to wake at that time.
11+
///
612
/// @unchecked Sendable: Protocol dependencies (SessionManager, DatabaseReader, AppLifecycle)
7-
/// are all Sendable. The `observers` array is marked `nonisolated(unsafe)` and only modified
8-
/// during init (setupObservers) and deinit. NotificationCenter callbacks use weak self
9-
/// and dispatch work to async Tasks.
13+
/// are all Sendable. The `observers` array is only modified during init and deinit.
14+
/// The `nextExpirationTask` is protected by `taskLock`.
1015
final class ExpiredConversationsWorker: ExpiredConversationsWorkerProtocol, @unchecked Sendable {
1116
private let sessionManager: any SessionManagerProtocol
1217
private let databaseReader: any DatabaseReader
1318
private let appLifecycle: any AppLifecycleProviding
1419
nonisolated(unsafe) private var observers: [NSObjectProtocol] = []
20+
private let taskLock: NSLock = NSLock()
21+
private var nextExpirationTask: Task<Void, Never>?
1522

1623
init(
1724
databaseReader: any DatabaseReader,
@@ -22,12 +29,13 @@ final class ExpiredConversationsWorker: ExpiredConversationsWorkerProtocol, @unc
2229
self.sessionManager = sessionManager
2330
self.appLifecycle = appLifecycle
2431
setupObservers()
25-
checkForExpiredConversations()
32+
checkAndReschedule()
2633
}
2734

2835
deinit {
2936
Log.warning("ExpiredConversationsWorker deinit - removing observers")
3037
observers.forEach { NotificationCenter.default.removeObserver($0) }
38+
cancelNextExpirationTask()
3139
}
3240

3341
private func setupObservers() {
@@ -38,7 +46,7 @@ final class ExpiredConversationsWorker: ExpiredConversationsWorkerProtocol, @unc
3846
object: nil,
3947
queue: .main
4048
) { [weak self] _ in
41-
self?.checkForExpiredConversations()
49+
self?.checkAndReschedule()
4250
})
4351

4452
observers.append(center.addObserver(
@@ -50,38 +58,95 @@ final class ExpiredConversationsWorker: ExpiredConversationsWorkerProtocol, @unc
5058
if let conversationId = notification.userInfo?["conversationId"] as? String {
5159
self?.handleExpiredConversation(conversationId: conversationId)
5260
} else {
53-
self?.checkForExpiredConversations()
61+
self?.checkAndReschedule()
5462
}
5563
})
5664

65+
observers.append(center.addObserver(
66+
forName: .conversationScheduledExplosion,
67+
object: nil,
68+
queue: .main
69+
) { [weak self] _ in
70+
self?.checkAndReschedule()
71+
})
72+
5773
observers.append(center.addObserver(
5874
forName: .explosionNotificationTapped,
5975
object: nil,
6076
queue: .main
6177
) { [weak self] _ in
62-
self?.checkForExpiredConversations()
78+
self?.checkAndReschedule()
6379
})
6480
}
6581

66-
private func checkForExpiredConversations() {
82+
private func handleExpiredConversation(conversationId: String) {
6783
Task { [weak self] in
6884
guard let self else { return }
69-
await self.queryAndProcessExpiredConversations()
85+
await self.processExpiredConversationById(conversationId)
86+
await self.scheduleNextExpirationCheck()
7087
}
7188
}
7289

73-
private func handleExpiredConversation(conversationId: String) {
90+
private func checkAndReschedule() {
7491
Task { [weak self] in
7592
guard let self else { return }
76-
await self.processExpiredConversationById(conversationId)
93+
await self.queryAndProcessExpiredConversations()
94+
await self.scheduleNextExpirationCheck()
95+
}
96+
}
97+
98+
private func scheduleNextExpirationCheck() async {
99+
cancelNextExpirationTask()
100+
101+
do {
102+
let nextExpiresAt = try await databaseReader.read { db -> Date? in
103+
try db.fetchNextExpiration()
104+
}
105+
106+
guard let nextExpiresAt else { return }
107+
108+
let interval = nextExpiresAt.timeIntervalSinceNow
109+
guard interval > 0 else {
110+
await queryAndProcessExpiredConversations()
111+
await scheduleNextExpirationCheck()
112+
return
113+
}
114+
115+
let bufferedInterval: Double = interval + Constant.expirationBuffer
116+
Log.info("ExpiredConversationsWorker: next expiration in \(Int(interval))s (sleeping \(bufferedInterval)s)")
117+
118+
let task = Task { [weak self] in
119+
try? await Task.sleep(for: .seconds(bufferedInterval))
120+
guard !Task.isCancelled else { return }
121+
self?.checkAndReschedule()
122+
}
123+
124+
replaceNextExpirationTask(task)
125+
} catch {
126+
Log.error("Failed to query next expiration: \(error)")
77127
}
78128
}
79129

130+
private nonisolated func cancelNextExpirationTask() {
131+
taskLock.lock()
132+
nextExpirationTask?.cancel()
133+
nextExpirationTask = nil
134+
taskLock.unlock()
135+
}
136+
137+
private nonisolated func replaceNextExpirationTask(_ task: Task<Void, Never>) {
138+
taskLock.lock()
139+
nextExpirationTask?.cancel()
140+
nextExpirationTask = task
141+
taskLock.unlock()
142+
}
143+
80144
private func processExpiredConversationById(_ conversationId: String) async {
81145
do {
82146
let conversation = try await databaseReader.read { db -> ExpiredConversation? in
83-
guard let row = try DBConversation.fetchOne(db, key: conversationId) else {
84-
Log.warning("Conversation not found for expiration: \(conversationId)")
147+
guard let row = try DBConversation.fetchOne(db, key: conversationId),
148+
let expiresAt = row.expiresAt,
149+
expiresAt <= Date() else {
85150
return nil
86151
}
87152
return ExpiredConversation(
@@ -104,16 +169,16 @@ final class ExpiredConversationsWorker: ExpiredConversationsWorkerProtocol, @unc
104169
try db.fetchExpiredConversations()
105170
}
106171
guard !expiredConversations.isEmpty else { return }
107-
await processExpiredConversations(expiredConversations)
172+
for conversation in expiredConversations {
173+
await cleanupExpiredConversation(conversation)
174+
}
108175
} catch {
109176
Log.error("Failed to query expired conversations: \(error)")
110177
}
111178
}
112179

113-
private func processExpiredConversations(_ conversations: [ExpiredConversation]) async {
114-
for conversation in conversations {
115-
await cleanupExpiredConversation(conversation)
116-
}
180+
private enum Constant {
181+
static let expirationBuffer: TimeInterval = 0.5
117182
}
118183

119184
private func cleanupExpiredConversation(_ conversation: ExpiredConversation) async {
@@ -139,7 +204,7 @@ struct ExpiredConversation {
139204
let inboxId: String
140205
}
141206

142-
fileprivate extension Database {
207+
private extension Database {
143208
func fetchExpiredConversations() throws -> [ExpiredConversation] {
144209
let rows = try DBConversation
145210
.filter(DBConversation.Columns.expiresAt != nil)
@@ -154,4 +219,13 @@ fileprivate extension Database {
154219
)
155220
}
156221
}
222+
223+
func fetchNextExpiration() throws -> Date? {
224+
try DBConversation
225+
.filter(DBConversation.Columns.expiresAt != nil)
226+
.filter(DBConversation.Columns.expiresAt > Date())
227+
.order(DBConversation.Columns.expiresAt.asc)
228+
.fetchOne(self)?
229+
.expiresAt
230+
}
157231
}

ConvosCore/Sources/ConvosCore/Storage/Workers/ScheduledExplosionManager.swift

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,15 @@ public protocol ScheduledExplosionManagerProtocol {}
66

77
/// @unchecked Sendable: Protocol dependencies (DatabaseReader, AppLifecycle)
88
/// are all Sendable. The `observers` array is only modified during init and deinit.
9-
/// The `schedulingTasks` and `expirationTasks` dictionaries are protected by `taskLock`
10-
/// to prevent data races from concurrent notification callbacks.
9+
/// The `schedulingTasks` dictionary is protected by `taskLock` to prevent data races
10+
/// from concurrent notification callbacks.
1111
final class ScheduledExplosionManager: ScheduledExplosionManagerProtocol, @unchecked Sendable {
1212
private let databaseReader: any DatabaseReader
1313
private let appLifecycle: any AppLifecycleProviding
1414
private let notificationCenter: any UserNotificationCenterProtocol
1515
nonisolated(unsafe) private var observers: [NSObjectProtocol] = []
1616
private let taskLock: NSLock = NSLock()
1717
private var _schedulingTasks: [String: Task<Void, Never>] = [:]
18-
private var _expirationTasks: [String: Task<Void, Never>] = [:]
1918

2019
private enum Constant {
2120
static let reminderIdentifierPrefix: String = "explosion-reminder-"
@@ -85,8 +84,6 @@ final class ScheduledExplosionManager: ScheduledExplosionManagerProtocol, @unche
8584
self.removeSchedulingTask(for: conversationId)
8685
}
8786
taskLock.unlock()
88-
89-
scheduleExpirationTask(conversationId: conversationId, expiresAt: expiresAt)
9087
}
9188

9289
private nonisolated func removeSchedulingTask(for conversationId: String) {
@@ -95,42 +92,6 @@ final class ScheduledExplosionManager: ScheduledExplosionManagerProtocol, @unche
9592
taskLock.unlock()
9693
}
9794

98-
private nonisolated func removeExpirationTask(for conversationId: String) {
99-
taskLock.lock()
100-
_expirationTasks[conversationId] = nil
101-
taskLock.unlock()
102-
}
103-
104-
private func scheduleExpirationTask(conversationId: String, expiresAt: Date) {
105-
taskLock.lock()
106-
_expirationTasks[conversationId]?.cancel()
107-
_expirationTasks[conversationId] = Task { [weak self] in
108-
guard let self else { return }
109-
110-
let interval = expiresAt.timeIntervalSinceNow
111-
guard interval > 0 else {
112-
await self.postConversationExpired(conversationId: conversationId)
113-
self.removeExpirationTask(for: conversationId)
114-
return
115-
}
116-
117-
try? await Task.sleep(for: .seconds(interval))
118-
guard !Task.isCancelled else { return }
119-
120-
await self.postConversationExpired(conversationId: conversationId)
121-
self.removeExpirationTask(for: conversationId)
122-
}
123-
taskLock.unlock()
124-
}
125-
126-
@MainActor
127-
private func postConversationExpired(conversationId: String) {
128-
NotificationCenter.default.post(
129-
name: .conversationExpired,
130-
object: nil,
131-
userInfo: ["conversationId": conversationId]
132-
)
133-
}
13495

13596
func hasSchedulingTask(for conversationId: String) -> Bool {
13697
taskLock.lock()
@@ -182,10 +143,6 @@ final class ScheduledExplosionManager: ScheduledExplosionManagerProtocol, @unche
182143
expiresAt: conversation.expiresAt,
183144
conversationName: conversation.name
184145
)
185-
scheduleExpirationTask(
186-
conversationId: conversation.conversationId,
187-
expiresAt: conversation.expiresAt
188-
)
189146
}
190147
} catch {
191148
Log.error("Failed to query scheduled explosions: \(error)")
@@ -199,7 +156,6 @@ final class ScheduledExplosionManager: ScheduledExplosionManagerProtocol, @unche
199156
) async {
200157
let reminderDate = expiresAt.addingTimeInterval(-Constant.oneHourInSeconds)
201158

202-
// Capture interval BEFORE any async operations to avoid time drift
203159
let reminderInterval = reminderDate.timeIntervalSinceNow
204160
guard reminderInterval > 0 else {
205161
Log.info("ScheduledExplosionManager: Skipping reminder for \(conversationId), less than 1 hour away")
@@ -297,18 +253,14 @@ final class ScheduledExplosionManager: ScheduledExplosionManagerProtocol, @unche
297253
private func cancelAllTasks() {
298254
taskLock.lock()
299255
_schedulingTasks.values.forEach { $0.cancel() }
300-
_expirationTasks.values.forEach { $0.cancel() }
301256
_schedulingTasks.removeAll()
302-
_expirationTasks.removeAll()
303257
taskLock.unlock()
304258
}
305259

306260
private func cancelNotifications(for conversationId: String) {
307261
taskLock.lock()
308262
_schedulingTasks[conversationId]?.cancel()
309263
_schedulingTasks[conversationId] = nil
310-
_expirationTasks[conversationId]?.cancel()
311-
_expirationTasks[conversationId] = nil
312264
taskLock.unlock()
313265
let reminderIdentifier = "\(Constant.reminderIdentifierPrefix)\(conversationId)"
314266
let explosionIdentifier = "\(Constant.explosionIdentifierPrefix)\(conversationId)"

0 commit comments

Comments
 (0)