From 3b18d74be6b441e0a055e2a8eed1e71c01d6ec0a Mon Sep 17 00:00:00 2001 From: Abhinandan Prateek Date: Thu, 17 Dec 2015 10:32:33 +0530 Subject: [PATCH] CLOUDSTACK-9174: A deleted account results in NPE When an account is deleted from cloudstack for which quota is still being calculated and if the quota reaches minimum threshold then quota service will try to alert the user. This results in NPE and is fixed by excluding such accounts from alerting and other quota related mechanisms. --- .../quota/QuotaAlertManagerImpl.java | 1 + .../cloudstack/quota/QuotaManagerImpl.java | 7 ++++--- .../cloudstack/quota/QuotaStatementImpl.java | 18 ++++++++++-------- .../api/command/QuotaSummaryCmd.java | 2 +- .../api/response/QuotaResponseBuilderImpl.java | 6 +++++- ui/plugins/quota/quota.js | 7 ------- 6 files changed, 21 insertions(+), 20 deletions(-) diff --git a/framework/quota/src/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java b/framework/quota/src/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java index a57e0c27db9..a25ed042825 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java +++ b/framework/quota/src/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java @@ -153,6 +153,7 @@ public class QuotaAlertManagerImpl extends ManagerBase implements QuotaAlertMana BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance(); if (accountBalance != null) { AccountVO account = _accountDao.findById(quotaAccount.getId()); + if (account == null) continue; // the account is removed if (s_logger.isDebugEnabled()) { s_logger.debug("checkAndSendQuotaAlertEmails: Check id=" + account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", lockable=" + lockable); } diff --git a/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java index ac14718fd4e..0a59fa20fd3 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java +++ b/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java @@ -360,10 +360,11 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { BigDecimal rawusage; // get service offering details ServiceOfferingVO serviceoffering = _serviceOfferingDao.findServiceOffering(usageRecord.getVmInstanceId(), usageRecord.getOfferingId()); + if (serviceoffering == null) return quotalist; rawusage = new BigDecimal(usageRecord.getRawUsage()); QuotaTariffVO tariff = _quotaTariffDao.findTariffPlanByUsageType(QuotaTypes.CPU_NUMBER, usageRecord.getEndDate()); - if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0) { + if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0 && serviceoffering.getCpu() != null) { BigDecimal cpu = new BigDecimal(serviceoffering.getCpu()); onehourcostpercpu = tariff.getCurrencyValue().multiply(aggregationRatio); cpuquotausgage = rawusage.multiply(onehourcostpercpu).multiply(cpu); @@ -373,7 +374,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { quotalist.add(quota_usage); } tariff = _quotaTariffDao.findTariffPlanByUsageType(QuotaTypes.CPU_CLOCK_RATE, usageRecord.getEndDate()); - if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0) { + if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0 && serviceoffering.getSpeed() != null) { BigDecimal speed = new BigDecimal(serviceoffering.getSpeed() / 100.00); onehourcostper100mhz = tariff.getCurrencyValue().multiply(aggregationRatio); speedquotausage = rawusage.multiply(onehourcostper100mhz).multiply(speed); @@ -383,7 +384,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { quotalist.add(quota_usage); } tariff = _quotaTariffDao.findTariffPlanByUsageType(QuotaTypes.MEMORY, usageRecord.getEndDate()); - if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0) { + if (tariff != null && tariff.getCurrencyValue().compareTo(BigDecimal.ZERO) != 0 && serviceoffering.getRamSize() != null) { BigDecimal memory = new BigDecimal(serviceoffering.getRamSize()); onehourcostper1mb = tariff.getCurrencyValue().multiply(aggregationRatio); memoryquotausage = rawusage.multiply(onehourcostper1mb).multiply(memory); diff --git a/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java b/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java index 682b2ef0366..dde30697213 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java +++ b/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java @@ -122,15 +122,17 @@ public class QuotaStatementImpl extends ManagerBase implements QuotaStatement { Date lastStatementDate = quotaAccount.getLastStatementDate(); if (interval != null) { AccountVO account = _accountDao.findById(quotaAccount.getId()); - if (lastStatementDate == null || getDifferenceDays(lastStatementDate, new Date()) >= s_LAST_STATEMENT_SENT_DAYS + 1) { - BigDecimal quotaUsage = _quotaUsage.findTotalQuotaUsage(account.getAccountId(), account.getDomainId(), null, interval[0].getTime(), interval[1].getTime()); - s_logger.info("For account=" + quotaAccount.getId() + ", quota used = " + quotaUsage); - // send statement - deferredQuotaEmailList.add(new DeferredQuotaEmail(account, quotaAccount, quotaUsage, QuotaConfig.QuotaEmailTemplateTypes.QUOTA_STATEMENT)); - } else { - if (s_logger.isDebugEnabled()) { - s_logger.debug("For " + quotaAccount.getId() + " the statement has been sent recently"); + if (account != null) { + if (lastStatementDate == null || getDifferenceDays(lastStatementDate, new Date()) >= s_LAST_STATEMENT_SENT_DAYS + 1) { + BigDecimal quotaUsage = _quotaUsage.findTotalQuotaUsage(account.getAccountId(), account.getDomainId(), null, interval[0].getTime(), interval[1].getTime()); + s_logger.info("For account=" + quotaAccount.getId() + ", quota used = " + quotaUsage); + // send statement + deferredQuotaEmailList.add(new DeferredQuotaEmail(account, quotaAccount, quotaUsage, QuotaConfig.QuotaEmailTemplateTypes.QUOTA_STATEMENT)); + } else { + if (s_logger.isDebugEnabled()) { + s_logger.debug("For " + quotaAccount.getId() + " the statement has been sent recently"); + } } } } else if (lastStatementDate != null) { diff --git a/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java b/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java index b4f2001f95b..7dd4dfee03e 100644 --- a/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java +++ b/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java @@ -59,7 +59,7 @@ public class QuotaSummaryCmd extends BaseListCmd { public void execute() { Account caller = CallContext.current().getCallingAccount(); List responses; - if (caller.getAccountId() <= 2) { //non root admin or system + if (caller.getAccountId() <= 2) { // root admin or system if (getAccountName() != null && getDomainId() != null) responses = _responseBuilder.createQuotaSummaryResponse(caller.getAccountName(), caller.getDomainId()); else diff --git a/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java b/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java index 45e0e69ec09..6fd3aac3cd6 100644 --- a/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java +++ b/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java @@ -138,6 +138,7 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder { } else { for (final QuotaAccountVO quotaAccount : _quotaAccountDao.listAllQuotaAccount()) { AccountVO account = _accountDao.findById(quotaAccount.getId()); + if (account == null) continue; QuotaSummaryResponse qr = getQuotaSummaryResponse(account); result.add(qr); } @@ -167,7 +168,7 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder { qr.setObjectName("summary"); return qr; } else { - throw new InvalidParameterValueException("Quota summary response for an account requires a valid account."); + return new QuotaSummaryResponse(); } } @@ -396,6 +397,9 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder { QuotaCreditsVO result = _quotaCreditsDao.saveCredits(credits); final AccountVO account = _accountDao.findById(accountId); + if (account == null) { + throw new InvalidParameterValueException("Account does not exist with account id " + accountId); + } final boolean lockAccountEnforcement = "true".equalsIgnoreCase(QuotaConfig.QuotaEnableEnforcement.value()); final BigDecimal currentAccountBalance = _quotaBalanceDao.lastQuotaBalance(accountId, domainId, startOfNextDay(new Date(despositedOn.getTime()))); if (s_logger.isDebugEnabled()) { diff --git a/ui/plugins/quota/quota.js b/ui/plugins/quota/quota.js index ea3621d6fda..fa478092b9f 100644 --- a/ui/plugins/quota/quota.js +++ b/ui/plugins/quota/quota.js @@ -333,13 +333,6 @@ }); }, detailView: { - viewAll: [{ - path: 'quota.quotastatement', - label: 'label.quota.statement.quota' - },{ - path: 'quota.balancestatement', - label: 'label.quota.statement.balance' - }], actions: { add: { label: 'label.quota.add.credits',