killbill-memoizeit

Minor code review for #88

10/10/2013 1:54:23 PM

Details

diff --git a/api/src/main/java/com/ning/billing/invoice/api/InvoiceInternalApi.java b/api/src/main/java/com/ning/billing/invoice/api/InvoiceInternalApi.java
index 6abfa0c..6f672c7 100644
--- a/api/src/main/java/com/ning/billing/invoice/api/InvoiceInternalApi.java
+++ b/api/src/main/java/com/ning/billing/invoice/api/InvoiceInternalApi.java
@@ -72,5 +72,14 @@ public interface InvoiceInternalApi {
     public void consumeExistingCBAOnAccountWithUnpaidInvoices(final UUID accountId, final InternalCallContext context) throws InvoiceApiException;
 
 
+    /**
+     * Insert a new notification with a notificationDate of today to trigger a new invoice on the account.
+     *
+     * @param accountId        account id
+     * @param accountTimeZone  timezone of the account
+     * @param context          the context
+     *
+     * @throws InvoiceApiException
+     */
     public void scheduleInvoiceForAccount(UUID accountId, DateTimeZone accountTimeZone, InternalCallContext context) throws InvoiceApiException;
 }
diff --git a/invoice/src/main/java/com/ning/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java b/invoice/src/main/java/com/ning/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java
index 7f22d92..1046548 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java
@@ -27,6 +27,8 @@ import javax.inject.Inject;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.LocalDate;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.ning.billing.ErrorCode;
 import com.ning.billing.callcontext.InternalCallContext;
@@ -57,6 +59,8 @@ import com.google.common.collect.Collections2;
 
 public class DefaultInvoiceInternalApi implements InvoiceInternalApi {
 
+    private static final Logger log = LoggerFactory.getLogger(DefaultInvoiceInternalApi.class);
+
     private final InvoiceDao dao;
     private final NextBillingDatePoster nextBillingDatePoster;
     private final SubscriptionBaseInternalApi subscriptionBaseApi;
@@ -159,9 +163,13 @@ public class DefaultInvoiceInternalApi implements InvoiceInternalApi {
                 }
             }
         }
-        if (targetSubscription != null) {
-            final DateAndTimeZoneContext timeZoneContext = new DateAndTimeZoneContext(targetSubscription.getStartDate(), accountTimeZone, clock);
-            nextBillingDatePoster.insertNextBillingNotification(accountId, targetSubscription.getId(), timeZoneContext.computeUTCDateTimeFromNow(), context.getUserToken());
+        if (targetSubscription == null) {
+            log.info("scheduleInvoiceForAccount : no active subscriptions for account {}", accountId);
+            return;
         }
+
+        final DateAndTimeZoneContext timeZoneContext = new DateAndTimeZoneContext(targetSubscription.getStartDate(), accountTimeZone, clock);
+        final DateTime futureNotificationTime = timeZoneContext.computeUTCDateTimeFromNow();
+        nextBillingDatePoster.insertNextBillingNotification(accountId, targetSubscription.getId(), futureNotificationTime, context.getUserToken());
     }
 }
diff --git a/overdue/src/main/java/com/ning/billing/overdue/applicator/OverdueStateApplicator.java b/overdue/src/main/java/com/ning/billing/overdue/applicator/OverdueStateApplicator.java
index 7ce6f08..4fe06e7 100644
--- a/overdue/src/main/java/com/ning/billing/overdue/applicator/OverdueStateApplicator.java
+++ b/overdue/src/main/java/com/ning/billing/overdue/applicator/OverdueStateApplicator.java
@@ -148,7 +148,7 @@ public class OverdueStateApplicator {
 
             cancelSubscriptionsIfRequired(account, nextOverdueState, context);
 
-            triggerInvoiceIfNeeded(account, previousOverdueState, nextOverdueState, context);
+            scheduleInvoiceIfNeeded(account, previousOverdueState, nextOverdueState, context);
 
             sendEmailIfRequired(billingState, account, nextOverdueState, context);
 
@@ -168,7 +168,15 @@ public class OverdueStateApplicator {
         }
     }
 
-    private void triggerInvoiceIfNeeded(final Account account, final OverdueState previousOverdueState, final OverdueState nextOverdueState, final InternalCallContext context) throws InvoiceApiException {
+    private void scheduleInvoiceIfNeeded(final Account account, final OverdueState previousOverdueState, final OverdueState nextOverdueState, final InternalCallContext context) throws InvoiceApiException {
+        //
+        // Invoice will re-enter a notification to schedule a new invoice with a notificationDate equivalent to today. For a given active subscription on this account:
+        // - If that notificationDate is less or equals than the chargeThroughDate of the given subscription, it means the invoice was previously invoiced and so blocking/unblocking will have an effect,
+        //   a new invoice will be generated
+        // - If that notificationDate is greater than the chargeThroughDate, then that subscription will be invoiced for the next period.
+        //
+        // So in both case a new invoice will be generated.
+        //
         if (isBlockBillingTransition(previousOverdueState, nextOverdueState) || isUnblockBillingTransition(previousOverdueState, nextOverdueState)) {
             invoiceInternalApi.scheduleInvoiceForAccount(account.getId(), account.getTimeZone(), context);
         }