Details
diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml
index 1eb38a9..1d459ec 100644
--- a/.idea/inspectionProfiles/Project_Default.xml
+++ b/.idea/inspectionProfiles/Project_Default.xml
@@ -28,7 +28,7 @@
<option name="useParameterizedTypeForCollectionMethods" value="true" />
<option name="doNotWeakenToJavaLangObject" value="true" />
<option name="onlyWeakentoInterface" value="true" />
- <stopClasses>org.killbill.billing.catalog.api.Plan</stopClasses>
+ <stopClasses>org.killbill.billing.catalog.api.Plan,org.killbill.billing.junction.BillingEventSet</stopClasses>
</inspection_tool>
<inspection_tool class="UnnecessaryConstantArrayCreationExpression" enabled="true" level="WARNING" enabled_by_default="true" />
<inspection_tool class="UseOfJDBCDriverClass" enabled="true" level="WARNING" enabled_by_default="true" />
diff --git a/account/src/main/java/org/killbill/billing/account/api/DefaultAccount.java b/account/src/main/java/org/killbill/billing/account/api/DefaultAccount.java
index 5e8e383..d36d976 100644
--- a/account/src/main/java/org/killbill/billing/account/api/DefaultAccount.java
+++ b/account/src/main/java/org/killbill/billing/account/api/DefaultAccount.java
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
@@ -304,6 +304,7 @@ public class DefaultAccount extends EntityBase implements Account {
* @return merged account data
*/
@Override
+ @Deprecated // TODO Get rid of this in 0.22
public Account mergeWithDelegate(final Account currentAccount) {
final DefaultMutableAccountData accountData = new DefaultMutableAccountData(this);
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
index 44e8b4d..8cf856a 100644
--- 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
@@ -101,7 +101,7 @@ public class DefaultAccountInternalApi extends DefaultAccountApiBase implements
final AccountModelDao accountToUpdate = new AccountModelDao(currentAccount.getId(), mutableAccountData);
bcdCacheController.remove(currentAccount.getId());
bcdCacheController.putIfAbsent(currentAccount.getId(), new Integer(bcd));
- accountDao.update(accountToUpdate, context);
+ accountDao.update(accountToUpdate, true, context);
}
@Override
diff --git a/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java b/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java
index be52cb0..c69c1a1 100644
--- a/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java
+++ b/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
@@ -156,30 +156,12 @@ public class DefaultAccountUserApi extends DefaultAccountApiBase implements Acco
@Override
public void updateAccount(final Account account, final CallContext context) throws AccountApiException {
-
- // Convert to DefaultAccount to make sure we can safely call validateAccountUpdateInput
- final DefaultAccount input = new DefaultAccount(account.getId(), account);
-
- final Account currentAccount = getAccountById(input.getId(), context);
- if (currentAccount == null) {
- throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_ID, input.getId());
- }
-
- input.validateAccountUpdateInput(currentAccount, true);
-
- final AccountModelDao updatedAccountModelDao = new AccountModelDao(currentAccount.getId(), input);
-
- accountDao.update(updatedAccountModelDao, internalCallContextFactory.createInternalCallContext(updatedAccountModelDao.getId(), context));
+ updateAccount(account.getId(), account, true, context);
}
@Override
public void updateAccount(final UUID accountId, final AccountData accountData, final CallContext context) throws AccountApiException {
- final Account currentAccount = getAccountById(accountId, context);
- if (currentAccount == null) {
- throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_ID, accountId);
- }
-
- updateAccount(currentAccount, accountData, context);
+ updateAccount(accountId, accountData, false, context);
}
@Override
@@ -189,18 +171,16 @@ public class DefaultAccountUserApi extends DefaultAccountApiBase implements Acco
throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_KEY, externalKey);
}
- updateAccount(currentAccount, accountData, context);
+ updateAccount(currentAccount.getId(), accountData, false, context);
}
- private void updateAccount(final Account currentAccount, final AccountData accountData, final CallContext context) throws AccountApiException {
- final Account updatedAccount = new DefaultAccount(currentAccount.getId(), accountData);
-
- // Set unspecified (null) fields to their current values
- final Account mergedAccount = updatedAccount.mergeWithDelegate(currentAccount);
-
- final AccountModelDao updatedAccountModelDao = new AccountModelDao(currentAccount.getId(), mergedAccount);
-
- accountDao.update(updatedAccountModelDao, internalCallContextFactory.createInternalCallContext(updatedAccountModelDao.getId(), context));
+ private void updateAccount(final UUID accountId, final AccountData account, final boolean treatNullValueAsReset, final CallContext context) throws AccountApiException {
+ final AccountModelDao updatedAccountModelDao = new AccountModelDao(accountId,
+ null,
+ null,
+ account,
+ false);
+ accountDao.update(updatedAccountModelDao, treatNullValueAsReset, internalCallContextFactory.createInternalCallContext(accountId, context));
}
@Override
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
index 81de578..dfd8722 100644
--- a/account/src/main/java/org/killbill/billing/account/dao/AccountDao.java
+++ b/account/src/main/java/org/killbill/billing/account/dao/AccountDao.java
@@ -1,7 +1,9 @@
/*
* Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
- * Ning licenses this file to you under the Apache License, version 2.0
+ * The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at:
*
@@ -45,7 +47,7 @@ public interface AccountDao extends EntityDao<AccountModelDao, Account, AccountA
*/
void updatePaymentMethod(UUID accountId, UUID paymentMethodId, InternalCallContext context) throws AccountApiException;
- void update(AccountModelDao account, InternalCallContext context) throws AccountApiException;
+ void update(AccountModelDao account, boolean treatNullValueAsReset, InternalCallContext context) throws AccountApiException;
void addEmail(AccountEmailModelDao email, InternalCallContext context) throws AccountApiException;
@@ -60,5 +62,4 @@ public interface AccountDao extends EntityDao<AccountModelDao, Account, AccountA
List<AuditLogWithHistory> getAuditLogsWithHistoryForId(UUID accountId, AuditLevel auditLevel, InternalTenantContext context) throws AccountApiException;
List<AuditLogWithHistory> getEmailAuditLogsWithHistoryForId(UUID accountEmailId, AuditLevel auditLevel, InternalTenantContext context) throws AccountApiException;
-
}
diff --git a/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java b/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java
index 5154025..6146f66 100644
--- a/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java
+++ b/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
@@ -64,15 +64,15 @@ public class AccountModelDao extends EntityModelDaoBase implements TimeZoneAware
public AccountModelDao() { /* For the DAO mapper */ }
- public AccountModelDao(final UUID id, final DateTime createdDate, final DateTime updatedDate, final String externalKey,
- final String email, final String name, final Integer firstNameLength, final Currency currency,
- final UUID parentAccountId, final Boolean isPaymentDelegatedToParent,
- final int billingCycleDayLocal, final UUID paymentMethodId, final DateTime referenceTime, final DateTimeZone timeZone,
- final String locale, final String address1, final String address2, final String companyName,
- final String city, final String stateOrProvince, final String country, final String postalCode,
- final String phone, final String notes, final Boolean migrated) {
+ private AccountModelDao(final UUID id, final DateTime createdDate, final DateTime updatedDate, final String externalKey,
+ final String email, final String name, final Integer firstNameLength, final Currency currency,
+ final UUID parentAccountId, final Boolean isPaymentDelegatedToParent,
+ final int billingCycleDayLocal, final UUID paymentMethodId, final DateTime referenceTime, final DateTimeZone timeZone,
+ final String locale, final String address1, final String address2, final String companyName,
+ final String city, final String stateOrProvince, final String country, final String postalCode,
+ final String phone, final String notes, final Boolean migrated, final boolean withDefaults) {
super(id, createdDate, updatedDate);
- this.externalKey = MoreObjects.firstNonNull(externalKey, id.toString());
+ this.externalKey = !withDefaults ? externalKey : MoreObjects.firstNonNull(externalKey, id.toString());
this.email = email;
this.name = name;
this.firstNameLength = firstNameLength;
@@ -82,7 +82,7 @@ public class AccountModelDao extends EntityModelDaoBase implements TimeZoneAware
this.billingCycleDayLocal = billingCycleDayLocal;
this.paymentMethodId = paymentMethodId;
this.referenceTime = referenceTime;
- this.timeZone = MoreObjects.firstNonNull(timeZone, DateTimeZone.UTC);
+ this.timeZone = !withDefaults ? timeZone : MoreObjects.firstNonNull(timeZone, DateTimeZone.UTC);
this.locale = locale;
this.address1 = address1;
this.address2 = address2;
@@ -96,7 +96,7 @@ public class AccountModelDao extends EntityModelDaoBase implements TimeZoneAware
this.migrated = migrated;
}
- private AccountModelDao(final UUID id, @Nullable final DateTime createdDate, @Nullable final DateTime updatedDate, final AccountData account) {
+ public AccountModelDao(final UUID id, @Nullable final DateTime createdDate, @Nullable final DateTime updatedDate, final AccountData account, final boolean withDefaults) {
this(id,
createdDate,
updatedDate,
@@ -121,19 +121,101 @@ public class AccountModelDao extends EntityModelDaoBase implements TimeZoneAware
account.getPostalCode(),
account.getPhone(),
account.getNotes(),
- account.isMigrated());
+ account.isMigrated(),
+ withDefaults);
}
public AccountModelDao(final UUID accountId, final AccountData account) {
- this(accountId, null, null, account);
+ this(accountId, null, null, account, true);
}
-
public AccountModelDao(final AccountData account) {
- this(UUIDs.randomUUID(), null, null, account);
+ this(UUIDs.randomUUID(), null, null, account, true);
+ }
+
+ public void mergeWithDelegate(final AccountModelDao currentAccount) {
+ setExternalKey(currentAccount.getExternalKey());
+
+ setCurrency(currentAccount.getCurrency());
+
+ if (currentAccount.getBillingCycleDayLocal() == DEFAULT_BILLING_CYCLE_DAY_LOCAL && // There is *not* already a BCD set
+ billingCycleDayLocal != DEFAULT_BILLING_CYCLE_DAY_LOCAL) { // and the proposed date is not 0
+ setBillingCycleDayLocal(billingCycleDayLocal);
+ } else {
+ setBillingCycleDayLocal(currentAccount.getBillingCycleDayLocal());
+ }
+
+ // Set all updatable fields with the new values if non null, otherwise defaults to the current values
+ setEmail(email != null ? email : currentAccount.getEmail());
+ setName(name != null ? name : currentAccount.getName());
+ final Integer firstNameLength = this.firstNameLength != null ? this.firstNameLength : currentAccount.getFirstNameLength();
+ if (firstNameLength != null) {
+ setFirstNameLength(firstNameLength);
+ }
+ setPaymentMethodId(paymentMethodId != null ? paymentMethodId : currentAccount.getPaymentMethodId());
+ setTimeZone(timeZone != null ? timeZone : currentAccount.getTimeZone());
+ setLocale(locale != null ? locale : currentAccount.getLocale());
+ setAddress1(address1 != null ? address1 : currentAccount.getAddress1());
+ setAddress2(address2 != null ? address2 : currentAccount.getAddress2());
+ setCompanyName(companyName != null ? companyName : currentAccount.getCompanyName());
+ setCity(city != null ? city : currentAccount.getCity());
+ setStateOrProvince(stateOrProvince != null ? stateOrProvince : currentAccount.getStateOrProvince());
+ setCountry(country != null ? country : currentAccount.getCountry());
+ setPostalCode(postalCode != null ? postalCode : currentAccount.getPostalCode());
+ setPhone(phone != null ? phone : currentAccount.getPhone());
+ setNotes(notes != null ? notes : currentAccount.getNotes());
+ setParentAccountId(parentAccountId != null ? parentAccountId : currentAccount.getParentAccountId());
+ setIsPaymentDelegatedToParent(isPaymentDelegatedToParent != null ? isPaymentDelegatedToParent : currentAccount.getIsPaymentDelegatedToParent());
+ final Boolean isMigrated = this.migrated != null ? this.migrated : currentAccount.getMigrated();
+ if (isMigrated != null) {
+ setMigrated(isMigrated);
+ }
}
+ public void validateAccountUpdateInput(final AccountModelDao currentAccount, final boolean ignoreNullInput) {
+ //
+ // We don't allow update on the following fields:
+ //
+ // All these conditions are written in the exact same way:
+ //
+ // There is already a defined value BUT those don't match (either input is null or different) => Not Allowed
+ // * ignoreNullInput = false (case where we allow to reset values)
+ // * ignoreNullInput = true (case where we DON'T allow to reset values and so if such value is null we ignore the check)
+ //
+ //
+ if ((ignoreNullInput || externalKey != null) &&
+ currentAccount.getExternalKey() != null &&
+ !currentAccount.getExternalKey().equals(externalKey)) {
+ throw new IllegalArgumentException(String.format("Killbill doesn't support updating the account external key: new=%s, current=%s",
+ externalKey, currentAccount.getExternalKey()));
+ }
+
+ if ((ignoreNullInput || currency != null) &&
+ currentAccount.getCurrency() != null &&
+ !currentAccount.getCurrency().equals(currency)) {
+ throw new IllegalArgumentException(String.format("Killbill doesn't support updating the account currency: new=%s, current=%s",
+ currency, currentAccount.getCurrency()));
+ }
+
+ if ((ignoreNullInput || (billingCycleDayLocal != DEFAULT_BILLING_CYCLE_DAY_LOCAL)) &&
+ currentAccount.getBillingCycleDayLocal() != DEFAULT_BILLING_CYCLE_DAY_LOCAL && // There is already a BCD set
+ !currentAccount.getBillingCycleDayLocal().equals(billingCycleDayLocal)) { // and it does not match we we have
+ throw new IllegalArgumentException(String.format("Killbill doesn't support updating the account BCD: new=%s, current=%s", billingCycleDayLocal, currentAccount.getBillingCycleDayLocal()));
+ }
+
+ if ((ignoreNullInput || timeZone != null) &&
+ currentAccount.getTimeZone() != null &&
+ !currentAccount.getTimeZone().equals(timeZone)) {
+ throw new IllegalArgumentException(String.format("Killbill doesn't support updating the account timeZone: new=%s, current=%s",
+ timeZone, currentAccount.getTimeZone()));
+ }
+
+ if (referenceTime != null && currentAccount.getReferenceTime().withMillisOfDay(0).compareTo(referenceTime.withMillisOfDay(0)) != 0) {
+ throw new IllegalArgumentException(String.format("Killbill doesn't support updating the account referenceTime: new=%s, current=%s",
+ referenceTime, currentAccount.getReferenceTime()));
+ }
+ }
@Override
public void setRecordId(final Long recordId) {
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
index 909db49..91cd877 100644
--- a/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java
+++ b/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java
@@ -197,7 +197,7 @@ public class DefaultAccountDao extends EntityDaoBase<AccountModelDao, Account, A
}
@Override
- public void update(final AccountModelDao specifiedAccount, final InternalCallContext context) throws AccountApiException {
+ public void update(final AccountModelDao specifiedAccount, final boolean treatNullValueAsReset, final InternalCallContext context) throws AccountApiException {
transactionalSqlDao.execute(false, AccountApiException.class, new EntitySqlDaoTransactionWrapper<Void>() {
@Override
public Void inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws EventBusException, AccountApiException {
@@ -209,6 +209,13 @@ public class DefaultAccountDao extends EntityDaoBase<AccountModelDao, Account, A
throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_ID, accountId);
}
+ specifiedAccount.validateAccountUpdateInput(currentAccount, treatNullValueAsReset);
+
+ if (!treatNullValueAsReset) {
+ // Set unspecified (null) fields to their current values
+ specifiedAccount.mergeWithDelegate(currentAccount);
+ }
+
transactional.update(specifiedAccount, context);
final AccountChangeInternalEvent changeEvent = new DefaultAccountChangeEvent(accountId,
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
index 69e8199..72970a3 100644
--- a/account/src/test/java/org/killbill/billing/account/dao/MockAccountDao.java
+++ b/account/src/test/java/org/killbill/billing/account/dao/MockAccountDao.java
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2016 Groupon, Inc
- * Copyright 2014-2016 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
@@ -81,7 +81,7 @@ public class MockAccountDao extends MockEntityDaoBase<AccountModelDao, Account,
}
@Override
- public void update(final AccountModelDao account, final InternalCallContext context) {
+ public void update(final AccountModelDao account, final boolean treatNullValueAsReset, final InternalCallContext context) {
super.update(account, context);
final AccountModelDao currentAccount = getById(account.getId(), context);
diff --git a/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java b/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java
index fabbe60..b9790d2 100644
--- a/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java
+++ b/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
@@ -84,6 +84,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testBasic() throws AccountApiException {
final AccountModelDao account = createTestAccount();
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
// Retrieve by key
AccountModelDao retrievedAccount = accountDao.getAccountByKey(account.getExternalKey(), internalCallContext);
@@ -143,6 +144,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testLongPhoneNumber() throws AccountApiException {
final AccountModelDao account = createTestAccount("123456789012345678901234");
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
final AccountModelDao retrievedAccount = accountDao.getAccountByKey(account.getExternalKey(), internalCallContext);
checkAccountsEqual(retrievedAccount, account);
@@ -198,6 +200,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testGetIdFromKey() throws AccountApiException {
final AccountModelDao account = createTestAccount();
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
final UUID accountId = accountDao.getIdFromKey(account.getExternalKey(), internalCallContext);
Assert.assertEquals(accountId, account.getId());
@@ -212,6 +215,8 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testUpdate() throws Exception {
final AccountModelDao account = createTestAccount();
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
+
final AccountModelDao createdAccount = accountDao.getAccountByKey(account.getExternalKey(), internalCallContext);
List<AuditLogWithHistory> auditLogsWithHistory = accountDao.getAuditLogsWithHistoryForId(account.getId(), AuditLevel.FULL, internalCallContext);
Assert.assertEquals(auditLogsWithHistory.size(), 1);
@@ -226,11 +231,10 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
Assert.assertEquals(history1.getLocale(), createdAccount.getLocale());
final AccountData accountData = new MockAccountBuilder(new DefaultAccount(account)).migrated(false)
- .timeZone(DateTimeZone.forID("Australia/Darwin"))
.locale("FR-CA")
.build();
final AccountModelDao updatedAccount = new AccountModelDao(account.getId(), accountData);
- accountDao.update(updatedAccount, internalCallContext);
+ accountDao.update(updatedAccount, true, internalCallContext);
final AccountModelDao retrievedAccount = accountDao.getAccountByKey(account.getExternalKey(), internalCallContext);
checkAccountsEqual(retrievedAccount, updatedAccount);
@@ -250,7 +254,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
final AccountData accountData2 = new MockAccountBuilder(new DefaultAccount(updatedAccount)).locale("en_US")
.build();
final AccountModelDao updatedAccount2 = new AccountModelDao(account.getId(), accountData2);
- accountDao.update(updatedAccount2, internalCallContext);
+ accountDao.update(updatedAccount2, true, internalCallContext);
final AccountModelDao retrievedAccount2 = accountDao.getAccountByKey(account.getExternalKey(), internalCallContext);
checkAccountsEqual(retrievedAccount2, updatedAccount2);
@@ -273,6 +277,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testUpdatePaymentMethod() throws Exception {
final AccountModelDao account = createTestAccount();
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
final UUID newPaymentMethodId = UUID.randomUUID();
accountDao.updatePaymentMethod(account.getId(), newPaymentMethodId, internalCallContext);
@@ -291,12 +296,13 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testShouldBeAbleToUpdateSomeFields() throws Exception {
final AccountModelDao account = createTestAccount();
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
final MutableAccountData otherAccount = new DefaultAccount(account).toMutableAccountData();
otherAccount.setAddress1(UUID.randomUUID().toString());
otherAccount.setEmail(UUID.randomUUID().toString());
final AccountModelDao newAccount = new AccountModelDao(account.getId(), otherAccount);
- accountDao.update(newAccount, internalCallContext);
+ accountDao.update(newAccount, true, internalCallContext);
final AccountModelDao retrievedAccount = accountDao.getById(account.getId(), internalCallContext);
checkAccountsEqual(retrievedAccount, newAccount);
@@ -306,6 +312,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
public void testShouldBeAbleToHandleBCDOfZero() throws Exception {
final AccountModelDao account = createTestAccount(0);
accountDao.create(account, internalCallContext);
+ refreshCallContext(account.getId());
// Same BCD (zero)
final AccountModelDao retrievedAccount = accountDao.getById(account.getId(), internalCallContext);
diff --git a/api/src/main/java/org/killbill/billing/junction/BillingInternalApi.java b/api/src/main/java/org/killbill/billing/junction/BillingInternalApi.java
index 52d1ec3..18ab04b 100644
--- a/api/src/main/java/org/killbill/billing/junction/BillingInternalApi.java
+++ b/api/src/main/java/org/killbill/billing/junction/BillingInternalApi.java
@@ -1,7 +1,9 @@
/*
- * Copyright 2010-2012 Ning, Inc.
+ * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
*
- * Ning licenses this file to you under the Apache License, version 2.0
+ * The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at:
*
@@ -27,6 +29,8 @@ import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
public interface BillingInternalApi {
/**
+ * Note: this method assumes the lock is taken (https://github.com/killbill/killbill/issues/282)
+ *
* @return an ordered list of billing event for the given accounts
*/
public BillingEventSet getBillingEventsForAccountAndUpdateAccountBCD(UUID accountId, DryRunArguments dryRunArguments, InternalCallContext context) throws CatalogApiException, AccountApiException, SubscriptionBaseApiException;
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithBCDUpdate.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithBCDUpdate.java
index d418097..56f11cb 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithBCDUpdate.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithBCDUpdate.java
@@ -814,7 +814,7 @@ public class TestWithBCDUpdate extends TestIntegrationBase {
// Record usage for first month
recordUsageData(aoSubscription.getId(), "tracking-1", "bullets", new LocalDate(2012, 4, 5), 100L, callContext);
- recordUsageData(aoSubscription.getId(), "tracking-1", "bullets", new LocalDate(2012, 4, 15), 100L, callContext);
+ recordUsageData(aoSubscription.getId(), "tracking-2", "bullets", new LocalDate(2012, 4, 15), 100L, callContext);
// 2012-05-01
busHandler.pushExpectedEvents(NextEvent.PHASE, NextEvent.NULL_INVOICE, NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
@@ -843,8 +843,8 @@ public class TestWithBCDUpdate extends TestIntegrationBase {
new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 1), new LocalDate(2012, 5, 5), InvoiceItemType.USAGE, BigDecimal.ZERO));
// Record usage for second month
- recordUsageData(aoSubscription.getId(), "tracking-1", "bullets", new LocalDate(2012, 5, 5), 100L, callContext);
- recordUsageData(aoSubscription.getId(), "tracking-1", "bullets", new LocalDate(2012, 6, 4), 100L, callContext);
+ recordUsageData(aoSubscription.getId(), "tracking-3", "bullets", new LocalDate(2012, 5, 5), 100L, callContext);
+ recordUsageData(aoSubscription.getId(), "tracking-4", "bullets", new LocalDate(2012, 6, 4), 100L, callContext);
// 2012-06-01
busHandler.pushExpectedEvents(NextEvent.NULL_INVOICE);
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
index 271b15c..767cd30 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
@@ -174,9 +174,7 @@ public class InvoiceDispatcher {
this.parkedAccountsManager = parkedAccountsManager;
}
-
public void processSubscriptionStartRequestedDate(final RequestedSubscriptionInternalEvent transition, final InternalCallContext context) {
-
final long dryRunNotificationTime = invoiceConfig.getDryRunNotificationSchedule(context).getMillis();
final boolean isInvoiceNotificationEnabled = dryRunNotificationTime > 0;
if (!isInvoiceNotificationEnabled) {
@@ -186,16 +184,28 @@ public class InvoiceDispatcher {
final UUID accountId;
try {
accountId = subscriptionApi.getAccountIdFromSubscriptionId(transition.getSubscriptionId(), context);
-
-
} catch (final SubscriptionBaseApiException e) {
log.warn("Failed handling SubscriptionBase change.",
new InvoiceApiException(ErrorCode.INVOICE_NO_ACCOUNT_ID_FOR_SUBSCRIPTION_ID, transition.getSubscriptionId().toString()));
return;
}
+ GlobalLock lock = null;
try {
+ lock = locker.lockWithNumberOfTries(LockerType.ACCNT_INV_PAY.toString(), accountId.toString(), invoiceConfig.getMaxGlobalLockRetries());
+
+ processSubscriptionStartRequestedDateWithLock(accountId, transition, context);
+ } catch (final LockFailedException e) {
+ log.warn("Failed to process RequestedSubscriptionInternalEvent for accountId='{}'", accountId.toString(), e);
+ } finally {
+ if (lock != null) {
+ lock.release();
+ }
+ }
+ }
+ private void processSubscriptionStartRequestedDateWithLock(final UUID accountId, final RequestedSubscriptionInternalEvent transition, final InternalCallContext context) {
+ try {
final BillingEventSet billingEvents = billingApi.getBillingEventsForAccountAndUpdateAccountBCD(accountId, null, context);
if (billingEvents.isEmpty()) {
return;
@@ -207,7 +217,6 @@ public class InvoiceDispatcher {
final ImmutableAccountData account = accountApi.getImmutableAccountDataById(accountId, context);
commitInvoiceAndSetFutureNotifications(account, notificationsBuilder.build(), context);
-
} catch (final SubscriptionBaseApiException e) {
log.warn("Failed handling SubscriptionBase change.",
new InvoiceApiException(ErrorCode.INVOICE_NO_ACCOUNT_ID_FOR_SUBSCRIPTION_ID, transition.getSubscriptionId().toString()));
diff --git a/jaxrs/src/main/java/org/killbill/billing/jaxrs/json/AccountJson.java b/jaxrs/src/main/java/org/killbill/billing/jaxrs/json/AccountJson.java
index d72ec1d..4711e07 100644
--- a/jaxrs/src/main/java/org/killbill/billing/jaxrs/json/AccountJson.java
+++ b/jaxrs/src/main/java/org/killbill/billing/jaxrs/json/AccountJson.java
@@ -297,7 +297,7 @@ public class AccountJson extends JsonBase {
@Override
public Account mergeWithDelegate(final Account delegate) {
- return null;
+ throw new UnsupportedOperationException();
}
};
}
diff --git a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
index 50c2af3..a096e5e 100644
--- a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
+++ b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
@@ -139,12 +139,35 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
}
}
- private void addBillingEventsForBundles(final List<SubscriptionBaseBundle> bundles, final ImmutableAccountData account, final DryRunArguments dryRunArguments, final InternalCallContext context,
- final DefaultBillingEventSet result, final Set<UUID> skipSubscriptionsSet, final Catalog catalog, final List<Tag> tagsForAccount) throws AccountApiException, CatalogApiException, SubscriptionBaseApiException {
- final boolean dryRunMode = dryRunArguments != null;
-
+ private void addBillingEventsForBundles(final List<SubscriptionBaseBundle> bundles,
+ final ImmutableAccountData account,
+ final DryRunArguments dryRunArguments,
+ final InternalCallContext context,
+ final DefaultBillingEventSet result,
+ final Set<UUID> skipSubscriptionsSet,
+ final Catalog catalog,
+ final List<Tag> tagsForAccount) throws AccountApiException, CatalogApiException, SubscriptionBaseApiException {
final int currentAccountBCD = accountApi.getBCD(context);
+ addBillingEventsForBundles(bundles,
+ account,
+ dryRunArguments,
+ context,
+ result,
+ skipSubscriptionsSet,
+ catalog,
+ tagsForAccount,
+ currentAccountBCD);
+ }
+ private void addBillingEventsForBundles(final List<SubscriptionBaseBundle> bundles,
+ final ImmutableAccountData account,
+ final DryRunArguments dryRunArguments,
+ final InternalCallContext context,
+ final DefaultBillingEventSet result,
+ final Set<UUID> skipSubscriptionsSet,
+ final Catalog catalog,
+ final List<Tag> tagsForAccount,
+ final int currentAccountBCD) throws AccountApiException, CatalogApiException, SubscriptionBaseApiException {
// In dryRun mode, when we care about invoice generated for new BASE subscription, no such bundle exists yet; we still
// want to tap into subscriptionBase logic, so we make up a bundleId
if (dryRunArguments != null &&
@@ -185,46 +208,58 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
}
// If dryRun is specified, we don't want to to update the account BCD value, so we initialize the flag updatedAccountBCD to true
- if (currentAccountBCD == 0 && !dryRunMode) {
- BillingEvent oldestAccountAlignedBillingEvent = null;
-
- for (final BillingEvent event : result) {
- if (event.getBillingAlignment() != BillingAlignment.ACCOUNT) {
- continue;
- }
+ if (currentAccountBCD == 0) {
+ final Integer accountBCDCandidate = computeAccountBCD(result);
+ if (accountBCDCandidate == null) {
+ return;
+ }
- final BigDecimal recurringPrice = event.getRecurringPrice(event.getEffectiveDate());
- final boolean hasRecurringPrice = recurringPrice != null; // Note: could be zero (BCD would still be set, by convention)
- final boolean hasUsage = event.getUsages() != null && !event.getUsages().isEmpty();
- if (!hasRecurringPrice &&
- !hasUsage) {
- // Nothing to bill, ignored for the purpose of BCD calculation
- continue;
- }
+ // Because we now have computed the real BCD, we need to re-compute the BillingEvents BCD for ACCOUNT alignments (see BillCycleDayCalculator#calculateBcdForAlignment).
+ // The code could maybe be optimized (no need to re-run the full function?), but since it's run once per account, it's probably not worth it.
+ result.clear();
+ addBillingEventsForBundles(bundles, account, dryRunArguments, context, result, skipSubscriptionsSet, catalog, tagsForAccount, accountBCDCandidate);
- if (oldestAccountAlignedBillingEvent == null ||
- event.getEffectiveDate().compareTo(oldestAccountAlignedBillingEvent.getEffectiveDate()) < 0 ||
- (event.getEffectiveDate().compareTo(oldestAccountAlignedBillingEvent.getEffectiveDate()) == 0 && event.getTotalOrdering().compareTo(oldestAccountAlignedBillingEvent.getTotalOrdering()) < 0)) {
- oldestAccountAlignedBillingEvent = event;
- }
+ final boolean dryRunMode = dryRunArguments != null;
+ if (!dryRunMode) {
+ log.info("Setting account BCD='{}', accountId='{}'", accountBCDCandidate, account.getId());
+ accountApi.updateBCD(account.getExternalKey(), accountBCDCandidate, context);
}
+ }
+ }
- if (oldestAccountAlignedBillingEvent == null) {
- return;
+ private Integer computeAccountBCD(final BillingEventSet result) throws CatalogApiException {
+ BillingEvent oldestAccountAlignedBillingEvent = null;
+
+ for (final BillingEvent event : result) {
+ if (event.getBillingAlignment() != BillingAlignment.ACCOUNT) {
+ continue;
}
- // BCD in the account timezone
- final int accountBCDCandidate = oldestAccountAlignedBillingEvent.getBillCycleDayLocal();
- Preconditions.checkState(accountBCDCandidate > 0, "Wrong Account BCD calculation for event: " + oldestAccountAlignedBillingEvent);
+ final BigDecimal recurringPrice = event.getRecurringPrice(event.getEffectiveDate());
+ final boolean hasRecurringPrice = recurringPrice != null; // Note: could be zero (BCD would still be set, by convention)
+ final boolean hasUsage = event.getUsages() != null && !event.getUsages().isEmpty();
+ if (!hasRecurringPrice &&
+ !hasUsage) {
+ // Nothing to bill, ignored for the purpose of BCD calculation
+ continue;
+ }
- log.info("Setting account BCD='{}', accountId='{}'", accountBCDCandidate, account.getId());
- accountApi.updateBCD(account.getExternalKey(), accountBCDCandidate, context);
+ if (oldestAccountAlignedBillingEvent == null ||
+ event.getEffectiveDate().compareTo(oldestAccountAlignedBillingEvent.getEffectiveDate()) < 0 ||
+ (event.getEffectiveDate().compareTo(oldestAccountAlignedBillingEvent.getEffectiveDate()) == 0 && event.getTotalOrdering().compareTo(oldestAccountAlignedBillingEvent.getTotalOrdering()) < 0)) {
+ oldestAccountAlignedBillingEvent = event;
+ }
+ }
- // Because we now have computed the real BCD, we need to re-compute the BillingEvents BCD for ACCOUNT alignments (see BillCycleDayCalculator#calculateBcdForAlignment).
- // The code could maybe be optimized (no need to re-run the full function?), but since it's run once per account, it's probably not worth it.
- result.clear();
- addBillingEventsForBundles(bundles, account, dryRunArguments, context, result, skipSubscriptionsSet, catalog, tagsForAccount);
+ if (oldestAccountAlignedBillingEvent == null) {
+ return null;
}
+
+ // BCD in the account timezone
+ final int accountBCDCandidate = oldestAccountAlignedBillingEvent.getBillCycleDayLocal();
+ Preconditions.checkState(accountBCDCandidate > 0, "Wrong Account BCD calculation for event: " + oldestAccountAlignedBillingEvent);
+
+ return accountBCDCandidate;
}
private void addBillingEventsForSubscription(final ImmutableAccountData account,
