Oved Ourfali has posted comments on this change.
Change subject: tools: Introduction and usage of CustomPropertiesValueHelper
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File
backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/ConfigKey.java
Line 98: * @throws InvalidParameterException
Line 99: * @throws Exception
Line 100: */
Line 101: public void safeSetValue(String value) throws
InvalidParameterException, Exception {
Line 102: System.out.println("Validation helper is " +
valueHelper.getClass().getName());
why having this print? Is doesn't seem relevant to the user.
Line 103: ValidationResult validationResult =
valueHelper.validate(this, value);
Line 104: if (!validationResult.isOk()) {
Line 105: StringBuilder invalidParamMsg = new StringBuilder();
Line 106: invalidParamMsg.append("Cannot set value ")
....................................................
File
backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CompositePasswordValueHelper.java
Line 68: while (tokenizer.hasMoreElements()) {
Line 69: String token = (String) tokenizer.nextElement();
Line 70: String[] pair = token.split(":", -1);
Line 71: String password = pair[1];
Line 72: if (!pwdValueHelper.validate(null, password).isOk()) {
The result here may contain the details, so why not just return it, instead of
constructing a new one below?
Line 73: returnValue = false;
Line 74: break;
Line 75: }
Line 76: }
....................................................
File
backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/entity/helper/CustomPropertiesValueHelper.java
Line 16: for (int counter = 0; counter < keyValuePairs.length;
counter++) {
Line 17: String keyValuePair = keyValuePairs[counter];
Line 18: String parts[] = keyValuePair.split("=");
Line 19: if (parts.length != 2) {
Line 20: 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");
Not sure I would use the word regex to users. You just check here for existence
of "=" only once, so you can specify that in the message, or just use:
The entered value is in imporper format. " + keyVa
luePair + " cannot be used for custom properties definition.\nA string of
key=value pair should be used instead.
Line 21: }
Line 22: try {
Line 23: Pattern.compile(parts[1]);
Line 24: } catch (PatternSyntaxException ex) {
--
To view, visit http://gerrit.ovirt.org/9457
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffbc359880e87a7159906579a18b2e5c63bd718
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
