killbill-aplcache

Applied code review comments from Jeff and fixed some issues

5/27/2012 5:14:07 PM

Details

diff --git a/api/src/main/java/com/ning/billing/junction/api/BillingEventSet.java b/api/src/main/java/com/ning/billing/junction/api/BillingEventSet.java
index 4801182..aed5c6a 100644
--- a/api/src/main/java/com/ning/billing/junction/api/BillingEventSet.java
+++ b/api/src/main/java/com/ning/billing/junction/api/BillingEventSet.java
@@ -26,7 +26,7 @@ public interface BillingEventSet extends SortedSet<BillingEvent> {
 
     public abstract boolean isAccountAutoInvoiceOff();
 
-    public abstract List<UUID> getSubscriptionAndBundleIdsWithAutoInvoiceOff();
+    public abstract List<UUID> getSubscriptionIdsWithAutoInvoiceOff();
     
     public boolean isLast(BillingEvent event);
 
diff --git a/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java b/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java
index 8f17236..b982a3f 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java
@@ -148,7 +148,7 @@ public class InvoiceDispatcher {
             BillingEventSet billingEvents = billingApi.getBillingEventsForAccountAndUpdateAccountBCD(accountId);
             
             List<Invoice> invoices = new ArrayList<Invoice>();
-            if (billingEvents.isAccountAutoInvoiceOff()) {
+            if (!billingEvents.isAccountAutoInvoiceOff()) {
                 invoices = invoiceDao.getInvoicesByAccount(accountId); //no need to fetch, invoicing is off on this account
             } 
 
diff --git a/invoice/src/main/java/com/ning/billing/invoice/model/DefaultInvoiceGenerator.java b/invoice/src/main/java/com/ning/billing/invoice/model/DefaultInvoiceGenerator.java
index 8486208..f27f4a3 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/model/DefaultInvoiceGenerator.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/model/DefaultInvoiceGenerator.java
@@ -65,7 +65,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
                                          @Nullable final List<Invoice> existingInvoices,
                                          DateTime targetDate,
                                          final Currency targetCurrency) throws InvoiceApiException {
-        if ((events == null) || (events.size() == 0)) {
+        if ((events == null) || (events.size() == 0) || events.isAccountAutoInvoiceOff()) {
             return null;
         }
 
@@ -77,8 +77,9 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
         if (existingInvoices != null) {
             for (Invoice invoice : existingInvoices) {
                 for(InvoiceItem item : invoice.getInvoiceItems()) {
-                    if(!events.getSubscriptionAndBundleIdsWithAutoInvoiceOff()
-                            .contains(item.getBundleId())) { //don't add items with auto_invoice_off tag 
+                    if(item.getSubscriptionId() == null || // Always include migration invoices, credits etc.  
+                      !events.getSubscriptionIdsWithAutoInvoiceOff()
+                            .contains(item.getSubscriptionId())) { //don't add items with auto_invoice_off tag 
                         existingItems.add(item);
                     }
                 }
@@ -263,7 +264,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
         while(eventIt.hasNext()) {
             BillingEvent thisEvent = nextEvent;
             nextEvent = eventIt.next();
-            if(!events.getSubscriptionAndBundleIdsWithAutoInvoiceOff().
+            if(!events.getSubscriptionIdsWithAutoInvoiceOff().
                     contains(thisEvent.getSubscription().getId())) { // don't consider events for subscriptions that have auto_invoice_off
                 BillingEvent adjustedNextEvent = (thisEvent.getSubscription().getId() == nextEvent.getSubscription().getId()) ? nextEvent : null;
                 items.addAll(processEvents(invoiceId, accountId, thisEvent, adjustedNextEvent, targetDate, currency));
diff --git a/invoice/src/test/java/com/ning/billing/invoice/MockBillingEventSet.java b/invoice/src/test/java/com/ning/billing/invoice/MockBillingEventSet.java
index 2610f9f..3ab1f0d 100644
--- a/invoice/src/test/java/com/ning/billing/invoice/MockBillingEventSet.java
+++ b/invoice/src/test/java/com/ning/billing/invoice/MockBillingEventSet.java
@@ -42,7 +42,7 @@ public class MockBillingEventSet extends TreeSet<BillingEvent> implements Billin
     }
 
     @Override
-    public List<UUID> getSubscriptionAndBundleIdsWithAutoInvoiceOff() {
+    public List<UUID> getSubscriptionIdsWithAutoInvoiceOff() {
         return subscriptionIdsWithAutoInvoiceOff;
     }
 
diff --git a/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingApi.java b/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingApi.java
index 322d040..cee92dc 100644
--- a/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingApi.java
+++ b/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingApi.java
@@ -113,7 +113,7 @@ public class DefaultBillingApi implements BillingApi {
             Map<String,Tag> bundleTags = tagApi.getTags(bundle.getId(), ObjectType.BUNDLE);
             if(bundleTags.get(ControlTagType.AUTO_INVOICING_OFF.name()) != null) {
                  for (final Subscription subscription: subscriptions) { // billing is off so list sub ids in set to be excluded
-                    result.getSubscriptionAndBundleIdsWithAutoInvoiceOff().add(subscription.getId());
+                    result.getSubscriptionIdsWithAutoInvoiceOff().add(subscription.getId());
                 }
             } else { // billing is not off
                 addBillingEventsForSubscription(subscriptions, bundle, account, context, result);                 
diff --git a/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingEventSet.java b/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingEventSet.java
index 69740d9..0962a73 100644
--- a/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingEventSet.java
+++ b/junction/src/main/java/com/ning/billing/junction/plumbing/billing/DefaultBillingEventSet.java
@@ -43,7 +43,7 @@ public class DefaultBillingEventSet extends TreeSet<BillingEvent> implements Sor
      * @see com.ning.billing.junction.plumbing.billing.BillingEventSet#getSubscriptionIdsWithAutoInvoiceOff()
      */
     @Override
-    public List<UUID> getSubscriptionAndBundleIdsWithAutoInvoiceOff() {
+    public List<UUID> getSubscriptionIdsWithAutoInvoiceOff() {
         return subscriptionIdsWithAutoInvoiceOff;
     }
 
diff --git a/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBillingApi.java b/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBillingApi.java
index d0b97bf..68469b6 100644
--- a/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBillingApi.java
+++ b/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBillingApi.java
@@ -523,8 +523,8 @@ public class TestBillingApi {
 
         BillingEventSet events = api.getBillingEventsForAccountAndUpdateAccountBCD(account.getId());
         
-        assertEquals(events.getSubscriptionAndBundleIdsWithAutoInvoiceOff().size(), 1);
-        assertEquals(events.getSubscriptionAndBundleIdsWithAutoInvoiceOff().get(0), subId);
+        assertEquals(events.getSubscriptionIdsWithAutoInvoiceOff().size(), 1);
+        assertEquals(events.getSubscriptionIdsWithAutoInvoiceOff().get(0), subId);
         assertEquals(events.size(),0);
     }
 
diff --git a/overdue/src/main/java/com/ning/billing/ovedue/notification/DefaultOverdueCheckNotifier.java b/overdue/src/main/java/com/ning/billing/ovedue/notification/DefaultOverdueCheckNotifier.java
index 9f45075..d13fdef 100644
--- a/overdue/src/main/java/com/ning/billing/ovedue/notification/DefaultOverdueCheckNotifier.java
+++ b/overdue/src/main/java/com/ning/billing/ovedue/notification/DefaultOverdueCheckNotifier.java
@@ -29,6 +29,7 @@ import com.ning.billing.overdue.listener.OverdueListener;
 import com.ning.billing.overdue.service.DefaultOverdueService;
 import com.ning.billing.util.notificationq.NotificationQueue;
 import com.ning.billing.util.notificationq.NotificationQueueService;
+import com.ning.billing.util.notificationq.NotificationQueueService.NoSuchNotificationQueue;
 import com.ning.billing.util.notificationq.NotificationQueueService.NotificationQueueAlreadyExists;
 import com.ning.billing.util.notificationq.NotificationQueueService.NotificationQueueHandler;
 
@@ -94,6 +95,11 @@ public class DefaultOverdueCheckNotifier implements  OverdueCheckNotifier {
     public void stop() {
         if (overdueQueue != null) {
         	overdueQueue.stopQueue();
+        	try {
+        	    notificationQueueService.deleteNotificationQueue(overdueQueue.getServiceName(), overdueQueue.getQueueName());
+        	} catch (NoSuchNotificationQueue e) {
+        	    log.error("Error deleting a queue by its own name - this should never happen", e);
+        	}
         }
     }
 
diff --git a/overdue/src/main/java/com/ning/billing/overdue/wrapper/OverdueWrapperFactory.java b/overdue/src/main/java/com/ning/billing/overdue/wrapper/OverdueWrapperFactory.java
index 6d3b967..0abc240 100644
--- a/overdue/src/main/java/com/ning/billing/overdue/wrapper/OverdueWrapperFactory.java
+++ b/overdue/src/main/java/com/ning/billing/overdue/wrapper/OverdueWrapperFactory.java
@@ -46,7 +46,7 @@ public class OverdueWrapperFactory {
     private final OverdueStateApplicator<SubscriptionBundle> overdueStateApplicatorBundle;
     private final BlockingApi api;
     private final Clock clock;
-    private OverdueStateSet<SubscriptionBundle> overdueStates;
+    private OverdueConfig config;
 
     @Inject
     public OverdueWrapperFactory(BlockingApi api, Clock clock, 
@@ -64,7 +64,7 @@ public class OverdueWrapperFactory {
     public <T extends Blockable> OverdueWrapper<T> createOverdueWrapperFor(T bloackable) throws OverdueError {
   
         if(bloackable instanceof SubscriptionBundle) {
-            return (OverdueWrapper<T>)new OverdueWrapper<SubscriptionBundle>((SubscriptionBundle)bloackable, api, overdueStates, 
+            return (OverdueWrapper<T>)new OverdueWrapper<SubscriptionBundle>((SubscriptionBundle)bloackable, api, getOverdueStateSetBundle(), 
                     clock, billingStateCalcuatorBundle, overdueStateApplicatorBundle);
         } else {
             throw new OverdueError(ErrorCode.OVERDUE_TYPE_NOT_SUPPORTED, bloackable.getId(), bloackable.getClass());
@@ -79,7 +79,7 @@ public class OverdueWrapperFactory {
             switch (state.getType()) {
             case SUBSCRIPTION_BUNDLE : {
                 SubscriptionBundle bundle = entitlementApi.getBundleFromId(id);
-                return (OverdueWrapper<T>) new OverdueWrapper<SubscriptionBundle>(bundle, api, overdueStates, 
+                return (OverdueWrapper<T>) new OverdueWrapper<SubscriptionBundle>(bundle, api, getOverdueStateSetBundle(), 
                         clock, billingStateCalcuatorBundle, overdueStateApplicatorBundle );
             }
             default : {
@@ -92,11 +92,9 @@ public class OverdueWrapperFactory {
         }
     }
     
-    public void setOverdueConfig(OverdueConfig config) {
-        if(config != null) {
-            overdueStates = config.getBundleStateSet();
-        } else {
-            overdueStates = new DefaultOverdueStateSet<SubscriptionBundle>() {
+    private OverdueStateSet<SubscriptionBundle> getOverdueStateSetBundle() {
+        if(config == null || config.getBundleStateSet() == null) {
+            return new DefaultOverdueStateSet<SubscriptionBundle>() {
 
                 @SuppressWarnings("unchecked")
                 @Override
@@ -104,7 +102,13 @@ public class OverdueWrapperFactory {
                     return new DefaultOverdueState[0];
                 }
             };
+        } else {
+           return config.getBundleStateSet();
         }
     }
+    
+    public void setOverdueConfig(OverdueConfig config) {   
+        this.config = config;
+    }
 
 }