Merge pull request apache#631 from d2r/d2r-add-key-to-config-validati…

…on-exceptions Includes key in invalid config exception message
cheng1234 · Jul 18, 2013 · 470e7cd · 470e7cd
2 parents 05fb469 + 8996825
commit 470e7cd
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 17 deletions.
diff --git a/storm-core/src/clj/backtype/storm/config.clj b/storm-core/src/clj/backtype/storm/config.clj
@@ -36,11 +36,11 @@
 (defmethod get-FieldValidator Object [klass]
   {:pre [(not (nil? klass))]}
   (reify ConfigValidation$FieldValidator
-    (validateField [this v]
+    (validateField [this name v]
       (if (and (not (nil? v))
                (not (instance? klass v)))
         (throw (IllegalArgumentException.
-                 (str "'" v "' must be a '" (.getName klass) "'")))))))
+                 (str "field " name " '" v "' must be a '" (.getName klass) "'")))))))
 
 ;; Create a mapping of config-string -> validator
 ;; Config fields must have a _SCHEMA field defined
@@ -94,7 +94,7 @@
   (doseq [[k v] conf
          :let [schema (CONFIG-SCHEMA-MAP k)]]
     (if (not (nil? schema))
-      (.validateField schema v))))
+      (.validateField schema k v))))
 
 (defn read-storm-config []
   (let [

diff --git a/storm-core/src/jvm/backtype/storm/ConfigValidation.java b/storm-core/src/jvm/backtype/storm/ConfigValidation.java
@@ -11,10 +11,11 @@ public class ConfigValidation {
     public static interface FieldValidator {
         /**
          * Validates the given field.
+         * @param name the name of the field.
          * @param field The field to be validated.
          * @throws IllegalArgumentException if the field fails validation.
          */
-        public void validateField(Object field) throws IllegalArgumentException;
+        public void validateField(String name, Object field) throws IllegalArgumentException;
     }
 
     /**
@@ -25,7 +26,7 @@ public static interface FieldValidator {
     static FieldValidator FieldListValidatorFactory(final Class cls) {
         return new FieldValidator() {
             @Override
-            public void validateField(Object field)
+            public void validateField(String name, Object field)
                     throws IllegalArgumentException {
                 if (field == null) {
                     // A null value is acceptable.
@@ -35,14 +36,14 @@ public void validateField(Object field)
                     for (Object e : (Iterable)field) {
                         if (! cls.isInstance(e)) {
                             throw new IllegalArgumentException(
-                                    "Each element of this list must be a " +
+                                    "Each element of the list " + name + " must be a " +
                                     cls.getName() + ".");
                         }
                     }
                     return;
                 }
                 throw new IllegalArgumentException(
-                        "Field must be an Iterable of " + cls.getName());
+                        "Field " + name + " must be an Iterable of " + cls.getName());
             }
         };
     }
@@ -62,7 +63,7 @@ public void validateField(Object field)
      */
     public static Object PowerOf2Validator = new FieldValidator() {
         @Override
-        public void validateField(Object o) throws IllegalArgumentException {
+        public void validateField(String name, Object o) throws IllegalArgumentException {
             if (o == null) {
                 // A null value is acceptable.
                 return;
@@ -76,7 +77,7 @@ public void validateField(Object o) throws IllegalArgumentException {
                     return;
                 }
             }
-            throw new IllegalArgumentException("Field must be a power of 2.");
+            throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
         }
     };
 }
diff --git a/storm-core/test/clj/backtype/storm/config_test.clj b/storm-core/test/clj/backtype/storm/config_test.clj
@@ -15,10 +15,10 @@
   (let [validator ConfigValidation/PowerOf2Validator]
     (doseq [x [42.42 42 23423423423 -33 -32 -1 -0.00001 0 -0 "Forty-two"]]
       (is (thrown-cause? java.lang.IllegalArgumentException
-        (.validateField validator x))))
+        (.validateField validator "test" x))))
 
     (doseq [x [64 4294967296 1 nil]]
-      (.validateField validator x))))
+      (.validateField validator "test" x))))
 
 (deftest test-list-validator
   (let [validator ConfigValidation/StringsValidator]
@@ -31,25 +31,25 @@
               ]]
       (is (thrown-cause-with-msg?
             java.lang.IllegalArgumentException #"(?i).*each element.*"
-        (.validateField validator x))))
+        (.validateField validator "test" x))))
 
     (doseq [x ["not a list at all"]]
       (is (thrown-cause-with-msg?
             java.lang.IllegalArgumentException #"(?i).*must be an iterable.*"
-        (.validateField validator x))))
+        (.validateField validator "test" x))))
 
     (doseq [x [
                ["one" "two" "three"]
                [""]
                ["42" "64"]
                nil
               ]]
-      (.validateField validator x))))
+      (.validateField validator "test" x))))
 
 (deftest test-topology-workers-is-number
   (let [validator (CONFIG-SCHEMA-MAP TOPOLOGY-WORKERS)]
-    (.validateField validator 42)
+    (.validateField validator "test" 42)
     ;; The float can be rounded down to an int.
-    (.validateField validator 3.14159)
+    (.validateField validator "test" 3.14159)
     (is (thrown-cause? java.lang.IllegalArgumentException
-      (.validateField validator "42")))))
+      (.validateField validator "test" "42")))))