address code smells - part 1

Pearl1594 · Pearl1594 · commit 6f382e7c6dff · 2023-09-07T09:42:31.000-04:00
diff --git a/api/src/main/java/com/cloud/network/Network.java b/api/src/main/java/com/cloud/network/Network.java
@@ -205,7 +205,6 @@ public static class Provider {
         //Add Tungsten Fabric provider
         public static final Provider Tungsten = new Provider("Tungsten", false);
 
-        //TODO: should it be true?
         public static final Provider Nsx = new Provider("Nsx", false);
 
         private final String name;
diff --git a/api/src/main/java/com/cloud/network/Networks.java b/api/src/main/java/com/cloud/network/Networks.java
@@ -343,7 +343,6 @@ public <T> URI toUri(T value) {
             }
         },
         Vswitch("vs", String.class), Undecided(null, null), Vnet("vnet", Long.class);
-//        NSX("nsx", String.class);
 
         private final String scheme;
         private final Class<?> type;
diff --git a/api/src/main/java/com/cloud/network/vpc/VpcOffering.java b/api/src/main/java/com/cloud/network/vpc/VpcOffering.java
@@ -29,7 +29,7 @@ public enum State {
     public static final String defaultVPCOfferingName = "Default VPC offering";
     public static final String defaultVPCNSOfferingName = "Default VPC  offering with Netscaler";
     public static final String redundantVPCOfferingName = "Redundant VPC offering";
-    public static final String defaultVPCNSXSOfferingName = "VPC offering with NSX";
+    public static final String DEFAULT_VPC_NSX_OFFERING_NAME = "VPC offering with NSX";
 
     /**
      *
diff --git a/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java b/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java
@@ -25,6 +25,7 @@
 import javax.persistence.GenerationType;
 import javax.persistence.Id;
 import javax.persistence.Table;
+import java.util.List;
 import java.util.UUID;
 
 @Entity
@@ -71,18 +72,6 @@ public NsxProviderVO() {
         this.uuid = UUID.randomUUID().toString();
     }
 
-    public NsxProviderVO( long zoneId, long hostId, String providerName, String hostname, String username, String password, String tier0Gateway, String edgeCluster) {
-        this.zoneId = zoneId;
-        this.hostId = hostId;
-        this.uuid = UUID.randomUUID().toString();
-        this.providerName = providerName;
-        this.hostname = hostname;
-        this.username = username;
-        this.password = password;
-        this.tier0Gateway = tier0Gateway;
-        this.edgeCluster = edgeCluster;
-    }
-
     @Override
     public long getId() {
         return id;
@@ -177,4 +166,78 @@ public String getEdgeCluster() {
     public void setEdgeCluster(String edgeCluster) {
         this.edgeCluster = edgeCluster;
     }
+
+    public static final class Builder {
+        private long zoneId;
+        private long hostId;
+        private String providerName;
+        private String hostname;
+        private String port;
+        private String username;
+        private String password;
+        private String tier0Gateway;
+        private String edgeCluster;
+
+        public Builder() {
+        }
+
+        public Builder setZoneId(long zoneId) {
+            this.zoneId = zoneId;
+            return this;
+        }
+
+        public Builder setHostId(long hostId) {
+            this.hostId = hostId;
+            return this;
+        }
+
+        public Builder setProviderName(String providerName) {
+            this.providerName = providerName;
+            return this;
+        }
+
+        public Builder setHostname(String hostname) {
+            this.hostname = hostname;
+            return this;
+        }
+
+        public Builder setPort(String port) {
+            this.port = port;
+            return this;
+        }
+
+        public Builder setUsername(String username) {
+            this.username = username;
+            return this;
+        }
+
+        public Builder setPassword(String password) {
+            this.password = password;
+            return this;
+        }
+
+        public Builder setTier0Gateway(String tier0Gateway) {
+            this.tier0Gateway = tier0Gateway;
+            return this;
+        }
+
+        public Builder setEdgeCluster(String edgeCluster) {
+            this.edgeCluster = edgeCluster;
+            return this;
+        }
+        public NsxProviderVO build() {
+            NsxProviderVO provider = new NsxProviderVO();
+            provider.setZoneId(this.zoneId);
+            provider.setHostId(this.hostId);
+            provider.setUuid(UUID.randomUUID().toString());
+            provider.setProviderName(this.providerName);
+            provider.setHostname(this.hostname);
+            provider.setPort(this.port);
+            provider.setUsername(this.username);
+            provider.setPassword(this.password);
+            provider.setTier0Gateway(this.tier0Gateway);
+            provider.setEdgeCluster(this.edgeCluster);
+            return provider;
+        }
+    }
 }
diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql
@@ -193,6 +193,8 @@ CREATE TABLE `cloud`.`nsx_providers` (
     `password` varchar(255) NOT NULL,
     `tier0_gateway` varchar(255),
     `edge_cluster` varchar(255),
+    `created` datetime NOT NULL COMMENT 'date created',
+    `removed` datetime COMMENT 'date removed if not null',
     PRIMARY KEY (`id`),
     CONSTRAINT `fk_nsx_providers__zone_id` FOREIGN KEY `fk_nsx_providers__zone_id` (`zone_id`) REFERENCES `data_center`(`id`) ON DELETE CASCADE,
     INDEX `i_nsx_providers__zone_id`(`zone_id`)
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java
@@ -18,6 +18,8 @@
 
 import com.cloud.network.dao.NetworkVO;
 
+import java.util.Objects;
+
 public class CreateNsxSegmentCommand extends CreateNsxTier1GatewayCommand {
     private NetworkVO tierNetwork;
     public CreateNsxSegmentCommand(String zoneName, Long zoneId, String accountName, Long accountId, String vpcName, NetworkVO tierNetwork) {
@@ -32,4 +34,18 @@ public NetworkVO getTierNetwork() {
     public void setTierNetwork(NetworkVO tierNetwork) {
         this.tierNetwork = tierNetwork;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateNsxSegmentCommand command = (CreateNsxSegmentCommand) o;
+        return Objects.equals(tierNetwork, command.tierNetwork);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), tierNetwork);
+    }
 }
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxTier1GatewayCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxTier1GatewayCommand.java
@@ -16,6 +16,8 @@
 // under the License.
 package org.apache.cloudstack.agent.api;
 
+import java.util.Objects;
+
 public class CreateNsxTier1GatewayCommand extends NsxCommand {
     private String vpcName;
 
@@ -31,4 +33,18 @@ public String getVpcName() {
     public void setVpcName(String vpcName) {
         this.vpcName = vpcName;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        CreateNsxTier1GatewayCommand that = (CreateNsxTier1GatewayCommand) o;
+        return Objects.equals(vpcName, that.vpcName);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), vpcName);
+    }
 }
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/NsxCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/NsxCommand.java
@@ -18,6 +18,8 @@
 
 import com.cloud.agent.api.Command;
 
+import java.util.Objects;
+
 public class NsxCommand extends Command {
     private String zoneName;
     private Long zoneId;
@@ -66,4 +68,18 @@ public void setAccountId(Long accountId) {
     public boolean executeInSequence() {
         return false;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        NsxCommand that = (NsxCommand) o;
+        return Objects.equals(zoneName, that.zoneName) && Objects.equals(zoneId, that.zoneId) && Objects.equals(accountName, that.accountName) && Objects.equals(accountId, that.accountId);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), zoneName, zoneId, accountName, accountId);
+    }
 }
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/api/command/AddNsxControllerCmd.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/api/command/AddNsxControllerCmd.java
@@ -53,7 +53,6 @@ public class AddNsxControllerCmd extends BaseCmd {
     @Parameter(name = ApiConstants.NSX_PROVIDER_HOSTNAME, type = CommandType.STRING, required = true, description = "NSX controller hostname / IP address")
     private String hostname;
 
-    // TODO: May not be required
     @Parameter(name = ApiConstants.NSX_PROVIDER_PORT, type = CommandType.STRING, description = "NSX controller port")
     private String port;
     @Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, required = true, description = "Username to log into NSX controller")
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/api/response/NsxControllerResponse.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/api/response/NsxControllerResponse.java
@@ -48,8 +48,6 @@ public class NsxControllerResponse extends BaseResponse {
     @Param(description = "NSX controller port")
     private String port;
 
-    // TODO: Should Password be returned?
-
     @SerializedName(ApiConstants.TIER0_GATEWAY)
     @Param(description = "The tier-0 gateway network. Tier-0 gateway is responsible for handling" +
             " traffic between logical and physical networks"
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java
@@ -53,6 +53,7 @@
 import org.apache.log4j.Logger;
 
 import javax.naming.ConfigurationException;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
@@ -124,7 +125,7 @@ public Answer executeRequest(Command cmd) {
 
     @Override
     public void disconnected() {
-
+        // Do nothing
     }
 
     @Override
@@ -134,6 +135,7 @@ public IAgentControl getAgentControl() {
 
     @Override
     public void setAgentControl(IAgentControl agentControl) {
+        // Do nothing
     }
 
     @Override
@@ -148,12 +150,12 @@ public void setName(String name) {
 
     @Override
     public void setConfigParams(Map<String, Object> params) {
-
+        // Do nothing
     }
 
     @Override
     public Map<String, Object> getConfigParams() {
-        return null;
+        return new HashMap<>();
     }
 
     @Override
@@ -163,7 +165,7 @@ public int getRunLevel() {
 
     @Override
     public void setRunLevel(int level) {
-
+        // Do nothing
     }
 
     @Override
@@ -222,7 +224,7 @@ private Answer executeRequest(ReadyCommand cmd) {
         return new ReadyAnswer(cmd);
     }
 
-    private Function<Class, Service> nsxService = svcClass -> { return nsxApi.getApiClient().createStub(svcClass); };
+    private Function<Class<? extends Service>, Service> nsxService = svcClass -> nsxApi.getApiClient().createStub(svcClass);
     private Answer executeRequest(CreateNsxTier1GatewayCommand cmd) {
         String name = getTier1GatewayName(cmd);
         Tier1 tier1 = getTier1Gateway(name);
@@ -315,24 +317,6 @@ private List<com.vmware.nsx_policy.model.LocaleServices> getTier0LocalServices(S
         }
     }
 
-    private EdgeCluster getEdgeClusterDetails(String edgeClusterName) {
-        try {
-            EdgeClusters edgeClusterService = (EdgeClusters) nsxService.apply(EdgeClusters.class);
-            return edgeClusterService.get(edgeClusterName);
-        } catch (Exception e) {
-            throw new CloudRuntimeException(String.format("Failed to fetch details of edge cluster: %s, due to: %s", edgeClusterName, e.getMessage()));
-        }
-    }
-
-    private ServiceSegmentListResult listServiceSegments() {
-        try {
-            ServiceSegments serviceSegmentSvc = (ServiceSegments) nsxService.apply(ServiceSegments.class);
-            return serviceSegmentSvc.list(null, null, null, true, null);
-        } catch (Exception e) {
-            throw new CloudRuntimeException(String.format("Failed to fetch service segment list due to %s", e.getMessage()));
-        }
-    }
-
     private Tier1 getTier1Gateway(String tier1GatewayId) {
         try {
             Tier1s tier1service = (Tier1s) nsxService.apply(Tier1s.class);
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApi.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApi.java
@@ -21,8 +21,6 @@
 
 public class NsxApi {
 
-    private static final Logger S_LOGGER = Logger.getLogger(NsxApi.class);
-
     ApiClient apiClient;
 
     public ApiClient getApiClient() {
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java
diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

Original file line number	Diff line number	Diff line change
`@@ -343,7 +343,6 @@ public <T> URI toUri(T value) {`
`343`	`343`	`}`
`344`	`344`	`},`
`345`	`345`	`Vswitch("vs", String.class), Undecided(null, null), Vnet("vnet", Long.class);`
`346`		`-// NSX("nsx", String.class);`
`347`	`346`
`348`	`347`	`private final String scheme;`
`349`	`348`	`private final Class<?> type;`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public enum State {`
`29`	`29`	`public static final String defaultVPCOfferingName = "Default VPC offering";`
`30`	`30`	`public static final String defaultVPCNSOfferingName = "Default VPC offering with Netscaler";`
`31`	`31`	`public static final String redundantVPCOfferingName = "Redundant VPC offering";`
`32`		`- public static final String defaultVPCNSXSOfferingName = "VPC offering with NSX";`
	`32`	`+ public static final String DEFAULT_VPC_NSX_OFFERING_NAME = "VPC offering with NSX";`
`33`	`33`
`34`	`34`	`/**`
`35`	`35`	`*`