Quota: consolidated lockable account check to a method. Added unit tests

to check lockablity of various accounts
This commit is contained in:
Abhinandan Prateek 2016-01-18 13:41:38 +05:30
parent 06fe7844db
commit 04ca7e57d5
11 changed files with 69 additions and 23 deletions

View File

@ -27,7 +27,6 @@ import com.cloud.user.dao.UserDao;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.exception.CloudRuntimeException;
import com.google.common.base.Strings;
import com.sun.mail.smtp.SMTPMessage;
import com.sun.mail.smtp.SMTPSSLTransport;
@ -83,6 +82,8 @@ public class QuotaAlertManagerImpl extends ManagerBase implements QuotaAlertMana
private ConfigurationDao _configDao;
@Inject
private QuotaUsageDao _quotaUsage;
@Inject
private QuotaManager _quotaManager;
private EmailQuotaAlert _emailQuotaAlert;
private boolean _lockAccountEnforcement = false;
@ -158,7 +159,7 @@ public class QuotaAlertManagerImpl extends ManagerBase implements QuotaAlertMana
}
if (accountBalance.compareTo(zeroBalance) < 0) {
if (_lockAccountEnforcement && (lockable == 1)) {
if (account.getType() == Account.ACCOUNT_TYPE_NORMAL) {
if (_quotaManager.isLockable(account)) {
s_logger.info("Locking account " + account.getAccountName() + " due to quota < 0.");
lockAccount(account.getId());
}
@ -382,7 +383,8 @@ public class QuotaAlertManagerImpl extends ManagerBase implements QuotaAlertMana
public void sendQuotaAlert(List<String> emails, String subject, String body) throws MessagingException, UnsupportedEncodingException {
if (_smtpSession == null) {
throw new CloudRuntimeException("Unable to create smtp session.");
s_logger.error("Unable to create smtp session.");
return;
}
SMTPMessage msg = new SMTPMessage(_smtpSession);
msg.setSender(new InternetAddress(_emailSender, _emailSender));

View File

@ -16,10 +16,13 @@
//under the License.
package org.apache.cloudstack.quota;
import com.cloud.user.AccountVO;
import com.cloud.utils.component.Manager;
public interface QuotaManager extends Manager {
boolean calculateQuotaUsage();
boolean isLockable(AccountVO account);
}

View File

@ -463,4 +463,9 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager {
return quota_usage;
}
@Override
public boolean isLockable(AccountVO account) {
return (account.getType() == AccountVO.ACCOUNT_TYPE_NORMAL || account.getType() == AccountVO.ACCOUNT_TYPE_DOMAIN_ADMIN);
}
}

View File

@ -156,7 +156,7 @@ public class QuotaBalanceDaoImpl extends GenericDaoBase<QuotaBalanceVO, Long> im
// get records before startDate to find start balance
for (QuotaBalanceVO entry : quotaUsageRecords) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("FindQuotaBalIance Entry=" + entry);
s_logger.debug("FindQuotaBalance Entry=" + entry);
}
if (entry.getCreditsId() > 0) {
trimmedRecords.add(entry);

View File

@ -197,4 +197,42 @@ public class QuotaManagerImplTest extends TestCase {
Mockito.verify(quotaAcc, Mockito.times(1)).persistQuotaAccount(Mockito.any(QuotaAccountVO.class));
}
private AccountVO accountVO = new AccountVO();
@Test
public void testAdminLockableAccount() {
accountVO.setType(Account.ACCOUNT_TYPE_ADMIN);
assertFalse(quotaManager.isLockable(accountVO));
}
@Test
public void testNormalLockableAccount() {
accountVO.setType(Account.ACCOUNT_TYPE_NORMAL);
assertTrue(quotaManager.isLockable(accountVO));
}
@Test
public void tesDomainAdmingLockableAccount() {
accountVO.setType(Account.ACCOUNT_TYPE_DOMAIN_ADMIN);
assertTrue(quotaManager.isLockable(accountVO));
}
@Test
public void testReadOnlyAdminLockableAccount() {
accountVO.setType(Account.ACCOUNT_TYPE_READ_ONLY_ADMIN);
assertFalse(quotaManager.isLockable(accountVO));
}
@Test
public void testResourceDomainAdminLockableAccount() {
accountVO.setType(Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN);
assertFalse(quotaManager.isLockable(accountVO));
}
@Test
public void testProjectLockableAccount() {
accountVO.setType(Account.ACCOUNT_TYPE_PROJECT);
assertFalse(quotaManager.isLockable(accountVO));
}
}

View File

@ -123,7 +123,7 @@ public class QuotaCreditsCmd extends BaseCmd {
if (getValue() == null) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Please send a valid non-empty quota value");
}
if (getQuotaEnforce() != null && getQuotaEnforce()) {
if (getQuotaEnforce() != null) {
_quotaService.setLockAccount(accountId, getQuotaEnforce());
}
if (getMinBalance() != null) {
@ -133,7 +133,7 @@ public class QuotaCreditsCmd extends BaseCmd {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Please set a value for min balance");
}
final QuotaCreditsResponse response = _responseBuilder.addQuotaCredits(accountId, getDomainId(), getValue(), CallContext.current().getCallingUserId());
final QuotaCreditsResponse response = _responseBuilder.addQuotaCredits(accountId, getDomainId(), getValue(), CallContext.current().getCallingUserId(), getQuotaEnforce());
response.setResponseName(getCommandName());
response.setObjectName("quotacredits");
setResponseObject(response);

View File

@ -51,7 +51,7 @@ public interface QuotaResponseBuilder {
List<QuotaBalanceVO> getQuotaBalance(QuotaBalanceCmd cmd);
QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Double amount, Long updatedBy);
QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Double amount, Long updatedBy, Boolean enforce);
List<QuotaEmailTemplateResponse> listQuotaEmailTemplates(QuotaEmailTemplateListCmd cmd);

View File

@ -32,6 +32,7 @@ import org.apache.cloudstack.api.command.QuotaEmailTemplateUpdateCmd;
import org.apache.cloudstack.api.command.QuotaStatementCmd;
import org.apache.cloudstack.api.command.QuotaTariffListCmd;
import org.apache.cloudstack.api.command.QuotaTariffUpdateCmd;
import org.apache.cloudstack.quota.QuotaManager;
import org.apache.cloudstack.quota.QuotaService;
import org.apache.cloudstack.quota.QuotaStatement;
import org.apache.cloudstack.quota.constant.QuotaConfig;
@ -98,6 +99,8 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
private AccountManager _accountMgr;
@Inject
private QuotaStatement _statement;
@Inject
private QuotaManager _quotaManager;
@Override
public QuotaTariffResponse createQuotaTariffResponse(QuotaTariffVO tariff) {
@ -384,11 +387,10 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
}
@Override
public QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Double amount, Long updatedBy) {
public QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Double amount, Long updatedBy, Boolean enforce) {
Date despositedOn = _quotaService.computeAdjustedTime(new Date());
QuotaBalanceVO qb = _quotaBalanceDao.findLaterBalanceEntry(accountId, domainId, despositedOn);
if (qb != null) {
throw new InvalidParameterValueException("Incorrect deposit date: " + despositedOn + " there are balance entries after this date");
}
@ -407,16 +409,12 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
_quotaService.saveQuotaAccount(account, currentAccountBalance, despositedOn);
if (lockAccountEnforcement) {
if (currentAccountBalance.compareTo(new BigDecimal(0)) >= 0) {
if (account.getType() == Account.ACCOUNT_TYPE_NORMAL) {
if (account.getState() == Account.State.locked) {
s_logger.info("UnLocking account " + account.getAccountName() + " , due to positive balance " + currentAccountBalance);
_accountMgr.enableAccount(account.getAccountName(), domainId, accountId);
}
} else {
s_logger.warn("Only normal accounts will get locked " + account.getAccountName() + " even if they have run out of quota " + currentAccountBalance);
if (account.getState() == Account.State.locked) {
s_logger.info("UnLocking account " + account.getAccountName() + " , due to positive balance " + currentAccountBalance);
_accountMgr.enableAccount(account.getAccountName(), domainId, accountId);
}
} else { // currentAccountBalance < 0 then lock the account
if (account.getState() == Account.State.enabled) {
if (_quotaManager.isLockable(account) && account.getState() == Account.State.enabled && enforce) {
s_logger.info("Locking account " + account.getAccountName() + " , due to negative balance " + currentAccountBalance);
_accountMgr.lockAccount(account.getAccountName(), domainId, accountId);
}

View File

@ -274,14 +274,14 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
}
@Override
public void setLockAccount(Long accountId, Boolean state) {
public void setLockAccount(Long accountId, Boolean enforce) {
QuotaAccountVO acc = _quotaAcc.findByIdQuotaAccount(accountId);
if (acc == null) {
acc = new QuotaAccountVO(accountId);
acc.setQuotaEnforce(state ? 1 : 0);
acc.setQuotaEnforce(enforce ? 1 : 0);
_quotaAcc.persistQuotaAccount(acc);
} else {
acc.setQuotaEnforce(state ? 1 : 0);
acc.setQuotaEnforce(enforce ? 1 : 0);
_quotaAcc.updateQuotaAccount(accountId, acc);
}
}

View File

@ -65,7 +65,7 @@ public class QuotaCreditsCmdTest extends TestCase {
AccountVO acc = new AccountVO();
acc.setId(2L);
Mockito.when(accountService.getActiveAccountByName(Mockito.anyString(), Mockito.anyLong())).thenReturn(acc);
Mockito.when(responseBuilder.addQuotaCredits(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyDouble(), Mockito.anyLong())).thenReturn(new QuotaCreditsResponse());
Mockito.when(responseBuilder.addQuotaCredits(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyDouble(), Mockito.anyLong(), Mockito.anyBoolean())).thenReturn(new QuotaCreditsResponse());
// No value provided test
try {
@ -79,7 +79,7 @@ public class QuotaCreditsCmdTest extends TestCase {
cmd.execute();
Mockito.verify(quotaService, Mockito.times(0)).setLockAccount(Mockito.anyLong(), Mockito.anyBoolean());
Mockito.verify(quotaService, Mockito.times(1)).setMinBalance(Mockito.anyLong(), Mockito.anyDouble());
Mockito.verify(responseBuilder, Mockito.times(1)).addQuotaCredits(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyDouble(), Mockito.anyLong());
Mockito.verify(responseBuilder, Mockito.times(1)).addQuotaCredits(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyDouble(), Mockito.anyLong(), Mockito.anyBoolean());
}

View File

@ -150,7 +150,7 @@ public class QuotaResponseBuilderImplTest extends TestCase {
account.setState(Account.State.locked);
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(account);
QuotaCreditsResponse resp = quotaResponseBuilder.addQuotaCredits(accountId, domainId, amount, updatedBy);
QuotaCreditsResponse resp = quotaResponseBuilder.addQuotaCredits(accountId, domainId, amount, updatedBy, true);
assertTrue(resp.getCredits().compareTo(credit.getCredit()) == 0);
}