mirrors/dragonfly
1
0
Fork 0
mirror of https://github.com/dragonflydb/dragonfly.git synced 2025-05-11 10:25:47 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

fix(json_family): Fix JSON.SET handling for nested fields (#4710)

Browse source 
fixes dragonflydb#4381

Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
This commit is contained in:
Stepan Bagritsevich 2025-03-06 09:53:23 +01:00 • committed by GitHub
parent faf626eed4
commit 5ffe939b3d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 65 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/core/json/jsonpath_lexer.lex
View file

@ -59,7 +59,7 @@
return Parser::make_INT(val, loc());
}
\w[\w_\-]* return Parser::make_UNQ_STR(str(), loc());
[\w_\-]+ return Parser::make_UNQ_STR(str(), loc());
<<EOF>> return Parser::make_YYEOF(loc());
. throw Parser::syntax_error(loc(), UnknownTokenMsg());
%%

4
src/server/detail/wrapped_json_path.h
View file

@ -269,6 +269,10 @@ class WrappedJsonPath {
return std::get<JsonExpression>(parsed_path_);
}
std::string_view Path() const {
return path_.view();
}
private:
CallbackResultOptions InitializePathType(CallbackResultOptions options) const {
if (!options.path_type) {

84
src/server/json_family.cc
View file

@ -50,7 +50,6 @@ using facade::RedisReplyBuilder;
using facade::SinkReplyBuilder;
using JsonExpression = jsonpath::jsonpath_expression<JsonType>;
using JsonReplaceVerify = std::function<void(JsonType&)>;
using CI = CommandId;
namespace {
@ -524,40 +523,38 @@ string ConvertToJsonPointer(string_view json_path) {
return result;
}
std::optional<std::string> ConvertExpressionToJsonPointer(string_view json_path) {
if (json_path.empty()) {
VLOG(1) << "retrieved malformed JSON path expression: " << json_path;
/* Converts a JSONPath to a JSONPointer.
E.g. $[a][b][0] -> /a/b/0.
V1 JSONPath is not supported. */
std::optional<std::string> ConvertJsonPathToJsonPointer(string_view json_path) {
auto parsed_path = json::ParsePath(json_path);
if (!parsed_path) {
VLOG(2) << "Error during conversion of JSONPath to JSONPointer: " << json_path
<< ". Invalid JSONPath.";
return std::nullopt;
}
// Remove prefix
if (json_path.front() == '$') {
json_path.remove_prefix(1);
}
if (json_path.front() == '.') {
json_path.remove_prefix(1);
}
DCHECK(json_path.length());
std::string pointer;
vector<string> splitted = absl::StrSplit(json_path, '.');
for (auto& it : splitted) {
if (it.front() == '[' && it.back() == ']') {
std::string index = it.substr(1, it.size() - 2);
if (index.empty()) {
const auto& path = parsed_path.value();
for (const auto& node : path) {
const auto& type = node.type();
if (type == json::SegmentType::IDENTIFIER) {
pointer += '/' + node.identifier();
} else if (type == json::SegmentType::INDEX) {
const auto& index = node.index();
if (index.first != index.second) {
VLOG(2) << "Error during conversion of JSONPath to JSONPointer: " << json_path
<< ". Index range is not supported.";
return std::nullopt;
}
for (char ch : index) {
if (!std::isdigit(ch)) {
return std::nullopt;
}
}
pointer += '/' + index;
pointer += '/' + std::to_string(node.index().first);
} else {
pointer += '/' + it;
VLOG(2) << "Error during conversion of JSONPath to JSONPointer: " << json_path
<< ". Unsupported segment type.";
return std::nullopt;
}
}
@ -613,7 +610,9 @@ OpResult<JsonCallbackResult<T>> JsonEvaluateOperation(const OpArgs& op_args, std
}
struct MutateOperationOptions {
JsonReplaceVerify verify_op;
using PostMutateCallback = absl::FunctionRef<OpStatus(JsonType*)>;
std::optional<PostMutateCallback> post_mutate_cb;
CallbackResultOptions cb_result_options = CallbackResultOptions::DefaultMutateOptions();
};
@ -638,9 +637,13 @@ OpResult<JsonCallbackResult<optional<T>>> JsonMutateOperation(const OpArgs& op_a
auto mutate_res = json_path.Mutate(json_val, cb, options.cb_result_options);
// Make sure that we don't have other internal issue with the operation
if (mutate_res && options.verify_op) {
options.verify_op(*json_val);
// Call post mutate callback
if (mutate_res && options.post_mutate_cb) {
auto res = options.post_mutate_cb.value()(json_val);
// We can not return result here, because we need to update the size
if (res != OpStatus::OK) {
mutate_res = res;
}
}
// we need to manually run this before the PostUpdater run
@ -648,7 +651,6 @@ OpResult<JsonCallbackResult<optional<T>>> JsonMutateOperation(const OpArgs& op_a
it_res->post_updater.Run();
op_args.shard->search_indices()->AddDoc(key, op_args.db_cntx, pv);
RETURN_ON_BAD_STATUS(mutate_res);
return mutate_res;
}
@ -1349,7 +1351,8 @@ OpResult<bool> OpSet(const OpArgs& op_args, string_view key, string_view path,
}
const JsonType& new_json = parsed_json.value();
auto cb = [&](std::optional<std::string_view>, JsonType* val) -> MutateCallbackResult<> {
// If the path exists, this callback will be called
auto mutate_cb = [&](std::optional<std::string_view>, JsonType* val) -> MutateCallbackResult<> {
path_exists = true;
if (!is_nx_condition) {
operation_result = true;
@ -1359,18 +1362,17 @@ OpResult<bool> OpSet(const OpArgs& op_args, string_view key, string_view path,
return {};
};
auto inserter = [&](JsonType& json) {
// If the path doesn't exist, this callback will be called
auto insert_cb = [&](JsonType* json) {
// Set a new value if the path doesn't exist and the xx condition is not set.
if (!path_exists && !is_xx_condition) {
auto pointer = ConvertExpressionToJsonPointer(path);
auto pointer = ConvertJsonPathToJsonPointer(json_path.Path());
if (!pointer) {
VLOG(1) << "Failed to convert the following expression path to a valid JSON pointer: "
<< path;
return OpStatus::SYNTAX_ERR;
}
error_code ec;
jsoncons::jsonpointer::add(json, pointer.value(), new_json, ec);
std::error_code ec;
jsoncons::jsonpointer::add(*json, pointer.value(), new_json, ec);
if (ec) {
VLOG(1) << "Failed to add a JSON value to the following path: " << path
<< " with the error: " << ec.message();
@ -1386,8 +1388,8 @@ OpResult<bool> OpSet(const OpArgs& op_args, string_view key, string_view path,
// JsonMutateOperation uses it's own JsonMemTracker. It will work, because updates to already
// existing json keys use copy assign, so we don't really need to account for the memory
// allocated by ShardJsonFromString above since it's not being moved here at all.
auto res = JsonMutateOperation<Nothing>(op_args, key, json_path, std::move(cb),
MutateOperationOptions{std::move(inserter)});
auto res = JsonMutateOperation<Nothing>(op_args, key, json_path, std::move(mutate_cb),
MutateOperationOptions{std::move(insert_cb)});
RETURN_ON_BAD_STATUS(res);
return operation_result;

17
src/server/json_family_test.cc
View file

@ -3046,4 +3046,21 @@ TEST_F(JsonFamilyTest, MaxNestingJsonDepth) {
EXPECT_THAT(resp, ErrArg("failed to parse JSON"));
}
TEST_F(JsonFamilyTest, SetNestedFields) {
auto resp = Run({"JSON.SET", "json", "$", "{}"});
EXPECT_THAT(resp, "OK");
resp = Run({"JSON.SET", "json", "$['field1']", "1"});
EXPECT_THAT(resp, "OK");
resp = Run({"JSON.GET", "json"});
EXPECT_EQ(resp, R"({"field1":1})");
resp = Run({"JSON.SET", "json", "$['-field2']", "2"});
EXPECT_THAT(resp, "OK");
resp = Run({"JSON.GET", "json"});
EXPECT_EQ(resp, R"({"-field2":2,"field1":1})");
}
} // namespace dfly