Oved Ourfali has uploaded a new change for review. Change subject: core: case-insensitive name uniqueness validation on DCs and Clusters ......................................................................
core: case-insensitive name uniqueness validation on DCs and Clusters This patch ensures no two DCs and Clusters are created with the same name, using a case-insensitive validation (rather than the current implementation which is case-sensitive). Change-Id: I316efa401f742638c070561b75fc863178d8c1a8 Bug-Url: https://bugzilla.redhat.com/828080 Signed-off-by: Oved Ourfali <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java 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/StoragePoolManagementCommandBase.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/UpdateVdsGroupCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java M packaging/dbscripts/storages_sp.sql M packaging/dbscripts/vds_groups_sp.sql 14 files changed, 78 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/14/21214/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java index bee280e..0d68116 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java @@ -74,7 +74,7 @@ boolean result = super.canDoAction(); getReturnValue().getCanDoActionMessages() .add(VdcBllMessages.VAR__ACTION__CREATE.toString()); - if (DbFacade.getInstance().getVdsGroupDao().getByName(getVdsGroup().getName()) != null) { + if (!isVdsGroupUnique(getVdsGroup().getName())) { addCanDoActionMessage(VdcBllMessages.VDS_GROUP_CANNOT_DO_ACTION_NAME_IN_USE); result = false; } else if (getVdsGroup().supportsVirtService() diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java index be33823..cd6910d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java @@ -147,10 +147,9 @@ addCanDoActionMessage(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID); result = false; } - // check that if name was changed, it was done to the same cluster + // if the name was changed then make sure the new name is unique if (result && !StringUtils.equals(oldGroup.getName(), getVdsGroup().getName())) { - VDSGroup groupWithName = getVdsGroupDAO().getByName(getVdsGroup().getName()); - if (groupWithName != null && !groupWithName.getId().equals(getVdsGroup().getId())) { + if (!isVdsGroupUnique(getVdsGroup().getName())) { addCanDoActionMessage(VdcBllMessages.VDS_GROUP_CANNOT_DO_ACTION_NAME_IN_USE); result = false; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java index f666bfb..980bdae 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java @@ -13,6 +13,7 @@ import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.scheduling.ClusterPolicy; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dao.VdsGroupDAO; import org.ovirt.engine.core.utils.customprop.SimpleCustomPropertiesUtil; import org.ovirt.engine.core.utils.customprop.ValidationError; @@ -106,4 +107,11 @@ } } } + + protected boolean isVdsGroupUnique(String vdsGroupName) { + VdsGroupDAO vdsGroupDao = getVdsGroupDAO(); + List<VDSGroup> vdsGroups = vdsGroupDao.getByName(vdsGroupName, false); + return (vdsGroups == null || vdsGroups.isEmpty()); + } + } 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 5fd31b0..a8800dd 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 @@ -17,7 +17,6 @@ import org.ovirt.engine.core.common.businessentities.network.VnicProfile; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; -import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.NetworkUtils; public class AddEmptyStoragePoolCommand<T extends StoragePoolManagementParameter> extends @@ -77,7 +76,7 @@ protected boolean canDoAction() { boolean result = true; StoragePoolValidator storagePoolValidator = new StoragePoolValidator(getStoragePool()); - if (result && DbFacade.getInstance().getStoragePoolDao().getByName(getStoragePool().getName()) != null) { + if (result && !(isStoragePoolUnique(getStoragePool().getName()))) { result = false; addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); } else if (!checkStoragePoolNameLengthValid()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolManagementCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolManagementCommandBase.java index e4def08..7c9bf14 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolManagementCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolManagementCommandBase.java @@ -8,6 +8,7 @@ import org.ovirt.engine.core.common.validation.group.CreateEntity; import org.ovirt.engine.core.common.validation.group.UpdateEntity; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dao.StoragePoolDAO; public abstract class StoragePoolManagementCommandBase<T extends StoragePoolManagementParameter> extends StorageHandlingCommandBase<T> { @@ -40,4 +41,10 @@ addValidationGroup(CreateEntity.class, UpdateEntity.class); return super.getValidationGroups(); } + + protected boolean isStoragePoolUnique(String storagePoolName) { + StoragePoolDAO spDao = getStoragePoolDAO(); + List<StoragePool> sps = spDao.getByName(storagePoolName, false); + return (sps == null || sps.isEmpty()); + } } 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 3e74bea..859c409 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 @@ -158,7 +158,7 @@ boolean returnValue = checkStoragePool(); _oldStoragePool = getStoragePoolDAO().get(getStoragePool().getId()); if (returnValue && !StringUtils.equals(_oldStoragePool.getName(), getStoragePool().getName()) - && getStoragePoolDAO().getByName(getStoragePool().getName()) != null) { + && !isStoragePoolUnique(getStoragePool().getName())) { returnValue = false; addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommandTest.java index 9c5bfd0..581dd1a 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommandTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; @@ -287,6 +288,9 @@ when(vdsGroupDAO.get(any(Guid.class))).thenReturn(createDefaultVdsGroup()); when(vdsGroupDAO.getByName(anyString())).thenReturn(createDefaultVdsGroup()); + List<VDSGroup> vdsGroupList = new ArrayList<VDSGroup>(); + vdsGroupList.add(createDefaultVdsGroup()); + when(vdsGroupDAO.getByName(anyString(), anyBoolean())).thenReturn(vdsGroupList); } private void createCommandWithDifferentName() { 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 460612a..be78bca 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 @@ -4,6 +4,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -146,7 +147,9 @@ private void newPoolNameIsAlreadyTaken() { when(spDao.get(any(Guid.class))).thenReturn(new StoragePool()); - when(spDao.getByName(anyString())).thenReturn(createDefaultStoragePool()); + List<StoragePool> storagePoolList = new ArrayList<StoragePool>(); + storagePoolList.add(createDefaultStoragePool()); + when(spDao.getByName(anyString(), anyBoolean())).thenReturn(new ArrayList<StoragePool>(storagePoolList)); } private void storagePoolWithVersionHigherThanCluster() { diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java index 6a1c5c4..0f7c92f 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java @@ -39,12 +39,23 @@ * Retrieves the storage pool with the given name. * * @param name - * the storage pool name + * the storage pool name (case-sensitive get) * @return the storage pool */ StoragePool getByName(String name); /** + * Retrieves the storage pools with the given name. + * + * @param name + * the storage pool name + * @param isCaseSensitive + * whether to do case-sensitive get or not + * @return the storage pool + */ + List<StoragePool> getByName(String name, boolean isCaseSensitive); + + /** * Retrieves the storage pool for the specified VDS. * * @param vds diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java index 175c21f..281506e 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java @@ -64,9 +64,18 @@ } @Override + public List<StoragePool> getByName(String name, boolean isCaseSensitive) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() + .addValue("name", name) + .addValue("is_case_sensitive", isCaseSensitive); + return getCallsHandler().executeReadList("Getstorage_poolByName", mapper, parameterSource); + } + + @Override public StoragePool getByName(String name) { MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() - .addValue("name", name); + .addValue("name", name) + .addValue("is_case_sensitive", true); return getCallsHandler().executeRead("Getstorage_poolByName", mapper, parameterSource); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAO.java index c49a784..9f0ad33 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAO.java @@ -43,6 +43,17 @@ VDSGroup getByName(String name); /** + * Retrieves the groups with the specified name. + * + * @param name + * the group name + * @param isCaseSensitive + * whether to do case-sensitive get or not + * @return the group + */ + List<VDSGroup> getByName(String name, boolean isCaseSensitive); + + /** * Retrieves the group with the specified name for the user * * @param name diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java index 989c56f..e50af61 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java @@ -45,7 +45,8 @@ @Override public VDSGroup getByName(String name) { MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() - .addValue("vds_group_name", name); + .addValue("vds_group_name", name) + .addValue("is_case_sensitive", true); return (VDSGroup) DbFacadeUtils.asSingleResult( getCallsHandler().executeReadList("GetVdsGroupByVdsGroupName", @@ -54,6 +55,17 @@ } @Override + public List<VDSGroup> getByName(String name, boolean isCaseSensitive) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() + .addValue("vds_group_name", name) + .addValue("is_case_sensitive", isCaseSensitive); + + return getCallsHandler().executeReadList("GetVdsGroupByVdsGroupName", + VdsGroupRowMapper.instance, + parameterSource); + } + + @Override public VDSGroup getByName(String name, Guid userID, boolean isFiltered) { MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() .addValue("vds_group_name", name).addValue("user_id", userID).addValue("is_filtered", isFiltered); diff --git a/packaging/dbscripts/storages_sp.sql b/packaging/dbscripts/storages_sp.sql index bc349ee..303f8e7 100644 --- a/packaging/dbscripts/storages_sp.sql +++ b/packaging/dbscripts/storages_sp.sql @@ -182,13 +182,13 @@ -Create or replace FUNCTION Getstorage_poolByName(v_name VARCHAR(40)) +Create or replace FUNCTION Getstorage_poolByName(v_name VARCHAR(40), v_is_case_sensitive BOOLEAN) RETURNS SETOF storage_pool STABLE AS $procedure$ BEGIN RETURN QUERY SELECT * FROM storage_pool - WHERE name = v_name; + WHERE name = v_name OR (NOT v_is_case_sensitive AND name ilike v_name); END; $procedure$ LANGUAGE plpgsql; diff --git a/packaging/dbscripts/vds_groups_sp.sql b/packaging/dbscripts/vds_groups_sp.sql index 7d47129..00217b3 100644 --- a/packaging/dbscripts/vds_groups_sp.sql +++ b/packaging/dbscripts/vds_groups_sp.sql @@ -134,12 +134,12 @@ -Create or replace FUNCTION GetVdsGroupByVdsGroupName(v_vds_group_name VARCHAR(40)) RETURNS SETOF vds_groups_view STABLE +Create or replace FUNCTION GetVdsGroupByVdsGroupName(v_vds_group_name VARCHAR(40), v_is_case_sensitive BOOLEAN) RETURNS SETOF vds_groups_view STABLE AS $procedure$ BEGIN RETURN QUERY SELECT vds_groups_view.* FROM vds_groups_view - WHERE name = v_vds_group_name; + WHERE name = v_vds_group_name OR (NOT v_is_case_sensitive AND name ilike v_vds_group_name); END; $procedure$ LANGUAGE plpgsql; -- To view, visit http://gerrit.ovirt.org/21214 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I316efa401f742638c070561b75fc863178d8c1a8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Oved Ourfali <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
