util: revisit DST handling

Instead of relying on a fixed +1/-1 day offset, ignore completely DST changes
for LocalDate <-> DateTime transformations.

TestDateAndTimeZoneContext changes:

* Remove computeOffsetFromUtc tests (functionality removed)
* Add more DST tests

TestWithTimeZones changes:

* The cancellation date will now use the PDT timezone (one hour earlier)
to compute the effective cancellation datetime

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>

api/src/main/java/org/killbill/billing/callcontext/TimeAwareContext.java

@@ -27,6 +27,9 @@
 // TODO Cache the accountTimeZone, reference time and clock in the context
 public class TimeAwareContext {
 
+    /// Generic functions
+    /// TODO Move to ClockUtil
+
     // From JDK to Joda (see http://www.joda.org/joda-time/userguide.html#JDK_Interoperability)
     public DateTime toUTCDateTime(final Date date) {
         return toUTCDateTime(new DateTime(date));
@@ -37,31 +40,43 @@ public DateTime toUTCDateTime(final DateTime dateTime) {
         return toDateTime(dateTime, DateTimeZone.UTC);
     }
 
-    // Create a DateTime object using the specified reference time and timezone (usually, the one on the account)
-    public DateTime toUTCDateTime(final LocalDate localDate, final DateTime referenceDateTime, final DateTimeZone accountTimeZone) {
-        return toUTCDateTime(toDateTime(localDate, referenceDateTime, accountTimeZone));
+    // Create a DateTime object using the specified timezone (usually, the one on the account)
+    public DateTime toDateTime(final DateTime dateTime, final DateTimeZone accountTimeZone) {
+        return dateTime.toDateTime(accountTimeZone);
     }
 
+    /// DateTime <-> LocalDate transformations
+
     // Create a DateTime object using the specified reference time and timezone (usually, the one on the account)
-    public DateTime toDateTime(final LocalDate localDate, final DateTime referenceDateTime, final DateTimeZone accountTimeZone) {
-        final LocalTime referenceLocalTime = toDateTime(referenceDateTime, accountTimeZone).toLocalTime();
+    public DateTime toUTCDateTime(final LocalDate localDate, final DateTime referenceDateTime, final DateTimeZone accountTimeZone) {
+        final DateTimeZone normalizedAccountTimezone = getNormalizedAccountTimezone(referenceDateTime, accountTimeZone);
+
+        final LocalTime referenceLocalTime = toDateTime(referenceDateTime, normalizedAccountTimezone).toLocalTime();
+
+        final DateTime targetDateTime = new DateTime(localDate.getYear(),
+                                                     localDate.getMonthOfYear(),
+                                                     localDate.getDayOfMonth(),
+                                                     referenceLocalTime.getHourOfDay(),
+                                                     referenceLocalTime.getMinuteOfHour(),
+                                                     referenceLocalTime.getSecondOfMinute(),
+                                                     normalizedAccountTimezone);
 
-        return new DateTime(localDate.getYear(),
-                            localDate.getMonthOfYear(),
-                            localDate.getDayOfMonth(),
-                            referenceLocalTime.getHourOfDay(),
-                            referenceLocalTime.getMinuteOfHour(),
-                            referenceLocalTime.getSecondOfMinute(),
-                            accountTimeZone);
+        return toUTCDateTime(targetDateTime);
     }
 
-    // Create a DateTime object using the specified timezone (usually, the one on the account)
-    public DateTime toDateTime(final DateTime dateTime, final DateTimeZone accountTimeZone) {
-        return dateTime.toDateTime(accountTimeZone);
+    // Create a LocalDate object using the specified timezone (usually, the one on the account), respecting the offset at the time of the referenceDateTime
+    public LocalDate toLocalDate(final DateTime dateTime, final DateTime referenceDateTime, final DateTimeZone accountTimeZone) {
+        final DateTimeZone normalizedAccountTimezone = getNormalizedAccountTimezone(referenceDateTime, accountTimeZone);
+        return new LocalDate(dateTime, normalizedAccountTimezone);
     }
 
-    // Create a LocalDate object using the specified timezone (usually, the one on the account)
-    public LocalDate toLocalDate(final DateTime dateTime, final DateTimeZone accountTimeZone) {
-        return new LocalDate(dateTime, accountTimeZone);
+    private DateTimeZone getNormalizedAccountTimezone(final DateTime referenceDateTime, final DateTimeZone accountTimeZone) {
+        // Check if DST was in effect at the reference date time
+        final boolean shouldUseDST = !accountTimeZone.isStandardOffset(referenceDateTime.getMillis());
+        if (shouldUseDST) {
+            return DateTimeZone.forOffsetMillis(accountTimeZone.getOffset(referenceDateTime.getMillis()));
+        } else {
+            return DateTimeZone.forOffsetMillis(accountTimeZone.getStandardOffset(referenceDateTime.getMillis()));
+        }
     }
 }

beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithTimeZones.java

@@ -107,7 +107,7 @@ public void testWithDayLightSaving() throws Exception {
         }
     }
 
-    // Verify cancellation logic when we exit daylight savind period
+    // Verify cancellation logic when we exit daylight saving period
     @Test(groups = "slow")
     public void testCancellationFrom_PDT_to_PST() throws Exception {
         // Start with a date in daylight saving period (PDT) and make sure we use a time of 7 hour so that we we reach standard time (PST)
@@ -140,22 +140,14 @@ public void testCancellationFrom_PDT_to_PST() throws Exception {
         // Cancel the next month specifying just a LocalDate
         final LocalDate cancellationDate = new LocalDate("2015-12-01", tz);
         entitlement = entitlement.cancelEntitlementWithDate(cancellationDate, true, ImmutableList.<PluginProperty>of(), callContext);
+        assertListenerStatus();
 
         // Verify first entitlement is correctly cancelled on the right date
         Assert.assertEquals(entitlement.getEffectiveEndDate(), cancellationDate);
 
-        busHandler.pushExpectedEvent(NextEvent.NULL_INVOICE);
-        // We move the clock to the date of the next invoice notification 2015-12-01 07:01:01 (invoice is using a fixed offset of 7 hours and all billing events are converted using that offset)
-        clock.setTime(new DateTime("2015-12-01T07:01:02"));
-        assertListenerStatus();
-
-        // We now move the clock to the date of the cancellation (one hour later), which match the cancellation day from the client point of view
-        //
-        // For the curious reader, the reason why the time end up being '8:01:0' comes from (https://github.com/killbill/killbill-commons/blob/master/clock/src/main/java/org/killbill/clock/ClockUtil.java#L51):
-        // We compute a DateTime in the account timezone by specifying explicitly the year-month-day we want to end up in, and shoving *a time*. The reason why we end up on an 8:01:02 is not necessarily so important,
-        // What's important is that by construction that DateTime is guaranteed to match a LocalDate of 2015-12-01
-        busHandler.pushExpectedEvents(NextEvent.CANCEL, NextEvent.BLOCK, NextEvent.NULL_INVOICE);
-        clock.setTime(new DateTime("2015-12-01T08:01:02"));
+        // We now move the clock to the date of the cancellation, which match the cancellation day from the client point of view
+        busHandler.pushExpectedEvents(NextEvent.NULL_INVOICE, NextEvent.CANCEL, NextEvent.BLOCK, NextEvent.NULL_INVOICE);
+        clock.setTime(new DateTime("2015-12-01T07:01:02Z"));
         assertListenerStatus();
 
         // Verify second that there was no repair (so the cancellation did correctly happen on the "2015-12-01")

entitlement/src/main/java/org/killbill/billing/entitlement/DefaultEntitlementService.java

@@ -156,9 +156,9 @@ private void processEntitlementNotification(final EntitlementNotificationKey key
                 EntitlementNotificationKeyAction.CANCEL.equals(entitlementNotificationKeyAction)) {
                 blockAddOnsIfRequired(key, (DefaultEntitlement) entitlement, callContext, internalCallContext);
             } else if (EntitlementNotificationKeyAction.PAUSE.equals(entitlementNotificationKeyAction)) {
-                entitlementInternalApi.pause(key.getBundleId(), internalCallContext.toLocalDate(key.getEffectiveDate(), immutableAccountData.getTimeZone()), ImmutableList.<PluginProperty>of(), internalCallContext);
+                entitlementInternalApi.pause(key.getBundleId(), internalCallContext.toLocalDate(key.getEffectiveDate(), ((DefaultEntitlement) entitlement).getSubscriptionBase().getStartDate(), immutableAccountData.getTimeZone()), ImmutableList.<PluginProperty>of(), internalCallContext);
             } else if (EntitlementNotificationKeyAction.RESUME.equals(entitlementNotificationKeyAction)) {
-                entitlementInternalApi.resume(key.getBundleId(), internalCallContext.toLocalDate(key.getEffectiveDate(), immutableAccountData.getTimeZone()), ImmutableList.<PluginProperty>of(), internalCallContext);
+                entitlementInternalApi.resume(key.getBundleId(), internalCallContext.toLocalDate(key.getEffectiveDate(), ((DefaultEntitlement) entitlement).getSubscriptionBase().getStartDate(), immutableAccountData.getTimeZone()), ImmutableList.<PluginProperty>of(), internalCallContext);
             }
         } catch (final EntitlementApiException e) {
             log.error("Error processing event for entitlement {}" + entitlement.getId(), e);

entitlement/src/main/java/org/killbill/billing/entitlement/api/BlockingStateOrdering.java

@@ -59,22 +59,24 @@ public static void insertSorted(final Iterable<Entitlement> entitlements, final
     private void computeEvents(final Iterable<Entitlement> entitlements, final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext, final LinkedList<SubscriptionEvent> result) {
         final Collection<UUID> allEntitlementUUIDs = new HashSet<UUID>();
         final Collection<BlockingState> blockingStates = new LinkedList<BlockingState>();
+        final Map<UUID, DateTime> referenceTimes = new HashMap<UUID, DateTime>();
         for (final Entitlement entitlement : entitlements) {
             allEntitlementUUIDs.add(entitlement.getId());
             Preconditions.checkState(entitlement instanceof DefaultEntitlement, "Entitlement %s is not a DefaultEntitlement", entitlement);
             blockingStates.addAll(((DefaultEntitlement) entitlement).getEventsStream().getBlockingStates());
+            referenceTimes.put(entitlement.getId(), ((DefaultEntitlement) entitlement).getSubscriptionBase().getStartDate());
         }
 
         // Trust the incoming ordering here: blocking states were sorted using ProxyBlockingStateDao#sortedCopy
         for (final BlockingState bs : blockingStates) {
             final List<SubscriptionEvent> newEvents = new ArrayList<SubscriptionEvent>();
-            final int index = insertFromBlockingEvent(accountTimeZone, internalTenantContext, allEntitlementUUIDs, result, bs, bs.getEffectiveDate(), newEvents);
+            final int index = insertFromBlockingEvent(referenceTimes, accountTimeZone, internalTenantContext, allEntitlementUUIDs, result, bs, bs.getEffectiveDate(), newEvents);
             insertAfterIndex(result, newEvents, index);
         }
     }
 
     // Returns the index and the newEvents generated from the incoming blocking state event. Those new events will all be created for the same effectiveDate and should be ordered.
-    private int insertFromBlockingEvent(final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext, final Collection<UUID> allEntitlementUUIDs, final List<SubscriptionEvent> result, final BlockingState bs, final DateTime bsEffectiveDate, final Collection<SubscriptionEvent> newEvents) {
+    private int insertFromBlockingEvent(final Map<UUID, DateTime> referenceTimes, final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext, final Collection<UUID> allEntitlementUUIDs, final List<SubscriptionEvent> result, final BlockingState bs, final DateTime bsEffectiveDate, final Collection<SubscriptionEvent> newEvents) {
         // Keep the current state per entitlement
         final Map<UUID, TargetState> targetStates = new HashMap<UUID, TargetState>();
         for (final UUID cur : allEntitlementUUIDs) {
@@ -133,7 +135,7 @@ private int insertFromBlockingEvent(final DateTimeZone accountTimeZone, final In
 
             final List<SubscriptionEventType> eventTypes = curTargetState.addStateAndReturnEventTypes(bs);
             for (final SubscriptionEventType t : eventTypes) {
-                newEvents.add(toSubscriptionEvent(prevNext[0], prevNext[1], targetEntitlementId, bs, t, accountTimeZone, internalTenantContext));
+                newEvents.add(toSubscriptionEvent(prevNext[0], prevNext[1], targetEntitlementId, bs, t, referenceTimes.get(targetEntitlementId), accountTimeZone, internalTenantContext));
             }
         }
         return index;
@@ -176,7 +178,7 @@ private SubscriptionEvent[] findPrevNext(final List<SubscriptionEvent> events, f
 
     private SubscriptionEvent toSubscriptionEvent(@Nullable final SubscriptionEvent prev, @Nullable final SubscriptionEvent next,
                                                   final UUID entitlementId, final BlockingState in, final SubscriptionEventType eventType,
-                                                  final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext) {
+                                                  final DateTime referenceTime, final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext) {
         final Product prevProduct;
         final Plan prevPlan;
         final PlanPhase prevPlanPhase;
@@ -257,6 +259,7 @@ private SubscriptionEvent toSubscriptionEvent(@Nullable final SubscriptionEvent
                                             nextPriceList,
                                             nextBillingPeriod,
                                             in.getCreatedDate(),
+                                            referenceTime,
                                             accountTimeZone,
                                             internalTenantContext);
     }

entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultEntitlement.java

@@ -244,7 +244,7 @@ public EntitlementSourceType getSourceType() {
 
     @Override
     public LocalDate getEffectiveStartDate() {
-        return internalTenantContext.toLocalDate(getSubscriptionBase().getStartDate(), eventsStream.getAccountTimeZone());
+        return internalTenantContext.toLocalDate(getSubscriptionBase().getStartDate(), getSubscriptionBase().getStartDate(), eventsStream.getAccountTimeZone());
     }
 
     @Override
@@ -460,7 +460,7 @@ private LocalDate getLocalDateFromEntitlementPolicy(final EntitlementActionPolic
                 break;
             case END_OF_TERM:
                 if (getSubscriptionBase().getChargedThroughDate() != null) {
-                    cancellationDate = internalTenantContext.toLocalDate(getSubscriptionBase().getChargedThroughDate(), eventsStream.getAccountTimeZone());
+                    cancellationDate = internalTenantContext.toLocalDate(getSubscriptionBase().getChargedThroughDate(), getSubscriptionBase().getStartDate(), eventsStream.getAccountTimeZone());
                 } else {
                     cancellationDate = clock.getToday(eventsStream.getAccountTimeZone());
                 }

entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultSubscription.java

@@ -31,7 +31,7 @@ public class DefaultSubscription extends DefaultEntitlement implements Subscript
 
     @Override
     public LocalDate getBillingStartDate() {
-        return internalTenantContext.toLocalDate(getSubscriptionBase().getStartDate(), getAccountTimeZone());
+        return internalTenantContext.toLocalDate(getSubscriptionBase().getStartDate(), getSubscriptionBase().getStartDate(), getAccountTimeZone());
     }
 
     @Override
@@ -51,12 +51,12 @@ public LocalDate getBillingEndDate() {
             futureOrCurrentEndDate = futureOrCurrentEndDateForSubscription;
         }
 
-        return futureOrCurrentEndDate != null ? internalTenantContext.toLocalDate(futureOrCurrentEndDate, getAccountTimeZone()) : null;
+        return futureOrCurrentEndDate != null ? internalTenantContext.toLocalDate(futureOrCurrentEndDate, getSubscriptionBase().getStartDate(), getAccountTimeZone()) : null;
     }
 
     @Override
     public LocalDate getChargedThroughDate() {
-        return getSubscriptionBase().getChargedThroughDate() != null ? internalTenantContext.toLocalDate(getSubscriptionBase().getChargedThroughDate(), getAccountTimeZone()) : null;
+        return getSubscriptionBase().getChargedThroughDate() != null ? internalTenantContext.toLocalDate(getSubscriptionBase().getChargedThroughDate(), getSubscriptionBase().getStartDate(), getAccountTimeZone()) : null;
     }
 
     @Override

entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultSubscriptionEvent.java

@@ -52,6 +52,7 @@ public class DefaultSubscriptionEvent implements SubscriptionEvent {
     private final PriceList nextPriceList;
     private final BillingPeriod nextBillingPeriod;
     private final DateTime createdDate;
+    private final DateTime referenceTime;
     private final DateTimeZone accountTimeZone;
     private final InternalTenantContext internalTenantContext;
 
@@ -74,6 +75,7 @@ public DefaultSubscriptionEvent(final UUID id,
                                     final PriceList nextPriceList,
                                     final BillingPeriod nextBillingPeriod,
                                     final DateTime createDate,
+                                    final DateTime referenceTime,
                                     final DateTimeZone accountTimeZone,
                                     final InternalTenantContext internalTenantContext) {
         this.id = id;
@@ -96,6 +98,7 @@ public DefaultSubscriptionEvent(final UUID id,
         this.nextPriceList = nextPriceList;
         this.nextBillingPeriod = nextBillingPeriod;
         this.createdDate = createDate;
+        this.referenceTime = referenceTime;
         this.accountTimeZone = accountTimeZone;
         this.internalTenantContext = internalTenantContext;
     }
@@ -124,12 +127,12 @@ public UUID getEntitlementId() {
 
     @Override
     public LocalDate getEffectiveDate() {
-        return effectiveDate != null ? internalTenantContext.toLocalDate(effectiveDate, accountTimeZone) : null;
+        return effectiveDate != null ? internalTenantContext.toLocalDate(effectiveDate, referenceTime, accountTimeZone) : null;
     }
 
     @Override
     public LocalDate getRequestedDate() {
-        return requestedDate != null ? internalTenantContext.toLocalDate(requestedDate, accountTimeZone) : null;
+        return requestedDate != null ? internalTenantContext.toLocalDate(requestedDate, referenceTime, accountTimeZone) : null;
     }
 
     @Override

entitlement/src/main/java/org/killbill/billing/entitlement/api/EntitlementDateHelper.java

@@ -65,9 +65,9 @@ public DateTime fromLocalDateAndReferenceTime(final LocalDate requestedDate, fin
      * @return true if the inputDate, once converted into a LocalDate using account timezone is less or equals than today
      */
     // TODO Move to ClockUtils
-    public boolean isBeforeOrEqualsToday(final DateTime inputDate, final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext) {
+    public boolean isBeforeOrEqualsToday(final DateTime inputDate, final DateTime referenceDatetime, final DateTimeZone accountTimeZone, final InternalTenantContext internalTenantContext) {
         final LocalDate localDateNowInAccountTimezone = clock.getToday(accountTimeZone);
-        final LocalDate targetDateInAccountTimezone = internalTenantContext.toLocalDate(inputDate, accountTimeZone);
+        final LocalDate targetDateInAccountTimezone = internalTenantContext.toLocalDate(inputDate, referenceDatetime, accountTimeZone);
         return targetDateInAccountTimezone.compareTo(localDateNowInAccountTimezone) <= 0;
     }
 }