killbill-memoizeit

invoice: code review for 49703cc7cef47d853a9f9fe17b6bf760c0cb118f Signed-off-by:

1/28/2016 8:33:48 PM

Details

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/overdue/TestOverdueIntegration.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/overdue/TestOverdueIntegration.java
index 905d19c..a974451 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/overdue/TestOverdueIntegration.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/overdue/TestOverdueIntegration.java
@@ -738,7 +738,7 @@ public class TestOverdueIntegration extends TestOverdueBase {
 
         // Now, refund the second (first non-zero dollar) invoice
         final Payment payment = paymentApi.getPayment(invoiceUserApi.getInvoicesByAccount(account.getId(), callContext).get(1).getPayments().get(0).getPaymentId(), false, PLUGIN_PROPERTIES, callContext);
-        refundPaymentAndCheckForCompletion(account, payment, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT, NextEvent.BLOCK, NextEvent.INVOICE_ADJUSTMENT);
+        refundPaymentAndCheckForCompletion(account, payment, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT, NextEvent.BLOCK);
         // We should now be in OD1
         checkODState("OD1");
         checkChangePlanWithOverdueState(baseEntitlement, true, true);
@@ -783,7 +783,7 @@ public class TestOverdueIntegration extends TestOverdueBase {
         // Now, create a chargeback for the second (first non-zero dollar) invoice
         final InvoicePayment invoicePayment = invoicePaymentApi.getInvoicePayments(invoiceUserApi.getInvoicesByAccount(account.getId(), callContext).get(1).getPayments().get(0).getPaymentId(), callContext).get(0);
         final Payment payment = paymentApi.getPayment(invoicePayment.getPaymentId(), false, ImmutableList.<PluginProperty>of(), callContext);
-        createChargeBackAndCheckForCompletion(account, payment, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT, NextEvent.BLOCK, NextEvent.INVOICE_ADJUSTMENT);
+        createChargeBackAndCheckForCompletion(account, payment, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT, NextEvent.BLOCK);
         // We should now be in OD1
         checkODState("OD1");
         checkChangePlanWithOverdueState(baseEntitlement, true, true);
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
index 0961652..c18cd69 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
@@ -96,7 +96,7 @@ public class TestInvoicePayment extends TestIntegrationBase {
 
 */
 
-        refundPaymentAndCheckForCompletion(account, payment1, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT, NextEvent.INVOICE_ADJUSTMENT);
+        refundPaymentAndCheckForCompletion(account, payment1, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
 
         invoice1 = invoiceUserApi.getInvoice(item1.getInvoiceId(), callContext);
         assertTrue(invoice1.getBalance().compareTo(new BigDecimal("4.00")) == 0);
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestPaymentRefund.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestPaymentRefund.java
index 91a0a9d..d428c8a 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestPaymentRefund.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestPaymentRefund.java
@@ -73,7 +73,7 @@ public class TestPaymentRefund extends TestIntegrationBase {
     @Test(groups = "slow")
     public void testRefundWithNoAdjustments() throws Exception {
         // Although we don't adjust the invoice, the invoicing system sends an event because invoice balance changes and overdue system-- in particular-- needs to know about it.
-        refundPaymentAndCheckForCompletion(account, payment, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT, NextEvent.INVOICE_ADJUSTMENT);
+        refundPaymentAndCheckForCompletion(account, payment, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
         refundChecker.checkRefund(payment.getId(), callContext, new ExpectedRefundCheck(payment.getId(), false, new BigDecimal("233.82"), Currency.USD, initialCreationDate.toLocalDate()));
     }
 
diff --git a/entitlement/src/main/java/org/killbill/billing/entitlement/engine/core/EntitlementUtils.java b/entitlement/src/main/java/org/killbill/billing/entitlement/engine/core/EntitlementUtils.java
index a4d33ac..fd44b4d 100644
--- a/entitlement/src/main/java/org/killbill/billing/entitlement/engine/core/EntitlementUtils.java
+++ b/entitlement/src/main/java/org/killbill/billing/entitlement/engine/core/EntitlementUtils.java
@@ -75,6 +75,7 @@ public class EntitlementUtils {
 
     public void setBlockingStateAndPostBlockingTransitionEvent(final BlockingState state, final InternalCallContext context) {
         UUID bundleId = null;
+        // We only need the bundle id in case of subscriptions (at the account level, we don't need it and at the bundle level, we already have it)
         if (state.getType() == BlockingStateType.SUBSCRIPTION) {
             try {
                 bundleId = subscriptionBaseInternalApi.getSubscriptionFromId(state.getBlockedId(), context).getBundleId();
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java b/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
index 6dbefd1..f22f497 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
@@ -31,6 +31,7 @@ import javax.annotation.Nullable;
 import org.joda.time.DateTime;
 import org.joda.time.LocalDate;
 import org.killbill.billing.ErrorCode;
+import org.killbill.billing.ObjectType;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
 import org.killbill.billing.catalog.api.Currency;
@@ -46,6 +47,7 @@ import org.killbill.billing.invoice.api.InvoicePaymentType;
 import org.killbill.billing.invoice.api.user.DefaultInvoiceAdjustmentEvent;
 import org.killbill.billing.invoice.notification.NextBillingDatePoster;
 import org.killbill.billing.util.UUIDs;
+import org.killbill.billing.util.cache.Cachable.CacheType;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
 import org.killbill.billing.util.callcontext.InternalCallContextFactory;
 import org.killbill.billing.util.config.InvoiceConfig;
@@ -98,6 +100,8 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
     private final CBADao cbaDao;
     private final InvoiceConfig invoiceConfig;
     private final Clock clock;
+    private final CacheControllerDispatcher cacheControllerDispatcher;
+    private final NonEntityDao nonEntityDao;
 
     @Inject
     public DefaultInvoiceDao(final IDBI dbi,
@@ -118,6 +122,8 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
         this.invoiceDaoHelper = invoiceDaoHelper;
         this.cbaDao = cbaDao;
         this.clock = clock;
+        this.cacheControllerDispatcher = cacheControllerDispatcher;
+        this.nonEntityDao = nonEntityDao;
     }
 
     @Override
@@ -507,9 +513,9 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
 
                 cbaDao.addCBAComplexityFromTransaction(invoice, entitySqlDaoWrapperFactory, context);
 
-                // Notify the bus since the balance of the invoice changed
-                notifyBusOfInvoiceAdjustment(entitySqlDaoWrapperFactory, invoice.getId(), invoice.getAccountId(), context.getUserToken(), context);
-
+                if (isInvoiceAdjusted) {
+                    notifyBusOfInvoiceAdjustment(entitySqlDaoWrapperFactory, invoice.getId(), invoice.getAccountId(), context.getUserToken(), context);
+                }
                 notifyBusOfInvoicePayment(entitySqlDaoWrapperFactory, refund, invoice.getAccountId(), context.getUserToken(), context);
 
                 return refund;
@@ -563,8 +569,6 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
 
                 cbaDao.addCBAComplexityFromTransaction(payment.getInvoiceId(), entitySqlDaoWrapperFactory, context);
 
-                notifyBusOfInvoiceAdjustment(entitySqlDaoWrapperFactory, payment.getInvoiceId(), accountId, context.getUserToken(), context);
-
                 notifyBusOfInvoicePayment(entitySqlDaoWrapperFactory, chargeBack, accountId, context.getUserToken(), context);
 
                 return chargeBack;
@@ -691,10 +695,7 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
                     }
                 }
 
-                final InvoiceSqlDao invoiceSqlDao = entitySqlDaoWrapperFactory.become(InvoiceSqlDao.class);
-                final InvoiceModelDao invoice = invoiceSqlDao.getById(invoicePayment.getInvoiceId().toString(), context);
-                // Should never be null, but be safe...
-                final UUID accountId = invoice == null ? null : invoice.getAccountId();
+                final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID), entitySqlDaoWrapperFactory.getHandle());
                 notifyBusOfInvoicePayment(entitySqlDaoWrapperFactory, invoicePayment, accountId, context.getUserToken(), context);
 
                 return null;