Amit Aviram has uploaded a new change for review. Change subject: core: Command refactoring. ......................................................................
core: Command refactoring. UpdateStoragePoolCommand's CDA was refactored to fit the CDA style in the system. as a side effect this makes the code a bit more efficient since once a validation fails within the CDA it makes the function return and not continue to the next validation. Change-Id: Icd0c28eb86af753a7e48e845da23fc5feca2019a Signed-off-by: Amit Aviram <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java 2 files changed, 25 insertions(+), 22 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/88/35788/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java index 7f1d443..cc89e82 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java @@ -163,28 +163,29 @@ @Override protected boolean canDoAction() { - boolean returnValue = checkStoragePool(); - if (returnValue && !StringUtils.equals(getOldStoragePool().getName(), getStoragePool().getName()) - && !isStoragePoolUnique(getStoragePool().getName())) { - returnValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); + if (!checkStoragePool()) { + return false; } - if (returnValue - && getOldStoragePool().isLocal() != getStoragePool().isLocal() + if (!StringUtils.equals(getOldStoragePool().getName(), getStoragePool().getName()) + && !isStoragePoolUnique(getStoragePool().getName())) { + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); + return false; + } + if ( getOldStoragePool().isLocal() != getStoragePool().isLocal() && getStorageDomainStaticDAO().getAllForStoragePool(getStoragePool().getId()).size() > 0) { - returnValue = false; - getReturnValue() - .getCanDoActionMessages() + getReturnValue().getCanDoActionMessages() .add(VdcBllMessages.ERROR_CANNOT_CHANGE_STORAGE_POOL_TYPE_WITH_DOMAINS .toString()); + return false; } - returnValue = returnValue && checkStoragePoolNameLengthValid(); - if (returnValue - && !getOldStoragePool().getcompatibility_version().equals(getStoragePool() - .getcompatibility_version())) { + if (!checkStoragePoolNameLengthValid()) { + return false; + } + if ( !getOldStoragePool().getcompatibility_version().equals(getStoragePool() + .getcompatibility_version())) { if (!isStoragePoolVersionSupported()) { addCanDoActionMessage(VersionSupport.getUnsupportedVersionMessage()); - returnValue = false; + return false; } // decreasing of compatibility version is allowed under conditions else if (getStoragePool().getcompatibility_version().compareTo(getOldStoragePool().getcompatibility_version()) < 0) { @@ -195,26 +196,26 @@ validator.setDataCenter(getStoragePool()); if (!NetworkUtils.isManagementNetwork(network) || !validator.canNetworkCompatabilityBeDecreased()) { - returnValue = false; addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION); + return false; } } else if (networks.size() > 1) { - returnValue = false; addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION); + return false; } } else { // Check all clusters has at least the same compatibility version. - returnValue = checkAllClustersLevel(); + if (!checkAllClustersLevel()) { + return false; + } } } StoragePoolValidator validator = createStoragePoolValidator(); - if (returnValue) { - returnValue = validate(validator.isNotLocalfsWithDefaultCluster()); - } - return returnValue; + return validate(validator.isNotLocalfsWithDefaultCluster()); } + protected boolean checkAllClustersLevel() { boolean returnValue = true; List<VDSGroup> clusters = getVdsGroupDAO().getAllForStoragePool(getStoragePool().getId()); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java index e7ed423..bc57b12 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java @@ -95,6 +95,7 @@ doReturn(vdsDao).when(cmd).getVdsDAO(); doReturn(networkDao).when(cmd).getNetworkDAO(); doReturn(validator).when(cmd).createStoragePoolValidator(); + doReturn(true).when(sdList).isEmpty(); mcr.mockConfigValue(ConfigValues.AutoRegistrationDefaultVdsGroupID, DEFAULT_VDS_GROUP_ID); mcr.mockConfigValue(ConfigValues.ManagementNetwork, "test_mgmt"); @@ -240,6 +241,7 @@ } private void domainListNotEmpty() { + when(sdList.isEmpty()).thenReturn(false); when(sdList.size()).thenReturn(1); } -- To view, visit http://gerrit.ovirt.org/35788 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icd0c28eb86af753a7e48e845da23fc5feca2019a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Amit Aviram <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
