[jira] [Commented] (STORM-1143) Validate topology Configs during topology submission

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/STORM-1143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14987588#comment-14987588 ] 

ASF GitHub Bot commented on STORM-1143:
---------------------------------------

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

    https://github.com/apache/storm/pull/825#discussion_r43772518
  
    --- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
    @@ -504,42 +509,45 @@ public static void validateField(String fieldName, Map conf, Class configClass)
          * @param field field that needs to be validated
          * @param conf  map of confs
          */
    -    public static void validateField(Field field, Map conf) throws NoSuchFieldException, IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {
    +    public static void validateField(Field field, Map conf) {
             Annotation[] annotations = field.getAnnotations();
             if (annotations.length == 0) {
                 LOG.warn("Field {} does not have validator annotation", field);
             }
    -
    -        for (Annotation annotation : annotations) {
    -            String type = annotation.annotationType().getName();
    -            Class validatorClass = null;
    -            Class<?>[] classes = ConfigValidationAnnotations.class.getDeclaredClasses();
    -            //check if annotation is one of our
    -            for (Class clazz : classes) {
    -                if (clazz.getName().equals(type)) {
    -                    validatorClass = clazz;
    -                    break;
    -                }
    -            }
    -            if (validatorClass != null) {
    -                Object v = validatorClass.cast(annotation);
    -                String key = (String) field.get(null);
    -                Class clazz = (Class) validatorClass
    -                        .getMethod(ConfigValidationAnnotations.ValidatorParams.VALIDATOR_CLASS).invoke(v);
    -                Validator o = null;
    -                Map<String, Object> params = getParamsFromAnnotation(validatorClass, v);
    -                //two constructor signatures used to initialize validators.
    -                //One constructor takes input a Map of arguments, the other doesn't take any arguments (default constructor)
    -                //If validator has a constructor that takes a Map as an argument call that constructor
    -                if(hasConstructor(clazz, Map.class)) {
    -                    o  = (Validator) clazz.getConstructor(Map.class).newInstance(params);
    +        try {
    +            for (Annotation annotation : annotations) {
    +                String type = annotation.annotationType().getName();
    +                Class validatorClass = null;
    +                Class<?>[] classes = ConfigValidationAnnotations.class.getDeclaredClasses();
    +                //check if annotation is one of our
    +                for (Class clazz : classes) {
    +                    if (clazz.getName().equals(type)) {
    +                        validatorClass = clazz;
    +                        break;
    +                    }
                     }
    -                //If not call default constructor
    -                else {
    -                    o  = (((Class<Validator>) clazz).newInstance());
    +                if (validatorClass != null) {
    +                    Object v = validatorClass.cast(annotation);
    +                    String key = (String) field.get(null);
    +                    Class clazz = (Class) validatorClass
    +                            .getMethod(ConfigValidationAnnotations.ValidatorParams.VALIDATOR_CLASS).invoke(v);
    +                    Validator o = null;
    +                    Map<String, Object> params = getParamsFromAnnotation(validatorClass, v);
    +                    //two constructor signatures used to initialize validators.
    +                    //One constructor takes input a Map of arguments, the other doesn't take any arguments (default constructor)
    +                    //If validator has a constructor that takes a Map as an argument call that constructor
    +                    if (hasConstructor(clazz, Map.class)) {
    +                        o = (Validator) clazz.getConstructor(Map.class).newInstance(params);
    +                    }
    +                    //If not call default constructor
    +                    else {
    +                        o = (((Class<Validator>) clazz).newInstance());
    +                    }
    +                    o.validateField(field.getName(), conf.get(key));
                     }
    -                o.validateField(field.getName(), conf.get(key));
                 }
    +        }  catch (NoSuchMethodException | IllegalAccessException | InstantiationException | InvocationTargetException e) {
    +            throw new IllegalStateException("ConfigValidation failed due to: " + e.getMessage() + "\n" + e.getStackTrace().toString());
    --- End diff --
    
    Would this be a operation done in an illegal state?  Maybe we could just wrap `e` in a RuntimeException?


> Validate topology Configs during topology submission
> ----------------------------------------------------
>
>                 Key: STORM-1143
>                 URL: https://issues.apache.org/jira/browse/STORM-1143
>             Project: Apache Storm
>          Issue Type: Improvement
>            Reporter: Boyang Jerry Peng
>            Assignee: Boyang Jerry Peng
>            Priority: Minor
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)