fix #55 return the right type on json.num* call

RedisJSON · Sep 11, 2019 · 62c47fa · 62c47fa
1 parent 4c46496
commit 62c47fa
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 33 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -299,33 +299,39 @@ where
 
     let key = args.next_string()?;
     let path = backwards_compat_path(args.next_string()?);
-    let number: f64 = args.next_string()?.parse()?;
+    let number = args.next_string()?;
 
     let key = ctx.open_key_writable(&key);
 
     key.get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
         .ok_or_else(RedisError::nonexistent_key)
         .and_then(|doc| {
-            doc.value_op(&path, |value| do_json_num_op(&fun, number, value))
+            doc.value_op(&path, |value| do_json_num_op(&fun, &number, value))
                 .map(|v| v.to_string().into())
                 .map_err(|e| e.into())
         })
 }
 
-fn do_json_num_op<F>(fun: F, number: f64, value: &Value) -> Result<Value, Error>
+fn do_json_num_op<F>(fun: F, number: &String, value: &Value) -> Result<Value, Error>
 where
     F: FnOnce(f64, f64) -> f64,
 {
-    value
-        .as_f64()
-        .ok_or_else(|| err_json(value, "number"))
-        .and_then(|curr_value| {
-            let res = fun(curr_value, number);
-
-            Number::from_f64(res)
-                .ok_or(Error::from("ERR cannot represent result as Number"))
-                .map(Value::Number)
-        })
+    if let Value::Number(curr_value) = value {
+        let in_value = serde_json::from_str(number.as_str())?;
+        if let Value::Number(value) = in_value {
+            // TODO avoid convert to f64 when not needed 
+            let num_res = fun(curr_value.as_f64().unwrap(), value.as_f64().unwrap());
+            if curr_value.is_f64() || value.is_f64() {
+                Ok(num_res.into())
+            } else {
+                Ok((num_res as i64).into())
+            }
+        } else {
+            Err(err_json(&in_value, "number"))
+        }
+    } else {
+        Err(err_json(value, "number"))
+    }
 }
 
 fn err_json(value: &Value, expected_value: &'static str) -> Error {

diff --git a/test/pytest/test.py b/test/pytest/test.py
@@ -599,10 +599,10 @@ def testNumIncrCommand(self):
             r.flushdb()
 
             self.assertOk(r.execute_command('JSON.SET', 'test', '.', '{ "foo": 0, "bar": "baz" }'))
-            # self.assertEqual('1', r.execute_command('JSON.NUMINCRBY', 'test', '.foo', 1))
-            # self.assertEqual('1', r.execute_command('JSON.GET', 'test', '.foo'))
-            # self.assertEqual('3', r.execute_command('JSON.NUMINCRBY', 'test', '.foo', 2))
-            # self.assertEqual('3.5', r.execute_command('JSON.NUMINCRBY', 'test', '.foo', .5))
+            self.assertEqual('1', r.execute_command('JSON.NUMINCRBY', 'test', '.foo', 1))
+            self.assertEqual('1', r.execute_command('JSON.GET', 'test', '.foo'))
+            self.assertEqual('3', r.execute_command('JSON.NUMINCRBY', 'test', '.foo', 2))
+            self.assertEqual('3.5', r.execute_command('JSON.NUMINCRBY', 'test', '.foo', .5))
 
             # test a wrong type
             with self.assertRaises(redis.exceptions.ResponseError) as cm:
@@ -612,22 +612,22 @@ def testNumIncrCommand(self):
     #         with self.assertRaises(redis.exceptions.ResponseError) as cm:
     #             r.execute_command('JSON.NUMINCRBY', 'test', '.fuzz', 1)
     #
-    #         # test issue #9
-    #         self.assertOk(r.execute_command('JSON.SET', 'num', '.', '0'))
-    #         self.assertEqual('1', r.execute_command('JSON.NUMINCRBY', 'num', '.', 1))
-    #         self.assertEqual('2.5', r.execute_command('JSON.NUMINCRBY', 'num', '.', 1.5))
-    #
-    #         # test issue 55
-    #         self.assertOk(r.execute_command('JSON.SET', 'foo', '.', '{"foo":0,"bar":42}'))
-    #         # Get the document once
-    #         r.execute_command('JSON.GET', 'foo', '.')
-    #         self.assertEqual('1', r.execute_command('JSON.NUMINCRBY', 'foo', 'foo', 1))
-    #         self.assertEqual('84', r.execute_command('JSON.NUMMULTBY', 'foo', 'bar', 2))
-    #         res = json.loads(r.execute_command('JSON.GET', 'foo', '.'))
-    #         self.assertEqual(1, res['foo'])
-    #         self.assertEqual(84, res['bar'])
-    #
-    #
+            # test issue #9
+            self.assertOk(r.execute_command('JSON.SET', 'num', '.', '0'))
+            self.assertEqual('1', r.execute_command('JSON.NUMINCRBY', 'num', '.', 1))
+            self.assertEqual('2.5', r.execute_command('JSON.NUMINCRBY', 'num', '.', 1.5))
+
+            # test issue 55
+            self.assertOk(r.execute_command('JSON.SET', 'foo', '.', '{"foo":0,"bar":42}'))
+            # Get the document once
+            r.execute_command('JSON.GET', 'foo', '.')
+            self.assertEqual('1', r.execute_command('JSON.NUMINCRBY', 'foo', 'foo', 1))
+            self.assertEqual('84', r.execute_command('JSON.NUMMULTBY', 'foo', 'bar', 2))
+            res = json.loads(r.execute_command('JSON.GET', 'foo', '.'))
+            self.assertEqual(1, res['foo'])
+            self.assertEqual(84, res['bar'])
+
+
     def testStrCommands(self):
         """Test JSON.STRAPPEND and JSON.STRLEN commands"""