killbill-memoizeit

Merge branch 'work-for-release-0.17.x' of github.com:killbill/killbill

7/7/2016 10:16:10 PM

Details

diff --git a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
index 37989fa..f78edf5 100644
--- a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
+++ b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
@@ -98,9 +98,9 @@ public class BlockingCalculator {
      *
      * @param billingEvents the original list of billing events to update (without overdue events)
      */
-    public void insertBlockingEvents(final SortedSet<BillingEvent> billingEvents, final Set<UUID> skippedSubscriptions, final InternalTenantContext context) throws CatalogApiException {
+    public boolean insertBlockingEvents(final SortedSet<BillingEvent> billingEvents, final Set<UUID> skippedSubscriptions, final InternalTenantContext context) throws CatalogApiException {
         if (billingEvents.size() <= 0) {
-            return;
+            return false;
         }
 
         final Hashtable<UUID, List<SubscriptionBase>> bundleMap = createBundleSubscriptionMap(billingEvents);
@@ -146,6 +146,8 @@ public class BlockingCalculator {
         for (final BillingEvent eventToRemove : billingEventsToRemove) {
             billingEvents.remove(eventToRemove);
         }
+
+        return !(billingEventsToAdd.isEmpty() && billingEventsToRemove.isEmpty());
     }
 
     final List<BlockingState> getAggregateBlockingEventsPerSubscription(final Iterable<BlockingState> subscriptionBlockingEvents, final Iterable<BlockingState> bundleBlockingEvents, final Iterable<BlockingState> accountBlockingEvents) {
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 70cf640..e556f31 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
@@ -70,66 +70,65 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
 
     private static final Logger log = LoggerFactory.getLogger(DefaultInternalBillingApi.class);
     private final AccountInternalApi accountApi;
-    private final BillCycleDayCalculator bcdCalculator;
     private final SubscriptionBaseInternalApi subscriptionApi;
     private final CatalogService catalogService;
     private final BlockingCalculator blockCalculator;
     private final TagInternalApi tagApi;
-    private final Clock clock;
 
     @Inject
     public DefaultInternalBillingApi(final AccountInternalApi accountApi,
-                                     final BillCycleDayCalculator bcdCalculator,
                                      final SubscriptionBaseInternalApi subscriptionApi,
                                      final BlockingCalculator blockCalculator,
                                      final CatalogService catalogService,
-                                     final TagInternalApi tagApi,
-                                     final Clock clock) {
+                                     final TagInternalApi tagApi) {
         this.accountApi = accountApi;
-        this.bcdCalculator = bcdCalculator;
         this.subscriptionApi = subscriptionApi;
         this.catalogService = catalogService;
         this.blockCalculator = blockCalculator;
         this.tagApi = tagApi;
-        this.clock = clock;
     }
 
     @Override
     public BillingEventSet getBillingEventsForAccountAndUpdateAccountBCD(final UUID accountId, final DryRunArguments dryRunArguments, final InternalCallContext context) throws CatalogApiException, AccountApiException {
-        final List<SubscriptionBaseBundle> bundles = subscriptionApi.getBundlesForAccount(accountId, context);
         final StaticCatalog currentCatalog = catalogService.getCurrentCatalog(context);
 
-        final ImmutableAccountData account = accountApi.getImmutableAccountDataById(accountId, context);
-        final DefaultBillingEventSet result = new DefaultBillingEventSet(false, currentCatalog.getRecurringBillingMode());
+        // Check to see if billing is off for the account
+        final List<Tag> accountTags = tagApi.getTags(accountId, ObjectType.ACCOUNT, context);
+        final boolean found_AUTO_INVOICING_OFF = is_AUTO_INVOICING_OFF(accountTags);
+        if (found_AUTO_INVOICING_OFF) {
+            return new DefaultBillingEventSet(true, currentCatalog.getRecurringBillingMode()); // billing is off, we are done
+        }
 
+        final List<SubscriptionBaseBundle> bundles = subscriptionApi.getBundlesForAccount(accountId, context);
 
+        final ImmutableAccountData account = accountApi.getImmutableAccountDataById(accountId, context);
+        final DefaultBillingEventSet result = new DefaultBillingEventSet(false, currentCatalog.getRecurringBillingMode());
 
         final Set<UUID> skippedSubscriptions = new HashSet<UUID>();
         try {
-            // Check to see if billing is off for the account
-            final List<Tag> accountTags = tagApi.getTags(accountId, ObjectType.ACCOUNT, context);
-            final boolean found_AUTO_INVOICING_OFF = is_AUTO_INVOICING_OFF(accountTags);
-            if (found_AUTO_INVOICING_OFF) {
-                return new DefaultBillingEventSet(true, currentCatalog.getRecurringBillingMode()); // billing is off, we are done
-            }
-
             addBillingEventsForBundles(bundles, account, dryRunArguments, context, result, skippedSubscriptions);
-        } catch (SubscriptionBaseApiException e) {
+        } catch (final SubscriptionBaseApiException e) {
             log.warn("Failed while getting BillingEvent", e);
         }
 
+        if (result.isEmpty()) {
+            log.info("No billing event for accountId='{}'", accountId);
+            return result;
+        }
+
         // Pretty-print the events, before and after the blocking calculator does its magic
         final StringBuilder logStringBuilder = new StringBuilder("Computed billing events for accountId='").append(accountId).append("'");
-        eventsToString(logStringBuilder, result, "\nBilling Events Raw");
-        blockCalculator.insertBlockingEvents(result, skippedSubscriptions, context);
-        eventsToString(logStringBuilder, result, "\nBilling Events After Blocking");
+        eventsToString(logStringBuilder, result);
+        if (blockCalculator.insertBlockingEvents(result, skippedSubscriptions, context)) {
+            logStringBuilder.append("\nBilling Events After Blocking");
+            eventsToString(logStringBuilder, result);
+        }
         log.info(logStringBuilder.toString());
 
         return result;
     }
 
-    private void eventsToString(final StringBuilder stringBuilder, final SortedSet<BillingEvent> events, final String title) {
-        stringBuilder.append(title);
+    private void eventsToString(final StringBuilder stringBuilder, final SortedSet<BillingEvent> events) {
         for (final BillingEvent event : events) {
             stringBuilder.append("\n").append(event.toString());
         }
@@ -185,8 +184,7 @@ 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
         boolean updatedAccountBCD = dryRunMode;
 
-
-        int currentAccountBCD = accountApi.getBCD(account.getId(), context);
+        final int currentAccountBCD = accountApi.getBCD(account.getId(), context);
         for (final SubscriptionBase subscription : subscriptions) {
 
             // The subscription did not even start, so there is nothing to do yet, we can skip and avoid some NPE down the line when calculating the BCD
@@ -218,6 +216,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
                                      calculateBcdForTransition(catalog, baseSubscription, subscription, account, currentAccountBCD, transition);
 
                 if (currentAccountBCD == 0 && !updatedAccountBCD) {
+                    log.info("Setting account BCD='{}', accountId='{}'", bcdLocal, account.getId());
                     accountApi.updateBCD(account.getExternalKey(), bcdLocal, context);
                     updatedAccountBCD = true;
                 }
@@ -231,7 +230,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
     private int calculateBcdForTransition(final Catalog catalog, final SubscriptionBase baseSubscription, final SubscriptionBase subscription, final ImmutableAccountData account, final int accountBillCycleDayLocal, final EffectiveSubscriptionInternalEvent transition)
             throws CatalogApiException, AccountApiException, SubscriptionBaseApiException {
         final BillingAlignment alignment = catalog.billingAlignment(getPlanPhaseSpecifierFromTransition(catalog, transition), transition.getEffectiveTransitionTime());
-        return bcdCalculator.calculateBcdForAlignment(subscription, baseSubscription, alignment, account.getTimeZone(), accountBillCycleDayLocal);
+        return BillCycleDayCalculator.calculateBcdForAlignment(subscription, baseSubscription, alignment, account.getTimeZone(), accountBillCycleDayLocal);
     }
 
     private PlanPhaseSpecifier getPlanPhaseSpecifierFromTransition(final Catalog catalog, final EffectiveSubscriptionInternalEvent transition) throws CatalogApiException {
@@ -255,7 +254,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
                                phase.getPhaseType());
     }
 
-    private final boolean is_AUTO_INVOICING_OFF(final List<Tag> tags) {
+    private boolean is_AUTO_INVOICING_OFF(final List<Tag> tags) {
         return ControlTagType.isAutoInvoicingOff(Collections2.transform(tags, new Function<Tag, UUID>() {
             @Nullable
             @Override
diff --git a/profiles/killbill/src/main/resources/logback.xml b/profiles/killbill/src/main/resources/logback.xml
index f2cdef8..c2c710a 100644
--- a/profiles/killbill/src/main/resources/logback.xml
+++ b/profiles/killbill/src/main/resources/logback.xml
@@ -43,7 +43,7 @@
                     </timeBasedFileNamingAndTriggeringPolicy>
                 </rollingPolicy>
                 <encoder>
-                    <pattern>%date [%thread] %maskedMsg%n</pattern>
+                    <pattern>%date{"yyyy-MM-dd'T'HH:mm:ss,SSSZ", UTC} lvl='%level', log='%logger{0}', th='%thread', xff='%X{req.xForwardedFor}', rId='%X{req.requestId}', aRId='%X{kb.accountRecordId}', tRId='%X{kb.tenantRecordId}', %maskedMsg%n</pattern>
                 </encoder>
             </appender>
         </sift>
@@ -64,7 +64,7 @@
                     </timeBasedFileNamingAndTriggeringPolicy>
                 </rollingPolicy>
                 <encoder>
-                    <pattern>%date [%thread] %maskedMsg%n</pattern>
+                    <pattern>%date{"yyyy-MM-dd'T'HH:mm:ss,SSSZ", UTC} lvl='%level', log='%logger{0}', th='%thread', xff='%X{req.xForwardedFor}', rId='%X{req.requestId}', aRId='%X{kb.accountRecordId}', tRId='%X{kb.tenantRecordId}', %maskedMsg%n</pattern>
                 </encoder>
             </appender>
         </sift>
@@ -85,7 +85,7 @@
                     </timeBasedFileNamingAndTriggeringPolicy>
                 </rollingPolicy>
                 <encoder>
-                    <pattern>%date [%thread] %maskedMsg%n</pattern>
+                    <pattern>%date{"yyyy-MM-dd'T'HH:mm:ss,SSSZ", UTC} lvl='%level', log='%logger{0}', th='%thread', xff='%X{req.xForwardedFor}', rId='%X{req.requestId}', aRId='%X{kb.accountRecordId}', tRId='%X{kb.tenantRecordId}', %maskedMsg%n</pattern>
                 </encoder>
             </appender>
         </sift>
@@ -106,7 +106,7 @@
                     </timeBasedFileNamingAndTriggeringPolicy>
                 </rollingPolicy>
                 <encoder>
-                    <pattern>%date [%thread] %maskedMsg%n</pattern>
+                    <pattern>%date{"yyyy-MM-dd'T'HH:mm:ss,SSSZ", UTC} lvl='%level', log='%logger{0}', th='%thread', xff='%X{req.xForwardedFor}', rId='%X{req.requestId}', aRId='%X{kb.accountRecordId}', tRId='%X{kb.tenantRecordId}', %maskedMsg%n</pattern>
                 </encoder>
             </appender>
         </sift>
@@ -127,7 +127,7 @@
                     </timeBasedFileNamingAndTriggeringPolicy>
                 </rollingPolicy>
                 <encoder>
-                    <pattern>%date [%thread] %maskedMsg%n</pattern>
+                    <pattern>%date{"yyyy-MM-dd'T'HH:mm:ss,SSSZ", UTC} lvl='%level', log='%logger{0}', th='%thread', xff='%X{req.xForwardedFor}', rId='%X{req.requestId}', aRId='%X{kb.accountRecordId}', tRId='%X{kb.tenantRecordId}', %maskedMsg%n</pattern>
                 </encoder>
             </appender>
         </sift>
@@ -148,7 +148,7 @@
                     </timeBasedFileNamingAndTriggeringPolicy>
                 </rollingPolicy>
                 <encoder>
-                    <pattern>%date [%thread] %maskedMsg%n</pattern>
+                    <pattern>%date{"yyyy-MM-dd'T'HH:mm:ss,SSSZ", UTC} lvl='%level', log='%logger{0}', th='%thread', xff='%X{req.xForwardedFor}', rId='%X{req.requestId}', aRId='%X{kb.accountRecordId}', tRId='%X{kb.tenantRecordId}', %maskedMsg%n</pattern>
                 </encoder>
             </appender>
         </sift>
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestPerTenantConfig.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestPerTenantConfig.java
index 9736a04..a180f70 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestPerTenantConfig.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestPerTenantConfig.java
@@ -18,6 +18,8 @@
 package org.killbill.billing.jaxrs;
 
 import java.util.HashMap;
+import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
 
 import org.killbill.billing.client.model.Account;
 import org.killbill.billing.client.model.Payments;
@@ -34,6 +36,8 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.inject.Inject;
+import com.jayway.awaitility.Awaitility;
+import com.jayway.awaitility.Duration;
 
 public class TestPerTenantConfig extends TestJaxrsBase {
 
@@ -55,7 +59,6 @@ public class TestPerTenantConfig extends TestJaxrsBase {
 
     @Test(groups = "slow")
     public void testFailedPaymentWithPerTenantRetryConfig() throws Exception {
-
         // Create the tenant
         final String apiKeyTenant1 = "tenantSuperTuned";
         final String apiSecretTenant1 = "2367$$ffr79";
@@ -66,7 +69,7 @@ public class TestPerTenantConfig extends TestJaxrsBase {
         killBillClient.createTenant(tenant1, createdBy, reason, comment);
 
         // Configure our plugin to fail
-        mockPaymentProviderPlugin.makeNextPaymentFailWithError();
+        mockPaymentProviderPlugin.makeAllInvoicesFailWithError(true);
 
         // Upload the config
         final ObjectMapper mapper = new ObjectMapper();
@@ -86,10 +89,6 @@ public class TestPerTenantConfig extends TestJaxrsBase {
         // Because we have specified a retry interval of one day we should see the new attempt after moving clock 1 day (and not 8 days which is default)
         //
 
-        // Configure our plugin to fail AGAIN
-        mockPaymentProviderPlugin.makeNextPaymentFailWithError();
-
-
         //
         // Now unregister special per tenant config and we the first retry occurs one day after (and still fails), it now sets a retry date of 8 days
         //
@@ -97,17 +96,24 @@ public class TestPerTenantConfig extends TestJaxrsBase {
         // org.killbill.tenant.broadcast.rate has been set to 1s
         crappyWaitForLackOfProperSynchonization(2000);
 
-
         clock.addDays(1);
-        crappyWaitForLackOfProperSynchonization(3000);
 
+        Awaitility.await()
+                  .atMost(4, TimeUnit.SECONDS)
+                  .pollInterval(Duration.ONE_SECOND)
+                  .until(new Callable<Boolean>() {
+                      @Override
+                      public Boolean call() throws Exception {
+
+                          return killBillClient.getPaymentsForAccount(accountJson.getAccountId()).get(0).getTransactions().size() == 2;
+                      }
+                  });
         final Payments payments2 = killBillClient.getPaymentsForAccount(accountJson.getAccountId());
         Assert.assertEquals(payments2.size(), 1);
         Assert.assertEquals(payments2.get(0).getTransactions().size(), 2);
         Assert.assertEquals(payments2.get(0).getTransactions().get(0).getStatus(), TransactionStatus.PAYMENT_FAILURE.name());
         Assert.assertEquals(payments2.get(0).getTransactions().get(1).getStatus(), TransactionStatus.PAYMENT_FAILURE.name());
 
-
         clock.addDays(1);
         crappyWaitForLackOfProperSynchonization(3000);
 
@@ -116,9 +122,18 @@ public class TestPerTenantConfig extends TestJaxrsBase {
         Assert.assertEquals(payments3.size(), 1);
         Assert.assertEquals(payments3.get(0).getTransactions().size(), 2);
 
+        mockPaymentProviderPlugin.makeAllInvoicesFailWithError(false);
         clock.addDays(7);
-        crappyWaitForLackOfProperSynchonization(3000);
 
+        Awaitility.await()
+                  .atMost(4, TimeUnit.SECONDS)
+                  .pollInterval(Duration.ONE_SECOND)
+                  .until(new Callable<Boolean>() {
+                      @Override
+                      public Boolean call() throws Exception {
+                          return killBillClient.getPaymentsForAccount(accountJson.getAccountId()).get(0).getTransactions().size() == 3;
+                      }
+                  });
         final Payments payments4 = killBillClient.getPaymentsForAccount(accountJson.getAccountId());
         Assert.assertEquals(payments4.size(), 1);
         Assert.assertEquals(payments4.get(0).getTransactions().size(), 3);
diff --git a/util/src/main/java/org/killbill/billing/util/bcd/BillCycleDayCalculator.java b/util/src/main/java/org/killbill/billing/util/bcd/BillCycleDayCalculator.java
index a451153..3ab047c 100644
--- a/util/src/main/java/org/killbill/billing/util/bcd/BillCycleDayCalculator.java
+++ b/util/src/main/java/org/killbill/billing/util/bcd/BillCycleDayCalculator.java
@@ -25,15 +25,10 @@ import org.killbill.clock.ClockUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.annotations.VisibleForTesting;
-
-public class BillCycleDayCalculator {
+public abstract class BillCycleDayCalculator {
 
     private static final Logger log = LoggerFactory.getLogger(BillCycleDayCalculator.class);
 
-    public BillCycleDayCalculator() {
-    }
-
     public static int calculateBcdForAlignment(final SubscriptionBase subscription, final SubscriptionBase baseSubscription, final BillingAlignment alignment, final DateTimeZone accountTimeZone, final int accountBillCycleDayLocal) {
         int result = 0;
         switch (alignment) {
@@ -50,12 +45,11 @@ public class BillCycleDayCalculator {
         return result;
     }
 
-    @VisibleForTesting
-    static int calculateBcdFromSubscription(final SubscriptionBase subscription, final DateTimeZone accountTimeZone) {
+    private static int calculateBcdFromSubscription(final SubscriptionBase subscription, final DateTimeZone accountTimeZone) {
         final DateTime date = subscription.getDateOfFirstRecurringNonZeroCharge();
         final int bcdLocal = ClockUtil.toDateTime(date, accountTimeZone).getDayOfMonth();
-        log.info("Calculated BCD: subscriptionId='{}', subscriptionStartDate='{}', accountTimeZone='{}', bcd='{}'",
-                 subscription.getId(), date.toDateTimeISO(), accountTimeZone, bcdLocal);
+        log.debug("Calculated BCD: subscriptionId='{}', subscriptionStartDate='{}', accountTimeZone='{}', bcd='{}'",
+                  subscription.getId(), date.toDateTimeISO(), accountTimeZone, bcdLocal);
         return bcdLocal;
     }
 }