From a7a5b6413703cf984d57a28ee9715d2949c53083 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Tue, 22 Sep 2026 13:05:04 +0200 Subject: [PATCH] feat(zones): ICU behind a seam, and one directory that remembers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ZoneNames` is the seam; `IcuZoneNames` is the only file in the app allowed to name `android.icu`, and a build rule keeps it that way. Every read is guarded — ICU returning blank or throwing gives back null rather than a half-written city. `ZoneDirectory` caches the catalog, the entries and the display names, all keyed on the locale tag, so a per-app language change re-resolves every name instead of leaving the cities in the old language under a freshly translated zone name. It answers in batches, so a tab with two dozen cities costs two dispatches a tick rather than two dozen. --- .../clockula/data/di/ZoneModule.kt | 15 ++ .../clockula/data/time/AndroidClocks.kt | 3 + .../clockula/data/zones/IcuZoneNames.kt | 67 +++++ .../clockula/data/zones/ZoneDirectory.kt | 144 +++++++++++ .../clockula/data/zones/ZoneNames.kt | 24 ++ .../clockula/data/zones/ZoneDirectoryTest.kt | 236 ++++++++++++++++++ .../clockula/testing/FakeZoneNames.kt | 73 ++++++ 7 files changed, 562 insertions(+) create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/di/ZoneModule.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/zones/IcuZoneNames.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectory.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneNames.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectoryTest.kt create mode 100644 app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeZoneNames.kt diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/di/ZoneModule.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/di/ZoneModule.kt new file mode 100644 index 0000000..6b6daeb --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/di/ZoneModule.kt @@ -0,0 +1,15 @@ +package de.jeanlucmakiola.clockula.data.di + +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent +import de.jeanlucmakiola.clockula.data.zones.IcuZoneNames +import de.jeanlucmakiola.clockula.data.zones.ZoneNames + +@Module +@InstallIn(SingletonComponent::class) +abstract class ZoneModule { + @Binds + abstract fun bindZoneNames(impl: IcuZoneNames): ZoneNames +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/time/AndroidClocks.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/time/AndroidClocks.kt index bd8c880..99b2e63 100644 --- a/app/src/main/java/de/jeanlucmakiola/clockula/data/time/AndroidClocks.kt +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/time/AndroidClocks.kt @@ -28,4 +28,7 @@ class SystemElapsedRealtimeClock @Inject constructor() : ElapsedRealtimeClock { @Singleton class SystemZoneProvider @Inject constructor() : ZoneProvider { override fun current(): java.time.ZoneId = java.time.ZoneId.systemDefault() + + /** The device's own tzdata, so the picker offers the zones this build knows. */ + override fun available(): Set = java.time.ZoneId.getAvailableZoneIds() } diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/IcuZoneNames.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/IcuZoneNames.kt new file mode 100644 index 0000000..cd69de8 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/IcuZoneNames.kt @@ -0,0 +1,67 @@ +package de.jeanlucmakiola.clockula.data.zones + +import android.icu.text.TimeZoneNames +import android.icu.util.ULocale +import java.util.Date +import kotlin.time.Instant +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The only file in the app that may name `android.icu` — a build rule pins it + * (`ArchitectureRulesTest`). Every call is guarded, so an OEM build with a + * broken or trimmed ICU degrades a caption rather than taking the tab down. + * + * Deliberately not unit-tested: `android.icu` does not exist on the JVM test + * classpath, and everything above it is asserted over `FakeZoneNames`. + */ +@Singleton +class IcuZoneNames @Inject constructor() : ZoneNames { + + /** ICU's own "worldwide" region — not a country anybody would recognise. */ + private val worldwide = "001" + + override fun localeTag(): String = guarded { ULocale.getDefault().toLanguageTag() }.orEmpty() + + override fun canonicalId(zoneId: String): String = + guarded { android.icu.util.TimeZone.getCanonicalID(zoneId) } ?: zoneId + + override fun cityOf(zoneId: String): String? = guarded { + TimeZoneNames.getInstance(ULocale.getDefault()).getExemplarLocationName(zoneId) + } + + override fun countryOf(zoneId: String): String? = guarded { + val region = android.icu.util.TimeZone.getRegion(zoneId) + if (region == null || region == worldwide) { + null + } else { + ULocale("und-$region").getDisplayCountry(ULocale.getDefault()) + } + } + + /** + * The metazone recipe: a zone's long name is its metazone's name in the + * phase it is actually in ("Central European Summer Time"), falling back to + * the generic name when ICU has no metazone for it. + */ + override fun displayNameOf(zoneId: String, at: Instant): String? = guarded { + val locale = ULocale.getDefault() + val zone = android.icu.util.TimeZone.getTimeZone(zoneId) + val millis = at.toEpochMilliseconds() + val daylight = zone.inDaylightTime(Date(millis)) + val names = TimeZoneNames.getInstance(locale) + val metaZoneId = names.getMetaZoneID(zoneId, millis) + val style = if (daylight) { + TimeZoneNames.NameType.LONG_DAYLIGHT + } else { + TimeZoneNames.NameType.LONG_STANDARD + } + metaZoneId + ?.let { names.getMetaZoneDisplayName(it, style) } + ?.takeIf { it.isNotBlank() } + ?: zone.getDisplayName(daylight, android.icu.util.TimeZone.LONG_GENERIC, locale) + } + + private fun guarded(call: () -> String?): String? = + runCatching(call).getOrNull()?.takeIf { it.isNotBlank() } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectory.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectory.kt new file mode 100644 index 0000000..2bd5f17 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectory.kt @@ -0,0 +1,144 @@ +package de.jeanlucmakiola.clockula.data.zones + +import de.jeanlucmakiola.clockula.domain.time.ZoneProvider +import de.jeanlucmakiola.clockula.domain.worldclock.ZoneCatalog +import de.jeanlucmakiola.clockula.domain.worldclock.ZoneEntry +import de.jeanlucmakiola.floret.di.IoDispatcher +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import java.time.ZoneId +import kotlin.time.Instant +import kotlin.time.toJavaInstant +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The locale-keyed zone catalog and the DST-keyed display-name memo — the one + * place [ZoneNames] is called from. + * + * Runs on [io] because ~450 ids times three ICU calls is not main-thread work + * and ICU dispatches nothing of its own: the second documented + * `@IoDispatcher` exception after `SystemRingtoneCatalog`. Cached on the + * locale tag, because AppCompat's per-app language can change under a running + * activity and a catalog built in the old language would keep showing it. Built + * **lazily**: the tab resolves one entry per stored clock through [entryFor], + * and three rows must not cost 450 ICU calls. + */ +@Singleton +class ZoneDirectory @Inject constructor( + private val names: ZoneNames, + private val zones: ZoneProvider, + @param:IoDispatcher private val io: CoroutineDispatcher, +) { + + private data class NameKey(val localeTag: String, val zoneId: String, val daylight: Boolean) + + private val catalogLock = Mutex() + private val entryLock = Mutex() + private val nameLock = Mutex() + + private var catalogLocaleTag: String? = null + private var catalog: List = emptyList() + + private var entryLocaleTag: String? = null + private val entries = mutableMapOf() + + private val displayNames = mutableMapOf() + + /** + * Every pickable zone, city-sorted. Built once per locale under one + * `Mutex`, so two collectors opening the picker at once build once. Never + * throws; empty only if the device's tzdata is. + */ + suspend fun entries(): List = withContext(io) { + val localeTag = localeTag() + catalogLock.withLock { + if (catalogLocaleTag != localeTag) { + catalog = build() + catalogLocaleTag = localeTag + } + catalog + } + } + + /** + * The entry for [zoneId], whether or not it is pickable — an alias, or a + * zone the device's tzdata has dropped. Never throws. + */ + suspend fun entryFor(zoneId: String): ZoneEntry = entriesFor(listOf(zoneId)).getValue(zoneId) + + /** + * One entry per id in [zoneIds], memoised on the locale tag. The memo lives + * here rather than in a caller so a per-app language change re-resolves the + * cities: a cache above this seam would keep showing the old language until + * the process died. Batched because a per-tick caller must pay one dispatch, + * not one per row. + */ + suspend fun entriesFor(zoneIds: List): Map = withContext(io) { + val localeTag = localeTag() + entryLock.withLock { + if (entryLocaleTag != localeTag) { + entries.clear() + entryLocaleTag = localeTag + } + zoneIds.associateWith { id -> entries.getOrPut(id) { entry(id) } } + } + } + + /** + * ICU's long zone name at [at], memoised on `(locale, zoneId, DST phase)`, + * so a per-tick caller costs a map lookup — and a tab left open across a + * transition picks "Summer Time" up on the next tick instead of going stale + * until the process restarts. Null when ICU has none. + */ + suspend fun displayNameOf(zoneId: String, at: Instant): String? = + displayNamesOf(listOf(zoneId), at)[zoneId] + + /** + * One display name per id in [zoneIds], on the same memo. Batched for the + * same reason [entriesFor] is: the tab asks five times a second, and a + * dispatch and a lock per row would be a hundred and twenty-five of each. + */ + suspend fun displayNamesOf(zoneIds: List, at: Instant): Map = + withContext(io) { + val localeTag = localeTag() + nameLock.withLock { + zoneIds.associateWith { zoneId -> + val key = NameKey(localeTag, zoneId, isDaylight(zoneId, at)) + if (key !in displayNames) { + displayNames[key] = guarded { names.displayNameOf(zoneId, at) } + } + displayNames[key] + } + } + } + + private fun build(): List { + val ids = ZoneCatalog.pickableIds(zones.available()) { id -> + guarded { names.canonicalId(id) } ?: id + } + return ZoneCatalog.sorted(ids.map(::entry)) + } + + private fun entry(zoneId: String): ZoneEntry = ZoneEntry( + zoneId = zoneId, + city = guarded { names.cityOf(zoneId) } ?: ZoneCatalog.cityFallbackFor(zoneId), + country = guarded { names.countryOf(zoneId) }, + ) + + private fun localeTag(): String = guarded { names.localeTag() }.orEmpty() + + private fun isDaylight(zoneId: String, at: Instant): Boolean = + runCatching { ZoneId.of(zoneId).rules.isDaylightSavings(at.toJavaInstant()) } + .getOrDefault(false) + + /** + * ICU on an unusual OEM build must degrade a caption, not crash a tab — so + * every call across the seam comes back as a value or as null, and a blank + * answer reads as no answer at all. + */ + private fun guarded(call: () -> String?): String? = + runCatching(call).getOrNull()?.takeIf { it.isNotBlank() } +} diff --git a/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneNames.kt b/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneNames.kt new file mode 100644 index 0000000..e3d9aad --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/clockula/data/zones/ZoneNames.kt @@ -0,0 +1,24 @@ +package de.jeanlucmakiola.clockula.data.zones + +import kotlin.time.Instant + +/** + * The ICU seam. Every method is **total and never throws**: ICU on an unusual + * OEM build must degrade a caption, not crash a tab. + */ +interface ZoneNames { + /** The locale ICU is currently resolving names in, as a language tag. */ + fun localeTag(): String + + /** The tzdata's canonical id for [zoneId]; [zoneId] itself when ICU cannot say. */ + fun canonicalId(zoneId: String): String + + /** ICU's localised exemplar city for [zoneId]; null when ICU has none. */ + fun cityOf(zoneId: String): String? + + /** The localised country name for [zoneId]'s region; null when unknown or worldwide. */ + fun countryOf(zoneId: String): String? + + /** ICU's long display name at [at]; null when ICU has none. */ + fun displayNameOf(zoneId: String, at: Instant): String? +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectoryTest.kt b/app/src/test/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectoryTest.kt new file mode 100644 index 0000000..e79fdeb --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/data/zones/ZoneDirectoryTest.kt @@ -0,0 +1,236 @@ +package de.jeanlucmakiola.clockula.data.zones + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.clockula.domain.worldclock.ZoneCatalog +import de.jeanlucmakiola.clockula.testing.FakeZoneNames +import de.jeanlucmakiola.clockula.testing.FakeZoneProvider +import de.jeanlucmakiola.clockula.testing.T0 +import kotlinx.coroutines.async +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.hours +import kotlin.time.Instant + +/** + * §5.6 — 14 cases. The catalog is built once per locale behind one `Mutex` + * (D4), the display name is memoised on the DST phase so a tab open across a + * transition picks the new name up, and an ICU failure degrades a caption + * rather than crashing a tab (D3). + */ +class ZoneDirectoryTest { + + private val berlin = "Europe/Berlin" + private val saoPaulo = "America/Sao_Paulo" + private val zurich = "Europe/Zurich" + + /** 2023-07-15T12:00:00Z — Berlin on summer time. */ + private val summer: Instant = Instant.fromEpochMilliseconds(1_689_422_400_000L) + + /** 2023-11-14T22:13:20Z — Berlin back on standard time. */ + private val winter: Instant = T0 + + private fun names() = FakeZoneNames( + cities = mapOf(berlin to "Berlin", saoPaulo to "São Paulo", zurich to "Zürich"), + countries = mapOf(berlin to "Germany", saoPaulo to "Brazil", zurich to "Switzerland"), + ) + + private fun zones(vararg ids: String) = + FakeZoneProvider(availableIds = ids.toSet()) + + private fun directory( + names: FakeZoneNames, + zones: FakeZoneProvider, + dispatcher: TestDispatcher, + ) = ZoneDirectory(names, zones, dispatcher) + + /** §5.6 #1 */ + @Test + fun `every pickable id comes back named by ICU`() = runTest { + val names = names() + val directory = + directory(names, zones(berlin, saoPaulo, zurich), UnconfinedTestDispatcher(testScheduler)) + + val entries = directory.entries() + + assertThat(entries.map { it.zoneId }) + .containsExactly(berlin, saoPaulo, zurich) + assertThat(entries.first { it.zoneId == berlin }.city).isEqualTo("Berlin") + } + + /** §5.6 #2 */ + @Test + fun `an id ICU cannot name falls back to its last segment and no country`() = runTest { + val names = FakeZoneNames() + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + + val entries = directory.entries() + + assertThat(entries.single().city).isEqualTo(ZoneCatalog.cityFallbackFor(berlin)) + assertThat(entries.single().country).isNull() + } + + /** §5.6 #3 */ + @Test + fun `the catalog comes back city-sorted`() = runTest { + val directory = + directory(names(), zones(zurich, saoPaulo, berlin), UnconfinedTestDispatcher(testScheduler)) + + val entries = directory.entries() + + assertThat(entries.map { it.city }) + .containsExactly("Berlin", "São Paulo", "Zürich").inOrder() + } + + /** §5.6 #4 */ + @Test + fun `the catalog is built once, not once per caller`() = runTest { + val names = names() + val directory = + directory(names, zones(berlin, saoPaulo, zurich), UnconfinedTestDispatcher(testScheduler)) + directory.entries() + val afterFirst = names.cityCalls + + directory.entries() + + assertThat(names.cityCalls).isEqualTo(afterFirst) + } + + /** §5.6 #5 */ + @Test + fun `a language change rebuilds the catalog in the new language`() = runTest { + val names = names() + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + directory.entries() + val afterFirst = names.cityCalls + + names.localeTag = "de" + names.cities = mapOf(berlin to "Berlin (de)") + val entries = directory.entries() + + assertThat(names.cityCalls).isGreaterThan(afterFirst) + assertThat(entries.single().city).isEqualTo("Berlin (de)") + } + + /** §5.6 #6 */ + @Test + fun `two collectors opening the picker at once build the catalog once`() = runTest { + val soloNames = names() + directory(soloNames, zones(berlin, saoPaulo, zurich), UnconfinedTestDispatcher(testScheduler)) + .entries() + val names = names() + val directory = + directory(names, zones(berlin, saoPaulo, zurich), UnconfinedTestDispatcher(testScheduler)) + + val first = async { directory.entries() } + val second = async { directory.entries() } + first.await() + second.await() + + assertThat(names.cityCalls).isEqualTo(soloNames.cityCalls) + } + + /** §5.6 #7 */ + @Test + fun `an alias the catalog drops still resolves to an entry`() = runTest { + val names = FakeZoneNames(cities = mapOf("Asia/Calcutta" to "Kolkata")) + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + + val entry = directory.entryFor("Asia/Calcutta") + + assertThat(entry.zoneId).isEqualTo("Asia/Calcutta") + assertThat(entry.city).isEqualTo("Kolkata") + } + + /** §5.6 #8 */ + @Test + fun `a zone the device no longer knows still resolves to an entry`() = runTest { + val directory = directory(names(), zones(berlin), UnconfinedTestDispatcher(testScheduler)) + + val entry = directory.entryFor("Mars/Olympus") + + assertThat(entry.city).isEqualTo("Olympus") + assertThat(entry.country).isNull() + } + + /** §5.6 #9 */ + @Test + fun `an ICU that throws on everything degrades the catalog rather than the tab`() = runTest { + val names = names().apply { failEverything = true } + val directory = + directory(names, zones(berlin, saoPaulo), UnconfinedTestDispatcher(testScheduler)) + + val entries = directory.entries() + + assertThat(entries.map { it.city }) + .containsExactly(ZoneCatalog.cityFallbackFor(berlin), ZoneCatalog.cityFallbackFor(saoPaulo)) + assertThat(entries.map { it.country }).containsExactly(null, null) + } + + /** §5.6 #10 */ + @Test + fun `an ICU that throws yields no display name rather than an error`() = runTest { + val names = names().apply { failEverything = true } + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + + val name = directory.displayNameOf(berlin, winter) + + assertThat(name).isNull() + } + + /** §5.6 #11 */ + @Test + fun `the display name is asked for once per DST phase`() = runTest { + val names = names().apply { displayNames = mapOf(berlin to "Central European Standard Time") } + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + + directory.displayNameOf(berlin, winter) + directory.displayNameOf(berlin, winter + 1.hours) + + assertThat(names.displayNameCalls).isEqualTo(1) + } + + /** §5.6 #12 */ + @Test + fun `a tab open across a transition asks again in the new phase`() = runTest { + val names = names().apply { displayNames = mapOf(berlin to "Central European Time") } + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + + directory.displayNameOf(berlin, summer) + directory.displayNameOf(berlin, winter) + + assertThat(names.displayNameCalls).isEqualTo(2) + } + + /** + * §5.6 #13 — the entry memo is keyed on the locale too: a per-app language + * change must re-resolve the cities the tab shows, not only the catalog the + * picker shows. + */ + @Test + fun `a language change re-resolves an entry in the new language`() = runTest { + val names = names() + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + assertThat(directory.entryFor(berlin).city).isEqualTo("Berlin") + + names.localeTag = "de" + names.cities = mapOf(berlin to "Berlin (de)") + + assertThat(directory.entryFor(berlin).city).isEqualTo("Berlin (de)") + } + + /** §5.6 #14 — and in the same language it is asked for once, not once per tick. */ + @Test + fun `an entry is resolved once per locale, not once per caller`() = runTest { + val names = names() + val directory = directory(names, zones(berlin), UnconfinedTestDispatcher(testScheduler)) + directory.entryFor(berlin) + val afterFirst = names.cityCalls + + directory.entryFor(berlin) + directory.entriesFor(listOf(berlin, berlin)) + + assertThat(names.cityCalls).isEqualTo(afterFirst) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeZoneNames.kt b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeZoneNames.kt new file mode 100644 index 0000000..bb8807c --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/clockula/testing/FakeZoneNames.kt @@ -0,0 +1,73 @@ +package de.jeanlucmakiola.clockula.testing + +import de.jeanlucmakiola.clockula.data.zones.ZoneNames +import kotlin.time.Instant + +/** + * A [ZoneNames] a test writes by hand, counts calls on, and can make throw. + * The counters are what makes "the catalog is built once" assertable; the + * [failEverything] flag is what makes "the directory never propagates an ICU + * failure" assertable (M8 D23). + */ +class FakeZoneNames( + var localeTag: String = "en", + var canonical: Map = emptyMap(), + var cities: Map = emptyMap(), + var countries: Map = emptyMap(), + var displayNames: Map = emptyMap(), +) : ZoneNames { + + var failEverything: Boolean = false + + var canonicalCalls: Int = 0 + private set + + var cityCalls: Int = 0 + private set + + var countryCalls: Int = 0 + private set + + var displayNameCalls: Int = 0 + private set + + override fun localeTag(): String { + failIfAsked() + return localeTag + } + + override fun canonicalId(zoneId: String): String { + canonicalCalls++ + failIfAsked() + return canonical[zoneId] ?: zoneId + } + + override fun cityOf(zoneId: String): String? { + cityCalls++ + failIfAsked() + return cities[zoneId] + } + + override fun countryOf(zoneId: String): String? { + countryCalls++ + failIfAsked() + return countries[zoneId] + } + + override fun displayNameOf(zoneId: String, at: Instant): String? { + displayNameCalls++ + failIfAsked() + return displayNames[zoneId] + } + + fun resetCounts() { + canonicalCalls = 0 + cityCalls = 0 + countryCalls = 0 + displayNameCalls = 0 + } + + private fun failIfAsked() { + if (failEverything) error("ICU is unavailable on this build") + } +}