Fix some LGTM alerts (apache#3143)

malcolmtaylor · GabrielBrascher · commit 97ddd8dffd4d · 2019-01-24T20:22:39.000-02:00
* Fix some LGTM alerts

* address review comments from Gabriel Brascher and Rafael Weingartner
diff --git a/api/src/main/java/com/cloud/deploy/DeployDestination.java b/api/src/main/java/com/cloud/deploy/DeployDestination.java
@@ -78,29 +78,35 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object obj) {
+        if (obj == null) {
+            return false;
+        }
+        if (!(obj instanceof DeployDestination)) {
+            return false;
+        }
         DeployDestination that = (DeployDestination)obj;
-        if (_dc == null || that._dc == null) {
+        if (this._dc == null || that._dc == null) {
             return false;
         }
-        if (_dc.getId() != that._dc.getId()) {
+        if (this._dc.getId() != that._dc.getId()) {
             return false;
         }
-        if (_pod == null || that._pod == null) {
+        if (this._pod == null || that._pod == null) {
             return false;
         }
-        if (_pod.getId() != that._pod.getId()) {
+        if (this._pod.getId() != that._pod.getId()) {
             return false;
         }
-        if (_cluster == null || that._cluster == null) {
+        if (this._cluster == null || that._cluster == null) {
             return false;
         }
-        if (_cluster.getId() != that._cluster.getId()) {
+        if (this._cluster.getId() != that._cluster.getId()) {
             return false;
         }
-        if (_host == null || that._host == null) {
+        if (this._host == null || that._host == null) {
             return false;
         }
-        return _host.getId() == that._host.getId();
+        return this._host.getId() == that._host.getId();
     }
 
     @Override
diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@ -780,7 +780,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) throws
                         }
 
                         final int devId = vmNic.getDeviceId();
-                        if (devId > deviceIds.length) {
+                        if (devId >= deviceIds.length) {
                             throw new IllegalArgumentException("Device id for nic is too large: " + vmNic);
                         }
                         if (deviceIds[devId]) {
diff --git a/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java b/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java
@@ -439,50 +439,10 @@ public final Answer executeRequest(final Command cmd) {
             return _vrResource.executeRequest((NetworkElementCommand)cmd);
         }if (clazz == CheckSshCommand.class) {
             answer = execute((CheckSshCommand)cmd);
-        } else if (clazz == GetDomRVersionCmd.class) {
-            answer = execute((GetDomRVersionCmd)cmd);
         } else if (cmd instanceof NetworkUsageCommand) {
             answer = execute((NetworkUsageCommand)cmd);
-        } else if (clazz == IpAssocCommand.class) {
-            answer = execute((IpAssocCommand)cmd);
-        } else if (clazz == DnsMasqConfigCommand.class) {
-            return execute((DnsMasqConfigCommand)cmd);
-        } else if (clazz == CreateIpAliasCommand.class) {
-            return execute((CreateIpAliasCommand)cmd);
-        } else if (clazz == DhcpEntryCommand.class) {
-            answer = execute((DhcpEntryCommand)cmd);
-        } else if (clazz == VmDataCommand.class) {
-            answer = execute((VmDataCommand)cmd);
-        } else if (clazz == SavePasswordCommand.class) {
-            answer = execute((SavePasswordCommand)cmd);
-        } else if (clazz == SetFirewallRulesCommand.class) {
-            answer = execute((SetFirewallRulesCommand)cmd);
-        } else if (clazz == LoadBalancerConfigCommand.class) {
-            answer = execute((LoadBalancerConfigCommand)cmd);
-        } else if (clazz == DeleteIpAliasCommand.class) {
-            return execute((DeleteIpAliasCommand)cmd);
         } else if (clazz == PingTestCommand.class) {
             answer = execute((PingTestCommand)cmd);
-        } else if (clazz == SetStaticNatRulesCommand.class) {
-            answer = execute((SetStaticNatRulesCommand)cmd);
-        } else if (clazz == CheckRouterCommand.class) {
-            answer = execute((CheckRouterCommand)cmd);
-        } else if (clazz == SetPortForwardingRulesCommand.class) {
-            answer = execute((SetPortForwardingRulesCommand)cmd);
-        } else if (clazz == SetSourceNatCommand.class) {
-            answer = execute((SetSourceNatCommand)cmd);
-        } else if (clazz == Site2SiteVpnCfgCommand.class) {
-            answer = execute((Site2SiteVpnCfgCommand)cmd);
-        } else if (clazz == CheckS2SVpnConnectionsCommand.class) {
-            answer = execute((CheckS2SVpnConnectionsCommand) cmd);
-        } else if (clazz == RemoteAccessVpnCfgCommand.class) {
-            answer = execute((RemoteAccessVpnCfgCommand) cmd);
-        } else if (clazz == VpnUsersCfgCommand.class) {
-            answer = execute((VpnUsersCfgCommand) cmd);
-        } else if (clazz == SetStaticRouteCommand.class) {
-            answer = execute((SetStaticRouteCommand) cmd);
-        } else if (clazz == SetMonitorServiceCommand.class) {
-            answer = execute((SetMonitorServiceCommand) cmd);
         } else if (clazz == PlugNicCommand.class) {
             answer = execute((PlugNicCommand)cmd);
         } else if (clazz == UnPlugNicCommand.class) {
diff --git a/plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3HypervisorResource.java b/plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3HypervisorResource.java
@@ -27,8 +27,6 @@
 import org.apache.cloudstack.storage.command.AttachCommand;
 import org.apache.cloudstack.storage.command.CopyCommand;
 import org.apache.cloudstack.storage.command.CreateObjectCommand;
-import org.apache.cloudstack.storage.command.DeleteCommand;
-import org.apache.cloudstack.storage.command.DettachCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
 import org.apache.log4j.Logger;
 
@@ -195,18 +193,12 @@ public Answer executeRequest(Command cmd) {
             return storageprocessor.execute((CopyCommand)cmd);
         } else if (cmd instanceof StorageSubSystemCommand) {
             return storageHandler.handleStorageCommands((StorageSubSystemCommand)cmd);
-        } else if (clazz == DeleteCommand.class) {
-            return storageprocessor.execute((DeleteCommand)cmd);
         } else if (clazz == CreateCommand.class) {
             return storageprocessor.execute((CreateCommand)cmd);
         } else if (clazz == CreateObjectCommand.class) {
             return storageprocessor.execute((CreateObjectCommand)cmd);
         } else if (clazz == AttachIsoCommand.class) {
             return storageprocessor.attachIso((AttachCommand)cmd);
-        } else if (clazz == DettachCommand.class) {
-            return storageprocessor.execute((DettachCommand)cmd);
-        } else if (clazz == AttachCommand.class) {
-            return storageprocessor.execute((AttachCommand)cmd);
         } else if (clazz == CreatePrivateTemplateFromVolumeCommand.class) {
             return storageprocessor.execute((CreatePrivateTemplateFromVolumeCommand)cmd);
         } else if (clazz == DestroyCommand.class) {
diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
@@ -980,7 +980,7 @@ public PodResponse createPodResponse(Pod pod, Boolean showCapacities) {
                 endIp.add(((existingPodIpRange.length > 1) && (existingPodIpRange[1] != null)) ? existingPodIpRange[1] : "");
                 forSystemVms.add((existingPodIpRange.length > 2) && (existingPodIpRange[2] != null) ? existingPodIpRange[2] : "0");
                 vlanIds.add((existingPodIpRange.length > 3) &&
-                        (existingPodIpRange[3] != null && !existingPodIpRange.equals("untagged")) ?
+                        (existingPodIpRange[3] != null && !existingPodIpRange[3].equals("untagged")) ?
                         BroadcastDomainType.Vlan.toUri(existingPodIpRange[3]).toString() :
                         BroadcastDomainType.Vlan.toUri(Vlan.UNTAGGED).toString());
             }
diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java
@@ -74,14 +74,10 @@
 import org.apache.cloudstack.api.ResponseObject.ResponseView;
 import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.auth.APIAuthenticationManager;
-import org.apache.cloudstack.api.command.admin.account.ListAccountsCmdByAdmin;
 import org.apache.cloudstack.api.command.admin.host.ListHostsCmd;
 import org.apache.cloudstack.api.command.admin.router.ListRoutersCmd;
 import org.apache.cloudstack.api.command.admin.storage.ListStoragePoolsCmd;
 import org.apache.cloudstack.api.command.admin.user.ListUsersCmd;
-import org.apache.cloudstack.api.command.admin.vm.ListVMsCmdByAdmin;
-import org.apache.cloudstack.api.command.admin.volume.ListVolumesCmdByAdmin;
-import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin;
 import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
 import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd;
 import org.apache.cloudstack.api.command.user.event.ListEventsCmd;
@@ -732,14 +728,13 @@ private String queueCommand(final BaseCmd cmdObj, final Map<String, String> para
             // if the command is of the listXXXCommand, we will need to also return the
             // the job id and status if possible
             // For those listXXXCommand which we have already created DB views, this step is not needed since async job is joined in their db views.
-            if (cmdObj instanceof BaseListCmd && !(cmdObj instanceof ListVMsCmd) && !(cmdObj instanceof ListVMsCmdByAdmin) && !(cmdObj instanceof ListRoutersCmd)
+            if (cmdObj instanceof BaseListCmd && !(cmdObj instanceof ListVMsCmd) && !(cmdObj instanceof ListRoutersCmd)
                     && !(cmdObj instanceof ListSecurityGroupsCmd) &&
                     !(cmdObj instanceof ListTagsCmd) && !(cmdObj instanceof ListEventsCmd) && !(cmdObj instanceof ListVMGroupsCmd) && !(cmdObj instanceof ListProjectsCmd) &&
                     !(cmdObj instanceof ListProjectAccountsCmd) && !(cmdObj instanceof ListProjectInvitationsCmd) && !(cmdObj instanceof ListHostsCmd) &&
-                    !(cmdObj instanceof ListVolumesCmd) && !(cmdObj instanceof ListVolumesCmdByAdmin) && !(cmdObj instanceof ListUsersCmd) && !(cmdObj instanceof ListAccountsCmd)
-                    && !(cmdObj instanceof ListAccountsCmdByAdmin) &&
-                    !(cmdObj instanceof ListStoragePoolsCmd) && !(cmdObj instanceof ListDiskOfferingsCmd) && !(cmdObj instanceof ListServiceOfferingsCmd) &&
-                    !(cmdObj instanceof ListZonesCmd) && !(cmdObj instanceof ListZonesCmdByAdmin)) {
+                    !(cmdObj instanceof ListVolumesCmd) && !(cmdObj instanceof ListUsersCmd) && !(cmdObj instanceof ListAccountsCmd)
+                    && !(cmdObj instanceof ListStoragePoolsCmd) && !(cmdObj instanceof ListDiskOfferingsCmd) && !(cmdObj instanceof ListServiceOfferingsCmd) &&
+                    !(cmdObj instanceof ListZonesCmd)) {
                 buildAsyncListResponse((BaseListCmd)cmdObj, caller);
             }
 

Original file line number	Diff line number	Diff line change
`@@ -78,29 +78,35 @@ public int hashCode() {`
`78`	`78`
`79`	`79`	`@Override`
`80`	`80`	`public boolean equals(Object obj) {`
	`81`	`+ if (obj == null) {`
	`82`	`+ return false;`
	`83`	`+ }`
	`84`	`+ if (!(obj instanceof DeployDestination)) {`
	`85`	`+ return false;`
	`86`	`+ }`
`81`	`87`	`DeployDestination that = (DeployDestination)obj;`
`82`		`- if (_dc == null \|\| that._dc == null) {`
	`88`	`+ if (this._dc == null \|\| that._dc == null) {`
`83`	`89`	`return false;`
`84`	`90`	`}`
`85`		`- if (_dc.getId() != that._dc.getId()) {`
	`91`	`+ if (this._dc.getId() != that._dc.getId()) {`
`86`	`92`	`return false;`
`87`	`93`	`}`
`88`		`- if (_pod == null \|\| that._pod == null) {`
	`94`	`+ if (this._pod == null \|\| that._pod == null) {`
`89`	`95`	`return false;`
`90`	`96`	`}`
`91`		`- if (_pod.getId() != that._pod.getId()) {`
	`97`	`+ if (this._pod.getId() != that._pod.getId()) {`
`92`	`98`	`return false;`
`93`	`99`	`}`
`94`		`- if (_cluster == null \|\| that._cluster == null) {`
	`100`	`+ if (this._cluster == null \|\| that._cluster == null) {`
`95`	`101`	`return false;`
`96`	`102`	`}`
`97`		`- if (_cluster.getId() != that._cluster.getId()) {`
	`103`	`+ if (this._cluster.getId() != that._cluster.getId()) {`
`98`	`104`	`return false;`
`99`	`105`	`}`
`100`		`- if (_host == null \|\| that._host == null) {`
	`106`	`+ if (this._host == null \|\| that._host == null) {`
`101`	`107`	`return false;`
`102`	`108`	`}`
`103`		`- return _host.getId() == that._host.getId();`
	`109`	`+ return this._host.getId() == that._host.getId();`
`104`	`110`	`}`
`105`	`111`
`106`	`112`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -780,7 +780,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) throws`
`780`	`780`	`}`
`781`	`781`
`782`	`782`	`final int devId = vmNic.getDeviceId();`
`783`		`- if (devId > deviceIds.length) {`
	`783`	`+ if (devId >= deviceIds.length) {`
`784`	`784`	`throw new IllegalArgumentException("Device id for nic is too large: " + vmNic);`
`785`	`785`	`}`
`786`	`786`	`if (deviceIds[devId]) {`
Original file line number	Diff line number	Diff line change
`@@ -980,7 +980,7 @@ public PodResponse createPodResponse(Pod pod, Boolean showCapacities) {`
`980`	`980`	`endIp.add(((existingPodIpRange.length > 1) && (existingPodIpRange[1] != null)) ? existingPodIpRange[1] : "");`
`981`	`981`	`forSystemVms.add((existingPodIpRange.length > 2) && (existingPodIpRange[2] != null) ? existingPodIpRange[2] : "0");`
`982`	`982`	`vlanIds.add((existingPodIpRange.length > 3) &&`
`983`		`- (existingPodIpRange[3] != null && !existingPodIpRange.equals("untagged")) ?`
	`983`	`+ (existingPodIpRange[3] != null && !existingPodIpRange[3].equals("untagged")) ?`
`984`	`984`	`BroadcastDomainType.Vlan.toUri(existingPodIpRange[3]).toString() :`
`985`	`985`	`BroadcastDomainType.Vlan.toUri(Vlan.UNTAGGED).toString());`
`986`	`986`	`}`