Maor Lipchuk has uploaded a new change for review. Change subject: core: Remove redundant Vds call at Add existingFileStorageDomain ......................................................................
core: Remove redundant Vds call at Add existingFileStorageDomain Remove redundant getStorageDomainsList VDS call. The call was used to know if the Storage Exists in the VDSM, but we can also know that from the next VDS command of getStorageDomainInfo. Changing the test accordingly Change-Id: I2294f83eb204cab44b6a52e7c7a763bf107aaa37 Bug-Url: https://bugzilla.redhat.com/1157240 Signed-off-by: Maor Lipchuk <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommand.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommandTest.java 2 files changed, 163 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/08/35108/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommand.java index 4e4092d..00a3cd8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommand.java @@ -12,7 +12,6 @@ import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.HSMGetStorageDomainInfoVDSCommandParameters; -import org.ovirt.engine.core.common.vdscommands.HSMGetStorageDomainsListVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; @@ -59,23 +58,22 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); } - List<Guid> storageIds = (ArrayList<Guid>) runVdsCommand(VDSCommandType.HSMGetStorageDomainsList, - new HSMGetStorageDomainsListVDSCommandParameters(getVdsId(), Guid.Empty, getStorageDomain() - .getStorageType(), getStorageDomain().getStorageDomainType(), "") - ).getReturnValue(); - - if (!storageIds.contains(getStorageDomain().getId())) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST); - } - Pair<StorageDomainStatic, Guid> domainFromIrs = (Pair<StorageDomainStatic, Guid>) runVdsCommand( VDSCommandType.HSMGetStorageDomainInfo, new HSMGetStorageDomainInfoVDSCommandParameters(getVdsId(), getStorageDomain().getId()) ).getReturnValue(); + if (domainFromIrs == null) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); + } return concreteCheckExistingStorageDomain(domainFromIrs); } + protected Pair<StorageDomainStatic, Guid> executeHSMGetStorageDomainInfo(HSMGetStorageDomainInfoVDSCommandParameters parameters) { + return (Pair<StorageDomainStatic, Guid>) runVdsCommand(VDSCommandType.HSMGetStorageDomainInfo, parameters).getReturnValue(); + } + + protected boolean concreteCheckExistingStorageDomain(Pair<StorageDomainStatic, Guid> domain) { boolean returnValue = false; StorageDomainStatic domainFromIrs = domain.getFirst(); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommandTest.java new file mode 100644 index 0000000..e3c53f3 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddExistingFileStorageDomainCommandTest.java @@ -0,0 +1,155 @@ +package org.ovirt.engine.core.bll.storage; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.CanDoActionTestUtils; +import org.ovirt.engine.core.common.action.StorageDomainManagementParameter; +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.StoragePool; +import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.VDSStatus; +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.utils.Pair; +import org.ovirt.engine.core.common.vdscommands.HSMGetStorageDomainInfoVDSCommandParameters; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.Version; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.StorageDomainStaticDAO; +import org.ovirt.engine.core.dao.StoragePoolDAO; +import org.ovirt.engine.core.dao.VdsDAO; +import org.ovirt.engine.core.utils.MockConfigRule; + +@RunWith(MockitoJUnitRunner.class) +public class AddExistingFileStorageDomainCommandTest { + + private AddExistingFileStorageDomainCommand<StorageDomainManagementParameter> command; + private StorageDomainManagementParameter parameters; + + private static final int SD_MAX_NAME_LENGTH = 50; + + @ClassRule + public static MockConfigRule mcr = new MockConfigRule( + mockConfig(ConfigValues.StorageDomainNameSizeLimit, SD_MAX_NAME_LENGTH), + mockConfig(ConfigValues.SupportedStorageFormats, Version.v3_4.toString(), "3"), + mockConfig(ConfigValues.SupportedStorageFormats, Version.v3_5.toString(), "3") + ); + + @Mock + private DbFacade dbFacade; + + @Mock + private VdsDAO vdsDAO; + + @Mock + private StoragePoolDAO storagePoolDAO; + + @Mock + private StorageDomainStaticDAO storageDomainStaticDAO; + + @Before + public void setUp() { + parameters = new StorageDomainManagementParameter(getStorageDomain()); + parameters.setVdsId(Guid.newGuid()); + parameters.setStoragePoolId(Guid.newGuid()); + command = spy(new AddExistingFileStorageDomainCommand<>(parameters)); + + command.setStoragePool(getStoragePool()); + + doReturn(dbFacade).when(command).getDbFacade(); + doReturn(vdsDAO).when(command).getVdsDAO(); + doReturn(storagePoolDAO).when(command).getStoragePoolDAO(); + doReturn(storageDomainStaticDAO).when(command).getStorageDomainStaticDAO(); + + doReturn(false).when(command).isStorageWithSameNameExists(); + + doNothing().when(command).addStorageDomainInDb(); + doNothing().when(command).updateStorageDomainDynamicFromIrs(); + + when(command.getVdsDAO().getAllForStoragePoolAndStatus(any(Guid.class), eq(VDSStatus.Up))).thenReturn(getHosts()); + when(command.getStoragePoolDAO().get(any(Guid.class))).thenReturn(getStoragePool()); + } + + @Test + public void testCandoPassSuccessfully() { + when(command.getStorageDomainStaticDAO().get(any(Guid.class))).thenReturn(null); + + StorageDomainStatic sdStatic = command.getStorageDomain().getStorageStaticData(); + doReturn(new Pair<>(sdStatic, sdStatic.getId())).when(command).executeHSMGetStorageDomainInfo( + any(HSMGetStorageDomainInfoVDSCommandParameters.class)); + + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); + } + + @Test + public void testAlreadyExistStorageDomain() { + when(command.getStorageDomainStaticDAO().get(any(Guid.class))).thenReturn(parameters.getStorageDomain()); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, + VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST); + } + + @Test + public void testNonExistingStorageDomain() { + when(command.getStorageDomainStaticDAO().get(any(Guid.class))).thenReturn(null); + + doReturn(null).when(command).executeHSMGetStorageDomainInfo( + any(HSMGetStorageDomainInfoVDSCommandParameters.class)); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, + VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); + } + + @Test + public void testSwitchStorageDomainType() { + when(command.getStorageDomainStaticDAO().get(any(Guid.class))).thenReturn(null); + + StorageDomainStatic sdStatic = command.getStorageDomain().getStorageStaticData(); + doReturn(new Pair<>(sdStatic, sdStatic.getId())).when(command).executeHSMGetStorageDomainInfo( + any(HSMGetStorageDomainInfoVDSCommandParameters.class)); + + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); + } + + + private static StorageDomainStatic getStorageDomain() { + StorageDomainStatic storageDomain = new StorageDomainStatic(); + storageDomain.setStorage(Guid.newGuid().toString()); + storageDomain.setStorageDomainType(StorageDomainType.Data); + storageDomain.setStorageFormat(StorageFormatType.V3); + return storageDomain; + } + + private static StoragePool getStoragePool() { + StoragePool storagePool = new StoragePool(); + storagePool.setId(Guid.newGuid()); + storagePool.setcompatibility_version(Version.v3_5); + return storagePool; + } + + private static List<VDS> getHosts() { + List<VDS> hosts = new ArrayList<>(); + VDS host = new VDS(); + host.setId(Guid.newGuid()); + host.setStatus(VDSStatus.Up); + hosts.add(host); + return hosts; + } +} -- To view, visit http://gerrit.ovirt.org/35108 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2294f83eb204cab44b6a52e7c7a763bf107aaa37 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Maor Lipchuk <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
