diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtil.java index 00ad32575..f3e1fbdbe 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtil.java @@ -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 getZoneId(final Account account, final Clock clock) { + public static Optional 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 timeZonesForNumber = + List 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 zoneRulesSortedByOffset = byOffset.keySet() @@ -105,4 +113,8 @@ public class SchedulingUtil { return Optional.empty(); } } + + static boolean isUnknownTimeZone(final List timeZonesForNumber) { + return timeZonesForNumber.equals(List.of(PhoneNumberToTimeZonesMapper.getUnknownTimeZone())); + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtilTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtilTest.java index 96f38ccf2..0b5e3bd09 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtilTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtilTest.java @@ -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)); } }