From e0ee75e0d02dad863535f4ff8995fa95198cdd35 Mon Sep 17 00:00:00 2001 From: Katherine Date: Tue, 22 Apr 2025 16:56:10 -0400 Subject: [PATCH] Fix Daylight Savings bug in recommended notification time calculation --- .../scheduler/SchedulingUtil.java | 11 ++++----- .../scheduler/SchedulingUtilTest.java | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) 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 65b07a0c4..579dc0391 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtil.java @@ -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 getZoneOffset(final Account account, final Clock clock) { + static Optional 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 sortedZoneOffsets = timeZonesForNumber + final List 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(); 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 13ec7aae9..ee7631d62 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtilTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/scheduler/SchedulingUtilTest.java @@ -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))); + } }