Skip to content

Commit 6134c16

Browse files
andre487andre487
authored
fix: move storage and system queries off the UI thread (#33)
* fix: move storage and system queries off the UI thread * test: drain the main looper while awaiting discovered apps
1 parent 9b4ba34 commit 6134c16

30 files changed

Lines changed: 576 additions & 194 deletions

‎app/build.gradle.kts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ dependencies {
101101
testImplementation("org.robolectric:robolectric:4.16")
102102
testImplementation("androidx.compose.ui:ui-test-junit4")
103103
debugImplementation("androidx.compose.ui:ui-test-manifest")
104+
androidTestImplementation(platform("androidx.compose:compose-bom:2025.01.01"))
105+
androidTestImplementation("androidx.compose.ui:ui-test-junit4")
104106
androidTestImplementation("androidx.test:runner:1.6.2")
105107
androidTestImplementation("androidx.test.ext:junit:1.2.1")
106108
androidTestImplementation("androidx.test:core:1.6.1")

‎app/src/androidTest/java/life/andre/message487/DeviceTestRunner.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ import androidx.test.runner.AndroidJUnitRunner
66

77
// Keep production workers and crash handlers out of isolated storage/transport tests.
88
class DeviceTestApplication : MessageApplication() {
9+
override val payloadCipher: PayloadCipher = object : PayloadCipher {
10+
private val delegate = KeystorePayloadCipher()
11+
private fun checkThread() = check(android.os.Looper.myLooper() != android.os.Looper.getMainLooper()) {
12+
"Keystore must not run on the UI thread"
13+
}
14+
override fun encrypt(value: String): ByteArray { checkThread(); return delegate.encrypt(value) }
15+
override fun decrypt(value: ByteArray): String { checkThread(); return delegate.decrypt(value) }
16+
}
917
override fun onCreate() {}
1018
}
1119

‎app/src/androidTest/java/life/andre/message487/StorageDeliveryDeviceTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class StorageDeliveryDeviceTest {
3737
encrypted[encrypted.lastIndex] = (encrypted.last().toInt() xor 1).toByte()
3838
assertThrows(Exception::class.java) { cipher.decrypt(encrypted) }
3939
SettingsStore(context).update { it.copy(authToken = token, url = "https://example.test/hook") }
40-
val reopened = SettingsStore(context).state.value
40+
val reopened = SettingsStore(context).load()
4141
assertEquals(token, reopened.authToken)
4242
assertTrue(reopened.ready())
4343
assertFalse(context.getSharedPreferences("connection", Context.MODE_PRIVATE).all.toString().contains(token))

‎app/src/androidTest/java/life/andre/message487/UiThreadDeviceTest.kt‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package life.andre.message487
2+
3+
import android.os.StrictMode
4+
import androidx.activity.ComponentActivity
5+
import androidx.compose.ui.test.*
6+
import androidx.compose.ui.test.junit4.createAndroidComposeRule
7+
import androidx.test.ext.junit.runners.AndroidJUnit4
8+
import androidx.test.platform.app.InstrumentationRegistry
9+
import org.junit.After
10+
import org.junit.Assert.*
11+
import org.junit.Rule
12+
import org.junit.Test
13+
import org.junit.runner.RunWith
14+
15+
/** Real Android disk/network detection, including first construction of the UI models. */
16+
@RunWith(AndroidJUnit4::class)
17+
class UiThreadDeviceTest {
18+
@get:Rule val compose = createAndroidComposeRule<ComponentActivity>()
19+
private var previousPolicy: StrictMode.ThreadPolicy? = null
20+
private val app get() = compose.activity.application as MessageApplication
21+
private lateinit var model: ConnectionViewModel
22+
23+
private fun node(id: Int) = compose.onNodeWithText(app.getString(id))
24+
private fun icon(id: Int) = compose.onNodeWithContentDescription(app.getString(id))
25+
26+
private fun show() {
27+
// This test never sends a message or changes the user's emulator: run on disposable devices.
28+
app.getSharedPreferences("connection", 0).edit().clear().commit()
29+
app.getSharedPreferences("updates", 0).edit().clear().commit()
30+
// Exercise real persisted-token decryption when the ViewModel starts.
31+
SettingsStore(app).update { it.copy(url = "https://example.test/hook", authToken = "synthetic-ui-token") }
32+
compose.runOnUiThread {
33+
previousPolicy = StrictMode.getThreadPolicy()
34+
StrictMode.setThreadPolicy(StrictMode.ThreadPolicy.Builder()
35+
.detectDiskReads().detectDiskWrites().detectNetwork().penaltyDeath().build())
36+
model = androidx.lifecycle.ViewModelProvider(compose.activity)[ConnectionViewModel::class.java]
37+
}
38+
compose.setContent { MessageTheme {
39+
MessageScreen(model)
40+
UpdateAvailableDialog(compose.activity, 0) {}
41+
} }
42+
compose.waitUntil(10_000) { model.settingsLoaded.value && !model.state.value.busy }
43+
}
44+
45+
@After fun restore() {
46+
InstrumentationRegistry.getInstrumentation().runOnMainSync {
47+
previousPolicy?.let(StrictMode::setThreadPolicy)
48+
}
49+
}
50+
51+
@Test fun navigateEditAndRefreshWithoutMainThreadIo() {
52+
show()
53+
node(R.string.connection_nav).performClick()
54+
node(R.string.webhook_url).assertIsDisplayed()
55+
icon(R.string.back).performClick()
56+
compose.onNode(hasText(app.getString(R.string.sources_tab)) and hasClickAction()).performClick()
57+
compose.onNode(hasScrollToNodeAction()).performScrollToNode(hasText(app.getString(R.string.image_max_edge)))
58+
node(R.string.image_max_edge).performScrollTo().performTextReplacement("2")
59+
node(R.string.image_max_edge).performTextInput("048")
60+
compose.waitUntil(10_000) { model.settings.value.imageMaxEdge == 2048 }
61+
node(R.string.image_max_edge).assertIsEnabled()
62+
icon(R.string.back).performClick()
63+
node(R.string.journal).performClick()
64+
icon(R.string.back).performClick()
65+
icon(R.string.diagnostics).performClick()
66+
node(R.string.refresh_log).performClick()
67+
icon(R.string.back).performClick()
68+
icon(R.string.updates).performClick()
69+
node(R.string.update_github).performScrollTo().performClick()
70+
compose.waitUntil(10_000) { AppUpdates(app).source() == UpdateSource.GITHUB }
71+
node(R.string.update_automatic).performScrollTo()
72+
compose.onNodeWithContentDescription(app.getString(R.string.update_automatic)).performClick()
73+
compose.waitUntil(10_000) { !UpdatePreferences(app).automatic }
74+
UpdatePreferences(app).detected(UpdateSource.GITHUB, AppUpdate(UpdateSource.GITHUB, "99.0.0"))
75+
// App settings, permission checks and the update dialog refresh on ON_RESUME.
76+
compose.activityRule.scenario.moveToState(androidx.lifecycle.Lifecycle.State.CREATED)
77+
compose.activityRule.scenario.moveToState(androidx.lifecycle.Lifecycle.State.RESUMED)
78+
compose.waitUntil(10_000) {
79+
compose.onAllNodesWithText(app.getString(R.string.update_remind_later)).fetchSemanticsNodes().isNotEmpty()
80+
}
81+
node(R.string.update_remind_later).performClick()
82+
compose.waitUntil(10_000) { UpdatePreferences(app).pending() == null }
83+
compose.waitUntil(10_000) { compose.onAllNodes(isDialog()).fetchSemanticsNodes().isEmpty() }
84+
app.getSharedPreferences("crash_state", 0).edit().putBoolean("pending", true).commit()
85+
app.crashHandler.load()
86+
compose.waitUntil(10_000) {
87+
compose.onAllNodesWithText(app.getString(R.string.crash_title)).fetchSemanticsNodes().isNotEmpty()
88+
}
89+
node(R.string.close).performClick()
90+
compose.waitUntil(10_000) { !app.crashHandler.pending }
91+
compose.runOnUiThread { app.graph.captureFailed(IllegalStateException("synthetic failure")) }
92+
compose.waitUntil(10_000) { model.settings.value.captureFailed }
93+
}
94+
95+
}

‎app/src/main/AndroidManifest.xml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?xml version="1.0" encoding="utf-8"?>
2-
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
2+
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
3+
xmlns:tools="http://schemas.android.com/tools">
34
<uses-permission android:name="android.permission.INTERNET" />
45
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
56
<uses-permission android:name="android.permission.RECEIVE_SMS" />
@@ -40,6 +41,12 @@
4041
android:roundIcon="@mipmap/ic_launcher_round"
4142
android:supportsRtl="true"
4243
android:theme="@style/Theme.Message487">
44+
<!-- Recovery initializes WorkManager on its background executor. -->
45+
<provider android:name="androidx.startup.InitializationProvider"
46+
android:authorities="${applicationId}.androidx-startup" android:exported="false" tools:node="merge">
47+
<meta-data android:name="androidx.work.WorkManagerInitializer"
48+
android:value="androidx.startup" tools:node="remove" />
49+
</provider>
4350
<provider
4451
android:name="androidx.core.content.FileProvider"
4552
android:authorities="${applicationId}.fileprovider"

‎app/src/main/java/life/andre/message487/AppUpdates.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ internal fun requireUpdateMetadataStatus(status: Int) {
105105
}
106106

107107
internal class AppUpdates(private val context: Context) {
108-
private val preferences = context.getSharedPreferences("updates", Context.MODE_PRIVATE)
108+
private val preferences by lazy { context.getSharedPreferences("updates", Context.MODE_PRIVATE) }
109109
private val manager = context.packageManager
110110

111111
@Suppress("DEPRECATION")
@@ -130,7 +130,7 @@ internal class AppUpdates(private val context: Context) {
130130
}
131131

132132
fun select(source: UpdateSource) {
133-
preferences.edit().putString("source", source.name).apply()
133+
preferences.edit().putString("source", source.name).commit()
134134
}
135135

136136
suspend fun check(source: UpdateSource): AppUpdate? = withContext(Dispatchers.IO) {

‎app/src/main/java/life/andre/message487/ConnectionViewModel.kt‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ data class PermissionState(val notifications: Boolean = false, val sms: Boolean
3939
class ConnectionViewModel(application: Application) : AndroidViewModel(application) {
4040
private val graph = MessageGraph.get(application)
4141
val settings = graph.settings.state
42+
val settingsLoaded = graph.settings.loaded
4243
val listenerConnected = ListenerState.connected
4344
private val mutableState = MutableStateFlow(settings.value.let {
44-
ConnectionState(it.url, it.deviceCode, it.requireAck, authToken = it.authToken)
45+
ConnectionState(it.url, it.deviceCode, it.requireAck, authToken = it.authToken, busy = true)
4546
})
4647
val state = mutableState.asStateFlow()
4748
private val mutableQueue = MutableStateFlow(QueueSnapshot())
@@ -55,6 +56,13 @@ class ConnectionViewModel(application: Application) : AndroidViewModel(applicati
5556
init {
5657
refreshPermissions()
5758
viewModelScope.launch {
59+
try {
60+
val saved = withContext(Dispatchers.IO) { graph.settings.load() }
61+
mutableState.value = ConnectionState(saved.url, saved.deviceCode, saved.requireAck, authToken = saved.authToken)
62+
} catch (error: Exception) {
63+
if (error is kotlinx.coroutines.CancellationException) throw error
64+
mutableState.value = state.value.copy(notice = R.string.local_error)
65+
}
5866
graph.outbox.revision.collect {
5967
try {
6068
mutableQueue.value = withContext(Dispatchers.IO) {
@@ -101,13 +109,24 @@ class ConnectionViewModel(application: Application) : AndroidViewModel(applicati
101109
}
102110
}
103111

112+
private var permissionRefresh: kotlinx.coroutines.Job? = null
104113
fun refreshPermissions() {
105114
refreshApps()
106-
val app = getApplication<Application>()
107-
mutablePermissions.value = PermissionState(
108-
notifications = app.packageName in NotificationManagerCompat.getEnabledListenerPackages(app),
109-
sms = ContextCompat.checkSelfPermission(app, Manifest.permission.RECEIVE_SMS) == PackageManager.PERMISSION_GRANTED,
110-
)
115+
permissionRefresh?.cancel()
116+
permissionRefresh = viewModelScope.launch {
117+
try {
118+
mutablePermissions.value = withContext(Dispatchers.IO) {
119+
val app = getApplication<Application>()
120+
PermissionState(
121+
notifications = app.packageName in NotificationManagerCompat.getEnabledListenerPackages(app),
122+
sms = ContextCompat.checkSelfPermission(app, Manifest.permission.RECEIVE_SMS) == PackageManager.PERMISSION_GRANTED,
123+
)
124+
}
125+
} catch (error: Exception) {
126+
if (error is kotlinx.coroutines.CancellationException) throw error
127+
mutableState.value = state.value.copy(notice = R.string.local_error)
128+
}
129+
}
111130
}
112131

113132
fun setAuthToken(token: String) { mutableState.value = state.value.copy(authToken = token, invalidToken = false, notice = null) }
@@ -118,6 +137,7 @@ class ConnectionViewModel(application: Application) : AndroidViewModel(applicati
118137
fun setTestOptions(options: TestMessageOptions) { mutableState.value = state.value.copy(testOptions = options) }
119138

120139
fun save(sendTest: Boolean) {
140+
if (state.value.busy) return
121141
val draft = state.value
122142
val url = draft.url.trim()
123143
val code = draft.deviceCode.trim()
@@ -132,18 +152,18 @@ class ConnectionViewModel(application: Application) : AndroidViewModel(applicati
132152

133153
fun sendTest() = action(R.string.test_queued) { graph.enqueueTest() }
134154
fun notifications(enabled: Boolean) = action { graph.settings.update { it.copy(notifications = enabled) } }
135-
fun deduplicationWindow(seconds: Int) = action {
155+
fun deduplicationWindow(seconds: Int) = numberSetting("deduplicationWindow") {
136156
graph.settings.update { it.copy(deduplicationWindowSeconds = seconds) }
137157
}
138158
fun smsDeduplication(enabled: Boolean) = action { graph.settings.update { it.copy(smsDeduplication = enabled) } }
139159
fun notificationDeduplication(enabled: Boolean) = action { graph.settings.update { it.copy(notificationDeduplication = enabled) } }
140160
fun forwardAvatars(enabled: Boolean) = action { graph.settings.update { it.copy(forwardAvatars = enabled) } }
141161
fun forwardAppIcons(enabled: Boolean) = action { graph.settings.update { it.copy(forwardAppIcons = enabled) } }
142162
fun imageLimits(enabled: Boolean) = action { graph.settings.update { it.copy(imageLimitsEnabled = enabled) } }
143-
fun imageMaxCount(value: Int) = action { graph.settings.update { it.copy(imageMaxCount = value) } }
144-
fun imageMaxEdge(value: Int) = action { graph.settings.update { it.copy(imageMaxEdge = value) } }
145-
fun imageMaxKiB(value: Int) = action { graph.settings.update { it.copy(imageMaxKiB = value) } }
146-
fun imageTotalKiB(value: Int) = action { graph.settings.update { it.copy(imageTotalKiB = value) } }
163+
fun imageMaxCount(value: Int) = numberSetting("imageMaxCount") { graph.settings.update { it.copy(imageMaxCount = value) } }
164+
fun imageMaxEdge(value: Int) = numberSetting("imageMaxEdge") { graph.settings.update { it.copy(imageMaxEdge = value) } }
165+
fun imageMaxKiB(value: Int) = numberSetting("imageMaxKiB") { graph.settings.update { it.copy(imageMaxKiB = value) } }
166+
fun imageTotalKiB(value: Int) = numberSetting("imageTotalKiB") { graph.settings.update { it.copy(imageTotalKiB = value) } }
147167
fun imageFormat(format: NotificationImageFormat) = action { graph.settings.update { it.copy(imageFormat = format) } }
148168
fun imageQuality(quality: Int) = action { graph.settings.update { it.copy(imageQuality = quality) } }
149169
fun sms(enabled: Boolean) = action { graph.settings.update { it.copy(sms = enabled) } }
@@ -175,13 +195,27 @@ class ConnectionViewModel(application: Application) : AndroidViewModel(applicati
175195
}
176196
fun delete(id: String) = action { graph.outbox.delete(id); graph.diagnostics.record(DiagnosticEvent.EVENT_DELETED) }
177197
fun clearError() = action { graph.settings.update { it.copy(captureFailed = false) } }
178-
fun rebind() {
198+
fun rebind() = action {
179199
NotificationListenerService.requestRebind(ComponentName(getApplication(), NotificationCaptureService::class.java))
180200
}
201+
202+
private val numberSaves = mutableMapOf<String, kotlinx.coroutines.Job>()
203+
private fun numberSetting(key: String, block: () -> Unit) {
204+
if (!graph.settings.loaded.value) return
205+
numberSaves.remove(key)?.cancel()
206+
numberSaves[key] = viewModelScope.launch {
207+
kotlinx.coroutines.delay(300)
208+
try { withContext(Dispatchers.IO) { block() } }
209+
catch (error: Exception) {
210+
if (error is kotlinx.coroutines.CancellationException) throw error
211+
mutableState.value = state.value.copy(notice = R.string.local_error)
212+
}
213+
}
214+
}
181215
fun showSettingsError() { mutableState.value = state.value.copy(notice = R.string.settings_unavailable) }
182216

183217
private fun action(notice: Int? = null, block: () -> Unit) {
184-
if (state.value.busy) return
218+
if (state.value.busy || !graph.settings.loaded.value) return
185219
mutableState.value = state.value.copy(busy = true, notice = null)
186220
viewModelScope.launch {
187221
val result = try {

‎app/src/main/java/life/andre/message487/DeliveryWorker.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class DeliveryWorker(context: Context, parameters: WorkerParameters) : Coroutine
4141
val graph = MessageGraph.get(applicationContext)
4242
val id = inputData.getString("event_id") ?: return@withContext Result.failure()
4343
try {
44-
if (graph.settings.state.value.paused) return@withContext Result.retry()
44+
if (graph.settings.load().paused) return@withContext Result.retry()
4545
val attempt = try {
4646
graph.outbox.beginAttempt(id)
4747
} catch (databaseError: android.database.SQLException) {

‎app/src/main/java/life/andre/message487/ForwardingSettings.kt‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,19 @@ class SettingsStore(
4949
private val cipher: PayloadCipher = (context.applicationContext as? MessageApplication)?.payloadCipher ?: KeystorePayloadCipher(),
5050
) {
5151
private val diagnostics = (context.applicationContext as? MessageApplication)?.diagnostics
52-
private val preferences = context.getSharedPreferences("connection", Context.MODE_PRIVATE)
53-
private val mutableState = MutableStateFlow(read())
52+
private val preferences by lazy { context.getSharedPreferences("connection", Context.MODE_PRIVATE) }
53+
private val mutableState = MutableStateFlow(ForwardingSettings())
54+
private val mutableLoaded = MutableStateFlow(false)
55+
val loaded = mutableLoaded.asStateFlow()
56+
57+
@Synchronized
58+
fun load(): ForwardingSettings {
59+
if (!mutableLoaded.value) {
60+
mutableState.value = read()
61+
mutableLoaded.value = true
62+
}
63+
return mutableState.value
64+
}
5465
val state = mutableState.asStateFlow()
5566

5667
private fun read() = ForwardingSettings(
@@ -86,6 +97,7 @@ class SettingsStore(
8697

8798
@Synchronized
8899
fun discoverPackages(packages: Set<String>, ownPackage: String) {
100+
load()
89101
val discovered = packages - ownPackage
90102
if (state.value.knownPackages.containsAll(discovered)) return
91103
update {
@@ -98,9 +110,11 @@ class SettingsStore(
98110

99111
@Synchronized
100112
fun update(transform: (ForwardingSettings) -> ForwardingSettings): ForwardingSettings {
113+
load()
101114
val next = transform(mutableState.value).let {
102115
if (it.deviceId.isBlank()) it.copy(deviceId = UUID.randomUUID().toString()) else it
103116
}
117+
if (next == mutableState.value) return next
104118
val editor = preferences.edit()
105119
if (next.authToken != mutableState.value.authToken) {
106120
require(validAuthToken(next.authToken))

0 commit comments

Comments
 (0)