killbill-aplcache

Fixing race condition in the update of account to add the BCD

2/22/2012 12:45:39 AM

Details

diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/billing/DefaultEntitlementBillingApi.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/billing/DefaultEntitlementBillingApi.java
index ee9ec81..788cda1 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/billing/DefaultEntitlementBillingApi.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/billing/DefaultEntitlementBillingApi.java
@@ -16,10 +16,24 @@ w * Copyright 2010-2011 Ning, Inc.
 
 package com.ning.billing.entitlement.api.billing;
 
+import java.util.Date;
+import java.util.List;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.UUID;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.skife.jdbi.v2.sqlobject.mixins.Transmogrifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.google.inject.Inject;
 import com.ning.billing.ErrorCode;
 import com.ning.billing.account.api.Account;
+import com.ning.billing.account.api.AccountApiException;
 import com.ning.billing.account.api.AccountUserApi;
+import com.ning.billing.account.api.DefaultAccount;
 import com.ning.billing.catalog.api.BillingAlignment;
 import com.ning.billing.catalog.api.Catalog;
 import com.ning.billing.catalog.api.CatalogApiException;
@@ -32,34 +46,23 @@ import com.ning.billing.entitlement.api.user.Subscription;
 import com.ning.billing.entitlement.api.user.SubscriptionBundle;
 import com.ning.billing.entitlement.api.user.SubscriptionData;
 import com.ning.billing.entitlement.api.user.SubscriptionFactory.SubscriptionBuilder;
-import com.ning.billing.entitlement.api.user.SubscriptionTransition.SubscriptionTransitionType;
 import com.ning.billing.entitlement.api.user.SubscriptionTransition;
+import com.ning.billing.entitlement.api.user.SubscriptionTransition.SubscriptionTransitionType;
 import com.ning.billing.entitlement.engine.dao.EntitlementDao;
 import com.ning.billing.entitlement.engine.dao.SubscriptionSqlDao;
-import org.joda.time.DateTime;
-import org.skife.jdbi.v2.sqlobject.mixins.Transmogrifier;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
-import java.util.SortedSet;
-import java.util.TreeSet;
-import java.util.UUID;
 
 
 public class DefaultEntitlementBillingApi implements EntitlementBillingApi {
 	private static final Logger log = LoggerFactory.getLogger(DefaultEntitlementBillingApi.class);
 
-    private final EntitlementDao dao;
+    private final EntitlementDao entitlementDao;
     private final AccountUserApi accountApi;
     private final CatalogService catalogService;
 
     @Inject
     public DefaultEntitlementBillingApi(final EntitlementDao dao, final AccountUserApi accountApi, final CatalogService catalogService) {
         super();
-        this.dao = dao;
+        this.entitlementDao = dao;
         this.accountApi = accountApi;
         this.catalogService = catalogService;
     }
@@ -68,23 +71,22 @@ public class DefaultEntitlementBillingApi implements EntitlementBillingApi {
     public SortedSet<BillingEvent> getBillingEventsForAccount(
             final UUID accountId) {
 
-        List<SubscriptionBundle> bundles = dao.getSubscriptionBundleForAccount(accountId);
-        List<Subscription> subscriptions = new ArrayList<Subscription>();
-        for (final SubscriptionBundle bundle: bundles) {
-            subscriptions.addAll(dao.getSubscriptions(bundle.getId()));
-        }
-
+        List<SubscriptionBundle> bundles = entitlementDao.getSubscriptionBundleForAccount(accountId);
         SortedSet<BillingEvent> result = new TreeSet<BillingEvent>();
-        for (final Subscription subscription: subscriptions) {
-        	for (final SubscriptionTransition transition : subscription.getAllTransitions()) {
-        		try {
-                    BillingEvent event = new DefaultBillingEvent(transition, subscription, calculateBCD(transition, accountId));
-        			result.add(event);
-        		} catch (CatalogApiException e) {
-        			log.error("Failing to identify catalog components while creating BillingEvent from transition: " +
-        					transition.getId().toString(), e);
-                } catch (Exception e) {
-                    log.warn("Failed while getting BillingEvent", e);
+        for (final SubscriptionBundle bundle: bundles) {
+        	List<Subscription> subscriptions = entitlementDao.getSubscriptions(bundle.getId());
+
+        	for (final Subscription subscription: subscriptions) {
+        		for (final SubscriptionTransition transition : subscription.getAllTransitions()) {
+        			try {
+        				BillingEvent event = new DefaultBillingEvent(transition, subscription, calculateBcd(bundle, subscription, transition, accountId));
+        				result.add(event);
+        			} catch (CatalogApiException e) {
+        				log.error("Failing to identify catalog components while creating BillingEvent from transition: " +
+        						transition.getId().toString(), e);
+        			} catch (Exception e) {
+        				log.warn("Failed while getting BillingEvent", e);
+        			}
         		}
         	}
         }
@@ -93,10 +95,10 @@ public class DefaultEntitlementBillingApi implements EntitlementBillingApi {
 
     @Override
     public UUID getAccountIdFromSubscriptionId(final UUID subscriptionId) {
-        return dao.getAccountIdFromSubscriptionId(subscriptionId);
+        return entitlementDao.getAccountIdFromSubscriptionId(subscriptionId);
     }
 
-    private int calculateBCD(final SubscriptionTransition transition, final UUID accountId) throws CatalogApiException {
+    private int calculateBcd(SubscriptionBundle bundle, Subscription subscription, final SubscriptionTransition transition, final UUID accountId) throws CatalogApiException, AccountApiException {
     	Catalog catalog = catalogService.getFullCatalog();
     	Plan plan =  (transition.getTransitionType() != SubscriptionTransitionType.CANCEL) ?
     	        transition.getNextPlan() : transition.getPreviousPlan();
@@ -111,41 +113,89 @@ public class DefaultEntitlementBillingApi implements EntitlementBillingApi {
     					transition.getNextPriceList(),
     					phase.getPhaseType()),
     					transition.getRequestedTransitionTime());
-    	int result = 0;
+    	int result = -1;
 
-        Account account = accountApi.getAccountById(accountId);
-
-    	switch (alignment) {
+		Account account = accountApi.getAccountById(accountId);
+		switch (alignment) {
     		case ACCOUNT :
     			result = account.getBillCycleDay();
+    			
+    			if(result == 0) {
+    				result = calculateBcdFromSubscription(subscription, plan, account);
+    			}
     		break;
     		case BUNDLE :
-    			SubscriptionBundle bundle = dao.getSubscriptionBundleFromId(transition.getBundleId());
-    			//TODO result = bundle.getStartDate().toDateTime(account.getTimeZone()).getDayOfMonth();
-    			result = bundle.getStartDate().getDayOfMonth();
+    			result = bundle.getStartDate().toDateTime(account.getTimeZone()).getDayOfMonth();
     		break;
     		case SUBSCRIPTION :
-    			Subscription subscription = dao.getSubscriptionFromId(transition.getSubscriptionId());
-    			//TODO result = subscription.getStartDate().toDateTime(account.getTimeZone()).getDayOfMonth();
-    			result = subscription.getStartDate().getDayOfMonth();
+    			result = subscription.getStartDate().toDateTime(account.getTimeZone()).getDayOfMonth();
     		break;
     	}
-    	if(result == 0) {
+    	if(result == -1) {
     		throw new CatalogApiException(ErrorCode.CAT_INVALID_BILLING_ALIGNMENT, alignment.toString());
     	}
     	return result;
 
     }
+    
+   	private int calculateBcdFromSubscription(Subscription subscription, Plan plan, Account account) throws AccountApiException {
+		int result = account.getBillCycleDay();
+        if(result != 0) {
+            return result;
+        }
+        result = new DateTime(account.getTimeZone()).getDayOfMonth();
 
+        try {
+        	result = billCycleDay(subscription.getStartDate(),account.getTimeZone(), plan);
+        } catch (CatalogApiException e) {
+            log.error("Unexpected catalog error encountered when updating BCD",e);
+        }
+        
+
+        Account modifiedAccount = new DefaultAccount(
+                account.getId(),
+                account.getExternalKey(),
+                account.getEmail(),
+                account.getName(),
+                account.getFirstNameLength(),
+                account.getCurrency(),
+                result,
+                account.getPaymentProviderName(),
+                account.getTimeZone(),
+                account.getLocale(),
+                account.getAddress1(),
+                account.getAddress2(),
+                account.getCompanyName(),
+                account.getCity(),
+                account.getStateOrProvince(),
+                account.getCountry(),
+                account.getPostalCode(),
+                account.getPhone(),
+                account.getCreatedDate(),
+                null // Updated date will be set internally
+        );
+        accountApi.updateAccount(modifiedAccount);
+        return result;
+    }
+
+    private int billCycleDay(DateTime requestedDate, DateTimeZone timeZone, 
+    		Plan plan) throws CatalogApiException {
+
+        DateTime date = plan.dateOfFirstRecurringNonZeroCharge(requestedDate);
+        return date.toDateTime(timeZone).getDayOfMonth();
+
+    }
+    
+    
     @Override
     public void setChargedThroughDate(final UUID subscriptionId, final DateTime ctd) {
-        SubscriptionData subscription = (SubscriptionData) dao.getSubscriptionFromId(subscriptionId);
+        SubscriptionData subscription = (SubscriptionData) entitlementDao.getSubscriptionFromId(subscriptionId);
 
         SubscriptionBuilder builder = new SubscriptionBuilder(subscription)
             .setChargedThroughDate(ctd)
             .setPaidThroughDate(subscription.getPaidThroughDate());
 
-        dao.updateSubscription(new SubscriptionData(builder));
+        entitlementDao.updateSubscription(new SubscriptionData(builder));
     }
 
     @Override
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementDao.java b/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementDao.java
index c9ddf90..5a04a47 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementDao.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementDao.java
@@ -16,17 +16,17 @@
 
 package com.ning.billing.entitlement.engine.dao;
 
-import com.ning.billing.entitlement.api.billing.EntitlementBillingApiException;
+import java.util.List;
+import java.util.UUID;
+
+import org.joda.time.DateTime;
+
 import com.ning.billing.entitlement.api.migration.AccountMigrationData;
-import com.ning.billing.entitlement.api.user.EntitlementUserApiException;
 import com.ning.billing.entitlement.api.user.Subscription;
 import com.ning.billing.entitlement.api.user.SubscriptionBundle;
 import com.ning.billing.entitlement.api.user.SubscriptionBundleData;
 import com.ning.billing.entitlement.api.user.SubscriptionData;
 import com.ning.billing.entitlement.events.EntitlementEvent;
-import java.util.Collection;
-import java.util.List;
-import java.util.UUID;
 
 public interface EntitlementDao {
 
@@ -76,4 +76,5 @@ public interface EntitlementDao {
     public void migrate(UUID acountId, AccountMigrationData data);
 
     public void undoMigration(UUID accountId);
-}
+
+	}
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementSqlDao.java b/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementSqlDao.java
index e5f15ed..a1a3dc5 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementSqlDao.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/EntitlementSqlDao.java
@@ -21,18 +21,18 @@ import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.UUID;
+
 import org.joda.time.DateTime;
 import org.skife.jdbi.v2.IDBI;
 import org.skife.jdbi.v2.Transaction;
 import org.skife.jdbi.v2.TransactionStatus;
-import org.skife.jdbi.v2.sqlobject.mixins.Transactional;
 import org.skife.jdbi.v2.sqlobject.mixins.Transmogrifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import com.google.inject.Inject;
 import com.ning.billing.ErrorCode;
 import com.ning.billing.catalog.api.ProductCategory;
-import com.ning.billing.entitlement.api.billing.EntitlementBillingApiException;
 import com.ning.billing.entitlement.api.migration.AccountMigrationData;
 import com.ning.billing.entitlement.api.migration.AccountMigrationData.BundleMigrationData;
 import com.ning.billing.entitlement.api.migration.AccountMigrationData.SubscriptionMigrationData;
@@ -53,7 +53,6 @@ import com.ning.billing.util.notificationq.NotificationKey;
 import com.ning.billing.util.notificationq.NotificationQueue;
 import com.ning.billing.util.notificationq.NotificationQueueService;
 import com.ning.billing.util.notificationq.NotificationQueueService.NoSuchNotificationQueue;
-import sun.jkernel.Bundle;
 
 
 public class EntitlementSqlDao implements EntitlementDao {
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/SubscriptionSqlDao.java b/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/SubscriptionSqlDao.java
index d896ebe..7b2ddcc 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/SubscriptionSqlDao.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/engine/dao/SubscriptionSqlDao.java
@@ -46,7 +46,8 @@ import java.util.UUID;
 @ExternalizedSqlViaStringTemplate3()
 public interface SubscriptionSqlDao extends Transactional<SubscriptionSqlDao>, CloseMe, Transmogrifier {
 
-    @SqlUpdate
+
+	@SqlUpdate
     public void insertSubscription(@Bind(binder = ISubscriptionDaoBinder.class) SubscriptionData sub);
 
     @SqlUpdate
@@ -62,7 +63,7 @@ public interface SubscriptionSqlDao extends Transactional<SubscriptionSqlDao>, C
 
     @SqlUpdate
     public void updateSubscription(@Bind("id") String id, @Bind("active_version") long activeVersion, @Bind("ctd_dt") Date ctd, @Bind("ptd_dt") Date ptd);
-
+   
     public static class ISubscriptionDaoBinder implements Binder<Bind, SubscriptionData> {
 
         private Date getDate(DateTime dateTime) {
diff --git a/entitlement/src/test/java/com/ning/billing/entitlement/api/billing/TestDefaultEntitlementBillingApi.java b/entitlement/src/test/java/com/ning/billing/entitlement/api/billing/TestDefaultEntitlementBillingApi.java
index 91ddc91..2d99f8d 100644
--- a/entitlement/src/test/java/com/ning/billing/entitlement/api/billing/TestDefaultEntitlementBillingApi.java
+++ b/entitlement/src/test/java/com/ning/billing/entitlement/api/billing/TestDefaultEntitlementBillingApi.java
@@ -17,12 +17,15 @@
 package com.ning.billing.entitlement.api.billing;
 
 
+import static org.testng.Assert.assertTrue;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.SortedSet;
 import java.util.UUID;
 
 import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
@@ -42,7 +45,6 @@ import com.ning.billing.catalog.api.PlanPhase;
 import com.ning.billing.catalog.api.PriceListSet;
 import com.ning.billing.catalog.glue.CatalogModule;
 import com.ning.billing.entitlement.api.TestApiBase;
-import com.ning.billing.entitlement.api.user.EntitlementUserApiException;
 import com.ning.billing.entitlement.api.user.Subscription;
 import com.ning.billing.entitlement.api.user.Subscription.SubscriptionState;
 import com.ning.billing.entitlement.api.user.SubscriptionBundle;
@@ -55,11 +57,11 @@ import com.ning.billing.entitlement.engine.dao.EntitlementDao;
 import com.ning.billing.entitlement.events.EntitlementEvent.EventType;
 import com.ning.billing.entitlement.events.user.ApiEventType;
 import com.ning.billing.lifecycle.KillbillService.ServiceException;
+import com.ning.billing.mock.BrainDeadProxyFactory;
+import com.ning.billing.mock.BrainDeadProxyFactory.ZombieControl;
 import com.ning.billing.util.clock.Clock;
 import com.ning.billing.util.glue.ClockModule;
 
-import static org.testng.Assert.assertTrue;
-
 public class TestDefaultEntitlementBillingApi {
 	private static final UUID zeroId = new UUID(0L,0L);
 	private static final UUID oneId = new UUID(1L,0L);
@@ -90,7 +92,7 @@ public class TestDefaultEntitlementBillingApi {
 	@BeforeMethod(alwaysRun=true)
 	public void setupEveryTime() {
 		bundles = new ArrayList<SubscriptionBundle>();
-		final SubscriptionBundle bundle = new SubscriptionBundleData( zeroId,"TestKey", oneId,  new DateTime().minusDays(4));
+		final SubscriptionBundle bundle = new SubscriptionBundleData( zeroId,"TestKey", oneId,  clock.getUTCNow().minusDays(4));
 		bundles.add(bundle);
 		
 		
@@ -98,7 +100,7 @@ public class TestDefaultEntitlementBillingApi {
 		subscriptions = new ArrayList<Subscription>();
 		
 		SubscriptionBuilder builder = new SubscriptionBuilder();
-		subscriptionStartDate = new DateTime().minusDays(3);
+		subscriptionStartDate = clock.getUTCNow().minusDays(3);
 		builder.setStartDate(subscriptionStartDate).setId(oneId);
 		subscription = new SubscriptionData(builder) {
 		    public List<SubscriptionTransition> getAllTransitions() {
@@ -194,15 +196,12 @@ public class TestDefaultEntitlementBillingApi {
 				zeroId, oneId, twoId, EventType.API_USER, ApiEventType.CREATE, then, now, null, null, null, null, SubscriptionState.ACTIVE, nextPlan, nextPhase, nextPriceList);
 		transitions.add(t);
 		
-		AccountUserApi accountApi = new BrainDeadAccountUserApi(){
-
-			@Override
-			public Account getAccountById(UUID accountId) {
-				return new BrainDeadAccount(){@Override
-				public int getBillCycleDay() {
-					return 1;
-				}};
-			}} ;
+		Account account = BrainDeadProxyFactory.createBrainDeadProxyFor(Account.class); 
+		((ZombieControl)account).addResult("getBillCycleDay", 1).addResult("getTimeZone", DateTimeZone.UTC);
+		
+		AccountUserApi accountApi = BrainDeadProxyFactory.createBrainDeadProxyFor(AccountUserApi.class);			
+		((ZombieControl)accountApi).addResult("getAccountById", account);
+				
 		DefaultEntitlementBillingApi api = new DefaultEntitlementBillingApi(dao,accountApi,catalogService);
 		SortedSet<BillingEvent> events = api.getBillingEventsForAccount(new UUID(0L,0L));
 		checkFirstEvent(events, nextPlan, subscription.getStartDate().getDayOfMonth(), oneId, now, nextPhase, ApiEventType.CREATE.toString());
@@ -244,15 +243,12 @@ public class TestDefaultEntitlementBillingApi {
 				zeroId, oneId, twoId, EventType.API_USER, ApiEventType.CREATE, then, now, null, null, null, null, SubscriptionState.ACTIVE, nextPlan, nextPhase, nextPriceList);
 		transitions.add(t);
 		
-		AccountUserApi accountApi = new BrainDeadAccountUserApi(){
-
-			@Override
-			public Account getAccountById(UUID accountId) {
-				return new BrainDeadAccount(){@Override
-				public int getBillCycleDay() {
-					return 1;
-				}};
-			}} ;
+		Account account = BrainDeadProxyFactory.createBrainDeadProxyFor(Account.class); 
+		((ZombieControl)account).addResult("getBillCycleDay", 1).addResult("getTimeZone", DateTimeZone.UTC);
+		
+		AccountUserApi accountApi = BrainDeadProxyFactory.createBrainDeadProxyFor(AccountUserApi.class);			
+		((ZombieControl)accountApi).addResult("getAccountById", account);
+				
 		DefaultEntitlementBillingApi api = new DefaultEntitlementBillingApi(dao,accountApi,catalogService);
 		SortedSet<BillingEvent> events = api.getBillingEventsForAccount(new UUID(0L,0L));
 		checkFirstEvent(events, nextPlan, bundles.get(0).getStartDate().getDayOfMonth(), oneId, now, nextPhase, ApiEventType.CREATE.toString());
diff --git a/util/src/test/java/com/ning/billing/mock/BrainDeadProxyFactory.java b/util/src/test/java/com/ning/billing/mock/BrainDeadProxyFactory.java
index 3acc4d0..31dcd30 100644
--- a/util/src/test/java/com/ning/billing/mock/BrainDeadProxyFactory.java
+++ b/util/src/test/java/com/ning/billing/mock/BrainDeadProxyFactory.java
@@ -30,9 +30,9 @@ public class BrainDeadProxyFactory {
 
 	public static interface ZombieControl {
 		
-		public void addResult(String method, Object result);
+		public ZombieControl addResult(String method, Object result);
 		
-		public void clearResults();
+		public ZombieControl clearResults();
 		
 	}
 
@@ -41,7 +41,7 @@ public class BrainDeadProxyFactory {
 		return (T) Proxy.newProxyInstance(clazz.getClassLoader(),
                 new Class[] { clazz , ZombieControl.class},
                 new InvocationHandler() {
-					private Map results = new HashMap<String,Object>();
+					private Map<String,Object> results = new HashMap<String,Object>();
 			
 					@Override
 					public Object invoke(Object proxy, Method method, Object[] args)
@@ -50,8 +50,10 @@ public class BrainDeadProxyFactory {
 						if(method.getDeclaringClass().equals(ZombieControl.class)) {
 							if(method.getName().equals("addResult")) {
 								results.put((String) args[0], args[1]);
+								return proxy;
 							} else if(method.getName().equals("clearResults")) {
 								results.clear();
+								return proxy;
 							}
 
 						} else {