Skip to content

Commit

Permalink
invoice: fetch tags outside of DAO transactions
Browse files Browse the repository at this point in the history
This fixes #720.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
pierre committed Mar 14, 2017
1 parent c69e14d commit 9971663
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 53 deletions.
17 changes: 11 additions & 6 deletions invoice/src/main/java/org/killbill/billing/invoice/dao/CBADao.java
Expand Up @@ -33,6 +33,7 @@
import org.killbill.billing.invoice.api.InvoiceStatus;
import org.killbill.billing.invoice.model.CreditBalanceAdjInvoiceItem;
import org.killbill.billing.util.entity.dao.EntitySqlDaoWrapperFactory;
import org.killbill.billing.util.tag.Tag;

import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
Expand Down Expand Up @@ -110,22 +111,25 @@ public boolean apply(@Nullable final InvoiceItemModelDao input) {
// We let the code below rehydrate the invoice before we can add the CBA item
// PERF: when possible, prefer the method below to avoid re-fetching the invoice
public void doCBAComplexityFromTransaction(final UUID invoiceId,
final List<Tag> invoicesTags,
final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory,
final InternalCallContext context) throws EntityPersistenceException, InvoiceApiException {
final InvoiceSqlDao transInvoiceDao = entitySqlDaoWrapperFactory.become(InvoiceSqlDao.class);
final InvoiceModelDao invoice = transInvoiceDao.getById(invoiceId.toString(), context);
invoiceDaoHelper.populateChildren(invoice, entitySqlDaoWrapperFactory, context);
invoiceDaoHelper.populateChildren(invoice, invoicesTags, entitySqlDaoWrapperFactory, context);

doCBAComplexityFromTransaction(invoice, entitySqlDaoWrapperFactory, context);
doCBAComplexityFromTransaction(invoice, invoicesTags, entitySqlDaoWrapperFactory, context);
}

public void doCBAComplexityFromTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory,
public void doCBAComplexityFromTransaction(final List<Tag> invoicesTags,
final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory,
final InternalCallContext context) throws EntityPersistenceException, InvoiceApiException {
doCBAComplexityFromTransaction((InvoiceModelDao) null, entitySqlDaoWrapperFactory, context);
doCBAComplexityFromTransaction((InvoiceModelDao) null, invoicesTags, entitySqlDaoWrapperFactory, context);
}

// Note! We expect an *up-to-date* invoice, with all the items and payments except the CBA, that we will compute in that method
public void doCBAComplexityFromTransaction(@Nullable final InvoiceModelDao invoice,
final List<Tag> invoicesTags,
final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory,
final InternalCallContext context) throws EntityPersistenceException, InvoiceApiException {
// PERF: It is expensive to retrieve and construct all invoice objects. To check if there is effectively something to use, compute the CBA by the database first
Expand All @@ -136,11 +140,12 @@ public void doCBAComplexityFromTransaction(@Nullable final InvoiceModelDao invoi
remainingAccountCBA = computeCBAComplexityAndCreateCBAItem(remainingAccountCBA, invoice, entitySqlDaoWrapperFactory, context);
}

useExistingCBAFromTransaction(remainingAccountCBA, entitySqlDaoWrapperFactory, context);
useExistingCBAFromTransaction(remainingAccountCBA, invoicesTags, entitySqlDaoWrapperFactory, context);
}

// Distribute account CBA across all COMMITTED unpaid invoices
private void useExistingCBAFromTransaction(final BigDecimal accountCBA,
final List<Tag> invoicesTags,
final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory,
final InternalCallContext context) throws InvoiceApiException, EntityPersistenceException {
if (accountCBA.compareTo(BigDecimal.ZERO) <= 0) {
Expand All @@ -149,7 +154,7 @@ private void useExistingCBAFromTransaction(final BigDecimal accountCBA,

// PERF: Computing the invoice balance is difficult to do in the DB, so we effectively need to retrieve all invoices on the account and filter the unpaid ones in memory.
// This should be infrequent though because of the account CBA check above.
final List<InvoiceModelDao> allInvoices = invoiceDaoHelper.getAllInvoicesByAccountFromTransaction(entitySqlDaoWrapperFactory, context);
final List<InvoiceModelDao> allInvoices = invoiceDaoHelper.getAllInvoicesByAccountFromTransaction(invoicesTags, entitySqlDaoWrapperFactory, context);
final List<InvoiceModelDao> unpaidInvoices = invoiceDaoHelper.getUnpaidInvoicesByAccountFromTransaction(allInvoices, null);
// We order the same os BillingStateCalculator-- should really share the comparator
final List<InvoiceModelDao> orderedUnpaidInvoices = Ordering.from(new Comparator<InvoiceModelDao>() {
Expand Down

1 comment on commit 9971663

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.