Martin Mucha has uploaded a new change for review. Change subject: core: Improved message when MAC Pool cannot be deleted ......................................................................
core: Improved message when MAC Pool cannot be deleted -standardized message, added listing of which datacenters are using it. -removed not used db function -added new one, listing all DCs for given MAC Pool Change-Id: I647420d7cc43853eddd2b17091cd627b9088a34d Signed-off-by: Martin Mucha <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDao.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDaoDbFacadeImpl.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/resources/bundles/AppErrors.properties M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/MacPoolDaoTest.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M packaging/dbscripts/mac_pools_sp.sql M packaging/dbscripts/storages_sp.sql 14 files changed, 68 insertions(+), 41 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/63/30663/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolValidator.java index 19158c2..21faff0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolValidator.java @@ -1,12 +1,16 @@ package org.ovirt.engine.core.bll; +import java.util.Collection; import java.util.List; import java.util.Objects; import org.ovirt.engine.core.common.businessentities.MacPool; +import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.MacPoolDao; +import org.ovirt.engine.core.dao.StoragePoolDAO; +import org.ovirt.engine.core.utils.ReplacementUtils; public class MacPoolValidator { @@ -22,9 +26,13 @@ } public ValidationResult notRemovingUsedPool() { - final int dcUsageCount = getMacPoolDao().getDcUsageCount(macPool.getId()); - return ValidationResult.failWith(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL). - when(dcUsageCount != 0); + final StoragePoolDAO storagePoolDao = DbFacade.getInstance().getStoragePoolDao(); + final List<StoragePool> dataCenters = storagePoolDao.getAllDataCentersByMacPoolId(macPool.getId()); + + final Collection<String> replacements = ReplacementUtils.replaceWithNameable("DATACENTERS_USING_MAC_POOL", dataCenters); + replacements.add(VdcBllMessages.VAR__ENTITIES__STORAGE_POOLS.name()); + return ValidationResult.failWith(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL, replacements.toArray(new String[0])). + when(dataCenters.size() != 0); } public ValidationResult macPoolExists() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java index a8b3179..56c0855 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java @@ -1,12 +1,12 @@ package org.ovirt.engine.core.bll; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.failsWith; import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid; import java.util.Arrays; +import java.util.Collections; import org.junit.Assert; import org.junit.Before; @@ -15,11 +15,13 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.ovirt.engine.core.common.businessentities.MacPool; +import org.ovirt.engine.core.common.businessentities.StoragePool; 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.dal.dbbroker.DbFacadeLocator; import org.ovirt.engine.core.dao.MacPoolDao; +import org.ovirt.engine.core.dao.StoragePoolDAO; @RunWith(MockitoJUnitRunner.class) public class MacPoolValidatorTest { @@ -29,6 +31,9 @@ @Mock private MacPoolDao macPoolDaoMock; + @Mock + private StoragePoolDAO storagePoolDao; + @Before public void setUp() throws Exception { macPool = new MacPool(); @@ -37,6 +42,7 @@ DbFacadeLocator.setDbFacade(dbFacadeMock); when(dbFacadeMock.getMacPoolDao()).thenReturn(macPoolDaoMock); + when(dbFacadeMock.getStoragePoolDao()).thenReturn(storagePoolDao); } @Test @@ -129,7 +135,10 @@ @Test public void testNotRemovingUsedPoolRecordIsUsed() throws Exception { macPool.setId(Guid.newGuid()); - when(macPoolDaoMock.getDcUsageCount(eq(macPool.getId()))).thenReturn(1); + final StoragePool storagePool = new StoragePool(); + storagePool.setName("storagePool"); + when(storagePoolDao.getAllDataCentersByMacPoolId(macPool.getId())) + .thenReturn(Collections.singletonList(storagePool)); final ValidationResult validationResult = new MacPoolValidator(macPool).notRemovingUsedPool(); Assert.assertThat(validationResult, @@ -139,7 +148,9 @@ @Test public void testNotRemovingUsedPoolRecordNotUsed() throws Exception { macPool.setId(Guid.newGuid()); - when(macPoolDaoMock.getDcUsageCount(eq(macPool.getId()))).thenReturn(0); + + when(storagePoolDao.getAllDataCentersByMacPoolId(macPool.getId())) + .thenReturn(Collections.<StoragePool>emptyList()); final ValidationResult validationResult = new MacPoolValidator(macPool).notRemovingUsedPool(); Assert.assertThat(validationResult, isValid()); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index ab947f9..dfb3354 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -5,6 +5,7 @@ Unassigned, VAR__TYPE__HOST, VAR__ENTITIES__HOSTS, + VAR__ENTITIES__STORAGE_POOLS, VAR__TYPE__VM, VAR__ENTITIES__VMS, VAR__TYPE__QUOTA, diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDao.java index f512135..6222065 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDao.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDao.java @@ -9,8 +9,6 @@ MacPool getDefaultPool(); - int getDcUsageCount(Guid id); - List<String> getAllMacsForMacPool(Guid macPoolId); MacPool getByDataCenterId(Guid id); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDaoDbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDaoDbFacadeImpl.java index c3451f8..d7a428c 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDaoDbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/MacPoolDaoDbFacadeImpl.java @@ -54,13 +54,6 @@ } @Override - public int getDcUsageCount(Guid macPoolId) { - return getCallsHandler().executeRead("GetMacPoolUsageCountById", - getIntegerMapper(), - createIdParameterMapper(macPoolId)); - } - - @Override public List<String> getAllMacsForMacPool(Guid macPoolId) { return getCallsHandler().executeReadList("GetAllMacsByMacPoolId", getStringMapper(), 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 0e66ac4..4535713 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 @@ -139,4 +139,11 @@ * @return the list storage types that are existing in the pool. */ List<StorageType> getStorageTypesInPool(Guid storagePoolId); + + /** + * @param macPoolId + * @return all StoragePool records bound to given macPoolId. + */ + List<StoragePool> getAllDataCentersByMacPoolId(Guid macPoolId); + } 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 a74fba6..44efb54 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 @@ -252,4 +252,11 @@ return StorageType.forValue(rs.getInt(1)); } }; + + @Override + public List<StoragePool> getAllDataCentersByMacPoolId(Guid macPoolId) { + return getCallsHandler().executeReadList("GetAllDataCentersByMacPoolId", + new StoragePoolRawMapper(), + getCustomMapSqlParameterSource().addValue("id", macPoolId)); + } } diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 877f31e..972780d 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -9,7 +9,8 @@ IRS_PROTOCOL_ERROR=Storage Manager protocol error. IRS_RESPONSE_ERROR=Storage Manager response error. MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. -ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. ${type} cannot be removed, because some data center is still using it. +ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. Several ${entities} (${ENTITIES_USING_NETWORK_COUNTER}) are using this ${type}:\n${DATACENTERS_USING_MAC_POOL}\n - Please remove it from all ${entities} that are using it and try again. +VAR__ENTITIES__STORAGE_POOLS=$entities Data Centers ACTION_TYPE_FAILED_CANNOT_REMOVE_DEFAULT_MAC_POOL=Cannot ${action} ${type}. Default ${type} cannot be removed. ACTION_TYPE_FAILED_MAC_POOL_DOES_NOT_EXIST=Cannot ${action} ${type}. ${type} does not exist. ACTION_TYPE_FAILED_CHANGING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Changing default ${type} is not supported. diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/MacPoolDaoTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/MacPoolDaoTest.java index 0980401..11cfdea 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/MacPoolDaoTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/MacPoolDaoTest.java @@ -24,17 +24,6 @@ } @Test - public void testGetDefaultMacPoolDcUsageCount() throws Exception { - final int dcUsageCount = dao.getDcUsageCount(FixturesTool.DEFAULT_MAC_POOL_ID); - assertThat(dcUsageCount, is(6)); - } - @Test - public void testGetNonDefaultMacPoolDcUsageCount() throws Exception { - final int dcUsageCount = dao.getDcUsageCount(FixturesTool.NON_DEFAULT_MAC_POOL); - assertThat(dcUsageCount, is(1)); - } - - @Test public void testGetAllMacsForMacPool() throws Exception { final List<String> allMacsForMacPool = dao.getAllMacsForMacPool(FixturesTool.NON_DEFAULT_MAC_POOL); diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java index 98e4a85..dc37408 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java @@ -1,9 +1,11 @@ package org.ovirt.engine.core.dao; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import java.util.ArrayList; @@ -13,8 +15,8 @@ import org.junit.Test; import org.ovirt.engine.core.common.businessentities.QuotaEnforcementTypeEnum; import org.ovirt.engine.core.common.businessentities.StorageFormatType; -import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.StoragePool; +import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; @@ -349,6 +351,11 @@ assertTrue("Number of storage types in a non existing pool returned results", storageTypes.isEmpty()); } + @Test + public void testGetAllDataCentersByMacPoolId() { + assertThat(dao.getAllDataCentersByMacPoolId(FixturesTool.DEFAULT_MAC_POOL_ID).size(), is(6)); + } + private void assertStorageTypes(List<StorageType> typesList, StorageType... expectedTypes) { assertEquals("Number of storage types different than expected storage type in the pool", typesList.size(), expectedTypes.length); diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 737841a..9e20910 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -37,9 +37,12 @@ @DefaultStringValue("Not enough MAC addresses left in MAC Address Pool.") String MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES(); - @DefaultStringValue("Cannot ${action} ${type}. Mac pool cannot be removed, because some DataCenter is still using it.") + @DefaultStringValue("ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. Several ${entities} (${ENTITIES_USING_NETWORK_COUNTER}) are using this ${type}:\n${DATACENTERS_USING_MAC_POOL}\n - Please remove it from all ${entities} that are using it and try again.") String ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL(); + @DefaultStringValue("$entities Data Centers") + String VAR__ENTITIES__STORAGE_POOLS(); + @DefaultStringValue("Cannot ${action} ${type}. Default MAC Pool cannot be removed.") String ACTION_TYPE_FAILED_CANNOT_REMOVE_DEFAULT_MAC_POOL(); diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 0003f6f..69955ea 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -9,7 +9,8 @@ IRS_PROTOCOL_ERROR=Storage Manager protocol error. IRS_RESPONSE_ERROR=Storage Manager response error. MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. -ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. ${type} cannot be removed, because some data center is still using it. +ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. Several ${entities} (${DATACENTERS_USING_MAC_POOL_COUNTER}) are using this ${type}:\n${DATACENTERS_USING_MAC_POOL}\n - Please remove it from all ${entities} that are using it and try again. +VAR__ENTITIES__STORAGE_POOLS=$entities Data Centers ACTION_TYPE_FAILED_CANNOT_REMOVE_DEFAULT_MAC_POOL=Cannot ${action} ${type}. Default ${type} cannot be removed. ACTION_TYPE_FAILED_MAC_POOL_DOES_NOT_EXIST=Cannot ${action} ${type}. ${type} does not exist. ACTION_TYPE_FAILED_CHANGING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Changing default ${type} is not supported. diff --git a/packaging/dbscripts/mac_pools_sp.sql b/packaging/dbscripts/mac_pools_sp.sql index 406a44f..3fb04e0 100644 --- a/packaging/dbscripts/mac_pools_sp.sql +++ b/packaging/dbscripts/mac_pools_sp.sql @@ -91,17 +91,6 @@ END; $procedure$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION GetMacPoolUsageCountById(v_id UUID) - RETURNS SETOF BIGINT STABLE -AS $procedure$ -BEGIN - RETURN QUERY SELECT count(*) - FROM storage_pool sp - WHERE sp.mac_pool_id=v_id; - -END; $procedure$ -LANGUAGE plpgsql; - CREATE OR REPLACE FUNCTION GetAllMacsByMacPoolId(v_id UUID) RETURNS SETOF VARCHAR STABLE AS $procedure$ diff --git a/packaging/dbscripts/storages_sp.sql b/packaging/dbscripts/storages_sp.sql index 8ce7d4d..a09c11c 100644 --- a/packaging/dbscripts/storages_sp.sql +++ b/packaging/dbscripts/storages_sp.sql @@ -939,3 +939,15 @@ WHERE storage_domain_id = v_storage_domain_id; END; $procedure$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION GetAllDataCentersByMacPoolId(v_id UUID) + RETURNS SETOF storage_pool STABLE +AS $procedure$ +BEGIN + RETURN QUERY SELECT sp.* + FROM storage_pool sp + WHERE sp.mac_pool_id=v_id; + +END; $procedure$ +LANGUAGE plpgsql; -- To view, visit http://gerrit.ovirt.org/30663 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I647420d7cc43853eddd2b17091cd627b9088a34d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
