From 728afba5d4879ece2d5457f34c96aad3cc5a3297 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 2 Apr 2019 13:22:30 +0530 Subject: [PATCH] refactorings Signed-off-by: Rohit Yadav --- .../java/com/cloud/offering/DiskOffering.java | 10 ++-- .../apache/cloudstack/api/ApiConstants.java | 3 +- .../admin/offering/CreateDiskOfferingCmd.java | 12 ++--- .../api/response/DiskOfferingResponse.java | 11 +++-- .../com/cloud/storage/DiskOfferingVO.java | 1 + .../main/java/com/cloud/api/ApiDBUtils.java | 49 +++++++------------ .../ConfigurationManagerImpl.java | 4 -- 7 files changed, 36 insertions(+), 54 deletions(-) diff --git a/api/src/main/java/com/cloud/offering/DiskOffering.java b/api/src/main/java/com/cloud/offering/DiskOffering.java index 98ba6c0f46d..bb6b8d55fe6 100644 --- a/api/src/main/java/com/cloud/offering/DiskOffering.java +++ b/api/src/main/java/com/cloud/offering/DiskOffering.java @@ -34,13 +34,13 @@ public interface DiskOffering extends InfrastructureEntity, Identity, InternalId Inactive, Active, } - public enum Type { + enum Type { Disk, Service }; State getState(); - public enum DiskCacheMode { + enum DiskCacheMode { NONE("none"), WRITEBACK("writeback"), WRITETHROUGH("writethrough"); private final String _diskCacheMode; @@ -67,11 +67,11 @@ public interface DiskOffering extends InfrastructureEntity, Identity, InternalId String getDisplayText(); - public ProvisioningType getProvisioningType(); + ProvisioningType getProvisioningType(); - public String getTags(); + String getTags(); - public String[] getTagsArray(); + String[] getTagsArray(); Date getCreated(); diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index e09abb9c408..c75d52c06d8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -114,7 +114,6 @@ public class ApiConstants { public static final String DOMAIN_ID = "domainid"; public static final String DOMAIN__ID = "domainId"; public static final String DOMAIN_ID_LIST = "domainids"; - public static final String DOMAIN_NAME_LIST = "domainnames"; public static final String DURATION = "duration"; public static final String ELIGIBLE = "eligible"; public static final String EMAIL = "email"; @@ -352,6 +351,7 @@ public class ApiConstants { public static final String VNET = "vnet"; public static final String IS_VOLATILE = "isvolatile"; public static final String VOLUME_ID = "volumeid"; + public static final String ZONE = "zone"; public static final String ZONE_ID = "zoneid"; public static final String ZONE_NAME = "zonename"; public static final String NETWORK_TYPE = "networktype"; @@ -713,7 +713,6 @@ public class ApiConstants { public static final String NETSCALER_SERVICEPACKAGE_ID = "netscalerservicepackageid"; public static final String ZONE_ID_LIST = "zoneids"; - public static final String ZONE_NAME_LIST = "zonenames"; public static final String DESTINATION_ZONE_ID_LIST = "destzoneids"; public static final String ADMIN = "admin"; public static final String CHECKSUM_PARAMETER_PREFIX_DESCRIPTION = "The parameter containing the checksum will be considered a MD5sum if it is not prefixed\n" diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java index 3976a7584ea..45aaa423b5e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java @@ -71,7 +71,6 @@ public class CreateDiskOfferingCmd extends BaseCmd { type = CommandType.LIST, collectionType = CommandType.UUID, entityType = DomainResponse.class, - required = false, description = "the ID of the domains offering is associated with, null for all domain offerings", since = "4.13") private List domainIds; @@ -80,7 +79,6 @@ public class CreateDiskOfferingCmd extends BaseCmd { type = CommandType.LIST, collectionType = CommandType.UUID, entityType = ZoneResponse.class, - required = false, description = "the ID of the zones offering is associated with, null for all zone offerings", since = "4.13") private List zoneIds; @@ -187,15 +185,11 @@ public class CreateDiskOfferingCmd extends BaseCmd { return maxIops; } - public Long getDomainId() { - return domainId; - } - public List getDomainIds() { + if (domainIds == null) { + domainIds = new ArrayList<>(); + } if (domainId != null) { - if (domainIds == null) { - domainIds = new ArrayList<>(); - } domainIds.add(domainId); } return domainIds; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java index 2f86629043d..d5dc44fd76c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.api.response; import java.util.Date; +import java.util.HashMap; import java.util.Map; import org.apache.cloudstack.api.ApiConstants; @@ -146,7 +147,7 @@ public class DiskOfferingResponse extends BaseResponse { @SerializedName(ApiConstants.DETAILS) @Param(description = "the details of the disk offering", since = "4.13.0") - private Map details; + private Map details = new HashMap<>(); public Boolean getDisplayOffering() { return displayOffering; @@ -333,11 +334,15 @@ public class DiskOfferingResponse extends BaseResponse { this.iopsWriteRateMaxLength = iopsWriteRateMaxLength; } - public Map getDetails() { + public Map getDetails() { return details; } - public void setDetails(Map details) { + public void putDetail(String key, Object value) { + this.details.put(key, value); + } + + public void setDetails(Map details) { this.details = details; } } diff --git a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java index 2293553f22e..5a5dcd4863d 100644 --- a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java @@ -50,6 +50,7 @@ public class DiskOfferingVO implements DiskOffering { @Column(name = "id") long id; + // TODO: remove me @Column(name = "domain_id") Long domainId = null; diff --git a/server/src/main/java/com/cloud/api/ApiDBUtils.java b/server/src/main/java/com/cloud/api/ApiDBUtils.java index 89958673bd8..dbcb5ab84a9 100644 --- a/server/src/main/java/com/cloud/api/ApiDBUtils.java +++ b/server/src/main/java/com/cloud/api/ApiDBUtils.java @@ -72,6 +72,7 @@ import org.apache.cloudstack.framework.jobs.dao.AsyncJobDao; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.commons.collections.MapUtils; import com.cloud.agent.api.VgpuTypesInfo; import com.cloud.api.query.dao.AccountJoinDao; @@ -187,6 +188,7 @@ import com.cloud.network.dao.AccountGuestVlanMapDao; import com.cloud.network.dao.AccountGuestVlanMapVO; import com.cloud.network.dao.FirewallRulesCidrsDao; import com.cloud.network.dao.FirewallRulesDao; +import com.cloud.network.dao.FirewallRulesDcidrsDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerDao; @@ -279,6 +281,7 @@ import com.cloud.template.TemplateManager; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountDetailsDao; +import com.cloud.user.AccountManager; import com.cloud.user.AccountService; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; @@ -314,9 +317,6 @@ import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.dao.VMSnapshotDao; -import com.cloud.user.AccountManager; -import com.cloud.network.dao.FirewallRulesDcidrsDao; -import com.google.common.base.Strings; public class ApiDBUtils { private static ManagementServer s_ms; @@ -1914,36 +1914,23 @@ public class ApiDBUtils { public static DiskOfferingResponse newDiskOfferingResponse(DiskOfferingJoinVO offering) { DiskOfferingResponse diskOfferingResponse = s_diskOfferingJoinDao.newDiskOfferingResponse(offering); - if(diskOfferingResponse!=null) { - Map details = s_diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); - if (details != null && !details.isEmpty()) { - if(details.containsKey(ApiConstants.DOMAIN_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { - String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); - List domains = s_domainDao.list(domainIdsArray); - List domainIdsList = new ArrayList<>(); - List domainNamesList = new ArrayList<>(); - for (DomainVO domain : domains) { - domainIdsList.add(domain.getUuid()); - domainNamesList.add(domain.getName()); - } - details.put(ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsList)); - details.put(ApiConstants.DOMAIN_NAME_LIST, String.join(", ", domainNamesList)); + if (diskOfferingResponse != null) { + Map details = s_diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId(), false); + if (MapUtils.isNotEmpty(details)) { + // Domains + String[] domainIds = details.getOrDefault(ApiConstants.DOMAIN_ID_LIST, "").split(","); + final Map domains = new HashMap<>(); + for (DomainVO domain : s_domainDao.list(domainIds)) { + domains.put(domain.getName(), domain.getUuid()); } - if(details.containsKey(ApiConstants.ZONE_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { - String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); - List zones = s_zoneDao.list(zoneIdsArray); - List zoneIdsList = new ArrayList<>(); - List zoneNamesList = new ArrayList<>(); - for (DataCenterVO zone : zones) { - zoneIdsList.add(zone.getUuid()); - zoneNamesList.add(zone.getName()); - } - details.put(ApiConstants.ZONE_ID_LIST, String.join(",", zoneIdsList)); - details.put(ApiConstants.ZONE_NAME_LIST, String.join(", ", zoneNamesList)); + diskOfferingResponse.putDetail(ApiConstants.DOMAIN, domains); + // Zones + String[] zoneIds = details.getOrDefault(ApiConstants.ZONE_ID_LIST, "").split(","); + final Map zones = new HashMap<>(); + for (DataCenterVO zone : s_zoneDao.list(zoneIds)) { + domains.put(zone.getName(), zone.getUuid()); } - diskOfferingResponse.setDetails(details); + diskOfferingResponse.putDetail(ApiConstants.ZONE, zones); } } return diskOfferingResponse; diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index c4be75b17d5..5d6e894143c 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2768,12 +2768,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati final Long numGibibytes = cmd.getDiskSize(); final boolean isDisplayOfferingEnabled = cmd.getDisplayOffering() != null ? cmd.getDisplayOffering() : true; final boolean isCustomized = cmd.isCustomized() != null ? cmd.isCustomized() : false; // false - // by - // default final String tags = cmd.getTags(); - final List domainIds = cmd.getDomainIds(); - final List zoneIds = cmd.getZoneIds(); if (!isCustomized && numGibibytes == null) {