fix arr_pop backward to 1.0 (RedisJSON#345)

gkorland · web-flow · commit 33837d26f64b · 2021-05-05T11:36:19.000+03:00
diff --git a/src/lib.rs b/src/lib.rs
@@ -606,13 +606,13 @@ fn json_arr_pop(ctx: &Context, args: Vec<String>) -> RedisResult {
         .next()
         .map(|p| {
             let path = backwards_compat_path(p);
-            let index = args.next_i64().unwrap_or(i64::MAX);
+            let index = args.next_i64().unwrap_or(-1);
             (path, index)
         })
         .unwrap_or((JSON_ROOT_PATH.to_string(), i64::MAX));
 
     let redis_key = ctx.open_key_writable(&key);
-    let mut res = Value::Null;
+    let mut res = None;
 
     redis_key
         .get_value::<RedisJSON>(&REDIS_JSON_TYPE)?
@@ -629,25 +629,32 @@ fn json_arr_pop(ctx: &Context, args: Vec<String>) -> RedisResult {
             )
             .map_err(|e| e.into())
         })?;
-    Ok(RedisJSON::serialize(&res, Format::JSON)?.into())
+
+    let result = match res {
+        None => ().into(),
+        Some(r) => RedisJSON::serialize(&r, Format::JSON)?.into(),
+    };
+    Ok(result)
 }
 
-fn do_json_arr_pop(mut index: i64, res: &mut Value, value: &mut Value) -> Result<Value, Error> {
+fn do_json_arr_pop(index: i64, res: &mut Option<Value>, value: &mut Value) -> Result<Value, Error> {
     if let Some(array) = value.as_array() {
-        // Verify legel index in bounds
-        let len = array.len() as i64;
-        index = index.min(len - 1);
-        if index < 0 {
-            index += len;
+        if array.is_empty() {
+            *res = None;
+            return Ok(value.clone());
         }
 
-        if index >= len || index < 0 {
-            return Err("ERR index out of bounds".into());
-        }
+        // Verify legel index in bounds
+        let len = array.len() as i64;
+        let index = if index < 0 {
+            0.max(len + index)
+        } else {
+            index.min(len - 1)
+        } as usize;
 
         let mut new_value = value.take();
         let curr = new_value.as_array_mut().unwrap();
-        *res = curr.remove(index as usize);
+        *res = Some(curr.remove(index as usize));
         Ok(new_value)
     } else {
         Err(err_json(value, "array"))
diff --git a/tests/pytest/test.py b/tests/pytest/test.py
@@ -600,9 +600,11 @@ def testArrPopCommand(env):
     r.assertEqual('1', r.execute_command('JSON.ARRPOP', 'test', '.', 0))
     r.assertEqual('4', r.execute_command('JSON.ARRPOP', 'test', '.', 2))
     r.assertEqual('6', r.execute_command('JSON.ARRPOP', 'test', '.', 99))
-    # r.assertEqual('2', r.execute_command('JSON.ARRPOP', 'test', '.', -99))
-    # r.assertEqual('3', r.execute_command('JSON.ARRPOP', 'test'))
-    # r.assertIsNone(r.execute_command('JSON.ARRPOP', 'test'))
+    r.assertEqual('2', r.execute_command('JSON.ARRPOP', 'test', '.', -99))
+    r.assertEqual('3', r.execute_command('JSON.ARRPOP', 'test'))
+    r.assertIsNone(r.execute_command('JSON.ARRPOP', 'test'))
+    r.assertIsNone(r.execute_command('JSON.ARRPOP', 'test', '.'))
+    r.assertIsNone(r.execute_command('JSON.ARRPOP', 'test', '.', 2))
 
 def testTypeCommand(env):
     """Test JSON.TYPE command"""
