Github user bhaisaab commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1489#discussion_r60772392
  
    --- Diff: server/src/com/cloud/api/dispatch/ParamProcessWorker.java ---
    @@ -92,6 +94,55 @@ public void handle(final DispatchTask task) {
             processParameters(task.getCmd(), task.getParams());
         }
     
    +    private void validateNonEmptyString(final Object param, final String 
argName) {
    +        if (param == null || Strings.isNullOrEmpty(param.toString())) {
    +            throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
String.format("Empty or null value provided for API arg: %s", argName));
    +        }
    +    }
    +
    +    private void validateNaturalNumber(final Object param, final String 
argName) {
    +        Long value = null;
    +        if (param != null && param instanceof Long) {
    +            value = (Long) param;
    +        } else if (param != null) {
    +            value = Long.valueOf(param.toString());
    +        }
    +        if (value == null || value < 1L) {
    +            throw new ServerApiException(ApiErrorCode.PARAM_ERROR, 
String.format("Invalid value provided for API arg: %s", argName));
    +        }
    +    }
    +
    +    private void validateField(final Object paramObj, final Parameter 
annotation) throws ServerApiException {
    +        if (annotation == null) {
    +            return;
    +        }
    +        final String argName = annotation.name();
    +        for (final ApiArgValidator validator : annotation.validations()) {
    +            if (validator == null) {
    +                continue;
    +            }
    +            switch (validator) {
    +                case NotNullOrEmpty:
    +                    switch (annotation.type()) {
    +                        case UUID:
    +                        case STRING:
    +                            validateNonEmptyString(paramObj, argName);
    +                            break;
    +                    }
    +                    break;
    +                case PositiveNumber:
    +                    switch (annotation.type()) {
    +                        case SHORT:
    +                        case INTEGER:
    +                        case LONG:
    +                            validateNaturalNumber(paramObj, argName);
    +                            break;
    +                    }
    --- End diff --
    
    annotation fields need an exact value, so it could either have a list of 
enum or classes; using a list of classes would make implementation complex and 
creation of several classes for each new validator. We also needed to know what 
the annotation type is to only execute operation based on type, for example 
PositiveNumber validator on a  string or boolean field/arg type won't make 
sense. It was easiest in terms of maintainability and implementation to pass in 
enum, and handle them here like this. I'm not sure the best way of doing this, 
advise if there are better ways of doing validation?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to