Skip to content

Commit c43dd20

Browse files
oshadmirafie
oshadmi
and
rafie
authored
error messages - prefix with ERR or WRONGTYPE (RedisJSON#525)
* error messages - prefix with ERR or WRONGTYPE * error messages - prefix with ERR or WRONGTYPE (2) * error messages - prefix with ERR or WRONGTYPE (3) * error messages - prefix with ERR or WRONGTYPE (4) Co-authored-by: Rafi Einstein <rafi@redislabs.com>
1 parent decd330 commit c43dd20

File tree

4 files changed

+277
-84
lines changed

4 files changed

+277
-84
lines changed

‎src/commands.rs

+71-64
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
1+
use crate::error::Error;
12
use crate::formatter::RedisJsonFormatter;
3+
use crate::manager::err_msg_json_path_doesnt_exist_with_param;
4+
use crate::manager::{err_msg_json_expected, err_msg_json_path_doesnt_exist_with_param_or};
25
use crate::manager::{AddUpdateInfo, Manager, ReadHolder, SetUpdateInfo, UpdateInfo, WriteHolder};
6+
use crate::nodevisitor::{StaticPathElement, StaticPathParser, VisitStatus};
37
use crate::redisjson::{normalize_arr_indices, Format, Path};
48
use jsonpath_lib::select::select_value::{SelectValue, SelectValueType};
9+
use jsonpath_lib::select::Selector;
510
use redis_module::{Context, RedisValue};
611
use redis_module::{NextArg, RedisError, RedisResult, RedisString, REDIS_OK};
712
use std::cmp::Ordering;
813

9-
use jsonpath_lib::select::Selector;
10-
11-
use crate::nodevisitor::{StaticPathElement, StaticPathParser, VisitStatus};
12-
13-
use crate::error::Error;
14-
1514
use crate::redisjson::SetOptions;
1615

1716
use serde_json::{Number, Value};
@@ -87,7 +86,9 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
8786
let results = self.get_values(path)?;
8887
match results.first() {
8988
Some(s) => Ok(s),
90-
None => Err("ERR path does not exist".into()),
89+
None => Err(err_msg_json_path_doesnt_exist_with_param(path)
90+
.as_str()
91+
.into()),
9192
}
9293
}
9394

@@ -104,10 +105,9 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
104105
.collect::<Vec<RedisValue>>()
105106
.into())
106107
} else {
107-
Err(RedisError::String(format!(
108-
"Path '{}' does not exist",
109-
path.get_path()
110-
)))
108+
Err(RedisError::String(
109+
err_msg_json_path_doesnt_exist_with_param(path.get_original()),
110+
))
111111
}
112112
}
113113
}
@@ -204,7 +204,7 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
204204
acc
205205
});
206206
if let Some(p) = missing_path {
207-
return Err(format!("ERR path {} does not exist", p).into());
207+
return Err(err_msg_json_path_doesnt_exist_with_param(p.as_str()).into());
208208
}
209209
Ok(Self::serialize_object(&temp_doc, indent, newline, space).into())
210210
}
@@ -233,7 +233,7 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
233233
format: Format,
234234
) -> Result<RedisValue, Error> {
235235
if format == Format::BSON {
236-
return Err("Soon to come...".into());
236+
return Err("ERR Soon to come...".into());
237237
}
238238
let is_legacy = !paths.iter().any(|p| !p.is_legacy());
239239
if paths.len() > 1 {
@@ -291,9 +291,9 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
291291
// if we reach here with array path we must be out of range
292292
// otherwise the path would be valid to be set and we would not
293293
// have reached here!!
294-
Err("array index out of range".into())
294+
Err("ERR array index out of range".into())
295295
} else {
296-
Err("path not an object or array".into())
296+
Err("ERR path not an object or array".into())
297297
}
298298
}
299299

@@ -325,7 +325,7 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
325325
pub fn serialize(results: &V, format: Format) -> Result<String, Error> {
326326
let res = match format {
327327
Format::JSON => serde_json::to_string(results)?,
328-
Format::BSON => return Err("Soon to come...".into()), //results.into() as Bson,
328+
Format::BSON => return Err("ERR Soon to come...".into()), //results.into() as Bson,
329329
};
330330
Ok(res)
331331
}
@@ -378,23 +378,35 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
378378
let first = self.get_first(path)?;
379379
match first.get_type() {
380380
SelectValueType::String => Ok(first.get_str().len()),
381-
_ => Err("ERR wrong type of path value".into()),
381+
_ => Err(
382+
err_msg_json_expected("string", self.get_type(path).unwrap().as_str())
383+
.as_str()
384+
.into(),
385+
),
382386
}
383387
}
384388

385389
pub fn arr_len(&self, path: &str) -> Result<usize, Error> {
386390
let first = self.get_first(path)?;
387391
match first.get_type() {
388392
SelectValueType::Array => Ok(first.len().unwrap()),
389-
_ => Err("ERR wrong type of path value".into()),
393+
_ => Err(
394+
err_msg_json_expected("array", self.get_type(path).unwrap().as_str())
395+
.as_str()
396+
.into(),
397+
),
390398
}
391399
}
392400

393401
pub fn obj_len(&self, path: &str) -> Result<usize, Error> {
394402
let first = self.get_first(path)?;
395403
match first.get_type() {
396404
SelectValueType::Object => Ok(first.len().unwrap()),
397-
_ => Err("ERR wrong type of path value".into()),
405+
_ => Err(
406+
err_msg_json_expected("object", self.get_type(path).unwrap().as_str())
407+
.as_str()
408+
.into(),
409+
),
398410
}
399411
}
400412

@@ -502,9 +514,11 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
502514
}
503515

504516
pub fn obj_keys(&self, path: &str) -> Result<Box<dyn Iterator<Item = &'_ str> + '_>, Error> {
505-
self.get_first(path)?
506-
.keys()
507-
.ok_or_else(|| "ERR wrong type of path value".into())
517+
self.get_first(path)?.keys().ok_or_else(|| {
518+
err_msg_json_expected("object", self.get_type(path).unwrap().as_str())
519+
.as_str()
520+
.into()
521+
})
508522
}
509523
}
510524

@@ -708,10 +722,9 @@ where
708722
let res = get_all_values_and_paths(path, doc)?;
709723
match res.is_empty() {
710724
false => Ok(filter_paths(res, f)),
711-
_ => Err(RedisError::String(format!(
712-
"Path '{}' does not exist",
713-
path
714-
))),
725+
_ => Err(RedisError::String(
726+
err_msg_json_path_doesnt_exist_with_param(path),
727+
)),
715728
}
716729
}
717730

@@ -726,10 +739,9 @@ where
726739
let res = get_all_values_and_paths(path, doc)?;
727740
match res.is_empty() {
728741
false => Ok(filter_values(res, f)),
729-
_ => Err(RedisError::String(format!(
730-
"Path '{}' does not exist",
731-
path
732-
))),
742+
_ => Err(RedisError::String(
743+
err_msg_json_path_doesnt_exist_with_param(path),
744+
)),
733745
}
734746
}
735747

@@ -1016,10 +1028,9 @@ where
10161028
redis_key.apply_changes(ctx, cmd)?;
10171029
Ok(res.unwrap().to_string().into())
10181030
} else {
1019-
Err(RedisError::String(format!(
1020-
"Path '{}' does not exist or does not contains a number",
1021-
path
1022-
)))
1031+
Err(RedisError::String(
1032+
err_msg_json_path_doesnt_exist_with_param_or(path, "does not contains a number"),
1033+
))
10231034
}
10241035
}
10251036

@@ -1109,10 +1120,9 @@ where
11091120
redis_key.apply_changes(ctx, "json.toggle")?;
11101121
Ok(res.to_string().into())
11111122
} else {
1112-
Err(RedisError::String(format!(
1113-
"Path '{}' does not exist or not a bool",
1114-
path
1115-
)))
1123+
Err(RedisError::String(
1124+
err_msg_json_path_doesnt_exist_with_param_or(path, "not a bool"),
1125+
))
11161126
}
11171127
}
11181128

@@ -1201,10 +1211,9 @@ where
12011211
redis_key.apply_changes(ctx, "json.strappend")?;
12021212
Ok(res.unwrap().into())
12031213
} else {
1204-
Err(RedisError::String(format!(
1205-
"Path '{}' does not exist or not a string",
1206-
path
1207-
)))
1214+
Err(RedisError::String(
1215+
err_msg_json_path_doesnt_exist_with_param_or(path, "not a string"),
1216+
))
12081217
}
12091218
}
12101219

@@ -1281,14 +1290,14 @@ pub fn command_json_arr_append<M: Manager>(
12811290
if !path.is_legacy() {
12821291
json_arr_append::<M>(&mut redis_key, ctx, path.get_path(), args)
12831292
} else {
1284-
json_arr_append_legacy::<M>(&mut redis_key, ctx, path.get_path(), args)
1293+
json_arr_append_legacy::<M>(&mut redis_key, ctx, &path, args)
12851294
}
12861295
}
12871296

12881297
fn json_arr_append_legacy<M>(
12891298
redis_key: &mut M::WriteHolder,
12901299
ctx: &Context,
1291-
path: &str,
1300+
path: &Path,
12921301
args: Vec<M::O>,
12931302
) -> RedisResult
12941303
where
@@ -1297,12 +1306,13 @@ where
12971306
let root = redis_key
12981307
.get_value()?
12991308
.ok_or_else(RedisError::nonexistent_key)?;
1300-
let mut paths = find_paths(path, root, |v| v.get_type() == SelectValueType::Array)?;
1309+
let mut paths = find_paths(path.get_path(), root, |v| {
1310+
v.get_type() == SelectValueType::Array
1311+
})?;
13011312
if paths.is_empty() {
1302-
Err(RedisError::String(format!(
1303-
"Path '{}' does not exist",
1304-
path
1305-
)))
1313+
Err(RedisError::String(
1314+
err_msg_json_path_doesnt_exist_with_param_or(path.get_original(), "not an array"),
1315+
))
13061316
} else if paths.len() == 1 {
13071317
let res = redis_key.arr_append(paths.pop().unwrap(), args)?;
13081318
redis_key.apply_changes(ctx, "json.arrappend")?;
@@ -1384,9 +1394,9 @@ pub fn command_json_arr_index<M: Manager>(
13841394
let is_legacy = path.is_legacy();
13851395
let scalar_value: Value = serde_json::from_str(json_scalar)?;
13861396
if !is_legacy && (scalar_value.is_array() || scalar_value.is_object()) {
1387-
return Err(RedisError::String(format!(
1388-
"ERR expected scalar but found {}",
1389-
json_scalar
1397+
return Err(RedisError::String(err_msg_json_expected(
1398+
"scalar",
1399+
json_scalar,
13901400
)));
13911401
}
13921402

@@ -1489,10 +1499,9 @@ where
14891499
redis_key.apply_changes(ctx, "json.arrinsert")?;
14901500
Ok(res.unwrap().into())
14911501
} else {
1492-
Err(RedisError::String(format!(
1493-
"Path '{}' does not exist or not an array",
1494-
path
1495-
)))
1502+
Err(RedisError::String(
1503+
err_msg_json_path_doesnt_exist_with_param_or(path, "not an array"),
1504+
))
14961505
}
14971506
}
14981507

@@ -1624,10 +1633,9 @@ where
16241633
None => Ok(().into()),
16251634
}
16261635
} else {
1627-
Err(RedisError::String(format!(
1628-
"Path '{}' does not exist or not an array",
1629-
path
1630-
)))
1636+
Err(RedisError::String(
1637+
err_msg_json_path_doesnt_exist_with_param_or(path, "not an array"),
1638+
))
16311639
}
16321640
}
16331641

@@ -1706,10 +1714,9 @@ where
17061714
redis_key.apply_changes(ctx, "json.arrtrim")?;
17071715
Ok(res.unwrap().into())
17081716
} else {
1709-
Err(RedisError::String(format!(
1710-
"Path '{}' does not exist or not an array",
1711-
path
1712-
)))
1717+
Err(RedisError::String(
1718+
err_msg_json_path_doesnt_exist_with_param_or(path, "not an array"),
1719+
))
17131720
}
17141721
}
17151722

‎src/manager.rs

+30-12
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,31 @@ pub trait Manager {
8989
}
9090

9191
fn err_json(value: &Value, expected_value: &'static str) -> Error {
92-
Error::from(format!(
93-
"ERR wrong type of path value - expected {} but found {}",
92+
Error::from(err_msg_json_expected(
9493
expected_value,
95-
RedisJSON::value_name(value)
94+
RedisJSON::value_name(value),
9695
))
9796
}
9897

98+
pub(crate) fn err_msg_json_expected<'a>(expected_value: &'static str, found: &str) -> String {
99+
format!(
100+
"WRONGTYPE wrong type of path value - expected {} but found {}",
101+
expected_value, found
102+
)
103+
}
104+
105+
pub(crate) fn err_msg_json_path_doesnt_exist_with_param(path: &str) -> String {
106+
format!("ERR Path '{}' does not exist", path)
107+
}
108+
109+
pub(crate) fn err_msg_json_path_doesnt_exist() -> String {
110+
"ERR Path does not exist".to_string()
111+
}
112+
113+
pub(crate) fn err_msg_json_path_doesnt_exist_with_param_or(path: &str, or: &str) -> String {
114+
format!("ERR Path '{}' does not exist or {}", path, or)
115+
}
116+
99117
pub struct KeyHolderWrite<'a> {
100118
key: RedisKeyWritable,
101119
key_name: RedisString,
@@ -203,7 +221,7 @@ impl<'a> KeyHolderWrite<'a> {
203221
Ok(res.clone())
204222
})?;
205223
match res {
206-
None => Err(RedisError::Str("path does not exists")),
224+
None => Err(RedisError::String(err_msg_json_path_doesnt_exist())),
207225
Some(n) => match n {
208226
Value::Number(n) => Ok(n),
209227
_ => Err(RedisError::Str("return value is not a number")),
@@ -341,7 +359,7 @@ impl<'a> WriteHolder<Value, Value> for KeyHolderWrite<'a> {
341359
Ok(Some(Value::Bool(val)))
342360
})?;
343361
match res {
344-
None => Err(RedisError::Str("path does not exists")),
362+
None => Err(RedisError::String(err_msg_json_path_doesnt_exist())),
345363
Some(n) => Ok(n),
346364
}
347365
}
@@ -356,13 +374,13 @@ impl<'a> WriteHolder<Value, Value> for KeyHolderWrite<'a> {
356374
Ok(Some(Value::String(new_str)))
357375
})?;
358376
match res {
359-
None => Err(RedisError::Str("path does not exists")),
377+
None => Err(RedisError::String(err_msg_json_path_doesnt_exist())),
360378
Some(l) => Ok(l),
361379
}
362380
} else {
363-
Err(RedisError::String(format!(
364-
"ERR wrong type of value - expected string but found {}",
365-
val
381+
Err(RedisError::String(err_msg_json_expected(
382+
"string",
383+
val.as_str(),
366384
)))
367385
}
368386
}
@@ -376,7 +394,7 @@ impl<'a> WriteHolder<Value, Value> for KeyHolderWrite<'a> {
376394
Ok(Some(v))
377395
})?;
378396
match res {
379-
None => Err(RedisError::Str("path does not exists")),
397+
None => Err(RedisError::String(err_msg_json_path_doesnt_exist())),
380398
Some(n) => Ok(n),
381399
}
382400
}
@@ -403,7 +421,7 @@ impl<'a> WriteHolder<Value, Value> for KeyHolderWrite<'a> {
403421
Ok(Some(new_value))
404422
})?;
405423
match res {
406-
None => Err(RedisError::Str("path does not exists")),
424+
None => Err(RedisError::String(err_msg_json_path_doesnt_exist())),
407425
Some(l) => Ok(l),
408426
}
409427
}
@@ -461,7 +479,7 @@ impl<'a> WriteHolder<Value, Value> for KeyHolderWrite<'a> {
461479
}
462480
})?;
463481
match res {
464-
None => Err(RedisError::Str("path does not exists")),
482+
None => Err(RedisError::String(err_msg_json_path_doesnt_exist())),
465483
Some(l) => Ok(l),
466484
}
467485
}

0 commit comments

Comments
 (0)