Repository: storm
Updated Branches:
  refs/heads/0.10.x-branch ac0ae4aad -> 94b4458bc


STORM-966 ConfigValidation.DoubleValidator doesn't really validate whether the 
type of the object is a double

* replacement of the double validator
* added united tests and changed exception message


Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/f84bd01c
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/f84bd01c
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/f84bd01c

Branch: refs/heads/0.10.x-branch
Commit: f84bd01cd4507aec89e19217082df707766a595d
Parents: ac0ae4a
Author: Jerry <jerry@ubuntu.(none)>
Authored: Wed Jul 29 21:00:49 2015 -0700
Committer: Jungtaek Lim <kabh...@gmail.com>
Committed: Thu Aug 6 11:16:45 2015 +0900

----------------------------------------------------------------------
 storm-core/src/jvm/backtype/storm/Config.java    |  2 +-
 .../src/jvm/backtype/storm/ConfigValidation.java | 15 +++++++--------
 .../test/clj/backtype/storm/config_test.clj      | 19 ++++++++++++-------
 3 files changed, 20 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/storm/blob/f84bd01c/storm-core/src/jvm/backtype/storm/Config.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/backtype/storm/Config.java 
b/storm-core/src/jvm/backtype/storm/Config.java
index 808572f..1549e1e 100644
--- a/storm-core/src/jvm/backtype/storm/Config.java
+++ b/storm-core/src/jvm/backtype/storm/Config.java
@@ -1134,7 +1134,7 @@ public class Config extends HashMap<String, Object> {
      * The percentage of tuples to sample to produce stats for a task.
      */
     public static final String 
TOPOLOGY_STATS_SAMPLE_RATE="topology.stats.sample.rate";
-    public static final Object TOPOLOGY_STATS_SAMPLE_RATE_SCHEMA = 
ConfigValidation.DoubleValidator;
+    public static final Object TOPOLOGY_STATS_SAMPLE_RATE_SCHEMA 
=ConfigValidation.PositiveNumberValidator;
 
     /**
      * The time period that builtin metrics data in bucketed into.

http://git-wip-us.apache.org/repos/asf/storm/blob/f84bd01c/storm-core/src/jvm/backtype/storm/ConfigValidation.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/backtype/storm/ConfigValidation.java 
b/storm-core/src/jvm/backtype/storm/ConfigValidation.java
index 24991d7..c8d0143 100644
--- a/storm-core/src/jvm/backtype/storm/ConfigValidation.java
+++ b/storm-core/src/jvm/backtype/storm/ConfigValidation.java
@@ -236,22 +236,21 @@ public class ConfigValidation {
     };
 
     /**
-     * Validates a Double.
+     * Validates a Positive Number
      */
-    public static Object DoubleValidator = new FieldValidator() {
+    public static Object PositiveNumberValidator = new FieldValidator() {
         @Override
         public void validateField(String name, Object o) throws 
IllegalArgumentException {
             if (o == null) {
                 // A null value is acceptable.
                 return;
             }
-
-            // we can provide a lenient way to convert int/long to double with 
losing some precision
-            if (o instanceof Number) {
-                return;
+            if(o instanceof Number) {
+                if(((Number)o).doubleValue() > 0.0) {
+                    return;
+                }
             }
-
-            throw new IllegalArgumentException("Field " + name + " must be an 
Double.");
+            throw new IllegalArgumentException("Field " + name + " must be a 
Positive Number");
         }
     };
 

http://git-wip-us.apache.org/repos/asf/storm/blob/f84bd01c/storm-core/test/clj/backtype/storm/config_test.clj
----------------------------------------------------------------------
diff --git a/storm-core/test/clj/backtype/storm/config_test.clj 
b/storm-core/test/clj/backtype/storm/config_test.clj
index a7498ba..d5f0240 100644
--- a/storm-core/test/clj/backtype/storm/config_test.clj
+++ b/storm-core/test/clj/backtype/storm/config_test.clj
@@ -85,14 +85,19 @@
     (is (thrown-cause? java.lang.IllegalArgumentException
           (.validateField validator "test" [-100 (inc Integer/MAX_VALUE)])))))
 
-(deftest test-double-validator
-  (let [validator ConfigValidation/DoubleValidator]
+(deftest test-positive-number-validator
+  (let [validator ConfigValidation/PositiveNumberValidator]
     (.validateField validator "test" nil)
-    (.validateField validator "test" 10)
-    ;; we can provide lenient way to convert int/long to double with losing 
precision
-    (.validateField validator "test" Integer/MAX_VALUE)
-    (.validateField validator "test" (inc Integer/MAX_VALUE))
-    (.validateField validator "test" Double/MAX_VALUE)))
+    (.validateField validator "test" 1.0)
+    (.validateField validator "test" 1)
+    (is (thrown-cause? java.lang.IllegalArgumentException
+          (.validateField validator "test" -1.0)))
+    (is (thrown-cause? java.lang.IllegalArgumentException
+          (.validateField validator "test" -1)))
+    (is (thrown-cause? java.lang.IllegalArgumentException
+          (.validateField validator "test" 0)))
+    (is (thrown-cause? java.lang.IllegalArgumentException
+          (.validateField validator "test" 0.0)))))
 
 (deftest test-topology-workers-is-integer
   (let [validator (CONFIG-SCHEMA-MAP TOPOLOGY-WORKERS)]

Reply via email to