diff --git a/SunHat/Services/Trigger/TriggerEngineManager.swift b/SunHat/Services/Trigger/TriggerEngineManager.swift index bc92cc1..7ccc74b 100644 --- a/SunHat/Services/Trigger/TriggerEngineManager.swift +++ b/SunHat/Services/Trigger/TriggerEngineManager.swift @@ -167,19 +167,14 @@ final class TriggerEngineManager: ObservableObject { private func handleBackgroundEvaluation(_ task: BGAppRefreshTask) async { logger.info("Starting background trigger evaluation") - guard let triggerEngine = triggerEngine else { - logger.error("TriggerEngine not configured for background evaluation") - task.setTaskCompleted(success: false) - return - } - - let startTime = Date() - - let workTask = Task { () -> [TriggerEvaluationResult] in - let results = await triggerEngine.evaluateAllActiveReminders() - guard !Task.isCancelled else { return [] } - await processEvaluationResults(results, isBackground: true) - return results + // Routed through the same guarded entry point as every other + // caller, so this can never run concurrently with a foreground or + // weather-refresh-triggered evaluation and double-count against + // the daily notification cap. evaluateAllReminders already does + // all the bookkeeping (isEvaluating, lastEvaluationTime, + // evaluationResults, performance metrics) this used to duplicate. + let workTask = Task { + await evaluateAllReminders(isBackground: true) } task.expirationHandler = { @@ -187,14 +182,7 @@ final class TriggerEngineManager: ObservableObject { workTask.cancel() } - let results = await workTask.value - let duration = Date().timeIntervalSince(startTime) - - lastEvaluationTime = Date() - evaluationResults = results - updatePerformanceMetrics(duration: duration) - - logger.info("Background evaluation completed: \(results.count) reminders in \(duration)s") + await workTask.value scheduleBackgroundEvaluation() task.setTaskCompleted(success: !workTask.isCancelled) } diff --git a/SunHat/ViewModels/DashboardViewModel.swift b/SunHat/ViewModels/DashboardViewModel.swift index fddc2e4..4597f13 100644 --- a/SunHat/ViewModels/DashboardViewModel.swift +++ b/SunHat/ViewModels/DashboardViewModel.swift @@ -162,6 +162,14 @@ final class DashboardViewModel: ObservableObject { logger.info("Weather data refresh completed successfully") + // Fresh weather may satisfy a reminder's condition. Evaluation + // fetches weather per reminder location (not just this + // dashboard's own location), so it runs as an independent task + // rather than blocking this refresh's own isLoading state. + Task { + await TriggerEngineManager.shared.evaluateAllReminders() + } + } catch is CancellationError { // A superseded refresh is not an error; leave current state as-is. isLoading = false diff --git a/SunHatTests/TriggerEngineManagerTests.swift b/SunHatTests/TriggerEngineManagerTests.swift index a3596c3..9a1806e 100644 --- a/SunHatTests/TriggerEngineManagerTests.swift +++ b/SunHatTests/TriggerEngineManagerTests.swift @@ -104,6 +104,61 @@ struct TriggerEngineManagerTests { #expect(persisted?.totalNotificationsSent == 1) } + @Test("The daily notification cap is enforced across different reminders in one evaluation cycle") + func dailyCapSuppressesLaterRemindersInSameCycle() async throws { + let container = try makeContainer() + let context = ModelContext(container) + + let conditionA = TriggerCondition() + let reminderA = WeatherReminder(title: "Water plants", triggerCondition: conditionA) + let conditionB = TriggerCondition() + let reminderB = WeatherReminder(title: "Walk the dog", triggerCondition: conditionB) + + let preferences = UserPreferences() + preferences.maximumDailyNotifications = 1 + preferences.dailyNotificationCount = 0 + // Avoid the test's outcome depending on the wall-clock time or day + // of week it happens to run on; only the daily cap is under test. + preferences.quietHoursEnabled = false + preferences.allowWeekendNotifications = true + + context.insert(reminderA) + context.insert(reminderB) + context.insert(preferences) + try context.save() + + let sender = RecordingTriggerNotificationSender() + let manager = TriggerEngineManager( + modelContainer: container, + registerBackgroundTask: false, + notificationManager: sender + ) + let resultA = TriggerEvaluationResult( + reminderId: reminderA.id, + conditionData: ModelDataConverter.convertTriggerCondition(conditionA), + triggered: true, + triggerReason: "Condition met" + ) + let resultB = TriggerEvaluationResult( + reminderId: reminderB.id, + conditionData: ModelDataConverter.convertTriggerCondition(conditionB), + triggered: true, + triggerReason: "Condition met" + ) + + // processEvaluationResults processes its results sequentially (a plain + // for loop, no concurrent dispatch), so within a single evaluation + // cycle the cap check for reminderB always sees reminderA's already- + // recorded delivery, regardless of the results' relative ordering. + await manager.processEvaluationResults([resultA, resultB]) + + #expect(await sender.deliveryCount == 1) + #expect(manager.triggeredReminders == [reminderA.id]) + + let refreshedPreferences = try Self.fetchPreferences(in: container) + #expect(refreshedPreferences?.dailyNotificationCount == 1) + } + @Test("A failed delivery remains eligible for a later retry") func failedDeliveryCanRetry() async throws { let container = try makeContainer() @@ -145,6 +200,13 @@ struct TriggerEngineManagerTests { return try context.fetch(descriptor).first } + /// Reads UserPreferences back through a fresh context, so assertions see + /// persisted state rather than the stale instance the test inserted. + private static func fetchPreferences(in container: ModelContainer) throws -> UserPreferences? { + let context = ModelContext(container) + return try context.fetch(FetchDescriptor()).first + } + private func makeContainer() throws -> ModelContainer { let configuration = ModelConfiguration( schema: SunHatModelSchema.schema, @@ -157,6 +219,19 @@ struct TriggerEngineManagerTests { } } +private actor RecordingTriggerNotificationSender: TriggerNotificationSending { + private(set) var deliveryCount = 0 + + func configure(modelContainer: ModelContainer?) async {} + + func sendTriggerNotification( + for result: TriggerEvaluationResult, + isBackground: Bool + ) async throws { + deliveryCount += 1 + } +} + private actor FailOnceTriggerNotificationSender: TriggerNotificationSending { private(set) var deliveryCount = 0