mirror of
https://github.com/signalapp/Signal-Server
synced 2026-02-15 13:55:39 +00:00
Improve SchedulingUtil.getZoneId for PhoneNumberType.UNKONWN numbers
This commit is contained in:
@@ -44,7 +44,7 @@ public class SchedulingUtil {
|
||||
final LocalTime preferredTime,
|
||||
final Clock clock) {
|
||||
|
||||
final ZonedDateTime candidateNotificationTime = getZoneId(account, clock)
|
||||
final ZonedDateTime candidateNotificationTime = getZoneId(account.getNumber(), clock)
|
||||
.map(zoneId -> {
|
||||
Metrics.counter(PARSED_TIMEZONE_COUNTER_NAME, HAS_TIMEZONE_TAG_NAME, String.valueOf(true)).increment();
|
||||
return ZonedDateTime.now(clock.withZone(zoneId)).with(preferredTime);
|
||||
@@ -64,15 +64,23 @@ public class SchedulingUtil {
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
static Optional<ZoneId> getZoneId(final Account account, final Clock clock) {
|
||||
public static Optional<ZoneId> getZoneId(final String e164, final Clock clock) {
|
||||
try {
|
||||
final Phonenumber.PhoneNumber phoneNumber = PhoneNumberUtil.getInstance().parse(account.getNumber(), null);
|
||||
final Phonenumber.PhoneNumber phoneNumber = PhoneNumberUtil.getInstance().parse(e164, null);
|
||||
|
||||
final List<String> timeZonesForNumber =
|
||||
List<String> timeZonesForNumber =
|
||||
PhoneNumberToTimeZonesMapper.getInstance().getTimeZonesForNumber(phoneNumber);
|
||||
|
||||
if (timeZonesForNumber.equals(List.of(PhoneNumberToTimeZonesMapper.getUnknownTimeZone()))) {
|
||||
return Optional.empty();
|
||||
if (isUnknownTimeZone(timeZonesForNumber)) {
|
||||
// The more general getTimeZonesForNumber has a guard on PhoneNumberType.UNKONWN, returning the “unknown”
|
||||
// time zone for these numbers. In these cases, we'll first fall back to the geographical lookup, which looks up
|
||||
// a result based on the country code and national significant number.
|
||||
timeZonesForNumber =
|
||||
PhoneNumberToTimeZonesMapper.getInstance().getTimeZonesForGeographicalNumber(phoneNumber);
|
||||
|
||||
if (isUnknownTimeZone(timeZonesForNumber)) {
|
||||
return Optional.empty();
|
||||
}
|
||||
}
|
||||
|
||||
final Instant now = clock.instant();
|
||||
@@ -91,7 +99,7 @@ public class SchedulingUtil {
|
||||
.collect(Collectors.toMap(
|
||||
ZoneId::getRules,
|
||||
Function.identity(),
|
||||
(v1, v2) -> v2));
|
||||
(_, v2) -> v2));
|
||||
|
||||
// Sort the ZoneRules by the offsets they produce
|
||||
final List<ZoneRules> zoneRulesSortedByOffset = byOffset.keySet()
|
||||
@@ -105,4 +113,8 @@ public class SchedulingUtil {
|
||||
return Optional.empty();
|
||||
}
|
||||
}
|
||||
|
||||
static boolean isUnknownTimeZone(final List<String> timeZonesForNumber) {
|
||||
return timeZonesForNumber.equals(List.of(PhoneNumberToTimeZonesMapper.getUnknownTimeZone()));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -91,33 +91,42 @@ class SchedulingUtilTest {
|
||||
|
||||
@Test
|
||||
void zoneIdSelectionSingleOffset() {
|
||||
final Account account = mock(Account.class);
|
||||
final Phonenumber.PhoneNumber phoneNumber = PhoneNumberUtil.getInstance().getExampleNumber("DE");
|
||||
|
||||
when(account.getNumber())
|
||||
.thenReturn(PhoneNumberUtil.getInstance().format(phoneNumber , PhoneNumberUtil.PhoneNumberFormat.E164));
|
||||
final String e164 = PhoneNumberUtil.getInstance().format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164);
|
||||
|
||||
final Instant now = Instant.now();
|
||||
|
||||
assertEquals(
|
||||
ZoneId.of("Europe/Berlin").getRules().getOffset(now),
|
||||
SchedulingUtil.getZoneId(account, TestClock.pinned(now)).orElseThrow().getRules().getOffset(now));
|
||||
SchedulingUtil.getZoneId(e164, TestClock.pinned(now)).orElseThrow().getRules().getOffset(now));
|
||||
}
|
||||
|
||||
@Test
|
||||
void zoneIdSelectionMultipleOffsets() {
|
||||
final Account account = mock(Account.class);
|
||||
|
||||
// A US VOIP number spans multiple time zones, we should pick a 'middle' one
|
||||
final Phonenumber.PhoneNumber phoneNumber =
|
||||
PhoneNumberUtil.getInstance().getExampleNumberForType("US", PhoneNumberUtil.PhoneNumberType.VOIP);
|
||||
when(account.getNumber())
|
||||
.thenReturn(PhoneNumberUtil.getInstance().format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164));
|
||||
final String e164 = PhoneNumberUtil.getInstance().format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164);
|
||||
|
||||
final Instant now = Instant.now();
|
||||
|
||||
assertEquals(
|
||||
ZoneId.of("America/Chicago").getRules().getOffset(now),
|
||||
SchedulingUtil.getZoneId(account, TestClock.pinned(now)).orElseThrow().getRules().getOffset(now));
|
||||
SchedulingUtil.getZoneId(e164, TestClock.pinned(now)).orElseThrow().getRules().getOffset(now));
|
||||
}
|
||||
|
||||
@Test
|
||||
void zoneIdSelectionUnknownNumber() {
|
||||
// An invalid number will fall back to a geographical lookup. Even if that is not technically correct for the number,
|
||||
// it will give a time zone for the region, rather than an Optional.empty() result
|
||||
final Phonenumber.PhoneNumber phoneNumber =
|
||||
PhoneNumberUtil.getInstance().getInvalidExampleNumber("US");
|
||||
final String e164 = PhoneNumberUtil.getInstance().format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164);
|
||||
|
||||
final Instant now = Instant.now();
|
||||
|
||||
assertEquals(
|
||||
ZoneId.of("America/New_York").getRules().getOffset(now),
|
||||
SchedulingUtil.getZoneId(e164, TestClock.pinned(now)).orElseThrow().getRules().getOffset(now));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user