diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4fd441dd..0e106d345 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,7 +148,7 @@ jobs: ./dragonfly_test ./multi_test --multi_exec_mode=1 ./multi_test --multi_exec_mode=3 - # GLOG_logtostderr=1 GLOG_vmodule=transaction=1,engine_shard_set=1 CTEST_OUTPUT_ON_FAILURE=1 ninja server/test + ./json_family_test --jsonpathv2 - name: Upload unit logs on failure if: failure() diff --git a/src/core/json/jsonpath_test.cc b/src/core/json/jsonpath_test.cc index 191d7c996..0223fbc6b 100644 --- a/src/core/json/jsonpath_test.cc +++ b/src/core/json/jsonpath_test.cc @@ -30,6 +30,12 @@ class TestDriver : public Driver { } }; +inline JsonType ValidJson(string_view str) { + auto res = ::dfly::JsonFromString(str, pmr::get_default_resource()); + CHECK(res) << "Failed to parse json: " << str; + return *res; +} + class JsonPathTest : public ::testing::Test { protected: JsonPathTest() { @@ -123,6 +129,19 @@ TEST_F(JsonPathTest, Parser) { EXPECT_EQ(0, Parse("$.plays[*].game")); } +TEST_F(JsonPathTest, Root) { + JsonType json = ValidJson(R"({"foo" : 1, "bar": "str" })"); + ASSERT_EQ(0, Parse("$")); + Path path = driver_.TakePath(); + int called = 0; + EvaluatePath(path, json, [&](optional, const JsonType& val) { + ++called; + ASSERT_TRUE(val.is_object()); + ASSERT_EQ(json, val); + }); + ASSERT_EQ(1, called); +} + TEST_F(JsonPathTest, Functions) { ASSERT_EQ(0, Parse("max($.plays[*].score)")); Path path = driver_.TakePath(); @@ -131,9 +150,7 @@ TEST_F(JsonPathTest, Functions) { EXPECT_THAT(path[1], SegType(SegmentType::IDENTIFIER)); EXPECT_THAT(path[2], SegType(SegmentType::WILDCARD)); EXPECT_THAT(path[3], SegType(SegmentType::IDENTIFIER)); - JsonType json = - JsonFromString(R"({"plays": [{"score": 1}, {"score": 2}]})", pmr::get_default_resource()) - .value(); + JsonType json = ValidJson(R"({"plays": [{"score": 1}, {"score": 2}]})"); int called = 0; EvaluatePath(path, json, [&](auto, const JsonType& val) { ASSERT_TRUE(val.is()); @@ -163,16 +180,15 @@ TEST_F(JsonPathTest, Descent) { TEST_F(JsonPathTest, Path) { Path path; - JsonType json = JsonFromString(R"({"v11":{ "f" : 1, "a2": [0]}, "v12": {"f": 2, "a2": [1]}, + JsonType json = ValidJson(R"({"v11":{ "f" : 1, "a2": [0]}, "v12": {"f": 2, "a2": [1]}, "v13": 3 - })", - pmr::get_default_resource()) - .value(); + })"); int called = 0; // Empty path EvaluatePath(path, json, [&](optional, const JsonType& val) { ++called; }); - ASSERT_EQ(0, called); + ASSERT_EQ(1, called); + called = 0; path.emplace_back(SegmentType::IDENTIFIER, "v13"); EvaluatePath(path, json, [&](optional key, const JsonType& val) { @@ -206,11 +222,10 @@ TEST_F(JsonPathTest, Path) { } TEST_F(JsonPathTest, EvalDescent) { - JsonType json = JsonFromString(R"( + JsonType json = ValidJson(R"( {"v11":{ "f" : 1, "a2": [0]}, "v12": {"f": 2, "v21": {"f": 3, "a2": [1]}}, "v13": { "a2" : { "b" : {"f" : 4}}} - })", - pmr::get_default_resource()); + })"); Path path; @@ -241,10 +256,9 @@ TEST_F(JsonPathTest, EvalDescent) { }); ASSERT_EQ(4, called); - json = JsonFromString(R"( + json = ValidJson(R"( {"a":[7], "inner": {"a": {"b": 2, "c": 1337}}} - )", - pmr::get_default_resource()); + )"); path.pop_back(); path.emplace_back(SegmentType::IDENTIFIER, "a"); @@ -263,7 +277,7 @@ TEST_F(JsonPathTest, Wildcard) { ASSERT_EQ(1, path.size()); EXPECT_THAT(path[0], SegType(SegmentType::WILDCARD)); - JsonType json = JsonFromString(R"([1, 2, 3])", pmr::get_default_resource()).value(); + JsonType json = ValidJson(R"([1, 2, 3])"); vector arr; EvaluatePath(path, json, [&](optional key, const JsonType& val) { ASSERT_FALSE(key); @@ -273,7 +287,7 @@ TEST_F(JsonPathTest, Wildcard) { } TEST_F(JsonPathTest, Mutate) { - JsonType json = JsonFromString(R"([1, 2, 3, 5, 6])", pmr::get_default_resource()).value(); + JsonType json = ValidJson(R"([1, 2, 3, 5, 6])"); ASSERT_EQ(0, Parse("$[*]")); Path path = driver_.TakePath(); MutateCallback cb = [&](optional, JsonType* val) { @@ -289,10 +303,9 @@ TEST_F(JsonPathTest, Mutate) { } ASSERT_THAT(arr, ElementsAre(2, 3, 4, 6, 7)); - json = JsonFromString(R"( + json = ValidJson(R"( {"a":[7], "inner": {"a": {"bool": true, "c": 42}}} - )", - pmr::get_default_resource()); + )"); ASSERT_EQ(0, Parse("$..a.*")); path = driver_.TakePath(); MutatePath( diff --git a/src/core/json/path.cc b/src/core/json/path.cc index bc56d58a4..bf0e33537 100644 --- a/src/core/json/path.cc +++ b/src/core/json/path.cc @@ -432,8 +432,10 @@ JsonType PathSegment::GetResult() const { } void EvaluatePath(const Path& path, const JsonType& json, PathCallback callback) { - if (path.empty()) + if (path.empty()) { // root node + callback(nullopt, json); return; + } if (path.front().type() != SegmentType::FUNCTION) { Dfs().Traverse(path, json, std::move(callback)); diff --git a/src/server/json_family.cc b/src/server/json_family.cc index 3644b51a1..75c661b1a 100644 --- a/src/server/json_family.cc +++ b/src/server/json_family.cc @@ -65,6 +65,17 @@ inline void Evaluate(const json::Path& expr, const JsonType& obj, ExprCallback c }); } +inline JsonType Evaluate(const JsonExpression& expr, const JsonType& obj) { + return expr.evaluate(obj); +} + +inline JsonType Evaluate(const json::Path& expr, const JsonType& obj) { + JsonType res(json_array_arg); + json::EvaluatePath(expr, obj, + [&res](optional, const JsonType& val) { res.push_back(val); }); + return res; +} + inline OpStatus JsonReplaceVerifyNoOp(JsonType&) { return OpStatus::OK; } @@ -418,7 +429,7 @@ void SendJsonValue(RedisReplyBuilder* rb, const JsonType& j) { } OpResult OpJsonGet(const OpArgs& op_args, string_view key, - const vector>>& expressions, + const vector>>& expressions, bool should_format, const OptString& indent, const OptString& new_line, const OptString& space) { OpResult result = GetJson(op_args, key); @@ -456,16 +467,17 @@ OpResult OpJsonGet(const OpArgs& op_args, string_view key, } } - auto eval_wrapped = [&json_entry](const optional& expr) { - return expr ? expr->evaluate(json_entry) : json_entry; + auto eval_wrapped = [&json_entry](const optional& expr) -> JsonType { + return expr ? visit([&](auto& arg) { return Evaluate(arg, json_entry); }, *expr) : json_entry; }; JsonType out{json_object_arg}; // see https://github.com/danielaparker/jsoncons/issues/482 if (expressions.size() == 1) { out = eval_wrapped(expressions[0].second); } else { - for (auto& [expr_str, expr] : expressions) + for (const auto& [expr_str, expr] : expressions) { out[expr_str] = eval_wrapped(expr); + } } if (should_format) { @@ -1826,7 +1838,9 @@ void JsonFamily::Get(CmdArgList args, ConnectionContext* cntx) { OptString indent; OptString new_line; OptString space; - vector>> expressions; + + // '.' corresponds to the legacy, non-array format and is passed as nullopt. + vector>> expressions; while (parser.HasNext()) { if (parser.Check("SPACE").IgnoreCase().ExpectTail(1)) { @@ -1842,17 +1856,12 @@ void JsonFamily::Get(CmdArgList args, ConnectionContext* cntx) { continue; } - optional expr; + optional expr; string_view expr_str = parser.Next(); if (expr_str != ".") { - io::Result res = ParseJsonPath(expr_str); - if (!res) { - LOG(WARNING) << "path '" << expr_str - << "': Invalid JSONPath syntax: " << res.error().message(); - return cntx->SendError(kSyntaxErr); - } - expr.emplace(std::move(*res)); + JsonPathV2 expression = PARSE_PATHV2(expr_str); + expr.emplace(std::move(expression)); } expressions.emplace_back(expr_str, std::move(expr)); diff --git a/src/server/json_family_test.cc b/src/server/json_family_test.cc index 856e65970..f3a96232a 100644 --- a/src/server/json_family_test.cc +++ b/src/server/json_family_test.cc @@ -276,6 +276,9 @@ TEST_F(JsonFamilyTest, Toggle) { auto resp = Run({"JSON.SET", "json", ".", json}); ASSERT_THAT(resp, "OK"); + resp = Run({"JSON.GET", "json", "$.*"}); + EXPECT_EQ(R"([true,false,1,null,"foo",[],{}])", resp); + resp = Run({"JSON.TOGGLE", "json", "$.*"}); ASSERT_THAT(resp, ArgType(RespExpr::ARRAY)); EXPECT_THAT(resp.GetVec(),