Alissa Bonas has uploaded a new change for review.

Change subject: core: improve CDA of AddStorageServerConnection
......................................................................

core: improve CDA of AddStorageServerConnection

Improve canDoACtion of AddStorageServerConnectionCommand
to have early returns in failures, and call to its super.canDoAction
since there are checks that are done there and
are not done by the overriding command.

Change-Id: I00a46aac7d642128c287ef633c492f3c1b85f572
Signed-off-by: Alissa Bonas <abo...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java
2 files changed, 8 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/26/15126/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
index e38cfb8..00b94df 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
@@ -67,28 +67,26 @@
 
     @Override
     protected boolean canDoAction() {
-        boolean returnValue = true;
         StorageServerConnections paramConnection = 
getParameters().getStorageServerConnection();
         if (paramConnection.getstorage_type() == StorageType.NFS
                 && !new 
NfsMountPointConstraint().isValid(paramConnection.getconnection(), null)) {
-            returnValue = false;
-            
addCanDoActionMessage(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_INVALID);
+            return 
failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_INVALID);
         }
         if (paramConnection.getstorage_type() == StorageType.POSIXFS && 
(StringUtils.isEmpty(paramConnection.getVfsType()))) {
             return 
failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE);
         }
 
         if (getParameters().getVdsId().equals(Guid.Empty)) {
-            returnValue = InitializeVds();
+            if (!InitializeVds()) {
+                return false;
+            }
         } else if (getVds() == null) {
-            addCanDoActionMessage(VdcBllMessages.VDS_INVALID_SERVER_ID);
-            returnValue = false;
+            return failCanDoAction(VdcBllMessages.VDS_INVALID_SERVER_ID);
         } else if (getVds().getStatus() != VDSStatus.Up) {
-            
addCanDoActionMessage(VdcBllMessages.VDS_ADD_STORAGE_SERVER_STATUS_MUST_BE_UP);
-            returnValue = false;
+            return 
failCanDoAction(VdcBllMessages.VDS_ADD_STORAGE_SERVER_STATUS_MUST_BE_UP);
         }
 
-        return returnValue;
+        return super.canDoAction();
     }
 
     @Override
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java
index 0027f3f..f849a2f 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java
@@ -62,6 +62,7 @@
         StorageServerConnections newPosixConnection = 
createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1",
 StorageType.POSIXFS, "nfs" , "timeo=30");
         parameters.setStorageServerConnection(newPosixConnection);
         parameters.setVdsId(Guid.Empty);
+        parameters.setStoragePoolId(Guid.Empty);
         doReturn(true).when(command).InitializeVds();
         CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command);
     }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I00a46aac7d642128c287ef633c492f3c1b85f572
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to