Arik Hadas has uploaded a new change for review. Change subject: webadmin: prevent several imported VMs with same name ......................................................................
webadmin: prevent several imported VMs with same name This patch prevents the user from assigning the same name for several imported VMs. if the user tries to assign a name that was already been assigned for a different VM that is going to be imported, the name field will be marked as invalid with a proper error message. When the user tried to import multiple VMs that already exist, he could choose a different name for each of them. when doing so, there was no validation that ensures the user don't assign the same name for several VMs, so the backend got that bunch of VMs to import with the given names and created them without checking that each name is unique (because it's done as multiple actions, thus not checking canDoAction on each of them), and the result is that there were multiple VMs created with the same name. So this patch prevents this situation from happening by ensuring that the UI don't pass several VMs with the same name to import to the backend, by adding a proper validation for the name field of the to-be-imported VM which checks that it's unique. Change-Id: I69eae7350ae840a06b440003c436f7adc0b032cd Bug-Url: https://bugzilla.redhat.com/907132 Signed-off-by: Arik Hadas <[email protected]> --- M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NotInCollectionValidation.java M frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java 3 files changed, 63 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/03/12203/1 diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java index 83a8d67..9ce2c5f 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java @@ -1,10 +1,13 @@ package org.ovirt.engine.ui.uicommonweb.models.storage; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.ovirt.engine.core.common.action.ImportVmParameters; import org.ovirt.engine.core.common.action.RemoveVmFromImportExportParamenters; @@ -42,16 +45,20 @@ import org.ovirt.engine.ui.uicommonweb.validation.IValidation; import org.ovirt.engine.ui.uicommonweb.validation.LengthValidation; import org.ovirt.engine.ui.uicommonweb.validation.NotEmptyValidation; +import org.ovirt.engine.ui.uicommonweb.validation.NotInCollectionValidation; +import org.ovirt.engine.ui.uicommonweb.validation.ValidationResult; import org.ovirt.engine.ui.uicompat.ConstantsManager; import org.ovirt.engine.ui.uicompat.FrontendMultipleActionAsyncResult; import org.ovirt.engine.ui.uicompat.IFrontendMultipleActionAsyncCallback; import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs; - public class VmBackupModel extends ManageBackupModel { private VmAppListModel privateAppListModel; protected List<Object> objectsToClone; + /** used to save the names that were assigned for VMs which are going + * to be created using import in case of choosing multiple VM imports */ + protected Set<String> assignedVmNames = new HashSet<String>(); protected Map<Guid, Object> cloneObjectMap; protected ImportVmModel importModel; @@ -222,8 +229,9 @@ } private void executeImportClone() { - //TODO: support running numbers (for suffix) + // TODO: support running numbers (for suffix) if (objectsToClone.size() == 0) { + assignedVmNames.clear(); executeImport(); return; } @@ -242,7 +250,6 @@ entity.getCommands().add(command); setConfirmWindow(entity); - } private void onClone() { @@ -280,11 +287,9 @@ protected void setObjectName(Object object, String name, boolean isSuffix) { VM vm = ((ImportVmData) object).getVm(); - if (isSuffix) { - vm.setName(vm.getName() + name); - } else { - vm.setName(name); - } + String nameForTheClonedVm = isSuffix ? vm.getName() + name : name; + vm.setName(nameForTheClonedVm); + assignedVmNames.add(nameForTheClonedVm); } protected boolean validateSuffix(String suffix, EntityModel entityModel) { @@ -316,7 +321,8 @@ .getMessages() .newNameWithSuffixCannotContainBlankOrSpecialChars(length); }; - } + }, + new AlreadyAssignedVmNameValidator(assignedVmNames) }); if (!temp.getIsValid()) { entity.setInvalidityReasons(temp.getInvalidityReasons()); @@ -334,7 +340,7 @@ ArrayList<VdcActionParametersBase> prms = new ArrayList<VdcActionParametersBase>(); for (Object item : importModel.getItems()) { - VM vm = ((ImportVmData)item).getVm(); + VM vm = ((ImportVmData) item).getVm(); ImportVmParameters prm = new ImportVmParameters(vm, getEntity().getId(), Guid.Empty, importModel.getStoragePool().getId(), @@ -421,7 +427,9 @@ .importVirtualMachinesTitle()); confirmModel.setHashName("import_virtual_machine"); //$NON-NLS-1$ importedVms = StringHelper.trimEnd(importedVms.trim(), ','); - confirmModel.setMessage(ConstantsManager.getInstance().getMessages().importProcessHasBegunForVms(importedVms)); + confirmModel.setMessage(ConstantsManager.getInstance() + .getMessages() + .importProcessHasBegunForVms(importedVms)); UICommand tempVar2 = new UICommand("CancelConfirm", //$NON-NLS-1$ vmBackupModel); tempVar2.setTitle(ConstantsManager.getInstance().getConstants().close()); @@ -514,4 +522,19 @@ return "VmBackupModel"; //$NON-NLS-1$ } + private class AlreadyAssignedVmNameValidator extends NotInCollectionValidation { + + public AlreadyAssignedVmNameValidator(Collection<?> collection) { + super(collection); + } + + @Override + public ValidationResult Validate(Object value) { + ValidationResult result = super.Validate(value); + if (!result.getSuccess()) { + result.getReasons().add(ConstantsManager.getInstance().getConstants().alreadyAssignedClonedVmName()); + } + return result; + } + } } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NotInCollectionValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NotInCollectionValidation.java new file mode 100644 index 0000000..a53ac73 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NotInCollectionValidation.java @@ -0,0 +1,26 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import java.util.Collection; + +/** + * Validator that returns true if the given object is contained + * in a given collection, and false otherwise + * + * Note: this validator is abstract as it doesn't return any + * reason when the result is false + */ +public abstract class NotInCollectionValidation implements IValidation { + + private Collection<?> collection; + + public NotInCollectionValidation(Collection<?> collection) { + this.collection = collection; + } + + @Override + public ValidationResult Validate(Object value) { + ValidationResult result = new ValidationResult(); + result.setSuccess(!collection.contains(value)); + return result; + } +} diff --git a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java index a8cd7cc..8f0bd71 100644 --- a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java +++ b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java @@ -619,6 +619,9 @@ @DefaultStringValue("Name can contain only alphanumeric, '.', '_' or '-' characters, and optionally one sequence of '" + VmPool.MASK_CHARACTER + "' to specify mask for the VM indexes") String poolNameValidationMsg(); + @DefaultStringValue("This name was already assigned to another cloned Virtual Machine.") + String alreadyAssignedClonedVmName(); + @DefaultStringValue("UTF characters are not allowed.") String nonUtfValidationMsg(); -- To view, visit http://gerrit.ovirt.org/12203 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I69eae7350ae840a06b440003c436f7adc0b032cd Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
