Refactored the code, regarding the code change suggetions by GutoVero…

…nezi.
apache · Jul 15, 2021 · 2ff1691 · 2ff1691
1 parent 317ecbb
commit 2ff1691
Show file tree

Hide file tree

Showing 13 changed files with 108 additions and 157 deletions.
diff --git a/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java b/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java
@@ -36,6 +36,7 @@
 import com.cloud.utils.Pair;
 
 import org.apache.cloudstack.network.lb.LoadBalancerConfig;
+import org.springframework.util.CollectionUtils;
 
 public class LoadBalancerTO {
     String uuid;
@@ -187,7 +188,7 @@ public LoadBalancerConfigTO[] getLbConfigs() {
     }
 
     public void setLbConfigs(List<? extends LoadBalancerConfig> lbConfigs) {
-        if (lbConfigs == null || lbConfigs.size() == 0) {
+        if (CollectionUtils.isEmpty(lbConfigs)) {
             this.lbConfigs = new LoadBalancerConfigTO[0];
             return;
         }

diff --git a/...java/org/apache/cloudstack/api/command/user/loadbalancer/AssignCertToLoadBalancerCmd.java b/...java/org/apache/cloudstack/api/command/user/loadbalancer/AssignCertToLoadBalancerCmd.java
@@ -17,6 +17,7 @@
 // under the License.
 package org.apache.cloudstack.api.command.user.loadbalancer;
 
+import org.apache.commons.lang3.BooleanUtils;
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.api.APICommand;
@@ -112,7 +113,7 @@ public Long getLbRuleId() {
     }
 
     public boolean isForced() {
-        return (forced != null) ? forced : false;
+        return BooleanUtils.toBoolean(forced);
     }
 
     @Override
@@ -133,9 +134,6 @@ public String getSyncObjType() {
     @Override
     public Long getSyncObjId() {
         LoadBalancer lb = _entityMgr.findById(LoadBalancer.class, getLbRuleId());
-        if (lb == null) {
-            return null;
-        }
-        return lb.getNetworkId();
+        return (lb != null )? lb.getNetworkId(): null;
     }
 }
diff --git a/...java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerConfigCmd.java b/...java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerConfigCmd.java
@@ -20,7 +20,6 @@
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
-import org.apache.cloudstack.api.BaseAsyncCmd;
 import org.apache.cloudstack.api.BaseAsyncCreateCmd;
 import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
@@ -30,6 +29,7 @@
 import org.apache.cloudstack.api.response.VpcResponse;
 import org.apache.cloudstack.network.lb.LoadBalancerConfig;
 import org.apache.cloudstack.network.lb.LoadBalancerConfig.Scope;
+import org.apache.commons.lang3.BooleanUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.event.EventTypes;
@@ -123,7 +123,7 @@ public String getValue() {
     }
 
     public boolean isForced() {
-        return (forced != null) ? forced : false;
+        return BooleanUtils.toBoolean(forced);
     }
 
     /////////////////////////////////////////////////////
@@ -138,9 +138,8 @@ public String getCommandName() {
     @Override
     public void execute() {
 
-        LoadBalancerConfig config = null;
         try {
-            config = _entityMgr.findById(LoadBalancerConfig.class, getEntityId());
+            LoadBalancerConfig config = _entityMgr.findById(LoadBalancerConfig.class, getEntityId());
             LoadBalancerConfigResponse lbConfigResponse = new LoadBalancerConfigResponse();
             if (config != null) {
                 lbConfigResponse = _responseGenerator.createLoadBalancerConfigResponse(config);
@@ -165,41 +164,22 @@ public void create() {
 
     @Override
     public String getSyncObjType() {
-        if (networkId != null) {
-            return BaseAsyncCmd.networkSyncObject;
-        } else if (vpcId != null) {
-            return BaseAsyncCmd.vpcSyncObject;
-        }
-        return null;
+        return LoadBalancerHelper.getSyncObjType(networkId, vpcId);
     }
 
     @Override
     public Long getSyncObjId() {
-        if (networkId != null) {
-            return getNetworkId();
-        } else if (vpcId != null) {
-            return getVpcId();
-        }
-        return null;
+        return LoadBalancerHelper.getSyncObjId(networkId, vpcId);
     }
 
     @Override
     public long getEntityOwnerId() {
         if (Scope.Network.name().equalsIgnoreCase(scope) && networkId != null) {
-            Network network = _entityMgr.findById(Network.class, networkId);
-            if (network != null) {
-                return network.getAccountId();
-            }
+            return LoadBalancerHelper.getEntityOwnerId(_entityMgr, Network.class, networkId);
         } else if (Scope.Vpc.name().equalsIgnoreCase(scope) && vpcId != null) {
-            Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
-            if (vpc != null) {
-                return vpc.getAccountId();
-            }
+            return LoadBalancerHelper.getEntityOwnerId(_entityMgr, Vpc.class, vpcId);
         } else if (Scope.LoadBalancerRule.name().equalsIgnoreCase(scope) && loadBalancerId != null) {
-            LoadBalancer lb = _entityMgr.findById(LoadBalancer.class, loadBalancerId);
-            if (lb != null) {
-                return lb.getAccountId();
-            }
+            return LoadBalancerHelper.getEntityOwnerId(_entityMgr, LoadBalancer.class, loadBalancerId);
         }
         throw new InvalidParameterValueException("Unable to find the entity owner");
     }

diff --git a/...n/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java b/...n/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
@@ -33,6 +33,7 @@
 import org.apache.cloudstack.api.response.NetworkResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.cloudstack.context.CallContext;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.dc.DataCenter;
@@ -255,7 +256,7 @@ public List<String> getSourceCidrList() {
     }
 
     public String getLbProtocol() {
-        return lbProtocol != null ? lbProtocol.toLowerCase().trim() : null;
+        return StringUtils.trim(StringUtils.lowerCase(lbProtocol));
     }
 
     /////////////////////////////////////////////////////

diff --git a/...java/org/apache/cloudstack/api/command/user/loadbalancer/DeleteLoadBalancerConfigCmd.java b/...java/org/apache/cloudstack/api/command/user/loadbalancer/DeleteLoadBalancerConfigCmd.java
@@ -84,57 +84,19 @@ public void execute() {
 
     @Override
     public String getSyncObjType() {
-        LoadBalancerConfig config = _entityMgr.findById(LoadBalancerConfig.class, getId());
-        if (config == null) {
-            throw new InvalidParameterValueException("Unable to find load balancer config: " + id);
-        }
-        if (config.getNetworkId() != null) {
-            return BaseAsyncCmd.networkSyncObject;
-        } else if (config.getVpcId() != null) {
-            return BaseAsyncCmd.vpcSyncObject;
-        }
-        return null;
+        return LoadBalancerHelper.getSyncObjType(_entityMgr, getId());
     }
 
     @Override
     public Long getSyncObjId() {
-        LoadBalancerConfig config = _entityMgr.findById(LoadBalancerConfig.class, getId());
-        if (config == null) {
-            throw new InvalidParameterValueException("Unable to find load balancer config: " + id);
-        }
-        if (config.getNetworkId() != null) {
-            return config.getNetworkId();
-        } else if (config.getVpcId() != null) {
-            return config.getVpcId();
-        }
-        return null;
+        return LoadBalancerHelper.getSyncObjId(_entityMgr, getId());
     }
 
     @Override
     public long getEntityOwnerId() {
-        LoadBalancerConfig config = _entityMgr.findById(LoadBalancerConfig.class, getId());
-        if (config != null) {
-            if (config.getNetworkId() != null) {
-                Network network = _entityMgr.findById(Network.class, config.getNetworkId());
-                if (network != null) {
-                    return network.getAccountId();
-                }
-            } else if (config.getVpcId() != null) {
-                Vpc vpc = _entityMgr.findById(Vpc.class, config.getVpcId());
-                if (vpc != null) {
-                    return vpc.getAccountId();
-                }
-            } else if (config.getLoadBalancerId() != null) {
-                FirewallRule rule = _entityMgr.findById(FirewallRule.class, config.getLoadBalancerId());
-                if (rule != null) {
-                    return rule.getAccountId();
-                }
-            }
-        }
-        throw new InvalidParameterValueException("Unable to find the entity owner");
+        return LoadBalancerHelper.getEntityOwnerId(_entityMgr, getId());
     }
 
-
     @Override
     public String getEventType() {
         return EventTypes.EVENT_LOAD_BALANCER_CONFIG_DELETE;

diff --git a/.../java/org/apache/cloudstack/api/command/user/loadbalancer/ListLoadBalancerConfigsCmd.java b/.../java/org/apache/cloudstack/api/command/user/loadbalancer/ListLoadBalancerConfigsCmd.java
@@ -30,6 +30,7 @@
 import org.apache.cloudstack.api.response.NetworkResponse;
 import org.apache.cloudstack.api.response.VpcResponse;
 import org.apache.cloudstack.network.lb.LoadBalancerConfig;
+import org.apache.commons.lang3.BooleanUtils;
 import org.apache.log4j.Logger;
 
 @APICommand(name = "listLoadBalancerConfigs", description = "Lists load balancer configs.",
@@ -115,7 +116,7 @@ public String getName() {
     }
 
     public boolean listAll() {
-        return listAll == null ? false : listAll;
+        return BooleanUtils.toBoolean(listAll);
     }
 
     // ///////////////////////////////////////////////////

diff --git a/...src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/LoadBalancerHelper.java b/...src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/LoadBalancerHelper.java
@@ -0,0 +1,68 @@
+package org.apache.cloudstack.api.command.user.loadbalancer;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.network.Network;
+import com.cloud.network.rules.FirewallRule;
+import com.cloud.network.vpc.Vpc;
+import com.cloud.user.OwnedBy;
+import com.cloud.utils.db.EntityManager;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.network.lb.LoadBalancerConfig;
+
+
+public interface LoadBalancerHelper {
+
+    static LoadBalancerConfig findConfigById(EntityManager entityMgr, Long id){
+        LoadBalancerConfig config = entityMgr.findById(LoadBalancerConfig.class, id);
+        if (config == null) {
+            throw new InvalidParameterValueException("Unable to find load balancer config: " + id);
+        }
+        return config;
+    }
+
+    static String getSyncObjType(EntityManager entityMgr, Long id){
+        LoadBalancerConfig config = findConfigById(entityMgr, id);
+        return getSyncObjType(config.getNetworkId(), config.getVpcId());
+    }
+
+    static String getSyncObjType(Long networkId, Long vpcId){
+        if (networkId != null) {
+            return BaseAsyncCmd.networkSyncObject;
+        } else if (vpcId != null) {
+            return BaseAsyncCmd.vpcSyncObject;
+        }
+        return null;
+    }
+
+    static Long getSyncObjId(EntityManager entityMgr, Long id){
+        LoadBalancerConfig config = findConfigById(entityMgr, id);
+        return getSyncObjId(config.getNetworkId(), config.getVpcId());
+    }
+
+    // Cause vpcId might be null, this method might return null
+    static Long getSyncObjId(Long networkId, Long vpcId){
+        if (networkId != null)
+            return networkId;
+        return vpcId;
+    }
+
+    static <T extends OwnedBy> long getEntityOwnerId(EntityManager entityMgr , Class<T> entityType, Long id){
+        T t = entityMgr.findById(entityType, id);
+        if (t != null) {
+            return t.getAccountId();
+        }
+        throw new InvalidParameterValueException("Unable to find the entity owner");
+    }
+
+    static long getEntityOwnerId(EntityManager entityMgr, Long id) {
+        LoadBalancerConfig config = findConfigById(entityMgr, id);
+        if (config.getNetworkId() != null) {
+            return LoadBalancerHelper.getEntityOwnerId(entityMgr, Network.class, config.getNetworkId());
+        } else if (config.getVpcId() != null) {
+            return LoadBalancerHelper.getEntityOwnerId(entityMgr, Vpc.class, config.getVpcId());
+        } else if (config.getLoadBalancerId() != null) {
+            return LoadBalancerHelper.getEntityOwnerId(entityMgr, FirewallRule.class, config.getLoadBalancerId());
+        }
+        throw new InvalidParameterValueException("Unable to find the entity owner");
+    }
+}
diff --git a/...va/org/apache/cloudstack/api/command/user/loadbalancer/RemoveCertFromLoadBalancerCmd.java b/...va/org/apache/cloudstack/api/command/user/loadbalancer/RemoveCertFromLoadBalancerCmd.java
@@ -110,9 +110,6 @@ public String getSyncObjType() {
     @Override
     public Long getSyncObjId() {
         LoadBalancer lb = _entityMgr.findById(LoadBalancer.class, getLbRuleId());
-        if (lb == null) {
-            return null;
-        }
-        return lb.getNetworkId();
+        return (lb != null )? lb.getNetworkId(): null;
     }
 }
diff --git a/...va/org/apache/cloudstack/api/command/user/loadbalancer/ReplaceLoadBalancerConfigsCmd.java b/...va/org/apache/cloudstack/api/command/user/loadbalancer/ReplaceLoadBalancerConfigsCmd.java
@@ -30,10 +30,10 @@
 import org.apache.cloudstack.api.response.SuccessResponse;
 import org.apache.cloudstack.api.response.VpcResponse;
 import org.apache.cloudstack.network.lb.LoadBalancerConfig;
+import org.apache.commons.collections.MapUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.event.EventTypes;
-import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.network.Network;
 import com.cloud.network.vpc.Vpc;
 
@@ -101,7 +101,7 @@ public Long getLoadBalancerId() {
     }
 
     public Map<String, String> getConfigList() {
-        if (configList == null || configList.isEmpty()) {
+        if (MapUtils.isEmpty(configList)) {
             return null;
         }
 
@@ -127,38 +127,20 @@ public void execute() {
 
     @Override
     public String getSyncObjType() {
-        if (networkId != null) {
-            return BaseAsyncCmd.networkSyncObject;
-        } else if (vpcId != null) {
-            return BaseAsyncCmd.vpcSyncObject;
-        }
-        return null;
+        return LoadBalancerHelper.getSyncObjType(networkId, vpcId);
     }
 
     @Override
     public Long getSyncObjId() {
-        if (networkId != null) {
-            return networkId;
-        } else if (vpcId != null) {
-            return vpcId;
-        }
-        return null;
+        return LoadBalancerHelper.getSyncObjId(networkId, vpcId);
     }
 
     @Override
     public long getEntityOwnerId() {
         if (networkId != null) {
-            Network network = _entityMgr.findById(Network.class, networkId);
-            if (network != null) {
-                return network.getAccountId();
-            }
-        } else if (vpcId != null) {
-            Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
-            if (vpc != null) {
-                return vpc.getAccountId();
-            }
+            return LoadBalancerHelper.getEntityOwnerId(_entityMgr, Network.class, networkId);
         }
-        throw new InvalidParameterValueException("Unable to find the entity owner");
+        return LoadBalancerHelper.getEntityOwnerId(_entityMgr, Vpc.class, vpcId);
     }
 
     @Override