diff --git a/.travis.yml b/.travis.yml index e8847be803..531da15473 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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' diff --git a/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java b/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java index d4747479cf..2eccd3cec6 100644 --- a/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java +++ b/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java @@ -60,4 +60,9 @@ public void updatePaymentMethod(@Bind("id") String accountId, @SqlQuery List getAccountsByParentId(@Bind("parentAccountId") UUID parentAccountId, @BindBean final InternalTenantContext context); + + @SqlQuery + public AccountModelDao luckySearch(@Bind("searchKey") final String searchKey, + @BindBean final InternalTenantContext context); + } diff --git a/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java b/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java index f1186a2a52..b959cb73c4 100644 --- a/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java +++ b/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java @@ -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; @@ -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 implements AccountDao { @@ -110,6 +112,25 @@ public AccountModelDao inTransaction(final EntitySqlDaoWrapperFactory entitySqlD @Override public Pagination 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() { + @Override + public AccountModelDao inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception { + return entitySqlDaoWrapperFactory.become(AccountSqlDao.class).luckySearch(searchKey, context); + } + }); + return new DefaultPagination(0L, + 1L, + accountModelDao == null ? 0L : 1L, + null, // We don't compute stats for speed in that case + accountModelDao == null ? ImmutableList.of().iterator() : ImmutableList.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() { @Override diff --git a/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg b/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg index d48a16bb65..f9ddfa5dd1 100644 --- a/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg +++ b/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg @@ -106,6 +106,64 @@ searchQuery(prefix) ::= << or company_name like :likeSearchKey >> +/** Unfortunately, we need to force MySQL to use the individual indexes */ +luckySearch() ::= << +select + +from t +join ( + select distinct + from ( + ( + select + from + where = :searchKey + + + limit 1 + ) + union all + ( + select + from + where name = :searchKey + + + limit 1 + ) + union all + ( + select + from + where email = :searchKey + + + limit 1 + ) + union all + ( + select + from + where external_key = :searchKey + + + limit 1 + ) + union all + ( + select + from + where company_name = :searchKey + + + limit 1 + ) + ) search_with_idx +) optimization on = +limit 1 +; +>> + getIdFromKey() ::= << SELECT id FROM accounts @@ -119,4 +177,4 @@ getAccountsByParentId() ::= << ; ->> \ No newline at end of file +>> diff --git a/account/src/main/resources/org/killbill/billing/account/ddl.sql b/account/src/main/resources/org/killbill/billing/account/ddl.sql index a24f27235c..195eb5b428 100644 --- a/account/src/main/resources/org/killbill/billing/account/ddl.sql +++ b/account/src/main/resources/org/killbill/billing/account/ddl.sql @@ -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 ( diff --git a/account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql b/account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql new file mode 100644 index 0000000000..54cd629925 --- /dev/null +++ b/account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql @@ -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); diff --git a/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java b/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java index 7934e126a2..8b532642a6 100644 --- a/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java +++ b/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java @@ -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 { @@ -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(); diff --git a/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java b/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java index 338c58cbe7..6149547a2f 100644 --- a/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java +++ b/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java @@ -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 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.copyOf(search1.iterator()).size(), 2); + + final Pagination 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.copyOf(search2.iterator()).size(), 1); + + final Pagination 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.copyOf(search3.iterator()).size(), 1); + + // Exact search will fail + final Pagination 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.copyOf(search4.iterator()).size(), 0); + + final Pagination 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.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(); diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java index 7e010b7489..daee9eb96e 100644 --- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java +++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java @@ -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"; @@ -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, @@ -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: diff --git a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java index bed7b5bf9d..020ff4974b 100644 --- a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java +++ b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java @@ -254,7 +254,7 @@ public Iterator build(final PaymentSqlDao paymentSqlDao, final @Override public Pagination 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 paymentStates = shouldSearchByStateNameOnly(searchKey); + final List paymentStates = expandSearchFilterToStateNames(searchKey); final String likeSearchKey = String.format("%%%s%%", searchKey); return paginationHelper.getPagination(PaymentSqlDao.class, @@ -274,7 +274,7 @@ public Iterator build(final PaymentSqlDao paymentSqlDao, final context); } - private List shouldSearchByStateNameOnly(final String searchKey) { + private List 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 diff --git a/profiles/killbill/src/test/resources/logback.travis.xml b/profiles/killbill/src/test/resources/logback.travis.xml index 32029744e1..c8380184fe 100644 --- a/profiles/killbill/src/test/resources/logback.travis.xml +++ b/profiles/killbill/src/test/resources/logback.travis.xml @@ -29,6 +29,12 @@ + + + + + +