killbill-memoizeit

junction: revisit account BCD calculation based on code review Change

12/3/2018 7:59:28 AM

Details

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 c2cb1ca..652f4ea 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
@@ -622,8 +622,7 @@ public class TestWithBCDUpdate extends TestIntegrationBase {
         final DateTime initialDate = new DateTime(2016, 5, 1, 0, 13, 42, 0, testTimeZone);
         clock.setTime(initialDate);
 
-        // Set account BCD to the first
-        final Account account = createAccountWithNonOsgiPaymentMethod(getAccountData(1));
+        final Account account = createAccountWithNonOsgiPaymentMethod(getAccountData(0));
         assertNotNull(account);
 
         final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Blowdart", BillingPeriod.MONTHLY, "notrial", null);
@@ -632,8 +631,11 @@ public class TestWithBCDUpdate extends TestIntegrationBase {
         final List<PlanPhasePriceOverride> overrides = new ArrayList<PlanPhasePriceOverride>();
         overrides.add(new DefaultPlanPhasePriceOverride("blowdart-monthly-notrial-evergreen", account.getCurrency(), null, BigDecimal.ZERO, ImmutableList.<UsagePriceOverride>of()));
         busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK, NextEvent.INVOICE);
+        // BP creation : Will set Account BCD to the first (DateOfFirstRecurringNonZeroCharge is the subscription start date in this case)
         final UUID baseEntitlementId = entitlementApi.createBaseEntitlement(account.getId(), new DefaultEntitlementSpecifier(spec, null, overrides), "bundleExternalKey", null, null, false, true, ImmutableList.<PluginProperty>of(), callContext);
         assertListenerStatus();
+        final Account accountBCD = accountUserApi.getAccountById(account.getId(), callContext);
+        assertEquals(accountBCD.getBillCycleDayLocal(), (Integer) 1);
         final Entitlement baseEntitlement = entitlementApi.getEntitlementForId(baseEntitlementId, callContext);
 
         invoiceChecker.checkInvoice(account.getId(), 1, callContext,
diff --git a/catalog/src/test/resources/catalogTest.xml b/catalog/src/test/resources/catalogTest.xml
index 6b64f5e..051639f 100644
--- a/catalog/src/test/resources/catalogTest.xml
+++ b/catalog/src/test/resources/catalogTest.xml
@@ -86,6 +86,9 @@
         <product name="Trebuchet">
             <category>BASE</category>
         </product>
+        <product name="Cannon">
+            <category>BASE</category>
+        </product>
         <product name="Cleaning">
             <category>ADD_ON</category>
         </product>
@@ -1437,6 +1440,24 @@
                 </usages>
             </finalPhase>
         </plan>
+        <plan name="cannon-monthly-in-arrear">
+            <product>Cannon</product>
+            <recurringBillingMode>IN_ARREAR</recurringBillingMode>
+            <finalPhase type="EVERGREEN">
+                <duration>
+                    <unit>UNLIMITED</unit>
+                </duration>
+                <recurring>
+                    <billingPeriod>MONTHLY</billingPeriod>
+                    <recurringPrice>
+                        <price>
+                            <currency>USD</currency>
+                            <value>500.00</value>
+                        </price>
+                    </recurringPrice>
+                </recurring>
+            </finalPhase>
+        </plan>
     </plans>
     <priceLists>
         <defaultPriceList name="DEFAULT">
@@ -1452,6 +1473,7 @@
                 <plan>shotgun-annual</plan>
                 <plan>assault-rifle-annual</plan>
                 <plan>trebuchet-usage-in-arrear</plan>
+                <plan>cannon-monthly-in-arrear</plan>
                 <plan>laser-scope-monthly</plan>
                 <plan>telescopic-scope-monthly</plan>
                 <plan>cabinet-triannual</plan>
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 3ece5ff..5e5d811 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
@@ -186,23 +186,20 @@ 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) {
-            DateTime oldestNonZeroRecurringCharge = null;
+            BillingEvent oldestBillingEvent = null;
 
             for (final BillingEvent event : result) {
-                final BigDecimal recurringPrice = event.getRecurringPrice(event.getEffectiveDate());
-                final boolean nonZeroRecurringCharge = recurringPrice != null && BigDecimal.ZERO.compareTo(recurringPrice) < 0;
-                if (nonZeroRecurringCharge &&
-                    (oldestNonZeroRecurringCharge == null || event.getEffectiveDate().compareTo(oldestNonZeroRecurringCharge) < 0)) {
-                    oldestNonZeroRecurringCharge = event.getEffectiveDate();
+                if (oldestBillingEvent == null || event.getEffectiveDate().compareTo(oldestBillingEvent.getEffectiveDate()) < 0) {
+                    oldestBillingEvent = event;
                 }
             }
 
-            if (oldestNonZeroRecurringCharge == null) {
+            if (oldestBillingEvent == null) {
                 return;
             }
 
             // BCD in the account timezone
-            final int accountBCDCandidate = context.toLocalDate(oldestNonZeroRecurringCharge).getDayOfMonth();
+            final int accountBCDCandidate = oldestBillingEvent.getBillCycleDayLocal();
             if (accountBCDCandidate != 0) {
                 log.info("Setting account BCD='{}', accountId='{}'", accountBCDCandidate, account.getId());
                 accountApi.updateBCD(account.getExternalKey(), accountBCDCandidate, context);
diff --git a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
index ff29ddf..b4a01d1 100644
--- a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
+++ b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
@@ -52,6 +52,104 @@ import com.google.common.collect.ImmutableList;
 
 public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbeddedDB {
 
+    @Test(groups = "slow")
+    public void testRecurringInArrear() throws Exception {
+        final LocalDate initialDate = new LocalDate(2013, 8, 7);
+        clock.setDay(initialDate);
+
+        // Account with no BCD
+        final Account account = createAccount(getAccountData(0));
+        Assert.assertEquals(account.getBillCycleDayLocal(), (Integer) 0);
+
+        // Create base entitlement
+        final String bundleKey = UUID.randomUUID().toString();
+        final EntitlementSpecifier entitlementSpecifierBase = new DefaultEntitlementSpecifier(new PlanPhaseSpecifier("Cannon", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null));
+        final BaseEntitlementWithAddOnsSpecifier specifier = new DefaultBaseEntitlementWithAddOnsSpecifier(null, bundleKey, ImmutableList.of(entitlementSpecifierBase), null, null, false);
+        testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK);
+        entitlementApi.createBaseEntitlementsWithAddOns(account.getId(),
+                                                        ImmutableList.of(specifier),
+                                                        false,
+                                                        ImmutableList.<PluginProperty>of(),
+                                                        callContext);
+        assertListenerStatus();
+
+        final List<Entitlement> entitlements = entitlementApi.getAllEntitlementsForAccountId(account.getId(), callContext);
+        Assert.assertEquals(entitlements.size(), 1);
+        Assert.assertEquals(entitlements.get(0).getLastActiveProduct().getName(), "Cannon");
+        Assert.assertEquals(entitlements.get(0).getBillCycleDayLocal(), (Integer) 7);
+
+        // Account still has no BCD
+        final Account accountNoBCD = accountApi.getAccountById(account.getId(), callContext);
+        Assert.assertEquals(accountNoBCD.getBillCycleDayLocal(), (Integer) 0);
+
+        List<BillingEvent> events = ImmutableList.<BillingEvent>copyOf(billingInternalApi.getBillingEventsForAccountAndUpdateAccountBCD(account.getId(), null, internalCallContext));
+        Assert.assertEquals(events.size(), 1);
+        Assert.assertEquals(events.get(0).getBillCycleDayLocal(), 7);
+
+        // Verify BCD
+        final Account accountWithBCD = accountApi.getAccountById(account.getId(), callContext);
+        Assert.assertEquals(accountWithBCD.getBillCycleDayLocal(), (Integer) 7);
+
+        // Verify GET
+        final List<Entitlement> entitlementsUpdated = entitlementApi.getAllEntitlementsForAccountId(account.getId(), callContext);
+        Assert.assertEquals(entitlementsUpdated.size(), 1);
+        Assert.assertEquals(entitlementsUpdated.get(0).getLastActiveProduct().getName(), "Cannon");
+        Assert.assertEquals(entitlementsUpdated.get(0).getBillCycleDayLocal(), (Integer) 7);
+
+        events = ImmutableList.<BillingEvent>copyOf(billingInternalApi.getBillingEventsForAccountAndUpdateAccountBCD(account.getId(), null, internalCallContext));
+        Assert.assertEquals(events.size(), 1);
+        Assert.assertEquals(events.get(0).getBillCycleDayLocal(), 7);
+    }
+
+    @Test(groups = "slow")
+    public void testUsageInArrear() throws Exception {
+        final LocalDate initialDate = new LocalDate(2013, 8, 7);
+        clock.setDay(initialDate);
+
+        // Account with no BCD
+        final Account account = createAccount(getAccountData(0));
+        Assert.assertEquals(account.getBillCycleDayLocal(), (Integer) 0);
+
+        // Create base entitlement
+        final String bundleKey = UUID.randomUUID().toString();
+        final EntitlementSpecifier entitlementSpecifierBase = new DefaultEntitlementSpecifier(new PlanPhaseSpecifier("Trebuchet", BillingPeriod.NO_BILLING_PERIOD, PriceListSet.DEFAULT_PRICELIST_NAME, null));
+        final BaseEntitlementWithAddOnsSpecifier specifier = new DefaultBaseEntitlementWithAddOnsSpecifier(null, bundleKey, ImmutableList.of(entitlementSpecifierBase), null, null, false);
+        testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK);
+        entitlementApi.createBaseEntitlementsWithAddOns(account.getId(),
+                                                        ImmutableList.of(specifier),
+                                                        false,
+                                                        ImmutableList.<PluginProperty>of(),
+                                                        callContext);
+        assertListenerStatus();
+
+        final List<Entitlement> entitlements = entitlementApi.getAllEntitlementsForAccountId(account.getId(), callContext);
+        Assert.assertEquals(entitlements.size(), 1);
+        Assert.assertEquals(entitlements.get(0).getLastActiveProduct().getName(), "Trebuchet");
+        Assert.assertEquals(entitlements.get(0).getBillCycleDayLocal(), (Integer) 7);
+
+        // Account still has no BCD
+        final Account accountNoBCD = accountApi.getAccountById(account.getId(), callContext);
+        Assert.assertEquals(accountNoBCD.getBillCycleDayLocal(), (Integer) 0);
+
+        List<BillingEvent> events = ImmutableList.<BillingEvent>copyOf(billingInternalApi.getBillingEventsForAccountAndUpdateAccountBCD(account.getId(), null, internalCallContext));
+        Assert.assertEquals(events.size(), 1);
+        Assert.assertEquals(events.get(0).getBillCycleDayLocal(), 7);
+
+        // Verify BCD
+        final Account accountWithBCD = accountApi.getAccountById(account.getId(), callContext);
+        Assert.assertEquals(accountWithBCD.getBillCycleDayLocal(), (Integer) 7);
+
+        // Verify GET
+        final List<Entitlement> entitlementsUpdated = entitlementApi.getAllEntitlementsForAccountId(account.getId(), callContext);
+        Assert.assertEquals(entitlementsUpdated.size(), 1);
+        Assert.assertEquals(entitlementsUpdated.get(0).getLastActiveProduct().getName(), "Trebuchet");
+        Assert.assertEquals(entitlementsUpdated.get(0).getBillCycleDayLocal(), (Integer) 7);
+
+        events = ImmutableList.<BillingEvent>copyOf(billingInternalApi.getBillingEventsForAccountAndUpdateAccountBCD(account.getId(), null, internalCallContext));
+        Assert.assertEquals(events.size(), 1);
+        Assert.assertEquals(events.get(0).getBillCycleDayLocal(), 7);
+    }
+
     @Test(groups = "slow", description = "https://github.com/killbill/killbill/issues/865")
     public void testBCDUpdateMultipleSubscriptionsAccountAligned() throws Exception {
         final LocalDate initialDate = new LocalDate(2013, 8, 7);
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java
index 904bda0..02fce0d 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java
@@ -96,7 +96,7 @@ public class TestCatalog extends TestJaxrsBase {
         Assert.assertEquals(catalogsJson.get(0).getName(), "Firearms");
         Assert.assertEquals(catalogsJson.get(0).getEffectiveDate().toLocalDate(), new LocalDate("2011-01-01"));
         Assert.assertEquals(catalogsJson.get(0).getCurrencies().size(), 3);
-        Assert.assertEquals(catalogsJson.get(0).getProducts().size(), 14);
+        Assert.assertEquals(catalogsJson.get(0).getProducts().size(), 15);
         Assert.assertEquals(catalogsJson.get(0).getPriceLists().size(), 7);
 
         for (final Product productJson : catalogsJson.get(0).getProducts()) {