Fix Daylight Savings bug in recommended notification time calculation

This commit is contained in:
Katherine 2025-04-22 16:56:10 -04:00 committed by GitHub
parent 1ef3a230a1
commit e0ee75e0d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 28 additions and 7 deletions

View File

@ -38,8 +38,8 @@ public class SchedulingUtil {
final LocalTime preferredTime,
final Clock clock) {
final ZonedDateTime candidateNotificationTime = getZoneOffset(account, clock)
.map(zoneOffset -> ZonedDateTime.now(zoneOffset).with(preferredTime))
final ZonedDateTime candidateNotificationTime = getZoneId(account, clock)
.map(zoneId -> ZonedDateTime.now(clock.withZone(zoneId)).with(preferredTime))
.orElseGet(() -> {
// We couldn't find a reasonable timezone for the account for some reason, so make an educated guess at a
// reasonable time to send a notification based on the account's creation time.
@ -59,7 +59,7 @@ public class SchedulingUtil {
}
@VisibleForTesting
static Optional<ZoneOffset> getZoneOffset(final Account account, final Clock clock) {
static Optional<ZoneId> getZoneId(final Account account, final Clock clock) {
try {
final Phonenumber.PhoneNumber phoneNumber = PhoneNumberUtil.getInstance().parse(account.getNumber(), null);
@ -70,7 +70,7 @@ public class SchedulingUtil {
return Optional.empty();
}
final List<ZoneOffset> sortedZoneOffsets = timeZonesForNumber
final List<ZoneId> sortedZoneOffsets = timeZonesForNumber
.stream()
.map(id -> {
try {
@ -80,9 +80,6 @@ public class SchedulingUtil {
}
})
.filter(Objects::nonNull)
.map(ZoneId::getRules)
.distinct()
.map(zoneRules -> zoneRules.getOffset(clock.instant()))
.sorted()
.toList();

View File

@ -6,8 +6,10 @@ import static org.mockito.Mockito.when;
import com.google.i18n.phonenumbers.PhoneNumberUtil;
import java.time.Clock;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import org.junit.jupiter.api.Test;
import org.whispersystems.textsecuregcm.storage.Account;
@ -65,4 +67,26 @@ class SchedulingUtilTest {
Clock.fixed(afterNotificationTime.toInstant(), ZoneId.systemDefault())));
}
}
@Test
void getNextRecommendedNotificationTimeDaylightSavings() {
final Account account = mock(Account.class);
// The account has a phone number that can be resolved to a region with known timezones
when(account.getNumber()).thenReturn(PhoneNumberUtil.getInstance().format(
PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164));
final LocalDateTime afterNotificationTime = LocalDateTime.of(2025, 3, 29, 15, 0);
final ZoneId berlinZoneId = ZoneId.of("Europe/Berlin");
final ZoneOffset berlineZoneOffset = berlinZoneId.getRules().getOffset(afterNotificationTime);
// Daylight Savings Time started on 2025-03-30 at 2:00AM in Germany.
// Instantiating a ZonedDateTime with a zone ID factors in daylight savings when we adjust the time.
final ZonedDateTime afterNotificationTimeWithZoneId = ZonedDateTime.of(afterNotificationTime, berlinZoneId);
assertEquals(
afterNotificationTimeWithZoneId.with(LocalTime.of(14, 0)).plusDays(1).toInstant(),
SchedulingUtil.getNextRecommendedNotificationTime(account, LocalTime.of(14, 0),
Clock.fixed(afterNotificationTime.toInstant(berlineZoneOffset), berlinZoneId)));
}
}