Tomas Jelinek has uploaded a new change for review. Change subject: webadmin: Add validation for create/edit pool (#849426) ......................................................................
webadmin: Add validation for create/edit pool (#849426) https://bugzilla.redhat.com/849426 The VM name length is limited. When the new pool is created, the names of the VMs in the pool is limited as well. When the limit is, say 6 chars, than when you create a pool with name like 'pool' with 10 VMs in it, the names of the VMs will be pool-1, pool-2,..., pool-10. But the pool-10 is 7 chars long and the creation of such a VM fails. This patch adds a validation to the new/edit pool dialog which prevents the user from doing this. The behavior of the validation is the following: New Pool: The name field gets validated with a message like: 'The max allowed name length is X for Y VMs in pool' Edit Pool: The 'Increase number of VMs in pool by' gets validated with a message like: 'The max allowed num of VMs is X when the length of the pool name is Y' Where the X is 'num of vms already assigned' + 'num of VMs by which the pool should be increased' Change-Id: I66f765ceb0567beb006d1ef4f45db3902ef95ea6 Signed-off-by: Tomas Jelinek <[email protected]> --- M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewPoolModelBehavior.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/DummyValidation.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidation.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NewPoolNameLengthValidation.java A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidation.java A frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidationTest.java A frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidationTest.java M frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Messages.java 11 files changed, 283 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/49/7649/1 diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java index 4a2ce72..b7751d5 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java @@ -5,9 +5,13 @@ import org.ovirt.engine.core.common.businessentities.DisplayType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmBase; +import org.ovirt.engine.core.common.businessentities.VmOsType; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.ui.uicommonweb.models.ListModel; +import org.ovirt.engine.ui.uicommonweb.validation.DummyValidation; +import org.ovirt.engine.ui.uicommonweb.validation.ExistingPoolNameLengthValidation; +import org.ovirt.engine.ui.uicommonweb.validation.IValidation; public class ExistingPoolModelBehavior extends PoolModelBehaviorBase { @@ -52,4 +56,19 @@ return null; } + @Override + protected IValidation[] getNumOfDesktopsSpecificValidations() { + return new IValidation[] { new ExistingPoolNameLengthValidation( + (String) getModel().getName().getEntity(), + getModel().getAssignedVms().AsConvertible().Integer() + + getModel().getNumOfDesktops().AsConvertible().Integer(), + (VmOsType) getModel().getOSType().getSelectedItem() + ) }; + } + + @Override + protected IValidation[] getPoolNameSpecificValidations() { + return new IValidation[] { new DummyValidation() }; + } + } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewPoolModelBehavior.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewPoolModelBehavior.java index 62a2f9e..c309c2a 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewPoolModelBehavior.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewPoolModelBehavior.java @@ -4,9 +4,13 @@ import org.ovirt.engine.core.common.businessentities.DisplayType; import org.ovirt.engine.core.common.businessentities.VmBase; +import org.ovirt.engine.core.common.businessentities.VmOsType; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.ui.uicommonweb.Linq; import org.ovirt.engine.ui.uicommonweb.models.ListModel; +import org.ovirt.engine.ui.uicommonweb.validation.DummyValidation; +import org.ovirt.engine.ui.uicommonweb.validation.IValidation; +import org.ovirt.engine.ui.uicommonweb.validation.NewPoolNameLengthValidation; public class NewPoolModelBehavior extends PoolModelBehaviorBase { @@ -34,4 +38,18 @@ return null; } + + @Override + protected IValidation[] getNumOfDesktopsSpecificValidations() { + return new IValidation[] { new DummyValidation() }; + } + + @Override + protected IValidation[] getPoolNameSpecificValidations() { + return new IValidation[] { new NewPoolNameLengthValidation( + (String) getModel().getName().getEntity(), + getModel().getNumOfDesktops().AsConvertible().Integer(), + (VmOsType) getModel().getOSType().getSelectedItem() + ) }; + } } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java index abb508d..9b3f935 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java @@ -306,22 +306,22 @@ @Override public boolean Validate() { - - // Revalidate name field. - // TODO: Make maximum characters value depend on number of desktops in pool. - // VmOsType os = (VmOsType) getModel().getOSType().getSelectedItem(); - boolean isNew = getModel().getIsNew(); int maxAllowedVms = getMaxVmsInPool(); int assignedVms = getModel().getAssignedVms().AsConvertible().Integer(); getModel().getNumOfDesktops().ValidateEntity( - new IValidation[] + + union(new IValidation[] { new NotEmptyValidation(), new LengthValidation(4), new IntegerValidation(isNew ? 1 : 0, isNew ? maxAllowedVms : maxAllowedVms - assignedVms) - }); + }, + + getNumOfDesktopsSpecificValidations() + ) + ); getModel().getPrestartedVms().ValidateEntity( new IValidation[] @@ -329,6 +329,10 @@ new NotEmptyValidation(), new IntegerValidation(0, assignedVms) }); + + if (getModel().getName().getIsValid()) { + getModel().getName().ValidateEntity(getPoolNameSpecificValidations()); + } getModel().setIsGeneralTabValid(getModel().getIsGeneralTabValid() && getModel().getName().getIsValid() @@ -343,4 +347,15 @@ && getModel().getPrestartedVms().getIsValid(); } + private IValidation[] union(IValidation[] array1, IValidation[] array2) { + IValidation[] arrayCombined = new IValidation[array1.length + array2.length]; + System.arraycopy(array1, 0, arrayCombined, 0, array1.length); + System.arraycopy(array2, 0, arrayCombined, array1.length, array2.length); + return arrayCombined; + } + + protected abstract IValidation[] getNumOfDesktopsSpecificValidations(); + + protected abstract IValidation[] getPoolNameSpecificValidations(); + } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java index 74c2b9f..92252be 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java @@ -1827,6 +1827,7 @@ getDescription().ValidateEntity(new IValidation[] { new AsciiOrNoneValidation() }); if (getOSType().getIsValid()) { + // TODO maybe get rid of this validation maybe VmOsType osType = (VmOsType) getOSType().getSelectedItem(); String nameExpr = "^[-\\w\\.]{1,"; //$NON-NLS-1$ diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/DummyValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/DummyValidation.java new file mode 100644 index 0000000..906f1dd --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/DummyValidation.java @@ -0,0 +1,12 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +public class DummyValidation implements IValidation { + + @Override + public ValidationResult Validate(Object value) { + ValidationResult res = new ValidationResult(); + res.setSuccess(true); + return res; + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidation.java new file mode 100644 index 0000000..ae40959 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidation.java @@ -0,0 +1,37 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import org.ovirt.engine.core.common.businessentities.VmOsType; +import org.ovirt.engine.ui.uicompat.ConstantsManager; + +public class ExistingPoolNameLengthValidation extends PoolNameLengthValidation { + + public ExistingPoolNameLengthValidation(String poolName, int numOfVmsInPool, VmOsType osType) { + super(poolName, numOfVmsInPool, osType); + } + + @Override + protected String getReason() { + return ConstantsManager.getInstance() + .getMessages() + .numOfVmsInPoolInvalod(generateMaxLength(), getPoolName().length()); + } + + private int generateMaxLength() { + return doGenerateMaxLength(getMaxNameLength(), getPoolName().length()); + } + + int doGenerateMaxLength(int maxNameLengt, int poolNameLength) { + int allowedLength = maxNameLengt - poolNameLength - 1; + StringBuilder sb = new StringBuilder(""); + + if (allowedLength == 0) { + return 0; + } + + for (int i = 0; i < allowedLength; i++) { + sb.append("9"); //$NON-NLS-1$ + } + + return Integer.parseInt(sb.toString()); + } +} diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NewPoolNameLengthValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NewPoolNameLengthValidation.java new file mode 100644 index 0000000..aa0d050 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/NewPoolNameLengthValidation.java @@ -0,0 +1,23 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import org.ovirt.engine.core.common.businessentities.VmOsType; +import org.ovirt.engine.ui.uicompat.ConstantsManager; + +public class NewPoolNameLengthValidation extends PoolNameLengthValidation { + + public NewPoolNameLengthValidation(String poolName, int numOfVmsInPool, VmOsType osType) { + super(poolName, numOfVmsInPool, osType); + } + + @Override + protected String getReason() { + return ConstantsManager.getInstance() + .getMessages() + .poolNameLengthInvalid(generateMaxLength(), getNumOfVmsInPool()); + } + + private int generateMaxLength() { + return getMaxNameLength() - getNumOfVmsInPoolLength() - 1; + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidation.java new file mode 100644 index 0000000..e7bc457 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidation.java @@ -0,0 +1,61 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import java.util.Arrays; + +import org.ovirt.engine.core.common.businessentities.VmOsType; +import org.ovirt.engine.ui.uicommonweb.DataProvider; +import org.ovirt.engine.ui.uicommonweb.models.vms.UnitVmModel; + +public class PoolNameLengthValidation implements IValidation { + + private String poolName; + + private int numOfVmsInPool; + + private VmOsType osType; + + public PoolNameLengthValidation(String poolName, int numOfVmsInPool, VmOsType osType) { + this.poolName = poolName; + this.numOfVmsInPool = numOfVmsInPool; + this.osType = osType; + } + + @Override + public ValidationResult Validate(Object value) { + int numOfVmsInPoolLengt = getNumOfVmsInPoolLength(); + + // the +1 is the '-' sign between the name of pool and the ID of the VM + boolean isOk = poolName.length() + numOfVmsInPoolLengt + 1 <= getMaxNameLength(); + + ValidationResult res = new ValidationResult(); + res.setSuccess(isOk); + if (!isOk) { + res.setReasons(Arrays.asList(getReason())); + } + + return res; + } + + protected int getNumOfVmsInPoolLength() { + return Integer.toString(numOfVmsInPool).length(); + } + + protected String getPoolName() { + return poolName; + } + + protected int getNumOfVmsInPool() { + return numOfVmsInPool; + } + + protected int getMaxNameLength() { + return DataProvider.IsWindowsOsType(osType) ? + UnitVmModel.WINDOWS_VM_NAME_MAX_LIMIT : + UnitVmModel.NON_WINDOWS_VM_NAME_MAX_LIMIT; + } + + protected String getReason() { + return ""; + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidationTest.java b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidationTest.java new file mode 100644 index 0000000..da29cf4 --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/ExistingPoolNameLengthValidationTest.java @@ -0,0 +1,32 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.assertThat; + +import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.VmOsType; + +public class ExistingPoolNameLengthValidationTest { + + @Test + public void getPoolName_noVmsAlloved() { + assertGeneratesCorrect(5, 4, 0); + } + + @Test + public void getPoolName_9VmsAllowed() { + assertGeneratesCorrect(5, 3, 9); + } + + @Test + public void getPoolName_99VmsAllowed() { + assertGeneratesCorrect(5, 2, 99); + } + + private void assertGeneratesCorrect(int maxNameLengt, int poolNameLength, int expectedMaxNumOfVms) { + ExistingPoolNameLengthValidation validation = new ExistingPoolNameLengthValidation("", 0, VmOsType.Other); + int res = validation.doGenerateMaxLength(maxNameLengt, poolNameLength); + assertThat(res, is(equalTo(expectedMaxNumOfVms))); + } +} diff --git a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidationTest.java b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidationTest.java new file mode 100644 index 0000000..7a0d77d --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/PoolNameLengthValidationTest.java @@ -0,0 +1,52 @@ +package org.ovirt.engine.ui.uicommonweb.validation; + +import static org.junit.Assert.assertThat; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsEqual.equalTo; +import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.VmOsType; + +public class PoolNameLengthValidationTest { + + @Test + public void validate_nameItselfTooLong() { + assertValidationWorks(200, 1, false); + } + + @Test + public void validate_farNotOk() { + assertValidationWorks(14, 500, false); + } + + @Test + public void validate_farOk() { + assertValidationWorks(5, 2, true); + } + + @Test + public void validate_okCorner() { + assertValidationWorks(12, 10, true); + } + + @Test + public void validate_notOkCorner() { + assertValidationWorks(13, 10, false); + } + + private void assertValidationWorks(int nameLength, int numOfVms, boolean expected) { + // enough to test for windows, the logic is the same + PoolNameLengthValidation validation = + new PoolNameLengthValidation(nameOfLength(nameLength), numOfVms, VmOsType.Windows2003); //$NON-NLS-1$ + assertThat(validation.Validate(null).getSuccess(), is(equalTo(expected))); + } + + private String nameOfLength(int nameLength) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < nameLength; i++) { + sb.append("x"); //$NON-NLS-1$ + } + + return sb.toString(); + } + +} diff --git a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Messages.java b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Messages.java index 03b13a5..d7f4f53 100644 --- a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Messages.java +++ b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Messages.java @@ -151,4 +151,10 @@ @DefaultMessage("Incorrect number of Total Virtual CPUs. It is not possible to compose this number from the available Virtual Sockets and Cores per Virtual Sockets") String incorrectVCPUNumber(); + + @DefaultMessage("The max allowed name length is {0} for {1} VMs in pool") + String poolNameLengthInvalid(int maxLength, int vmsInPool); + + @DefaultMessage("The max allowed num of VMs is {0} when the length of the pool name is {1}") + String numOfVmsInPoolInvalod(int maxLength, int vmsInPool); } -- To view, visit http://gerrit.ovirt.org/7649 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I66f765ceb0567beb006d1ef4f45db3902ef95ea6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
