Allon Mureinik has uploaded a new change for review. Change subject: core: Standardize StoragePoolValidator ......................................................................
core: Standardize StoragePoolValidator Standardized StoragePoolValidator to look like all the other validators. The constructor now accepts only a storage_pool object and all the validation methods return ValidationResult objects with the VdcBllMessage instead of returning a boolean and adding the VdcBllMessage to a list. Change-Id: I79ede9de954b209c9c27451e430e9d6219cded0f Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddEmptyStoragePoolCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java 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/StoragePoolValidatorTest.java 4 files changed, 30 insertions(+), 38 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/20/11320/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddEmptyStoragePoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddEmptyStoragePoolCommand.java index bda1b5b..e479072 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddEmptyStoragePoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddEmptyStoragePoolCommand.java @@ -5,9 +5,9 @@ import org.ovirt.engine.core.bll.AddVdsGroupCommand; import org.ovirt.engine.core.bll.MultiLevelAdministrationHandler; +import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VersionSupport; import org.ovirt.engine.core.common.AuditLogType; -import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.StoragePoolManagementParameter; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; @@ -58,8 +58,7 @@ addCanDoActionMessage(VdcBllMessages.VAR__ACTION__CREATE); boolean result = super.canDoAction(); - StoragePoolValidator storagePoolValidator = - new StoragePoolValidator(getStoragePool(), getReturnValue().getCanDoActionMessages()); + StoragePoolValidator storagePoolValidator = new StoragePoolValidator(getStoragePool()); if (result && DbFacade.getInstance().getStoragePoolDao().getByName(getStoragePool().getname()) != null) { result = false; addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); @@ -69,7 +68,7 @@ )) { addCanDoActionMessage(VersionSupport.getUnsupportedVersionMessage()); result = false; - } else if (!storagePoolValidator.isPosixDcAndMatchingCompatiblityVersion()) { + } else if (!validate(storagePoolValidator.isPosixDcAndMatchingCompatiblityVersion())) { result = false; } return result; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java index bd48553..a712b4b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java @@ -2,6 +2,7 @@ import java.util.List; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.storage_pool; @@ -16,43 +17,35 @@ */ public class StoragePoolValidator { private storage_pool storagePool; - private List<String> canDoActionMessages; - public StoragePoolValidator(storage_pool storagePool, List<String> canDoActionMessages) { + public StoragePoolValidator(storage_pool storagePool) { this.storagePool = storagePool; - this.canDoActionMessages = canDoActionMessages; } /** * Checks in case the DC is of POSIX type that the compatibility version matches. In case there is mismatch, a * proper canDoAction message will be added * - * @return true if the version matches + * @return The result of the validation */ - public boolean isPosixDcAndMatchingCompatiblityVersion() { + public ValidationResult isPosixDcAndMatchingCompatiblityVersion() { if (storagePool.getstorage_pool_type() == StorageType.POSIXFS && !Config.<Boolean> GetValue (ConfigValues.PosixStorageEnabled, storagePool.getcompatibility_version().toString())) { - canDoActionMessages.add(VdcBllMessages.DATA_CENTER_POSIX_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION.toString()); - return false; + return new ValidationResult(VdcBllMessages.DATA_CENTER_POSIX_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); } - return true; - } - - public List<String> getCanDoActionMessages() { - return canDoActionMessages; + return ValidationResult.VALID; } protected VdsGroupDAO getVdsGroupDao() { return DbFacade.getInstance().getVdsGroupDao(); } - public boolean isNotLocalfsWithDefaultCluster() { + public ValidationResult isNotLocalfsWithDefaultCluster() { if (storagePool.getstorage_pool_type() == StorageType.LOCALFS && containsDefaultCluster()) { - canDoActionMessages.add(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_WITH_DEFAULT_VDS_GROUP_CANNOT_BE_LOCALFS.toString()); - return false; + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_WITH_DEFAULT_VDS_GROUP_CANNOT_BE_LOCALFS); } - return true; + return ValidationResult.VALID; } protected boolean containsDefaultCluster() { 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 f1e4df1..31ef0a4 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 @@ -10,12 +10,12 @@ import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.StoragePoolManagementParameter; import org.ovirt.engine.core.common.businessentities.QuotaEnforcementTypeEnum; +import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.StorageFormatType; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDSGroup; -import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; import org.ovirt.engine.core.common.vdscommands.SetStoragePoolDescriptionVDSCommandParameters; @@ -192,10 +192,10 @@ StoragePoolValidator validator = createStoragePoolValidator(); if (returnValue) { - returnValue = validator.isNotLocalfsWithDefaultCluster(); + returnValue = validate(validator.isNotLocalfsWithDefaultCluster()); } if (returnValue) { - returnValue = validator.isPosixDcAndMatchingCompatiblityVersion(); + returnValue = validate(validator.isPosixDcAndMatchingCompatiblityVersion()); } addCanDoActionMessage(VdcBllMessages.VAR__ACTION__UPDATE); return returnValue; @@ -223,7 +223,7 @@ } protected StoragePoolValidator createStoragePoolValidator() { - return new StoragePoolValidator(getStoragePool(), getReturnValue().getCanDoActionMessages()); + return new StoragePoolValidator(getStoragePool()); } protected boolean isStoragePoolVersionSupported() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java index 51202ae..2f518d9 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java @@ -1,16 +1,16 @@ package org.ovirt.engine.core.bll.storage; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; -import java.util.ArrayList; - import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.storage_pool; @@ -40,50 +40,50 @@ "test", StorageType.UNKNOWN.getValue(), StoragePoolStatus.Up.getValue()); - validator = spy(new StoragePoolValidator(storagePool, new ArrayList<String>())); + validator = spy(new StoragePoolValidator(storagePool)); } @Test public void testPosixDcAndMatchingCompatiblityVersion() { storagePool.setcompatibility_version(Version.v3_1); storagePool.setstorage_pool_type(StorageType.POSIXFS); - assertTrue(validator.isPosixDcAndMatchingCompatiblityVersion()); + assertTrue(validator.isPosixDcAndMatchingCompatiblityVersion().isValid()); } @Test public void testPosixDcAndNotMatchingCompatiblityVersion() { storagePool.setcompatibility_version(Version.v3_0); storagePool.setstorage_pool_type(StorageType.POSIXFS); - assertFalse(validator.isPosixDcAndMatchingCompatiblityVersion()); - assertMessages(VdcBllMessages.DATA_CENTER_POSIX_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); + ValidationResult result = validator.isPosixDcAndMatchingCompatiblityVersion(); + assertFalse(result.isValid()); + assertMessage(result, VdcBllMessages.DATA_CENTER_POSIX_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); } @Test public void testLocalDcAndMatchingCompatiblityVersion() { storagePool.setcompatibility_version(Version.v3_0); storagePool.setstorage_pool_type(StorageType.LOCALFS); - assertTrue(validator.isPosixDcAndMatchingCompatiblityVersion()); + assertTrue(validator.isPosixDcAndMatchingCompatiblityVersion().isValid()); } @Test public void testIsNotLocalFsWithDefaultCluster() { storagePool.setstorage_pool_type(StorageType.LOCALFS); doReturn(false).when(validator).containsDefaultCluster(); - assertTrue(validator.isNotLocalfsWithDefaultCluster()); + assertTrue(validator.isNotLocalfsWithDefaultCluster().isValid()); } @Test public void testIsNotLocalFsWithDefaultClusterWhenClusterIsDefault() { storagePool.setstorage_pool_type(StorageType.LOCALFS); doReturn(true).when(validator).containsDefaultCluster(); - assertFalse(validator.isNotLocalfsWithDefaultCluster()); - assertMessages(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_WITH_DEFAULT_VDS_GROUP_CANNOT_BE_LOCALFS); + ValidationResult result = validator.isNotLocalfsWithDefaultCluster(); + assertFalse(result.isValid()); + assertMessage(result, VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_WITH_DEFAULT_VDS_GROUP_CANNOT_BE_LOCALFS); } - protected void assertMessages(VdcBllMessages bllMsg) { - assertTrue("Wrong number of canDoActionMessages is returned", validator.getCanDoActionMessages().size() == 1); - assertTrue("Wrong canDoAction message is returned", validator.getCanDoActionMessages() - .contains(bllMsg.toString())); + private static void assertMessage(ValidationResult result, VdcBllMessages bllMsg) { + assertEquals("Wrong canDoAction message is returned", bllMsg, result.getMessage()); } } -- To view, visit http://gerrit.ovirt.org/11320 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I79ede9de954b209c9c27451e430e9d6219cded0f Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
