Skip to content

Commit

Permalink
Merge branch 'master' of github.com:killbill/killbill
Browse files Browse the repository at this point in the history
  • Loading branch information
sbrossie committed Jan 24, 2017
2 parents 00e81cf + e01070c commit 6ce617c
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -6,7 +6,7 @@ sudo: false
# directories:
# - $HOME/.m2

script: if [[ -v COMMAND ]]; then $COMMAND; else travis_retry mvn -q -Djava.security.egd=file:/dev/./urandom -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN -Dorg.slf4j.simpleLogger.log.org.killbill.billing.util.cache=ERROR -Dorg.slf4j.simpleLogger.log.org.killbill.billing.lifecycle=ERROR -Dlogback.configurationFile=$PWD/profiles/killbill/src/test/resources/logback.travis.xml clean install $PHASE 2>&1 | egrep -v 'Download|Install|[ \t]*at [ \ta-zA-Z0-9\.\:\(\)]+'; [ ${PIPESTATUS[0]} == 0 ]; fi
script: if [[ -v COMMAND ]]; then $COMMAND; else travis_retry mvn -q -Djava.security.egd=file:/dev/./urandom -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN -Dorg.slf4j.simpleLogger.log.org.killbill.billing.util.cache=ERROR -Dorg.slf4j.simpleLogger.log.org.killbill.billing.lifecycle=ERROR -Dlogback.configurationFile=$PWD/profiles/killbill/src/test/resources/logback.travis.xml clean install $PHASE -pl '!beatrix,!profiles,!profiles/killbill,!profiles/killpay' 2>&1 | egrep -v 'Download|Install|[ \t]*at [ \ta-zA-Z0-9\.\:\(\)]+'; [ ${PIPESTATUS[0]} == 0 ]; fi
# Remove --quiet to avoid timeouts
install: mvn -U install -DskipTests=true | egrep -v 'Download|Install'

Expand Down
Expand Up @@ -60,4 +60,9 @@ public void updatePaymentMethod(@Bind("id") String accountId,
@SqlQuery
List<AccountModelDao> getAccountsByParentId(@Bind("parentAccountId") UUID parentAccountId,
@BindBean final InternalTenantContext context);

@SqlQuery
public AccountModelDao luckySearch(@Bind("searchKey") final String searchKey,
@BindBean final InternalTenantContext context);

}
Expand Up @@ -38,6 +38,7 @@
import org.killbill.billing.util.cache.CacheControllerDispatcher;
import org.killbill.billing.util.callcontext.InternalCallContextFactory;
import org.killbill.billing.util.dao.NonEntityDao;
import org.killbill.billing.util.entity.DefaultPagination;
import org.killbill.billing.util.entity.Pagination;
import org.killbill.billing.util.entity.dao.DefaultPaginationSqlDaoHelper.PaginationIteratorBuilder;
import org.killbill.billing.util.entity.dao.EntityDaoBase;
Expand All @@ -51,6 +52,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;

public class DefaultAccountDao extends EntityDaoBase<AccountModelDao, Account, AccountApiException> implements AccountDao {
Expand Down Expand Up @@ -110,6 +112,25 @@ public AccountModelDao inTransaction(final EntitySqlDaoWrapperFactory entitySqlD

@Override
public Pagination<AccountModelDao> searchAccounts(final String searchKey, final Long offset, final Long limit, final InternalTenantContext context) {
final boolean userIsFeelingLucky = limit == 1 && offset == -1;
if (userIsFeelingLucky) {
// The use-case we can optimize is when the user is looking for an exact match (e.g. he knows the full email). In that case, we can speed up the queries
// by doing exact searches only.
final AccountModelDao accountModelDao = transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<AccountModelDao>() {
@Override
public AccountModelDao inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
return entitySqlDaoWrapperFactory.become(AccountSqlDao.class).luckySearch(searchKey, context);
}
});
return new DefaultPagination<AccountModelDao>(0L,
1L,
accountModelDao == null ? 0L : 1L,
null, // We don't compute stats for speed in that case
accountModelDao == null ? ImmutableList.<AccountModelDao>of().iterator() : ImmutableList.<AccountModelDao>of(accountModelDao).iterator());
}

// Otherwise, we pretty much need to do a full table scan (leading % in the like clause).
// Note: forcing MySQL to search indexes (like luckySearch above) doesn't always seem to help on large tables, especially with large offsets
return paginationHelper.getPagination(AccountSqlDao.class,
new PaginationIteratorBuilder<AccountModelDao, Account, AccountSqlDao>() {
@Override
Expand Down
Expand Up @@ -106,6 +106,64 @@ searchQuery(prefix) ::= <<
or <prefix>company_name like :likeSearchKey
>>

/** Unfortunately, we need to force MySQL to use the individual indexes */
luckySearch() ::= <<
select
<allTableFields("t.")>
from <tableName()> t
join (
select distinct <recordIdField()>
from (
(
select <recordIdField()>
from <tableName()>
where <idField()> = :searchKey
<andCheckSoftDeletionWithComma()>
<AND_CHECK_TENANT()>
limit 1
)
union all
(
select <recordIdField()>
from <tableName()>
where name = :searchKey
<andCheckSoftDeletionWithComma()>
<AND_CHECK_TENANT()>
limit 1
)
union all
(
select <recordIdField()>
from <tableName()>
where email = :searchKey
<andCheckSoftDeletionWithComma()>
<AND_CHECK_TENANT()>
limit 1
)
union all
(
select <recordIdField()>
from <tableName()>
where external_key = :searchKey
<andCheckSoftDeletionWithComma()>
<AND_CHECK_TENANT()>
limit 1
)
union all
(
select <recordIdField()>
from <tableName()>
where company_name = :searchKey
<andCheckSoftDeletionWithComma()>
<AND_CHECK_TENANT()>
limit 1
)
) search_with_idx
) optimization on <recordIdField("optimization.")> = <recordIdField("t.")>
limit 1
;
>>

getIdFromKey() ::= <<
SELECT id
FROM accounts
Expand All @@ -119,4 +177,4 @@ getAccountsByParentId() ::= <<
<AND_CHECK_TENANT()>
<defaultOrderBy()>
;
>>
>>
Expand Up @@ -37,6 +37,9 @@ CREATE UNIQUE INDEX accounts_id ON accounts(id);
CREATE UNIQUE INDEX accounts_external_key ON accounts(external_key, tenant_record_id);
CREATE INDEX accounts_parents ON accounts(parent_account_id);
CREATE INDEX accounts_tenant_record_id ON accounts(tenant_record_id);
CREATE INDEX accounts_email_tenant_record_id ON accounts(email, tenant_record_id);
CREATE INDEX accounts_company_name_tenant_record_id ON accounts(company_name, tenant_record_id);
CREATE INDEX accounts_name_tenant_record_id ON accounts(name, tenant_record_id);

DROP TABLE IF EXISTS account_history;
CREATE TABLE account_history (
Expand Down
@@ -0,0 +1,3 @@
alter table accounts add index accounts_email_tenant_record_id(email, tenant_record_id);
alter table accounts add index accounts_company_name_tenant_record_id(company_name, tenant_record_id);
alter table accounts add index accounts_name_tenant_record_id(name, tenant_record_id);
Expand Up @@ -20,12 +20,12 @@
import java.util.UUID;

import org.joda.time.DateTimeZone;
import org.testng.Assert;

import org.killbill.billing.account.api.AccountData;
import org.killbill.billing.account.api.DefaultMutableAccountData;
import org.killbill.billing.account.api.MutableAccountData;
import org.killbill.billing.account.dao.AccountModelDao;
import org.killbill.billing.catalog.api.Currency;
import org.testng.Assert;

public abstract class AccountTestUtils {

Expand Down Expand Up @@ -81,11 +81,11 @@ private static AccountModelDao createTestAccount(final int billCycleDayUTC, fina
return new AccountModelDao(UUID.randomUUID(), accountData);
}

public static AccountData createAccountData() {
public static MutableAccountData createAccountData() {
return createAccountData(30, 31, UUID.randomUUID().toString().substring(0, 4));
}

private static AccountData createAccountData(final int billCycleDayUTC, final int billCycleDayLocal, final String phone) {
private static MutableAccountData createAccountData(final int billCycleDayUTC, final int billCycleDayLocal, final String phone) {
final String externalKey = UUID.randomUUID().toString();
final String email = UUID.randomUUID().toString().substring(0, 4) + '@' + UUID.randomUUID().toString().substring(0, 4);
final String name = UUID.randomUUID().toString();
Expand Down
Expand Up @@ -34,18 +34,76 @@
import org.killbill.billing.account.dao.AccountModelDao;
import org.killbill.billing.catalog.api.Currency;
import org.killbill.billing.events.AccountCreationInternalEvent;
import org.killbill.billing.util.entity.Pagination;
import org.testng.Assert;
import org.testng.annotations.Test;

import com.google.common.collect.ImmutableList;
import com.google.common.eventbus.Subscribe;

import static com.jayway.awaitility.Awaitility.await;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.killbill.billing.account.AccountTestUtils.createAccountData;
import static org.killbill.billing.account.AccountTestUtils.createTestAccount;
import static org.testng.Assert.assertEquals;

public class TestDefaultAccountUserApi extends AccountTestSuiteWithEmbeddedDB {

@Test(groups = "slow", description = "Test Account search")
public void testSearch() throws Exception {
final MutableAccountData mutableAccountData1 = createAccountData();
mutableAccountData1.setEmail("john@acme.com");
mutableAccountData1.setCompanyName("Acme, Inc.");
final AccountModelDao account1ModelDao = new AccountModelDao(UUID.randomUUID(), mutableAccountData1);
final AccountData accountData1 = new DefaultAccount(account1ModelDao);
accountUserApi.createAccount(accountData1, callContext);

final MutableAccountData mutableAccountData2 = createAccountData();
mutableAccountData2.setEmail("bob@gmail.com");
mutableAccountData2.setCompanyName("Acme, Inc.");
final AccountModelDao account2ModelDao = new AccountModelDao(UUID.randomUUID(), mutableAccountData2);
final AccountData accountData2 = new DefaultAccount(account2ModelDao);
accountUserApi.createAccount(accountData2, callContext);

final Pagination<Account> search1 = accountUserApi.searchAccounts("Inc.", 0L, 5L, callContext);
Assert.assertEquals(search1.getCurrentOffset(), (Long) 0L);
Assert.assertNull(search1.getNextOffset());
Assert.assertEquals(search1.getMaxNbRecords(), (Long) 2L);
Assert.assertEquals(search1.getTotalNbRecords(), (Long) 2L);
Assert.assertEquals(ImmutableList.<Account>copyOf(search1.iterator()).size(), 2);

final Pagination<Account> search2 = accountUserApi.searchAccounts("Inc.", 0L, 1L, callContext);
Assert.assertEquals(search2.getCurrentOffset(), (Long) 0L);
Assert.assertEquals(search2.getNextOffset(), (Long) 1L);
Assert.assertEquals(search2.getMaxNbRecords(), (Long) 2L);
Assert.assertEquals(search2.getTotalNbRecords(), (Long) 2L);
Assert.assertEquals(ImmutableList.<Account>copyOf(search2.iterator()).size(), 1);

final Pagination<Account> search3 = accountUserApi.searchAccounts("acme.com", 0L, 5L, callContext);
Assert.assertEquals(search3.getCurrentOffset(), (Long) 0L);
Assert.assertNull(search3.getNextOffset());
Assert.assertEquals(search3.getMaxNbRecords(), (Long) 2L);
Assert.assertEquals(search3.getTotalNbRecords(), (Long) 1L);
Assert.assertEquals(ImmutableList.<Account>copyOf(search3.iterator()).size(), 1);

// Exact search will fail
final Pagination<Account> search4 = accountUserApi.searchAccounts("acme.com", -1L, 1L, callContext);
Assert.assertEquals(search4.getCurrentOffset(), (Long) 0L);
Assert.assertNull(search4.getNextOffset());
// Not computed
Assert.assertNull(search4.getMaxNbRecords());
Assert.assertEquals(search4.getTotalNbRecords(), (Long) 0L);
Assert.assertEquals(ImmutableList.<Account>copyOf(search4.iterator()).size(), 0);

final Pagination<Account> search5 = accountUserApi.searchAccounts("john@acme.com", -1L, 1L, callContext);
Assert.assertEquals(search5.getCurrentOffset(), (Long) 0L);
Assert.assertNull(search5.getNextOffset());
// Not computed
Assert.assertNull(search5.getMaxNbRecords());
Assert.assertEquals(search5.getTotalNbRecords(), (Long) 1L);
Assert.assertEquals(ImmutableList.<Account>copyOf(search5.iterator()).size(), 1);
}

@Test(groups = "slow", description = "Test Account creation generates an event")
public void testBusEvents() throws Exception {
final AccountEventHandler eventHandler = new AccountEventHandler();
Expand Down
Expand Up @@ -68,7 +68,7 @@ public class PaymentStateMachineHelper {
private static final String VOID_FAILED = "VOID_FAILED";
private static final String CHARGEBACK_FAILED = "CHARGEBACK_FAILED";

private static final String AUTH_ERRORED = "AUTH_ERRORED";
private static final String AUTHORIZE_ERRORED = "AUTH_ERRORED";
private static final String CAPTURE_ERRORED = "CAPTURE_ERRORED";
private static final String PURCHASE_ERRORED = "PURCHASE_ERRORED";
private static final String REFUND_ERRORED = "REFUND_ERRORED";
Expand All @@ -78,7 +78,7 @@ public class PaymentStateMachineHelper {

private final StateMachineConfigCache stateMachineConfigCache;

public static final String[] STATE_NAMES = {AUTH_ERRORED,
public static final String[] STATE_NAMES = {AUTHORIZE_ERRORED,
AUTHORIZE_FAILED,
AUTHORIZE_PENDING,
AUTHORIZE_SUCCESS,
Expand Down Expand Up @@ -161,7 +161,7 @@ public String getPendingStateForTransaction(final TransactionType transactionTyp
public String getErroredStateForTransaction(final TransactionType transactionType) {
switch (transactionType) {
case AUTHORIZE:
return AUTH_ERRORED;
return AUTHORIZE_ERRORED;
case CAPTURE:
return CAPTURE_ERRORED;
case PURCHASE:
Expand Down
Expand Up @@ -254,7 +254,7 @@ public Iterator<PaymentModelDao> build(final PaymentSqlDao paymentSqlDao, final
@Override
public Pagination<PaymentModelDao> searchPayments(final String searchKey, final Long offset, final Long limit, final InternalTenantContext context) {
// Optimization: if the search key looks like a state name (e.g. _ERRORED), assume the user is searching by state only
final List<String> paymentStates = shouldSearchByStateNameOnly(searchKey);
final List<String> paymentStates = expandSearchFilterToStateNames(searchKey);

final String likeSearchKey = String.format("%%%s%%", searchKey);
return paginationHelper.getPagination(PaymentSqlDao.class,
Expand All @@ -274,7 +274,7 @@ public Iterator<PaymentModelDao> build(final PaymentSqlDao paymentSqlDao, final
context);
}

private List<String> shouldSearchByStateNameOnly(final String searchKey) {
private List<String> expandSearchFilterToStateNames(final String searchKey) {
final Pattern pattern = Pattern.compile(".*" + searchKey + ".*");

// Note that technically, we should look at all of the available state names in the database instead since the state machine is configurable. The common use-case
Expand Down
6 changes: 6 additions & 0 deletions profiles/killbill/src/test/resources/logback.travis.xml
Expand Up @@ -29,6 +29,12 @@
<logger name="jdbc.resultsettable" level="OFF"/>
<logger name="jdbc.connection" level="OFF"/>

<logger name="org.apache.shiro.realm.text.IniRealm" level="ERROR"/>
<logger name="org.slf4j.Logger" level="ERROR"/>

<logger name="org.killbill.billing.osgi" level="ERROR"/>
<logger name="org.killbill.bus" level="ERROR"/>

<root level="WARN">
<appender-ref ref="STDOUT"/>
</root>
Expand Down

0 comments on commit 6ce617c

Please sign in to comment.