Martin Peřina has uploaded a new change for review. Change subject: core: Enables empty value in custom properties ......................................................................
core: Enables empty value in custom properties Enables empty values in custom VM and device properties. Change-Id: Id5ff5b2a839e8f1f5a56c9c1258252e3f24a08df Bug-Url: https://bugzilla.redhat.com/1046611 Signed-off-by: Martin Perina <mper...@redhat.com> --- M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtils.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtils.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/SimpleCustomPropertiesUtil.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtilsTest.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtilsTest.java 6 files changed, 36 insertions(+), 27 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/23062/1 diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtils.java index 4ce665e..1bf03c7 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtils.java @@ -59,7 +59,7 @@ * Regex describing property definition - key=value */ protected static final String KEY_VALUE_REGEX_STR = "((" + LEGITIMATE_CHARACTER_FOR_KEY + ")+)=((" - + LEGITIMATE_CHARACTER_FOR_VALUE + ")+)"; + + LEGITIMATE_CHARACTER_FOR_VALUE + ")*)"; /** * Regex describing properties definition. They can be in the from of "key=value" or "key1=value1;... key-n=value_n" @@ -83,7 +83,7 @@ CustomPropertiesUtils() { semicolonPattern = Pattern.compile(PROPERTIES_DELIMETER); keyPattern = Pattern.compile("(" + LEGITIMATE_CHARACTER_FOR_KEY + ")+"); - valuePattern = Pattern.compile("(" + LEGITIMATE_CHARACTER_FOR_VALUE + ")+"); + valuePattern = Pattern.compile("(" + LEGITIMATE_CHARACTER_FOR_VALUE + ")*"); validationPattern = Pattern.compile(VALIDATION_STR); invalidSyntaxValidationError = Arrays.asList(new ValidationError(ValidationFailureReason.SYNTAX_ERROR, "")); } @@ -121,13 +121,13 @@ if (properties != null && !properties.isEmpty()) { for (Map.Entry<String, String> e : properties.entrySet()) { String key = e.getKey(); - String value = e.getValue(); if (key == null || !keyPattern.matcher(key).matches()) { // syntax error in property name error = true; break; } - if (value == null || !valuePattern.matcher(value).matches()) { + + if (!valuePattern.matcher(StringUtils.defaultString(e.getValue())).matches()) { // syntax error in property value error = true; break; @@ -290,7 +290,7 @@ for (Map.Entry<String, String> e : properties.entrySet()) { sb.append(e.getKey()); sb.append(KEY_VALUE_DELIMETER); - sb.append(e.getValue()); + sb.append(StringUtils.defaultString(e.getValue())); sb.append(PROPERTIES_DELIMETER); } // remove last PROPERTIES_DELIMETER diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtils.java index 2629c06..f1f6920 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtils.java @@ -12,6 +12,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.config.Config; @@ -329,13 +330,13 @@ for (Map.Entry<String, String> e : properties.entrySet()) { String key = e.getKey(); - String value = e.getValue(); if (key == null || !deviceProperties.get(version).get(type).containsKey(key)) { errorsSet.add(new ValidationError(ValidationFailureReason.KEY_DOES_NOT_EXIST, key)); continue; } - if (value == null || !deviceProperties.get(version).get(type).get(key).matcher(value).matches()) { + String value = StringUtils.defaultString(e.getValue()); + if (!deviceProperties.get(version).get(type).get(key).matcher(value).matches()) { errorsSet.add(new ValidationError(ValidationFailureReason.INCORRECT_VALUE, key)); continue; } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/SimpleCustomPropertiesUtil.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/SimpleCustomPropertiesUtil.java index c59d5b7..3b92939 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/SimpleCustomPropertiesUtil.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/SimpleCustomPropertiesUtil.java @@ -10,6 +10,8 @@ import java.util.Set; import java.util.regex.Pattern; +import org.apache.commons.lang.StringUtils; + public class SimpleCustomPropertiesUtil extends CustomPropertiesUtils { private static final SimpleCustomPropertiesUtil instance = new SimpleCustomPropertiesUtil(); @@ -47,13 +49,12 @@ for (Entry<String, String> e : properties.entrySet()) { String key = e.getKey(); - String value = e.getValue(); if (key == null || !regExMap.containsKey(key)) { errorsSet.add(new ValidationError(ValidationFailureReason.KEY_DOES_NOT_EXIST, key)); continue; } - if (value == null || !keysToRegex.get(key).matcher(value).matches()) { + if (!keysToRegex.get(key).matcher(StringUtils.defaultString(e.getValue())).matches()) { errorsSet.add(new ValidationError(ValidationFailureReason.INCORRECT_VALUE, key)); continue; } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java index cd932db..ebb778f 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java @@ -247,7 +247,7 @@ for (String keyValuePairStr : keyValuePairs) { String[] pairParts = keyValuePairStr.split(KEY_VALUE_DELIMETER, 2); String key = pairParts[0]; - String value = pairParts[1]; + String value = StringUtils.defaultString(pairParts[1]); if (propertiesMap.containsKey(key)) { errorsSet.add(new ValidationError(ValidationFailureReason.DUPLICATE_KEY, key)); continue; @@ -282,12 +282,16 @@ Set<Entry<String, String>> entries = propertiesMap.entrySet(); Iterator<Entry<String, String>> iterator = entries.iterator(); Entry<String, String> entry = iterator.next(); - result.append(entry.getKey()).append("=").append(entry.getValue()); + result.append(entry.getKey()) + .append("=") + .append(StringUtils.defaultString(entry.getValue())); while (iterator.hasNext()) { result.append(";"); entry = iterator.next(); if (entry != null) { - result.append(entry.getKey()).append("=").append(entry.getValue()); + result.append(entry.getKey()) + .append("=") + .append(StringUtils.defaultString(entry.getValue())); } } return result.toString(); @@ -308,7 +312,7 @@ Set<String> userdefinedPropertiesKeys = userdefinedProperties.get(version).keySet(); for (Entry<String, String> propertiesEntry : propertiesEntries) { String propertyKey = propertiesEntry.getKey(); - String propertyValue = propertiesEntry.getValue(); + String propertyValue = StringUtils.defaultString(propertiesEntry.getValue()); if (predefinedPropertiesKeys.contains(propertyKey)) { predefinedPropertiesMap.put(propertyKey, propertyValue); } diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtilsTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtilsTest.java index 77f7d47..c0553f6 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtilsTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/CustomPropertiesUtilsTest.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.utils.customprop; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.ovirt.engine.core.utils.customprop.PropertiesUtilsTestHelper.validatePropertyMap; import static org.ovirt.engine.core.utils.customprop.PropertiesUtilsTestHelper.validatePropertyValue; @@ -22,9 +23,7 @@ CustomPropertiesUtils utils = new CustomPropertiesUtils(); assertTrue("Invalid key/value delimiter", utils.syntaxErrorInProperties("speed:1024;duplex=full")); - assertTrue("Key/value delimiter present, but value missing", - utils.syntaxErrorInProperties("speed=1024;duplex=")); - assertTrue("Missing value", utils.syntaxErrorInProperties("speed=1024;duplex")); + assertTrue("Missing key/value delimiter", utils.syntaxErrorInProperties("speed=1024;duplex")); assertTrue("Invalid key character", utils.syntaxErrorInProperties("spe*ed=1024;duplex=full")); assertTrue("Invalid value character", utils.syntaxErrorInProperties("speed=1024;duplex=fu;ll")); } @@ -38,8 +37,9 @@ Map<String, String> propMap = new LinkedHashMap<>(); propMap.put("speed", "1024"); propMap.put("duplex", null); + propMap.put("debug", ""); - assertTrue(utils.syntaxErrorInProperties(propMap)); + assertFalse(utils.syntaxErrorInProperties(propMap)); } /** @@ -48,12 +48,13 @@ @Test public void convertValidPropertiesToMap() { CustomPropertiesUtils utils = new CustomPropertiesUtils(); - String propStr = "speed=1024;duplex=half"; + String propStr = "speed=1024;duplex=half;debug="; Map<String, String> map = utils.convertProperties(propStr); - validatePropertyMap(map, 2); + validatePropertyMap(map, 3); validatePropertyValue(map, "speed", "1024"); validatePropertyValue(map, "duplex", "half"); + validatePropertyValue(map, "debug", ""); } /** @@ -65,9 +66,11 @@ Map<String, String> propMap = new LinkedHashMap<>(); propMap.put("speed", "1024"); propMap.put("duplex", "half"); + propMap.put("debug", null); + propMap.put("verbose", ""); String propStr = utils.convertProperties(propMap); // order of properties in string is known if LinkedHashMap is used - assertEquals("speed=1024;duplex=half", propStr); + assertEquals("speed=1024;duplex=half;debug=;verbose=", propStr); } } diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtilsTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtilsTest.java index cf253e0..13005ad 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtilsTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtilsTest.java @@ -54,7 +54,7 @@ */ private DevicePropertiesUtils mockDevicePropertiesUtils() { return mockDevicePropertiesUtils("{type=disk;prop={bootable=^(true|false)$}};" - + "{type=interface;prop={speed=^([0-9]{1,5})$;duplex=^(full|half)$}}"); + + "{type=interface;prop={speed=^([0-9]{1,5})$;duplex=^(full|half)$;debug=([a-z0-9A-Z]*)$}}"); } /** @@ -110,24 +110,24 @@ } /** - * Tries to set property with no value to a device with supported version + * Tries to set property with no value (value is invalid due provided REGEX) to a device with supported version */ @Test - public void invalidPropertiesFormat() { + public void invalidPropertyValue1() { DevicePropertiesUtils utils = mockDevicePropertiesUtils(); Map<String, String> map = new HashMap<String, String>(); map.put("bootable", null); List<ValidationError> errors = utils.validateProperties(Version.v3_3, VmDeviceGeneralType.DISK, map); - validateFailure(errors, ValidationFailureReason.SYNTAX_ERROR); + validateFailure(errors, ValidationFailureReason.INCORRECT_VALUE); } /** * Tries to set invalid property value to a device with supported version */ @Test - public void invalidPropertyValue() { + public void invalidPropertyValue2() { DevicePropertiesUtils utils = mockDevicePropertiesUtils(); List<ValidationError> errors = utils.validateProperties(Version.v3_3, VmDeviceGeneralType.DISK, utils.convertProperties("bootable=x")); @@ -173,7 +173,7 @@ List<ValidationError> errors = utils.validateProperties(Version.v3_3, VmDeviceGeneralType.INTERFACE, - utils.convertProperties("speed=10;duplex=half")); + utils.convertProperties("speed=10;duplex=half;debug=")); assertTrue(errors.isEmpty()); } @@ -184,7 +184,7 @@ @Test public void validCustomDevPropSpec() { String customDevPropSpec = "{type=disk;prop={bootable=^(true|false)$}};" - + "{type=interface;prop={speed=[0-9]{1,5};duplex=^(full|half)$}};" + + "{type=interface;prop={speed=[0-9]{1,5};duplex=^(full|half)$;debug=([a-z0-9A-Z]*)$}};" + "{type=video;prop={turned_on=^(true|false)$}};" + "{type=sound;prop={volume=[0-9]{1,2}}};" + "{type=controller;prop={hotplug=^(true|false)$}};" -- To view, visit http://gerrit.ovirt.org/23062 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id5ff5b2a839e8f1f5a56c9c1258252e3f24a08df Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
