CLOUDSTACK-9153: When negative credits are added to an account the

balance credits can become negative for that account. This will fix will
lock the account if quota is enforced.
This commit is contained in:
Abhinandan Prateek 2015-12-13 09:28:02 +05:30
parent 90cea824e7
commit 5025011d31
5 changed files with 79 additions and 35 deletions

View File

@ -47,8 +47,6 @@ public interface QuotaResponseBuilder {
QuotaBalanceResponse createQuotaLastBalanceResponse(List<QuotaBalanceVO> quotaBalance, Date startDate);
QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Double amount, Long updatedBy, Date despositedOn);
List<QuotaUsageVO> getQuotaUsage(QuotaStatementCmd cmd);
List<QuotaBalanceVO> getQuotaBalance(QuotaBalanceCmd cmd);

View File

@ -20,6 +20,7 @@ import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.AccountVO;
import com.cloud.user.User;
import com.cloud.user.dao.AccountDao;
@ -47,7 +48,6 @@ import org.apache.cloudstack.quota.vo.QuotaCreditsVO;
import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
import org.apache.cloudstack.quota.vo.QuotaTariffVO;
import org.apache.cloudstack.quota.vo.QuotaUsageVO;
import org.apache.cloudstack.region.RegionManager;
import org.apache.commons.lang.StringEscapeUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@ -95,7 +95,7 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
@Inject
private DomainDao _domainDao;
@Inject
private RegionManager _regionMgr;
private AccountManager _accountMgr;
@Inject
private QuotaStatement _statement;
@ -383,31 +383,41 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
@Override
public QuotaCreditsResponse addQuotaCredits(Long accountId, Long domainId, Double amount, Long updatedBy) {
Date depositDate = new Date();
Date adjustedStartDate = _quotaService.computeAdjustedTime(depositDate);
QuotaBalanceVO qb = _quotaBalanceDao.findLaterBalanceEntry(accountId, domainId, adjustedStartDate);
Date despositedOn = _quotaService.computeAdjustedTime(new Date());
QuotaBalanceVO qb = _quotaBalanceDao.findLaterBalanceEntry(accountId, domainId, despositedOn);
if (qb != null) {
throw new InvalidParameterValueException("Incorrect deposit date: " + adjustedStartDate + " there are balance entries after this date");
throw new InvalidParameterValueException("Incorrect deposit date: " + despositedOn + " there are balance entries after this date");
}
return addQuotaCredits(accountId, domainId, amount, updatedBy, adjustedStartDate);
}
@Override
public QuotaCreditsResponse addQuotaCredits(final Long accountId, final Long domainId, final Double amount, final Long updatedBy, final Date despositedOn) {
QuotaCreditsVO credits = new QuotaCreditsVO(accountId, domainId, new BigDecimal(amount), updatedBy);
s_logger.debug("AddQuotaCredits: Depositing " + amount + " on adjusted date " + despositedOn);
credits.setUpdatedOn(despositedOn);
QuotaCreditsVO result = _quotaCreditsDao.saveCredits(credits);
final AccountVO account = _accountDao.findById(accountId);
final boolean lockAccountEnforcement = "true".equalsIgnoreCase(QuotaConfig.QuotaEnableEnforcement.value());
final BigDecimal currentAccountBalance = _quotaBalanceDao.lastQuotaBalance(accountId, domainId, startOfNextDay(despositedOn));
if (lockAccountEnforcement && (currentAccountBalance.compareTo(new BigDecimal(0)) >= 0)) {
if (account.getState() == Account.State.locked) {
_regionMgr.enableAccount(account.getAccountName(), domainId, accountId);
final BigDecimal currentAccountBalance = _quotaBalanceDao.lastQuotaBalance(accountId, domainId, startOfNextDay(new Date(despositedOn.getTime())));
if (s_logger.isDebugEnabled()) {
s_logger.debug("AddQuotaCredits: Depositing " + amount + " on adjusted date " + despositedOn + ", current balance " + currentAccountBalance);
}
// update quota account with the balance
_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);
}
} else { // currentAccountBalance < 0 then lock the account
if (account.getState() == Account.State.enabled) {
s_logger.info("Locking account " + account.getAccountName() + " , due to negative balance " + currentAccountBalance);
_accountMgr.lockAccount(account.getAccountName(), domainId, accountId);
}
}
}
@ -500,8 +510,11 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
Calendar c = Calendar.getInstance();
c.setTime(dt);
c.add(Calendar.DATE, 1);
dt = c.getTime();
return dt;
c.set(Calendar.HOUR, 0);
c.set(Calendar.MINUTE, 0);
c.set(Calendar.SECOND, 0);
c.set(Calendar.MILLISECOND, 0);
return c.getTime();
}
@Override
@ -509,8 +522,11 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder {
Calendar c = Calendar.getInstance();
c.setTime(new Date());
c.add(Calendar.DATE, 1);
Date dt = c.getTime();
return dt;
c.set(Calendar.HOUR, 0);
c.set(Calendar.MINUTE, 0);
c.set(Calendar.SECOND, 0);
c.set(Calendar.MILLISECOND, 0);
return c.getTime();
}
}

View File

@ -16,11 +16,13 @@
//under the License.
package org.apache.cloudstack.quota;
import com.cloud.user.AccountVO;
import com.cloud.utils.component.PluggableService;
import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
import org.apache.cloudstack.quota.vo.QuotaUsageVO;
import java.math.BigDecimal;
import java.util.Date;
import java.util.List;
@ -38,4 +40,6 @@ public interface QuotaService extends PluggableService {
Boolean isQuotaServiceEnabled();
boolean saveQuotaAccount(AccountVO account, BigDecimal aggrUsage, Date endDate);
}

View File

@ -138,8 +138,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] { QuotaPluginEnabled, QuotaEnableEnforcement, QuotaCurrencySymbol, QuotaStatementPeriod, QuotaSmtpHost, QuotaSmtpPort, QuotaSmtpTimeout, QuotaSmtpUser, QuotaSmtpPassword,
QuotaSmtpAuthType, QuotaSmtpSender };
return new ConfigKey<?>[] {QuotaPluginEnabled, QuotaEnableEnforcement, QuotaCurrencySymbol, QuotaStatementPeriod, QuotaSmtpHost, QuotaSmtpPort, QuotaSmtpTimeout,
QuotaSmtpUser, QuotaSmtpPassword, QuotaSmtpAuthType, QuotaSmtpSender};
}
@Override
@ -193,8 +193,8 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
} else if (startDate.before(endDate)) {
Date adjustedEndDate = computeAdjustedTime(endDate);
if (s_logger.isDebugEnabled()) {
s_logger.debug("getQuotaBalance2: Getting quota balance records for account: " + accountId + ", domainId: " + domainId + ", between " + adjustedStartDate + " and "
+ adjustedEndDate);
s_logger.debug("getQuotaBalance2: Getting quota balance records for account: " + accountId + ", domainId: " + domainId + ", between " + adjustedStartDate
+ " and " + adjustedEndDate);
}
List<QuotaBalanceVO> qbrecords = _quotaBalanceDao.findQuotaBalance(accountId, domainId, adjustedStartDate, adjustedEndDate);
if (s_logger.isDebugEnabled()) {
@ -286,6 +286,30 @@ public class QuotaServiceImpl extends ManagerBase implements QuotaService, Confi
}
}
@Override
public boolean saveQuotaAccount(final AccountVO account, final BigDecimal aggrUsage, final Date endDate) {
// update quota_accounts
QuotaAccountVO quota_account = _quotaAcc.findByIdQuotaAccount(account.getAccountId());
if (quota_account == null) {
quota_account = new QuotaAccountVO(account.getAccountId());
quota_account.setQuotaBalance(aggrUsage);
quota_account.setQuotaBalanceDate(endDate);
if (s_logger.isDebugEnabled()) {
s_logger.debug(quota_account);
}
_quotaAcc.persistQuotaAccount(quota_account);
return true;
} else {
quota_account.setQuotaBalance(aggrUsage);
quota_account.setQuotaBalanceDate(endDate);
if (s_logger.isDebugEnabled()) {
s_logger.debug(quota_account);
}
return _quotaAcc.updateQuotaAccount(account.getAccountId(), quota_account);
}
}
@Override
public void setMinBalance(Long accountId, Double balance) {
QuotaAccountVO acc = _quotaAcc.findByIdQuotaAccount(accountId);

View File

@ -18,6 +18,7 @@ package org.apache.cloudstack.api.response;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserDao;
@ -35,7 +36,6 @@ import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
import org.apache.cloudstack.quota.vo.QuotaCreditsVO;
import org.apache.cloudstack.quota.vo.QuotaEmailTemplatesVO;
import org.apache.cloudstack.quota.vo.QuotaTariffVO;
import org.apache.cloudstack.region.RegionManager;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Test;
@ -50,6 +50,8 @@ import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import javax.inject.Inject;
@RunWith(MockitoJUnitRunner.class)
public class QuotaResponseBuilderImplTest extends TestCase {
@ -68,8 +70,8 @@ public class QuotaResponseBuilderImplTest extends TestCase {
QuotaService quotaService;
@Mock
AccountDao accountDao;
@Mock
RegionManager regionMgr;
@Inject
AccountManager accountMgr;
QuotaResponseBuilderImpl quotaResponseBuilder = new QuotaResponseBuilderImpl();
@ -106,9 +108,9 @@ public class QuotaResponseBuilderImplTest extends TestCase {
accountDaoField.setAccessible(true);
accountDaoField.set(quotaResponseBuilder, accountDao);
Field regionMgrField = QuotaResponseBuilderImpl.class.getDeclaredField("_regionMgr");
Field regionMgrField = QuotaResponseBuilderImpl.class.getDeclaredField("_accountMgr");
regionMgrField.setAccessible(true);
regionMgrField.set(quotaResponseBuilder, regionMgr);
regionMgrField.set(quotaResponseBuilder, accountMgr);
}
private QuotaTariffVO makeTariffTestData() {
@ -133,22 +135,22 @@ public class QuotaResponseBuilderImplTest extends TestCase {
@Test
public void testAddQuotaCredits() {
final long accountId = 2L;
final long domainId = 2L;
final long domainId = 1L;
final double amount = 11.0;
final long updatedBy = 2L;
final Date now = new Date();
QuotaCreditsVO credit = new QuotaCreditsVO();
credit.setCredit(new BigDecimal(amount));
Mockito.when(quotaCreditsDao.saveCredits(Mockito.any(QuotaCreditsVO.class))).thenReturn(credit);
Mockito.when(quotaBalanceDao.lastQuotaBalance(Mockito.anyLong(), Mockito.anyLong(), Mockito.any(Date.class))).thenReturn(new BigDecimal(111));
Mockito.when(quotaService.computeAdjustedTime(Mockito.any(Date.class))).thenReturn(new Date());
AccountVO account = new AccountVO();
account.setState(Account.State.locked);
Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(account);
QuotaCreditsResponse resp = quotaResponseBuilder.addQuotaCredits(accountId, domainId, amount, updatedBy, now);
QuotaCreditsResponse resp = quotaResponseBuilder.addQuotaCredits(accountId, domainId, amount, updatedBy);
assertTrue(resp.getCredits().compareTo(credit.getCredit()) == 0);
}