GutoVeronezi commented on code in PR #8307:
URL: https://github.com/apache/cloudstack/pull/8307#discussion_r1491715284


##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));

Review Comment:
   Use new log format.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        AccountVO account = _accountDao.findById(quotaAccount.getId());
+        if (account == null) {
+            logger.debug(String.format("Account of %s is removed, thus it will 
not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        checkBalanceAndAddToEmailList(deferredQuotaEmailList, quotaAccount, 
account, accountBalance);
+    }
+
+    private void checkBalanceAndAddToEmailList(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount, AccountVO account, 
BigDecimal accountBalance) {
+        Date balanceDate = quotaAccount.getQuotaBalanceDate();
+        Date alertDate = quotaAccount.getQuotaAlertDate();
+        int lockable = quotaAccount.getQuotaEnforce();
+        BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
+
+        logger.debug(String.format("Checking %s with accountBalance [%s], 
alertDate [%s] and lockable [%s] to see if a quota alert email should be 
sent.", account,
+                accountBalance, alertDate, lockable));
+
+        QuotaEmailConfigurationVO quotaEmpty = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_EMPTY);
+        QuotaEmailConfigurationVO quotaLow = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_LOW);
+
+        boolean shouldSendEmail = alertDate == null || 
(balanceDate.after(alertDate) && getDifferenceDays(alertDate, new Date()) > 1);
+
+        if (accountBalance.compareTo(BigDecimal.ZERO) < 0) {
+            if (_lockAccountEnforcement && lockable == 1 && 
_quotaManager.isLockable(account)) {
+                logger.info(String.format("Locking %s, as quota balance is 
lower than 0.", account));
+                lockAccount(account.getId());
+            }
+            if (quotaEmpty != null && quotaEmpty.isEnabled() && 
shouldSendEmail) {
+                logger.debug(String.format("Adding %s to the deferred emails 
list, as quota balance is lower than 0.", account));
+                deferredQuotaEmailList.add(new DeferredQuotaEmail(account, 
quotaAccount, QuotaEmailTemplateTypes.QUOTA_EMPTY));
+                return;
+            }
+        } else if (accountBalance.compareTo(thresholdBalance) < 0 && quotaLow 
!= null && quotaLow.isEnabled() && shouldSendEmail) {
+            logger.debug(String.format("Adding %s to the deferred emails list, 
as quota balance [%s] is below the threshold [%s].", account, accountBalance, 
thresholdBalance));
+            deferredQuotaEmailList.add(new DeferredQuotaEmail(account, 
quotaAccount, QuotaEmailTemplateTypes.QUOTA_LOW));
+            return;
+        }
+        logger.debug(String.format("%s will not receive any quota alert emails 
in this round.", account));

Review Comment:
   Use new log format.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));

Review Comment:
   Use new log format.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        AccountVO account = _accountDao.findById(quotaAccount.getId());
+        if (account == null) {
+            logger.debug(String.format("Account of %s is removed, thus it will 
not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        checkBalanceAndAddToEmailList(deferredQuotaEmailList, quotaAccount, 
account, accountBalance);
+    }
+
+    private void checkBalanceAndAddToEmailList(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount, AccountVO account, 
BigDecimal accountBalance) {
+        Date balanceDate = quotaAccount.getQuotaBalanceDate();
+        Date alertDate = quotaAccount.getQuotaAlertDate();
+        int lockable = quotaAccount.getQuotaEnforce();
+        BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
+
+        logger.debug(String.format("Checking %s with accountBalance [%s], 
alertDate [%s] and lockable [%s] to see if a quota alert email should be 
sent.", account,
+                accountBalance, alertDate, lockable));
+
+        QuotaEmailConfigurationVO quotaEmpty = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_EMPTY);
+        QuotaEmailConfigurationVO quotaLow = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_LOW);
+
+        boolean shouldSendEmail = alertDate == null || 
(balanceDate.after(alertDate) && getDifferenceDays(alertDate, new Date()) > 1);
+
+        if (accountBalance.compareTo(BigDecimal.ZERO) < 0) {
+            if (_lockAccountEnforcement && lockable == 1 && 
_quotaManager.isLockable(account)) {
+                logger.info(String.format("Locking %s, as quota balance is 
lower than 0.", account));

Review Comment:
   Use new log format.



##########
plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java:
##########
@@ -654,4 +659,98 @@ public boolean deleteQuotaTariff(String quotaTariffUuid) {
         quotaTariff.setRemoved(_quotaService.computeAdjustedTime(new Date()));
         return _quotaTariffDao.updateQuotaTariff(quotaTariff);
     }
+
+    @Override
+    public Pair<QuotaEmailConfigurationVO, Double> 
configureQuotaEmail(QuotaConfigureEmailCmd cmd) {
+        validateQuotaConfigureEmailCmdParameters(cmd);
+
+        Double minBalance = cmd.getMinBalance();
+
+        if (minBalance != null) {
+            _quotaService.setMinBalance(cmd.getAccountId(), 
cmd.getMinBalance());
+        }
+
+        QuotaEmailConfigurationVO configurationVO = 
getQuotaEmailConfigurationVo(cmd);
+        return new Pair<>(configurationVO, minBalance);
+    }
+
+    private QuotaEmailConfigurationVO 
getQuotaEmailConfigurationVo(QuotaConfigureEmailCmd cmd) {

Review Comment:
   We could turn to `protected` and add unit tests.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        AccountVO account = _accountDao.findById(quotaAccount.getId());
+        if (account == null) {
+            logger.debug(String.format("Account of %s is removed, thus it will 
not receive quota alert emails.", quotaAccount));

Review Comment:
   Use new log format.



##########
plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java:
##########
@@ -654,4 +659,98 @@ public boolean deleteQuotaTariff(String quotaTariffUuid) {
         quotaTariff.setRemoved(_quotaService.computeAdjustedTime(new Date()));
         return _quotaTariffDao.updateQuotaTariff(quotaTariff);
     }
+
+    @Override
+    public Pair<QuotaEmailConfigurationVO, Double> 
configureQuotaEmail(QuotaConfigureEmailCmd cmd) {
+        validateQuotaConfigureEmailCmdParameters(cmd);
+
+        Double minBalance = cmd.getMinBalance();
+
+        if (minBalance != null) {
+            _quotaService.setMinBalance(cmd.getAccountId(), 
cmd.getMinBalance());
+        }
+
+        QuotaEmailConfigurationVO configurationVO = 
getQuotaEmailConfigurationVo(cmd);
+        return new Pair<>(configurationVO, minBalance);
+    }
+
+    private QuotaEmailConfigurationVO 
getQuotaEmailConfigurationVo(QuotaConfigureEmailCmd cmd) {
+        if (cmd.getTemplateName() != null) {
+            List<QuotaEmailTemplatesVO> templateVO = 
_quotaEmailTemplateDao.listAllQuotaEmailTemplates(cmd.getTemplateName());
+            if (templateVO.isEmpty()) {
+                throw new InvalidParameterValueException(String.format("Could 
not find template with name [%s].", cmd.getTemplateName()));
+            }
+            long templateId = templateVO.get(0).getId();
+            QuotaEmailConfigurationVO configurationVO = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateId(cmd.getAccountId(),
 templateId);
+
+            if (configurationVO == null) {
+                configurationVO = new 
QuotaEmailConfigurationVO(cmd.getAccountId(), templateId, cmd.getEnable());
+                
quotaEmailConfigurationDao.persistQuotaEmailConfiguration(configurationVO);
+                return configurationVO;
+            }
+
+            configurationVO.setEnabled(cmd.getEnable());
+            return 
quotaEmailConfigurationDao.updateQuotaEmailConfiguration(configurationVO);
+        }

Review Comment:
   We could invert the if to reduce indentation.
   
   



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        AccountVO account = _accountDao.findById(quotaAccount.getId());
+        if (account == null) {
+            logger.debug(String.format("Account of %s is removed, thus it will 
not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        checkBalanceAndAddToEmailList(deferredQuotaEmailList, quotaAccount, 
account, accountBalance);
+    }
+
+    private void checkBalanceAndAddToEmailList(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount, AccountVO account, 
BigDecimal accountBalance) {
+        Date balanceDate = quotaAccount.getQuotaBalanceDate();
+        Date alertDate = quotaAccount.getQuotaAlertDate();
+        int lockable = quotaAccount.getQuotaEnforce();
+        BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
+
+        logger.debug(String.format("Checking %s with accountBalance [%s], 
alertDate [%s] and lockable [%s] to see if a quota alert email should be 
sent.", account,
+                accountBalance, alertDate, lockable));
+
+        QuotaEmailConfigurationVO quotaEmpty = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_EMPTY);
+        QuotaEmailConfigurationVO quotaLow = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_LOW);
+
+        boolean shouldSendEmail = alertDate == null || 
(balanceDate.after(alertDate) && getDifferenceDays(alertDate, new Date()) > 1);
+
+        if (accountBalance.compareTo(BigDecimal.ZERO) < 0) {
+            if (_lockAccountEnforcement && lockable == 1 && 
_quotaManager.isLockable(account)) {
+                logger.info(String.format("Locking %s, as quota balance is 
lower than 0.", account));
+                lockAccount(account.getId());
+            }
+            if (quotaEmpty != null && quotaEmpty.isEnabled() && 
shouldSendEmail) {
+                logger.debug(String.format("Adding %s to the deferred emails 
list, as quota balance is lower than 0.", account));
+                deferredQuotaEmailList.add(new DeferredQuotaEmail(account, 
quotaAccount, QuotaEmailTemplateTypes.QUOTA_EMPTY));
+                return;
+            }
+        } else if (accountBalance.compareTo(thresholdBalance) < 0 && quotaLow 
!= null && quotaLow.isEnabled() && shouldSendEmail) {
+            logger.debug(String.format("Adding %s to the deferred emails list, 
as quota balance [%s] is below the threshold [%s].", account, accountBalance, 
thresholdBalance));

Review Comment:
   Use new log format.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        AccountVO account = _accountDao.findById(quotaAccount.getId());
+        if (account == null) {
+            logger.debug(String.format("Account of %s is removed, thus it will 
not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        checkBalanceAndAddToEmailList(deferredQuotaEmailList, quotaAccount, 
account, accountBalance);
+    }
+
+    private void checkBalanceAndAddToEmailList(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount, AccountVO account, 
BigDecimal accountBalance) {
+        Date balanceDate = quotaAccount.getQuotaBalanceDate();
+        Date alertDate = quotaAccount.getQuotaAlertDate();
+        int lockable = quotaAccount.getQuotaEnforce();
+        BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
+
+        logger.debug(String.format("Checking %s with accountBalance [%s], 
alertDate [%s] and lockable [%s] to see if a quota alert email should be 
sent.", account,

Review Comment:
   Use new log format.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaStatementImpl.java:
##########
@@ -107,10 +117,16 @@ public boolean stop() {
     public void sendStatement() {
 
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
+        QuotaEmailTemplatesVO templateVO = 
quotaEmailTemplatesDao.listAllQuotaEmailTemplates(QuotaConfig.QuotaEmailTemplateTypes.QUOTA_STATEMENT.toString()).get(0);
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
             if (quotaAccount.getQuotaBalance() == null) {
                 continue; // no quota usage for this account ever, ignore
             }
+            QuotaEmailConfigurationVO quotaEmailConfigurationVO = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateId(quotaAccount.getAccountId(),
 templateVO.getId());
+            if (quotaEmailConfigurationVO != null && 
!quotaEmailConfigurationVO.isEnabled()) {
+                logger.debug(String.format("%s has [%s] email disabled. 
Therefore the email will not be sent.", quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_STATEMENT));

Review Comment:
   Use new log format.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java:
##########
@@ -141,52 +146,72 @@ public boolean stop() {
     @Override
     public void checkAndSendQuotaAlertEmails() {
         List<DeferredQuotaEmail> deferredQuotaEmailList = new 
ArrayList<DeferredQuotaEmail>();
-        final BigDecimal zeroBalance = new BigDecimal(0);
+
+        logger.info("Checking and sending quota alert emails.");
         for (final QuotaAccountVO quotaAccount : 
_quotaAcc.listAllQuotaAccount()) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails accId=" + 
quotaAccount.getId());
-            }
-            BigDecimal accountBalance = quotaAccount.getQuotaBalance();
-            Date balanceDate = quotaAccount.getQuotaBalanceDate();
-            Date alertDate = quotaAccount.getQuotaAlertDate();
-            int lockable = quotaAccount.getQuotaEnforce();
-            BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
-            if (accountBalance != null) {
-                AccountVO account = _accountDao.findById(quotaAccount.getId());
-                if (account == null) {
-                    continue; // the account is removed
-                }
-                if (logger.isDebugEnabled()) {
-                    logger.debug("checkAndSendQuotaAlertEmails: Check id=" + 
account.getId() + " bal=" + accountBalance + ", alertDate=" + alertDate + ", 
lockable=" + lockable);
-                }
-                if (accountBalance.compareTo(zeroBalance) < 0) {
-                    if (_lockAccountEnforcement && (lockable == 1)) {
-                        if (_quotaManager.isLockable(account)) {
-                            logger.info("Locking account " + 
account.getAccountName() + " due to quota < 0.");
-                            lockAccount(account.getId());
-                        }
-                    }
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota < 0.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_EMPTY));
-                    }
-                } else if (accountBalance.compareTo(thresholdBalance) < 0) {
-                    if (alertDate == null || (balanceDate.after(alertDate) && 
getDifferenceDays(alertDate, new Date()) > 1)) {
-                        logger.info("Sending alert " + 
account.getAccountName() + " due to quota below threshold.");
-                        deferredQuotaEmailList.add(new 
DeferredQuotaEmail(account, quotaAccount, 
QuotaConfig.QuotaEmailTemplateTypes.QUOTA_LOW));
-                    }
-                }
-            }
+            checkQuotaAlertEmailForAccount(deferredQuotaEmailList, 
quotaAccount);
         }
 
         for (DeferredQuotaEmail emailToBeSent : deferredQuotaEmailList) {
-            if (logger.isDebugEnabled()) {
-                logger.debug("checkAndSendQuotaAlertEmails: Attempting to send 
quota alert email to users of account: " + 
emailToBeSent.getAccount().getAccountName());
-            }
+            logger.debug(String.format("Attempting to send a quota alert email 
to users of account [%s].", emailToBeSent.getAccount().getAccountName()));
             sendQuotaAlert(emailToBeSent);
         }
     }
 
+    /**
+     * Checks a given quota account to see if they should receive any emails. 
First by checking if it has any balance at all, if its account can be found, 
then checks
+     * if they should receive either QUOTA_EMPTY or QUOTA_LOW emails, taking 
into account if these email templates are disabled or not for that account.
+     * */
+    protected void checkQuotaAlertEmailForAccount(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount) {
+        logger.debug(String.format("Checking %s for email alerts.", 
quotaAccount));
+        BigDecimal accountBalance = quotaAccount.getQuotaBalance();
+
+        if (accountBalance == null) {
+            logger.debug(String.format("%s has a null balance, therefore it 
will not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        AccountVO account = _accountDao.findById(quotaAccount.getId());
+        if (account == null) {
+            logger.debug(String.format("Account of %s is removed, thus it will 
not receive quota alert emails.", quotaAccount));
+            return;
+        }
+
+        checkBalanceAndAddToEmailList(deferredQuotaEmailList, quotaAccount, 
account, accountBalance);
+    }
+
+    private void checkBalanceAndAddToEmailList(List<DeferredQuotaEmail> 
deferredQuotaEmailList, QuotaAccountVO quotaAccount, AccountVO account, 
BigDecimal accountBalance) {
+        Date balanceDate = quotaAccount.getQuotaBalanceDate();
+        Date alertDate = quotaAccount.getQuotaAlertDate();
+        int lockable = quotaAccount.getQuotaEnforce();
+        BigDecimal thresholdBalance = quotaAccount.getQuotaMinBalance();
+
+        logger.debug(String.format("Checking %s with accountBalance [%s], 
alertDate [%s] and lockable [%s] to see if a quota alert email should be 
sent.", account,
+                accountBalance, alertDate, lockable));
+
+        QuotaEmailConfigurationVO quotaEmpty = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_EMPTY);
+        QuotaEmailConfigurationVO quotaLow = 
quotaEmailConfigurationDao.findByAccountIdAndEmailTemplateType(account.getAccountId(),
 QuotaEmailTemplateTypes.QUOTA_LOW);
+
+        boolean shouldSendEmail = alertDate == null || 
(balanceDate.after(alertDate) && getDifferenceDays(alertDate, new Date()) > 1);
+
+        if (accountBalance.compareTo(BigDecimal.ZERO) < 0) {
+            if (_lockAccountEnforcement && lockable == 1 && 
_quotaManager.isLockable(account)) {
+                logger.info(String.format("Locking %s, as quota balance is 
lower than 0.", account));
+                lockAccount(account.getId());
+            }
+            if (quotaEmpty != null && quotaEmpty.isEnabled() && 
shouldSendEmail) {
+                logger.debug(String.format("Adding %s to the deferred emails 
list, as quota balance is lower than 0.", account));

Review Comment:
   Use new log format.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to