Skip to content

Commit b8c44d0

Browse files
Hidden-NodeHidden-Node
committed
refactor(util): extract pure ScanStateReducer from VpnManager (#47)
Lifts the regex-based scan-line scraping out of the private stateful `VpnManager.parseScanLine` into an `internal` pure `(prev: ScanStateBundle, line: String) -> ScanStateBundle` function in a new `ScanStateReducer.kt`. The singleton's public API is unchanged; `parseScanLine` becomes a 10-line wrapper that threads its three StateFlows (`_scanStatus`, `_activeResolvers`, `_connectionWarning`) through the reducer and emits new values only when they change. ## Why `parseScanLine` ran up to 10 regex matches per Go-core log line and mutated 9 `MutableStateFlow` fields. It was private and stateful, so the regex contract could not be unit-tested without driving the singleton through `appendLog` and observing StateFlows via Turbine. This made every change to the log-scraping logic risky and uncharacterizable. Extracting a pure reducer creates a trivially table-testable seam (the cheapest seam for the future god-object decompose of `VpnManager`, finding 9) at zero behavior change. ## What changed ### `android/app/src/main/java/com/masterdns/vpn/util/ScanStateReducer.kt` (new, +155) - `internal data class ScanStateBundle` -- immutable input/output bundle holding `scanStatus: VpnManager.ScanStatus`, `activeResolvers: List<String>`, `connectionWarning: String?`. - `internal object ScanStateReducer` -- owns the 10 regex constants moved verbatim from `VpnManager.kt` (patterns byte-identical, same `RegexOption.IGNORE_CASE` / `DOT_MATCHES_ALL` where applicable): `INDEXED_PROGRESS`, `TOTAL_CANDIDATES`, `SCAN_TOTALS`, `ACTIVE_RESOLVERS`, `TOTAL_ACTIVE`, `REMAINING`, `SYNCED_MTU`, `RESOLVER_ADDED`, `RESOLVER_REMOVED`, `SESSION_INIT_BACKOFF`. - `internal fun reduce(prev, line)` -- pre-computes the 6 early-return matches (`scanMatch`, `activeMatch`, `totalActiveMatch`, `remainingMatch`, `syncedMtuMatch`, `testingMtu`) before a `when` cascade, then a single `if (anyMatched) return` short-circuits the trailing `MTU Testing Completed` / `Session Initialized Successfully` triggers and `SESSION_INIT_BACKOFF` blocks. The 4 non-returning pre-blocks (`RESOLVER_ADDED`, `RESOLVER_REMOVED`, `INDEXED_PROGRESS`, `TOTAL_CANDIDATES`) run before the cascade, unchanged. ### `android/app/src/main/java/com/masterdns/vpn/util/VpnManager.kt` (+14/-131) - `parseScanLine` body (was 93 lines, ended at line 451) replaced with a 10-line wrapper: snapshot the 3 StateFlows into a `ScanStateBundle`, call `ScanStateReducer.reduce(prev, line)`, emit each changed field back via `if (next.X != prev.X) _X.value = next.X` guards. - The 10 `private val *_REGEX` constants (previously at lines 101-140) removed from `VpnManager` -- they now live in `ScanStateReducer`. - `TIMESTAMP_CANDIDATES` and `TimestampCandidate` (timestamp normalization, not scan-state) stay in `VpnManager`, untouched. - The remaining `_scanStatus.value = _scanStatus.value.copy(...)` site inside `updateState` (the `VpnState.CONNECTED` reset, line 147) is unrelated to scan-line parsing and is intentionally preserved. ### `android/app/src/test/java/com/masterdns/vpn/util/ScanStateReducerTest.kt` (new, +120) - 14 plain JUnit4 `@Test` methods (no new test dependencies -- reuses the existing `testImplementation("junit:junit:4.13.2")` and `kotlinx-coroutines-test:1.9.0` orchestrated by plan 005). Pattern follows the existing `GlobalSettingsPortRangeTest.kt` skeleton (same package `com.masterdns.vpn.util`, plain `assertEquals`/`assertTrue`). - Cases cover: empty/non-matching lines, the `INDEXED_PROGRESS` happy path and non-numeric-total skip, `SCAN_TOTALS` Accepted/Rejected, `RESOLVER_ADDED` idempotence, `RESOLVER_REMOVED` removal, `"Testing MTU sizes"` / `"MTU Testing Completed"` / `"Session Initialized Successfully"` triggers, `SESSION_INIT_BACKOFF`, the precedence invariant (case 13: a line matching both `SCAN_TOTALS` and `ACTIVE_RESOLVERS` fires only the first), and `SYNCED_MTU`. ## Behavior preservation Public API of `VpnManager` (`state`, `scanStatus`, `activeResolvers`, `connect`, `disconnect`, `appendLog`, `appendCoreLog`, etc.) is unchanged -- `MasterDnsVpnService.kt`, `VpnTileService.kt`, `ResolversScreen.kt`, and `HomeStatusCards.kt` continue to consume the same StateFlows with no edits. The `if (next != prev)` emission guards are an additive optimization: observers receive exactly the same sequence of distinct StateFlow values as before; the no-match case no longer fires a redundant emission. ## Scope 3 files in `android/app/src/main/java/com/masterdns/vpn/util/` and `android/app/src/test/java/com/masterdns/vpn/util/`. No out-of-scope edits; no `mobile/`, no `cmd/`/`internal/`, no `go.mod`/`go.sum`, no other UI files. The `plans/` folder (locally gitignored via `.git/info/exclude`) is not committed and stays on the local checkout for reference. ## Verification Gradle `compileDebugKotlin` + `testDebugUnitTest` were not run locally per the user's no-local-build constraint; this PR is the gate -- `android-ci.yml` runs `assembleDebug` which compiles and runs the unit-test suite on push. The 14 new tests are JVM-runnable (no Robolectric, no device) and target the pure reducer directly. Plan 005's `ResolverAnalyzerTest` and the existing `GlobalSettingsPortRangeTest` prove the testDebugUnitTest task is wired and runs in CI. ## Maintenance notes - **Adding a new scan-state field**: extend `ScanStatus` and `ScanStateBundle`, then add a new `?.let { match -> ... }` block in `ScanStateReducer.reduce`. Do NOT re-mutate `_scanStatus` from `parseScanLine` directly -- that is the anti-pattern this refactor removed. - **Performance**: the `if (next != prev)` guards are pure optimizations; if a StateFlow-observer race appears under heavy load, removing the guards is safe (emitting an identical value is legal and cheap). - **`ScanStateReducer` is the first Hilt-injectable seam for `VpnManager`**: a follow-up can make it a `@Singleton` injected interface so an `androidTest` can swap a fake reducer that asserts on the lines it received. Squash-merges planner commits 985dfb0 (extraction) and 8b02a0c (precedence-cascade bugfix discovered in self-review).
1 parent 6fd18bd commit b8c44d0

3 files changed

Lines changed: 284 additions & 131 deletions

File tree

‎android/app/src/main/java/com/masterdns/vpn/util/ScanStateReducer.kt‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package com.masterdns.vpn.util
2+
3+
/**
4+
* Pure reducer that maps a (prev scan-state bundle, core log line) pair to a
5+
* new scan-state bundle. Extracted from VpnManager.parseScanLine in plan 017
6+
* to make the regex-based log-scraping contract testable in isolation.
7+
*
8+
* The mutation surface is reduced to a single input → output transformation;
9+
* StateFlow emission is the caller's concern (VpnManager wraps each call).
10+
*/
11+
internal data class ScanStateBundle(
12+
val scanStatus: VpnManager.ScanStatus = VpnManager.ScanStatus(),
13+
val activeResolvers: List<String> = emptyList(),
14+
val connectionWarning: String? = null
15+
)
16+
17+
internal object ScanStateReducer {
18+
19+
private val INDEXED_PROGRESS_REGEX = Regex(
20+
"(?:scan|scanning|resolver|resolvers|mtu|accepted|rejected).{0,40}?(\\d+)\\s*/\\s*(\\d+)",
21+
setOf(RegexOption.IGNORE_CASE, RegexOption.DOT_MATCHES_ALL)
22+
)
23+
private val TOTAL_CANDIDATES_REGEX = Regex(
24+
"(?:valid\\s+resolvers|resolvers\\s+for\\s+scan|scan\\s+pool|resolver\\s+pool|total\\s+resolvers).{0,20}?(\\d+)",
25+
RegexOption.IGNORE_CASE
26+
)
27+
private val SCAN_TOTALS_REGEX = Regex(
28+
"via\\s+([^\\s|]+)\\s*\\|.*totals:\\s*valid=(\\d+),\\s*rejected=(\\d+)",
29+
RegexOption.IGNORE_CASE
30+
)
31+
private val ACTIVE_RESOLVERS_REGEX = Regex(
32+
"Active Resolvers\\s*[:=]\\s*[^\\d-]*(\\d+)",
33+
RegexOption.IGNORE_CASE
34+
)
35+
private val TOTAL_ACTIVE_REGEX = Regex(
36+
"total\\s+active\\s*[:=]\\s*[^\\d-]*(\\d+)",
37+
RegexOption.IGNORE_CASE
38+
)
39+
private val REMAINING_REGEX = Regex(
40+
"remaining\\s*[:=]\\s*[^\\d-]*(\\d+)",
41+
RegexOption.IGNORE_CASE
42+
)
43+
private val SYNCED_MTU_REGEX = Regex(
44+
"Selected Synced Upload MTU:\\s*(\\d+)\\s*\\|\\s*Selected Synced Download MTU:\\s*(\\d+)",
45+
RegexOption.IGNORE_CASE
46+
)
47+
private val RESOLVER_ADDED_REGEX = Regex(
48+
"(?:✅ Accepted|🔄 DNS Resolver Reactivated).*?(\\d+\\.\\d+\\.\\d+\\.\\d+:\\d+|\\[[a-fA-F0-9:]+\\]:\\d+)",
49+
RegexOption.IGNORE_CASE
50+
)
51+
private val RESOLVER_REMOVED_REGEX = Regex(
52+
"DNS Resolver disabled.*?(\\d+\\.\\d+\\.\\d+\\.\\d+:\\d+|\\[[a-fA-F0-9:]+\\]:\\d+)",
53+
RegexOption.IGNORE_CASE
54+
)
55+
private val SESSION_INIT_BACKOFF_REGEX = Regex(
56+
"Session init retry backoff:\\s*(.*)",
57+
RegexOption.IGNORE_CASE
58+
)
59+
60+
internal fun reduce(prev: ScanStateBundle, line: String): ScanStateBundle {
61+
var scanStatus = prev.scanStatus
62+
var activeResolvers = prev.activeResolvers
63+
var connectionWarning = prev.connectionWarning
64+
65+
RESOLVER_ADDED_REGEX.find(line)?.let { match ->
66+
val res = match.groupValues[1]
67+
activeResolvers = (activeResolvers.toSet() + res).toList()
68+
}
69+
70+
RESOLVER_REMOVED_REGEX.find(line)?.let { match ->
71+
val res = match.groupValues[1]
72+
activeResolvers = (activeResolvers.toSet() - res).toList()
73+
}
74+
75+
INDEXED_PROGRESS_REGEX.find(line)?.let { match ->
76+
val total = match.groupValues[2].toIntOrNull()
77+
if (total != null && total > 0) {
78+
scanStatus = scanStatus.copy(scanTotalFromCore = total)
79+
}
80+
}
81+
82+
TOTAL_CANDIDATES_REGEX.find(line)?.let { match ->
83+
val total = match.groupValues[1].toIntOrNull()
84+
if (total != null && total > 0) {
85+
scanStatus = scanStatus.copy(scanTotalFromCore = total)
86+
}
87+
}
88+
89+
// ponytail: pre-compute the 6 early-return matches, then a when cascade
90+
// mutates scanStatus for the first matching arm only, then a single
91+
// guarded return short-circuits the trailing blocks. Mirrors the
92+
// original parseScanLine early-returns (SCAN_TOTALS, ACTIVE_RESOLVERS,
93+
// TOTAL_ACTIVE, REMAINING, SYNCED_MTU, "Testing MTU sizes") so a line
94+
// matching multiple patterns only fires the first match.
95+
val scanMatch = SCAN_TOTALS_REGEX.find(line)
96+
val activeMatch = ACTIVE_RESOLVERS_REGEX.find(line)
97+
val totalActiveMatch = TOTAL_ACTIVE_REGEX.find(line)
98+
val remainingMatch = REMAINING_REGEX.find(line)
99+
val syncedMtuMatch = SYNCED_MTU_REGEX.find(line)
100+
val testingMtu = line.contains("Testing MTU sizes", ignoreCase = true)
101+
102+
when {
103+
scanMatch != null -> {
104+
val resolver = scanMatch!!.groupValues[1]
105+
val valid = scanMatch!!.groupValues[2].toIntOrNull() ?: scanStatus.validCount
106+
val rejected = scanMatch!!.groupValues[3].toIntOrNull() ?: scanStatus.rejectedCount
107+
val decision = when {
108+
line.contains("Accepted", ignoreCase = true) -> "Accepted"
109+
line.contains("Rejected", ignoreCase = true) -> "Rejected"
110+
else -> ""
111+
}
112+
scanStatus = scanStatus.copy(
113+
scanning = true,
114+
lastResolver = resolver,
115+
lastDecision = decision,
116+
validCount = valid,
117+
rejectedCount = rejected
118+
)
119+
}
120+
activeMatch != null -> scanStatus = scanStatus.copy(
121+
activeResolvers = activeMatch!!.groupValues[1].toIntOrNull() ?: scanStatus.activeResolvers
122+
)
123+
totalActiveMatch != null -> scanStatus = scanStatus.copy(
124+
activeResolvers = totalActiveMatch!!.groupValues[1].toIntOrNull() ?: scanStatus.activeResolvers
125+
)
126+
remainingMatch != null -> scanStatus = scanStatus.copy(
127+
activeResolvers = remainingMatch!!.groupValues[1].toIntOrNull() ?: scanStatus.activeResolvers
128+
)
129+
syncedMtuMatch != null -> scanStatus = scanStatus.copy(
130+
syncedUploadMtu = syncedMtuMatch!!.groupValues[1].toIntOrNull() ?: 0,
131+
syncedDownloadMtu = syncedMtuMatch!!.groupValues[2].toIntOrNull() ?: 0
132+
)
133+
testingMtu -> scanStatus = scanStatus.copy(scanning = true)
134+
}
135+
136+
if (scanMatch != null || activeMatch != null || totalActiveMatch != null ||
137+
remainingMatch != null || syncedMtuMatch != null || testingMtu
138+
) {
139+
return ScanStateBundle(scanStatus, activeResolvers, connectionWarning)
140+
}
141+
142+
if (line.contains("MTU Testing Completed", ignoreCase = true) ||
143+
line.contains("Session Initialized Successfully", ignoreCase = true)
144+
) {
145+
scanStatus = scanStatus.copy(scanning = false)
146+
}
147+
148+
SESSION_INIT_BACKOFF_REGEX.find(line)?.let { match ->
149+
val backoff = match.groupValues[1].trim()
150+
connectionWarning = "Session init retry backoff: $backoff"
151+
}
152+
153+
return ScanStateBundle(scanStatus, activeResolvers, connectionWarning)
154+
}
155+
}

‎android/app/src/main/java/com/masterdns/vpn/util/VpnManager.kt‎

Lines changed: 9 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -98,46 +98,6 @@ object VpnManager {
9898
private val logBuffer = ArrayDeque<LogEntry>(MAX_LOG_LINES)
9999
private var logBufferVersion = 0L
100100

101-
private val INDEXED_PROGRESS_REGEX = Regex(
102-
"(?:scan|scanning|resolver|resolvers|mtu|accepted|rejected).{0,40}?(\\d+)\\s*/\\s*(\\d+)",
103-
setOf(RegexOption.IGNORE_CASE, RegexOption.DOT_MATCHES_ALL)
104-
)
105-
private val TOTAL_CANDIDATES_REGEX = Regex(
106-
"(?:valid\\s+resolvers|resolvers\\s+for\\s+scan|scan\\s+pool|resolver\\s+pool|total\\s+resolvers).{0,20}?(\\d+)",
107-
RegexOption.IGNORE_CASE
108-
)
109-
private val SCAN_TOTALS_REGEX = Regex(
110-
"via\\s+([^\\s|]+)\\s*\\|.*totals:\\s*valid=(\\d+),\\s*rejected=(\\d+)",
111-
RegexOption.IGNORE_CASE
112-
)
113-
private val ACTIVE_RESOLVERS_REGEX = Regex(
114-
"Active Resolvers\\s*[:=]\\s*[^\\d-]*(\\d+)",
115-
RegexOption.IGNORE_CASE
116-
)
117-
private val TOTAL_ACTIVE_REGEX = Regex(
118-
"total\\s+active\\s*[:=]\\s*[^\\d-]*(\\d+)",
119-
RegexOption.IGNORE_CASE
120-
)
121-
private val REMAINING_REGEX = Regex(
122-
"remaining\\s*[:=]\\s*[^\\d-]*(\\d+)",
123-
RegexOption.IGNORE_CASE
124-
)
125-
private val SYNCED_MTU_REGEX = Regex(
126-
"Selected Synced Upload MTU:\\s*(\\d+)\\s*\\|\\s*Selected Synced Download MTU:\\s*(\\d+)",
127-
RegexOption.IGNORE_CASE
128-
)
129-
private val RESOLVER_ADDED_REGEX = Regex(
130-
"(?:✅ Accepted|🔄 DNS Resolver Reactivated).*?(\\d+\\.\\d+\\.\\d+\\.\\d+:\\d+|\\[[a-fA-F0-9:]+\\]:\\d+)",
131-
RegexOption.IGNORE_CASE
132-
)
133-
private val RESOLVER_REMOVED_REGEX = Regex(
134-
"DNS Resolver disabled.*?(\\d+\\.\\d+\\.\\d+\\.\\d+:\\d+|\\[[a-fA-F0-9:]+\\]:\\d+)",
135-
RegexOption.IGNORE_CASE
136-
)
137-
private val SESSION_INIT_BACKOFF_REGEX = Regex(
138-
"Session init retry backoff:\\s*(.*)",
139-
RegexOption.IGNORE_CASE
140-
)
141101
private val TIMESTAMP_CANDIDATES = listOf(
142102
TimestampCandidate(
143103
Regex("^(\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{3}Z)(.*)$"),
@@ -357,97 +317,15 @@ object VpnManager {
357317
}
358318

359319
private fun parseScanLine(line: String) {
360-
RESOLVER_ADDED_REGEX.find(line)?.let { match ->
361-
val res = match.groupValues[1]
362-
val current = _activeResolvers.value.toMutableSet()
363-
current.add(res)
364-
_activeResolvers.value = current.toList()
365-
}
366-
367-
RESOLVER_REMOVED_REGEX.find(line)?.let { match ->
368-
val res = match.groupValues[1]
369-
val current = _activeResolvers.value.toMutableSet()
370-
current.remove(res)
371-
_activeResolvers.value = current.toList()
372-
}
373-
374-
INDEXED_PROGRESS_REGEX.find(line)?.let { match ->
375-
val total = match.groupValues[2].toIntOrNull()
376-
if (total != null && total > 0) {
377-
_scanStatus.value = _scanStatus.value.copy(scanTotalFromCore = total)
378-
}
379-
}
380-
381-
TOTAL_CANDIDATES_REGEX.find(line)?.let { match ->
382-
val total = match.groupValues[1].toIntOrNull()
383-
if (total != null && total > 0) {
384-
_scanStatus.value = _scanStatus.value.copy(scanTotalFromCore = total)
385-
}
386-
}
387-
388-
SCAN_TOTALS_REGEX.find(line)?.let { match ->
389-
val resolver = match.groupValues[1]
390-
val valid = match.groupValues[2].toIntOrNull() ?: _scanStatus.value.validCount
391-
val rejected = match.groupValues[3].toIntOrNull() ?: _scanStatus.value.rejectedCount
392-
val decision = when {
393-
line.contains("Accepted", ignoreCase = true) -> "Accepted"
394-
line.contains("Rejected", ignoreCase = true) -> "Rejected"
395-
else -> ""
396-
}
397-
_scanStatus.value = _scanStatus.value.copy(
398-
scanning = true,
399-
lastResolver = resolver,
400-
lastDecision = decision,
401-
validCount = valid,
402-
rejectedCount = rejected
403-
)
404-
return
405-
}
406-
407-
ACTIVE_RESOLVERS_REGEX.find(line)?.let { match ->
408-
_scanStatus.value = _scanStatus.value.copy(
409-
activeResolvers = match.groupValues[1].toIntOrNull() ?: _scanStatus.value.activeResolvers
410-
)
411-
return
412-
}
413-
414-
TOTAL_ACTIVE_REGEX.find(line)?.let { match ->
415-
_scanStatus.value = _scanStatus.value.copy(
416-
activeResolvers = match.groupValues[1].toIntOrNull() ?: _scanStatus.value.activeResolvers
417-
)
418-
return
419-
}
420-
421-
REMAINING_REGEX.find(line)?.let { match ->
422-
_scanStatus.value = _scanStatus.value.copy(
423-
activeResolvers = match.groupValues[1].toIntOrNull() ?: _scanStatus.value.activeResolvers
424-
)
425-
return
426-
}
427-
428-
SYNCED_MTU_REGEX.find(line)?.let { match ->
429-
_scanStatus.value = _scanStatus.value.copy(
430-
syncedUploadMtu = match.groupValues[1].toIntOrNull() ?: 0,
431-
syncedDownloadMtu = match.groupValues[2].toIntOrNull() ?: 0
432-
)
433-
return
434-
}
435-
436-
if (line.contains("Testing MTU sizes", ignoreCase = true)) {
437-
_scanStatus.value = _scanStatus.value.copy(scanning = true)
438-
return
439-
}
440-
441-
if (line.contains("MTU Testing Completed", ignoreCase = true) ||
442-
line.contains("Session Initialized Successfully", ignoreCase = true)
443-
) {
444-
_scanStatus.value = _scanStatus.value.copy(scanning = false)
445-
}
446-
447-
SESSION_INIT_BACKOFF_REGEX.find(line)?.let { match ->
448-
val backoff = match.groupValues[1].trim()
449-
_connectionWarning.value = "Session init retry backoff: $backoff"
450-
}
320+
val prev = ScanStateBundle(
321+
scanStatus = _scanStatus.value,
322+
activeResolvers = _activeResolvers.value,
323+
connectionWarning = _connectionWarning.value
324+
)
325+
val next = ScanStateReducer.reduce(prev, line)
326+
if (next.scanStatus != prev.scanStatus) _scanStatus.value = next.scanStatus
327+
if (next.activeResolvers != prev.activeResolvers) _activeResolvers.value = next.activeResolvers
328+
if (next.connectionWarning != prev.connectionWarning) _connectionWarning.value = next.connectionWarning
451329
}
452330

453331
private fun normalizeLogTimestampToLocal(line: String): String {

0 commit comments

Comments
 (0)