Github user jburwell commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/768#discussion_r41749582
  
    --- Diff: usage/src/org/apache/cloudstack/quota/QuotaManagerImpl.java ---
    @@ -0,0 +1,397 @@
    +//Licensed to the Apache Software Foundation (ASF) under one
    +//or more contributor license agreements.  See the NOTICE file
    +//distributed with this work for additional information
    +//regarding copyright ownership.  The ASF licenses this file
    +//to you under the Apache License, Version 2.0 (the
    +//"License"); you may not use this file except in compliance
    +//with the License.  You may obtain a copy of the License at
    +//
    +//http://www.apache.org/licenses/LICENSE-2.0
    +//
    +//Unless required by applicable law or agreed to in writing,
    +//software distributed under the License is distributed on an
    +//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +//KIND, either express or implied.  See the License for the
    +//specific language governing permissions and limitations
    +//under the License.
    +package org.apache.cloudstack.quota;
    +
    +import com.cloud.usage.UsageVO;
    +import com.cloud.usage.dao.UsageDao;
    +import com.cloud.user.AccountVO;
    +import com.cloud.user.dao.AccountDao;
    +import com.cloud.utils.Pair;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.quota.constant.QuotaTypes;
    +import org.apache.cloudstack.quota.dao.QuotaAccountDao;
    +import org.apache.cloudstack.quota.dao.QuotaBalanceDao;
    +import org.apache.cloudstack.quota.dao.QuotaTariffDao;
    +import org.apache.cloudstack.quota.dao.QuotaUsageDao;
    +import org.apache.cloudstack.quota.dao.ServiceOfferingDao;
    +import org.apache.cloudstack.quota.vo.QuotaAccountVO;
    +import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
    +import org.apache.cloudstack.quota.vo.QuotaTariffVO;
    +import org.apache.cloudstack.quota.vo.QuotaUsageVO;
    +import org.apache.cloudstack.quota.vo.ServiceOfferingVO;
    +import org.apache.cloudstack.utils.usage.UsageUtils;
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import javax.ejb.Local;
    +import javax.inject.Inject;
    +import javax.naming.ConfigurationException;
    +import java.math.BigDecimal;
    +import java.math.RoundingMode;
    +import java.util.ArrayList;
    +import java.util.Date;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.TimeZone;
    +
    +@Component
    +@Local(value = QuotaManager.class)
    +public class QuotaManagerImpl extends ManagerBase implements QuotaManager {
    +    private static final Logger s_logger = 
Logger.getLogger(QuotaManagerImpl.class.getName());
    +
    +    @Inject
    +    private AccountDao _accountDao;
    +    @Inject
    +    private QuotaAccountDao _quotaAcc;
    +    @Inject
    +    private UsageDao _usageDao;
    +    @Inject
    +    private QuotaTariffDao _quotaTariffDao;
    +    @Inject
    +    private QuotaUsageDao _quotaUsageDao;
    +    @Inject
    +    private ServiceOfferingDao _serviceOfferingDao;
    +    @Inject
    +    private QuotaBalanceDao _quotaBalanceDao;
    +    @Inject
    +    private ConfigurationDao _configDao;
    +
    +    private TimeZone _usageTimezone;
    +    private int _aggregationDuration = 0;
    +
    +    final static BigDecimal s_hoursInMonth = new BigDecimal(30 * 24);
    +    final static BigDecimal s_minutesInMonth = new BigDecimal(30 * 24 * 
60);
    +    final static BigDecimal s_gb = new BigDecimal(1024 * 1024 * 1024);
    +
    +    int _pid = 0;
    +
    +    public QuotaManagerImpl() {
    +        super();
    +    }
    +
    +    private void mergeConfigs(Map<String, String> dbParams, Map<String, 
Object> xmlParams) {
    +        for (Map.Entry<String, Object> param : xmlParams.entrySet()) {
    +            dbParams.put(param.getKey(), (String) param.getValue());
    +        }
    +    }
    +
    +    @Override
    +    public boolean configure(String name, Map<String, Object> params) 
throws ConfigurationException {
    +        super.configure(name, params);
    +
    +        Map<String, String> configs = _configDao.getConfiguration(params);
    +
    +        if (params != null) {
    +            mergeConfigs(configs, params);
    +        }
    +
    +        String aggregationRange = 
configs.get("usage.stats.job.aggregation.range");
    +        String timeZoneStr = configs.get("usage.aggregation.timezone");
    +
    +        if (timeZoneStr == null) {
    +            timeZoneStr = "GMT";
    +        }
    +        _usageTimezone = TimeZone.getTimeZone(timeZoneStr);
    +
    +        _aggregationDuration = Integer.parseInt(aggregationRange);
    +        if (_aggregationDuration < UsageUtils.USAGE_AGGREGATION_RANGE_MIN) 
{
    +            s_logger.warn("Usage stats job aggregation range is to small, 
using the minimum value of " + UsageUtils.USAGE_AGGREGATION_RANGE_MIN);
    +            _aggregationDuration = UsageUtils.USAGE_AGGREGATION_RANGE_MIN;
    +        }
    +        s_logger.info("Usage timezone = " + _usageTimezone + " 
AggregationDuration=" + _aggregationDuration);
    +
    +        return true;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        if (s_logger.isInfoEnabled()) {
    +            s_logger.info("Starting Quota Manager");
    +        }
    +        _pid = Integer.parseInt(System.getProperty("pid"));
    +        return true;
    +    }
    +
    +    @Override
    +    public boolean stop() {
    +        if (s_logger.isInfoEnabled()) {
    +            s_logger.info("Stopping Quota Manager");
    +        }
    +        return true;
    +    }
    +
    +    public List<QuotaUsageVO> aggregatePendingQuotaRecordsForAccount(final 
AccountVO account, final Pair<List<? extends UsageVO>, Integer> usageRecords) {
    +        List<QuotaUsageVO> quotaListForAccount = new ArrayList<>();
    +        if (usageRecords == null || usageRecords.first() == null || 
usageRecords.first().isEmpty()) {
    +            return quotaListForAccount;
    +        }
    +        s_logger.info("Getting pending quota records for account=" + 
account.getAccountName());
    +        for (UsageVO usageRecord: usageRecords.first()) {
    +            BigDecimal aggregationRatio = new 
BigDecimal(_aggregationDuration).divide(s_minutesInMonth, 8, 
RoundingMode.HALF_EVEN);
    +            switch (usageRecord.getUsageType()) {
    --- End diff --
    
    @bhaisaab switching on a enumerated type has a number of problems.  First, 
there is no compiler check to ensure that all enumeration keys are handled.   
Any changes to the enumeration's keys require much wider changes throughout 
code base.  In the case of addition, it is very likely that ``switch`` 
statements will be missed.  Second, it is very difficult to test these switch 
statements properly.  It requires setting up elaborate fixtures and mocking.  
Finally, these switch blocks can get very long and difficult to follow.  In 
short, the are a hard to test, leaky abstraction.
    
    When you control the enumeration implementation a better approach is to 
define an abstract method on the enumeration for the condition passing in any 
required state and implement it for every value.  Not only does this make the 
code far more readable (replace a big ``switch`` block with a one line callback 
that expresses the intent), but it far easier to write a unit test for the 
enumeration that verifies the operation of each implementation.  Most 
importantly, the compiler will check that the method is implemented for each 
value -- ensuring that changes to the structure of the enumeration can be made 
safely.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to