From 93e471f838f53709eb266ca02e63c85bd0c42b0c Mon Sep 17 00:00:00 2001 From: Rafi Einstein Date: Wed, 5 May 2021 14:09:56 +0300 Subject: [PATCH] [feature-search-json] Sync with master (2) (#344) * Update to Redis 6.2.2 (#332) * Fix json.arrappend performance (#324) * improve arr_pop, arr_insert & arr_trim performance * refactor do_json_* functions * normalize index for backward (#327) * normalize index for backward * adpat for full backward with 1.0.x * fix #318 (#333) * CircleCI: macOS and benchmark fixes (#337) * Update config.yml (#338) * Add JSON.CLEAR command (#306) * Restore optional path (default is $) for JSON.DEL * Add JSON.CLEAR * Handle JSON.CLEAR with $ path * Add JSON.CLEAR - Fix failure in test:testSetInvalidPathShouldFail * Idiomatic fixes per Gavrie review Co-authored-by: Guy Korland * Update RS_VERSIONS last promoted builds * Document flags for running tests * fix arr_pop backward to 1.0 (#345) * JSON.INSERTARR backward compatability for 1.0 (#343) * Fix JSON.INSERTARR backward compatibility with 1.0.x * Update test.py * Fix build error (use of undeclared crate or module `rawmod`) * Fix format Co-authored-by: Guy Korland Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com> Co-authored-by: tomerhekredis <72793005+tomerhekredis@users.noreply.github.com> Co-authored-by: Gavrie Philipson Co-authored-by: oshadmi --- .circleci/config.yml | 8 +- Cargo.lock | 2 +- Cargo.toml | 2 +- Dockerfile | 2 +- Makefile | 10 ++ README.md | 8 ++ deps/readies | 2 +- src/lib.rs | 325 ++++++++++++++++++++++++++----------------- src/redisjson.rs | 130 ++++++++++++----- tests/pytest/test.py | 119 ++++++++++++++-- tests/qa/RS_VERSIONS | 8 +- 11 files changed, 431 insertions(+), 185 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e1fbad334..45fb9041d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -88,7 +88,9 @@ commands: name: Setup automation and install dependencies command: | ./deps/readies/bin/getpy3 + python3 -m pip list -v ./system-setup.py + python3 -m pip list -v source $HOME/.cargo/env rustc --version cargo --version @@ -100,7 +102,7 @@ commands: redis-server --version - restore_cache: keys: - - v2-dependencies-{{ arch }}-{{ checksum "Cargo.lock" }} + - v3-dependencies-{{ arch }}-{{ checksum "Cargo.lock" }} - run: name: Check formatting command: bash -l -c "make lint" @@ -111,7 +113,7 @@ commands: name: Build release command: bash -l -c "make build pack <>" - save_cache: - key: v2-dependencies-{{ arch }}-{{ checksum "Cargo.lock" }} + key: v3-dependencies-{{ arch }}-{{ checksum "Cargo.lock" }} paths: - "~/.cargo" - "./target" @@ -213,7 +215,7 @@ jobs: build-macos: macos: - xcode: 11.3.0 + xcode: 11.6.0 steps: - build-steps - test-steps diff --git a/Cargo.lock b/Cargo.lock index 49f1d4317..098dcd2c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -444,7 +444,7 @@ dependencies = [ [[package]] name = "redisjson" -version = "1.2.0" +version = "99.99.99" dependencies = [ "bson", "jsonpath_lib", diff --git a/Cargo.toml b/Cargo.toml index e777efe81..588871412 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ serde_json = "1.0" serde = "1.0" libc = "0.2" jsonpath_lib = { git="https://github.com/RedisJSON/jsonpath.git", branch="public-parser" } -# redis-module = { version="0.15.0", features = ["experimental-api"]} +# redis-module = { version="0.15", features = ["experimental-api"]} redis-module = { git = "https://github.com/RedisLabsModules/redismodule-rs", features = ["experimental-api"] } lazy_static = "1.4.0" # readies-wd40 = { path = "deps/readies/wd40" } diff --git a/Dockerfile b/Dockerfile index 09d2f30c7..486090239 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # BUILD redisfab/rejson:${VERSION}-${ARCH}-${OSNICK} -ARG REDIS_VER=6.2.1 +ARG REDIS_VER=6.2.2 # stretch|bionic|buster ARG OSNICK=buster diff --git a/Makefile b/Makefile index 7009ac1b5..6ac4977b4 100644 --- a/Makefile +++ b/Makefile @@ -9,6 +9,8 @@ include $(ROOT)/deps/readies/mk/main #---------------------------------------------------------------------------------------------- define HELP +make setup # install prerequisites + make build DEBUG=1 # build debug variant make clean # remove binary files @@ -73,6 +75,14 @@ all: build #---------------------------------------------------------------------------------------------- +setup: + ./deps/readies/bin/getpy3 + ./system-setup.py + +.PHONY: setup + +#---------------------------------------------------------------------------------------------- + lint: cargo fmt -- --check diff --git a/README.md b/README.md index b25bdd9ec..e6f88b8b1 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,14 @@ Then, build as usual: cargo build --release ``` +When running the tests, you need to explicitly specify the `test` feature to disable use of the Redis memory allocator when testing: + +```bash +cargo test --features test +``` + +If you forget to do this, you'll see an error mentioning `signal: 4, SIGILL: illegal instruction`. + ## Run ### Linux diff --git a/deps/readies b/deps/readies index 7d74e0e24..6cb2c11ed 160000 --- a/deps/readies +++ b/deps/readies @@ -1 +1 @@ -Subproject commit 7d74e0e244b4a0c9ed00c51d0451d1bd08ba4b15 +Subproject commit 6cb2c11edc19ac7442a3ccf435c0e07195fe1307 diff --git a/src/lib.rs b/src/lib.rs index 89e1ed2c0..148ca585c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,8 +4,7 @@ extern crate redis_module; use std::{i64, usize}; use redis_module::raw::RedisModuleTypeMethods; -use redis_module::{native_types::RedisType, NotifyEvent}; -use redis_module::{raw as rawmod, NextArg}; +use redis_module::{native_types::RedisType, raw as rawmod, NextArg, NotifyEvent}; use redis_module::{Context, RedisError, RedisResult, RedisValue, REDIS_OK}; use serde_json::{Number, Value}; @@ -25,6 +24,7 @@ mod redisjson; // extern crate readies_wd40; // use crate::readies_wd40::{BB, _BB, getenv}; +const JSON_ROOT_PATH: &'static str = "$"; pub const REDIS_JSON_TYPE_VERSION: i32 = 3; static REDIS_JSON_TYPE: RedisType = RedisType::new( @@ -45,7 +45,7 @@ static REDIS_JSON_TYPE: RedisType = RedisType::new( // Auxiliary data (v2) aux_load: Some(redisjson::type_methods::aux_load), aux_save: None, - aux_save_triggers: rawmod::Aux::Before as i32, + aux_save_triggers: 0, free_effort: None, unlink: None, @@ -77,7 +77,9 @@ fn json_del(ctx: &Context, args: Vec) -> RedisResult { let mut args = args.into_iter().skip(1); let key = args.next_string()?; - let path = backwards_compat_path(args.next_string()?); + let path = args + .next_string() + .map_or_else(|_| JSON_ROOT_PATH.to_string(), |v| backwards_compat_path(v)); let redis_key = ctx.open_key_writable(&key); let deleted = match redis_key.get_value::(&REDIS_JSON_TYPE)? { @@ -194,7 +196,7 @@ fn json_get(ctx: &Context, args: Vec) -> RedisResult { // path is optional -> no path found we use root "$" if paths.is_empty() { - paths.push(Path::new("$".to_string())); + paths.push(Path::new(JSON_ROOT_PATH.to_string())); } let key = ctx.open_key_writable(&key); @@ -303,20 +305,23 @@ fn json_bool_toggle(ctx: &Context, args: Vec) -> RedisResult { .get_value::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| { - value - .as_bool() - .ok_or_else(|| err_json(value, "boolean")) - .and_then(|curr| { - let result = curr ^ true; - Ok(Value::Bool(result)) - }) - }) - .map(|v| { - ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_toggle", key.as_str()); - ctx.replicate_verbatim(); - v.to_string().into() - }) + doc.value_op( + &path, + |value| { + value + .as_bool() + .ok_or_else(|| err_json(value, "boolean")) + .map(|curr| { + let result = curr ^ true; + Value::Bool(result) + }) + }, + |result| { + ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_toggle", key.as_str()); + ctx.replicate_verbatim(); + Ok(result.to_string().into()) + }, + ) .map_err(|e| e.into()) }) } @@ -343,14 +348,15 @@ where .get_value::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| { - do_json_num_op(&number, value, &op_i64, &op_f64) - }) - .map(|v| { - ctx.notify_keyspace_event(NotifyEvent::MODULE, cmd, key.as_str()); - ctx.replicate_verbatim(); - v.to_string().into() - }) + doc.value_op( + &path, + |value| do_json_num_op(&number, value, &op_i64, &op_f64), + |result| { + ctx.notify_keyspace_event(NotifyEvent::MODULE, cmd, key.as_str()); + ctx.replicate_verbatim(); + Ok(result.to_string().into()) + }, + ) .map_err(|e| e.into()) }) } @@ -411,7 +417,7 @@ fn json_str_append(ctx: &Context, args: Vec) -> RedisResult { path = backwards_compat_path(path_or_json); json = val; } else { - path = "$".to_string(); + path = JSON_ROOT_PATH.to_string(); json = path_or_json; } @@ -421,13 +427,19 @@ fn json_str_append(ctx: &Context, args: Vec) -> RedisResult { .get_value::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| do_json_str_append(&json, value)) - .map(|v| { + doc.value_op( + &path, + |value| do_json_str_append(&json, value), + |result| { ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_strappend", key.as_str()); ctx.replicate_verbatim(); - v.as_str().map_or(usize::MAX, |v| v.len()).into() - }) - .map_err(|e| e.into()) + Ok(result + .as_str() + .map_or(usize::MAX, |result| result.len()) + .into()) + }, + ) + .map_err(|e| e.into()) }) } @@ -464,31 +476,37 @@ fn json_arr_append(ctx: &Context, args: Vec) -> RedisResult { .get_value::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| do_json_arr_append(args.clone(), value)) - .map(|v| { + doc.value_op( + &path, + |value| do_json_arr_append(args.clone(), value), + |result| { ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrappend", key.as_str()); ctx.replicate_verbatim(); - v.as_array().map_or(usize::MAX, |v| v.len()).into() - }) - .map_err(|e| e.into()) + Ok(result + .as_array() + .map_or(usize::MAX, |result| result.len()) + .into()) + }, + ) + .map_err(|e| e.into()) }) } -fn do_json_arr_append(args: I, value: &Value) -> Result +fn do_json_arr_append(args: I, value: &mut Value) -> Result where I: Iterator, { - value - .as_array() - .ok_or_else(|| err_json(value, "array")) - .and_then(|curr| { - let items: Vec = args - .map(|json| serde_json::from_str(&json)) - .collect::>()?; + if !value.is_array() { + return Err(err_json(value, "array")); + } - let new_value = [curr.as_slice(), &items].concat(); - Ok(Value::Array(new_value)) - }) + let mut items: Vec = args + .map(|json| serde_json::from_str(&json)) + .collect::>()?; + + let mut new_value = value.take(); + new_value.as_array_mut().unwrap().append(&mut items); + Ok(new_value) } /// @@ -503,7 +521,7 @@ fn json_arr_index(ctx: &Context, args: Vec) -> RedisResult { let path = backwards_compat_path(args.next_string()?); let json_scalar = args.next_string()?; let start: i64 = args.next().map(|v| v.parse()).unwrap_or(Ok(0))?; - let end: i64 = args.next().map(|v| v.parse()).unwrap_or(Ok(i64::MAX))?; + let end: i64 = args.next().map(|v| v.parse()).unwrap_or(Ok(0))?; args.done()?; // TODO: Add to other functions as well to terminate args list @@ -535,43 +553,45 @@ fn json_arr_insert(ctx: &Context, args: Vec) -> RedisResult { .get_value::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| { - do_json_arr_insert(args.clone(), index, value) - }) - .map(|v| { - ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrinsert", key.as_str()); - ctx.replicate_verbatim(); - v.as_array().map_or(usize::MAX, |v| v.len()).into() - }) + doc.value_op( + &path, + |value| do_json_arr_insert(args.clone(), index, value), + |result| { + ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrinsert", key.as_str()); + ctx.replicate_verbatim(); + Ok(result + .as_array() + .map_or(usize::MAX, |result| result.len()) + .into()) + }, + ) .map_err(|e| e.into()) }) } -fn do_json_arr_insert(args: I, index: i64, value: &Value) -> Result +fn do_json_arr_insert(args: I, index: i64, value: &mut Value) -> Result where I: Iterator, { - value - .as_array() - .ok_or_else(|| err_json(value, "array")) - .and_then(|curr| { - let len = curr.len() as i64; - - if !(-len..len).contains(&index) { - return Err("ERR index out of bounds".into()); - } - - let index = index.normalize(len); - - let items: Vec = args - .map(|json| serde_json::from_str(&json)) - .collect::>()?; - - let mut new_value = curr.to_owned(); - new_value.splice(index..index, items.into_iter()); - - Ok(Value::Array(new_value)) - }) + if let Some(array) = value.as_array() { + // Verify legal index in bounds + let len = array.len() as i64; + let index = if index < 0 { len + index } else { index }; + if !(0..=len).contains(&index) { + return Err("ERR index out of bounds".into()); + } + let index = index as usize; + + let items: Vec = args + .map(|json| serde_json::from_str(&json)) + .collect::>()?; + let mut new_value = value.take(); + let curr = new_value.as_array_mut().unwrap(); + curr.splice(index..index, items.into_iter()); + Ok(new_value) + } else { + Err(err_json(value, "array")) + } } /// @@ -593,50 +613,59 @@ fn json_arr_pop(ctx: &Context, args: Vec) -> 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(("$".to_string(), i64::MAX)); + .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::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| do_json_arr_pop(index, &mut res, value)) - .map(|v| { + doc.value_op( + &path, + |value| do_json_arr_pop(index, &mut res, value), + |_result| { ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrpop", key.as_str()); ctx.replicate_verbatim(); - v - }) - .map_err(|e| e.into()) + Ok(()) // fake result doesn't use it uses `res` instead + }, + ) + .map_err(|e| e.into()) })?; - Ok(RedisJSON::serialize(&res, Format::JSON)?.into()) -} -fn do_json_arr_pop(mut index: i64, res: &mut Value, value: &Value) -> Result { - value - .as_array() - .ok_or_else(|| err_json(value, "array")) - .and_then(|curr| { - let len = curr.len() as i64; - - index = index.min(len - 1); + let result = match res { + None => ().into(), + Some(r) => RedisJSON::serialize(&r, Format::JSON)?.into(), + }; + Ok(result) +} - if index < 0 { - index += len; - } +fn do_json_arr_pop(index: i64, res: &mut Option, value: &mut Value) -> Result { + if let Some(array) = value.as_array() { + 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 = curr.to_owned(); - *res = new_value.remove(index as usize); - Ok(Value::Array(new_value)) - }) + let mut new_value = value.take(); + let curr = new_value.as_array_mut().unwrap(); + *res = Some(curr.remove(index as usize)); + Ok(new_value) + } else { + Err(err_json(value, "array")) + } } /// @@ -656,33 +685,42 @@ fn json_arr_trim(ctx: &Context, args: Vec) -> RedisResult { .get_value::(&REDIS_JSON_TYPE)? .ok_or_else(RedisError::nonexistent_key) .and_then(|doc| { - doc.value_op(&path, |value| do_json_arr_trim(start, stop, &value)) - .map(|v| { + doc.value_op( + &path, + |value| do_json_arr_trim(start, stop, value), + |result| { ctx.notify_keyspace_event(NotifyEvent::MODULE, "json_arrtrim", key.as_str()); ctx.replicate_verbatim(); - v.as_array().map_or(usize::MAX, |v| v.len()).into() - }) - .map_err(|e| e.into()) + Ok(result + .as_array() + .map_or(usize::MAX, |result| result.len()) + .into()) + }, + ) + .map_err(|e| e.into()) }) } -fn do_json_arr_trim(start: i64, stop: i64, value: &Value) -> Result { - value - .as_array() - .ok_or_else(|| err_json(value, "array")) - .and_then(|curr| { - let len = curr.len() as i64; - let stop = stop.normalize(len); +fn do_json_arr_trim(start: i64, stop: i64, value: &mut Value) -> Result { + if let Some(array) = value.as_array() { + let len = array.len() as i64; + let stop = stop.normalize(len); - let range = if start > len || start > stop as i64 { - 0..0 // Return an empty array - } else { - start.normalize(len)..(stop + 1) - }; + let range = if start > len || start > stop as i64 { + 0..0 // Return an empty array + } else { + start.normalize(len)..(stop + 1) + }; - let res = &curr[range]; - Ok(Value::Array(res.to_vec())) - }) + let mut new_value = value.take(); + let curr = new_value.as_array_mut().unwrap(); + curr.rotate_left(range.start); + curr.resize(range.end - range.start, Value::Null); + + Ok(new_value) + } else { + Err(err_json(value, "array")) + } } /// @@ -710,6 +748,32 @@ fn json_obj_len(ctx: &Context, args: Vec) -> RedisResult { json_len(ctx, args, |doc, path| doc.obj_len(path)) } +/// +/// JSON.CLEAR [path ...] +/// +fn json_clear(ctx: &Context, args: Vec) -> RedisResult { + let mut args = args.into_iter().skip(1); + let key = args.next_string()?; + let paths = args.map(Path::new).collect::>(); + + let paths = if paths.is_empty() { + vec![Path::new(JSON_ROOT_PATH.to_string())] + } else { + paths + }; + + // FIXME: handle multi paths + let key = ctx.open_key_writable(&key); + let deleted = match key.get_value::(&REDIS_JSON_TYPE)? { + Some(doc) => { + let res = doc.clear(paths.first().unwrap().fixed.as_str())?; + ctx.replicate_verbatim(); + res + } + None => 0, + }; + Ok(deleted.into()) +} /// /// JSON.DEBUG /// @@ -751,7 +815,9 @@ fn json_resp(ctx: &Context, args: Vec) -> RedisResult { let mut args = args.into_iter().skip(1); let key = args.next_string()?; - let path = backwards_compat_path(args.next_string()?); + let path = args + .next_string() + .map_or_else(|_| JSON_ROOT_PATH.to_string(), |v| backwards_compat_path(v)); let key = ctx.open_key(&key); match key.get_value::(&REDIS_JSON_TYPE)? { @@ -852,6 +918,7 @@ redis_module! { ["json.arrtrim", json_arr_trim, "write", 1,1,1], ["json.objkeys", json_obj_keys, "readonly", 1,1,1], ["json.objlen", json_obj_len, "readonly", 1,1,1], + ["json.clear", json_clear, "write", 1,1,1], ["json.debug", json_debug, "readonly", 1,1,1], ["json.forget", json_del, "write", 1,1,1], ["json.resp", json_resp, "readonly", 1,1,1], diff --git a/src/redisjson.rs b/src/redisjson.rs index b8054037a..5f7c79774 100644 --- a/src/redisjson.rs +++ b/src/redisjson.rs @@ -202,6 +202,31 @@ impl RedisJSON { Ok(deleted) } + pub fn clear(&mut self, path: &str) -> Result { + let current_data = self.data.take(); + let mut cleared = 0; + + let clear_func = &mut |v| match v { + Value::Object(mut obj) => { + obj.clear(); + cleared += 1; + Some(Value::from(obj)) + } + Value::Array(mut arr) => { + arr.clear(); + cleared += 1; + Some(Value::from(arr)) + } + _ => Some(v), + }; + + self.data = if path == "$" { + clear_func(current_data).unwrap() + } else { + jsonpath_lib::replace_with(current_data, path, clear_func)? + }; + Ok(cleared) + } pub fn to_string(&self, path: &str, format: Format) -> Result { let results = self.get_first(path)?; Self::serialize(results, format) @@ -298,20 +323,35 @@ impl RedisJSON { } let v: Value = serde_json::from_str(scalar)?; - let end: usize = if end == 0 || end == -1 { - // default end of array - arr.len() - 1 + let len = arr.len() as i64; + + // Normalize start + let start = if start < 0 { + 0.max(len + start) + } else { + // start >= 0 + start.min(len - 1) + }; + + // Normalize end + let end = if end == 0 { + len + } else if end < 0 { + len + end } else { - (end as usize).min(arr.len() - 1) + // end > 0 + end.min(len) }; - let start = start.max(0) as usize; + if end < start { + // don't search at all return Ok(-1); } - let slice = &arr[start..=end]; + + let slice = &arr[start as usize..end as usize]; match slice.iter().position(|r| r == &v) { - Some(i) => Ok((start + i) as i64), + Some(i) => Ok((start as usize + i) as i64), None => Ok(-1), } } else { @@ -358,20 +398,28 @@ impl RedisJSON { } } - pub fn value_op(&mut self, path: &str, mut fun: F) -> Result + pub fn value_op(&mut self, path: &str, mut op_fun: F, res_func: R) -> Result where - F: FnMut(&Value) -> Result, + F: FnMut(&mut Value) -> Result, + R: Fn(&Value) -> Result, { + // take the root before updating the value must be returned at the end let current_data = self.data.take(); let mut errors = vec![]; - let mut result = Value::Null; // TODO handle case where path not found - - let mut collect_fun = |value: Value| { - fun(&value) - .map(|new_value| { - result = new_value.clone(); - new_value + let mut result = None; + + // A wrapper function that is called by replace_with + // calls op_fun and then res_func + let mut collect_fun = |mut value: Value| { + op_fun(&mut value) + .and_then(|new_value| { + // after calling op_fun calling res_func + // to prepae the command result + res_func(&new_value).map(|res| { + result = Some(res); + new_value + }) }) .map_err(|e| { errors.push(e); @@ -379,27 +427,35 @@ impl RedisJSON { .unwrap_or(value) }; - self.data = if path == "$" { + if path == "$" { // root needs special handling - collect_fun(current_data) + self.data = collect_fun(current_data) } else { - SelectorMut::new() - .str_path(path) - .and_then(|selector| { - Ok(selector - .value(current_data.clone()) - .replace_with(&mut |v| Some(collect_fun(v)))? - .take() - .unwrap_or(Value::Null)) - }) - .map_err(|e| { + match SelectorMut::new().str_path(path) { + Ok(selector) => { + let replace_result = selector + .value(current_data) + .replace_with(&mut |v| Some(collect_fun(v))); + + if let Err(e) = replace_result { + errors.push(e.into()); + } + // reassign the modified root + self.data = selector.take().unwrap(); + } + Err(e) => { errors.push(e.into()); - }) - .unwrap_or(current_data) + // reassign the original root + self.data = current_data; + } + } }; match errors.len() { - 0 => Ok(result), + 0 => match result { + Some(r) => Ok(r), + None => Err(format!("Path '{}' does not exist", path).into()), + }, 1 => Err(errors.remove(0)), _ => Err(errors.into_iter().map(|e| e.msg).collect::().into()), } @@ -441,7 +497,17 @@ pub mod type_methods { 0 => RedisJSON { data: backward::json_rdb_load(rdb), }, - 2 | 3 => { + 2 => { + let data = raw::load_string(rdb); + // Backward support for modules that had AUX field for RediSarch + // TODO remove in future versions + if raw::load_unsigned(rdb) > 0 { + raw::load_string(rdb); + raw::load_string(rdb); + } + RedisJSON::from_str(&data, Format::JSON).unwrap() + } + 3 => { let data = raw::load_string(rdb); if raw::load_unsigned(rdb) > 0 { raw::load_string(rdb); diff --git a/tests/pytest/test.py b/tests/pytest/test.py index 517c47eee..2dec4205e 100644 --- a/tests/pytest/test.py +++ b/tests/pytest/test.py @@ -310,7 +310,7 @@ def testToggleCommand(env): # Test Toggeling Empty Path r.assertOk(r.execute_command('JSON.SET', 'test', '.', '{"foo":"bar"}')) - r.assertEqual(r.execute_command('JSON.TOGGLE', 'test', '.bar'), 'null') + r.expect('JSON.TOGGLE', 'test', '.bar').raiseError() # Test Toggeling Non Boolean r.assertOk(r.execute_command('JSON.SET', 'test', '.', '{"foo":"bar"}')) @@ -409,6 +409,54 @@ def testObjectCRUD(env): r.assertEqual(1, r.execute_command('JSON.DEL', 'test', '.')) r.assertIsNone(r.execute_command('JSON.GET', 'test', '.')) + # Test deleting with default (root) path + r.assertOk(r.execute_command('JSON.SET', 'test', '.', '{"foo": "bar"}')) + r.assertEqual(1, r.execute_command('JSON.DEL', 'test')) + r.assertIsNone(r.execute_command('JSON.GET', 'test', '.')) + +def testClear(env): + """Test JSON.CLEAR command""" + + r = env + r.expect('JSON.SET', 'test', '.', r'{"n":42,"s":"42","arr":[{"n":44},"s",{"n":{"a":1,"b":2}},{"n2":{"x":3.02,"n":["to","be","cleared",4],"y":4.91}}]}')\ + .ok() + + # Test get multi results (using .. recursive descent) + # TODO: Enable when supporting multi results (and not only the first) + #r.expect('JSON.GET', 'test', '$..n').equal([42,44,{"a": 1,"b": 2},["to","be","cleared",4]]) + + # Make sure specific obj content exists before clear + obj_content = r'{"a":1,"b":2}' + r.expect('JSON.GET', 'test', '$.arr[2].n').equal(obj_content) + # Make sure specific arr content exists before clear + arr_content = r'["to","be","cleared",4]' + r.expect('JSON.GET', 'test', '$.arr[3].n2.n').equal(arr_content) + + # Clear obj and arr with specific paths + r.expect('JSON.CLEAR', 'test', '$.arr[2].n').equal(1) + r.expect('JSON.CLEAR', 'test', '$.arr[3].n2.n').equal(1) + + # Fail clear on inappropriate path (not obj or arr) + r.expect('JSON.CLEAR', 'test', '$.arr[1]').equal(0) + + # Make sure specific obj content was cleared + r.expect('JSON.GET', 'test', '$.arr[2].n').equal('{}') + # Make sure specific arr content was cleared + r.expect('JSON.GET', 'test', '$.arr[3].n2.n').equal('[]') + + # Make sure only appropriate content (obj and arr) was cleared - and that errors were printed for inappropriate content (string and numeric) + # TODO: Enable when supporting multi results (and not only the first) + #r.expect('JSON.GET', 'test', '$..n').equal([42, 44, {}, []]) + + # Clear root + # TODO: switch order of the following paths and expect .equals(2) when supporting multi-paths + r.expect('JSON.CLEAR', 'test', '$', '$.arr[2].n').equal(1) + r.expect('JSON.GET', 'test', '$').equal('{}') + + r.expect('JSON.SET', 'test', '$', obj_content).ok() + r.expect('JSON.CLEAR', 'test').equal(1) + r.expect('JSON.GET', 'test', '$').equal('{}') + def testArrayCRUD(env): """Test JSON Array CRUDness""" @@ -478,25 +526,68 @@ def testArrIndexCommand(env): """Test JSON.ARRINDEX command""" r = env + r.assertOk(r.execute_command('JSON.SET', 'test', + '.', '{ "arr": [0, 1, 2, 3, 2, 1, 0] }')) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0), 0) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 3), 3) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 4), -1) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 1), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, -1), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 6), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 4, -0), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 5, -1), -1) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, -2, 6), -1) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '"foo"'), -1) + + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', 4, '[4]'), 8) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 3), 3) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, 3), 5) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '[4]'), 4) + +def testArrInsertCommand(env): + """Test JSON.ARRINSERT command""" + r = env + + r.assertOk(r.execute_command('JSON.SET', 'test', '.', '{ "arr": [] }')) + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', 0, '1'), 1) + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', -1, '2'), 2) + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', -2, '3'), 3) + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', 3, '4'), 4) + r.assertEqual(r.execute_command('JSON.GET', 'test', '.arr'), "[3,2,1,4]") + + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', 1, '5'), 5) + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', -2, '6'), 6) + r.assertEqual(r.execute_command('JSON.GET', 'test', '.arr'), "[3,5,2,6,1,4]") + + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', -3, '7', '{"A":"Z"}', '9'), 9) + r.assertEqual(r.execute_command('JSON.GET', 'test', '.arr'), '[3,5,2,7,{"A":"Z"},9,6,1,4]') + + r.expect('JSON.ARRINSERT', 'test', '.arr', -10, '10').raiseError() + r.expect('JSON.ARRINSERT', 'test', '.arr', 10, '10').raiseError() + +def testArrIndexMixCommand(env): + """Test JSON.ARRINDEX command with mixed values""" + r = env + r.assertOk(r.execute_command('JSON.SET', 'test', '.', '{ "arr": [0, 1, 2, 3, 2, 1, 0, {"val": 4}, {"val": 9}, [3,4,8], ["a", "b", 8]] }')) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0), 0) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 3), 3) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 4), -1) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 1), 6) - # r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, -1), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, -5), 6) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 6), 6) - # r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 4, -0), 6) - # r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 5, -1), -1) - # r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, -2, 6), -1) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 4, -0), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 5, -1), 6) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, -2, 6), -1) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '"foo"'), -1) - # r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', 4, '[4]'), 8) + r.assertEqual(r.execute_command('JSON.ARRINSERT', 'test', '.arr', 4, '[4]'), 12) r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 3), 3) - r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, 3), 4) - # r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '[4]'), -1) - r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '{\"val\":4}'), 7) - r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '["a", "b", 8]'), 10) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, 3), 5) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '[4]'), 4) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '{\"val\":4}'), 8) + r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '["a", "b", 8]'), 11) def testArrTrimCommand(env): """Test JSON.ARRTRIM command""" @@ -530,9 +621,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""" diff --git a/tests/qa/RS_VERSIONS b/tests/qa/RS_VERSIONS index b746160af..b78ee7dbe 100644 --- a/tests/qa/RS_VERSIONS +++ b/tests/qa/RS_VERSIONS @@ -1,5 +1,5 @@ -6.0.20-52 -6.0.6-35 -6.0.8-30 -6.0.12-41 +6.0.20-69 +6.0.6-39 +6.0.8-32 +6.0.12-58 100.0.0-2199