From 1304406fd188722dd884c49f545828e7c1546667 Mon Sep 17 00:00:00 2001 From: Abhinandan Prateek Date: Wed, 23 Dec 2015 10:12:11 +0530 Subject: [PATCH] Quota: findbug fixes Findbug fixes for cloud-framework-quota and cloud-plugin-database-quota. --- .../cloudstack/quota/QuotaStatementImpl.java | 2 +- .../quota/vo/ServiceOfferingVO.java | 89 +++++-------------- .../response/QuotaResponseBuilderImpl.java | 4 +- .../QuotaResponseBuilderImplTest.java | 2 +- 4 files changed, 25 insertions(+), 72 deletions(-) diff --git a/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java b/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java index 682b2ef0366..0f957be5d71 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java +++ b/framework/quota/src/org/apache/cloudstack/quota/QuotaStatementImpl.java @@ -85,7 +85,7 @@ public class QuotaStatementImpl extends ManagerBase implements QuotaStatement { mergeConfigs(configs, params); } String period_str = configs.get(QuotaConfig.QuotaStatementPeriod.key()); - int period = period_str == null ? 1 : Integer.valueOf(period_str); + int period = period_str == null ? 1 : Integer.parseInt(period_str); STATEMENT_PERIODS _period = STATEMENT_PERIODS.values()[period]; return true; diff --git a/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java b/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java index 2d11edda779..795a1782383 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java +++ b/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java @@ -16,6 +16,7 @@ //under the License. package org.apache.cloudstack.quota.vo; +import java.util.HashMap; import java.util.Map; import javax.persistence.Column; @@ -74,14 +75,9 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering @Column(name = "deployment_planner") private String deploymentPlanner = null; - // This is a delayed load value. If the value is null, - // then this field has not been loaded yet. - // Call service offering dao to load it. @Transient - Map details; + Map details = new HashMap(); - // This flag is required to tell if the offering is dynamic once the cpu, memory and speed are set. - // In some cases cpu, memory and speed are set to non-null values even if the offering is dynamic. @Transient boolean isDynamic; @@ -90,7 +86,7 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering } public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, String displayText, - ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, boolean defaultUse) { + ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, boolean defaultUse) { super(name, displayText, provisioningType, false, tags, recreatable, useLocalStorage, systemUse, true); this.cpu = cpu; this.ramSize = ramSize; @@ -105,7 +101,8 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering } public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitCpuUse, - boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, Long domainId) { + boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, + VirtualMachine.Type vmType, Long domainId) { super(name, displayText, provisioningType, false, tags, recreatable, useLocalStorage, systemUse, true, domainId); this.cpu = cpu; this.ramSize = ramSize; @@ -118,68 +115,26 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering this.vmType = vmType == null ? null : vmType.toString().toLowerCase(); } - public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, - boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, - VirtualMachine.Type vmType, Long domainId, String hostTag) { - this(name, - cpu, - ramSize, - speed, - rateMbps, - multicastRateMbps, - offerHA, - limitResourceUse, - volatileVm, - displayText, - provisioningType, - useLocalStorage, - recreatable, - tags, - systemUse, - vmType, - domainId); + public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitResourceUse, + boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, + VirtualMachine.Type vmType, Long domainId, String hostTag) { + this(name, cpu, ramSize, speed, rateMbps, multicastRateMbps, offerHA, limitResourceUse, volatileVm, displayText, provisioningType, useLocalStorage, recreatable, tags, + systemUse, vmType, domainId); this.hostTag = hostTag; } - public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, - boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, - VirtualMachine.Type vmType, Long domainId, String hostTag, String deploymentPlanner) { - this(name, - cpu, - ramSize, - speed, - rateMbps, - multicastRateMbps, - offerHA, - limitResourceUse, - volatileVm, - displayText, - provisioningType, - useLocalStorage, - recreatable, - tags, - systemUse, - vmType, - domainId, - hostTag); + public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitResourceUse, + boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, + VirtualMachine.Type vmType, Long domainId, String hostTag, String deploymentPlanner) { + this(name, cpu, ramSize, speed, rateMbps, multicastRateMbps, offerHA, limitResourceUse, volatileVm, displayText, provisioningType, useLocalStorage, recreatable, tags, + systemUse, vmType, domainId, hostTag); this.deploymentPlanner = deploymentPlanner; } public ServiceOfferingVO(ServiceOfferingVO offering) { - super(offering.getId(), - offering.getName(), - offering.getDisplayText(), - offering.getProvisioningType(), - false, - offering.getTags(), - offering.isRecreatable(), - offering.getUseLocalStorage(), - offering.getSystemUse(), - true, - offering.isCustomizedIops()== null ? false:offering.isCustomizedIops(), - offering.getDomainId(), - offering.getMinIops(), - offering.getMaxIops()); + super(offering.getId(), offering.getName(), offering.getDisplayText(), offering.getProvisioningType(), false, offering.getTags(), offering.isRecreatable(), + offering.getUseLocalStorage(), offering.getSystemUse(), true, offering.isCustomizedIops() == null ? false : offering.isCustomizedIops(), offering.getDomainId(), + offering.getMinIops(), offering.getMaxIops()); cpu = offering.getCpu(); ramSize = offering.getRamSize(); speed = offering.getSpeed(); @@ -310,14 +265,10 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering } public String getDetail(String name) { - assert (details != null) : "Did you forget to load the details?"; - - return details != null ? details.get(name) : null; + return details.get(name); } - public void setDetail(String name, String value) { - assert (details != null) : "Did you forget to load the details?"; - + public void addDetail(String name, String value) { details.put(name, value); } 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 1b6f400f407..746c7c32935 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 @@ -174,10 +174,12 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder { @Override public QuotaBalanceResponse createQuotaBalanceResponse(List quotaBalance, Date startDate, Date endDate) { if (quotaBalance == null || quotaBalance.isEmpty()) { - new InvalidParameterValueException("The request period does not contain balance entries."); + throw new InvalidParameterValueException("The request period does not contain balance entries."); } Collections.sort(quotaBalance, new Comparator() { public int compare(QuotaBalanceVO o1, QuotaBalanceVO o2) { + o1 = o1 == null ? new QuotaBalanceVO() : o1; + o2 = o2 == null ? new QuotaBalanceVO() : o2; return o2.getUpdatedOn().compareTo(o1.getUpdatedOn()); // desc } }); diff --git a/plugins/database/quota/test/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java b/plugins/database/quota/test/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java index 73ead4a71bf..c113e551647 100644 --- a/plugins/database/quota/test/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java +++ b/plugins/database/quota/test/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java @@ -118,7 +118,7 @@ public class QuotaResponseBuilderImplTest extends TestCase { tariffVO.setUsageType(QuotaTypes.IP_ADDRESS); tariffVO.setUsageName("ip address"); tariffVO.setUsageUnit("IP-Month"); - tariffVO.setCurrencyValue(new BigDecimal(100.19)); + tariffVO.setCurrencyValue(BigDecimal.valueOf(100.19)); tariffVO.setEffectiveOn(new Date()); tariffVO.setUsageDiscriminator(""); return tariffVO;