Moti Asayag has uploaded a new change for review. Change subject: engine: Introduce VnicProfileValidator ......................................................................
engine: Introduce VnicProfileValidator The patch introduces VnicProfileValidator to act as vnic profile commands validator. Change-Id: Ia77ee44ad8af055ef85d92ad1daa25298339953d Signed-off-by: Moti Asayag <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVnicProfileCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVnicProfileCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVnicProfileCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VnicProfileValidator.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VnicProfileValidatorTest.java 5 files changed, 295 insertions(+), 114 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/88/16888/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVnicProfileCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVnicProfileCommand.java index 84cdc74..d9dd717 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVnicProfileCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVnicProfileCommand.java @@ -5,11 +5,10 @@ import org.ovirt.engine.core.bll.network.cluster.NetworkHelper; import org.ovirt.engine.core.bll.utils.PermissionSubject; +import org.ovirt.engine.core.bll.validator.VnicProfileValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.VnicProfileParameters; -import org.ovirt.engine.core.common.businessentities.Entities; -import org.ovirt.engine.core.common.businessentities.network.VnicProfile; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.validation.group.CreateEntity; import org.ovirt.engine.core.compat.Guid; @@ -22,23 +21,10 @@ @Override protected boolean canDoAction() { - if (getVnicProfile() == null) { - // TODO: Add CDA message 'profile not found' - return false; - } - - if (getNetworkDAO().get(getVnicProfile().getNetworkId()) == null) { - // TODO: Add CDA message 'network not found' - return false; - } - - List<VnicProfile> profiles = getVnicProfileDao().getAllForNetwork(getVnicProfile().getNetworkId()); - if (Entities.entitiesByName(profiles).containsKey(getVnicProfile().getName())) { - // Add CDA message 'vnic profile name already in use' - return false; - } - - return true; + VnicProfileValidator validator = new VnicProfileValidator(getVnicProfile()); + return validate(validator.vnicProfileIsSet()) + && validate(validator.networkExists()) + && validate(validator.vnicProfileNameNotUsed()); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVnicProfileCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVnicProfileCommand.java index 2a62491..099d571 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVnicProfileCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVnicProfileCommand.java @@ -2,14 +2,11 @@ import java.util.List; +import org.ovirt.engine.core.bll.validator.VnicProfileValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.VnicProfileParameters; -import org.ovirt.engine.core.common.businessentities.VM; -import org.ovirt.engine.core.common.businessentities.VmTemplate; -import org.ovirt.engine.core.common.businessentities.network.VnicProfile; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.validation.group.RemoveEntity; -import org.ovirt.engine.core.utils.ReplacementUtils; public class RemoveVnicProfileCommand<T extends VnicProfileParameters> extends VnicProfileCommon<T> { @@ -19,32 +16,11 @@ @Override protected boolean canDoAction() { - if (getVnicProfile() == null) { - // TODO: Add CDA message 'profile not found' - return false; - } - - VnicProfile profile = getVnicProfileDao().get(getVnicProfile().getId()); - if (profile == null) { - // TODO: Add CDA message 'profile not found' - return false; - } - - List<VM> vms = getVmDAO().getAllForVnicProfile(getVnicProfile().getId()); - if (!vms.isEmpty()) { - // TODO: Add CDA message 'cannot modify vnic profiles network if there are VMs using the vnic profile - ReplacementUtils.replaceWithNameable("VMS_USING_VNIC_PROFILE", vms); - return false; - } - - List<VmTemplate> templates = getVmTemplateDAO().getAllForVnicProfile(getVnicProfile().getId()); - if (!templates.isEmpty()) { - // TODO: Add CDA message 'cannot modify vnic profiles network if there are templates using the vnic profile - ReplacementUtils.replaceWithNameable("TEMPLATES_USING_VNIC_PROFILE", templates); - return false; - } - - return true; + VnicProfileValidator validator = new VnicProfileValidator(getVnicProfile()); + return validate(validator.vnicProfileIsSet()) + && validate(validator.vnicProfileExists()) + && validate(validator.vnicProfileNotUsedByVms()) + && validate(validator.vnicProfileNotUsedByTemplates()); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVnicProfileCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVnicProfileCommand.java index 6d91130..f86da3f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVnicProfileCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVnicProfileCommand.java @@ -1,19 +1,12 @@ package org.ovirt.engine.core.bll.network.vm; -import java.util.ArrayList; import java.util.List; -import org.apache.commons.lang.ObjectUtils; +import org.ovirt.engine.core.bll.validator.VnicProfileValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.VnicProfileParameters; -import org.ovirt.engine.core.common.businessentities.Entities; -import org.ovirt.engine.core.common.businessentities.VM; -import org.ovirt.engine.core.common.businessentities.VMStatus; -import org.ovirt.engine.core.common.businessentities.network.Network; -import org.ovirt.engine.core.common.businessentities.network.VnicProfile; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.validation.group.UpdateEntity; -import org.ovirt.engine.core.utils.ReplacementUtils; public class UpdateVnicProfileCommand<T extends VnicProfileParameters> extends VnicProfileCommon<T> { @@ -23,47 +16,12 @@ @Override protected boolean canDoAction() { - if (getVnicProfile() == null) { - // TODO: Add CDA message 'profile not found' - return false; - } - - VnicProfile profile = getVnicProfileDao().get(getVnicProfile().getId()); - if (profile == null) { - // TODO: Add CDA message 'profile not found' - return false; - } - - Network network = getNetworkDAO().get(getVnicProfile().getNetworkId()); - if (network == null) { - // TODO: Add CDA message 'network not found' - return false; - } - - if (!ObjectUtils.equals(getVnicProfile().getNetworkId(), network.getId())) { - List<VM> vms = getVmDAO().getAllForVnicProfile(getVnicProfile().getId()); - List<VM> vmUsingProfile = new ArrayList<>(); - for (VM vm : vms) { - if (vm.getStatus() != VMStatus.Down) { - vmUsingProfile.add(vm); - } - } - - if (!vmUsingProfile.isEmpty()) { - // TODO: Add CDA message 'cannot modify vnic profiles network if there are VMs with status other than - // 'Down' - ReplacementUtils.replaceWithNameable("VMS_USING_VNIC_PROFILE", vmUsingProfile); - return false; - } - } - - List<VnicProfile> profiles = getVnicProfileDao().getAllForNetwork(getVnicProfile().getNetworkId()); - if (Entities.entitiesByName(profiles).containsKey(getVnicProfile().getName())) { - // Add CDA message 'vnic profile name already in use' - return false; - } - - return true; + VnicProfileValidator validator = new VnicProfileValidator(getVnicProfile()); + return validate(validator.vnicProfileIsSet()) + && validate(validator.vnicProfileExists()) + && validate(validator.networkExists()) + && validate(validator.vnicProfileNameNotUsed()) + && validate(validator.changingNetworkAllowed()); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VnicProfileValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VnicProfileValidator.java index c570979..1143195 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VnicProfileValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VnicProfileValidator.java @@ -6,6 +6,7 @@ import org.apache.commons.lang.ObjectUtils; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.Nameable; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.network.Network; @@ -20,6 +21,7 @@ private VnicProfile oldVnicProfile; private Network network; private List<VnicProfile> vnicProfiles; + private List<VM> vms; public VnicProfileValidator(VnicProfile vnicProfile) { this.vnicProfile = vnicProfile; @@ -31,7 +33,7 @@ public ValidationResult vnicProfileIsSet() { return vnicProfile == null - ? new ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS) + ? new ValidationResult(VdcBllMessages.VNIC_PROFILE_NOT_EXISTS) : ValidationResult.VALID; } @@ -49,7 +51,7 @@ public ValidationResult vnicProfileNameNotUsed() { for (VnicProfile profile : getVnicProfiles()) { - if (profile.getName().equals(vnicProfile.getName())) { + if (profile.getName().equals(vnicProfile.getName()) && !profile.getId().equals(vnicProfile.getId())) { return new ValidationResult(VdcBllMessages.VNIC_PROFILE_NAME_IN_USE); } } @@ -58,24 +60,37 @@ } public ValidationResult changingNetworkAllowed() { - if (!ObjectUtils.equals(vnicProfile.getNetworkId(), getOldVnicProfile().getNetworkId())) { - List<VM> vms = getDbFacade().getVmDao().getAllForVnicProfile(vnicProfile.getId()); - List<VM> vmUsingProfile = new ArrayList<>(); - for (VM vm : vms) { - if (vm.getStatus() != VMStatus.Down) { - vmUsingProfile.add(vm); - } - } + if (ObjectUtils.equals(vnicProfile.getNetworkId(), getOldVnicProfile().getNetworkId())) { + return ValidationResult.VALID; + } - if (!vmUsingProfile.isEmpty()) { - Collection<String> replacements = - ReplacementUtils.replaceWithNameable("ENTITIES_USING_VNIC_PROFILE", vmUsingProfile); - replacements.add(VdcBllMessages.VAR__ENTITIES__VMS.name()); - return new ValidationResult(VdcBllMessages.VNIC_PROFILE_IN_USE, replacements); + List<VM> vmUsingProfile = new ArrayList<>(); + for (VM vm : getVmsUsingProfile()) { + if (vm.getStatus() != VMStatus.Down) { + vmUsingProfile.add(vm); } } - return ValidationResult.VALID; + return vnicProfileNotUsed(vmUsingProfile, VdcBllMessages.VAR__ENTITIES__VMS); + } + + public ValidationResult vnicProfileNotUsedByVms() { + return vnicProfileNotUsed(getVmsUsingProfile(), VdcBllMessages.VAR__ENTITIES__VMS); + } + + public ValidationResult vnicProfileNotUsedByTemplates() { + return vnicProfileNotUsed(getDbFacade().getVmTemplateDao().getAllForVnicProfile(vnicProfile.getId()), + VdcBllMessages.VAR__ENTITIES__VM_TEMPLATES); + } + + protected ValidationResult vnicProfileNotUsed(List<? extends Nameable> entities, VdcBllMessages entitiesReplacement) { + if (entities.isEmpty()) { + return ValidationResult.VALID; + } + + Collection<String> replacements = ReplacementUtils.replaceWithNameable("ENTITIES_USING_VNIC_PROFILE", entities); + replacements.add(entitiesReplacement.name()); + return new ValidationResult(VdcBllMessages.VNIC_PROFILE_IN_USE, replacements); } protected Network getNetwork() { @@ -98,7 +113,15 @@ if (oldVnicProfile == null) { oldVnicProfile = getDbFacade().getVnicProfileDao().get(vnicProfile.getId()); } + return oldVnicProfile; } + protected List<VM> getVmsUsingProfile() { + if (vms == null) { + vms = getDbFacade().getVmDao().getAllForVnicProfile(vnicProfile.getId()); + } + + return vms; + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VnicProfileValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VnicProfileValidatorTest.java new file mode 100644 index 0000000..5c6dfe4 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VnicProfileValidatorTest.java @@ -0,0 +1,238 @@ +package org.ovirt.engine.core.bll.validator; + +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +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.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.businessentities.VMStatus; +import org.ovirt.engine.core.common.businessentities.VmTemplate; +import org.ovirt.engine.core.common.businessentities.network.Network; +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.dao.VmDAO; +import org.ovirt.engine.core.dao.VmTemplateDAO; +import org.ovirt.engine.core.dao.network.NetworkDao; +import org.ovirt.engine.core.dao.network.VnicProfileDao; + +@RunWith(MockitoJUnitRunner.class) +public class VnicProfileValidatorTest { + + private final String NAMEABLE_NAME = "nameable"; + private final String DEFAULT_VNIC_PROFILE_NAME = "myvnicprofile"; + private final String OTHER_VNIC_PROFILE_NAME = "myothervnicprofile"; + private final Guid DEFAULT_GUID = Guid.newGuid(); + private final Guid OTHER_GUID = Guid.newGuid(); + + @Mock + private DbFacade dbFacade; + + @Mock + private VnicProfileDao vnicProfileDao; + + @Mock + private NetworkDao networkDao; + + @Mock + private VmDAO vmDao; + + @Mock + private VnicProfile vnicProfile; + + @Mock + private Network network; + + private List<VnicProfile> vnicProfiles = new ArrayList<>(); + + private VnicProfileValidator validator; + + @Before + public void setup() { + + // spy on attempts to access the database + validator = spy(new VnicProfileValidator(vnicProfile)); + doReturn(dbFacade).when(validator).getDbFacade(); + + // mock some commonly used DAOs + when(dbFacade.getVnicProfileDao()).thenReturn(vnicProfileDao); + when(dbFacade.getNetworkDao()).thenReturn(networkDao); + when(dbFacade.getVmDao()).thenReturn(vmDao); + + // mock their getters + when(vnicProfileDao.get(any(Guid.class))).thenReturn(vnicProfile); + when(vnicProfileDao.getAllForNetwork(any(Guid.class))).thenReturn(vnicProfiles); + } + + @Test + public void vnicProfileSet() throws Exception { + assertThat(validator.vnicProfileIsSet(), isValid()); + } + + @Test + public void vnicProfileNull() throws Exception { + validator = new VnicProfileValidator(null); + assertThat(validator.vnicProfileIsSet(), failsWith(VdcBllMessages.VNIC_PROFILE_NOT_EXISTS)); + } + + @Test + public void vnicProfileExists() throws Exception { + assertThat(validator.vnicProfileExists(), isValid()); + } + + @Test + public void vnicProfileDoesNotExist() throws Exception { + when(vnicProfileDao.get(any(Guid.class))).thenReturn(null); + assertThat(validator.vnicProfileExists(), failsWith(VdcBllMessages.VNIC_PROFILE_NOT_EXISTS)); + } + + @Test + public void networkExists() throws Exception { + when(networkDao.get(any(Guid.class))).thenReturn(network); + assertThat(validator.networkExists(), isValid()); + } + + @Test + public void networkDoesntExist() throws Exception { + when(networkDao.get(any(Guid.class))).thenReturn(null); + assertThat(validator.networkExists(), failsWith(VdcBllMessages.NETWORK_NOT_EXISTS)); + } + + private void vnicProfileAvailableTest(Matcher<ValidationResult> matcher, List<VnicProfile> vnicProfiles) { + this.vnicProfiles.addAll(vnicProfiles); + when(vnicProfile.getName()).thenReturn(DEFAULT_VNIC_PROFILE_NAME); + when(vnicProfile.getId()).thenReturn(DEFAULT_GUID); + + assertThat(validator.vnicProfileNameNotUsed(), matcher); + } + + private List<VnicProfile> getSingletonNamedVnicProfileList(String vnicProfileName, Guid vnicProfileId) { + VnicProfile vnicProfile = mock(VnicProfile.class); + when(vnicProfile.getName()).thenReturn(vnicProfileName); + when(vnicProfile.getId()).thenReturn(vnicProfileId); + return Collections.singletonList(vnicProfile); + } + + @Test + public void vnicProfileNameNoVnicProfiles() throws Exception { + vnicProfileAvailableTest(isValid(), Collections.<VnicProfile> emptyList()); + } + + @Test + public void vnicProfileNameAvailable() throws Exception { + vnicProfileAvailableTest(isValid(), getSingletonNamedVnicProfileList(OTHER_VNIC_PROFILE_NAME, OTHER_GUID)); + } + + @Test + public void vnicProfileNameTakenByDifferentVnicProfile() throws Exception { + vnicProfileAvailableTest(failsWith(VdcBllMessages.VNIC_PROFILE_NAME_IN_USE), + getSingletonNamedVnicProfileList(DEFAULT_VNIC_PROFILE_NAME, OTHER_GUID)); + } + + @Test + public void vnicProfileNameTakenCaseSensitivelyByDifferentVnicProfile() throws Exception { + vnicProfileAvailableTest(isValid(), + getSingletonNamedVnicProfileList(DEFAULT_VNIC_PROFILE_NAME.toUpperCase(), OTHER_GUID)); + } + + @Test + public void vnicProfileNameTakenBySameVnicProfile() throws Exception { + vnicProfileAvailableTest(isValid(), + getSingletonNamedVnicProfileList(DEFAULT_VNIC_PROFILE_NAME, DEFAULT_GUID)); + } + + private Matcher<ValidationResult> failsWithVnicProfileInUse() { + return failsWith(VdcBllMessages.VNIC_PROFILE_IN_USE); + } + + private void changingNetworkAllowedTest(Matcher<ValidationResult> matcher, List<VM> vms) { + when(vmDao.getAllForVnicProfile(any(Guid.class))).thenReturn(vms); + assertThat(validator.changingNetworkAllowed(), matcher); + } + + @Test + public void changingNetworkAllowedNoVmsUsingProfile() throws Exception { + changingNetworkAllowedTest(isValid(), Collections.<VM> emptyList()); + } + + @Test + public void changingNetworkAllowed() throws Exception { + mockVnicProfileNetworkChange(); + changingNetworkAllowedTest(isValid(), Collections.singletonList(mockVmWithStatus(VMStatus.Down))); + } + + @Test + public void changingNetworkNotAllowed() throws Exception { + mockVnicProfileNetworkChange(); + changingNetworkAllowedTest(failsWithVnicProfileInUse(), + Collections.singletonList(mockVmWithStatus(VMStatus.Up))); + } + + private void mockVnicProfileNetworkChange() { + VnicProfile vnicProfile = mock(VnicProfile.class); + when(this.vnicProfile.getNetworkId()).thenReturn(Guid.newGuid()); + when(vnicProfile.getNetworkId()).thenReturn(Guid.newGuid()); + when(vnicProfileDao.get(any(Guid.class))).thenReturn(vnicProfile); + } + + private VM mockVmWithStatus(VMStatus status) { + VM vm = mock(VM.class); + when(vm.getStatus()).thenReturn(status); + when(vm.getName()).thenReturn(NAMEABLE_NAME); + return vm; + } + + private void vnicProfileNotUsedByVmsTest(Matcher<ValidationResult> matcher, List<VM> vms) { + when(vmDao.getAllForVnicProfile(any(Guid.class))).thenReturn(vms); + assertThat(validator.vnicProfileNotUsedByVms(), matcher); + } + + @Test + public void vnicProfileNotInUseByVms() throws Exception { + vnicProfileNotUsedByVmsTest(isValid(), Collections.<VM> emptyList()); + } + + @Test + public void vnicProfileInUseByVms() throws Exception { + VM vm = mock(VM.class); + when(vm.getName()).thenReturn(NAMEABLE_NAME); + vnicProfileNotUsedByVmsTest(failsWithVnicProfileInUse(), Collections.singletonList(vm)); + } + + private void vnicProfileNotUsedByTemplatesTest(Matcher<ValidationResult> matcher, List<VmTemplate> templates) { + VmTemplateDAO templateDao = mock(VmTemplateDAO.class); + when(templateDao.getAllForVnicProfile(any(Guid.class))).thenReturn(templates); + when(dbFacade.getVmTemplateDao()).thenReturn(templateDao); + assertThat(validator.vnicProfileNotUsedByTemplates(), matcher); + } + + @Test + public void vnicProfileNotInUseByTemplates() throws Exception { + vnicProfileNotUsedByTemplatesTest(isValid(), Collections.<VmTemplate> emptyList()); + } + + @Test + public void vnicProfileInUseByTemplates() throws Exception { + VmTemplate template = mock(VmTemplate.class); + when(template.getName()).thenReturn(NAMEABLE_NAME); + + vnicProfileNotUsedByTemplatesTest(failsWithVnicProfileInUse(), Collections.singletonList(template)); + } +} -- To view, visit http://gerrit.ovirt.org/16888 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia77ee44ad8af055ef85d92ad1daa25298339953d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
