Lior Vernia has uploaded a new change for review. Change subject: core: Extract common Custom Properties code ......................................................................
core: Extract common Custom Properties code Moved basic validation from SimpleCustomPropertiesUtil to CustomPropertiesUtils, as well as some validation from VmPropertiesUtils that should be common to all custom properties. Removed a lot of additional logic from VmPropertiesUtils that could be accomplished using logic already existing in CustomPropertiesUtils (in some cases after small modifications). Change-Id: I24efa004b1b7fe4359046bca1e6c87188cececf9 Signed-off-by: Lior Vernia <[email protected]> --- 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/SimpleCustomPropertiesUtil.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java 3 files changed, 93 insertions(+), 206 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/84/27384/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 048f93c..fd6a3ff 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 @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -9,6 +10,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.Map.Entry; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; @@ -174,6 +176,52 @@ } /** + * validate a map of specific custom properties against provided regex map + * @param regExMap + * <key, regex> map + * @param properties + * <key, value> map, custom properties to validate + * @return + */ + public List<ValidationError> validateProperties(Map<String, String> regExMap, + Map<String, String> properties) { + + if (properties == null || properties.isEmpty()) { + // No errors in case of empty value + return Collections.emptyList(); + } + + if (syntaxErrorInProperties(properties)) { + return invalidSyntaxValidationError; + } + + Set<ValidationError> errorsSet = new HashSet<ValidationError>(); + Set<String> foundKeys = new HashSet<String>(); + for (Entry<String, String> e : properties.entrySet()) { + String key = e.getKey(); + if (foundKeys.contains(key)) { + errorsSet.add(new ValidationError(ValidationFailureReason.DUPLICATE_KEY, key)); + continue; + } + foundKeys.add(key); + + if (key == null || !regExMap.containsKey(key)) { + errorsSet.add(new ValidationError(ValidationFailureReason.KEY_DOES_NOT_EXIST, key)); + continue; + } + + String value = e.getValue() == null ? "" : e.getValue(); + if (!value.matches(regExMap.get(key))) { + errorsSet.add(new ValidationError(ValidationFailureReason.INCORRECT_VALUE, key)); + continue; + } + } + List<ValidationError> results = new ArrayList<ValidationError>(); + results.addAll(errorsSet); + return results; + } + + /** * Generates an error message to be displayed by frontend * * @param validationErrors @@ -233,6 +281,31 @@ return sb.toString(); } + protected Map<String, String> convertProperties(String properties, Map<String, String> regExMap) { + if (syntaxErrorInProperties(properties)) { + throw new IllegalArgumentException("Invalid properties syntax!"); + } + + Map<String, String> map = new LinkedHashMap<String, String>(); + if (!StringHelper.isNullOrEmpty(properties)) { + String keyValuePairs[] = properties.split(PROPERTIES_DELIMETER); + for (String keyValuePairStr : keyValuePairs) { + String[] pairParts = keyValuePairStr.split(KEY_VALUE_DELIMETER, 2); + String key = pairParts[0]; + String value = pairParts[1]; + map.put(key, value); + } + } + + if (regExMap != null) { + for (ValidationError error : validateProperties(regExMap, map)) { + map.remove(error.getKeyName()); + } + } + + return map; + } + /** * Converts device custom properties from string to map. * @@ -244,22 +317,7 @@ * constant) */ public Map<String, String> convertProperties(String properties) { - if (syntaxErrorInProperties(properties)) { - throw new IllegalArgumentException("Invalid properties syntax!"); - } - - Map<String, String> map = new LinkedHashMap<String, String>(); - if (!StringHelper.isNullOrEmpty(properties)) { - String keyValuePairs[] = properties.split(PROPERTIES_DELIMETER); - for (String keyValuePairStr : keyValuePairs) { - String[] pairParts = keyValuePairStr.split(KEY_VALUE_DELIMETER, 2); - String key = pairParts[0]; - // property value may be null - String value = pairParts[1]; - map.put(key, value); - } - } - return map; + return convertProperties(properties, null); } /** 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 3e1fe53..61df789 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 @@ -1,58 +1,10 @@ package org.ovirt.engine.core.utils.customprop; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; - -import org.ovirt.engine.core.compat.StringHelper; - public class SimpleCustomPropertiesUtil extends CustomPropertiesUtils { private static final SimpleCustomPropertiesUtil instance = new SimpleCustomPropertiesUtil(); public static SimpleCustomPropertiesUtil getInstance() { return instance; - } - - /** - * validate a map of specific custom properties against provided regex map - * @param regExMap - * <key, regex> map - * @param properties - * <key, value> map, custom properties to validate - * @return - */ - public List<ValidationError> validateProperties(Map<String, String> regExMap, - Map<String, String> properties) { - - if (properties == null || properties.isEmpty()) { - // No errors in case of empty value - return Collections.emptyList(); - } - - if (syntaxErrorInProperties(properties)) { - return invalidSyntaxValidationError; - } - - Set<ValidationError> errorsSet = new HashSet<ValidationError>(); - for (Entry<String, String> e : properties.entrySet()) { - String key = e.getKey(); - if (key == null || !regExMap.containsKey(key)) { - errorsSet.add(new ValidationError(ValidationFailureReason.KEY_DOES_NOT_EXIST, key)); - continue; - } - - if (!StringHelper.defaultString(e.getValue()).matches(regExMap.get(key))) { - errorsSet.add(new ValidationError(ValidationFailureReason.INCORRECT_VALUE, key)); - continue; - } - } - List<ValidationError> results = new ArrayList<ValidationError>(); - results.addAll(errorsSet); - return results; } } 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 d2f44e3..896dbf0 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 @@ -1,10 +1,6 @@ package org.ovirt.engine.core.utils.customprop; -import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -32,7 +28,7 @@ private Map<Version, Map<String, String>> predefinedProperties; private Map<Version, Map<String, String>> userdefinedProperties; - private Map<Version, String> allVmProperties; + private Map<Version, Map<String, String>> allVmProperties; public static VmPropertiesUtils getInstance() { return vmPropertiesUtils; @@ -42,25 +38,16 @@ try { predefinedProperties = new HashMap<Version, Map<String, String>>(); userdefinedProperties = new HashMap<Version, Map<String, String>>(); - allVmProperties = new HashMap<Version, String>(); + allVmProperties = new HashMap<Version, Map<String, String>>(); Set<Version> versions = getSupportedClusterLevels(); - String predefinedVMPropertiesStr, userDefinedVMPropertiesStr; - StringBuilder sb; for (Version version : versions) { - predefinedVMPropertiesStr = getPredefinedVMProperties(version); - userDefinedVMPropertiesStr = getUserdefinedVMProperties(version); - sb = new StringBuilder(""); - sb.append(predefinedVMPropertiesStr); - if (!predefinedVMPropertiesStr.isEmpty() && !userDefinedVMPropertiesStr.isEmpty()) { - sb.append(";"); - } - sb.append(userDefinedVMPropertiesStr); - allVmProperties.put(version, sb.toString()); - predefinedProperties.put(version, new HashMap<String, String>()); userdefinedProperties.put(version, new HashMap<String, String>()); - parsePropertiesRegex(predefinedVMPropertiesStr, predefinedProperties.get(version)); - parsePropertiesRegex(userDefinedVMPropertiesStr, userdefinedProperties.get(version)); + allVmProperties.put(version, new HashMap<String, String>()); + parsePropertiesRegex(getPredefinedVMProperties(version), predefinedProperties.get(version)); + parsePropertiesRegex(getUserdefinedVMProperties(version), userdefinedProperties.get(version)); + allVmProperties.get(version).putAll(predefinedProperties.get(version)); + allVmProperties.get(version).putAll(userdefinedProperties.get(version)); } } catch (Throwable ex) { throw new InitializationException(ex); @@ -106,8 +93,8 @@ HashMap<String, String> predefinedPropertiesMap = new HashMap<String, String>(); convertCustomPropertiesStrToMaps(version, propertiesStr, predefinedPropertiesMap, userDefinedPropertiesMap); - return new VMCustomProperties(vmPropertiesToString(predefinedPropertiesMap), - vmPropertiesToString(userDefinedPropertiesMap)); + return new VMCustomProperties(convertProperties(predefinedPropertiesMap), + convertProperties(userDefinedPropertiesMap)); } /** @@ -117,29 +104,7 @@ * @return a list of validation errors. if there are no errors - the list will be empty */ public List<ValidationError> validateVmProperties(Version version, String properties) { - if (StringHelper.isNullOrEmpty(properties)) { // No errors in case of empty value - return Collections.emptyList(); - } - if (syntaxErrorInProperties(properties)) { - return invalidSyntaxValidationError; - } - // Transform the VM custom properties from string value to hash map - // check the following for each one of the keys: - // 1. Check if the key exists in either the predefined or the userdefined key sets - // 2. Check if the value of the key is valid - // In case either 1 or 2 fails, add an error to the errors list - Map<String, String> map = new HashMap<String, String>(); - return populateVMProperties(version, properties, map); - } - - private boolean isValueValid(Version version, String key, String value) { - // Checks that the value for the given property is valid by running by trying to perform - // regex validation - String userDefinedPattern = userdefinedProperties.get(version).get(key); - String predefinedPattern = predefinedProperties.get(version).get(key); - - return (userDefinedPattern != null && value.matches(userDefinedPattern)) - || (predefinedPattern != null && value.matches(predefinedPattern)); + return validateProperties(allVmProperties.get(version), convertProperties(properties)); } /** @@ -157,33 +122,11 @@ } public Map<Version, String> getAllVmProperties() { - return allVmProperties; - } - - /** - * Get a map containing user defined properties from VM - * - * @param vm - * vm to get the map for - * @return map containing the UserDefined properties - */ - public Map<String, String> getUserDefinedProperties(Version version, VmStatic vmStatic) { - Map<String, String> map = new HashMap<String, String>(); - getUserDefinedProperties(version, vmStatic, map); - return map; - } - - /** - * Gets a map containing the predefined properties from VM - * - * @param vm - * vm to get the map for - * @return map containing the vm properties - */ - public Map<String, String> getPredefinedProperties(Version version, VmStatic vmStatic) { - Map<String, String> map = new HashMap<String, String>(); - getPredefinedProperties(version, vmStatic, map); - return map; + Map<Version, String> allVmPropertiesString = new HashMap<Version, String>(); + for (Map.Entry<Version, Map<String, String>> entry : allVmProperties.entrySet()) { + allVmPropertiesString.put(entry.getKey(), convertProperties(entry.getValue())); + } + return allVmPropertiesString; } private void getPredefinedProperties(Version version, VmStatic vmStatic, Map<String, String> propertiesMap) { @@ -204,89 +147,23 @@ * @param vmPropertiesFieldValue * the string value that contains the properties */ - public void getVMProperties(Version version, Map<String, String> propertiesMap, String vmPropertiesFieldValue) { + private void getVMProperties(Version version, Map<String, String> propertiesMap, String vmPropertiesFieldValue) { // format of properties is key1=val1,key2=val2,key3=val3,key4=val4 if (StringHelper.isNullOrEmpty(vmPropertiesFieldValue)) { return; } - populateVMProperties(version, vmPropertiesFieldValue, propertiesMap); - - } - - /** - * Parses a string of VM properties to a map of key,value - * - * @param vmPropertiesFieldValue - * the string to parse - * @param propertiesMap - * the filled map - * @return list of errors during parsing (currently holds list of duplicate keys) - */ - private List<ValidationError> populateVMProperties(Version version, String vmPropertiesFieldValue, - Map<String, String> propertiesMap) { - Set<ValidationError> errorsSet = new HashSet<ValidationError>(); - List<ValidationError> results = new ArrayList<ValidationError>(); - if (!StringHelper.isNullOrEmpty(vmPropertiesFieldValue)) { - String keyValuePairs[] = vmPropertiesFieldValue.split(PROPERTIES_DELIMETER); - - for (String keyValuePairStr : keyValuePairs) { - String[] pairParts = keyValuePairStr.split(KEY_VALUE_DELIMETER, 2); - String key = pairParts[0]; - String value = StringHelper.defaultString(pairParts[1]); - if (propertiesMap.containsKey(key)) { - errorsSet.add(new ValidationError(ValidationFailureReason.DUPLICATE_KEY, key)); - continue; - } - if (!keyExistsInVersion(predefinedProperties, version, key) - && !keyExistsInVersion(userdefinedProperties, version, key)) { - errorsSet.add(new ValidationError(ValidationFailureReason.KEY_DOES_NOT_EXIST, key)); - continue; - } - - if (!isValueValid(version, key, value)) { - errorsSet.add(new ValidationError(ValidationFailureReason.INCORRECT_VALUE, key)); - continue; - } - propertiesMap.put(key, value); - } - } - results.addAll(errorsSet); - return results; + propertiesMap.putAll(convertProperties(vmPropertiesFieldValue, allVmProperties.get(version))); } protected boolean keyExistsInVersion(Map<Version, Map<String, String>> propertiesMap, Version version, String key) { return propertiesMap.get(version).containsKey(key); } - private static String vmPropertiesToString(Map<String, String> propertiesMap) { - if (propertiesMap == null || propertiesMap.size() == 0) { - return ""; - } - - StringBuilder result = new StringBuilder(); - 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(StringHelper.defaultString(entry.getValue())); - while (iterator.hasNext()) { - result.append(";"); - entry = iterator.next(); - if (entry != null) { - result.append(entry.getKey()) - .append("=") - .append(StringHelper.defaultString(entry.getValue())); - } - } - return result.toString(); - } - private void convertCustomPropertiesStrToMaps(Version version, String propertiesValue, Map<String, String> predefinedPropertiesMap, Map<String, String> userDefinedPropertiesMap) { - Map<String, String> propertiesMap = new HashMap<String, String>(); - populateVMProperties(version, propertiesValue, propertiesMap); + Map<String, String> propertiesMap = + convertProperties(propertiesValue, allVmProperties.get(version)); Set<Entry<String, String>> propertiesEntries = propertiesMap.entrySet(); // Go over all the properties - if the key of the property exists in the -- To view, visit http://gerrit.ovirt.org/27384 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I24efa004b1b7fe4359046bca1e6c87188cececf9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
