Skip to content

Commit 7768a95

Browse files
committed
Common: TimeZone.SYSTEM -> TimeZone.currentSystemDefault()
In addition to the renaming, caching of the system timezone was disabled. Before, due to querying the timezone being an expensive operation (especially on Native), we acquired the system timezone only once. The main motivation behind it was that having an expensive operation on a `val` was against the expectations of users, and most JRE implementations do not update the system timezone. However, it turned out that at least on Android the changes to system timezone are tracked by the Java runtime, so it is not viable to just assume that the system timezone won't change. Thus, we decided to perform the renaming and thus communitate that the operation may be an expensive one.
1 parent ce197e5 commit 7768a95

File tree

9 files changed

+67
-22
lines changed

9 files changed

+67
-22
lines changed

core/commonMain/src/TimeZone.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public expect open class TimeZone {
1313
* @throws RuntimeException if the name of the system time-zone is invalid or the time-zone specified as the
1414
* system one cannot be found.
1515
*/
16-
val SYSTEM: TimeZone
16+
fun currentSystemDefault(): TimeZone
1717
val UTC: TimeZone
1818

1919
/**

core/commonTest/src/InstantTest.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class InstantTest {
5050
fun instantToLocalDTConversion() {
5151
val now = Clock.System.now()
5252
println(now.toLocalDateTime(TimeZone.UTC))
53-
println(now.toLocalDateTime(TimeZone.SYSTEM))
53+
println(now.toLocalDateTime(TimeZone.currentSystemDefault()))
5454
}
5555

5656
/* Based on the ThreeTenBp project.
@@ -164,8 +164,8 @@ class InstantTest {
164164
val instant1 = Instant.fromEpochMilliseconds(millis1)
165165
val instant2 = Instant.fromEpochMilliseconds(millis2)
166166

167-
val diff = instant1.periodUntil(instant2, TimeZone.SYSTEM)
168-
val instant3 = instant1.plus(diff, TimeZone.SYSTEM)
167+
val diff = instant1.periodUntil(instant2, TimeZone.currentSystemDefault())
168+
val instant3 = instant1.plus(diff, TimeZone.currentSystemDefault())
169169

170170
if (instant2 != instant3)
171171
println("start: $instant1, end: $instant2, start + diff: $instant3, diff: $diff")

core/commonTest/src/LocalDateTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class LocalDateTest {
7171

7272
@Test
7373
fun tomorrow() {
74-
val today = Clock.System.todayAt(TimeZone.SYSTEM)
74+
val today = Clock.System.todayAt(TimeZone.currentSystemDefault())
7575

7676
val nextMonthPlusDay1 = today.plus(DateTimeUnit.MONTH).plus(1, DateTimeUnit.DAY)
7777
val nextMonthPlusDay2 = today + DatePeriod(months = 1, days = 1)

core/commonTest/src/LocalDateTimeTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class LocalDateTimeTest {
6161

6262
@Test
6363
fun getCurrentHMS() {
64-
with(Clock.System.now().toLocalDateTime(TimeZone.SYSTEM)) {
64+
with(Clock.System.now().toLocalDateTime(TimeZone.currentSystemDefault())) {
6565
println("${hour}h ${minute}m")
6666
}
6767
}

core/commonTest/src/TimeZoneTest.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ class TimeZoneTest {
2020

2121
@Test
2222
fun system() {
23-
println(TimeZone.SYSTEM)
23+
val tz = TimeZone.currentSystemDefault()
24+
println(tz)
25+
val offset = Clock.System.now().offsetAt(tz)
26+
assertTrue(offset.totalSeconds in -18 * 60 * 60 .. 18 * 60 * 60)
27+
// assertTrue(tz.id.contains('/')) // does not work on build agents, whose timezone is "UTC"
2428
// TODO: decide how to assert system tz properties
2529
}
2630

@@ -31,7 +35,7 @@ class TimeZoneTest {
3135
allTzIds.forEach(::println)
3236

3337
assertNotEquals(0, allTzIds.size)
34-
assertTrue(TimeZone.SYSTEM.id in allTzIds)
38+
assertTrue(TimeZone.currentSystemDefault().id in allTzIds)
3539
assertTrue("UTC" in allTzIds)
3640
}
3741

core/jsMain/src/TimeZone.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ actual open class TimeZone internal constructor(internal val zoneId: ZoneId) {
2424
override fun toString(): String = zoneId.toString()
2525

2626
actual companion object {
27-
actual val SYSTEM: TimeZone = ZoneId.systemDefault().let(::TimeZone)
27+
actual fun currentSystemDefault(): TimeZone = ZoneId.systemDefault().let(::TimeZone)
2828
actual val UTC: TimeZone = jtZoneOffset.UTC.let(::TimeZone)
2929
actual fun of(zoneId: String): TimeZone = ZoneId.of(zoneId).let(::TimeZone)
3030
actual val availableZoneIds: Set<String> get() = ZoneId.getAvailableZoneIds().toSet()

core/jvmMain/src/TimeZone.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ actual open class TimeZone internal constructor(internal val zoneId: ZoneId) {
2525
override fun toString(): String = zoneId.toString()
2626

2727
actual companion object {
28-
actual val SYSTEM: TimeZone = ZoneId.systemDefault().let(::TimeZone)
28+
actual fun currentSystemDefault(): TimeZone = ZoneId.systemDefault().let(::TimeZone)
2929
actual val UTC: TimeZone = jtZoneOffset.UTC.let(::TimeZone)
3030
actual fun of(zoneId: String): TimeZone = ZoneId.of(zoneId).let(::TimeZone)
3131
actual val availableZoneIds: Set<String> get() = ZoneId.getAvailableZoneIds()

core/nativeMain/cinterop/cpp/apple.mm

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,58 @@ static TZID id_by_name(NSString *zone_name)
7373

7474
char * get_system_timezone(TZID *tzid)
7575
{
76-
CFTimeZoneRef zone = CFTimeZoneCopySystem(); // always succeeds
77-
auto name = CFTimeZoneGetName(zone);
78-
*tzid = id_by_name((__bridge NSString *)name);
79-
CFIndex bufferSize = CFStringGetLength(name) + 1;
80-
char * buffer = check_allocation((char *)malloc(sizeof(char) * bufferSize));
81-
// only fails if the name is not UTF8-encoded, which is an anomaly.
82-
auto result = CFStringGetCString(name, buffer, bufferSize, kCFStringEncodingUTF8);
83-
assert(result);
84-
CFRelease(zone);
85-
return buffer;
76+
/* The framework has its own cache of the system timezone. Calls to
77+
[NSTimeZone systemTimeZone] do not reflect changes to the system timezone
78+
and instead just return the cached value. Thus, to acquire the current
79+
system timezone, first, the cache should be cleared.
80+
81+
This solution is not without flaws, however. In particular, resetting the
82+
system timezone also resets the default timezone ([NSTimeZone default]) if
83+
it's the same as the cached system timezone:
84+
85+
NSTimeZone.defaultTimeZone = [NSTimeZone
86+
timeZoneWithName: [[NSTimeZone systemTimeZone] name]];
87+
NSLog(@"%@", NSTimeZone.defaultTimeZone.name);
88+
NSLog(@"Change the system time zone, then press Enter");
89+
getchar();
90+
[NSTimeZone resetSystemTimeZone];
91+
NSLog(@"%@", NSTimeZone.defaultTimeZone.name); // will also change
92+
93+
This is a fairly marginal problem:
94+
* It is only a problem when the developer deliberately sets the default
95+
timezone to the region that just happens to be the one that the user
96+
is in, and then the user moves to another region, and the app also
97+
uses the system timezone.
98+
* Since iOS 11, the significance of the default timezone has been
99+
de-emphasized. In particular, it is not included in the API for
100+
Swift: https://forums.swift.org/t/autoupdating-type-properties/4608/4
101+
102+
Another possible solution could involve using [NSTimeZone localTimeZone].
103+
This is documented to reflect the current, uncached system timezone on
104+
iOS 11 and later:
105+
https://developer.apple.com/documentation/foundation/nstimezone/1387209-localtimezone
106+
However:
107+
* Before iOS 11, this was the same as the default timezone and did not
108+
reflect the system timezone.
109+
* Worse, on a Mac (10.15.5), I failed to get it to work as documented.
110+
NSLog(@"%@", NSTimeZone.localTimeZone.name);
111+
NSLog(@"Change the system time zone, then press Enter");
112+
getchar();
113+
// [NSTimeZone resetSystemTimeZone]; // uncomment to make it work
114+
NSLog(@"%@", NSTimeZone.localTimeZone.name);
115+
The printed strings are the same even if I wait for good 10 minutes
116+
before pressing Enter, unless the line with "reset" is uncommented--
117+
then the timezone is updated, as it should be. So, for some reason,
118+
NSTimeZone.localTimeZone, too, is cached.
119+
With no iOS device to test this on, it doesn't seem worth the effort
120+
to avoid just resetting the system timezone due to one edge case
121+
that's hard to avoid.
122+
*/
123+
[NSTimeZone resetSystemTimeZone];
124+
NSTimeZone *zone = [NSTimeZone systemTimeZone];
125+
NSString *name = [zone name];
126+
*tzid = id_by_name(name);
127+
return strdup([name UTF8String]);
86128
}
87129

88130
char ** available_zone_ids()

core/nativeMain/src/TimeZone.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ public actual open class TimeZone internal constructor(private val tzid: TZID, a
1616

1717
actual companion object {
1818

19-
@SharedImmutable
20-
actual val SYSTEM: TimeZone = memScoped {
19+
actual fun currentSystemDefault(): TimeZone = memScoped {
2120
val tzid = alloc<TZIDVar>()
2221
val string = get_system_timezone(tzid.ptr)
2322
?: throw RuntimeException("Failed to get the system timezone.")

0 commit comments

Comments
 (0)