mirror of
https://github.com/dragonflydb/dragonfly.git
synced 2025-05-11 18:35:46 +02:00
fix: JSON.STRAPPEND (#3597)
* fix: JSON.STRAPPEND JSON.STRAPPEND was completely broken. First, it accepts exactly 3 arguments, i.e. a single value to append. Secondly, the value must be a json string, not the regular string. Meaning it must be in double quotes. So, before we parsed: `JSON.STRAPPEND key $.field bar` and now we parse: `JSON.STRAPPEND key $.field "bar"` In addition fixed the behavior of JSON.STRLEN to return "no such key" error in case the json key does not exist and path is specified. --------- Signed-off-by: Roman Gershman <roman@dragonflydb.io>
This commit is contained in:
Roman Gershman
GitHub
parent
88229cf365
commit
b74bd5a7b2
3 changed files with 58 additions and 46 deletions
|
|
@ -822,24 +822,23 @@ auto OpObjKeys(const OpArgs& op_args, string_view key, const WrappedJsonPath& js
|
|||
return JsonEvaluateOperation<StringVec>(op_args, key, json_path, std::move(cb), {true, true});
|
||||
}
|
||||
|
||||
auto OpStrAppend(const OpArgs& op_args, string_view key, const WrappedJsonPath& path,
|
||||
facade::ArgRange strs) {
|
||||
using StrAppendResult = std::optional<std::size_t>;
|
||||
|
||||
OpResult<JsonCallbackResult<StrAppendResult>> OpStrAppend(const OpArgs& op_args, string_view key,
|
||||
const WrappedJsonPath& path,
|
||||
string_view value) {
|
||||
auto cb = [&](std::optional<std::string_view>,
|
||||
JsonType* val) -> MutateCallbackResult<std::optional<std::size_t>> {
|
||||
if (val->is_string()) {
|
||||
string new_val = val->as_string();
|
||||
for (string_view str : strs) {
|
||||
new_val += str;
|
||||
}
|
||||
JsonType* val) -> MutateCallbackResult<StrAppendResult> {
|
||||
if (!val->is_string())
|
||||
return {false, std::nullopt};
|
||||
|
||||
*val = new_val;
|
||||
return {false, new_val.size()};
|
||||
}
|
||||
|
||||
return {false, std::nullopt};
|
||||
string new_val = absl::StrCat(val->as_string_view(), value);
|
||||
size_t len = new_val.size();
|
||||
*val = std::move(new_val);
|
||||
return {false, len}; // do not delete, new value len
|
||||
};
|
||||
|
||||
return UpdateEntry<std::optional<std::size_t>>(op_args, key, path, std::move(cb));
|
||||
return UpdateEntry<StrAppendResult>(op_args, key, path, std::move(cb));
|
||||
}
|
||||
|
||||
// Returns the numbers of values cleared.
|
||||
|
|
@ -1476,7 +1475,8 @@ void JsonFamily::ArrIndex(CmdArgList args, ConnectionContext* cntx) {
|
|||
|
||||
WrappedJsonPath json_path = GET_OR_SEND_UNEXPECTED(ParseJsonPath(path));
|
||||
|
||||
optional<JsonType> search_value = JsonFromString(ArgS(args, 2));
|
||||
optional<JsonType> search_value =
|
||||
dfly::JsonFromString(ArgS(args, 2), PMR_NS::get_default_resource());
|
||||
if (!search_value) {
|
||||
cntx->SendError(kSyntaxErr);
|
||||
return;
|
||||
|
|
@ -1651,12 +1651,19 @@ void JsonFamily::Clear(CmdArgList args, ConnectionContext* cntx) {
|
|||
void JsonFamily::StrAppend(CmdArgList args, ConnectionContext* cntx) {
|
||||
string_view key = ArgS(args, 0);
|
||||
string_view path = ArgS(args, 1);
|
||||
string_view value = ArgS(args, 2);
|
||||
|
||||
WrappedJsonPath json_path = GET_OR_SEND_UNEXPECTED(ParseJsonPath(path));
|
||||
auto strs = args.subspan(2);
|
||||
|
||||
// We try parsing the value into json string object first.
|
||||
optional<JsonType> parsed_json = dfly::JsonFromString(value, PMR_NS::get_default_resource());
|
||||
if (!parsed_json || !parsed_json->is_string()) {
|
||||
return cntx->SendError("expected string value", kSyntaxErrType);
|
||||
};
|
||||
|
||||
string_view json_string = parsed_json->as_string_view();
|
||||
auto cb = [&](Transaction* t, EngineShard* shard) {
|
||||
return OpStrAppend(t->GetOpArgs(shard), key, json_path, facade::ArgRange{strs});
|
||||
return OpStrAppend(t->GetOpArgs(shard), key, json_path, json_string);
|
||||
};
|
||||
|
||||
Transaction* trans = cntx->transaction;
|
||||
|
|
@ -1903,7 +1910,7 @@ void JsonFamily::Register(CommandRegistry* registry) {
|
|||
*registry << CI{"JSON.FORGET", CO::WRITE, -2, 1, 1, acl::JSON}.HFUNC(
|
||||
Del); // An alias of JSON.DEL.
|
||||
*registry << CI{"JSON.OBJKEYS", CO::READONLY | CO::FAST, -2, 1, 1, acl::JSON}.HFUNC(ObjKeys);
|
||||
*registry << CI{"JSON.STRAPPEND", CO::WRITE | CO::DENYOOM | CO::FAST, -4, 1, 1, acl::JSON}.HFUNC(
|
||||
*registry << CI{"JSON.STRAPPEND", CO::WRITE | CO::DENYOOM | CO::FAST, 4, 1, 1, acl::JSON}.HFUNC(
|
||||
StrAppend);
|
||||
*registry << CI{"JSON.CLEAR", CO::WRITE | CO::FAST, -2, 1, 1, acl::JSON}.HFUNC(Clear);
|
||||
*registry << CI{"JSON.ARRPOP", CO::WRITE | CO::FAST, -2, 1, 1, acl::JSON}.HFUNC(ArrPop);
|
||||
|
|
|
|||
|
|
@ -1329,7 +1329,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
|
||||
/* Test simple response from only one value */
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.a", "a", "b"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.a", "\"ab\""});
|
||||
EXPECT_THAT(resp, IntArg(3));
|
||||
|
||||
resp = Run({"JSON.GET", "json"});
|
||||
|
|
@ -1337,7 +1337,9 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp,
|
||||
R"({"a":{"a":"aab"},"b":{"a":"a","b":1},"c":{"a":"a","b":"bb"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", "a"});
|
||||
const char kVal[] = "\"a\"";
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", kVal});
|
||||
EXPECT_THAT(resp, IntArg(4));
|
||||
|
||||
resp = Run({"JSON.GET", "json"});
|
||||
|
|
@ -1345,7 +1347,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp,
|
||||
R"({"a":{"a":"aaba"},"b":{"a":"a","b":1},"c":{"a":"a","b":"bb"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.c.b", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.c.b", kVal});
|
||||
EXPECT_THAT(resp, IntArg(3));
|
||||
|
||||
resp = Run({"JSON.GET", "json"});
|
||||
|
|
@ -1358,7 +1360,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
In JSON V2, the response is an array of all possible values
|
||||
*/
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(2), ArgType(RespExpr::NIL)));
|
||||
|
||||
|
|
@ -1367,7 +1369,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp,
|
||||
R"({"a":{"a":"aaba"},"b":{"a":"aa","b":1},"c":{"a":"a","b":"bba"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.c.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.c.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(2), IntArg(4)));
|
||||
|
||||
|
|
@ -1376,7 +1378,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp,
|
||||
R"({"a":{"a":"aaba"},"b":{"a":"aa","b":1},"c":{"a":"aa","b":"bbaa"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.d.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.d.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(),
|
||||
ElementsAre(ArgType(RespExpr::NIL), IntArg(2), ArgType(RespExpr::NIL)));
|
||||
|
|
@ -1393,14 +1395,14 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(2), IntArg(3), IntArg(4)));
|
||||
|
||||
resp = Run({"JSON.GET", "json"});
|
||||
EXPECT_EQ(resp, R"({"a":{"a":"aa","b":"aaa","c":"aaaa"},"b":{"a":"aaa","b":"aa","c":"a"}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(4), IntArg(3), IntArg(2)));
|
||||
|
||||
|
|
@ -1414,7 +1416,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(2), IntArg(3), ArgType(RespExpr::NIL)));
|
||||
|
||||
|
|
@ -1423,7 +1425,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp,
|
||||
R"({"a":{"a":"aa","b":"aaa","c":["aaaaa","aaaaa"]},"b":{"a":"aaa","b":["aaaaa","aaaaa"],"c":"a"}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(4), ArgType(RespExpr::NIL), IntArg(2)));
|
||||
|
||||
|
|
@ -1439,7 +1441,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(2), IntArg(3), ArgType(RespExpr::NIL)));
|
||||
|
||||
|
|
@ -1448,7 +1450,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp,
|
||||
R"({"a":{"a":"aa","b":"aaa","c":{"c":"aaaaa"}},"b":{"a":"aaa","b":{"b":"aaaaa"},"c":"a"}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$.b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(4), ArgType(RespExpr::NIL), IntArg(2)));
|
||||
|
||||
|
|
@ -1464,7 +1466,7 @@ TEST_F(JsonFamilyTest, StrAppend) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$..a", "bar"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "$..a", "\"bar\""});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
|
||||
EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(6), IntArg(6), ArgType(RespExpr::NIL)));
|
||||
|
||||
|
|
@ -1482,7 +1484,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
|
||||
/* Test simple response from only one value */
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.a", "a", "b"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.a", "\"ab\""});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(3));
|
||||
|
||||
|
|
@ -1491,7 +1493,9 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp,
|
||||
R"({"a":{"a":"aab"},"b":{"a":"a","b":1},"c":{"a":"a","b":"bb"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", "a"});
|
||||
const char kVal[] = "\"a\"";
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(4));
|
||||
|
||||
|
|
@ -1500,7 +1504,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp,
|
||||
R"({"a":{"a":"aaba"},"b":{"a":"a","b":1},"c":{"a":"a","b":"bb"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".c.b", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".c.b", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(3));
|
||||
|
||||
|
|
@ -1515,7 +1519,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
string.
|
||||
*/
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(2));
|
||||
|
||||
|
|
@ -1524,7 +1528,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp,
|
||||
R"({"a":{"a":"aaba"},"b":{"a":"aa","b":1},"c":{"a":"a","b":"bba"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".c.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".c.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(4));
|
||||
|
||||
|
|
@ -1533,7 +1537,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp,
|
||||
R"({"a":{"a":"aaba"},"b":{"a":"aa","b":1},"c":{"a":"aa","b":"bbaa"},"d":{"a":1,"b":"b","c":3}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".d.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".d.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(2));
|
||||
|
||||
|
|
@ -1549,14 +1553,14 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(4));
|
||||
|
||||
resp = Run({"JSON.GET", "json"});
|
||||
EXPECT_EQ(resp, R"({"a":{"a":"aa","b":"aaa","c":"aaaa"},"b":{"a":"aaa","b":"aa","c":"a"}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(2));
|
||||
|
||||
|
|
@ -1570,7 +1574,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(3));
|
||||
|
||||
|
|
@ -1579,7 +1583,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp,
|
||||
R"({"a":{"a":"aa","b":"aaa","c":["aaaaa","aaaaa"]},"b":{"a":"aaa","b":["aaaaa","aaaaa"],"c":"a"}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(2));
|
||||
|
||||
|
|
@ -1595,7 +1599,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".a.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(3));
|
||||
|
||||
|
|
@ -1604,7 +1608,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp,
|
||||
R"({"a":{"a":"aa","b":"aaa","c":{"c":"aaaaa"}},"b":{"a":"aaa","b":{"b":"aaaaa"},"c":"a"}})");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", "a"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", ".b.*", kVal});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(2));
|
||||
|
||||
|
|
@ -1620,7 +1624,7 @@ TEST_F(JsonFamilyTest, StrAppendLegacyMode) {
|
|||
resp = Run({"JSON.SET", "json", ".", json});
|
||||
EXPECT_EQ(resp, "OK");
|
||||
|
||||
resp = Run({"JSON.STRAPPEND", "json", "..a", "bar"});
|
||||
resp = Run({"JSON.STRAPPEND", "json", "..a", "\"bar\""});
|
||||
ASSERT_THAT(resp, ArgType(RespExpr::INT64));
|
||||
EXPECT_THAT(resp, IntArg(6));
|
||||
|
||||
|
|
|
|||
|
|
@ -321,8 +321,9 @@ def test_jsonstrlen(r: redis.Redis):
|
|||
assert r.json().strlen("doc1", "$.nested2.a") == [None]
|
||||
|
||||
# Test missing key
|
||||
with pytest.raises(redis.ResponseError):
|
||||
r.json().strlen("non_existing_doc", "$..a")
|
||||
# Note: Dragonfly returns NIL in the accordance to the official docs
|
||||
# with pytest.raises(redis.ResponseError):
|
||||
# r.json().strlen("non_existing_doc", "$..a")
|
||||
|
||||
|
||||
def test_toggle(r: redis.Redis):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue