Code review integration for 8637993

Throw an exception in updateBCD if the BCD is already set Simplify BCD update (Long v.s int) for consistency)
killbill · Oct 2, 2015 · ad41bf1 · ad41bf1 · pierre · Oct 3, 2015
1 parent 8637993
commit ad41bf1
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 9 deletions.
diff --git a/account/src/main/java/org/killbill/billing/account/api/svcs/DefaultAccountInternalApi.java b/account/src/main/java/org/killbill/billing/account/api/svcs/DefaultAccountInternalApi.java
@@ -92,6 +92,9 @@ public void updateBCD(final String externalKey, final int bcd,
         if (currentAccount == null) {
             throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_KEY, externalKey);
         }
+        if (currentAccount.getBillCycleDayLocal() != DefaultMutableAccountData.DEFAULT_BILLING_CYCLE_DAY_LOCAL) {
+            throw new AccountApiException(ErrorCode.ACCOUNT_UPDATE_FAILED);
+        }
 
         final MutableAccountData mutableAccountData = currentAccount.toMutableAccountData();
         mutableAccountData.setBillCycleDayLocal(bcd);
@@ -157,8 +160,8 @@ private AccountModelDao getAccountModelDaoByRecordId(final Long recordId, final
     }
 
     private int getBCDInternal(final UUID accountId, final InternalTenantContext context) {
-        final Long bcd = accountDao.getAccountBCD(accountId, context);
-        return bcd != null ? bcd.intValue() : DefaultMutableAccountData.DEFAULT_BILLING_CYCLE_DAY_LOCAL;
+        final Integer bcd = accountDao.getAccountBCD(accountId, context);
+        return bcd != null ? bcd : DefaultMutableAccountData.DEFAULT_BILLING_CYCLE_DAY_LOCAL;
     }
 
     private CacheLoaderArgument createImmutableAccountCacheLoaderArgument(final InternalTenantContext context) {

diff --git a/account/src/main/java/org/killbill/billing/account/dao/AccountDao.java b/account/src/main/java/org/killbill/billing/account/dao/AccountDao.java
@@ -51,5 +51,5 @@ public interface AccountDao extends EntityDao<AccountModelDao, Account, AccountA
 
     List<AccountEmailModelDao> getEmailsByAccountId(UUID accountId, InternalTenantContext context);
 
-    Long getAccountBCD(UUID accountId, InternalTenantContext context);
+    Integer getAccountBCD(UUID accountId, InternalTenantContext context);
 }
diff --git a/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java b/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java
@@ -42,7 +42,7 @@ public UUID getIdFromKey(@Bind("externalKey") final String key,
                              @BindBean final InternalTenantContext context);
 
     @SqlQuery
-    public Long getBCD(@Bind("id") String accountId,
+    public Integer getBCD(@Bind("id") String accountId,
                        @BindBean final InternalTenantContext context);
 
     @SqlUpdate

diff --git a/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java b/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java
@@ -247,10 +247,10 @@ public List<AccountEmailModelDao> inTransaction(final EntitySqlDaoWrapperFactory
     }
 
     @Override
-    public Long getAccountBCD(final UUID accountId, final InternalTenantContext context) {
-        return transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Long>() {
+    public Integer getAccountBCD(final UUID accountId, final InternalTenantContext context) {
+        return transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Integer>() {
             @Override
-            public Long inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
+            public Integer inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
                 return entitySqlDaoWrapperFactory.become(AccountSqlDao.class).getBCD(accountId.toString(), context);
             }
         });

diff --git a/account/src/test/java/org/killbill/billing/account/dao/MockAccountDao.java b/account/src/test/java/org/killbill/billing/account/dao/MockAccountDao.java
@@ -164,9 +164,9 @@ public boolean apply(final AccountEmailModelDao input) {
     }
 
     @Override
-    public Long getAccountBCD(final UUID accountId, final InternalTenantContext context) {
+    public Integer getAccountBCD(final UUID accountId, final InternalTenantContext context) {
         final AccountModelDao account = getById(accountId, context);
-        return account != null ? account.getBillingCycleDayLocal() : 0L;
+        return account != null ? account.getBillingCycleDayLocal() : 0;
     }
 
 }
diff --git a/...c/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java b/...c/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
@@ -25,6 +25,7 @@
 import org.killbill.billing.ObjectType;
 import org.killbill.billing.account.api.AccountApiException;
 import org.killbill.billing.account.api.AccountInternalApi;
+import org.killbill.billing.account.api.AccountUserApi;
 import org.killbill.billing.account.api.ImmutableAccountData;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.catalog.api.CatalogApiException;
@@ -191,6 +192,9 @@ private void addBillingEventsForSubscription(final ImmutableAccountData account,
                 } catch (CatalogApiException e) {
                     log.error("Failing to identify catalog components while creating BillingEvent from transition: " +
                               transition.getId().toString(), e);
+                } catch (AccountApiException e) {
+                    // This is unexpected  (failed to update BCD) but if this happens we don't want to ignore..
+                    throw e;
                 } catch (Exception e) {
                     log.warn("Failed while getting BillingEvent", e);
                 }