Skip to content

Commit 9ac0a56

Browse files
RasulOsclaude
RasulOs
and
claude
committed
refactor(triggers): compiler-enforced enum mappings + predicate renames
- Extract handlePortalEvent's EventType -> TriggerSource mapping into triggerSourceFor with an exhaustive when (no else), so a new EventType fails to compile until it gets an explicit decision - Make visibilityFor exhaustive: the five sources that use the editor defaults are now listed explicitly instead of hiding behind else - Rename the two isNotificationSource functions to what they decide: requiresNotificationAccess (editor/permissions, POSTED||REMOVED) vs shouldDebounce (runtime buffer, POSTED only), and document why they differ on purpose - Pin all three decision tables with allow-list tests over the full enum entries, mirroring the existing description-coverage pattern Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0e51428 commit 9ac0a56

6 files changed

Lines changed: 208 additions & 27 deletions

File tree

‎app/src/main/java/com/mobilerun/portal/triggers/TriggerApi.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class TriggerApi(
176176

177177
var ruleToSave = validRule
178178
if (ruleToSave.enabled &&
179-
TriggerEditorSupport.isNotificationSource(ruleToSave.source) &&
179+
TriggerEditorSupport.requiresNotificationAccess(ruleToSave.source) &&
180180
!environmentStatusProvider.get(appContext).notificationAccessEnabled
181181
) {
182182
ruleToSave = ruleToSave.copy(enabled = false)
@@ -209,7 +209,7 @@ class TriggerApi(
209209
"Trigger rule not found: $ruleId",
210210
)
211211
if (enabled &&
212-
TriggerEditorSupport.isNotificationSource(existingRule.source) &&
212+
TriggerEditorSupport.requiresNotificationAccess(existingRule.source) &&
213213
!environmentStatusProvider.get(appContext).notificationAccessEnabled
214214
) {
215215
return TriggerApiResult.Error(
@@ -230,7 +230,7 @@ class TriggerApi(
230230
val rule = operations.getRule(ruleId) ?: return TriggerApiResult.Error(
231231
"Trigger rule not found: $ruleId",
232232
)
233-
if (TriggerEditorSupport.isNotificationSource(rule.source) &&
233+
if (TriggerEditorSupport.requiresNotificationAccess(rule.source) &&
234234
!environmentStatusProvider.get(appContext).notificationAccessEnabled
235235
) {
236236
return TriggerApiResult.Error(

‎app/src/main/java/com/mobilerun/portal/triggers/TriggerEditorSupport.kt‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,13 @@ object TriggerEditorSupport {
7676
showCooldown = false,
7777
)
7878

79-
else -> Visibility()
79+
// Sources that use the editor defaults on purpose.
80+
TriggerSource.BATTERY_LOW,
81+
TriggerSource.BATTERY_OKAY,
82+
TriggerSource.POWER_CONNECTED,
83+
TriggerSource.POWER_DISCONNECTED,
84+
TriggerSource.USER_PRESENT,
85+
-> Visibility()
8086
}
8187
}
8288

@@ -143,7 +149,12 @@ object TriggerEditorSupport {
143149
)
144150
}
145151

146-
fun isNotificationSource(source: TriggerSource): Boolean {
152+
/**
153+
* True for sources that need the notification-listener permission.
154+
* Distinct from [TriggerRuntime.shouldDebounce]: removed notifications
155+
* need access too, but are never debounced.
156+
*/
157+
fun requiresNotificationAccess(source: TriggerSource): Boolean {
147158
return source == TriggerSource.NOTIFICATION_POSTED ||
148159
source == TriggerSource.NOTIFICATION_REMOVED
149160
}

‎app/src/main/java/com/mobilerun/portal/triggers/TriggerRuntime.kt‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -159,23 +159,36 @@ object TriggerRuntime {
159159
evaluateRule(rule, buildTestSignal(rule), isTestRun = true)
160160
}
161161

162+
/**
163+
* Exhaustive on purpose: a new [EventType] must get an explicit decision
164+
* here — otherwise the API catalogue (and, via its paired [TriggerSource],
165+
* the UI picker) would advertise a trigger that never fires.
166+
*/
167+
internal fun triggerSourceFor(type: EventType): TriggerSource? = when (type) {
168+
EventType.NOTIFICATION_POSTED -> TriggerSource.NOTIFICATION_POSTED
169+
EventType.NOTIFICATION_REMOVED -> TriggerSource.NOTIFICATION_REMOVED
170+
EventType.APP_ENTERED -> TriggerSource.APP_ENTERED
171+
EventType.APP_EXITED -> TriggerSource.APP_EXITED
172+
EventType.BATTERY_LOW -> TriggerSource.BATTERY_LOW
173+
EventType.BATTERY_OKAY -> TriggerSource.BATTERY_OKAY
174+
EventType.BATTERY_LEVEL_CHANGED -> TriggerSource.BATTERY_LEVEL_CHANGED
175+
EventType.POWER_CONNECTED -> TriggerSource.POWER_CONNECTED
176+
EventType.POWER_DISCONNECTED -> TriggerSource.POWER_DISCONNECTED
177+
EventType.USER_PRESENT -> TriggerSource.USER_PRESENT
178+
EventType.NETWORK_CONNECTED -> TriggerSource.NETWORK_CONNECTED
179+
EventType.NETWORK_TYPE_CHANGED -> TriggerSource.NETWORK_TYPE_CHANGED
180+
EventType.SMS_RECEIVED -> TriggerSource.SMS_RECEIVED
181+
// Protocol/legacy members that never fire triggers.
182+
EventType.NOTIFICATION,
183+
EventType.PING,
184+
EventType.PONG,
185+
EventType.UNKNOWN,
186+
-> null
187+
}
188+
162189
private fun handlePortalEvent(event: PortalEvent) {
163-
val signal = when (event.type) {
164-
EventType.NOTIFICATION_POSTED -> portalEventToSignal(TriggerSource.NOTIFICATION_POSTED, event)
165-
EventType.NOTIFICATION_REMOVED -> portalEventToSignal(TriggerSource.NOTIFICATION_REMOVED, event)
166-
EventType.APP_ENTERED -> portalEventToSignal(TriggerSource.APP_ENTERED, event)
167-
EventType.APP_EXITED -> portalEventToSignal(TriggerSource.APP_EXITED, event)
168-
EventType.BATTERY_LOW -> portalEventToSignal(TriggerSource.BATTERY_LOW, event)
169-
EventType.BATTERY_OKAY -> portalEventToSignal(TriggerSource.BATTERY_OKAY, event)
170-
EventType.BATTERY_LEVEL_CHANGED -> portalEventToSignal(TriggerSource.BATTERY_LEVEL_CHANGED, event)
171-
EventType.POWER_CONNECTED -> portalEventToSignal(TriggerSource.POWER_CONNECTED, event)
172-
EventType.POWER_DISCONNECTED -> portalEventToSignal(TriggerSource.POWER_DISCONNECTED, event)
173-
EventType.USER_PRESENT -> portalEventToSignal(TriggerSource.USER_PRESENT, event)
174-
EventType.NETWORK_CONNECTED -> portalEventToSignal(TriggerSource.NETWORK_CONNECTED, event)
175-
EventType.NETWORK_TYPE_CHANGED -> portalEventToSignal(TriggerSource.NETWORK_TYPE_CHANGED, event)
176-
EventType.SMS_RECEIVED -> portalEventToSignal(TriggerSource.SMS_RECEIVED, event)
177-
else -> null
178-
} ?: return
190+
val source = triggerSourceFor(event.type) ?: return
191+
val signal = portalEventToSignal(source, event) ?: return
179192

180193
val nowMs = System.currentTimeMillis()
181194
repository.listRules()
@@ -185,7 +198,7 @@ object TriggerRuntime {
185198
.filterNot { isCoolingDown(it, nowMs) }
186199
.filter { TriggerMatcher.matches(it, signal) }
187200
.forEach { rule ->
188-
if (isNotificationSource(signal.source)) {
201+
if (shouldDebounce(signal.source)) {
189202
logRun(
190203
rule = rule,
191204
disposition = TriggerRunDisposition.DEBOUNCED,
@@ -199,7 +212,13 @@ object TriggerRuntime {
199212
}
200213
}
201214

202-
private fun isNotificationSource(source: TriggerSource): Boolean =
215+
/**
216+
* Only posted notifications are debounced: the buffer batches message
217+
* title/text per sender, which removed notifications don't carry — those
218+
* evaluate immediately. Not the same predicate as
219+
* [TriggerEditorSupport.requiresNotificationAccess].
220+
*/
221+
internal fun shouldDebounce(source: TriggerSource): Boolean =
203222
source == TriggerSource.NOTIFICATION_POSTED
204223

205224
private fun evaluateRule(rule: TriggerRule, signal: TriggerSignal, isTestRun: Boolean) {

‎app/src/main/java/com/mobilerun/portal/ui/triggers/TriggerRuleEditorActivity.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ class TriggerRuleEditorActivity : AppCompatActivity() {
779779
var rule = buildRuleOrShowErrors() ?: return null
780780

781781
val needsNotificationAccess = rule.enabled &&
782-
TriggerEditorSupport.isNotificationSource(rule.source) &&
782+
TriggerEditorSupport.requiresNotificationAccess(rule.source) &&
783783
!TriggerEditorSupport.isNotificationAccessEnabled(this)
784784

785785
if (needsNotificationAccess) {
@@ -825,7 +825,7 @@ class TriggerRuleEditorActivity : AppCompatActivity() {
825825

826826
private fun testRule() {
827827
val savedRule = saveRule(finishAfterSave = false, showToast = false) ?: return
828-
if (TriggerEditorSupport.isNotificationSource(savedRule.source) &&
828+
if (TriggerEditorSupport.requiresNotificationAccess(savedRule.source) &&
829829
!TriggerEditorSupport.isNotificationAccessEnabled(this)
830830
) {
831831
Toast.makeText(this, "Cannot test: notification listener access is not granted", Toast.LENGTH_SHORT).show()

‎app/src/main/java/com/mobilerun/portal/ui/triggers/TriggerRulesActivity.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class TriggerRulesActivity : AppCompatActivity() {
141141
ruleAdapter = RuleAdapter(
142142
onToggle = { rule, enabled ->
143143
if (enabled &&
144-
TriggerEditorSupport.isNotificationSource(rule.source) &&
144+
TriggerEditorSupport.requiresNotificationAccess(rule.source) &&
145145
!TriggerEditorSupport.isNotificationAccessEnabled(this)
146146
) {
147147
showNotificationAccessWarning(rule.id)
@@ -275,7 +275,7 @@ class TriggerRulesActivity : AppCompatActivity() {
275275
if (isNotificationAccessEnabled()) return
276276
var changed = false
277277
for (rule in TriggerRuntime.listRules()) {
278-
if (rule.enabled && TriggerEditorSupport.isNotificationSource(rule.source)) {
278+
if (rule.enabled && TriggerEditorSupport.requiresNotificationAccess(rule.source)) {
279279
TriggerRuntime.setRuleEnabled(rule.id, false)
280280
changed = true
281281
}

‎app/src/test/java/com/mobilerun/portal/triggers/TriggerCoreTest.kt‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package com.mobilerun.portal.triggers
22

3+
import com.mobilerun.portal.events.model.EventType
34
import com.mobilerun.portal.taskprompt.PortalTaskSettings
45
import org.junit.Assert.assertEquals
56
import org.junit.Assert.assertFalse
67
import org.junit.Assert.assertNotNull
8+
import org.junit.Assert.assertNull
79
import org.junit.Assert.assertTrue
810
import org.junit.Test
911
import org.json.JSONObject
@@ -251,6 +253,155 @@ class TriggerCoreTest {
251253
}
252254
}
253255

256+
@Test
257+
fun `every trigger source has an intended visibility and cooldown decision`() {
258+
val defaults = TriggerEditorSupport.Visibility()
259+
val expected = mapOf(
260+
TriggerSource.TIME_DELAY to TriggerEditorSupport.Visibility(
261+
showDelay = true,
262+
showCooldown = false,
263+
showRunLimit = false,
264+
),
265+
TriggerSource.TIME_ABSOLUTE to TriggerEditorSupport.Visibility(
266+
showAbsoluteTime = true,
267+
showCooldown = false,
268+
showRunLimit = false,
269+
),
270+
TriggerSource.TIME_DAILY to TriggerEditorSupport.Visibility(
271+
showRecurringTime = true,
272+
showCooldown = false,
273+
),
274+
TriggerSource.TIME_WEEKLY to TriggerEditorSupport.Visibility(
275+
showRecurringTime = true,
276+
showCooldown = false,
277+
),
278+
TriggerSource.NOTIFICATION_POSTED to TriggerEditorSupport.Visibility(
279+
showMatchMode = true,
280+
showPackageName = true,
281+
showTitleFilter = true,
282+
showTextFilter = true,
283+
),
284+
TriggerSource.NOTIFICATION_REMOVED to TriggerEditorSupport.Visibility(
285+
showMatchMode = true,
286+
showPackageName = true,
287+
showTitleFilter = true,
288+
showTextFilter = true,
289+
),
290+
TriggerSource.APP_ENTERED to TriggerEditorSupport.Visibility(
291+
showMatchMode = true,
292+
showPackageName = true,
293+
),
294+
TriggerSource.APP_EXITED to TriggerEditorSupport.Visibility(
295+
showMatchMode = true,
296+
showPackageName = true,
297+
),
298+
TriggerSource.BATTERY_LEVEL_CHANGED to TriggerEditorSupport.Visibility(
299+
showThreshold = true,
300+
),
301+
TriggerSource.NETWORK_CONNECTED to TriggerEditorSupport.Visibility(
302+
showNetworkType = true,
303+
),
304+
TriggerSource.NETWORK_TYPE_CHANGED to TriggerEditorSupport.Visibility(
305+
showNetworkType = true,
306+
),
307+
TriggerSource.SMS_RECEIVED to TriggerEditorSupport.Visibility(
308+
showMatchMode = true,
309+
showPhoneNumber = true,
310+
showMessageFilter = true,
311+
),
312+
// Intended-defaults allow-list: these sources deliberately use
313+
// the plain editor defaults.
314+
TriggerSource.BATTERY_LOW to defaults,
315+
TriggerSource.BATTERY_OKAY to defaults,
316+
TriggerSource.POWER_CONNECTED to defaults,
317+
TriggerSource.POWER_DISCONNECTED to defaults,
318+
TriggerSource.USER_PRESENT to defaults,
319+
)
320+
321+
assertEquals(expected.keys, TriggerSource.entries.toSet())
322+
expected.forEach { (source, visibility) ->
323+
assertEquals(source.name, visibility, TriggerEditorSupport.visibilityFor(source))
324+
val capabilities = TriggerEditorSupport.capabilitiesFor(source)
325+
assertEquals(source.name, visibility.showCooldown, capabilities.supportsCooldown)
326+
assertEquals(source.name, visibility.showRunLimit, capabilities.supportsRunLimit)
327+
assertEquals(
328+
source.name,
329+
if (visibility.showCooldown) 60L else 0L,
330+
TriggerEditorSupport.defaultCooldownSecondsFor(source).toLong(),
331+
)
332+
}
333+
}
334+
335+
@Test
336+
fun `portal event mapping decides every event type`() {
337+
val expected = mapOf(
338+
EventType.NOTIFICATION_POSTED to TriggerSource.NOTIFICATION_POSTED,
339+
EventType.NOTIFICATION_REMOVED to TriggerSource.NOTIFICATION_REMOVED,
340+
EventType.APP_ENTERED to TriggerSource.APP_ENTERED,
341+
EventType.APP_EXITED to TriggerSource.APP_EXITED,
342+
EventType.BATTERY_LOW to TriggerSource.BATTERY_LOW,
343+
EventType.BATTERY_OKAY to TriggerSource.BATTERY_OKAY,
344+
EventType.BATTERY_LEVEL_CHANGED to TriggerSource.BATTERY_LEVEL_CHANGED,
345+
EventType.POWER_CONNECTED to TriggerSource.POWER_CONNECTED,
346+
EventType.POWER_DISCONNECTED to TriggerSource.POWER_DISCONNECTED,
347+
EventType.USER_PRESENT to TriggerSource.USER_PRESENT,
348+
EventType.NETWORK_CONNECTED to TriggerSource.NETWORK_CONNECTED,
349+
EventType.NETWORK_TYPE_CHANGED to TriggerSource.NETWORK_TYPE_CHANGED,
350+
EventType.SMS_RECEIVED to TriggerSource.SMS_RECEIVED,
351+
)
352+
val intentionallyUnmapped = setOf(
353+
EventType.NOTIFICATION,
354+
EventType.PING,
355+
EventType.PONG,
356+
EventType.UNKNOWN,
357+
)
358+
359+
assertEquals(expected.keys, EventType.entries.toSet() - intentionallyUnmapped)
360+
expected.forEach { (type, source) ->
361+
assertEquals(type.name, source, TriggerRuntime.triggerSourceFor(type))
362+
}
363+
intentionallyUnmapped.forEach { type ->
364+
assertNull(type.name, TriggerRuntime.triggerSourceFor(type))
365+
}
366+
}
367+
368+
@Test
369+
fun `every trigger source has a notification access and debounce decision`() {
370+
// Pair(requiresNotificationAccess, shouldDebounce) per source.
371+
val expected = mapOf(
372+
TriggerSource.TIME_DELAY to Pair(false, false),
373+
TriggerSource.TIME_ABSOLUTE to Pair(false, false),
374+
TriggerSource.TIME_DAILY to Pair(false, false),
375+
TriggerSource.TIME_WEEKLY to Pair(false, false),
376+
TriggerSource.NOTIFICATION_POSTED to Pair(true, true),
377+
// Removed notifications need listener access but carry no
378+
// message text, so they are never debounced.
379+
TriggerSource.NOTIFICATION_REMOVED to Pair(true, false),
380+
TriggerSource.APP_ENTERED to Pair(false, false),
381+
TriggerSource.APP_EXITED to Pair(false, false),
382+
TriggerSource.BATTERY_LOW to Pair(false, false),
383+
TriggerSource.BATTERY_OKAY to Pair(false, false),
384+
TriggerSource.BATTERY_LEVEL_CHANGED to Pair(false, false),
385+
TriggerSource.POWER_CONNECTED to Pair(false, false),
386+
TriggerSource.POWER_DISCONNECTED to Pair(false, false),
387+
TriggerSource.USER_PRESENT to Pair(false, false),
388+
TriggerSource.NETWORK_CONNECTED to Pair(false, false),
389+
TriggerSource.NETWORK_TYPE_CHANGED to Pair(false, false),
390+
TriggerSource.SMS_RECEIVED to Pair(false, false),
391+
)
392+
393+
assertEquals(expected.keys, TriggerSource.entries.toSet())
394+
expected.forEach { (source, decision) ->
395+
val (needsAccess, debounced) = decision
396+
assertEquals(
397+
source.name,
398+
needsAccess,
399+
TriggerEditorSupport.requiresNotificationAccess(source),
400+
)
401+
assertEquals(source.name, debounced, TriggerRuntime.shouldDebounce(source))
402+
}
403+
}
404+
254405
@Test
255406
fun `editor support clears stale fields for battery triggers`() {
256407
val sanitized = TriggerEditorSupport.sanitize(

0 commit comments

Comments
 (0)