Skip to content

Commit 1b4bcb4

Browse files
SysAdminDocSysAdminDoc
committed
fix: re-read the home region after a SIM unlocks, without a restart
SpamRepository built its canonicalizer once, so a SIM still locked at boot (no country yet) left canonicalization, the numbering plan and the SMS region checks on the network's or the locale's region until the app restarted. The repository now reads through the one-minute region cache the screening path already uses, and the carrier scam-label checker falls back to the region of the call it's checking instead of one fixed when the checker was built. The test fixture resets that cache so each test reads its own shadows.
1 parent 4814361 commit 1b4bcb4

8 files changed

Lines changed: 96 additions & 28 deletions

File tree

‎README.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
<p align="center">
1313
<a href="https://github.com/SysAdminDoc/CallShield/releases/latest"><img src="https://img.shields.io/github/v/release/SysAdminDoc/CallShield?style=flat-square&color=a6e3a1" alt="Release"></a>
1414
<img src="https://img.shields.io/badge/Spam%20Numbers-51%2C362-f38ba8?style=flat-square" alt="51,362 Numbers">
15-
<img src="https://img.shields.io/badge/JVM%20unit%20tests-1887-94e2d5?style=flat-square" alt="1887 JVM unit tests">
15+
<img src="https://img.shields.io/badge/JVM%20unit%20tests-1888-94e2d5?style=flat-square" alt="1888 JVM unit tests">
1616
<img src="https://img.shields.io/badge/Android-10%2B-89b4fa?style=flat-square" alt="Android 10+">
1717
<img src="https://img.shields.io/badge/License-MIT-cba6f7?style=flat-square" alt="MIT License">
1818
<img src="https://img.shields.io/badge/API%20Keys-None-fab387?style=flat-square" alt="No required API keys">
@@ -530,12 +530,12 @@ RELEASE_KEY_PASSWORD=...
530530
## Testing
531531

532532
```bash
533-
./gradlew testDebugUnitTest # 1887 tests
533+
./gradlew testDebugUnitTest # 1888 tests
534534
./gradlew connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.sysadmindoc.callshield.platform.TargetSdkBehaviorSmokeTest
535535
./gradlew verifyPipelineTests # Cloudflare Worker (node) + data-pipeline and translation checks (python)
536536
```
537537

538-
The suite is **1887 total JVM unit tests**, run with Robolectric wherever a screen, a service or the database is involved.
538+
The suite is **1888 total JVM unit tests**, run with Robolectric wherever a screen, a service or the database is involved.
539539

540540
Two GitHub workflows run without building the app. **Validation** runs the Worker and
541541
pipeline suites on every push except report-only ones (`run-pipeline-tests.ps1 -CorrectnessOnly`),
@@ -572,7 +572,7 @@ language in [issue #7](https://github.com/SysAdminDoc/CallShield/issues/7).
572572
| Community API | Cloudflare Workers |
573573
| URL Safety | Local spam-domain data, with optional PhishTank and OpenPhish |
574574
| Verification | Local Gradle, lint, and release-artifact checks |
575-
| Tests | 1887 JVM unit tests (JUnit) |
575+
| Tests | 1888 JVM unit tests (JUnit) |
576576
| Strings | 1766 string resources and 38 plural groups (translation-ready) |
577577
| Accessibility | 100+ content descriptions, 48dp touch targets |
578578
| Min SDK | 29 (Android 10) |

‎app/src/main/java/com/sysadmindoc/callshield/data/SpamRepository.kt‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,18 @@ class SpamRepository(
8989
checkerDependencies: CheckerDependencies = CheckerDependencies(),
9090
settingsDataStore: DataStore<Preferences>? = null,
9191
privateSettingsDataStore: DataStore<Preferences>? = null,
92-
private val phoneIdentityCanonicalizer: PhoneIdentityCanonicalizer =
93-
PhoneIdentityCanonicalizer.fromContext(context.applicationContext),
92+
/** A fixed canonicalizer, for tests; null follows the phone's current region. */
93+
phoneIdentityCanonicalizer: PhoneIdentityCanonicalizer? = null,
9494
externalBlocklistDataSource: ExternalBlocklistDataSource = OkHttpExternalBlocklistDataSource(),
9595
wallClock: () -> Long = System::currentTimeMillis,
9696
) {
9797
private val appContext: Context = context.applicationContext
98+
99+
// Read through the short-lived region cache, not once per process: a SIM
100+
// still locked at boot reports no country, and the network's or the
101+
// locale's region would otherwise stand in until the app restarted.
102+
private val canonicalizer: () -> PhoneIdentityCanonicalizer =
103+
phoneIdentityCanonicalizer?.let { fixed -> { fixed } } ?: { PhoneIdentityCanonicalizer.cachedFromContext(appContext) }
98104
private val db: AppDatabase = database
99105
private val dao: SpamDao = database.spamDao()
100106
private val settingsRepository =
@@ -109,10 +115,10 @@ class SpamRepository(
109115
dao = dao,
110116
settingsRepository = settingsRepository,
111117
checkerDependencies = checkerDependencies,
112-
normalizePhone = phoneIdentityCanonicalizer::canonicalizePhone,
113-
normalizeSenderIdentity = phoneIdentityCanonicalizer::canonicalizeIdentity,
114-
senderRegionIso = phoneIdentityCanonicalizer.homeRegionIso,
115-
equivalentForms = phoneIdentityCanonicalizer::equivalentForms,
118+
normalizePhone = { canonicalizer().canonicalizePhone(it) },
119+
normalizeSenderIdentity = { canonicalizer().canonicalizeIdentity(it) },
120+
senderRegionIso = { canonicalizer().homeRegionIso },
121+
equivalentForms = { canonicalizer().equivalentForms(it) },
116122
wallClock = wallClock,
117123
)
118124
private val syncRepository =
@@ -121,7 +127,7 @@ class SpamRepository(
121127
dao = dao,
122128
remote = remote,
123129
settingsRepository = settingsRepository,
124-
normalizeNumber = phoneIdentityCanonicalizer::canonicalizePhone,
130+
normalizeNumber = { canonicalizer().canonicalizePhone(it) },
125131
invalidateAllCaches = spamRepositoryImpl::invalidateAllCaches,
126132
externalBlocklistDataSource = externalBlocklistDataSource,
127133
clock = wallClock,
@@ -131,9 +137,9 @@ class SpamRepository(
131137
context = appContext,
132138
dao = dao,
133139
settingsRepository = settingsRepository,
134-
normalizeNumber = phoneIdentityCanonicalizer::canonicalizePhone,
135-
normalizeLogIdentity = phoneIdentityCanonicalizer::canonicalizeIdentity,
136-
equivalentForms = phoneIdentityCanonicalizer::equivalentForms,
140+
normalizeNumber = { canonicalizer().canonicalizePhone(it) },
141+
normalizeLogIdentity = { canonicalizer().canonicalizeIdentity(it) },
142+
equivalentForms = { canonicalizer().equivalentForms(it) },
137143
invalidateWildcardCache = spamRepositoryImpl::invalidateWildcardCache,
138144
invalidateKeywordCache = spamRepositoryImpl::invalidateKeywordCache,
139145
invalidateHashWildcardCache = spamRepositoryImpl::invalidateHashWildcardCache,
@@ -695,7 +701,7 @@ class SpamRepository(
695701
internal suspend fun findExactSpamNumber(normalized: String) = spamRepositoryImpl.findByNumberInternal(normalized)
696702

697703
/** Forms of a normalized number to match stored rows against, as the screening pipeline does. */
698-
internal fun lookupForms(normalized: String): List<String> = phoneIdentityCanonicalizer.equivalentForms(normalized)
704+
internal fun lookupForms(normalized: String): List<String> = canonicalizer().equivalentForms(normalized)
699705

700706
/** When [normalized] last appeared in a text CallShield flagged, within the last 30 days, or null. */
701707
internal suspend fun lastFlaggedTextSighting(
@@ -1219,9 +1225,9 @@ class SpamRepository(
12191225
// ── Search log ───────────────────────────────────────────────────
12201226
fun searchLog(query: String): Flow<List<BlockedCall>> = blocklistRepository.searchLog(query)
12211227

1222-
fun normalizeNumber(number: String): String = phoneIdentityCanonicalizer.canonicalizePhone(number)
1228+
fun normalizeNumber(number: String): String = canonicalizer().canonicalizePhone(number)
12231229

1224-
fun normalizeSenderIdentity(sender: String): String = phoneIdentityCanonicalizer.canonicalizeIdentity(sender)
1230+
fun normalizeSenderIdentity(sender: String): String = canonicalizer().canonicalizeIdentity(sender)
12251231
}
12261232

12271233
/**

‎app/src/main/java/com/sysadmindoc/callshield/data/checker/Checkers.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,6 @@ internal class SmsContextTrustChecker : IChecker {
12161216
* to [homeRegionIso].
12171217
*/
12181218
internal class CarrierScamLabelChecker(
1219-
private val homeRegionIso: String?,
12201219
private val receivingSimCountry: (Context, Int?) -> String? = ::receivingSimCountryIso,
12211220
) : IChecker {
12221221
override val priority = CheckerPriority.SMS_CARRIER_LABEL
@@ -1227,7 +1226,7 @@ internal class CarrierScamLabelChecker(
12271226
override suspend fun check(ctx: CheckContext): BlockResult? {
12281227
val label =
12291228
carrierScamLabel(ctx.number) {
1230-
receivingSimCountry(ctx.appContext, ctx.subscriptionId)?.takeIf { it.isNotBlank() } ?: homeRegionIso
1229+
receivingSimCountry(ctx.appContext, ctx.subscriptionId)?.takeIf { it.isNotBlank() } ?: ctx.homeRegionIso
12311230
} ?: return null
12321231
return BlockResult.block(
12331232
matchSource = "carrier_label",

‎app/src/main/java/com/sysadmindoc/callshield/data/checker/IChecker.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,10 +529,9 @@ object SpamCheckers {
529529
repo: SpamRepositoryImpl,
530530
appContext: Context,
531531
dependencies: CheckerDependencies = CheckerDependencies(),
532-
homeRegionIso: String? = null,
533532
): List<IChecker> =
534533
buildList {
535-
add(CarrierScamLabelChecker(homeRegionIso, dependencies.receivingSimCountry))
534+
add(CarrierScamLabelChecker(dependencies.receivingSimCountry))
536535
add(SmsContextTrustChecker())
537536
add(SmsBurstChecker(appContext, dependencies.smsContextChecker))
538537
add(SmsKeywordChecker(repo))

‎app/src/main/java/com/sysadmindoc/callshield/data/repository/SpamRepositoryImpl.kt‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ class SpamRepositoryImpl(
4040
private val normalizePhone: (String) -> String,
4141
private val normalizeSenderIdentity: (String) -> String,
4242
private val senderProvenanceResolver: SenderProvenanceResolver = SenderProvenanceResolver(),
43-
private val senderRegionIso: String? = null,
43+
/** The phone's home region now; it can change while the process runs. */
44+
private val senderRegionIso: () -> String? = { null },
4445
/** Every stored spelling of a canonical number, canonical first. */
4546
private val equivalentForms: (String) -> List<String> = { listOf(it) },
4647
private val wallClock: () -> Long = System::currentTimeMillis,
@@ -229,7 +230,7 @@ class SpamRepositoryImpl(
229230
SpamCheckers.buildCallChain(this, context, checkerDependencies)
230231
}
231232
private val smsExtensions: List<IChecker> by lazy {
232-
SpamCheckers.buildSmsExtensions(this, context, checkerDependencies, senderRegionIso)
233+
SpamCheckers.buildSmsExtensions(this, context, checkerDependencies)
233234
}
234235

235236
suspend fun isSpam(
@@ -258,7 +259,7 @@ class SpamRepositoryImpl(
258259
smsContextTrusted = smsContextTrusted,
259260
senderProvenance = senderProvenance,
260261
alternateForms = equivalentForms(normalized).drop(1),
261-
homeRegionIso = senderRegionIso,
262+
homeRegionIso = senderRegionIso(),
262263
)
263264

264265
val pipelineRun = CheckerPipeline.runWithDiagnostics(callChain, ctx)
@@ -280,7 +281,7 @@ class SpamRepositoryImpl(
280281
subscriptionId: Int? = null,
281282
): SpamCheckResult {
282283
val prefs = prefsSnapshot ?: settingsRepository.readPrefsSnapshot()
283-
val senderProvenance = senderProvenanceResolver.resolve(number, senderRegionIso)
284+
val senderProvenance = senderProvenanceResolver.resolve(number, senderRegionIso())
284285
val canonicalPhone = normalizePhone(number)
285286
val smsContextTrusted =
286287
canonicalPhone.isNotBlank() &&
@@ -336,7 +337,7 @@ class SpamRepositoryImpl(
336337
smsContextTrusted = smsContextTrusted,
337338
smsFirstContact = smsFirstContact,
338339
senderProvenance = senderProvenance,
339-
homeRegionIso = senderRegionIso,
340+
homeRegionIso = senderRegionIso(),
340341
subscriptionId = subscriptionId,
341342
)
342343
val pipelineRun = CheckerPipeline.runWithDiagnostics(smsExtensions, ctx)
@@ -367,7 +368,7 @@ class SpamRepositoryImpl(
367368
realtimeCall = false,
368369
prefs = prefs,
369370
alternateForms = equivalentForms(normalized).drop(1),
370-
homeRegionIso = senderRegionIso,
371+
homeRegionIso = senderRegionIso(),
371372
)
372373
return CheckerPipeline.traceAll(callChain, ctx)
373374
}

‎app/src/test/java/com/sysadmindoc/callshield/data/HomeRegionRefreshTest.kt‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package com.sysadmindoc.callshield.data
2+
3+
import android.content.Context
4+
import android.telephony.TelephonyManager
5+
import androidx.datastore.preferences.core.edit
6+
import androidx.test.core.app.ApplicationProvider
7+
import kotlinx.coroutines.runBlocking
8+
import org.junit.After
9+
import org.junit.Assert.assertEquals
10+
import org.junit.Assert.assertFalse
11+
import org.junit.Test
12+
import org.junit.runner.RunWith
13+
import org.robolectric.RobolectricTestRunner
14+
import org.robolectric.Shadows.shadowOf
15+
import org.robolectric.annotation.Config
16+
17+
/**
18+
* A SIM still locked at boot reports no country, so the phone's region comes
19+
* from the locale until the SIM unlocks. The repository used to keep that
20+
* first reading for the life of the process.
21+
*/
22+
@RunWith(RobolectricTestRunner::class)
23+
@Config(sdk = [34])
24+
class HomeRegionRefreshTest {
25+
private val context: Context = ApplicationProvider.getApplicationContext()
26+
private val telephony = shadowOf(context.getSystemService(TelephonyManager::class.java))
27+
28+
@After
29+
fun tearDown() {
30+
SpamHeuristics.updateHotRanges(emptyList())
31+
telephony.setSimCountryIso("")
32+
PhoneIdentityCanonicalizer.resetCacheForTests()
33+
}
34+
35+
@Test
36+
fun `a SIM that unlocks after startup sets the region without a restart`() {
37+
telephony.setSimCountryIso("")
38+
telephony.setNetworkCountryIso("")
39+
// A VoIP range plus a live campaign range clears the heuristic bar for a North American number.
40+
SpamHeuristics.updateHotRanges(listOf("202555"))
41+
42+
IsolatedRepositoryFixture(context).use { fixture ->
43+
runBlocking { fixture.settingsStore.edit { it[SpamRepository.KEY_ML_SCORER] = false } }
44+
assertEquals("+12025551234", fixture.repository.normalizeNumber("2025551234"))
45+
assertEquals("heuristic", runBlocking { fixture.repository.isSpam("2025551234") }.matchSource)
46+
47+
telephony.setSimCountryIso("gb")
48+
// Stands in for the region cache's minute running out.
49+
PhoneIdentityCanonicalizer.resetCacheForTests()
50+
51+
assertEquals("+442079460123", fixture.repository.normalizeNumber("02079460123"))
52+
// Another number in the same range, so no earlier verdict can answer.
53+
assertFalse(runBlocking { fixture.repository.isSpam("2025551299") }.isSpam)
54+
}
55+
}
56+
}

‎app/src/test/java/com/sysadmindoc/callshield/data/IsolatedRepositoryFixture.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ internal class IsolatedRepositoryFixture(
2626
wallClock: () -> Long = System::currentTimeMillis,
2727
checkerDependencies: CheckerDependencies = CheckerDependencies(),
2828
) : AutoCloseable {
29+
init {
30+
// The repository reads the region through a process-wide cache, so a
31+
// fixture starts from what this test's telephony shadows say.
32+
PhoneIdentityCanonicalizer.resetCacheForTests()
33+
}
34+
2935
private val storeJob = SupervisorJob()
3036
private val storeScope = CoroutineScope(storeJob + Dispatchers.IO)
3137
private val storeDirectory = Files.createTempDirectory("callshield-test-datastore-").toFile()

‎app/src/test/java/com/sysadmindoc/callshield/data/checker/CarrierScamLabelCheckerTest.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class CarrierScamLabelCheckerTest {
9393
var simCountry = ""
9494
var reads = 0
9595
val checker =
96-
CarrierScamLabelChecker(homeRegionIso = "US") { _, _ ->
96+
CarrierScamLabelChecker { _, _ ->
9797
reads++
9898
simCountry
9999
}
@@ -105,6 +105,7 @@ class CarrierScamLabelCheckerTest {
105105
realtimeCall = true,
106106
prefs = emptyPreferences(),
107107
subscriptionId = 2,
108+
homeRegionIso = "US",
108109
)
109110

110111
assertNull("a locked SIM reads empty and the region falls back", checker.check(text))

0 commit comments

Comments
 (0)