Details
diff --git a/beatrix/src/test/java/com/ning/billing/beatrix/integration/inv_ent/TestBasic.java b/beatrix/src/test/java/com/ning/billing/beatrix/integration/inv_ent/TestBasic.java
index ed43c33..e708803 100644
--- a/beatrix/src/test/java/com/ning/billing/beatrix/integration/inv_ent/TestBasic.java
+++ b/beatrix/src/test/java/com/ning/billing/beatrix/integration/inv_ent/TestBasic.java
@@ -176,7 +176,7 @@ public class TestBasic {
testBasePlanComplete(clock.getUTCNow().minusDays(1).getDayOfMonth());
}
- @Test(groups = "fast", enabled = false)
+ @Test(groups = "fast", enabled = true)
public void testBasePlanCompleteWithBillingDayPresent() throws Exception {
testBasePlanComplete(clock.getUTCNow().getDayOfMonth());
}
@@ -189,7 +189,7 @@ public class TestBasic {
private void testBasePlanComplete(int billingDay) throws Exception {
long DELAY = 5000 * 10;
- Account account = accountUserApi.createAccount(getAccountData(), null, null);
+ Account account = accountUserApi.createAccount(getAccountData(billingDay), null, null);
assertNotNull(account);
SubscriptionBundle bundle = entitlementUserApi.createBundleForAccount(account.getId(), "whatever");
@@ -207,7 +207,6 @@ public class TestBasic {
new PlanPhaseSpecifier(productName, ProductCategory.BASE, term, planSetName, null), null);
assertNotNull(subscription);
-
assertTrue(busHandler.isCompleted(DELAY));
log.info("testSimple passed first busHandler checkpoint.");
@@ -242,6 +241,9 @@ public class TestBasic {
busHandler.pushExpectedEvent(NextEvent.INVOICE);
clock.setDeltaFromReality(AT_LEAST_ONE_MONTH_MS);
+ Thread.sleep(600000);
+ assertTrue(busHandler.isCompleted(DELAY));
+
//
// CHANGE PLAN EOT AND EXPECT NOTHING
//
@@ -297,7 +299,7 @@ public class TestBasic {
public void testHappyPath() throws AccountApiException, EntitlementUserApiException {
long DELAY = 5000 * 10;
- Account account = accountUserApi.createAccount(getAccountData(), null, null);
+ Account account = accountUserApi.createAccount(getAccountData(3), null, null);
assertNotNull(account);
SubscriptionBundle bundle = entitlementUserApi.createBundleForAccount(account.getId(), "whatever");
@@ -331,7 +333,7 @@ public class TestBasic {
}
- protected AccountData getAccountData() {
+ protected AccountData getAccountData(final int billingDay) {
AccountData accountData = new AccountData() {
@Override
public String getName() {
@@ -355,7 +357,7 @@ public class TestBasic {
}
@Override
public int getBillCycleDay() {
- return 1;
+ return billingDay;
}
@Override
public Currency getCurrency() {
diff --git a/beatrix/src/test/resources/catalogSample.xml b/beatrix/src/test/resources/catalogSample.xml
index 898b7df..7356d74 100644
--- a/beatrix/src/test/resources/catalogSample.xml
+++ b/beatrix/src/test/resources/catalogSample.xml
@@ -202,8 +202,8 @@ Use Cases to do:
<initialPhases>
<phase type="TRIAL">
<duration>
- <unit>DAYS</unit>
- <number>30</number>
+ <unit>MONTHS</unit>
+ <number>1</number>
</duration>
<billingPeriod>MONTHLY</billingPeriod>
<fixedPrice> <!-- empty price implies $0 -->
@@ -227,10 +227,10 @@ Use Cases to do:
<initialPhases>
<phase type="TRIAL">
<duration>
- <unit>DAYS</unit>
- <number>30</number>
+ <unit>MONTHS</unit>
+ <number>1</number>
</duration>
- <billingPeriod>NO_BILLING_PERIOD</billingPeriod>
+ <billingPeriod>MONTHLY</billingPeriod>
<recurringPrice>
<price><currency>GBP</currency><value>29.95</value></price>
<price><currency>EUR</currency><value>29.95</value></price>
@@ -257,8 +257,8 @@ Use Cases to do:
<initialPhases>
<phase type="TRIAL">
<duration>
- <unit>DAYS</unit>
- <number>30</number>
+ <unit>MONTHS</unit>
+ <number>1</number>
</duration>
<billingPeriod>MONTHLY</billingPeriod>
<recurringPrice>
diff --git a/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java b/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
index 31424ba..19eaad5 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
@@ -136,6 +136,8 @@ public class DefaultInvoiceDao implements InvoiceDao {
invoiceSqlDao.inTransaction(new Transaction<Void, InvoiceSqlDao>() {
@Override
public Void inTransaction(final InvoiceSqlDao invoiceDao, final TransactionStatus status) throws Exception {
+
+ // STEPH this seems useless
Invoice currentInvoice = invoiceDao.getById(invoice.getId().toString());
if (currentInvoice == null) {
@@ -148,6 +150,8 @@ public class DefaultInvoiceDao implements InvoiceDao {
notifyOfFutureBillingEvents(invoiceSqlDao, invoiceItems);
setChargedThroughDates(invoiceSqlDao, invoiceItems);
+
+ // STEPH Why do we need that? Are the payments not always null at this point?
List<InvoicePayment> invoicePayments = invoice.getPayments();
InvoicePaymentSqlDao invoicePaymentSqlDao = invoiceDao.become(InvoicePaymentSqlDao.class);
invoicePaymentSqlDao.create(invoicePayments);
@@ -188,7 +192,7 @@ public class DefaultInvoiceDao implements InvoiceDao {
public BigDecimal getAccountBalance(final UUID accountId) {
return invoiceSqlDao.getAccountBalance(accountId.toString());
}
-
+
@Override
public void notifyOfPaymentAttempt(InvoicePayment invoicePayment) {
invoicePaymentSqlDao.notifyOfPaymentAttempt(invoicePayment);
@@ -211,6 +215,7 @@ public class DefaultInvoiceDao implements InvoiceDao {
@Override
public boolean lockAccount(final UUID accountId) {
+ /*
try {
invoiceSqlDao.lockAccount(accountId.toString());
return true;
@@ -218,16 +223,21 @@ public class DefaultInvoiceDao implements InvoiceDao {
log.error("Ouch! I broke", e);
return false;
}
+ */
+ return true;
}
@Override
public boolean releaseAccount(final UUID accountId) {
+ /*
try {
invoiceSqlDao.releaseAccount(accountId.toString());
return true;
} catch (Exception e) {
return false;
}
+ */
+ return true;
}
@Override
@@ -264,19 +274,19 @@ public class DefaultInvoiceDao implements InvoiceDao {
private void notifyOfFutureBillingEvents(final InvoiceSqlDao dao, final List<InvoiceItem> invoiceItems) {
for (final InvoiceItem item : invoiceItems) {
- if (item.getRecurringRate() != null) {
- if (item.getRecurringAmount().compareTo(BigDecimal.ZERO) > 0) {
- notifier.insertNextBillingNotification(dao, item.getSubscriptionId(), item.getEndDate());
- }
+ if ((item.getEndDate() != null) &&
+ (item.getRecurringAmount() == null || item.getRecurringAmount().compareTo(BigDecimal.ZERO) >= 0)) {
+ notifier.insertNextBillingNotification(dao, item.getSubscriptionId(), item.getEndDate());
}
}
}
private void setChargedThroughDates(final InvoiceSqlDao dao, final Collection<InvoiceItem> invoiceItems) {
- for (InvoiceItem invoiceItem : invoiceItems) {
- if (invoiceItem.getEndDate() != null) {
- log.info("Setting CTD for invoice item {} to {}", invoiceItem.getId().toString(), invoiceItem.getEndDate().toString());
- entitlementBillingApi.setChargedThroughDateFromTransaction(dao, invoiceItem.getSubscriptionId(), invoiceItem.getEndDate());
+ for (InvoiceItem item : invoiceItems) {
+ if ((item.getEndDate() != null) &&
+ (item.getRecurringAmount() == null || item.getRecurringAmount().compareTo(BigDecimal.ZERO) >= 0)) {
+ log.info("Setting CTD for invoice item {} to {}", item.getId().toString(), item.getEndDate().toString());
+ entitlementBillingApi.setChargedThroughDateFromTransaction(dao, item.getSubscriptionId(), item.getEndDate());
}
}
}
diff --git a/invoice/src/main/java/com/ning/billing/invoice/InvoiceListener.java b/invoice/src/main/java/com/ning/billing/invoice/InvoiceListener.java
index de018be..174b44d 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/InvoiceListener.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/InvoiceListener.java
@@ -42,6 +42,7 @@ import com.ning.billing.invoice.model.BillingEventSet;
import com.ning.billing.invoice.model.InvoiceGenerator;
import com.ning.billing.invoice.model.InvoiceItemList;
import com.ning.billing.invoice.notification.NextBillingDateEvent;
+import com.ning.billing.util.clock.Clock;
public class InvoiceListener {
private final static Logger log = LoggerFactory.getLogger(InvoiceListener.class);
@@ -50,17 +51,20 @@ public class InvoiceListener {
private final EntitlementBillingApi entitlementBillingApi;
private final AccountUserApi accountUserApi;
private final InvoiceDao invoiceDao;
+ private final Clock clock;
private final static boolean VERBOSE_OUTPUT = false;
@Inject
public InvoiceListener(final InvoiceGenerator generator, final AccountUserApi accountUserApi,
final EntitlementBillingApi entitlementBillingApi,
- final InvoiceDao invoiceDao) {
+ final InvoiceDao invoiceDao,
+ final Clock clock) {
this.generator = generator;
this.entitlementBillingApi = entitlementBillingApi;
this.accountUserApi = accountUserApi;
this.invoiceDao = invoiceDao;
+ this.clock = clock;
}
@Subscribe
@@ -70,7 +74,8 @@ public class InvoiceListener {
@Subscribe
public void handleNextBillingDateEvent(final NextBillingDateEvent event) {
- processSubscription(event.getSubscriptionId(), new DateTime());
+ // STEPH should we use the date of the event instead?
+ processSubscription(event.getSubscriptionId(), clock.getUTCNow());
}
private void processSubscription(final SubscriptionTransition transition) {
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 a4203d0..a8b7c88 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
@@ -75,6 +75,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
currentItems.add(new DefaultInvoiceItem(item, invoiceId));
}
+ // STEPH why clone? Why cast?
InvoiceItemList existingItems = (InvoiceItemList) existingInvoiceItems.clone();
Collections.sort(currentItems);
@@ -86,6 +87,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
// see if there are any existing items that are covered by the current item
while (it.hasNext()) {
InvoiceItem existingItem = it.next();
+ // STEPH this is more like 'contained' that 'duplicates'
if (currentItem.duplicates(existingItem)) {
currentItem.subtract(existingItem);
it.remove();
@@ -98,6 +100,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
// add existing items that aren't covered by current items as credit items
for (final InvoiceItem existingItem : existingItems) {
+ // STEPH do we really want to credit if that has not been paid yet?
currentItems.add(existingItem.asCredit(existingItem.getInvoiceId()));
}
@@ -119,7 +122,8 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
BillingEvent thisEvent = events.get(i);
BillingEvent nextEvent = events.get(i + 1);
- if (thisEvent.getSubscription().getId() == nextEvent.getSubscription().getId()) {
+
+ if (thisEvent.getSubscription().getId().equals(nextEvent.getSubscription().getId())) {
processEvents(invoiceId, thisEvent, nextEvent, items, targetDate, targetCurrency);
} else {
processEvent(invoiceId, thisEvent, items, targetDate, targetCurrency);
@@ -137,6 +141,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
private void processEvent(final UUID invoiceId, final BillingEvent event, final InvoiceItemList items,
final DateTime targetDate, final Currency targetCurrency) {
try {
+ // STEPH should not that check apply to next method as well?
if (event.getEffectiveDate().compareTo(targetDate) > 0) {
return;
}
@@ -158,10 +163,13 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
}
BigDecimal effectiveFixedPrice = items.hasInvoiceItemForPhase(event.getPlanPhase().getName()) ? null : fixedPrice;
+
+ // STEPH don't we also need to check for if (Days.daysBetween(firstEvent.getEffectiveDate(), billThroughDate).getDays() > 0)
addInvoiceItem(invoiceId, items, event, billThroughDate, recurringAmount, recurringRate, effectiveFixedPrice, targetCurrency);
} catch (CatalogApiException e) {
- log.error(String.format("Encountered a catalog error processing invoice %s for billing event on date %s",
- invoiceId.toString(),
+ // STEPH same remark for catalog exception.
+ log.error(String.format("Encountered a catalog error processing invoice %s for billing event on date %s",
+ invoiceId.toString(),
ISODateTimeFormat.basicDateTime().print(event.getEffectiveDate())), e);
}
}
@@ -191,6 +199,8 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
addInvoiceItem(invoiceId, items, firstEvent, billThroughDate, recurringAmount, recurringRate, effectiveFixedPrice, targetCurrency);
}
} catch (CatalogApiException e) {
+
+ // STEPH That needs to be thrown so we stop that invoice generation
log.error(String.format("Encountered a catalog error processing invoice %s for billing event on date %s",
invoiceId.toString(),
ISODateTimeFormat.basicDateTime().print(firstEvent.getEffectiveDate())), e);
diff --git a/invoice/src/main/java/com/ning/billing/invoice/model/InAdvanceBillingMode.java b/invoice/src/main/java/com/ning/billing/invoice/model/InAdvanceBillingMode.java
index 0d5b261..b8f5c42 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/model/InAdvanceBillingMode.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/model/InAdvanceBillingMode.java
@@ -20,6 +20,7 @@ import com.ning.billing.catalog.api.BillingPeriod;
import org.joda.time.DateTime;
import org.joda.time.Days;
import org.joda.time.Months;
+import org.joda.time.MutableDateTime;
import java.math.BigDecimal;
@@ -50,24 +51,23 @@ public class InAdvanceBillingMode extends BillingModeBase {
protected DateTime calculateBillingCycleDateOnOrAfter(final DateTime date, final int billingCycleDay) {
int lastDayOfMonth = date.dayOfMonth().getMaximumValue();
- DateTime proposedDate;
+
+ MutableDateTime tmp = date.toMutableDateTime();
if (billingCycleDay > lastDayOfMonth) {
- proposedDate = new DateTime(date.getYear(), date.getMonthOfYear(), lastDayOfMonth,
- date.getHourOfDay(), date.getMinuteOfHour(),
- date.getSecondOfMinute(), date.getMillisOfSecond());
+ tmp.setDayOfMonth(lastDayOfMonth);
} else {
- proposedDate = new DateTime(date.getYear(), date.getMonthOfYear(), billingCycleDay,
- date.getHourOfDay(), date.getMinuteOfHour(),
- date.getSecondOfMinute(), date.getMillisOfSecond());
+ tmp.setDayOfMonth(billingCycleDay);
}
+ DateTime proposedDate = tmp.toDateTime();
while (proposedDate.isBefore(date)) {
+ // STEPH could be an annual ?
proposedDate = proposedDate.plusMonths(1);
}
-
return proposedDate;
}
+ @Override
protected DateTime calculateBillingCycleDateAfter(final DateTime date, final DateTime billingCycleDate, final int billingCycleDay, final BillingPeriod billingPeriod) {
DateTime proposedDate = billingCycleDate;
