Yair Zaslavsky has uploaded a new change for review. Change subject: tools: Introduction and usage of CustomPropertiesValueHelper ......................................................................
tools: Introduction and usage of CustomPropertiesValueHelper The following patch introduces the CustomPropertiesValueHelper to be used with UserdefinedVmProperties. In order to provide a more accurate description, validate method is changed to return a ValidationResult object that contains a descriptive message. engine-config.properties is changed accordingly. Bug-Id: https://bugzilla.redhat.com/ 878964 Change-Id: Ieffbc359880e87a7159906579a18b2e5c63bd718 Signed-off-by: Yair Zaslavsky <[email protected]> --- M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/ConfigKey.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CompositePasswordValueHelper.java A backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CustomPropertiesValueHelper.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/IntegerValueHelper.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/PasswordValueHelper.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringMultipleValueHelper.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringValueHelper.java A backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValidationResult.java M backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValueHelper.java M backend/manager/tools/engine-config/src/main/resources/engine-config.properties 11 files changed, 106 insertions(+), 22 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/57/9457/1 diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java index a14853a..bf76f27 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java @@ -389,10 +389,13 @@ configKey.safeSetValue(value); res = (getConfigDAO().updateKey(configKey) == 1); } catch (InvalidParameterException ipe) { - message = ( - "'" + value + "' is not a valid value for type " + configKey.getType() + ". " + - (configKey.getValidValues().isEmpty() ? "" : "Valid values are " + configKey.getValidValues()) - ); + message = ipe.getMessage(); + if (message == null) { + message = ( + "'" + value + "' is not a valid value for type " + configKey.getType() + ". " + + (configKey.getValidValues().isEmpty() ? "" : "Valid values are " + configKey.getValidValues()) + ); + } } catch (Exception e) { message = "Error setting " + key + "'s value. " + e.getMessage(); log.debug("Error details: ", e); diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/ConfigKey.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/ConfigKey.java index d6eb951..ac2aa77 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/ConfigKey.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/ConfigKey.java @@ -5,8 +5,10 @@ import java.util.Arrays; import java.util.List; +import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.ovirt.engine.core.config.EngineConfigCLIParser; +import org.ovirt.engine.core.config.entity.helper.ValidationResult; import org.ovirt.engine.core.config.entity.helper.ValueHelper; public class ConfigKey { @@ -97,8 +99,17 @@ * @throws Exception */ public void safeSetValue(String value) throws InvalidParameterException, Exception { - if (!valueHelper.validate(this, value)) { - throw new InvalidParameterException("Cannot set value " + value + "to key " + keyName); + System.out.println("Validation helper is " + valueHelper.getClass().getName()); + ValidationResult validationResult = valueHelper.validate(this, value); + if (!validationResult.isOk()) { + StringBuilder invalidParamMsg = new StringBuilder(); + invalidParamMsg.append("Cannot set value ") + .append(value) + .append(" to key ") + .append(keyName) + .append(". ") + .append(StringUtils.isNotEmpty(validationResult.getDetails()) ? validationResult.getDetails() : ""); + throw new InvalidParameterException(invalidParamMsg.toString()); } this.value = valueHelper.setValue(value); } diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CompositePasswordValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CompositePasswordValueHelper.java index 87ccdd5..e9f2ec6 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CompositePasswordValueHelper.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CompositePasswordValueHelper.java @@ -62,19 +62,19 @@ } @Override - public boolean validate(ConfigKey key, String value) { + public ValidationResult validate(ConfigKey key, String value) { boolean returnValue = true; StringTokenizer tokenizer = new StringTokenizer(value, ","); while (tokenizer.hasMoreElements()) { String token = (String) tokenizer.nextElement(); String[] pair = token.split(":", -1); String password = pair[1]; - if (!pwdValueHelper.validate(null, password)) { + if (!pwdValueHelper.validate(null, password).isOk()) { returnValue = false; break; } } - return returnValue; + return new ValidationResult(returnValue); } @Override diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CustomPropertiesValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CustomPropertiesValueHelper.java new file mode 100644 index 0000000..d1e53357 --- /dev/null +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CustomPropertiesValueHelper.java @@ -0,0 +1,31 @@ +package org.ovirt.engine.core.config.entity.helper; + +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; + +import org.ovirt.engine.core.config.entity.ConfigKey; + +public class CustomPropertiesValueHelper extends StringValueHelper { + + public CustomPropertiesValueHelper() { + } + + @Override + public ValidationResult validate(ConfigKey key, String value) { + String[] keyValuePairs = value.split(";"); + for (int counter = 0; counter < keyValuePairs.length; counter++) { + String keyValuePair = keyValuePairs[counter]; + String parts[] = keyValuePair.split("="); + if (parts.length != 2) { + return new ValidationResult(false,"The entered value is in imporper format. " + keyValuePair + " cannot be used for custom properties definition.\nA string of key=value pair should be used instead, where the value should be a correct regex expression"); + } + try { + Pattern.compile(parts[1]); + } catch (PatternSyntaxException ex) { + return new ValidationResult(false,"The entered value is in imporper format. " + parts[1] + " must be a valid regex expression"); + } + + } + return new ValidationResult(true); + } +} diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/IntegerValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/IntegerValueHelper.java index 81951a1..2420113 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/IntegerValueHelper.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/IntegerValueHelper.java @@ -18,7 +18,7 @@ } @Override - public boolean validate(ConfigKey key, String value) { + public ValidationResult validate(ConfigKey key, String value) { try { int intValue = Integer.parseInt(value); @@ -46,10 +46,10 @@ } } } - return isValid; + return new ValidationResult(isValid); } catch (NumberFormatException e) { // invalid number - return false; + return new ValidationResult(false); } } diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/PasswordValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/PasswordValueHelper.java index 0a34e6c..4d9b810 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/PasswordValueHelper.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/PasswordValueHelper.java @@ -114,20 +114,21 @@ } @Override - public boolean validate(ConfigKey key, String value) { + public ValidationResult validate(ConfigKey key, String value) { // check if value is file path if (StringUtils.isNotBlank(value) && new File(value).exists()) { - return true; + return new ValidationResult(true); } // The only valid value is "Interactive" if (StringUtils.isNotBlank(value) && value.equalsIgnoreCase(INTERACTIVE_MODE)) { - return true; + return new ValidationResult(true); } // or if we have the password in --admin-pass-file if (StringUtils.isNotBlank(parser.getAdminPassFile()) && new File(parser.getAdminPassFile()).exists()) { - return true; + return new ValidationResult(true); } - return false; + return new ValidationResult(false); + } @Override diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringMultipleValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringMultipleValueHelper.java index aebe717..78ca478 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringMultipleValueHelper.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringMultipleValueHelper.java @@ -25,7 +25,7 @@ } @Override - public boolean validate(ConfigKey key, String value) { + public ValidationResult validate(ConfigKey key, String value) { Collection<String> validValues = key.getValidValues(); @@ -35,7 +35,7 @@ } else { isValid = CollectionUtils.isSubCollection(Arrays.asList(value.split(",")), validValues); } - return isValid; + return new ValidationResult(isValid); } @Override diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringValueHelper.java index 2c9469f..d18f2a2 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringValueHelper.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/StringValueHelper.java @@ -18,7 +18,7 @@ } @Override - public boolean validate(ConfigKey key, String value) { + public ValidationResult validate(ConfigKey key, String value) { List<String> validValues = key.getValidValues(); boolean isValid = false; @@ -27,7 +27,7 @@ } else { isValid = validValues.contains(value); } - return isValid; + return new ValidationResult(isValid); } @Override diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValidationResult.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValidationResult.java new file mode 100644 index 0000000..8c56459 --- /dev/null +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValidationResult.java @@ -0,0 +1,37 @@ +package org.ovirt.engine.core.config.entity.helper; + +/** + * Represents the result of the Value Helper validation + */ +public class ValidationResult { + + + public ValidationResult(boolean ok) { + this(ok,""); + } + + public ValidationResult(boolean ok, String details) { + this.ok = ok; + this.details = details; + } + + private boolean ok; + private String details; + + public boolean isOk() { + return ok; + } + + public void setOk(boolean ok) { + this.ok = ok; + } + + public String getDetails() { + return details; + } + + public void setDetails(String details) { + this.details = details; + } + +} diff --git a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValueHelper.java b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValueHelper.java index a20eb3a..c7e1e3d 100644 --- a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValueHelper.java +++ b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/ValueHelper.java @@ -9,7 +9,7 @@ String setValue(String value) throws Exception; - boolean validate(ConfigKey key, String value); + ValidationResult validate(ConfigKey key, String value); void setParser(EngineConfigCLIParser parser); } diff --git a/backend/manager/tools/engine-config/src/main/resources/engine-config.properties b/backend/manager/tools/engine-config/src/main/resources/engine-config.properties index 0e119d6..6026a4d 100644 --- a/backend/manager/tools/engine-config/src/main/resources/engine-config.properties +++ b/backend/manager/tools/engine-config/src/main/resources/engine-config.properties @@ -150,6 +150,7 @@ TimeToReduceFailedRunOnVdsInMinutes.description="Time to Reduce Failed Run on Host (in minutes)" TimeToReduceFailedRunOnVdsInMinutes.type=Integer UserDefinedVMProperties.description="User defined VM properties" +UserDefinedVMProperties.type=CustomProperties UserRefreshRate.description="Refresh Rate of Users Data from Active Directory (in seconds)" UserRefreshRate.type=Integer UtilizationThresholdInPercent.description="The Utilization Threshold (in percent)" -- To view, visit http://gerrit.ovirt.org/9457 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ieffbc359880e87a7159906579a18b2e5c63bd718 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
