Arik Hadas has uploaded a new change for review.

Change subject: core: cleanup in MoveOrCopyDiskCommand
......................................................................

core: cleanup in MoveOrCopyDiskCommand

- use CommandBase#failCanDoAction
- replace if-else statement with ternary if

Change-Id: Id28853bca622bf19ef66c983fa2d7ea38b943fa7
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
1 file changed, 21 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/07/18007/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
index 2579404..7b0e68e 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
@@ -52,11 +52,9 @@
 
     @Override
     protected void setActionMessageParameters() {
-        if (getParameters().getOperation() == ImageOperation.Copy) {
-            addCanDoActionMessage(VdcBllMessages.VAR__ACTION__COPY);
-        } else {
-            addCanDoActionMessage(VdcBllMessages.VAR__ACTION__MOVE);
-        }
+        addCanDoActionMessage(getParameters().getOperation() == 
ImageOperation.Copy ?
+                        VdcBllMessages.VAR__ACTION__COPY
+                        : VdcBllMessages.VAR__ACTION__MOVE);
         addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM_DISK);
     }
 
@@ -81,16 +79,14 @@
     protected boolean isSourceAndDestTheSame() {
         if (getParameters().getOperation() == ImageOperation.Move
                 && 
getParameters().getSourceDomainId().equals(getParameters().getStorageDomainId()))
 {
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME);
-            return false;
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME);
         }
         return true;
     }
 
     protected boolean isImageExist() {
         if (getImage() == null) {
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_NOT_EXIST);
-            return false;
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_NOT_EXIST);
         }
         return true;
     }
@@ -99,12 +95,11 @@
         DiskImage diskImage = getImage();
         if (diskImage.getImageStatus() == ImageStatus.LOCKED) {
             if (getParameters().getOperation() == ImageOperation.Move) {
-                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED);
-                addCanDoActionMessage(String.format("$%1$s %2$s", 
"diskAliases", diskImage.getDiskAlias()));
+                return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED,
+                        String.format("$%1$s %2$s", "diskAliases", 
diskImage.getDiskAlias()));
             } else {
-                
addCanDoActionMessage(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED);
+                return 
failCanDoAction(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED);
             }
-            return false;
         }
         return true;
     }
@@ -116,19 +111,16 @@
      * @return
      */
     protected boolean checkOperationIsCorrect() {
-        boolean retValue = true;
         if (getParameters().getOperation() == ImageOperation.Copy
                 && getImage().getVmEntityType() != VmEntityType.TEMPLATE) {
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK);
-            retValue = false;
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK);
         }
 
-        if (retValue && getParameters().getOperation() == ImageOperation.Move
+        if (getParameters().getOperation() == ImageOperation.Move
                 && getImage().getVmEntityType() == VmEntityType.TEMPLATE) {
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK);
-            retValue = false;
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK);
         }
-        return retValue;
+        return true;
     }
 
     protected boolean validateDestStorage() {
@@ -168,8 +160,7 @@
     protected boolean checkIfNeedToBeOverride() {
         if (getParameters().getOperation() == ImageOperation.Copy && 
!getParameters().getForceOverride()
                 && 
getImage().getStorageIds().contains(getStorageDomain().getId())) {
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS);
-            return false;
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS);
         }
         return true;
     }
@@ -236,16 +227,14 @@
      * @return
      */
     protected boolean checkTemplateInDestStorageDomain() {
-        boolean retValue = true;
         if (getParameters().getOperation() == ImageOperation.Move
                 && !Guid.Empty.equals(getImage().getImageTemplateId())) {
             DiskImage templateImage = 
getDiskImageDao().get(getImage().getImageTemplateId());
             if 
(!templateImage.getStorageIds().contains(getParameters().getStorageDomainId())) 
{
-                retValue = false;
-                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN);
+                return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN);
             }
         }
-        return retValue;
+        return true;
     }
 
     protected VmDeviceDAO getVmDeviceDAO() {
@@ -352,18 +341,15 @@
      * @return
      */
     private boolean canFindVmOrTemplate() {
-        boolean retValue = true;
         if (getParameters().getOperation() == ImageOperation.Copy) {
             Collection<VmTemplate> templates = 
getVmTemplateDAO().getAllForImage(getImage().getImageId()).values();
             if (templates.isEmpty()) {
-                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
-                retValue = false;
-            } else {
-                VmTemplate vmTemplate = templates.iterator().next();
-                setVmTemplate(vmTemplate);
-                sharedLockMap = 
Collections.singletonMap(vmTemplate.getId().toString(),
-                        
LockMessagesMatchUtil.makeLockingPair(LockingGroup.TEMPLATE, 
VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED));
+                return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
             }
+            VmTemplate vmTemplate = templates.iterator().next();
+            setVmTemplate(vmTemplate);
+            sharedLockMap = 
Collections.singletonMap(vmTemplate.getId().toString(),
+                    
LockMessagesMatchUtil.makeLockingPair(LockingGroup.TEMPLATE, 
VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED));
         } else {
             List<VM> vmsForDisk = getVmsForDiskId();
             if (!vmsForDisk.isEmpty()) {
@@ -375,7 +361,7 @@
                 lockForMove(lockMap);
             }
         }
-        return retValue;
+        return true;
     }
 
     protected void lockForMove(Map<String, Pair<String, String>> lockMap) {


-- 
To view, visit http://gerrit.ovirt.org/18007
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id28853bca622bf19ef66c983fa2d7ea38b943fa7
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to