Skip to content

Commit 7174dc2

Browse files
psjostromclaude
andauthored
fix: declare expiryClearJobs before pauseExpiryFlows in AlertManager (#221)
AlertManager constructor crashed with NullPointerException on cold start whenever an alert pause was already in prefs. Took StrimmaService down with it on every reboot, install, or memory-pressure rebind — and ActivityManager backs off restarts up to 1 hour, effectively killing CGM coverage until the user re-launches the app. Root cause: `pauseExpiryFlows`'s initializer calls `scheduleExpiryClear` for any active pause, and `scheduleExpiryClear` touches `expiryClearJobs` — but `expiryClearJobs` was declared *after* `pauseExpiryFlows`. Kotlin's top-down property init meant `expiryClearJobs` was null at the moment of first use. Fix: move `expiryClearJobs` declaration above `pauseExpiryFlows`, and add a construction-time regression test (`AlertManagerInitOrderTest`) that pre-populates an active pause before constructing the manager. The existing `AlertManagerTest` @before clears prefs *after* construction, which is why the regression slipped past unit tests. Introduced in #216 (Pause icon fix + configurable notification action, 2026-05-10). Shipped in 1.3.0-rc.1. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 758a720 commit 7174dc2

2 files changed

Lines changed: 76 additions & 2 deletions

File tree

app/src/main/java/com/psjostrom/strimma/notification/AlertManager.kt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,14 @@ class AlertManager @Inject constructor(
553553
private fun alertCategoryAndLevel(alertId: Int): Pair<AlertCategory, Int>? =
554554
alertIdToCategoryLevel[alertId]
555555

556+
// Declared before pauseExpiryFlows because the flow initializer below calls
557+
// scheduleExpiryClear for any active pause, and scheduleExpiryClear touches
558+
// expiryClearJobs. Kotlin property init runs top-down — flipping the order
559+
// crashes AlertManager construction (NPE on a null ConcurrentHashMap) on any
560+
// cold start where a pause is already in prefs, taking the foreground service
561+
// and CGM coverage down with it.
562+
private val expiryClearJobs = ConcurrentHashMap<AlertCategory, Job>()
563+
556564
// One MutableStateFlow per category, keyed by the enum so iteration over
557565
// AlertCategory.entries can never silently bypass a category. The flow's value
558566
// is the source of truth for "is this category currently paused" — a delayed
@@ -569,8 +577,6 @@ class AlertManager @Inject constructor(
569577
val pauseLowExpiryMs: StateFlow<Long?> = pauseExpiryFlows.getValue(AlertCategory.LOW)
570578
val pauseHighExpiryMs: StateFlow<Long?> = pauseExpiryFlows.getValue(AlertCategory.HIGH)
571579

572-
private val expiryClearJobs = ConcurrentHashMap<AlertCategory, Job>()
573-
574580
fun pauseAlertCategory(category: AlertCategory, durationMs: Long, level: Int = ALERT_LEVEL_URGENT) {
575581
pauseAlertCategoryAt(category, System.currentTimeMillis() + durationMs, level)
576582
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package com.psjostrom.strimma.notification
2+
3+
import android.content.Context
4+
import androidx.test.core.app.ApplicationProvider
5+
import com.psjostrom.strimma.createTestDataStore
6+
import com.psjostrom.strimma.data.SettingsRepository
7+
import com.psjostrom.strimma.data.workout.WorkoutModeManager
8+
import com.psjostrom.strimma.testutil.workout.FakeCalendarPoller
9+
import com.psjostrom.strimma.testutil.workout.MutableClock
10+
import com.psjostrom.strimma.widget.WidgetSettingsRepository
11+
import kotlinx.coroutines.CoroutineScope
12+
import kotlinx.coroutines.Dispatchers
13+
import kotlinx.coroutines.SupervisorJob
14+
import kotlinx.coroutines.cancel
15+
import org.junit.After
16+
import org.junit.Test
17+
import org.junit.runner.RunWith
18+
import org.robolectric.RobolectricTestRunner
19+
20+
/**
21+
* Regression test for the AlertManager construction crash that took CGM coverage
22+
* down on every cold start when a pause was already in prefs.
23+
*
24+
* Root cause: pauseExpiryFlows' initializer called scheduleExpiryClear for any
25+
* active pause, and scheduleExpiryClear touches expiryClearJobs — but
26+
* expiryClearJobs was declared *after* pauseExpiryFlows, so Kotlin's top-down
27+
* property init left it null at the moment of use → NPE → StrimmaService.onCreate
28+
* fails → ActivityManager backs off restarts up to an hour.
29+
*
30+
* The default AlertManagerTest @Before constructs the instance with an empty
31+
* snooze prefs, which is why the regression never tripped a unit test. This test
32+
* pre-populates an active pause *before* construction.
33+
*/
34+
@RunWith(RobolectricTestRunner::class)
35+
class AlertManagerInitOrderTest {
36+
37+
private val managerScope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined)
38+
39+
@After
40+
fun tearDown() {
41+
managerScope.cancel()
42+
}
43+
44+
@Test
45+
fun `construction succeeds when an active pause is already in prefs`() {
46+
val context = ApplicationProvider.getApplicationContext<Context>()
47+
// Write an active pause directly to the snooze prefs that the production
48+
// AlertManager reads from in alertPauseExpiryMs (-> pauseExpiryMs).
49+
// An hour out, well in the future, so scheduleExpiryClear will treat it
50+
// as live and try to register an expiry job.
51+
context.getSharedPreferences("strimma_snooze", Context.MODE_PRIVATE)
52+
.edit()
53+
.putLong("pause_low", System.currentTimeMillis() + 60 * 60 * 1000L)
54+
.commit()
55+
56+
val widgetSettings = WidgetSettingsRepository(context)
57+
val settings = SettingsRepository(context, widgetSettings, createTestDataStore())
58+
val workoutModeManager = WorkoutModeManager(
59+
settings,
60+
FakeCalendarPoller(),
61+
MutableClock(System.currentTimeMillis()),
62+
managerScope,
63+
)
64+
65+
// Will throw NullPointerException with the regressed declaration order.
66+
AlertManager(context, settings, workoutModeManager, managerScope)
67+
}
68+
}

0 commit comments

Comments
 (0)